[Service Bus] Correlation Filter: Types for userProperties and SqlParameter, matching userProperties in the rules with those set on the message

[HarshaNalluru]

Thoughts/Needs work

Followups/todos on the types allowed in userParameters for the createRule(or updateRule) method in the ATOM Management API.

• [New bug found] Service did not respect the "l68:date" type (which is what the SqlParameter was relying upon). There is no service documentation on which types are supported.
• For now, userProperties in a correlation filter in the SDK support "string", "number" and "boolean" types with [Service Bus] Bug Fix: userProperties aren't populated while using createRule() from ATOM API #9794 .
• Tests for sqlParameter only verify "string" and "number", we should look into the other types with more emphasized testing on the supported types for the SqlParameter.
• Python supports strings only as of now. From here, .NET seems to support "long", "double", "dateTime" and "duration" as the types other than the ones that were added for userProperties in JS through [Service Bus] Bug Fix: userProperties aren't populated while using createRule() from ATOM API #9794.
• Compare with the userProperties set on the message since they are the ones that should be matched with these from the correlationFilters, I don't think we do any sort of type validations for the userProperties on the message, but we seem to be applying JSON.stringify() on the message/message-body which might affect the rules/matching. In the end, we need to ensure that the userProperties in the rules match the same ones set on the message.
• Currently, we don't distinguish if the "number" is "int" or "double", we are mapping all "number"s to "int". Need to handle serialization and deserialization carefully for the interop.
• Bug - Error when a single property is set in the correlation rule filter [Service Bus] ATOM API: Anomalous behavior when only one "property" is set on the correlation filter #10462

Action Items

• Bug - Error when a single property is set in the correlation rule filter [Service Bus] ATOM API: Anomalous behavior when only one "property" is set on the correlation filter #10462
• Support for string, number and boolean for userProperties and sqlParameters
• Update User props in correaltion filter properties: {[key: string]: string | number | boolean}
• Update sqlParams with sqlParameters: {[key: string]: string | number | boolean}
• Update props on message with properties: {[key: string]: string | number | boolean | Date}
• Investigate support for Date in the value for XML requests [Service Bus] ATOM API: Supporting "Date" in the correlation-rule-filter props #11117
• Investigate support for duration in the value for XML requests
• Investigate support for Long in the value for XML requests (This is not supported in the AMQP side, we may have to use "Long" from the "long" library and hack around serialization, hence deferring this to a later time upon a user requirement)
• To allow interop, properly differentiate "number" as "int" and "double" as expected by the service and the .NET SDK(Both during serialization and deserialization, currently "number" is mapped as "int")
• (Add tests) Make sure the properties set on the message can be filtered by the properties set on the correlation-filter [Service Bus] Tests - Props on AMQP Message to match with rule props set using ATOM API #11128
  • Number
  • Bool
  • String
  • Date

Final Thoughts - #9850 (comment)

[HarshaNalluru]

Supported types for the values set in user-properties in the correlation-filter in .NET and the equivalents in JS/TS

"string"     -> "string"
"int"        -> "number"
"double"     -> "number"
"boolean"    -> "boolean"
"long"       -> Do we have to rely on Long type from "long" library?
"dateTime"   -> We have `Date`, service doesn't seem to like the request that I send with the date object, tried wrapping it as a string too, couldn't get it to work.
"duration"   -> No equivalent in JS. So, do we ignore?

FYI: For supporting "long", "dateTime" and "duration", I guess I would need to compare the serialized request with that of .NET's to make them work.

What about "bigint"?

[richardpark-msft]

The representation of a long is an interesting topic that has come up in several services now, almost at the same time.

I believe this is being discussed at the top levels: CC: @bterlson, @xirzec

[bterlson]

Yes, but sharing some thoughts:

Do we have to rely on Long type from "long" library?

I hope not! Honestly strings are better than Long because at least with strings it's easy to get a standard bigint out of it. Unfortunately bigints aren't broadly available enough to be our only story here 👍

We have Date, service doesn't seem to like the request that I send with the date object, tried wrapping it as a string too, couldn't get it to work.

Date is what you should use. You may have to manually serialize it to an expected date format (e.g. via toISOString or toGMTString).

No equivalent in JS. So, do we ignore?

I wouldn't ignore, but represent as a number of seconds or similar.

[HarshaNalluru]

Summarizing my discussion with @JoshLove-msft

• .NET supports allows more types in the properties on a message.
  However, the correlation rule filter only allows ["string", "int", "boolean", "long", "double", "dateTime" and "duration"] as the types of values to be filtered in the properties.
  Since this inconsistency was never a problem in track 1 and since no user complained about this, track 2 doesn't have to change.
• For supporting "long", "dateTime" and "duration" in JS, I would need to compare the serialized request with that of .NET's to make them work, waiting for the sample XML requests from Josh for these 3 types to proceed towards further investigation/support in JS.

[HarshaNalluru]

• In .NET,
  • All these types for the properties on a message that are supported
  • The values in the properties set in a correlation rule filter support ["string", "int", "boolean", "long", "double", "dateTime" and "duration"]
• In JS,
  • We have properties: { [key: string]: any } on the message,
  • The values in the properties set in a correlation rule filter support ["string", "number", "boolean"]. (This set would potentially expand with equivalents for "long", "dateTime" and "duration".)

Question:
For JS, should I change properties: { [key: string]: any } on the message to properties: { [key: string]: "string" | "number" | "boolean" } to be consistent with the properties on the rule filters?

[xirzec]

Stronger typing is preferred if it's accurate.

[HarshaNalluru]

More findings on the values in properties of the message

• When tried arrays, JSON objects, Long (from "long" library) and Buffer as values in user properties on the message, some of them failed at the serialization and some failed at the service side resulting in an error.
• TS complains about "bigint" because we target ES2015

  

From the above, it seems "number", "boolean", "string", and Date are the only types allowed for values in the message properties.

Conclusions(from my findings)

To conclude, the properties should be properties: { [key: string]: "number" | "boolean" | "string" | Date } on the message.
And on the correlation rule filter, it should be properties: { [key: string]: "string" | "number" | "boolean" } since only those are supported through ATOM API.
If added support for long/duration/date, those types can be added incrementally.

Thoughts anybody?

[richardpark-msft]

If added support for long/duration/date, those types can be added incrementally.

What do you get if someone uses a separate library to save the properties (like .NET) and then you try to deserialize them? An error?

[HarshaNalluru]

What do you get if someone uses a separate library to save the properties (like .NET) and then you try to deserialize them? An error?

As of now.. we throw an error. The goal is to support those types too, both during serializing and deserializing, before GA.

[JoshLove-msft]

Here is the properties section of the correlation filter creation request containing string, long, datetime, and duration and the corresponding code that sets the correlationRuleFilter properties:

correlationRuleFilter.Properties.Add("customKey", "customValue");
correlationRuleFilter.Properties.Add("long", (long) 4);
correlationRuleFilter.Properties.Add("dto", DateTime.UtcNow);
correlationRuleFilter.Properties.Add("timespan", TimeSpan.FromDays(1));

"        \u003CProperties\u003E\r\n",
"          \u003CKeyValueOfstringanyType\u003E\r\n",
"            \u003CKey\u003EcustomKey\u003C/Key\u003E\r\n",
"            \u003CValue p4:type=\u0022l28:string\u0022 xmlns:l28=\u0022http://www.w3.org/2001/XMLSchema\u0022\u003EcustomValue\u003C/Value\u003E\r\n",
"          \u003C/KeyValueOfstringanyType\u003E\r\n",
"          \u003CKeyValueOfstringanyType\u003E\r\n",
"            \u003CKey\u003Elong\u003C/Key\u003E\r\n",
"            \u003CValue p4:type=\u0022l28:long\u0022 xmlns:l28=\u0022http://www.w3.org/2001/XMLSchema\u0022\u003E4\u003C/Value\u003E\r\n",
"          \u003C/KeyValueOfstringanyType\u003E\r\n",
"          \u003CKeyValueOfstringanyType\u003E\r\n",
"            \u003CKey\u003Edto\u003C/Key\u003E\r\n",
"            \u003CValue p4:type=\u0022l28:dateTime\u0022 xmlns:l28=\u0022http://www.w3.org/2001/XMLSchema\u0022\u003E2020-08-16T05:00:45.9283806Z\u003C/Value\u003E\r\n",
"          \u003C/KeyValueOfstringanyType\u003E\r\n",
"          \u003CKeyValueOfstringanyType\u003E\r\n",
"            \u003CKey\u003Etimespan\u003C/Key\u003E\r\n",
"            \u003CValue p4:type=\u0022l28:duration\u0022 xmlns:l28=\u0022http://www.w3.org/2001/XMLSchema\u0022\u003EP1D\u003C/Value\u003E\r\n",
"          \u003C/KeyValueOfstringanyType\u003E\r\n",
"        \u003C/Properties\u003E\r\n",

[HarshaNalluru]

Final thoughts regarding correlation-rule props and types supported keeping interop with .NET SDK in mind... #9850

• Long
  • Setting long values(from "long" library) in props on message doesn't work through AMQP side as of now
  • If we ever add support, it would be non-breaking to add in the mgmt side with a new type
• Number as int and double
  • Currently, all numbers are treated as "int"
  • Plan is to distinguish int and double while serializing, and treat int and double as numbers while deserializing
  • Can be added incrementally, non-breaking
• Duration
  • With a clean design of key value pairs, there is no way we can support "duration"(JS limitation)
  • Either drop the support for duration as value 
  • Or treat duration as number during deserialization - Non-breaking
  • Or treat all the ISO8601 strings as duration if provided in the values - Doesn't change the API surface(only internal details)
  • Or change the key-value object as key-value-type object so that users can pass in "duration" string in the third field

For duration, @xirzec 's suggestion (with reference to the tables SDK)

properties: {[key: string]: string | number | boolean | Date | SBDuration}
SBDuration = {val:string, kind:"dateTime"} // val would be ISO-8601 string
// Same for Long and anything else that we don't have a type for

(And that makes all the three issues mentioned to be non-breaking and it should be fine to add support even after GA.)

Feedback from @bterlson: (offline discussion)

• Fine with the duration suggestion from @xirzec
• On "Number as int and double" bug fix, to allow proper interop(considering multiple hops between say.. .NET and JS libraries),
  • Consider all the numbers are "double"(and serialize accordingly).
  • If a user requests for the requirement of "int", we can support it through {val, kind} way similar to "dateTime".

[HarshaNalluru]

Moving the PR #12570 that attempted to add the duration support to a branch as suggested by @ramya-rao-a for future reference.
(That PR was more from ATOM cross-language interop perspective, not with the AMQP side.)

https://github.com/Azure/azure-sdk-for-js/tree/harshan/sb%2Fissue-9850%2Ffilter-interop

It's not straightforward for the AMQP side, needs more testing w.r.t duration for AMQP.. questions to be asked below

• how the ISO8601 string would behave when set?
• does the rule filter match properly if duration is set as a prop on the message using JS libraries?
• is it interoperable?
• when we receive a message, do we know if the property is of the duration type in the AMQP land? (For ATOM side, we do know for the rule filter from the XML response.)

[github-actions]

Hi @HarshaNalluru, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.