-
Notifications
You must be signed in to change notification settings - Fork 164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stabilize messaging semantic conventions for tracing #192
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this up! I left a few things.
d392e89
to
b890a85
Compare
Co-authored-by: Joao Grassi <joao@joaograssi.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing just how much (valuable!) discussion this OTEP generates, I would strongly suggest to change the goal here:
Do not aim to overhaul & stabilize the messaging semantic conventions in one go. Overhaul it first, and make stabilizing (which is currently the OTEP title) a separate, next goal that comes after.
This document aims to describe the necessary changes for bringing the [existing semantic conventions for messaging](https://github.com/open-telemetry/opentelemetry-specification/blob/a1a8676a43dce6a4e447f65518aef8e98784306c/specification/trace/semantic_conventions/messaging.md) | ||
from the current [experimental](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/versioning-and-stability.md#experimental) | ||
to a [stable](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/versioning-and-stability.md#stable) | ||
state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This document aims to describe the necessary changes for bringing the [existing semantic conventions for messaging](https://github.com/open-telemetry/opentelemetry-specification/blob/a1a8676a43dce6a4e447f65518aef8e98784306c/specification/trace/semantic_conventions/messaging.md) | |
from the current [experimental](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/versioning-and-stability.md#experimental) | |
to a [stable](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/versioning-and-stability.md#stable) | |
state. | |
This document aims to overhaul the [existing semantic conventions for messaging](https://github.com/open-telemetry/opentelemetry-specification/blob/a1a8676a43dce6a4e447f65518aef8e98784306c/specification/trace/semantic_conventions/messaging.md). |
@@ -0,0 +1,659 @@ | |||
# Stabilizing messaging semantic conventions for tracing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Stabilizing messaging semantic conventions for tracing | |
# Overhauling messaging semantic conventions for tracing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stabilizing has a more concrete meaning in the context of the spec. Overhauling is just a change to something else that is not stable and can change again.
If the purpose is getting this done for a fist version, I would be in favor of stabilizing.
[`messaging.destination.kind`](#messagingdestinationkind) | string | No | ||
[`messaging.destination.temporary`](#messagingdestinationtemporary) | string | No | ||
[`messaging.destination.anonymous`](#messagingdestinationanonymous) | string | No | ||
[`messaging.source.name`](#messagingsourcename) | string | For consumer spans |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the OTEP text should document why the decision was made to split the "destination" attributes were split into "destination" and "source" (and maybe reconsider).
[`net.app.protocol.name`](#netappprotocolname) | string | No | ||
[`net.app.protocol.version`](#netappprotocolversion) | string | No | ||
[`net.peer.ip`](#netpeerip) | string | No | ||
[`net.peer.name`](#netpeername) | string | No |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to suggest a new "concept" here, which is that of the "receipt handle". In AWS SQS, when you call the ReceiveMesssage API, you get, along with the message ID (which is also missing as an attribute here), a unique "Receipt handle" for each message. The receipt handle is what you need to provide to DeleteMessage or DeleteMessageBatch to settle the message (instead of, e.g., the message ID).
The receipt handle is a rather but not terribly long base64 string. I wonder if...
- ...Other messaging systems have a similar mechanism (if not, all the other questions are irrelevant at this point)
- ...This concept should be described in the "individual message settlement" section
- ...It would make sense defining an attribute for it, e.g. to correlate with logs. It is possible that this only makes sense in a future step together with intermediary instrumentation. But it could be interesting to track whether a message is re-processed without being re-delivered/received. Maybe too much of an edge case 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh I have never used AWS SQS and don't know any other messaging system having this concept, can you mention another one please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know one, that's why I'm asking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read that wrong ;-) Now it let me think that something like that should be in the Service Bus as well but in general in AMQP 1.0.
Let's take a look at the REST interface for Service Bus via HTTP. Other than a receive and delete message, there is a two-steps way: peek lock and delete. During the Peek Lock the consumer gets the message and a sequencenumber
with it. This will be used in the next Delete Message together with the message id to delete the message.
But even from an AMQP 1.0 point of view it makes sense because looking at the specification, the disposition
frame (which is the ack from the consumer) defines a delivery-id
. This maps with the Azure Service Bus AMQP 1.0 implementation here where you can search for the sequence-number again for deleting a message. I guess that the sequence number
maps to the delivery-id
at AMQP 1.0 level.
Said that, I would agree that maybe a concept like a receipt handle, sequence number or delivery id could make sense to add for the "settlement" by the consumer.
particular set of producers and consumer. Often such destinations are unnamed | ||
or have an auto-generated name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "often"? Isn't that the definition of it being anonymous?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree but in the end it's never unnamed but always auto-generated (it's needed to be addressed). This is the reason why, for example, AMQP 1.0 protocol define it as "dynamic" not "anonymous".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It already says "unnamed or have an auto-generated name". So why is that only "often"? If it is named and the name is not auto-generated, can it still be anonymous?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh I don't know what it's referring to as "unnamed" destination. Right now "anonymous" sounds to be wrong, unless someone else can raise an example of why it's right.
For each producer scenario, a "Publish" span needs to be created. This span | ||
measures the duration of the call or operation that provides messages for | ||
sending or publishing to an intermediary. This call or operation (and the | ||
related "Publish" span) can either refer to a single message or a batch of | ||
multiple messages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For each producer scenario, a "Publish" span needs to be created. This span | |
measures the duration of the call or operation that provides messages for | |
sending or publishing to an intermediary. This call or operation (and the | |
related "Publish" span) can either refer to a single message or a batch of | |
multiple messages. | |
For each producer scenario, a "Publish" span needs to be created. This span measures the time for publishing the message which, depending on the messaging system and the producer API, could include: | |
* just storing the message in an internal producer buffer, and asynchronously sent to the intermediary | |
* actual synchronous sending to the intermediary | |
* one of the above but also getting an acknowledgement/settlement back from the intermediary | |
This call or operation (and the related "Publish" span) can either refer to a single message or a batch of multiple messages. |
|
||
For each producer scenario, a "Publish" span needs to be created. This span | ||
measures the duration of the call or operation that provides messages for | ||
sending or publishing to an intermediary. This call or operation (and the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a suggestion to list the scenarios.
messages in order to fulfill the [requirements for context propagation](#context-propagation). | ||
While preserving the freedom for instrumentor to choose how to propagate | ||
context, in the future these conventions should list recommended ways of how to | ||
propagate context using popular messaging protocols. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, just leaving a comment as I came across an issue that can be resolved if some sort of standards are in place.
If you refer to the issue above, it is apparent that there is already a divergence of approaches to propagate context with different instrumentors in different languages. I wonder if more attention should be given to establishing a convention before everyone starts doing things differently and becoming out of control.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @melvinkcx
A while ago this OTEP https://github.com/open-telemetry/oteps/blob/main/text/trace/0205-messaging-semantic-conventions-context-propagation.md was merged to make the context propagation a bit more explicit.
It doesn't go in exact details on how this should be done (where in the message to put the context, under which "name" etc) because there's no finalized standards for it. The OTEP only makes it clear that the context should be transported together with the message and should be immutable. Since each protocol/messaging system does something different, it's hard to come up with a specification that works for everyone. Our intentions for now is to define the minimum on how propagation will work, and leave to instrumentation to implement it using their appropriate idioms. For ex, this draft document for AMQP tells to add the context in application-properties
.
Of course, once stable specifications are available, we will update the recommendations in the OTel conventions, like the AMQP one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the insights!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since each protocol/messaging system does something different, it's hard to come up with a specification that works for everyone
How about specifying just a guideline / best-practice like: "Call the propagator on the message headers or equivalent"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened a PR to bring the contents of the OTP into the spec. Maybe that already gives the direction we need, without having to be precise on where to add. open-telemetry/opentelemetry-specification#2750. It also mentions when more standards are available, we will be updating the guidelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One interesting line on the W3C document for AMQP linked above:
"...AMQP message section "application-properties" is immutable collection of properties"
They recommend to use application-properties because is immutable data that brokers cannot change. This aligns with the immutable Creation Context from above.
The MQTT doc doesn't mention immutability but defines user-properties and the method to use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brunobat and that's for MQTT v5 you are referring to I guess. Thinking at MQTT 3.1.1 there is no notion of application/user properties, everything is in your payload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check #207 because the propagation will soon might have:
- Baggage. For app data
- Context-scoped attributes. It is proposed that Context-scoped attributes MUST not be propagated.
- Instrumentation scope attributes. Not sure.
Seems that we need to clarify what tracing data will be transported in the message and how.
@@ -0,0 +1,659 @@ | |||
# Stabilizing messaging semantic conventions for tracing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stabilizing has a more concrete meaning in the context of the spec. Overhauling is just a change to something else that is not stable and can change again.
If the purpose is getting this done for a fist version, I would be in favor of stabilizing.
identifiable. | ||
|
||
In the strict sense, a _message_ is a payload that is sent to a specific | ||
destination, whereas an _event_ is a signal emitted by a component upon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the case for a topic, unless the destination is defined as a broker.
Wouldn't it be better to say something like:
_message_ is a payload sent by a Producer to one or more Consumers, either directly or by using Intermediaries
I think it aligns better with what's written bellow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with a topic, the destination is the topic. That's how the current ("old") messaging semantic conventions have it defined: The destination of a message is a queue or a topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good to me @Oberon00
A _creation context_ allows correlating the producer with the consumers of a | ||
message, regardless of intermediary instrumentation. The creation context is | ||
created by the producer and must be propagated to the consumers. It must not be | ||
altered by intermediaries. This context helps to model dependencies between |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It must not be altered by intermediaries.
The only way to ensure this is to sign the propagated attributes, like what's done with a Json Web Token (JTW). Not saying we need to sign stuff now, but in the future we might need, to prevent tampering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this part sorted out in the other PR open-telemetry/opentelemetry-specification#2750 (comment)
> A producer SHOULD attach a creation context to each message. The creation context | ||
> SHOULD be attached in a way so that it is not possible to be changed by intermediaries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my understanding, we are going to push for a message span that goes from producer send to consumer receive. Right?
| Create m2 | . . . . . . . . | ||
+-----------+---------+ . +------------+ | ||
| Publish | . . | Receive m2 | | ||
+---------+ +------------+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't seem to be consistent within pull-based consumer scenarios in what the parent context should be for a receive span. I believe the intent is that the parent should be the application's ambient span?
Although intermediary instrumentation is out of scope, it's something I'm thinking about and there is a decision to make as to how the transport context of the message fits into these traces. If you care about the path the message takes, it's nice to have the receive span as a child of the message's context. But since it is the application's "pull" that caused the span, it's probably more correct for the receive span to be a child of the application's ambient span.
If we agree this is the right approach, it probably makes the most sense for the receive span to link to the message's transport context (although no need to mention here as that would be out of scope). If there is no ambient context, it seems as though it would be nicer to be a child rather than create a brand new trace with no parent at all. On the one hand, this means we have inconsistent structure. On the other hand, does splitting the traces up help with anything?
1. The _creation context layer_ allows correlating the producer with the | ||
consumers of a message, regardless of intermediary instrumentation. The | ||
creation context is created by the producer and must be propagated to the | ||
consumers. It must not be altered by intermediaries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"It Should not be altered by intermediaries."
Because we are going to discuss intermediaries later.
One possibility to seamlessly integrate producer/consumer and intermediary | ||
instrumentation in a flexible and extensible way would be the introduction of a | ||
second transport context layer in addition to the creation context layer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of an "immutable" creation context for the message because it seems simpler and less ambiguous to understand, define and implement.
Transport context seem too broad. It can be used for anything transport related and in each message, it might contain arbitrary pieces of data that were added or modified along the way. Super useful, but makes every message a non obvious subject requiring analysis. Even graphic representation might be convoluted.
instrumentation. | ||
2. An additional _transport context layer_ allows correlating the producer and | ||
the consumer with an intermediary. It also allows to correlate multiple | ||
intermediaries among each other. The transport context can be changed by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correlation should be message based. "Correlating intermediaries" sounds like broker framework related work. Do we want to provide tools to make that easier? Which ones?
messages in order to fulfill the [requirements for context propagation](#context-propagation). | ||
While preserving the freedom for instrumentor to choose how to propagate | ||
context, in the future these conventions should list recommended ways of how to | ||
propagate context using popular messaging protocols. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One interesting line on the W3C document for AMQP linked above:
"...AMQP message section "application-properties" is immutable collection of properties"
They recommend to use application-properties because is immutable data that brokers cannot change. This aligns with the immutable Creation Context from above.
The MQTT doc doesn't mention immutability but defines user-properties and the method to use
The messaging workgroup was capturing findings and results of discussions in this draft PR. As this draft PR got very big and it got hard to keep track of discussions going on in comments, the workgroup decided to close this PR and work on different artifacts:
|
This OTEP aims at defining consistent conventions about what spans to create for messaging scenarios, and at defining how those spans relate to each other. Instrumentors should be able to rely on a consistent set of conventions, as opposed to deducing conventions from a set of examples. This was split from OTEP #192, which became too comprehensive.
This OTEP aims at defining consistent conventions about what spans to create for messaging scenarios, and at defining how those spans relate to each other. Instrumentors should be able to rely on a consistent set of conventions, as opposed to deducing conventions from a set of examples. This was split from OTEP open-telemetry#192, which became too comprehensive.
This OTEP aims at defining consistent conventions about what spans to create for messaging scenarios, and at defining how those spans relate to each other. Instrumentors should be able to rely on a consistent set of conventions, as opposed to deducing conventions from a set of examples. This was split from OTEP open-telemetry#192, which became too comprehensive.
This OTEP aims at defining consistent conventions about what spans to create for messaging scenarios, and at defining how those spans relate to each other. Instrumentors should be able to rely on a consistent set of conventions, as opposed to deducing conventions from a set of examples. This was split from OTEP open-telemetry#192, which became too comprehensive.
This OTEP aims at defining consistent conventions about what spans to create for messaging scenarios, and at defining how those spans relate to each other. Instrumentors should be able to rely on a consistent set of conventions, as opposed to deducing conventions from a set of examples. This was split from OTEP open-telemetry#192, which became too comprehensive.
This OTEP aims at defining consistent conventions about what spans to create for messaging scenarios, and at defining how those spans relate to each other. Instrumentors should be able to rely on a consistent set of conventions, as opposed to deducing conventions from a set of examples. This was split from OTEP open-telemetry#192, which became too comprehensive.
This OTEP aims to describe the necessary changes for bringing the existing semantic conventions for messaging from the current experimental to a stable state.
It is based on OTEP 0173, which defines basic terms and describes messaging scenarios that should be supported by the semantic conventions.
NOTE: This is an early draft document. It captures results of discussions currently going on in the messaging workgroup.