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

Allow tracer components to be cleanly restarted after shutdown #1177

Merged
merged 4 commits into from Sep 18, 2020

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Sep 16, 2020

We have inconsistent behaviour with Configuration#shutdown! where we shut down our components object ( components.shutdown!), but we keep its lingering instance in an instance variable (@components), with partially living references.

This prevented us from ever being able to restart the tracer, only ever returning dirty instances of the tracer on every call to Datadog.tracer after a shutdown.

This PR now allows the tracer components to be cleanly shutdown! and restarted, without lingering references.

@marcotc marcotc added the dev/testing Involves testing processes (e.g. RSpec) label Sep 16, 2020
@marcotc marcotc requested a review from a team September 16, 2020 20:32
@marcotc marcotc self-assigned this Sep 16, 2020
@@ -56,7 +56,10 @@ def logger
end

def shutdown!
components.shutdown! if instance_variable_defined?(:@components) && @components
if instance_variable_defined?(:@components) && @components
Copy link
Member

Choose a reason for hiding this comment

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

should this change be in a different PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the important change, I updated the description of the PR to match.

@ericmustin
Copy link
Contributor

I think a commit slipped in here unrelated, otherwise lgtm and thanks for tracking this down, these flaky tests have been killing us

@marcotc marcotc changed the title Avoid *_any_instance_of when running tests with parallelism Allow tracer components to be cleanly restarted after shutdown Sep 18, 2020
@marcotc
Copy link
Member Author

marcotc commented Sep 18, 2020

@ericmustin I removed the test fix, and kept the important change regarding tracer shutdown.

@marcotc marcotc requested review from a team and brettlangdon September 18, 2020 18:43
@marcotc marcotc added core Involves Datadog core libraries and removed dev/testing Involves testing processes (e.g. RSpec) labels Sep 18, 2020
@marcotc marcotc merged commit 904288b into master Sep 18, 2020
@marcotc marcotc deleted the test/fix-parallelism-with-_any_instance_of branch September 18, 2020 21:16
@marcotc marcotc added this to the 0.41.0 milestone Sep 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants