Add stable messaging.kafka.offset attribute via SemconvStability (Fixes #15723)#17785
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for the stable Kafka messaging offset semantic convention (messaging.kafka.offset) behind existing semantic-convention stability toggles, while preserving the deprecated attribute for backwards compatibility.
Changes:
- Added messaging-specific semconv stability toggles to
SemconvStability(messagingandmessaging/dup). - Updated Kafka producer/consumer attribute extractors to emit
messaging.kafka.offsetwhen stable messaging semconv is enabled. - Retained emission of
messaging.kafka.message.offsetwhen old messaging semconv is enabled.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
instrumentation/kafka/kafka-clients/kafka-clients-common-0.11/library/.../KafkaProducerAttributesExtractor.java |
Conditionally emits stable vs deprecated Kafka offset attribute based on SemconvStability flags. |
instrumentation/kafka/kafka-clients/kafka-clients-common-0.11/library/.../KafkaConsumerAttributesExtractor.java |
Conditionally emits stable vs deprecated Kafka offset attribute based on SemconvStability flags. |
instrumentation-api/src/main/java/.../SemconvStability.java |
Introduces messaging semconv stability toggles (old/stable) consistent with existing database/rpc/service.peer toggles. |
| @@ -54,8 +59,36 @@ public final class SemconvStability { | |||
|
|
|||
| emitOldRpcSemconv = shouldEmitOld("rpc", v3Preview, optInValues); | |||
| emitStableRpcSemconv = shouldEmitStable("rpc", v3Preview, optInValues); | |||
|
|
|||
| emitOldMessagingSemconv = shouldEmitOld("messaging", v3Preview, optInValues); | |||
| emitStableMessagingSemconv = shouldEmitStable("messaging", v3Preview, optInValues); | |||
| } | |||
There was a problem hiding this comment.
This introduces new SemconvStability opt-in keys for "messaging" and "messaging/dup" that change emitted attributes across instrumentations, but there’s no direct verification that these new flags behave as intended (stable-only vs old-only vs both). Consider adding a focused test (or extending an existing semconv-stability integration test) that asserts the behavior for the new messaging keys to prevent accidental behavior changes.
Tusharika725
force-pushed
the
fix-kafka-offset-semconv
branch
from
90d8213 to
75ed427
Compare
| private static final boolean emitOldMessagingSemconv; | ||
| private static final boolean emitStableMessagingSemconv; |
There was a problem hiding this comment.
I think I'd prefer to put these breaking changes under v3 preview flag, since it's probably going to be a while for stable messaging, and I'd also like to get away from our pattern of adding "stable" flag but then introducing further breaking changes under the stable flag while we build out the stable telemetry
@laurit wdyt?
There was a problem hiding this comment.
It depends on how we are going to treat this. If we wish to do this change in v3 then v3 preview would be a good fit. If we treat this as part of the messaging semconv stabilization, which is not going to be ready for v3, then the semconv stability opt-in would be better. My understanding is that this is the first step for implementing the latest messaging semconv so using the opt-in flag seems fine to me.
There was a problem hiding this comment.
My understanding is that this is the first step for implementing the latest messaging semconv so using the opt-in flag seems fine to me.
ya, makes sense
I still don't love our existing practice of the OTEL_SEMCONV_STABILITY_OPT_IN=database flag not following our stability guidelines (it looks like a stable option, but it's not since it's essentially in-progress work)
messaging would be a good chance to switch, since the others will hopefully get wrapped up by 3.0 and this will hopefully be the only one to extend into 3.x
maybe we could use OTEL_SEMCONV_STABILITY_PREVIEW=messaging?
|
Thank you for your contribution @Tusharika725! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. |
Fixes #15723
This PR addresses the deprecated messaging string by deploying feature toggles for the Messaging Semantic Conventions and wiring them into the Kafka interceptors.
Changes:
Added emitStableMessagingSemconv() and emitOldMessagingSemconv() to SemconvStability.java.
Wired the stable messaging.kafka.offset attribute into KafkaProducerAttributesExtractor and KafkaConsumerAttributesExtractor while maintaining the old attribute for messaging/dup compatibility.
Note for Reviewers: I am contributing from a machine with strict memory constraints and was unable to spin up the Testcontainers/Docker environment locally to verify KafkaClientDefaultTest without freezing my system. I am relying on the CI pipeline to catch the expected assertion failure so I can update the test accordingly. Thank you for your patience!