AMQP draft: PR was moved to different repository #217
Conversation
spec/AMQP/20-AMQP_FORMAT.md
Outdated
| ## Batching | ||
|
|
||
| TODO: Batching of messages – recommended semantics of TraceContext fields when |
There was a problem hiding this comment.
I think we should have a discussion and general guidance for batching, but it probably is related more to the instrumentation and tracing system behavior.
My (very small) experience with AMQP involves Azure ServiceBus and EventHubs, I can miss a lot of details here, but my take on this was:
-
SB/EH message could be represented by AMQP message. Or a batch of SB/EH messages is a single AMQP message. If it is the typical case for AMQP, we should give guidance to the AMQP implementations on how to treat these two cases.
-
every SB/EH message carry trace context - they probably were produced in different contexts under different spans. Such messages should be annotated during creation, not during publishing. This is instrumentation guidance.
-
AMQP message constructed from the batch has new trace context, potentially unrelated to each message in the batch. Relation should be established by something like opencensus links. But again this is instrumentation and tracing system details, not the protocol.
There was a problem hiding this comment.
The specification should define a binding to the AMQP "message format 0", i.e. the layout that is defined in the Messaging section of the the AMQP spec, but should not go into further detail on HOW those messages are being transferred.
AMQP is enormously efficient when filling link credit with sending messages in sequence without any special overlaid batching.
Service Bus and Event Hubs indeed have a special message format that can indeed batch multiple AMQP message format messages into one, but the way to bind to that format is by specifying a binding to the core message format and then have the proprietary batching model pick that up via the binding to the standard message.
|
@yurishkuro I'm working with Azure IoT team to implement it. Binary format for |
| of AMQP specification. | ||
|
|
||
| Property name MUST be `traceparent` - all lowercase without delimiters. |
There was a problem hiding this comment.
Considering the case that the traceparent and tracestate fields are transported via message-annotations:
As for the base type of the message-annotations, the AMQP spec says (http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-annotations):
The annotations type is a map where the keys are restricted to be of type symbol or of type ulong. All ulong keys, and all symbolic keys except those beginning with "x-" are reserved. Keys beginning with "x-opt-" MUST be ignored if not understood. On receiving an annotation key which is not understood, and which does not begin with "x-opt", the receiving AMQP container MUST detach the link with a not-implemented error.
Concerning the names of message-annotations:
A registry of defined annotations and their meanings is maintained [AMQPMESSANN].
The question here: Are the names of the traceparent and tracestate fields supposed to be somehow registered as reserved with OASIS. Otherwise it looks to me as if the prefix "x-opt-" should be used.
| See definition of type map in section | ||
| [1.6.23](http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-types-v1.0-os.html#type-map) | ||
| of AMQP specification. |
There was a problem hiding this comment.
Considering the case that the traceparent and tracestate fields are transported via application-properties:
About application-properties, the AMQP spec says (http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-application-properties):
The keys of this map are restricted to be of type string (which excludes the possibility of a null key) and the values are restricted to be of simple types only, that is, excluding map, list, and array types.
This looks incompatible with wanting to store a tracestate AMQP map in there.
|
|
||
| Strings duplicating the size of a field, using list of binaries will require to | ||
| redefine the way the field serialized, parsed, and versioned. So re-using binary | ||
| protocol looks like a logical solution. |
There was a problem hiding this comment.
Where is that 'binary protocol' defined? (I found https://w3c.github.io/trace-context/extension-binary.html but it seems to be empty?). AMQP has quite a complex type-system that will already be required to decode the message, so it might be simpler to use that to encode traceparent in some way rather than requiring further decoding of the field.
There was a problem hiding this comment.
Actually, to @calohmn's point re the restrictions on application-property values, perhaps you are right and the binary format is best (it is not very arduous to parse). (Alternatively an amqp list or a map with integer keys would be similar to the binary encoding).
There was a problem hiding this comment.
@grs yeah. It may be better to switch to binary protocol than. At least some consistency - either text (like HTTP or MQTT) or binary (like GRPC or AMQP).
SergeyKanzhelev
changed the title
PR was moved |
* moved from w3c/trace-context#217 * Update spec/20-AMQP_FORMAT.md Co-Authored-By: SergeyKanzhelev <[email protected]> Co-authored-by: Sergey Kanzhelev <[email protected]> Co-authored-by: Nik Molnar <[email protected]>
PR was moved
w3c/trace-context-amqp#1