-
Notifications
You must be signed in to change notification settings - Fork 628
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
[Ep2024-sprints][gh-1638] - Bump aio-pika schema version from 1.11.0 to 1.26.0 #2705
base: main
Are you sure you want to change the base?
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.
Question is: should be keep some kind of compat for current users or not? Messaging is still experimental so it's not required but still.
@xrmx From my understanding here: #2351, it is up to us to decide whether to include an opt-in mode for instrumentation, but I think this case is different since the attributes are in |
Pushed a bigger change that should ideally be reviewed by someone with more intimate knowledge of RabbitMQ. Previously, the We can validate this via:
This PR, in addition to updating the instrumentation to follow the spec more closely, also follows the zen of python “Explicit is better than implicit.” - what is “destination”? The destination has multiple dimensions, and both exchange & routing_key carry identifying information. |
MessagingOperationValues, | ||
SpanAttributes, | ||
from opentelemetry.semconv._incubating.attributes import ( | ||
messaging_attributes as SpanAttributes, |
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.
After the new semconv package reorganization people are just importing the variables straight from their module, I guess for the long names. In this case something like:
from opentelemetry.semconv._incubating.attributes.messaging_attributes import MESSAGING_DESTINATION_NAME, MESSAGING_RABBITMQ_DESTINATION_ROUTING_KEY, MESSAGING_MESSAGE_ID, MESSAGING_MESSAGE_CONVERSATION_ID, SpanAttributes.MESSAGING_OPERATION_TYPE, SpanAttributes.MESSAGING_OPERATION_TYPE]
from opentelemetry.semconv._incubating.attributes.net_attributes import NET_PEER_NAME, NET_PEER_PORT
Description
Make sure aio-pika instrumentation follows semantic conventions.
Fixes #1638
Breaking Attribute Changes:
messaging.destination
tomessaging.destination.name
,messaging.rabbitmq.destination.routing_key
messaging.conversation_id
tomessaging.message.conversation_id
messaging.temp_destination
tomessaging.destination.temporary
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.