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

Fixes warning filter being reset #3147

Closed

Conversation

mjvankampen
Copy link
Contributor

@mjvankampen mjvankampen commented Jan 27, 2023

Description

Warning filters were being reset, resulting in warnings suddenly popping up in unrelated spots due to instrumenting with OTel.

Fixes based on https://docs.python.org/3/library/warnings.html#temporarily-suppressing-warnings although there it is noted that this is not threadsafe. I haven't been able to make sure if this is an issue yet. @ocelotl was this the reason you used a different approach? In my opinion this non-thread safe approach makes more sense than just throwing away all filters.

Fixes #3102

Type of change

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

How Has This Been Tested?

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

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

@mjvankampen mjvankampen requested a review from a team January 27, 2023 13:02
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 27, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@ocelotl
Copy link
Contributor

ocelotl commented Feb 3, 2023

I have been looking at this PR, I think there is a way to avoid the context manager and a lock if we tighten the warning filter to use line number, will add a more detailed comment when I get back home.

@mjvankampen
Copy link
Contributor Author

Wouldn't it also be an option to just leave the deprecation warning until the root cause itself is fixed?

@ocelotl
Copy link
Contributor

ocelotl commented Feb 13, 2023

@mjvankampen Thanks for your PR! This issue has been fixed by #3164, so I'll be closing this PR now 🙂

@ocelotl ocelotl closed this Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using get_tracer removes all warnings filters
4 participants