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

Add feature gate for refactor of OTLP->Datadog span translation #37171

Conversation

IbraheemA
Copy link
Contributor

Description

Add a feature gate exporter.datadogexporter.EnableReceiveResourceSpansV2. Enabling this gate uses a refactored implementation of OTLP->Datadog Span translation which improves performance by 10%, and deprecates the following functionality:
- No longer checks for resource-related values (container, env, hostname) in span attributes. This previous behavior did not follow the OTel spec.

After a migration period of 6 versions, we plan to change this to opt-out, then deprecate the old functionality after another 6 versions.

Link to tracking issue

#37077

Testing

Add unit tests for new functionality (verify that relevant span attributes aren't used). Ran collector with new feature flag enabled.

Documentation

Added changelog entry

@IbraheemA IbraheemA requested a review from dineshg13 January 13, 2025 21:33
connector/datadogconnector/connector.go Outdated Show resolved Hide resolved
connector/datadogconnector/go.mod Outdated Show resolved Hide resolved
exporter/datadogexporter/traces_exporter.go Outdated Show resolved Hide resolved
exporter/datadogexporter/traces_exporter_test.go Outdated Show resolved Hide resolved
internal/datadog/gates.go Outdated Show resolved Hide resolved
@github-actions github-actions bot added the cmd/otelcontribcol otelcontribcol command label Jan 14, 2025
@IbraheemA IbraheemA requested review from mx-psi and songy23 January 15, 2025 18:34
@@ -127,6 +127,7 @@ extension/sumologicextension/ @open-telemetry/collector-cont
internal/aws/ @open-telemetry/collector-contrib-approvers @Aneurysm9 @mxiamxia
internal/collectd/ @open-telemetry/collector-contrib-approvers @atoulme
internal/coreinternal/ @open-telemetry/collector-contrib-approvers @open-telemetry/collector-approvers
internal/datadog/ @open-telemetry/collector-contrib-approvers @IbraheemA @songy23
Copy link
Member

Choose a reason for hiding this comment

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

We already have pkg/datadog, what is the benefit of having internal/datadog? From what I can tell they are the same


import "go.opentelemetry.io/collector/featuregate"

var ReceiveResourceSpansV2FeatureGate = featuregate.GlobalRegistry().MustRegister(
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest moving this file to pkg/datadog then delete internal/datadog. Not worth adding a new module AFAICS

@@ -26,7 +26,7 @@ confmap/provider/s3provider/ @open-telemetry/collector-cont
confmap/provider/secretsmanagerprovider/ @open-telemetry/collector-contrib-approvers @driverpt @atoulme

connector/countconnector/ @open-telemetry/collector-contrib-approvers @djaglowski @jpkrohling
connector/datadogconnector/ @open-telemetry/collector-contrib-approvers @mx-psi @dineshg13 @ankitpatel96 @jade-guiton-dd
connector/datadogconnector/ @open-telemetry/collector-contrib-approvers @mx-psi @dineshg13 @ankitpatel96 @jade-guiton-dd @IbraheemA
Copy link
Member

Choose a reason for hiding this comment

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

You can add yourself to pkg/datadog too

@songy23
Copy link
Member

songy23 commented Jan 15, 2025

needs make gotidy

@dmitryax dmitryax merged commit c47c746 into open-telemetry:main Jan 15, 2025
164 checks passed
@github-actions github-actions bot added this to the next release milestone Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants