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 support for DD_ENV, DD_SERVICE, and DD_VERSION environment variables #137

Merged

Conversation

jdgumz
Copy link
Contributor

@jdgumz jdgumz commented Mar 10, 2020

This PR adds support for:

  • DD_ENV
  • DD_SERVICE
  • DD_VERSION

to be used by the statsd client at initialization. The first three are added as global tags for {env, service, version}, if they are set.

statsd/statsd.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
statsd/client_test.go Outdated Show resolved Hide resolved
statsd/client_test.go Outdated Show resolved Hide resolved
@jdgumz jdgumz changed the title Add support for DD_ENV, DD_SERVICE, and DD_VERSION env vars Add support for DD_ENV, DD_SERVICE, DD_VERSION, and DD_TAGS env vars Mar 11, 2020
statsd/statsd.go Outdated
Comment on lines 294 to 300
parts := strings.Split(tag, ":")
if len(parts) == 1 {
c.Tags = append(c.Tags, parts[0])
}
if len(parts) == 2 {
c.Tags = append(c.Tags, fmt.Sprintf("%s:%s", parts[0], parts[1]))
}
Copy link
Member

Choose a reason for hiding this comment

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

Why split ? What is the value of a tag contains : ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, we do support : in tag values.

I've updated the logic and test cases accordingly.

@jdgumz jdgumz changed the title Add support for DD_ENV, DD_SERVICE, DD_VERSION, and DD_TAGS env vars Add support for DD_ENV, DD_SERVICE, and DD_VERSION env vars Mar 12, 2020
@jdgumz jdgumz force-pushed the jesse.gumz/add-support-for-dd-env-service-version-as-global-tags branch from 88e94f5 to 9d27212 Compare March 12, 2020 17:42
@jdgumz jdgumz changed the title Add support for DD_ENV, DD_SERVICE, and DD_VERSION env vars Add support for DD_ENV, DD_SERVICE, and DD_VERSION environment variables Mar 12, 2020
statsd/statsd.go Outdated
Comment on lines 86 to 88
// Client-side entity ID injection for container tagging
/*
Client-side entity ID injection for container tagging.
*/
Copy link
Member

Choose a reason for hiding this comment

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

No need to change the comment

statsd/statsd.go Outdated
Comment on lines 89 to 92
const (
entityIDEnvName = "DD_ENTITY_ID"
entityIDTagName = "dd.internal.entity_id"
)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to keep those variables around, we could hardcode them in the ddEnvTagsMapping like the other ones.

@jdgumz jdgumz merged commit a0db29f into master Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants