Refactor messaging attributes and per-message attributes in batching scenarios#2763
Closed
lmolkova wants to merge 12 commits intoopen-telemetry:mainfrom
Closed
Refactor messaging attributes and per-message attributes in batching scenarios#2763lmolkova wants to merge 12 commits intoopen-telemetry:mainfrom
lmolkova wants to merge 12 commits intoopen-telemetry:mainfrom
Conversation
lmolkova
force-pushed
the
messaging-batching-attributes
branch
from
e2b201f to
29fa0b6
Compare
blumamir
reviewed
Aug 31, 2022
blumamir
reviewed
Aug 31, 2022
joaopgrassi
reviewed
Aug 31, 2022
reyang
added
semconv:messaging
area:semantic-conventions
dpauls
reviewed
Sep 8, 2022
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Member
|
Not stale |
joaopgrassi
reviewed
Sep 29, 2022
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Member
|
Not stale |
pyohannes
reviewed
Oct 12, 2022
lmolkova
force-pushed
the
messaging-batching-attributes
branch
from
6cf659b to
bfb6c8e
Compare
lmolkova
changed the title
lmolkova
force-pushed
the
messaging-batching-attributes
branch
4 times, most recently
from
637a5d0 to
98eca80
Compare
brunobat
reviewed
Nov 3, 2022
lmolkova
force-pushed
the
messaging-batching-attributes
branch
2 times, most recently
from
25f5759 to
ceaff1b
Compare
blumamir
reviewed
Nov 3, 2022
ppatierno
reviewed
Nov 6, 2022
pyohannes
reviewed
Nov 9, 2022
pyohannes
reviewed
Nov 9, 2022
lmolkova
force-pushed
the
messaging-batching-attributes
branch
4 times, most recently
from
ee0fa7b to
cdab761
Compare
lmolkova
force-pushed
the
messaging-batching-attributes
branch
from
cdab761 to
747411a
Compare
dpauls
suggested changes
Nov 10, 2022
blumamir
approved these changes
Nov 16, 2022
Comment on lines
+346
to
+350
| * message-specific under `messaging.{system}.message` | ||
| * destination-specific under `messaging.{system}.destination` | ||
| * source-specific under `messaging.{system}.source` |
Member
There was a problem hiding this comment.
For completeness and consistency, and also since I think these cases exist, I want to suggest also adding messaging.{system}.batch to this list
| | Attribute | Type | Description | Examples | Requirement Level | | ||
| |---|---|---|---|---| | ||
| | `messaging.rabbitmq.routing_key` | string | RabbitMQ message routing key. | `myKey` | Conditionally Required: If not empty. | | ||
| | `messaging.rabbitmq.message.routing_key` | string | RabbitMQ message routing key. | `myKey` | Conditionally Required: If not empty. | |
Member
There was a problem hiding this comment.
Should this be messaging.rabbitmq.**destination**.routing_key?
Member
Author
There was a problem hiding this comment.
Per Rabbit docs, routing key is a message property. It's also available on consumers
Member
|
Also, I support moving this PR out of draft and asking for reviews from the community so we can progress towards merging this. |
pyohannes
approved these changes
Nov 16, 2022
Contributor
pyohannes
left a comment
pyohannes
left a comment
There was a problem hiding this comment.
I think this is ready for some wider discussion. We should open an issue, and if it's accepted, open a new clean PR based on this draft.
added 12 commits
November 16, 2022 11:11
Add destination name - it could be per-message
lmolkova
force-pushed
the
messaging-batching-attributes
branch
from
9c0160e to
54c07c3
Compare
Member
Author
|
Thanks everyone for the review! Per discussion at the last SIG meeting, I'm going to close this one and open a new PR - #2957 (to simplify final review process). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Messaging instrumentation SIG is working on spec changes and this change is one of the first steps to bring the consensus we reached in SIG to the spec.
This change clarifies that per-message attributes should be set on links when the corresponding span represents a batching operation. It introduced breaking changes (attribute renames)
messaging.message_idtomessaging.message.idmessaging.conversation_idtomessaging.message.conversation_idmessaging.message_payload_size_bytestomessaging.message.payload_size_bytesmessaging.message_payload_compressed_size_bytestomessaging.message.payload_compressed_size_bytes.Going forward, we'd like to reserve
messaging.message.namespace for other possible per-message attributes.It also adds the
messaging.batch_sizeattribute which intends to: