need AMQP 1.0 crate for Service Bus

[cataggar]

There does not appear to be an AMQP 1.0 spec compliant crate. Crates like https://github.com/amqp-rs/lapin implement 0.9.1, but Service Bus requires 1.0. https://docs.microsoft.com/en-us/azure/service-bus-messaging/service-bus-amqp-overview

[andrei-ionescu]

What about Dove?

• https://github.com/lulf/dove
• https://crates.io/crates/dove

[cataggar]

There is also https://github.com/ntex-rs/ntex-amqp. Looks like there are contributors who work at Microsoft ntex-rs/ntex-amqp#15.

[minghuaw]

I have been working on a tokio and serde based AMQP 1.0 implementation called fe2o3-amqp https://github.com/minghuaw/fe2o3-amqp , and I am planning to start working on the qpid integration test soon. It might be worth a try, and any feedback would be really appreciated.

[minghuaw]

How much of the AMQP 1.0 spec is needed for Azure SDK? I have currently implemented most of the core spec (both client and server) as well as the web socket binding (only the client), and I am currently debating which extension spec to implement next.

[minghuaw]

I have made two working examples showing how to use fe2o3-amqp with Azure Service Bus, one over TLS and the other one over WebSocket. Both have been tested with my Azure free trial.

1. https://github.com/minghuaw/fe2o3-amqp/tree/main/examples/service_bus
2. https://github.com/minghuaw/fe2o3-amqp/tree/main/examples/service_bus_over_websocket

I might start working on implementing the claim based security next. Would love to have some feedback :)

[minghuaw]

Here is another quick update. I have added some working examples of sending to and receiving from Azure Event Hubs.

• https://github.com/minghuaw/fe2o3-amqp/tree/main/examples/event_hubs

The receiver example, however, seems like it's not moving the offset with an Accepted disposition.

[minghuaw]

I think I figured out the offset issue with the filter example https://github.com/minghuaw/fe2o3-amqp/blob/main/examples/event_hubs/src/bin/receiver_with_filter.rs

[minghuaw]

I am planning to work on an amqp client for servicebus and event hub. Is there any recommendations? Would mimicking the dotnet sdk be a good starting point?

[cataggar]

Sounds great @minghuaw! Yes, mimicking https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/servicebus/Azure.Messaging.ServiceBus and not the older https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/servicebus/Microsoft.Azure.ServiceBus is the way to go.

[minghuaw]

A quick question on getter/setter convention.

Is there any preference on the getter/setter naming convention? The rust official naming convention guide (https://rust-lang.github.io/api-guidelines/naming.html#getter-names-follow-rust-convention-c-getter) suggests xxx() and xxx_mut() (possibly set_xxx()) instead of get_xxx() and set_xxx().

[rylev]

Yes, I believe we want to follow the conventions laid out in that guide.

[minghuaw]

Is there any public documentation on how these servicebus-specific message annotations (https://docs.microsoft.com/en-us/azure/service-bus-messaging/service-bus-amqp-protocol-guide#message-annotations) are encoded in AMQP?

[minghuaw]

@cataggar Is there a public spec on how Service Bus session ID should be encoded? It seems like the current dotnet SDK (Azure.Messaging.ServiceBus) impl doesn't really comply with the AMQP 1.0 core spec.

The way it is implemented in Azure.Messaging.ServiceBus is like following (Line 681)

// even if supplied sessionId is null, we need to add the Session filter if it is a session receiver
if (isSessionReceiver)
{
    filters.Add(AmqpClientConstants.SessionFilterName, sessionId);
}

var linkSettings = new AmqpLinkSettings
{
    Source = new Source { Address = endpoint.AbsolutePath, FilterSet = filters }, // Line 690
    // ... other setting
};

where sessionId is just a string. The sessionId is added as one entry to the filter-set field of the receiver link's Source

However, regarding the filter-set type, the AMQP 1.0 core spec states that (http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-filter-set)

A set of named filters. Every key in the map MUST be of type symbol, every value MUST be either null or of a described type which provides the archetype filter.

and string is not a described type. The entry may still be serialized as a described type, but I was unable to find a spec or code that does so.

For the rust implementation, this could be worked around by kinda hacking the upstream type definition (relax the value type from Described<Value> to just Value, https://github.com/minghuaw/fe2o3-amqp/blob/40c78b8e435edc9e69a5734508a55e8a6bc919a6/fe2o3-amqp-types/src/messaging/mod.rs#L61). I am debating whether to make that change in the upstream though.

[minghuaw]

The discussion about the session ID in Azure/azure-amqp can be found in this issue: Azure/azure-amqp#231

[minghuaw]

The discussion about the session ID in Azure/azure-amqp can be found in this issue: Azure/azure-amqp#231

The answer regarding session filter is copied from that issue just for reference:

<type name="com.microsoft:session-filter" class="restricted" source="string" provides="filter">
    <descriptor name="com.microsoft:session-filter" code="0x00000137:000000C"/>
</type>

[minghuaw]

@cataggar When is the planned next release date? I just want to see whether I would be able to have something before the next release.

I almost have a minimal viable impl for all APIs except for the ones related to processors. I haven't got the time to write enough tests either.

[bmc-msft]

We work towards a monthly release cadence.

[minghuaw]

I have a working partial AMQP 1.0 implementation that currently includes:

1. ServiceBusClient
2. ServiceBusSender
3. ServiceBusReceiver
4. ServiceBusSessionReceiver

What remains to be implemented include (in no particular order):

1. Transaction
2. ServiceBusRuleManager
3. ServiceBusProcessor
4. ServiceBusSessionProcessor
5. Azure.Messaging.ServiceBus.Administration (this might need a separate crate or gated behind a feature).
6. Diagnostics and metrics

These implemented APIs cover roughly the same "surface area" provided by the golang SDK (azservicebus). And the implementation should look very similar to that of the dotnet SDK.

I have only run some quick live tests with my own service bus namespace, so I definitely need to add more testing. However, this also raises a problem, suppose we need to run the live tests in GitHub Actions, is there a service bus namespace that can be used publicly? Or is there a way to set up the Actions to not leak the testing service bus namespace?

Another question is that whether this would be a good point (after adding more tests) to submit a PR and then start working on the remaining of the planned components? Or is it better to implement everything before submitting a PR?

Thanks in advance :)

[mario-guerra]

Hi @minghuaw, thanks for your work on this! I think I may be able to set up a service bus namespace you can use for testing, although it would be temporary. Would that suffice or are you looking for something permanent?

[minghuaw]

Thank you @mario-guerra . That would be great. A temporary one would suffice.

[mario-guerra]

Hi @minghuaw, can you confirm the email address listed in your profile is correct? If so, I will send you a connection string for an instance of Service Bus you can use for testing.

[minghuaw]

@mario-guerra Thanks. That one is correct ([email protected])

[mario-guerra]

@minghuaw thanks for confirming, email with details has been sent.

[mario-guerra]

Hi @minghuaw, how is the testing going? Is there anything else you need?

[minghuaw]

Hi @mario-guerra, thanks for checking out. Sorry I was sick for the past few days.

The testing with the basic plan queue went fine, but one problem with the basic plan is that I couldn't test session-enabled queues, which I am still using my own service bus namespace for the testing.

I am currently working on the documentation and examples and will soon be ready to submit a PR.

[mario-guerra]

@minghuaw that's great! Thanks for the update, we look forward to seeing your PR.

[minghuaw]

Another quick update, the service still carries a legacy SessionFilter value in the responded Attach frame (Azure/azure-amqp#232). This would require supporting the legacy Filter in fe2o3-amqp-types, which is already implemented in a dev branch. I would just want to see whether the service plans to fix this or not, and this may introduce a few more days of delay for the PR.

[mario-guerra]

I've tagged the maintainer in the repo for comment, thanks for the heads up.

[minghuaw]

While waiting for updates on Azure/azure-amqp#232, I will just provide some updates to the current implementation. And I think there are a few design decisions that could be discussed here.

BTW, the current impl should be ready for PR (other than a few intra-doc-links that need to be fixed) whether or not they decide to fix that issue, but their decision is needed so that I can decide whether to publish a breaking update to the AMQP 1.0 dependencies

Summary of current implementation status

What key public APIs have been implemented:

1. ServiceBusClient
2. ServiceBusSender
3. ServiceBusMessageBatch
4. ServiceBusReceiver
5. ServiceBusSessionReceiver
6. ServiceBusRuleManager

What remains to be implemented:

1. Transaction
2. ServiceBusProcessor
3. ServiceBusSessionProcessor
4. ServiceBusAdministrationClient
5. In general, reducing amount of .clone()

Some design decisions that could be discussed right now

1. Use of generics in ServiceBusClient, ServiceBusSender, ServiceBusReceiver, ServiceBusSessionReceiver, ServiceBusRuleManager These types all have a inner field that is a generic type that implements a trait (TransportClient, TransportSender, TransportReceiver, TransportSessionReceiver, TransportRuleManager). This was originally just to follow what the dotnet sdk does. However, there is only one implementation of each of these traits, making the generic look kind of unnecessary. So the question is essentially
   • Should we change the generic to the concrete type that implements the corresponding trait?
2. Whether AzureNamedkeyCredential and AzureSasCredential should be moved to azure_core crate?
3. Current implementation implements a ServiceBusRetryPolicy trait that mimics what dotnet sdk does, and it is not exactly the same as azure_core::RetryPolicy. Plus, the current retry policy is a generic with trait bound on AmqpClient, AmqpSender, AmqpReceiver, AmqpSessionReceiver, and AmqpRuleManager. So the questions are:
   • Should we switch to azure_core::RetryPolicy?
   • Should we change retry policy to a trait object instead of a generic?
4. The current method I use to test connection recovery is by manually turning off my network and then turn it back on, and the loss of network is verified with a ping on a separate terminal. Is there a better or automated way to test connection recovery?

[mario-guerra]

Hi @minghuaw, for the sake of moving this PR forward I believe it's safe for you to assume the service team will not change the legacy behavior you identified any time soon. Given that it would be a breaking change, the odds of it happening are low.

[minghuaw]

Thank you @mario-guerra . That sounds reasonable. I will try to get the PR ready later today or tomorrow then.

[mario-guerra]

@minghuaw that's great! I look forward to reviewing it.

[minghuaw]

I have submitted the PR #1185. I will be happy to discuss and provide any explanation or help for your reviews :)

[mario-guerra]

Great! I see Ryan and Cameron are already tagged as reviewers. I believe they are both out of office for the remainder of the year, so we will review the PR after the new year. Thanks again for your work on this!

[minghuaw]

This is kind of a general question regarding CI (more or less in the long run).

It looks like the current CI setup doesn't run integration test for wasm32-unknown-unknown target. I guess setting tests for wasm32-unknown-unknown itself is already tricky (especially because tokio "net" and "time" doesn't work in browser), but given that there seems to be a general goal of supporting wasm, would it be better to have some kind of strategy to run maybe a selected set of unit/integration tests?

[mario-guerra]

Hi @minghuaw, great question. I do think it makes sense to have some amount of testing for wasm but I'll defer to @cataggar on this one.

[minghuaw]

I feel like the current wasm ecosystem is quite fragmented given the various wasm targets (web, node, wasi, etc.). Is there a general scope of what wasm targets need to be supported?

[cataggar]

See @minghuaw's https://crates.io/crates/azservicebus
More info in #1185