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

Ensure OpenTracer test does not leak writer thread #1163

Merged
merged 1 commit into from
Sep 2, 2020

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Sep 2, 2020

Fixes this CI issue:

Failures:

  1) OpenTracer context propagation via OpenTracing::FORMAT_RACK when receiving a carrier with no headers 
     Failure/Error: Datadog::Statsd.new(default_hostname, default_port)
     
     ArgumentError:
       wrong number of arguments (given 2, expected 0)
     # ./lib/ddtrace/metrics.rb:47:in `initialize'
     # ./lib/ddtrace/metrics.rb:47:in `new'
     # ./lib/ddtrace/metrics.rb:47:in `default_statsd_client'
     # ./lib/ddtrace/metrics.rb:16:in `block in initialize'

This error happens because the OpenTracing test was not properly closing the worker thread crated by its write, causing it to interfere with subsequent test runs. This PR fixes that by properly shutting down the tracer after each run.

This error can be reproduced with the following command:
bundle exec rspec ./spec/ddtrace/opentracer/propagation_integration_spec.rb --seed=42061
You might have to run it a few times to it to fail.

@marcotc marcotc added the dev/testing Involves testing processes (e.g. RSpec) label Sep 2, 2020
@marcotc marcotc requested a review from a team September 2, 2020 19:42
@marcotc marcotc self-assigned this Sep 2, 2020
Copy link
Contributor

@ericmustin ericmustin left a comment

Choose a reason for hiding this comment

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

👍 :shipit:

@marcotc marcotc merged commit 7eb8bea into master Sep 2, 2020
@marcotc marcotc deleted the test/open-tracer-leak branch September 2, 2020 20:24
@marcotc marcotc added this to the 0.40.0 milestone Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/testing Involves testing processes (e.g. RSpec)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants