Fix/change some messaging attributes to match conventions #1624
Conversation
According to docs [here][1] it should be `consumer.group` instead of snake_case `consumer_group`. [1]: https://opentelemetry.io/docs/specs/semconv/messaging/kafka/#span-attributes
According to docs [here][1] it should be `message.key` instead of snake_case `message_key`. [1]: https://opentelemetry.io/docs/specs/semconv/messaging/kafka/#span-attributes
There is already `messaging.kafka.partition`, but [docs][1] have `messaging.kafka.destination.partition` instead. I currently kept the old attribute for backwards compatibility. [1]: https://opentelemetry.io/docs/specs/semconv/messaging/kafka/#span-attributes
s22su
requested review from
fbogsany,
mwear,
robertlaurin,
dazuma,
ericmustin,
arielvalentin,
ahayworth,
plantfansam,
robbkidd and
kaylareopelle
as code owners
The committers listed above are authorized under a signed CLA. |
|
👋 Hi @s22su! Thank you for your contribution! I'm not sure about the exact status of the semantic conventions gem right now. The approvers/maintainers spoke in January about wanting to rework it. I'll bring your PR up during the SIG meeting tomorrow and try to get some next steps. |
|
@kaylareopelle do you have any updates on this? |
|
Hi @s22su, I'm sorry for taking so long to get back to you on this. Our approach to semantic conventions is in flux. Our current approach is brittle for a number of reasons, including we don't have a great way to handle removals of constants without potentially breaking things for users upstream. We're grateful that you pointed out the changes in the messaging conventions and opened this PR. Your PR reinvigorated our discussion about #1567, opened by @robbkidd. This PR presents an alternate approach for the semantic conventions gem. As an alternative to your PR, we'd like to update the messaging conventions portion of #1567 to include the latest content. Would you be willing to test out that PR, once we've updated it, to see if it works in your environment? |
|
@kaylareopelle it seems to me like the intended usage of the gem after linked PR is merged, will be to update When next version will be released I'll happily use it in our internal library. Feel free to close this PR, I'll keep an eye on #1567 to know when I can update. |
|
Thanks for your understanding, @s22su. |
I used
opentelemetry-semantic_conventionsin one of my projects and noticed that some attributes are different from the docs on following pages:All kafka attributes described in docs are present in
opentelemetry-semantic_conventionsgem now.Messaging batch count is added because I wat to use it in my code and it was missing before.