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

Default system tags #61

Merged
merged 8 commits into from
Sep 6, 2023
Merged

Default system tags #61

merged 8 commits into from
Sep 6, 2023

Conversation

cottinisimone
Copy link
Contributor

@cottinisimone cottinisimone commented Aug 25, 2023

Hello @mcasper,

I took inspiration, for this PR, to ruby dogstatsd library. The general idea of this PR is to automatically set the env, service and version tags in every metric we send. This could be accomplished setting DD_ENV, DD_SERVICE and DD_VERSION environment variables.

The code i refer to in the ruby dogstatsd library is this one

Please let me know if there's something else i can do and if you agree with this change.

Thank you in advance

Copy link

@MaeIsBad MaeIsBad left a comment

Choose a reason for hiding this comment

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

(just for the sake of clarity: I work with Simone)

Should this be behind a (default) feature flag?

I think this is a good addition, especially since the official datadog libs are already doing this

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Comment on lines +1133 to +1142
fn with_default_system_tags<T, F: FnOnce() -> T>(f: F) -> T {
std::env::set_var("DD_ENV", "production");
std::env::set_var("DD_SERVICE", "service");
std::env::set_var("DD_VERSION", "0.0.1");
let t = f();
std::env::remove_var("DD_ENV");
std::env::remove_var("DD_SERVICE");
std::env::remove_var("DD_VERSION");
t
}

Choose a reason for hiding this comment

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

This causes flakiness. I've had test_new_default_tags randomly fail locally because of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can i propose to put in the readme something like a test section suggesting to test using the --test-threads 1?

src/lib.rs Outdated Show resolved Hide resolved
@mcasper mcasper merged commit 7b80cca into mcasper:master Sep 6, 2023
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.

3 participants