Skip to content
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

Open Telemetry: Fixes attribute name following otel convention #4765

Merged

Conversation

sourabh1007
Copy link
Contributor

@sourabh1007 sourabh1007 commented Oct 8, 2024

Description

What is the change?

db.cosmosdb.operation_type --> removed (moved to classic sdk support)
db.cosmosdb.machine_id --> removed (moved to classic sdk support)
db.cosmosdb.client_id --> db.cosmosdb.client.id it will remain same (aligns with OTel messaging conventions)
db.cosmosdb.request_content_length --> db.cosmosdb.request.content_length it will remain same (aligns with OTel HTTP conventions)
db.cosmosdb.response_content_length --> db.cosmosdb.request.content_length it will remain same (aligns with OTel HTTP conventions)
db.cosmosdb.status_code --> db.response.status_code (moved db.cosmosdb.status_code to classic sdk support)
db.cosmosdb.consistency_level -> added
db.cosmosdb.request_content_length_bytes -> added for classic sdk support

Why?

Lot of discussions are going on with Azure Core and Open Telemetry Community teams. As part of that discussion, we are making these changes as our goal is to be Open Telemetry and Azure compliant,

Type of change

Please delete options that are not relevant.

  • [] Bug fix (non-breaking change which fixes an issue)

Closing issues

To automatically close an issue: closes #4553

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good!

@sourabh1007 sourabh1007 changed the title Open Telemetry : fixes attribute name follwing otel convention Open Telemetry : Fixes attribute name following otel convention Oct 8, 2024
@sourabh1007 sourabh1007 changed the title Open Telemetry : Fixes attribute name following otel convention Open Telemetry: Fixes attribute name following otel convention Oct 8, 2024
@sourabh1007 sourabh1007 marked this pull request as ready for review October 8, 2024 08:26
Copy link
Contributor

@philipthomas-MSFT philipthomas-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Just some encouragement to provide more explicit context on why these changes are important.

@sourabh1007 sourabh1007 self-assigned this Oct 8, 2024
@sourabh1007 sourabh1007 force-pushed the users/sourabhjain/tracesnamechanges branch from a6f3081 to 4aedd08 Compare October 8, 2024 18:28
@sourabh1007 sourabh1007 force-pushed the users/sourabhjain/tracesnamechanges branch 2 times, most recently from 4f23195 to 1b42f26 Compare October 9, 2024 11:50
kirankumarkolli
kirankumarkolli previously approved these changes Oct 9, 2024
@lmolkova
Copy link
Member

lmolkova commented Oct 9, 2024

The proposal looks good to me!

There is a caveat that we can't update OTel semantic conventions with this:

db.cosmosdb.client_id --> db.cosmosdb.client.id (aligns with OTel messaging conventions)
db.cosmosdb.request_content_length --> db.request.content_length (aligns with OTel HTTP conventions)
db.cosmosdb.response_content_length --> db.response.content_length (aligns with OTel HTTP conventions)

because of some tooling/process issues. I'm working on addressing them, but in the meantime we'll need to either hold back this part of the change or be inconsistent with what's documented in the OTel today until tooling problems are resolved.

@kirankumarkolli
Copy link
Member

The proposal looks good to me!

There is a caveat that we can't update OTel semantic conventions with this:

db.cosmosdb.client_id --> db.cosmosdb.client.id (aligns with OTel messaging conventions)
db.cosmosdb.request_content_length --> db.request.content_length (aligns with OTel HTTP conventions)
db.cosmosdb.response_content_length --> db.response.content_length (aligns with OTel HTTP conventions)

because of some tooling/process issues. I'm working on addressing them, but in the meantime we'll need to either hold back this part of the change or be inconsistent with what's documented in the OTel today until tooling problems are resolved.

What's the timeline for tooling readiness?

@sourabh1007 sourabh1007 force-pushed the users/sourabhjain/tracesnamechanges branch from 8cff9df to d9a96a2 Compare October 10, 2024 03:12
@sourabh1007 sourabh1007 force-pushed the users/sourabhjain/tracesnamechanges branch from edeb94f to e49571c Compare October 10, 2024 06:06
@sourabh1007 sourabh1007 added the auto-merge Enables automation to merge PRs label Oct 11, 2024
@sourabh1007
Copy link
Contributor Author

The proposal looks good to me!
There is a caveat that we can't update OTel semantic conventions with this:

db.cosmosdb.client_id --> db.cosmosdb.client.id (aligns with OTel messaging conventions)
db.cosmosdb.request_content_length --> db.request.content_length (aligns with OTel HTTP conventions)
db.cosmosdb.response_content_length --> db.response.content_length (aligns with OTel HTTP conventions)

because of some tooling/process issues. I'm working on addressing them, but in the meantime we'll need to either hold back this part of the change or be inconsistent with what's documented in the OTel today until tooling problems are resolved.

What's the timeline for tooling readiness?

We discussed this with @lmolkova and estimated that tool readiness would take about one month. However, we agreed it's not a high-priority task, as we're comfortable with the current naming, provided the OpenTelemetry community accepts it. We would, however, be hesitant to make changes in the future.

@microsoft-github-policy-service microsoft-github-policy-service bot merged commit 72eed8a into master Oct 15, 2024
23 checks passed
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the users/sourabhjain/tracesnamechanges branch October 15, 2024 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Enables automation to merge PRs Telemetry
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Distributed Tracing/Open Telemetry: Feature requests
5 participants