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

Implement global configuration using configuration API #714

Merged
merged 4 commits into from
Mar 14, 2019

Conversation

delner
Copy link
Contributor

@delner delner commented Mar 11, 2019

To allow for better management of configuration options, this pull request implements global configuration options as a configuration object accessible via Datadog.configuration.

The configuration API remains backwards compatible, meaning Datadog.configure { |c| c.tracer options } and other configuration paradigms will continue to function. However, it provides a base for new options to be defined (or extracted from elsewhere). For example the following option could be defined in Datadog::Configuration::Settings:

option :debug, default: false

Then it can be set using:

Datadog.configure |c|
  c.debug = true
end

Or accessed using via Datadog.configuration.debug.

(Note this is just an example: debug option has not yet been defined this way.)

While implementing this feature, this pull request does a few ambitious things:

  • Removes obsolete configuration API components (e.g. Configuration::Proxy, Contrib::Base, etc.)
  • Promotes core configuration API components (e.g. Options) to Datadog::Configuration from Contrib
  • Demotes integration specific configuration to Contrib (e.g. Registry) and the Contrib::Extensions module.
  • Replaces Datadog::Configuration with Datadog::Configuration::Settings, which implements options just like integrations do.

@delner delner added core Involves Datadog core libraries feature Involves a product feature dev/refactor Involves refactoring existing components labels Mar 11, 2019
@delner delner self-assigned this Mar 11, 2019
@delner delner force-pushed the feature/datadog_global_config_settings branch from 8fa4a8b to f8b69bb Compare March 11, 2019 03:47
@delner delner changed the base branch from master to 0.21-dev March 11, 2019 03:48
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.

🔥 🔥 🔥

Nice, a lot of this looks like shuffling around existing classes? Looks that way, but not sure why the diff didn't pick them up as a file name?

@delner
Copy link
Contributor Author

delner commented Mar 11, 2019

@brettlangdon Yeah, exactly. There's a lot of shuffling, and a few classes/modules had their responsibilities split and redistributed elsewhere: e.g. Datadog::Configuration doesn't define a registry anymore, Contrib::Extensions adds that back to Datadog::Configuration because registry is a Contrib component now. Almost all of the test logic is the same, except for a few places where obsolete configuration code was ripped out.

If there aren't any referential errors after this refactor (e.g. if I missed a constant name on a code path or something like that), then the only other risk posed by this re-shuffle would be if someone referenced these constant names directly elsewhere, which I think is low probability and out of scope of what's supported in our public API.

I will test this locally a bit more, make sure everything seems sane still.

@delner delner force-pushed the feature/datadog_global_config_settings branch from f8b69bb to 5f20991 Compare March 14, 2019 15:28
@delner delner force-pushed the feature/datadog_global_config_settings branch from 5f20991 to c28ce31 Compare March 14, 2019 19:53
@delner delner added this to the 0.21.0 milestone Mar 14, 2019
@delner delner merged commit b70c87b into 0.21-dev Mar 14, 2019
@delner delner deleted the feature/datadog_global_config_settings branch March 14, 2019 20:27
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 feature Involves a product feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants