feat: Update attributes to match semconv in racecar #1613
Conversation
thompson-tomo
requested review from
fbogsany,
mwear,
robertlaurin,
dazuma,
ericmustin,
arielvalentin,
ahayworth,
plantfansam,
robbkidd,
simi,
kaylareopelle,
xuan-cao-swi and
a team
as code owners
thompson-tomo
changed the title
There was a problem hiding this comment.
Hi @thompson-tomo! 👋 Thank you for this PR and for your patience with our review!
Since the current attributes adhere to a different semantic conventions version, I think we ought to add the OTEL_SEMCONV_STABILITY_OPT_IN environment variable to conditionally apply the new span name and add the new attributes. We'll keep the environment variable in place until the messaging conventions go stable.
To minimize performance overhead, our current pattern to implement the environment variable is to create different modules for the instrumentation that represent the old (what's currently shipped with the instrumentation), dup (both old and new), and stable (only new) attributes and span names. Then, at installation time, we prepend the module that corresponds with the user's configuration.
This PR is a good example of the pattern: #1592
For HTTP and DB conventions, the triplication feels easier to swallow because it's just going to be around for six months. I'm not sure when the messaging conventions will go stable, so this triple-module system may need to stick around longer.
What are your thoughts on the three-module approach for the messaging conventions? (I'll also make a note to add this to the SIG discussion on Tuesday at 9am PT, which you're welcome to join!)
| * Renamed messaging.kafka.message_key to messaging.kafka.message.key to match semantic conventions. | ||
|
|
||
| * Renamed messaging.destination to messaging.destination.name to match semantic conventions. | ||
|
|
||
| * Renamed messaging.kafka.partition to messaging.destination.partition.id to match semantic conventions. | ||
|
|
||
| * Removed messaging.destination_kind to match semantic conventions. | ||
|
|
||
| * Add messaging.operation.type attribute as per semantic conventions. |
There was a problem hiding this comment.
Our release tooling will add the changelog entry. The default is based on the name of the PR. I'm happy to edit the entry to match this text, but we'll have to wait until the release phase to do so.
|
No worries, so I have been looking at that stability guidance but I have concerns about how to achieve it in non-stable signals based on the current phrasing. As such I have proposed changes to the requirement levels to make this transition easier. open-telemetry/semantic-conventions#2607 it would be good to get additional feedback. |
Progresses #1583
Open questions: