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

[exporter/datadog] Add feature flag for updated operation and resourc… #36419

Merged

Conversation

IbraheemA
Copy link
Contributor

@IbraheemA IbraheemA commented Nov 18, 2024

…e name logic

Description

Datadog associates two name fields with each span: "resource name" and "operation name." Resource name is analogous to span.name in OTel, but operation name has no 1:1 correspondence with any OTel fields. (Read more about what operation name respresents here: https://docs.datadoghq.com/tracing/guide/configuring-primary-operation/)

Previously, the datadog exporter would compute operation name as {scope name}.{span kind}. This would result in long, difficult to read operation names in the Datadog UI - a prior discussion on this issue can be found here.

In this PR, add a feature gate datadog.EnableOperationAndResourceNameV2. This gate enables new logic for operation and resource name that improves readability of these fields by producing shorter names and better aligning with the OTel spec.

The names computed in the new logic follow those computed in dd-trace-go.

Testing

Unit tests and E2E tests for the new logic are done in the datadog-agent repo. Added a test in this repo that verifies that setting the feature gate enables the new logic; also ran the collector with the feature flag and verified new logic manually.

Documentation

Changelog added

Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

Generated code is out of date, please run "make generate" and commit the changes in this PR.

component: datadogexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Add a feature gate exporter.datadogexporter.EnableOperationAndResourceNameV2. Enabling this gate modifies the logic for computing operation and resource names from OTLP spans to produce shorter, more readable names and improve alignment with OpenTelemetry specifications."
Copy link
Member

Choose a reason for hiding this comment

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

Is there a public doc that we can link here for customers to know exactly what changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet - should I write one up before I merge this?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, please coordinate with Anna

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a discussion today, we decided we'll merge first and add a link to the announcement/migration guide afterwards.

Copy link

@JM89 JM89 Jan 24, 2025

Choose a reason for hiding this comment

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

Hi @IbraheemA , customer here :D Where can I find the migration guide, please, I am very interested :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@IbraheemA IbraheemA requested a review from songy23 November 19, 2024 15:03
Copy link
Contributor

github-actions bot commented Dec 4, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added Stale and removed Stale labels Dec 4, 2024
@songy23 songy23 added the never stale Issues marked with this label will be never staled and automatically removed label Dec 7, 2024
@songy23
Copy link
Member

songy23 commented Jan 9, 2025

@IbraheemA needs a rebase

@IbraheemA IbraheemA force-pushed the ibraheem/add-gate-for-operation-name-v2 branch from 0ad2755 to 765e1e1 Compare January 9, 2025 18:02
@IbraheemA IbraheemA requested a review from songy23 January 16, 2025 22:54
@IbraheemA IbraheemA requested a review from songy23 January 17, 2025 17:56
@songy23 songy23 merged commit 7254f00 into open-telemetry:main Jan 17, 2025
164 checks passed
@github-actions github-actions bot added this to the next release milestone Jan 17, 2025
chengchuanpeng pushed a commit to chengchuanpeng/opentelemetry-collector-contrib that referenced this pull request Jan 26, 2025
open-telemetry#36419)

…e name logic

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Datadog associates two name fields with each span: "resource name" and
"operation name." Resource name is analogous to `span.name` in OTel, but
operation name has no 1:1 correspondence with any OTel fields. (Read
more about what operation name respresents here:
https://docs.datadoghq.com/tracing/guide/configuring-primary-operation/)

Previously, the datadog exporter [would compute operation name as
`{scope name}.{span
kind}`](https://github.com/DataDog/datadog-agent/blob/8b92185df483f39456c41766b5666a2bd22307e6/pkg/trace/traceutil/otel_util.go#L367).
This would result in long, difficult to read operation names in the
Datadog UI - [a prior discussion on this issue can be found
here](open-telemetry#1909).

In this PR, add a feature gate
`datadog.EnableOperationAndResourceNameV2`. This gate enables new logic
for operation and resource name that improves readability of these
fields by producing shorter names and better aligning with the OTel
spec.

The names computed in the new logic follow those computed in
[dd-trace-go](https://github.com/DataDog/dd-trace-go).

<!--Describe what testing was performed and which tests were added.-->
#### Testing

Unit tests and E2E tests for the new logic are done in the
`datadog-agent` repo. Added a test in this repo that verifies that
setting the feature gate enables the new logic; also ran the collector
with the feature flag and verified new logic manually.

<!--Describe the documentation added.-->
#### Documentation

Changelog added

<!--Please delete paragraphs that you did not use before submitting.-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connector/datadog exporter/datadog Datadog components never stale Issues marked with this label will be never staled and automatically removed pkg/datadog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants