-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fixing OpenTelemetryTraces to include the GA V1 semantic conventionttributes #45929
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
Fixing OpenTelemetryTraces to include the GA V1 semantic conventionttributes #45929
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.
Pull Request Overview
This PR enhances OpenTelemetry tracing in the Azure Cosmos SDK by emitting both Pre-V1 and GA V1 semantic convention attributes by default and making this behavior configurable via an environment variable.
- Introduce
AttributeNamingSchemeand integrate it intoCosmosClientTelemetryConfig. - Refactor tracer creation to use
LibraryTelemetryOptions. - Extend
DiagnosticsProviderto conditionally emit span attributes according to the selected naming scheme. - Add configuration support in
Configsfor the new environment variable and update tests accordingly.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
CosmosClientTelemetryConfig.java |
Added attributeNamingSchemes, getOrCreateTracer refactor, and new getter/setter for naming scheme. |
AttributeNamingScheme.java |
New enum to parse and represent Pre-V1 and V1 schemes. |
AttributeNamesV1.java |
New enum listing GA V1 semantic convention attribute names. |
AttributeNamesPreV1.java |
New enum listing Pre-V1 semantic convention attribute names. |
AttributeNamesCommon.java |
New enum listing common attribute names. |
ImplementationBridgeHelpers.java |
Extended accessor interface to include naming-scheme methods. |
DiagnosticsProvider.java |
Updated span construction to emit attributes by naming scheme. |
Configs.java |
Added environment variable support for OTEL_SPAN_ATTRIBUTE_NAMING_SCHEME. |
CosmosTracerTest.java |
Updated tests to cover and validate the new naming-scheme behavior. |
Comments suppressed due to low confidence (3)
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/ImplementationBridgeHelpers.java:1643
- The method name 'setOtelSpanAttributeNamingSchema' (and its getter) uses 'Schema' while the corresponding methods in CosmosClientTelemetryConfig use 'Scheme'. This mismatch will break the implementation bridge — rename to 'setOtelSpanAttributeNamingScheme' and 'getOtelSpanAttributeNamingScheme'.
CosmosClientTelemetryConfig setOtelSpanAttributeNamingSchema(
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/models/CosmosClientTelemetryConfig.java:468
- This setter is package-private, but it configures a public SDK behavior. Consider making it
publicto allow users to fluently configure the naming scheme on the client builder.
CosmosClientTelemetryConfig setOtelSpanAttributeNamingScheme(String name) {
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/models/CosmosClientTelemetryConfig.java:474
- [nitpick] Public-facing getters and setters for the new naming scheme should have JavaDoc explaining their purpose and valid values (e.g., 'All', 'Pre_V1_Release', 'V1').
EnumSet<AttributeNamingScheme> getOtelSpanAttributeNamingScheme() {
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DiagnosticsProvider.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/DiagnosticsProvider.java
Show resolved
Hide resolved
…ntation/DiagnosticsProvider.java Co-authored-by: Copilot <[email protected]>
|
/azp run java - cosmos - tests |
|
/azp run java - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run java - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run java - cosmos - spark |
|
/azp run java - cosmos - kafka |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run java - cosmos - tests |
|
/azp run java - cosmos - kafka |
|
/azp run java - cosmos - spark |
|
Azure Pipelines successfully started running 1 pipeline(s). |
2 similar comments
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Azure Pipelines successfully started running 1 pipeline(s). |
...mos/src/main/java/com/azure/cosmos/implementation/clienttelemetry/AttributeNamingScheme.java
Outdated
Show resolved
Hide resolved
...mos/src/main/java/com/azure/cosmos/implementation/clienttelemetry/AttributeNamingScheme.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/Configs.java
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/Configs.java
Show resolved
Hide resolved
|
/azp run java - cosmos - tests |
|
/azp run java - cosmos - kafka |
|
/azp run java - cosmos - spark |
|
Azure Pipelines successfully started running 1 pipeline(s). |
2 similar comments
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Azure Pipelines successfully started running 1 pipeline(s). |
...mos/src/main/java/com/azure/cosmos/implementation/clienttelemetry/AttributeNamingScheme.java
Outdated
Show resolved
Hide resolved
…ntation/clienttelemetry/AttributeNamingScheme.java Co-authored-by: Annie Liang <[email protected]>
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.
LGTM, thanks
|
/azp run java - cosmos - spark |
|
/azp run java - cosmos - kafka |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run java - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
Description
The original version of OpenTelemetry tracing was based off of a pre-V1 semantic convention spec. This PR ensures that by default the attributes for both the Pre-V1 spec as well as the V1 spec are emitted. Optionally via the environment variable
COSMOS_OTEL_SPAN_ATTRIBUTE_NAMING_SCHEMEor system propertyCOSMOS.OTEL_SPAN_ATTRIBUTE_NAMING_SCHEMEthe behavior can also be changed to only emit the Pre-V1 or only the V1 attributes (for slightly better efficiency). Valid values areALL,V1andPRE_V1_RELEASE.All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines