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

Remove Datadog::Pin#tracer= #1073

Merged
merged 1 commit into from
Jun 10, 2020
Merged

Remove Datadog::Pin#tracer= #1073

merged 1 commit into from
Jun 10, 2020

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Jun 8, 2020

Work towards fixing #1072

This PR removes the ability to explicitly set the tracer instance on a Datadog::Pin object. This is problematic because most call sites were resolving the tracer instance at "patch" time, which normally happens very early in the application life-cycle. This causes any updates to the tracer instance to not be captured by integrations using the pin.

All references to the tracer by pins have been replace with an indirect reference to the tracer though the integration configuration (e.g. tracer: ->{ Datadog.configuration[:redis][tracer] }). This allows for configuration to still take place at integration level. One thing to note is that :tracer in our integration configurations, when not set, defaults to delegating to ->{ Datadog.tracer }, which is now the most common code path.

The API being removed in this PR is not public nor externally documented.

For some background information, we use this Pin internally to store configuration information for integrations that instantiate new client objects (e.g. a Connection from an HTTP client library) and that require different settings for different client objects (e.g. HTTP connections to domain internal.com have a different service_name than domain external.com).

@marcotc marcotc added the core Involves Datadog core libraries label Jun 8, 2020
@marcotc marcotc requested a review from a team June 8, 2020 23:01
@marcotc marcotc self-assigned this Jun 8, 2020
Comment on lines +32 to +33
Datadog.configure { |c| c.use :redis, tracer: tracer }
Datadog.configure(client_from_driver(driver))
Copy link
Member Author

Choose a reason for hiding this comment

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

Given it is not possible to pass tracer: to the Pin anymore (which happens through Datadog.configure(client) here), we have to move the tracer configuration to the integration level.

This explicit tracer: configuration at integration level is also going away in a future PR, but not on the scope of this one.

Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

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

👍 We reviewed this together and this seems like the right way to go.

@marcotc marcotc merged commit 0aa2dc4 into master Jun 10, 2020
@marcotc marcotc deleted the fix/deprecate-pin-tracer-cache branch June 10, 2020 17:52
@marcotc marcotc added this to the 0.37.0 milestone Jun 10, 2020
@marcotc marcotc linked an issue Jun 10, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some integrations in 0.36.0 have stale tracer instances
2 participants