Skip to content

ref(logging): New scopes API in LoggingIntegration #2855

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

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

szokeasaurusrex
Copy link
Member

Comment on lines +186 to 188
client = sentry_sdk.get_client()
if not client.is_active():
return
Copy link
Contributor

@sentrivana sentrivana Mar 19, 2024

Choose a reason for hiding this comment

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

Not 100% sure but it might be we don't need this check since if the current client is a noop client, you can still do stuff with it, it'll just all be noop.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure, there is a lot of code after this line that we skip over. Even if this is not strictly necessary, it might improve performance slightly since we skip a lot of operations. I will merge for now without removing this, we can always change it later!

@szokeasaurusrex szokeasaurusrex merged commit f660707 into sentry-sdk-2.0 Mar 19, 2024
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/logging-new-scopes branch March 19, 2024 15:39
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.

2 participants