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.

Sign up for GitHub

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

Jump to bottom

Fix race in set_tracer_provider() #2182

Merged
merged 8 commits into from
Oct 12, 2021
Merged

Fix race in set_tracer_provider() #2182

merged 8 commits into from
Oct 12, 2021

Conversation

aabmass
Copy link
Member

@aabmass aabmass commented Oct 5, 2021 •
edited
Loading

Description

Fixes an unlikely race condition where multiple threads can set the global tracer provider at once, bypassing the check that the global tracer provider can only be set once.

Fixes #2181

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Added unit tests that try to call set_tracer_provider() in 100 threads
  • Existing tests are passing fine otherwise

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated N/A

@aabmass aabmass changed the title Fix race in set_tracer_provider Fix race in set_tracer_provider() Oct 5, 2021
@aabmass aabmass force-pushed the set-tracer-provider-race branch from ec0056b to 7edb787 Compare October 5, 2021 20:25
@aabmass aabmass force-pushed the set-tracer-provider-race branch from 7edb787 to 30ba497 Compare October 5, 2021 20:56
@aabmass
Copy link
Member Author

aabmass commented Oct 5, 2021

I believe all the Public API checks are false positives (tests or private module).

@aabmass aabmass marked this pull request as ready for review October 5, 2021 21:15
@aabmass aabmass requested review from a team and owais October 5, 2021 21:15
@aabmass aabmass mentioned this pull request Oct 5, 2021
Merged
@ocelotl ocelotl added the Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary label Oct 7, 2021
@owais owais enabled auto-merge (squash) October 7, 2021 17:53
@lzchen lzchen closed this Oct 12, 2021
auto-merge was automatically disabled October 12, 2021 19:23

Pull request was closed

@lzchen lzchen reopened this Oct 12, 2021
@owais owais enabled auto-merge (squash) October 12, 2021 20:14
@owais owais merged commit 7867202 into open-telemetry:main Oct 12, 2021
@aabmass aabmass deleted the set-tracer-provider-race branch October 12, 2021 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lzchen lzchen lzchen approved these changes

@owais owais owais approved these changes

@ocelotl ocelotl ocelotl approved these changes

Assignees
No one assigned
Labels
Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Race condition in set_tracer_provider()
4 participants
@aabmass @owais @ocelotl @lzchen