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 support for DD_VERSION and DD_ENV variables #803
Conversation
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.
This looks good to me, but I am surprised that tests did not fail.
@brettlangdon thanks for the work on this. I can download it tomorrow first thing in the morning and play with it to understand why it is not failing, as I am surprised as well |
well, by default it won't add actually modify anything, if no env variables are provided, then no changes to spans. |
…ngdon/dd.version.env
…nctions dual approach
$this->assertTrue(empty($traces[0][0]['meta']['env'])); | ||
} | ||
|
||
public function testDDEnvHasPrecendenceOverGlobalTags() |
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.
$this->assertTrue(empty($traces[0][0]['meta']['version'])); | ||
} | ||
|
||
public function testDDVersionHasPrecendenceOverGlobalTags() |
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.
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.
Excellent work @brettlangdon! 🔝
|
||
// Application version | ||
if (null !== $this->serviceVersion) { | ||
$span->setTag(Tag::VERSION, $this->serviceVersion); |
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.
Just want to confirm that it was intentional to not use Tag::SERVICE_VERSION
here, correct?
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.
That's correct. Tag::SERVICE_VERSION =service.version
is only there for compatibility for OpenTelemetry. What we want to set is version
.
Description
Adding support for
DD_VERSION
andDD_ENV
environment variables.These variables should set default global tags of
version
andenv
on every span generated.Readiness checklist
Reviewer checklist