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

Datadog update env clobbering behavior #3851

Merged

Conversation

ericmustin
Copy link
Contributor

@ericmustin ericmustin commented Jun 21, 2021

Description:

Sometimes users will set both a deployment.environment (the opentelemetry semantically correct way to define an environment attribute) attribute and an env attribute (the datadog way to define an environment span tag) on the same application, but they will have different values. This creates a variety of unintended behaviors within the Datadog UI, which correctly uses deployment.environment values for most critical details, but hardcodes some links to other views using the env value, leading confusing behavior. for the sake of a coherent experience, it makes sense to ensure these span tags are always set to the same value (the deployment.environment) even if that means clobbering the original env value.

Link to tracking Issue: n/a

Testing: added a unit test

Documentation: adding docs to the Datadog docs/onboarding page encouraging users to rely on the opentelemetry best practice deployment.environment only. cc @mx-psi

@ericmustin ericmustin requested a review from mx-psi as a code owner June 21, 2021 19:29
@ericmustin ericmustin requested a review from a team June 21, 2021 19:29
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Looks good, just two minor comments

exporter/datadogexporter/translate_traces_test.go Outdated Show resolved Hide resolved
exporter/datadogexporter/translate_traces.go Outdated Show resolved Hide resolved
ericmustin and others added 2 commits June 22, 2021 08:38
Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>
Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>
@ericmustin ericmustin requested a review from mx-psi June 22, 2021 12:38
mx-psi
mx-psi previously approved these changes Jun 22, 2021
@bogdandrutu
Copy link
Member

golangci-lint run --allow-parallel-runners
translate_traces_test.go:1424: File is not `gofmt`-ed with `-s` (gofmt)
		"env":                    pdata.NewAttributeValueString("incorrectenv"),
make[1]: *** [../../Makefile.Common:93: lint] Error 1

@mx-psi
Copy link
Member

mx-psi commented Jun 22, 2021

@ericmustin see comment above

@mx-psi mx-psi dismissed their stale review June 22, 2021 13:06

need to gofmt

@ericmustin
Copy link
Contributor Author

@mx-psi @bogdandrutu sorry about that, should be good to go now (would love to get this in v0.29.0 if you don't mind)

@ericmustin
Copy link
Contributor Author

@jchengsfx @bogdandrutu it would be great to get this in for v0.29.0 if possible

@mx-psi mx-psi added the ready to merge Code review completed; ready to merge by maintainers label Jun 23, 2021
@bogdandrutu bogdandrutu merged commit ff5099c into open-telemetry:main Jun 23, 2021
@mx-psi mx-psi deleted the datadog_update_env_clobbering_behavior branch June 23, 2021 14:16
alexperez52 referenced this pull request in open-o11y/opentelemetry-collector-contrib Aug 18, 2021
#3851)

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants