From 5241bc4499dbf774571cc5dd08c55b7655421f8e Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 29 Nov 2022 14:49:07 -0800 Subject: [PATCH 1/3] feat(profiling): Introduce active thread id on scope Upto this point, simply taking the current thread when the transaction/profile was started was good enough. When using ASGI apps with non async handlers, the request is received on the main thread. This is also where the transaction or profile was started. However, the request is handled on another thread using a thread pool. To support this use case, we want to be able to set the active thread id on the scope where we can read it when we need it to allow the active thread id to be set elsewhere. --- sentry_sdk/client.py | 2 +- sentry_sdk/profiler.py | 14 +++++++++++--- sentry_sdk/scope.py | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index bf1e483634..cffd089fec 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -429,7 +429,7 @@ def capture_event( if is_transaction: if profile is not None: - envelope.add_profile(profile.to_json(event_opt, self.options)) + envelope.add_profile(profile.to_json(event_opt, self.options, scope)) envelope.add_transaction(event_opt) else: envelope.add_event(event_opt) diff --git a/sentry_sdk/profiler.py b/sentry_sdk/profiler.py index 3d3b7cf5a0..dfacfe7166 100644 --- a/sentry_sdk/profiler.py +++ b/sentry_sdk/profiler.py @@ -51,6 +51,7 @@ from typing import Sequence from typing import Tuple from typing_extensions import TypedDict + import sentry_sdk.scope import sentry_sdk.tracing RawSampleData = Tuple[int, Sequence[Tuple[str, Sequence[RawFrameData]]]] @@ -265,8 +266,8 @@ def __exit__(self, ty, value, tb): self.scheduler.stop_profiling() self._stop_ns = nanosecond_time() - def to_json(self, event_opt, options): - # type: (Any, Dict[str, Any]) -> Dict[str, Any] + def to_json(self, event_opt, options, scope): + # type: (Any, Dict[str, Any], Optional[sentry_sdk.scope.Scope]) -> Dict[str, Any] assert self._start_ns is not None assert self._stop_ns is not None @@ -278,6 +279,9 @@ def to_json(self, event_opt, options): profile["frames"], options["in_app_exclude"], options["in_app_include"] ) + # the active thread id from the scope always take priorty if it exists + active_thread_id = None if scope is None else scope.active_thread_id + return { "environment": event_opt.get("environment"), "event_id": uuid.uuid4().hex, @@ -309,7 +313,11 @@ def to_json(self, event_opt, options): # because we end the transaction after the profile "relative_end_ns": str(self._stop_ns - self._start_ns), "trace_id": self.transaction.trace_id, - "active_thread_id": str(self.transaction._active_thread_id), + "active_thread_id": str( + self.transaction._active_thread_id + if active_thread_id is None else + active_thread_id + ), } ], } diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index e0a2dc7a8d..12fae749f3 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -94,6 +94,11 @@ class Scope(object): "_session", "_attachments", "_force_auto_session_tracking", + + # The thread that is handling the bulk of the work. This can just + # be the main thread, but that's not always true. For web frameworks, + # this would be the thread handling the request. + "_active_thread_id", ) def __init__(self): @@ -125,6 +130,8 @@ def clear(self): self._session = None # type: Optional[Session] self._force_auto_session_tracking = None # type: Optional[bool] + self._active_thread_id = None # type: Optional[int] + @_attr_setter def level(self, value): # type: (Optional[str]) -> None @@ -217,6 +224,17 @@ def span(self): """Get/set current tracing span or transaction.""" return self._span + @property + def active_thread_id(self): + # type: () -> Optional[int] + """Get/set the current active thread id.""" + return self._active_thread_id + + def set_active_thread_id(self, active_thread_id): + # type: (Optional[int]) -> None + """Set the current active thread id.""" + self._active_thread_id = active_thread_id + @span.setter def span(self, span): # type: (Optional[Span]) -> None @@ -447,6 +465,8 @@ def update_from_scope(self, scope): self._span = scope._span if scope._attachments: self._attachments.extend(scope._attachments) + if scope._active_thread_id is not None: + self._active_thread_id = scope._active_thread_id def update_from_kwargs( self, @@ -496,6 +516,8 @@ def __copy__(self): rv._force_auto_session_tracking = self._force_auto_session_tracking rv._attachments = list(self._attachments) + rv._active_thread_id = self._active_thread_id + return rv def __repr__(self): From a66e3554c1d334f8104fc373fc9bfa2aa80b9d32 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Thu, 1 Dec 2022 13:17:45 -0800 Subject: [PATCH 2/3] run black --- sentry_sdk/client.py | 4 +++- sentry_sdk/profiler.py | 4 ++-- sentry_sdk/scope.py | 1 - 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index cffd089fec..8ac9d0a5b8 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -429,7 +429,9 @@ def capture_event( if is_transaction: if profile is not None: - envelope.add_profile(profile.to_json(event_opt, self.options, scope)) + envelope.add_profile( + profile.to_json(event_opt, self.options, scope) + ) envelope.add_transaction(event_opt) else: envelope.add_event(event_opt) diff --git a/sentry_sdk/profiler.py b/sentry_sdk/profiler.py index dfacfe7166..fbaefca8f6 100644 --- a/sentry_sdk/profiler.py +++ b/sentry_sdk/profiler.py @@ -315,8 +315,8 @@ def to_json(self, event_opt, options, scope): "trace_id": self.transaction.trace_id, "active_thread_id": str( self.transaction._active_thread_id - if active_thread_id is None else - active_thread_id + if active_thread_id is None + else active_thread_id ), } ], diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index 12fae749f3..dbe9a1e601 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -94,7 +94,6 @@ class Scope(object): "_session", "_attachments", "_force_auto_session_tracking", - # The thread that is handling the bulk of the work. This can just # be the main thread, but that's not always true. For web frameworks, # this would be the thread handling the request. From 9190317a41557b9e40b38c683f49f5a17af6bb8b Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 5 Dec 2022 14:05:36 +0100 Subject: [PATCH 3/3] Make sure properties are next to each other --- sentry_sdk/scope.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index dbe9a1e601..f5ac270914 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -223,17 +223,6 @@ def span(self): """Get/set current tracing span or transaction.""" return self._span - @property - def active_thread_id(self): - # type: () -> Optional[int] - """Get/set the current active thread id.""" - return self._active_thread_id - - def set_active_thread_id(self, active_thread_id): - # type: (Optional[int]) -> None - """Set the current active thread id.""" - self._active_thread_id = active_thread_id - @span.setter def span(self, span): # type: (Optional[Span]) -> None @@ -245,6 +234,17 @@ def span(self, span): if transaction.name: self._transaction = transaction.name + @property + def active_thread_id(self): + # type: () -> Optional[int] + """Get/set the current active thread id.""" + return self._active_thread_id + + def set_active_thread_id(self, active_thread_id): + # type: (Optional[int]) -> None + """Set the current active thread id.""" + self._active_thread_id = active_thread_id + def set_tag( self, key, # type: str