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

Support configuring the client from environment variables #4

Closed
ahmed-mez opened this issue Mar 20, 2019 · 10 comments
Closed

Support configuring the client from environment variables #4

ahmed-mez opened this issue Mar 20, 2019 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@ahmed-mez
Copy link

ahmed-mez commented Mar 20, 2019

All of the official Datadog dogstatsd clients can now be configured via standard environment variables. As the maintainer of the C++ dogstatsd library, could you port the same functionality to this client? We would prefer that configuration be consistent across client libraries.

Sample changes from the Go implementation can be found here: DataDog/datadog-go#78

Let me know if you have any questions about the code or the feature more generally.

Thanks,

Ahmed

@boardy boardy self-assigned this Mar 20, 2019
@boardy boardy added the enhancement New feature or request label Mar 20, 2019
@boardy
Copy link
Member

boardy commented Mar 20, 2019

Hi, I can certainly take a look, don't know anything about containers or orchestration though unfortunately. Presumably that part doesn't matter the environment variable can be used regardless of it being in a container or not?

At a quick glance, it sounds like what should happen in this case is if you call the constructor which doesn't specify the host and/or port, then it should read it from the environment variable DD_AGENT_HOST and DD_DOGSTATSD_PORT, otherwise it defaults as it does currently. Does this sound right?

Thanks

Chris

@ahmed-mez
Copy link
Author

Hi,

That's right, it is about the DD_ENTITY_ID env var also, if set, it would be very useful to add it as a tag with the tag name dd.internal.entity_id to identify the pod later in the server.

Thanks.

@boardy
Copy link
Member

boardy commented Mar 20, 2019

Ah OK, so if the environment variable DD_ENTITY_ID is set also, then a tag called dd.internal.entity_id with the value of the environment variable is added along with any other tags that might be in use.

I can definitely look into implementing this, probably won't be able to test with the container side of things as not sure how they work so if you're OK to test that side of things when I've implemented it that would be great.

Thanks

Chris

@ahmed-mez
Copy link
Author

I think some unit tests mocking the DD_ENTITY_ID env var would be sufficient to test the logic.

Thanks.

@boardy
Copy link
Member

boardy commented Apr 4, 2019

Hi,

I've been implementing this but just need a little confirmation on something. Do you know if the dd.internal.entity_id is only applied if the agent is in the container.

I've implemented the code change with the name dd.internal.entity_id but when looking at the metric summary the tag doesn't get shown where as other do. However, if I rename the tag name to be entity_id the tag then gets applied to the metric. So it kind of looks like the dd.internal.entity_id doesn't get applied if its not within a container.

Do you know if this is right?

Thanks

Chris

@ahmed-mez
Copy link
Author

Hi Chris,

Thanks for implementing the feature.

What you are saying is correct, and it is because dd.internal.entity_id is used by the DogStatsd server to add tags of the kubernetes pod with the same entity-id.

Please let me know if you have any other questions

Thanks,
Ahmed

@boardy
Copy link
Member

boardy commented Apr 5, 2019

That's great, I thought I was going mad :).

I'm going to do a bit more testing I should be able to get a release out hopefully early next week.

Thanks
Chris

@boardy
Copy link
Member

boardy commented Apr 9, 2019

Hi Ahmed,

I've just tagged 1.1.0.5 that adds support for setting the library up via environment variables. Let me know how you get on and if there's any issues.

Thanks

Chris

@ahmed-mez
Copy link
Author

Hi @boardy,

Thank you for your work! 💯

Ahmed

@boardy
Copy link
Member

boardy commented Apr 11, 2019

No problem Ahmed, glad to help.

I'll close this issue off, but if there's any problems then please let me know.

Thanks

Chris

@boardy boardy closed this as completed Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants