Skip to content

feat(datadogexporter): adds remap_metrics feature gate #35025

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

Merged

Conversation

mabdinur
Copy link
Contributor

@mabdinur mabdinur commented Sep 5, 2024

Description:

Adds a metrics configuration that enables/disables the conversion of OpenTelemetry metrics to Datadog semantics in the Datadog Exporter. This conversion will soon occur in a datadog semantic processor.

Link to tracking Issue: N/A

Testing: Unit tests

̶̶D̶o̶c̶u̶m̶e̶n̶t̶a̶t̶i̶o̶n̶:̶̶̶ O̶n̶c̶e̶ t̶h̶i̶s̶ c̶o̶n̶f̶i̶g̶u̶r̶a̶t̶i̶o̶n̶ i̶s̶ G̶A̶ w̶e̶ w̶i̶l̶l̶ u̶p̶d̶a̶t̶e̶ t̶h̶e̶ d̶o̶c̶s̶ h̶e̶r̶e̶:̶ h̶t̶t̶p̶s̶:̶//d̶o̶c̶s̶.d̶a̶t̶a̶d̶o̶g̶h̶q̶.c̶o̶m̶/o̶p̶e̶n̶t̶e̶l̶e̶m̶e̶t̶r̶y̶/c̶o̶l̶l̶e̶c̶t̶o̶r̶_̶e̶x̶p̶o̶r̶t̶e̶r̶/c̶o̶n̶f̶i̶g̶u̶r̶a̶t̶i̶o̶n̶/

@github-actions github-actions bot added the exporter/datadog Datadog components label Sep 5, 2024
@mabdinur mabdinur marked this pull request as ready for review September 5, 2024 20:55
@mabdinur mabdinur requested a review from a team September 5, 2024 20:55
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.

Why does this need to be a config, i.e. when does a customer want to disable it?

If this is only for internal migration to semantic core, we don't need to expose it

@mabdinur
Copy link
Contributor Author

mabdinur commented Sep 5, 2024

Why does this need to be a config, i.e. when does a customer want to disable it?

If this is only for internal migration to semantic core, we don't need to expose it

It is internal to semantic-core. Good point we should avoid updating documentation.

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.

Overall I still don't quite understand why we need this PR. I think doing the remapping is mandatory, remapping should never be turned off at any time, otherwise users will have broken user experience. It is an implementation detail how we do the remapping and end users probably wouldn't care

@mabdinur
Copy link
Contributor Author

mabdinur commented Sep 6, 2024

Overall I still don't quite understand why we need this PR. I think doing the remapping is mandatory, remapping should never be turned off at any time, otherwise users will have broken user experience. It is an implementation detail how we do the remapping and end users probably wouldn't care

Good point. We need a mechanism to disable remapping in the Exporter so we can handle this logic in a Processor. Remapping will always occur we are just changing where this can happen. End user's will not set this configuration. I will move this change back to a draft state and update this PR to use a feature gate instead.

@mabdinur mabdinur marked this pull request as draft September 6, 2024 17:04
@mabdinur mabdinur changed the title feat(datadogexporter): adds remap_metrics configuration feat(datadogexporter): adds remap_metrics feature gate Sep 9, 2024
@mabdinur mabdinur force-pushed the munir/add-option-to-avoid-remapping branch from 9e31b0f to 8bbbc5a Compare September 9, 2024 21:11
@mabdinur mabdinur requested a review from songy23 September 9, 2024 22:32
@mabdinur mabdinur marked this pull request as ready for review September 9, 2024 22:32
@mabdinur
Copy link
Contributor Author

mabdinur commented Sep 9, 2024

@songy23 How should I test this change?

@songy23
Copy link
Member

songy23 commented Sep 10, 2024

You can follow the similar in DataDog/datadog-agent#28228 (review) with make otelcontribcol. Run the binary with the feature gate disabled and check no DD conventions are sent to the backend.

@mabdinur mabdinur force-pushed the munir/add-option-to-avoid-remapping branch from c2e417e to fcd9aad Compare September 11, 2024 19:56
@mabdinur mabdinur force-pushed the munir/add-option-to-avoid-remapping branch from fcd9aad to b38e973 Compare September 11, 2024 19:58
@mabdinur mabdinur requested a review from mx-psi September 11, 2024 20:30
@mx-psi mx-psi merged commit 8243d20 into open-telemetry:main Sep 16, 2024
155 of 156 checks passed
@github-actions github-actions bot added this to the next release milestone Sep 16, 2024
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this pull request Oct 4, 2024
…y#35025)

**Description:** <Describe what has changed.>

Adds a metrics configuration that enables/disables the conversion of
OpenTelemetry metrics to Datadog semantics in the Datadog Exporter. This
conversion will soon occur in a datadog semantic processor.

**Link to tracking Issue:** N/A

**Testing:** Unit tests

*̶*̶D̶o̶c̶u̶m̶e̶n̶t̶a̶t̶i̶o̶n̶:̶*̶*̶ O̶n̶c̶e̶ t̶h̶i̶s̶
c̶o̶n̶f̶i̶g̶u̶r̶a̶t̶i̶o̶n̶ i̶s̶ G̶A̶ w̶e̶ w̶i̶l̶l̶ u̶p̶d̶a̶t̶e̶ t̶h̶e̶
d̶o̶c̶s̶ h̶e̶r̶e̶:̶
h̶t̶t̶p̶s̶:̶//d̶o̶c̶s̶.d̶a̶t̶a̶d̶o̶g̶h̶q̶.c̶o̶m̶/o̶p̶e̶n̶t̶e̶l̶e̶m̶e̶t̶r̶y̶/c̶o̶l̶l̶e̶c̶t̶o̶r̶_̶e̶x̶p̶o̶r̶t̶e̶r̶/c̶o̶n̶f̶i̶g̶u̶r̶a̶t̶i̶o̶n̶/

---------

Co-authored-by: Yang Song <[email protected]>
Co-authored-by: Pablo Baeyens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/datadog Datadog components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants