BoardiesITSolutions / cpp-datadogstatsd

Send metrics from your C++ applications to your Datadog account
BSD 3-Clause "New" or "Revised" License
9 stars 6 forks source link

Support configuring the client from environment variables #4

Closed ahmed-mez closed 5 years ago

ahmed-mez commented 5 years ago

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: https://github.com/DataDog/datadog-go/pull/78

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

Thanks,

Ahmed

boardy commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

Thanks.

boardy commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Hi @boardy,

Thank you for your work! 💯

Ahmed

boardy commented 5 years ago

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