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.

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

Merged
merged 1 commit into from
Mar 19, 2024
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions sentry_sdk/integrations/logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from datetime import datetime, timezone
from fnmatch import fnmatch

from sentry_sdk.hub import Hub
import sentry_sdk
from sentry_sdk.utils import (
to_string,
event_from_exception,
Expand Down Expand Up @@ -101,7 +101,9 @@ def sentry_patched_callhandlers(self, record):
# into a recursion error when the integration is resolved
# (this also is slower).
if ignored_loggers is not None and record.name not in ignored_loggers:
integration = Hub.current.get_integration(LoggingIntegration)
integration = sentry_sdk.get_client().get_integration(
LoggingIntegration
)
if integration is not None:
integration._handle_record(record)

Expand Down Expand Up @@ -181,11 +183,11 @@ def _emit(self, record):
if not self._can_record(record):
return

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

@sentrivana sentrivana Mar 19, 2024 •
edited
Loading

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!


client_options = hub.client.options
client_options = client.options

# exc_info might be None or (None, None, None)
#
Expand Down Expand Up @@ -250,7 +252,7 @@ def _emit(self, record):

event["extra"] = self._extra_from_record(record)

hub.capture_event(event, hint=hint)
sentry_sdk.capture_event(event, hint=hint)


# Legacy name
Expand All @@ -275,7 +277,7 @@ def _emit(self, record):
if not self._can_record(record):
return

Hub.current.add_breadcrumb(
sentry_sdk.add_breadcrumb(
self._breadcrumb_from_record(record), hint={"log_record": record}
)

Expand Down