Merge branch 'getsentry:master' into master

kwnath · web-flow · commit a370b30a2d58 · 2024-10-25T10:10:25.000+01:00
diff --git a/sentry_sdk/integrations/django/asgi.py b/sentry_sdk/integrations/django/asgi.py
@@ -172,6 +172,10 @@ def wrap_async_view(callback):
     @functools.wraps(callback)
     async def sentry_wrapped_callback(request, *args, **kwargs):
         # type: (Any, *Any, **Any) -> Any
+        current_scope = sentry_sdk.get_current_scope()
+        if current_scope.transaction is not None:
+            current_scope.transaction.update_active_thread()
+
         sentry_scope = sentry_sdk.get_isolation_scope()
         if sentry_scope.profile is not None:
             sentry_scope.profile.update_active_thread_id()
diff --git a/sentry_sdk/integrations/django/views.py b/sentry_sdk/integrations/django/views.py
@@ -76,6 +76,10 @@ def _wrap_sync_view(callback):
     @functools.wraps(callback)
     def sentry_wrapped_callback(request, *args, **kwargs):
         # type: (Any, *Any, **Any) -> Any
+        current_scope = sentry_sdk.get_current_scope()
+        if current_scope.transaction is not None:
+            current_scope.transaction.update_active_thread()
+
         sentry_scope = sentry_sdk.get_isolation_scope()
         # set the active thread id to the handler thread for sync views
         # this isn't necessary for async views since that runs on main
diff --git a/sentry_sdk/integrations/fastapi.py b/sentry_sdk/integrations/fastapi.py
@@ -88,9 +88,14 @@ def _sentry_get_request_handler(*args, **kwargs):
             @wraps(old_call)
             def _sentry_call(*args, **kwargs):
                 # type: (*Any, **Any) -> Any
+                current_scope = sentry_sdk.get_current_scope()
+                if current_scope.transaction is not None:
+                    current_scope.transaction.update_active_thread()
+
                 sentry_scope = sentry_sdk.get_isolation_scope()
                 if sentry_scope.profile is not None:
                     sentry_scope.profile.update_active_thread_id()
+
                 return old_call(*args, **kwargs)
 
             dependant.call = _sentry_call
diff --git a/sentry_sdk/integrations/quart.py b/sentry_sdk/integrations/quart.py
@@ -1,6 +1,5 @@
 import asyncio
 import inspect
-import threading
 from functools import wraps
 
 import sentry_sdk
@@ -122,11 +121,13 @@ def decorator(old_func):
                 @ensure_integration_enabled(QuartIntegration, old_func)
                 def _sentry_func(*args, **kwargs):
                     # type: (*Any, **Any) -> Any
-                    scope = sentry_sdk.get_isolation_scope()
-                    if scope.profile is not None:
-                        scope.profile.active_thread_id = (
-                            threading.current_thread().ident
-                        )
+                    current_scope = sentry_sdk.get_current_scope()
+                    if current_scope.transaction is not None:
+                        current_scope.transaction.update_active_thread()
+
+                    sentry_scope = sentry_sdk.get_isolation_scope()
+                    if sentry_scope.profile is not None:
+                        sentry_scope.profile.update_active_thread_id()
 
                     return old_func(*args, **kwargs)
 
diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py
@@ -487,8 +487,11 @@ def _sentry_sync_func(*args, **kwargs):
                 if integration is None:
                     return old_func(*args, **kwargs)
 
-                sentry_scope = sentry_sdk.get_isolation_scope()
+                current_scope = sentry_sdk.get_current_scope()
+                if current_scope.transaction is not None:
+                    current_scope.transaction.update_active_thread()
 
+                sentry_scope = sentry_sdk.get_isolation_scope()
                 if sentry_scope.profile is not None:
                     sentry_scope.profile.update_active_thread_id()
 
diff --git a/sentry_sdk/integrations/strawberry.py b/sentry_sdk/integrations/strawberry.py
@@ -31,13 +31,18 @@
     from strawberry import Schema
     from strawberry.extensions import SchemaExtension  # type: ignore
     from strawberry.extensions.tracing.utils import should_skip_tracing as strawberry_should_skip_tracing  # type: ignore
+    from strawberry.http import async_base_view, sync_base_view  # type: ignore
+except ImportError:
+    raise DidNotEnable("strawberry-graphql is not installed")
+
+try:
     from strawberry.extensions.tracing import (  # type: ignore
         SentryTracingExtension as StrawberrySentryAsyncExtension,
         SentryTracingExtensionSync as StrawberrySentrySyncExtension,
     )
-    from strawberry.http import async_base_view, sync_base_view  # type: ignore
 except ImportError:
-    raise DidNotEnable("strawberry-graphql is not installed")
+    StrawberrySentryAsyncExtension = None
+    StrawberrySentrySyncExtension = None
 
 from typing import TYPE_CHECKING
 
diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py
@@ -329,8 +329,7 @@ def __init__(
         self._span_recorder = None  # type: Optional[_SpanRecorder]
         self._local_aggregator = None  # type: Optional[LocalAggregator]
 
-        thread_id, thread_name = get_current_thread_meta()
-        self.set_thread(thread_id, thread_name)
+        self.update_active_thread()
         self.set_profiler_id(get_profiler_id())
 
     # TODO this should really live on the Transaction class rather than the Span
@@ -732,6 +731,11 @@ def get_profile_context(self):
             "profiler_id": profiler_id,
         }
 
+    def update_active_thread(self):
+        # type: () -> None
+        thread_id, thread_name = get_current_thread_meta()
+        self.set_thread(thread_id, thread_name)
+
 
 class Transaction(Span):
     """The Transaction is the root element that holds all the spans
diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py
@@ -215,15 +215,7 @@ def __init__(self, options):
         )  # type: DefaultDict[Tuple[EventDataCategory, str], int]
         self._last_client_report_sent = time.time()
 
-        self._pool = self._make_pool(
-            self.parsed_dsn,
-            http_proxy=options["http_proxy"],
-            https_proxy=options["https_proxy"],
-            ca_certs=options["ca_certs"],
-            cert_file=options["cert_file"],
-            key_file=options["key_file"],
-            proxy_headers=options["proxy_headers"],
-        )
+        self._pool = self._make_pool()
 
         # Backwards compatibility for deprecated `self.hub_class` attribute
         self._hub_cls = sentry_sdk.Hub
@@ -532,8 +524,8 @@ def _serialize_envelope(self, envelope):
 
         return content_encoding, body
 
-    def _get_pool_options(self, ca_certs, cert_file=None, key_file=None):
-        # type: (Self, Optional[Any], Optional[Any], Optional[Any]) -> Dict[str, Any]
+    def _get_pool_options(self):
+        # type: (Self) -> Dict[str, Any]
         raise NotImplementedError()
 
     def _in_no_proxy(self, parsed_dsn):
@@ -547,17 +539,8 @@ def _in_no_proxy(self, parsed_dsn):
                 return True
         return False
 
-    def _make_pool(
-        self,
-        parsed_dsn,  # type: Dsn
-        http_proxy,  # type: Optional[str]
-        https_proxy,  # type: Optional[str]
-        ca_certs,  # type: Optional[Any]
-        cert_file,  # type: Optional[Any]
-        key_file,  # type: Optional[Any]
-        proxy_headers,  # type: Optional[Dict[str, str]]
-    ):
-        # type: (...) -> Union[PoolManager, ProxyManager, httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool]
+    def _make_pool(self):
+        # type: (Self) -> Union[PoolManager, ProxyManager, httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool]
         raise NotImplementedError()
 
     def _request(
@@ -631,8 +614,8 @@ class HttpTransport(BaseHttpTransport):
     if TYPE_CHECKING:
         _pool: Union[PoolManager, ProxyManager]
 
-    def _get_pool_options(self, ca_certs, cert_file=None, key_file=None):
-        # type: (Self, Any, Any, Any) -> Dict[str, Any]
+    def _get_pool_options(self):
+        # type: (Self) -> Dict[str, Any]
 
         num_pools = self.options.get("_experiments", {}).get("transport_num_pools")
         options = {
@@ -658,42 +641,43 @@ def _get_pool_options(self, ca_certs, cert_file=None, key_file=None):
             options["socket_options"] = socket_options
 
         options["ca_certs"] = (
-            ca_certs  # User-provided bundle from the SDK init
+            self.options["ca_certs"]  # User-provided bundle from the SDK init
             or os.environ.get("SSL_CERT_FILE")
             or os.environ.get("REQUESTS_CA_BUNDLE")
             or certifi.where()
         )
 
-        options["cert_file"] = cert_file or os.environ.get("CLIENT_CERT_FILE")
-        options["key_file"] = key_file or os.environ.get("CLIENT_KEY_FILE")
+        options["cert_file"] = self.options["cert_file"] or os.environ.get(
+            "CLIENT_CERT_FILE"
+        )
+        options["key_file"] = self.options["key_file"] or os.environ.get(
+            "CLIENT_KEY_FILE"
+        )
 
         return options
 
-    def _make_pool(
-        self,
-        parsed_dsn,  # type: Dsn
-        http_proxy,  # type: Optional[str]
-        https_proxy,  # type: Optional[str]
-        ca_certs,  # type: Any
-        cert_file,  # type: Any
-        key_file,  # type: Any
-        proxy_headers,  # type: Optional[Dict[str, str]]
-    ):
-        # type: (...) -> Union[PoolManager, ProxyManager]
+    def _make_pool(self):
+        # type: (Self) -> Union[PoolManager, ProxyManager]
+        if self.parsed_dsn is None:
+            raise ValueError("Cannot create HTTP-based transport without valid DSN")
+
         proxy = None
-        no_proxy = self._in_no_proxy(parsed_dsn)
+        no_proxy = self._in_no_proxy(self.parsed_dsn)
 
         # try HTTPS first
-        if parsed_dsn.scheme == "https" and (https_proxy != ""):
+        https_proxy = self.options["https_proxy"]
+        if self.parsed_dsn.scheme == "https" and (https_proxy != ""):
             proxy = https_proxy or (not no_proxy and getproxies().get("https"))
 
         # maybe fallback to HTTP proxy
+        http_proxy = self.options["http_proxy"]
         if not proxy and (http_proxy != ""):
             proxy = http_proxy or (not no_proxy and getproxies().get("http"))
 
-        opts = self._get_pool_options(ca_certs, cert_file, key_file)
+        opts = self._get_pool_options()
 
         if proxy:
+            proxy_headers = self.options["proxy_headers"]
             if proxy_headers:
                 opts["proxy_headers"] = proxy_headers
 
@@ -783,10 +767,11 @@ def _request(
             )
             return response
 
-        def _get_pool_options(self, ca_certs, cert_file=None, key_file=None):
-            # type: (Any, Any, Any) -> Dict[str, Any]
+        def _get_pool_options(self):
+            # type: (Self) -> Dict[str, Any]
             options = {
-                "http2": True,
+                "http2": self.parsed_dsn is not None
+                and self.parsed_dsn.scheme == "https",
                 "retries": 3,
             }  # type: Dict[str, Any]
 
@@ -805,45 +790,41 @@ def _get_pool_options(self, ca_certs, cert_file=None, key_file=None):
 
             ssl_context = ssl.create_default_context()
             ssl_context.load_verify_locations(
-                ca_certs  # User-provided bundle from the SDK init
+                self.options["ca_certs"]  # User-provided bundle from the SDK init
                 or os.environ.get("SSL_CERT_FILE")
                 or os.environ.get("REQUESTS_CA_BUNDLE")
                 or certifi.where()
             )
-            cert_file = cert_file or os.environ.get("CLIENT_CERT_FILE")
-            key_file = key_file or os.environ.get("CLIENT_KEY_FILE")
+            cert_file = self.options["cert_file"] or os.environ.get("CLIENT_CERT_FILE")
+            key_file = self.options["key_file"] or os.environ.get("CLIENT_KEY_FILE")
             if cert_file is not None:
                 ssl_context.load_cert_chain(cert_file, key_file)
 
             options["ssl_context"] = ssl_context
 
             return options
 
-        def _make_pool(
-            self,
-            parsed_dsn,  # type: Dsn
-            http_proxy,  # type: Optional[str]
-            https_proxy,  # type: Optional[str]
-            ca_certs,  # type: Any
-            cert_file,  # type: Any
-            key_file,  # type: Any
-            proxy_headers,  # type: Optional[Dict[str, str]]
-        ):
-            # type: (...) -> Union[httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool]
+        def _make_pool(self):
+            # type: (Self) -> Union[httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool]
+            if self.parsed_dsn is None:
+                raise ValueError("Cannot create HTTP-based transport without valid DSN")
             proxy = None
-            no_proxy = self._in_no_proxy(parsed_dsn)
+            no_proxy = self._in_no_proxy(self.parsed_dsn)
 
             # try HTTPS first
-            if parsed_dsn.scheme == "https" and (https_proxy != ""):
+            https_proxy = self.options["https_proxy"]
+            if self.parsed_dsn.scheme == "https" and (https_proxy != ""):
                 proxy = https_proxy or (not no_proxy and getproxies().get("https"))
 
             # maybe fallback to HTTP proxy
+            http_proxy = self.options["http_proxy"]
             if not proxy and (http_proxy != ""):
                 proxy = http_proxy or (not no_proxy and getproxies().get("http"))
 
-            opts = self._get_pool_options(ca_certs, cert_file, key_file)
+            opts = self._get_pool_options()
 
             if proxy:
+                proxy_headers = self.options["proxy_headers"]
                 if proxy_headers:
                     opts["proxy_headers"] = proxy_headers
 
diff --git a/tests/integrations/django/asgi/test_asgi.py b/tests/integrations/django/asgi/test_asgi.py
@@ -104,14 +104,16 @@ async def test_async_views(sentry_init, capture_events, application):
 @pytest.mark.skipif(
     django.VERSION < (3, 1), reason="async views have been introduced in Django 3.1"
 )
-async def test_active_thread_id(sentry_init, capture_envelopes, endpoint, application):
+async def test_active_thread_id(
+    sentry_init, capture_envelopes, teardown_profiling, endpoint, application
+):
     with mock.patch(
         "sentry_sdk.profiler.transaction_profiler.PROFILE_MINIMUM_SAMPLES", 0
     ):
         sentry_init(
             integrations=[DjangoIntegration()],
             traces_sample_rate=1.0,
-            _experiments={"profiles_sample_rate": 1.0},
+            profiles_sample_rate=1.0,
         )
 
         envelopes = capture_envelopes()
@@ -121,17 +123,26 @@ async def test_active_thread_id(sentry_init, capture_envelopes, endpoint, applic
         await comm.wait()
 
         assert response["status"] == 200, response["body"]
-        assert len(envelopes) == 1
 
-        profiles = [item for item in envelopes[0].items if item.type == "profile"]
-        assert len(profiles) == 1
+    assert len(envelopes) == 1
+
+    profiles = [item for item in envelopes[0].items if item.type == "profile"]
+    assert len(profiles) == 1
+
+    data = json.loads(response["body"])
+
+    for item in profiles:
+        transactions = item.payload.json["transactions"]
+        assert len(transactions) == 1
+        assert str(data["active"]) == transactions[0]["active_thread_id"]
 
-        data = json.loads(response["body"])
+    transactions = [item for item in envelopes[0].items if item.type == "transaction"]
+    assert len(transactions) == 1
 
-        for profile in profiles:
-            transactions = profile.payload.json["transactions"]
-            assert len(transactions) == 1
-            assert str(data["active"]) == transactions[0]["active_thread_id"]
+    for item in transactions:
+        transaction = item.payload.json
+        trace_context = transaction["contexts"]["trace"]
+        assert str(data["active"]) == trace_context["data"]["thread.id"]
 
 
 @pytest.mark.asyncio
diff --git a/tests/integrations/fastapi/test_fastapi.py b/tests/integrations/fastapi/test_fastapi.py
@@ -184,7 +184,7 @@ def test_legacy_setup(
 def test_active_thread_id(sentry_init, capture_envelopes, teardown_profiling, endpoint):
     sentry_init(
         traces_sample_rate=1.0,
-        _experiments={"profiles_sample_rate": 1.0},
+        profiles_sample_rate=1.0,
     )
     app = fastapi_app_factory()
     asgi_app = SentryAsgiMiddleware(app)
@@ -203,11 +203,19 @@ def test_active_thread_id(sentry_init, capture_envelopes, teardown_profiling, en
     profiles = [item for item in envelopes[0].items if item.type == "profile"]
     assert len(profiles) == 1
 
-    for profile in profiles:
-        transactions = profile.payload.json["transactions"]
+    for item in profiles:
+        transactions = item.payload.json["transactions"]
         assert len(transactions) == 1
         assert str(data["active"]) == transactions[0]["active_thread_id"]
 
+    transactions = [item for item in envelopes[0].items if item.type == "transaction"]
+    assert len(transactions) == 1
+
+    for item in transactions:
+        transaction = item.payload.json
+        trace_context = transaction["contexts"]["trace"]
+        assert str(data["active"]) == trace_context["data"]["thread.id"]
+
 
 @pytest.mark.asyncio
 async def test_original_request_not_scrubbed(sentry_init, capture_events):
diff --git a/tests/integrations/quart/test_quart.py b/tests/integrations/quart/test_quart.py
diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py
diff --git a/tests/integrations/strawberry/test_strawberry.py b/tests/integrations/strawberry/test_strawberry.py
diff --git a/tests/test_transport.py b/tests/test_transport.py
diff --git a/tox.ini b/tox.ini
