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

Change configuration resolution misses to return default configuration #651

Merged
merged 3 commits into from
Dec 10, 2018

Conversation

delner
Copy link
Contributor

@delner delner commented Dec 9, 2018

This pull request changes a few things:

  • Datadog::Utils::Database values to constants.
  • Return the default configuration instead of nil when a configuration with a mismatching key is requested.
  • In ActiveRecord, if the tracer settings have a service name with the default value defaultdb, then default instead to the adapter name.

These changes were in order to get ActiveRecord to return service names more consistently. If connections were manually established after the AR integration was activated (when it could not determine a good default service name from a connection), spans could end up with bad service names (defaultdb).

By always returning at minimum the default settings object, ActiveRecord could then use those settings to determine the best name for the span based on what's available.

@delner delner added core Involves Datadog core libraries integrations Involves tracing integrations dev/refactor Involves refactoring existing components labels Dec 9, 2018
@delner delner added this to the 0.18.0 milestone Dec 9, 2018
@delner delner self-assigned this Dec 9, 2018
@delner delner mentioned this pull request Dec 9, 2018
module_function

def normalize_vendor(vendor)
case vendor
when nil
'defaultdb'
VENDOR_DEFAULT
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@delner delner merged commit 67a32f8 into 0.18-dev Dec 10, 2018
@delner delner deleted the refactor/configuration_resolution_for_mismatches branch December 10, 2018 19:42
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 integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants