-
Notifications
You must be signed in to change notification settings - Fork 608
feat(instrumentation-amqp): adds stable semantic conventions #2976
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
base: main
Are you sure you want to change the base?
feat(instrumentation-amqp): adds stable semantic conventions #2976
Conversation
@lucas-gregoire - looks like the messaging semantic conventions are not actually stable yet, so this PR is blocked until is is formally marked as such. (Unless this is part of an effort to create prototypes to mark the messaging semconv as stable, if that is the case, please link the corresponding issue from SemConv here, thanks 🙂) |
ATTR_MESSAGING_SYSTEM, | ||
MESSAGING_OPERATION_TYPE_VALUE_PROCESS, | ||
MESSAGING_OPERATION_TYPE_VALUE_SEND, | ||
} from '@opentelemetry/semantic-conventions/incubating'; |
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.
Will need to move this to a local "src/semconv.ts" file rather than using the '/incubating' entry-point directly, per https://github.com/open-telemetry/opentelemetry-js/blob/main/semantic-conventions/README.md#unstable-semconv
You can typically used the https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/scripts/gen-semconv-ts.js tool to generate the src/semconv.ts file.
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.
See comments above.
Some context: We discussed this in the OTel JS SIG call today. Doing semconv changes to this instrumentation to get it closer to the current semconv spec state isn't blocked until messaging semconv is stabilized. However, because messaging semconv isn't stable yet, we cannot call these changes "stable" and we probably shouldn't use the |
Which problem is this PR solving?
Currently, the
amqplib
instrumentation only exports legacy attributes.However, according to the OpenTelemetry specifications, we will have to migrate to new attributes to ensure better compatibility and consistency across tools and libraries.
The v1.36.0+ conventions are not yet stable but will become stable in the future, so I think it's important to begin implementing the export of these new attributes, in order to anticipate the deprecation of the legacy ones, improve interoperability with other tools, and align with community best practices.
Short description of the changes
Adds support for stable semantic conventions in the amqplib instrumentation, controlled by the
OTEL_SEMCONV_STABILITY_OPT_IN=messaging
environment variable.Key changes:
I used these specifications for implementing new attributes:
Migration guide
When upgrading to the new semantic conventions, it is recommended to follow this migration path:
@opentelemetry/instrumentation-amqplib
to the latest versionOTEL_SEMCONV_STABILITY_OPT_IN=messaging/dup
to emit both old and new semantic conventionsOTEL_SEMCONV_STABILITY_OPT_IN=messaging
to emit only the new semantic conventions