[event-hubs] add AmqpAnnotatedMessage support #15939

chradek · 2021-06-24T01:40:42Z

Resolves #15769

Goal

Increase interopability with other AMQP-based services (e.g. Service Bus).

Background

EventData currently exposes the curated set of information available from messages using the AMQP protocol. There isn't currently a way for users to send or receive a raw AMQP message. The Service Bus client library was updated to add support for reading the AmqpAnnotatedMessage from a received message, and sending an AmqpAnnotatedMessage via adding it to a batch, or sending it directly to the service.

Changes

Updates ReceivedEventData with a getRawAmqpMessage() method that returns AmqpAnnotatedMessage.
Add the following properties to EventData:
- contentType: string
- messageId: string
- correlationId: string

Differences from Service Bus

ServiceBus has a readonly _rawAmqpMessage property on ServiceBusReceivedMessage instead of the getRawAmqpMessage() method. Since this is a public API, we should not have prefixed the field with an underscore. Changing this to a method has us similar to what other languages are doing, and gives us flexibility to pass options in the future (for example, if we decide we want to parse timestamps as dates instead of numbers, we can control that in an option in the future)

If this API is accepted, I will deprecate _rawAmqpMessage and add the same method in Service Bus.

References:

Azure/azure-sdk-for-net#20105

chradek · 2021-06-24T16:15:34Z

/azp run js - event-hubs - tests

azure-pipelines · 2021-06-24T16:15:46Z

Azure Pipelines successfully started running 1 pipeline(s).

richardpark-msft

Some small things, nothing fatal.

There's quite a bit of code here and it looks pretty much like what Service Bus has. Is there anything notable that I should pay more attention to?

richardpark-msft · 2021-06-24T21:41:39Z

sdk/eventhub/event-hubs/src/dataTransformer.ts

+    // customer on receive.
+    if (body === undefined) body = null;
+    if (bodyType === "value") {
+      // TODO: Expose value_section from `rhea` similar to the data_section and sequence_section.


I know this is actually my TODO but we should probably file an issue to push this change into rhea.

richardpark-msft · 2021-06-24T22:30:04Z

sdk/eventhub/event-hubs/src/dataTransformer.ts

+    // string, undefined, null, boolean, array, object, number should end up here
+    // coercing undefined to null as that will ensure that null value will be given to the
+    // customer on receive.
+    if (body === undefined) body = null;


I'd throw in a blank line here - it's really easy to think the 'if' and the 'underlying else's' all are part of the same chain.

With that said - the 'body being null' thing seems incorrect to me. I know this came from SB, so it's potentially wrong there as well.

For instance, if you pass null you get this from JSON.stringify:

> Buffer.from(JSON.stringify(null), "utf8") <Buffer 6e 75 6c 6c> > Buffer.from(JSON.stringify(null), "utf8").toString() 'null'

I haven't looked, but perhaps this all plays nicely with the rhea encoding method for message.<data|value|sequence>_section() methods?

The only time null would have some transformation done on it is for the data_section, which case it will be JSON stringified. I'd agree that this would cause a problem, except we also attempt to JSON parse any data_section bodies. This would result in the body being null, not "null".

That said, we might run into a problem is when talking to the other languages. There's no guarantee other languages are also calling JSON.parse (in fact I doubt they are), so they may see 'null' as a string rather than a null type. It looks like this is already existing behavior, but since null and 'null' both result in null being returned from JSON.parse, updating this is safe.

richardpark-msft · 2021-06-24T22:31:04Z

sdk/eventhub/event-hubs/src/dataTransformer.ts

+            return { body: body.content, bodyType: "value" };
+        }
+      } else {
+        // TODO: test case


Is this a TODO because it's not done yet or is there an interesting challenge here?

Haha, I meant to erase that but the problem with adding TODOs is you have to actually go back and grep them...

richardpark-msft · 2021-06-24T22:37:46Z

sdk/eventhub/event-hubs/src/eventData.ts

+    };
+  } else {
+    let bodyType: BodyTypes = "data";
+    if (typeof (data as EventDataInternal).getRawAmqpMessage === "function") {


So this check is basically isReceivedEventData(), more or less?

Right now, yes, exactly.

richardpark-msft · 2021-06-24T22:43:22Z

sdk/eventhub/event-hubs/src/eventHubReceiver.ts

      properties: data.properties,
      offset: data.offset!,
      sequenceNumber: data.sequenceNumber!,
      enqueuedTimeUtc: data.enqueuedTimeUtc!,
      partitionKey: data.partitionKey!,
-      systemProperties: data.systemProperties
+      systemProperties: data.systemProperties,
+      getRawAmqpMessage() {


I take the lack of an underscore here is a personal rebuke.

chradek · 2021-06-28T16:50:48Z

Is there anything notable that I should pay more attention to?

The only notable difference is that in Event Hubs, I'm using getRawAmqpMessage() instead of _rawAmqpMessage to get the AmqpAnnotatedMessage.

chradek · 2021-06-28T23:46:18Z

/azp run js - event-hubs - tests

azure-pipelines · 2021-06-28T23:46:29Z

Azure Pipelines successfully started running 1 pipeline(s).

chradek · 2021-06-29T03:02:13Z

/azp run js - event-hubs - tests

azure-pipelines · 2021-06-29T03:02:31Z

Azure Pipelines successfully started running 1 pipeline(s).

[Hub Generated] Review request for Microsoft.LabServices to add version preview/2021-10-01-preview (Azure#15939) * Adds base for updating Microsoft.LabServices from version stable/2018-10-15 to version 2021-10-01-preview * Updates readme * Updates API version in new specs and examples * remove 2018 api spec files * add 2021-10-01-preview spec files from generation * update preview api files in readme.md input * update 2020 -> 2021 in new api version in readme * spelling fixes * fix object body mutability * add type:object where missing * added default error response * tracker resource operation naming and description adding * fix shutdownOnIdleEnabled in examples * fix shutdownonIdle enum in examples * examples update username to adminUser object and remove locations from vmprofile * fix examples for vmprofile * rest of model validaiton fixes * prettier fix * removed skus api from 2021-10-01 version since it's not finished * fix operationresults * small images fix Co-authored-by: Nick Depinet <ndepinet@microsoft.com>

[event-hubs] add AmqpAnnotatedMessage support

4890de0

chradek requested review from bterlson, HarshaNalluru, richardpark-msft and xirzec as code owners June 24, 2021 01:40

ghost added Event Hubs Service Bus labels Jun 24, 2021

undo experimental deletion of decode method from service bus

fbb34c4

richardpark-msft approved these changes Jun 24, 2021

View reviewed changes

chradek added 2 commits June 28, 2021 16:43

address feedback

8335ec6

Merge remote-tracking branch 'upstream/main' into eh-annotated-message

2a8881c

chradek merged commit 19d0e76 into Azure:main Jun 30, 2021

chradek deleted the eh-annotated-message branch June 30, 2021 17:38

ramya-rao-a mentioned this pull request Oct 15, 2021

[@azure/event-hubs] Adding disableDeserialization option when subscribing #18173

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[event-hubs] add AmqpAnnotatedMessage support #15939

[event-hubs] add AmqpAnnotatedMessage support #15939

chradek commented Jun 24, 2021

chradek commented Jun 24, 2021

azure-pipelines bot commented Jun 24, 2021

richardpark-msft left a comment

richardpark-msft Jun 24, 2021

richardpark-msft Jun 24, 2021

chradek Jun 28, 2021

richardpark-msft Jun 24, 2021

chradek Jun 25, 2021

richardpark-msft Jun 24, 2021

chradek Jun 25, 2021

richardpark-msft Jun 24, 2021

chradek commented Jun 28, 2021

chradek commented Jun 28, 2021

azure-pipelines bot commented Jun 28, 2021

chradek commented Jun 29, 2021

azure-pipelines bot commented Jun 29, 2021

[event-hubs] add AmqpAnnotatedMessage support #15939

[event-hubs] add AmqpAnnotatedMessage support #15939

Conversation

chradek commented Jun 24, 2021

Goal

Background

Changes

Differences from Service Bus

References:

chradek commented Jun 24, 2021

azure-pipelines bot commented Jun 24, 2021

richardpark-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chradek commented Jun 28, 2021

chradek commented Jun 28, 2021

azure-pipelines bot commented Jun 28, 2021

chradek commented Jun 29, 2021

azure-pipelines bot commented Jun 29, 2021