Refactor messaging attributes and per-message attributes in batching scenarios #2763
Conversation
This file contains 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
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. |
|
Not stale |
joaopgrassi
reviewed
Sep 29, 2022
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
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` |
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
|
|
||
| <!-- semconv messaging.rabbitmq --> | ||
| | 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. | |
There was a problem hiding this comment.
Should this be messaging.rabbitmq.**destination**.routing_key?
There was a problem hiding this comment.
Per Rabbit docs, routing key is a message property. It's also available on consumers
|
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
Add destination name - it could be per-message
lmolkova
force-pushed
the
messaging-batching-attributes
branch
from
9c0160e
to
54c07c3
Compare
|
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). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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: