-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[receiver/datadog] Address semconv noncompliance #39678
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
[receiver/datadog] Address semconv noncompliance #39678
Conversation
d067d37
to
e7a4705
Compare
79e758b
to
9c76ee6
Compare
Back to draft as we've noted some issues with span names, I'll address them. |
23b7702
to
424fb90
Compare
Addressed the issues and fixed some others. Ready for review 👍 |
Here are all tests showing this PR fixes many semconv compliance issues: HTTP server spans Spec: https://opentelemetry.io/docs/specs/semconv/http/http-spans/ DB client spans Specs: https://opentelemetry.io/docs/specs/semconv/database/database-spans/#common-attributes PostgreSQL client span Redis client spans |
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.
Looks like a great improvement to the receiver - once you resolve the linter issues we can move forward. Please let us know if you need any assistance with the linting issues.
Please resolve linting issues and conflicts and mark ready for review again. |
This addresses part of the concerns raised in open-telemetry#36924. Up until now, datadogreceiver used v1.16 of the semantic conventions, leading to mismatches in field names.
Without this, internal spans such as postgresql queries have a service.name set to postgresql.
3f35879
to
88eeb55
Compare
Rebased, fixed fmt and switched to otel-go's semconvs, which bumped them to v1.30.0. |
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.
One nit in the changelog then 👍
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description **Note: this PR addresses several things, happy to split it in multiple PRs if it helps.** In open-telemetry#36924, a number of noncompliances were identified. This PR addresses most of them: > Misalignments applying to all span types > > * A resource attribute deployment.resource.name should be defined instead of the span attribute deployment.environment: Str(production) > * A resource attribute process.pid should be defined instead of the attribute process.id Addressed by bumping semconv to latest in the collector (1.30.0) > Misalignments applying to HTTP server span > > * Adopt new OTel Semantic Conventions http.request.method and http.response.status_code instead of http.method and http.status_code > * The span name should be ${http.request.method} ${http.route}, the attribute dd.span.Resource is right for this span type > * OTTL transformation to fix the pb: set (name, attributes["dd.span.Resource"]) where name == "servlet.request" Added in 89e2bff > Misalignment applying to internal Spring Framework spans(span name spring.handler) > > Span should be kind = SpanKind.SPAN_KIND_INTERNAL > * OTTL transformation to fix the pb: set (kind, SPAN_KIND_INTERNAL) where kind == SPAN_KIND_SERVER and name == "spring.handler" > * Span name should be the Java method nam hat is carried over by the dd.span.Resource attribute > * OTTL transformation to fix the pb: `set (name, attributes["dd.span.Resource"]) where name I didn't add the span kind transformation for now as it seemed too involved for the receiver, happy to get feedback on that. Rest in 89e2bff > Misalignments applying to Database client spans > > * The resource attribute service.name should be set to the value of the span attribute _dd.base_service Done in d067d37. This might be a big change though. > * Rename the following span attributes > * db.type -> db.system > * db.operation -> db.operation.name > * db.instance -> db.name > * The span name should be set to ${db.operation.name} ${} Done in f78caac <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue open-telemetry#36924 <!--Describe what testing was performed and which tests were added.--> #### Testing Unit tests were updated. <!--Please delete paragraphs that you did not use before submitting.-->
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description **Note: this PR addresses several things, happy to split it in multiple PRs if it helps.** In open-telemetry#36924, a number of noncompliances were identified. This PR addresses most of them: > Misalignments applying to all span types > > * A resource attribute deployment.resource.name should be defined instead of the span attribute deployment.environment: Str(production) > * A resource attribute process.pid should be defined instead of the attribute process.id Addressed by bumping semconv to latest in the collector (1.30.0) > Misalignments applying to HTTP server span > > * Adopt new OTel Semantic Conventions http.request.method and http.response.status_code instead of http.method and http.status_code > * The span name should be ${http.request.method} ${http.route}, the attribute dd.span.Resource is right for this span type > * OTTL transformation to fix the pb: set (name, attributes["dd.span.Resource"]) where name == "servlet.request" Added in 89e2bff > Misalignment applying to internal Spring Framework spans(span name spring.handler) > > Span should be kind = SpanKind.SPAN_KIND_INTERNAL > * OTTL transformation to fix the pb: set (kind, SPAN_KIND_INTERNAL) where kind == SPAN_KIND_SERVER and name == "spring.handler" > * Span name should be the Java method nam hat is carried over by the dd.span.Resource attribute > * OTTL transformation to fix the pb: `set (name, attributes["dd.span.Resource"]) where name I didn't add the span kind transformation for now as it seemed too involved for the receiver, happy to get feedback on that. Rest in 89e2bff > Misalignments applying to Database client spans > > * The resource attribute service.name should be set to the value of the span attribute _dd.base_service Done in d067d37. This might be a big change though. > * Rename the following span attributes > * db.type -> db.system > * db.operation -> db.operation.name > * db.instance -> db.name > * The span name should be set to ${db.operation.name} ${} Done in f78caac <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue open-telemetry#36924 <!--Describe what testing was performed and which tests were added.--> #### Testing Unit tests were updated. <!--Please delete paragraphs that you did not use before submitting.-->
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description **Note: this PR addresses several things, happy to split it in multiple PRs if it helps.** In open-telemetry#36924, a number of noncompliances were identified. This PR addresses most of them: > Misalignments applying to all span types > > * A resource attribute deployment.resource.name should be defined instead of the span attribute deployment.environment: Str(production) > * A resource attribute process.pid should be defined instead of the attribute process.id Addressed by bumping semconv to latest in the collector (1.30.0) > Misalignments applying to HTTP server span > > * Adopt new OTel Semantic Conventions http.request.method and http.response.status_code instead of http.method and http.status_code > * The span name should be ${http.request.method} ${http.route}, the attribute dd.span.Resource is right for this span type > * OTTL transformation to fix the pb: set (name, attributes["dd.span.Resource"]) where name == "servlet.request" Added in 89e2bff > Misalignment applying to internal Spring Framework spans(span name spring.handler) > > Span should be kind = SpanKind.SPAN_KIND_INTERNAL > * OTTL transformation to fix the pb: set (kind, SPAN_KIND_INTERNAL) where kind == SPAN_KIND_SERVER and name == "spring.handler" > * Span name should be the Java method nam hat is carried over by the dd.span.Resource attribute > * OTTL transformation to fix the pb: `set (name, attributes["dd.span.Resource"]) where name I didn't add the span kind transformation for now as it seemed too involved for the receiver, happy to get feedback on that. Rest in 89e2bff > Misalignments applying to Database client spans > > * The resource attribute service.name should be set to the value of the span attribute _dd.base_service Done in d067d37. This might be a big change though. > * Rename the following span attributes > * db.type -> db.system > * db.operation -> db.operation.name > * db.instance -> db.name > * The span name should be set to ${db.operation.name} ${} Done in f78caac <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue open-telemetry#36924 <!--Describe what testing was performed and which tests were added.--> #### Testing Unit tests were updated. <!--Please delete paragraphs that you did not use before submitting.-->
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description **Note: this PR addresses several things, happy to split it in multiple PRs if it helps.** In open-telemetry#36924, a number of noncompliances were identified. This PR addresses most of them: > Misalignments applying to all span types > > * A resource attribute deployment.resource.name should be defined instead of the span attribute deployment.environment: Str(production) > * A resource attribute process.pid should be defined instead of the attribute process.id Addressed by bumping semconv to latest in the collector (1.30.0) > Misalignments applying to HTTP server span > > * Adopt new OTel Semantic Conventions http.request.method and http.response.status_code instead of http.method and http.status_code > * The span name should be ${http.request.method} ${http.route}, the attribute dd.span.Resource is right for this span type > * OTTL transformation to fix the pb: set (name, attributes["dd.span.Resource"]) where name == "servlet.request" Added in 89e2bff > Misalignment applying to internal Spring Framework spans(span name spring.handler) > > Span should be kind = SpanKind.SPAN_KIND_INTERNAL > * OTTL transformation to fix the pb: set (kind, SPAN_KIND_INTERNAL) where kind == SPAN_KIND_SERVER and name == "spring.handler" > * Span name should be the Java method nam hat is carried over by the dd.span.Resource attribute > * OTTL transformation to fix the pb: `set (name, attributes["dd.span.Resource"]) where name I didn't add the span kind transformation for now as it seemed too involved for the receiver, happy to get feedback on that. Rest in 89e2bff > Misalignments applying to Database client spans > > * The resource attribute service.name should be set to the value of the span attribute _dd.base_service Done in d067d37. This might be a big change though. > * Rename the following span attributes > * db.type -> db.system > * db.operation -> db.operation.name > * db.instance -> db.name > * The span name should be set to ${db.operation.name} ${} Done in f78caac <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue open-telemetry#36924 <!--Describe what testing was performed and which tests were added.--> #### Testing Unit tests were updated. <!--Please delete paragraphs that you did not use before submitting.-->
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description **Note: this PR addresses several things, happy to split it in multiple PRs if it helps.** In open-telemetry#36924, a number of noncompliances were identified. This PR addresses most of them: > Misalignments applying to all span types > > * A resource attribute deployment.resource.name should be defined instead of the span attribute deployment.environment: Str(production) > * A resource attribute process.pid should be defined instead of the attribute process.id Addressed by bumping semconv to latest in the collector (1.30.0) > Misalignments applying to HTTP server span > > * Adopt new OTel Semantic Conventions http.request.method and http.response.status_code instead of http.method and http.status_code > * The span name should be ${http.request.method} ${http.route}, the attribute dd.span.Resource is right for this span type > * OTTL transformation to fix the pb: set (name, attributes["dd.span.Resource"]) where name == "servlet.request" Added in 89e2bff > Misalignment applying to internal Spring Framework spans(span name spring.handler) > > Span should be kind = SpanKind.SPAN_KIND_INTERNAL > * OTTL transformation to fix the pb: set (kind, SPAN_KIND_INTERNAL) where kind == SPAN_KIND_SERVER and name == "spring.handler" > * Span name should be the Java method nam hat is carried over by the dd.span.Resource attribute > * OTTL transformation to fix the pb: `set (name, attributes["dd.span.Resource"]) where name I didn't add the span kind transformation for now as it seemed too involved for the receiver, happy to get feedback on that. Rest in 89e2bff > Misalignments applying to Database client spans > > * The resource attribute service.name should be set to the value of the span attribute _dd.base_service Done in d067d37. This might be a big change though. > * Rename the following span attributes > * db.type -> db.system > * db.operation -> db.operation.name > * db.instance -> db.name > * The span name should be set to ${db.operation.name} ${} Done in f78caac <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue open-telemetry#36924 <!--Describe what testing was performed and which tests were added.--> #### Testing Unit tests were updated. <!--Please delete paragraphs that you did not use before submitting.-->
Description
Note: this PR addresses several things, happy to split it in multiple PRs if it helps.
In #36924, a number of noncompliances were identified. This PR addresses most of them:
Addressed by bumping semconv to latest in the collector (1.30.0)
Added in 89e2bff
I didn't add the span kind transformation for now as it seemed too involved for the receiver, happy to get feedback on that. Rest in 89e2bff
Done in d067d37. This might be a big change though.
Done in f78caac
Link to tracking issue
#36924
Testing
Unit tests were updated.