-
Notifications
You must be signed in to change notification settings - Fork 137
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 more environment variables #140
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.
LGTM 👍
@kbogtob will probably want to give it a look before merging though.
@remeh Okay, there's one more question I had... if we set these global tags (say Just thinking of the trade-off between saving on packet size vs saving on memory/CPU it would take to de-dup this. |
Thanks @delner for your contribution! It's true that we don't deduplicate the tags and I think we should be able to override specific tags in this order: ENV < Global tags < Call tags. I will discuss it internally and create an issue. PS: Thanks for ClimateControl, didn't know that gem before :). It's nice. |
501c1ba
to
ebcb740
Compare
@kbogtob Changes seem fine to me. Also just added a minor fix for a warning that was blowing up our test suite (when we'd close Statsd without using it.) |
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.
LGTM. Just left a small nit.
04a9d62
to
cbe3b72
Compare
This pull request adds global tags for:
DD_TAGS
(acts as base set of global tags, overridden by other tags)DD_ENV
-->env:<env name>
DD_SERVICE
-->service:<service name>
DD_VERSION
-->version:<version string>
Precedence in order from most to least preferred is manually defined global tags (via
#initialize
),DD_ENV
/DD_SERVICE
/DD_VERSION
, then finallyDD_TAGS
.