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

Move env vars to configuration settings #985

Merged
merged 3 commits into from
Mar 27, 2020

Conversation

delner
Copy link
Contributor

@delner delner commented Mar 27, 2020

We'd like service, env, tags, and version to be configurable from Datadog.configure as well as from their respective environment variables.

This pull request moves these into Datadog::Configuration::Settings to make them configurable.

@delner delner added core Involves Datadog core libraries dev/refactor Involves refactoring existing components labels Mar 27, 2020
@delner delner requested review from marcotc and brettlangdon March 27, 2020 03:09
@delner delner self-assigned this Mar 27, 2020
@delner delner force-pushed the refactor/env_vars_to_configuration branch from 3c642ba to 90f4825 Compare March 27, 2020 03:33
@delner
Copy link
Contributor Author

delner commented Mar 27, 2020

Something things I'd still like to do with this before its ready:

  • Add deprecation warnings to c.tracer when passed tags, env to use new settings instead.
  • Add documentation explaining how to set tags, env, version, service.
  • Possibly move log and debug from c.tracer as well, while we're deprecating options.

Copy link
Member

@brettlangdon brettlangdon left a comment

Choose a reason for hiding this comment

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

so far looks good to me

@delner delner force-pushed the refactor/env_vars_to_configuration branch from 90f4825 to d2e80fc Compare March 27, 2020 13:53
@delner delner force-pushed the refactor/env_vars_to_configuration branch from d2e80fc to 4912342 Compare March 27, 2020 15:29
@delner
Copy link
Contributor Author

delner commented Mar 27, 2020

Decided to hold off on deprecation warnings for now; want to give this a thorough treatment but the changes are probably too much for this PR.

Should be ready for a final review pending CI passing.

@delner delner marked this pull request as ready for review March 27, 2020 15:30
@delner delner requested a review from a team March 27, 2020 15:30
@delner
Copy link
Contributor Author

delner commented Mar 27, 2020

Looks like there was a bug where setting Datadog::Logger.debug_logging = false would ignore the value and leave debug logging on. Fixed this as well.

EDIT: Never mind, this is evidently intentional behavior for custom loggers. The test for this was doing too many things at once and conflating things a bit; refactored it a bit for correctness and clarity.

marcotc
marcotc previously approved these changes Mar 27, 2020
Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

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

:shipit:!

@delner delner force-pushed the refactor/env_vars_to_configuration branch from 325541a to 2bf178f Compare March 27, 2020 17:42
@delner delner force-pushed the refactor/env_vars_to_configuration branch from 2bf178f to 1c0fbb6 Compare March 27, 2020 20:07
@delner delner merged commit 03ea152 into master Mar 27, 2020
@delner delner deleted the refactor/env_vars_to_configuration branch March 27, 2020 21:09
@delner delner added this to the 0.34.0 milestone Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries dev/refactor Involves refactoring existing components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants