Merge branch 'potel-base' into potel-base-run-all-tests

Author: sl0thentr0py

MIGRATION_GUIDE.md

@@ -10,8 +10,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
 
 - The SDK now supports Python 3.7 and higher.
 - `sentry_sdk.start_span` now only takes keyword arguments.
-- `sentry_sdk.start_span` no longer takes an explicit `span` argument.
-- `sentry_sdk.start_span` no longer takes explicit `trace_id`, `span_id` or `parent_span_id` arguments.
+- `sentry_sdk.start_transaction`/`sentry_sdk.start_span` no longer takes the following arguments: `span`, `parent_sampled`, `trace_id`, `span_id` or `parent_span_id`.
 - The `Span()` constructor does not accept a `hub` parameter anymore.
 - `Span.finish()` does not accept a `hub` parameter anymore.
 - The `Profile()` constructor does not accept a `hub` parameter anymore.
@@ -21,6 +20,18 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
 - clickhouse-driver integration: The query is now available under the `db.query.text` span attribute (only if `send_default_pii` is `True`).
 - `sentry_sdk.init` now returns `None` instead of a context manager.
 - The `sampling_context` argument of `traces_sampler` now additionally contains all span attributes known at span start.
+- The `sampling_context` argument of `traces_sampler` doesn't contain the `asgi_scope` object anymore for ASGI frameworks. Instead, the individual properties on the scope, if available, are accessible as follows:
+
+  | Scope property | Sampling context key(s)         |
+  | -------------- | ------------------------------- |
+  | `type`         | `network.protocol.name`         |
+  | `scheme`       | `url.scheme`                    |
+  | `path`         | `url.path`                      |
+  | `http_version` | `network.protocol.version`      |
+  | `method`       | `http.request.method`           |
+  | `server`       | `server.address`, `server.port` |
+  | `client`       | `client.address`, `client.port` |
+  | full URL       | `url.full`                      |
 
 ### Removed
 

sentry_sdk/integrations/_asgi_common.py

@@ -32,8 +32,8 @@ def _get_headers(asgi_scope):
     return headers
 
 
-def _get_url(asgi_scope, default_scheme, host):
-    # type: (Dict[str, Any], Literal["ws", "http"], Optional[Union[AnnotatedValue, str]]) -> str
+def _get_url(asgi_scope, default_scheme=None, host=None):
+    # type: (Dict[str, Any], Optional[Literal["ws", "http"]], Optional[Union[AnnotatedValue, str]]) -> str
     """
     Extract URL from the ASGI scope, without also including the querystring.
     """

sentry_sdk/integrations/asgi.py

@@ -15,6 +15,7 @@
 
 from sentry_sdk.integrations._asgi_common import (
     _get_headers,
+    _get_query,
     _get_request_data,
     _get_url,
 )
@@ -57,6 +58,14 @@
 
 TRANSACTION_STYLE_VALUES = ("endpoint", "url")
 
+ASGI_SCOPE_PROPERTY_TO_ATTRIBUTE = {
+    "http_version": "network.protocol.version",
+    "method": "http.request.method",
+    "path": "url.path",
+    "scheme": "url.scheme",
+    "type": "network.protocol.name",
+}
+
 
 def _capture_exception(exc, mechanism_type="asgi"):
     # type: (Any, str) -> None
@@ -209,27 +218,25 @@ async def _run_app(self, scope, receive, send, asgi_version):
                                 name=transaction_name,
                                 source=transaction_source,
                                 origin=self.span_origin,
-                                custom_sampling_context={"asgi_scope": scope},
+                                attributes=_prepopulate_attributes(scope),
                             )
                             if should_trace
                             else nullcontext()
-                        ) as transaction:
-                            if transaction is not None:
-                                logger.debug(
-                                    "[ASGI] Started transaction: %s", transaction
-                                )
-                                transaction.set_tag("asgi.type", ty)
+                        ) as span:
+                            if span is not None:
+                                logger.debug("[ASGI] Started transaction: %s", span)
+                                span.set_tag("asgi.type", ty)
                             try:
 
                                 async def _sentry_wrapped_send(event):
                                     # type: (Dict[str, Any]) -> Any
                                     is_http_response = (
                                         event.get("type") == "http.response.start"
-                                        and transaction is not None
+                                        and span is not None
                                         and "status" in event
                                     )
                                     if is_http_response:
-                                        transaction.set_http_status(event["status"])
+                                        span.set_http_status(event["status"])
 
                                     return await send(event)
 
@@ -324,3 +331,35 @@ def _get_transaction_name_and_source(self, transaction_style, asgi_scope):
             return name, source
 
         return name, source
+
+
+def _prepopulate_attributes(scope):
+    # type: (Any) -> dict[str, Any]
+    """Unpack ASGI scope into serializable OTel attributes."""
+    scope = scope or {}
+
+    attributes = {}
+    for attr, key in ASGI_SCOPE_PROPERTY_TO_ATTRIBUTE.items():
+        if scope.get(attr):
+            attributes[key] = scope[attr]
+
+    for attr in ("client", "server"):
+        if scope.get(attr):
+            try:
+                host, port = scope[attr]
+                attributes[f"{attr}.address"] = host
+                attributes[f"{attr}.port"] = port
+            except Exception:
+                pass
+
+    try:
+        full_url = _get_url(scope)
+        query = _get_query(scope)
+        if query:
+            full_url = f"{full_url}?{query}"
+
+        attributes["url.full"] = full_url
+    except Exception:
+        pass
+
+    return attributes

sentry_sdk/integrations/opentelemetry/sampler.py

@@ -120,24 +120,34 @@ def should_sample(
         if not has_tracing_enabled(client.options):
             return dropped_result(parent_span_context, attributes)
 
+        # parent_span_context.is_valid means this span has a parent, remote or local
+        is_root_span = not parent_span_context.is_valid or parent_span_context.is_remote
+
         # Explicit sampled value provided at start_span
         if attributes.get(SentrySpanAttribute.CUSTOM_SAMPLED) is not None:
-            sample_rate = float(attributes[SentrySpanAttribute.CUSTOM_SAMPLED])
-            if sample_rate > 0:
-                return sampled_result(parent_span_context, attributes, sample_rate)
+            if is_root_span:
+                sample_rate = float(attributes[SentrySpanAttribute.CUSTOM_SAMPLED])
+                if sample_rate > 0:
+                    return sampled_result(parent_span_context, attributes, sample_rate)
+                else:
+                    return dropped_result(parent_span_context, attributes)
             else:
-                return dropped_result(parent_span_context, attributes)
+                logger.debug(
+                    f"[Tracing] Ignoring sampled param for non-root span {name}"
+                )
 
         sample_rate = None
 
         # Check if there is a traces_sampler
         # Traces_sampler is responsible to check parent sampled to have full transactions.
         has_traces_sampler = callable(client.options.get("traces_sampler"))
-        if has_traces_sampler:
+
+        if is_root_span and has_traces_sampler:
             sampling_context = {
                 "transaction_context": {
                     "name": name,
                     "op": attributes.get(SentrySpanAttribute.OP),
+                    "source": attributes.get(SentrySpanAttribute.SOURCE),
                 },
                 "parent_sampled": get_parent_sampled(parent_span_context, trace_id),
             }
@@ -161,8 +171,7 @@ def should_sample(
             return dropped_result(parent_span_context, attributes)
 
         # Down-sample in case of back pressure monitor says so
-        # TODO: this should only be done for transactions (aka root spans)
-        if client.monitor:
+        if is_root_span and client.monitor:
             sample_rate /= 2**client.monitor.downsample_factor
 
         # Roll the dice on sample rate

sentry_sdk/tracing.py

@@ -1228,9 +1228,11 @@ def __init__(
         if otel_span is not None:
             self._otel_span = otel_span
         else:
-            skip_span = (
-                only_if_parent and not get_current_span().get_span_context().is_valid
-            )
+            skip_span = False
+            if only_if_parent:
+                parent_span_context = get_current_span().get_span_context()
+                skip_span = not parent_span_context.is_valid or parent_span_context.is_remote
+
             if skip_span:
                 self._otel_span = INVALID_SPAN
             else:
@@ -1250,6 +1252,7 @@ def __init__(
                 # Prepopulate some attrs so that they're accessible in traces_sampler
                 attributes = attributes or {}
                 attributes[SentrySpanAttribute.OP] = op
+                attributes[SentrySpanAttribute.SOURCE] = source
                 if sampled is not None:
                     attributes[SentrySpanAttribute.CUSTOM_SAMPLED] = sampled
 
@@ -1260,7 +1263,6 @@ def __init__(
                 self.origin = origin or DEFAULT_SPAN_ORIGIN
                 self.description = description
                 self.name = span_name
-                self.source = source
 
                 if status is not None:
                     self.set_status(status)

tests/integrations/asgi/test_asgi.py

@@ -721,3 +721,24 @@ async def test_custom_transaction_name(
     assert transaction_event["type"] == "transaction"
     assert transaction_event["transaction"] == "foobar"
     assert transaction_event["transaction_info"] == {"source": "custom"}
+
+
+@pytest.mark.asyncio
+async def test_asgi_scope_in_traces_sampler(sentry_init, asgi3_app):
+    def dummy_traces_sampler(sampling_context):
+        assert sampling_context["url.path"] == "/test"
+        assert sampling_context["url.scheme"] == "http"
+        assert sampling_context["url.full"] == "/test?hello=there"
+        assert sampling_context["http.request.method"] == "GET"
+        assert sampling_context["network.protocol.version"] == "1.1"
+        assert sampling_context["network.protocol.name"] == "http"
+
+    sentry_init(
+        traces_sampler=dummy_traces_sampler,
+        traces_sample_rate=1.0,
+    )
+
+    app = SentryAsgiMiddleware(asgi3_app)
+
+    async with TestClient(app) as client:
+        await client.get("/test?hello=there")

tests/integrations/fastapi/test_fastapi.py

@@ -246,7 +246,6 @@ async def _error(request: Request):
     assert event["request"]["headers"]["authorization"] == "[Filtered]"
 
 
-@pytest.mark.asyncio
 def test_response_status_code_ok_in_transaction_context(sentry_init, capture_envelopes):
     """
     Tests that the response status code is added to the transaction "response" context.
@@ -275,7 +274,6 @@ def test_response_status_code_ok_in_transaction_context(sentry_init, capture_env
     assert transaction["contexts"]["response"]["status_code"] == 200
 
 
-@pytest.mark.asyncio
 def test_response_status_code_error_in_transaction_context(
     sentry_init,
     capture_envelopes,
@@ -312,7 +310,6 @@ def test_response_status_code_error_in_transaction_context(
     assert transaction["contexts"]["response"]["status_code"] == 500
 
 
-@pytest.mark.asyncio
 def test_response_status_code_not_found_in_transaction_context(
     sentry_init,
     capture_envelopes,

tests/tracing/test_sampling.py

@@ -6,34 +6,33 @@
 
 import sentry_sdk
 from sentry_sdk import start_span, start_transaction, capture_exception
-from sentry_sdk.tracing import Transaction
 from sentry_sdk.utils import logger
 
 
-def test_sampling_decided_only_for_transactions(sentry_init, capture_events):
+def test_sampling_decided_only_for_root_spans(sentry_init):
     sentry_init(traces_sample_rate=0.5)
 
-    with start_transaction(name="hi") as transaction:
-        assert transaction.sampled is not None
+    with start_span(name="outer1") as root_span1:
+        assert root_span1.sampled is not None
 
-        with start_span() as span:
-            assert span.sampled == transaction.sampled
+        with start_span(name="inner") as span:
+            assert span.sampled == root_span1.sampled
 
-    with start_span() as span:
-        assert span.sampled is None
+    with start_span(name="outer2") as root_span2:
+        assert root_span2.sampled is not None
 
 
 @pytest.mark.parametrize("sampled", [True, False])
-def test_nested_transaction_sampling_override(sentry_init, sampled):
+def test_nested_span_sampling_override(sentry_init, sampled):
     sentry_init(traces_sample_rate=1.0)
 
-    with start_transaction(name="outer", sampled=sampled) as outer_transaction:
-        assert outer_transaction.sampled is sampled
-        with start_transaction(
-            name="inner", sampled=(not sampled)
-        ) as inner_transaction:
-            assert inner_transaction.sampled is not sampled
-        assert outer_transaction.sampled is sampled
+    with start_span(name="outer", sampled=sampled) as outer_span:
+        assert outer_span.sampled is sampled
+        with start_span(name="inner", sampled=(not sampled)) as inner_span:
+            # won't work because the child span inherits the sampling decision
+            # from the parent
+            assert inner_span.sampled is sampled
+        assert outer_span.sampled is sampled
 
 
 def test_no_double_sampling(sentry_init, capture_events):
@@ -147,10 +146,17 @@ def test_ignores_inherited_sample_decision_when_traces_sampler_defined(
     traces_sampler = mock.Mock(return_value=not parent_sampling_decision)
     sentry_init(traces_sampler=traces_sampler)
 
-    transaction = start_transaction(
-        name="dogpark", parent_sampled=parent_sampling_decision
+    sentry_trace_header = (
+        "12312012123120121231201212312012-1121201211212012-{sampled}".format(
+            sampled=int(parent_sampling_decision)
+        )
     )
-    assert transaction.sampled is not parent_sampling_decision
+
+    with sentry_sdk.continue_trace({"sentry-trace": sentry_trace_header}):
+        with sentry_sdk.start_span(name="dogpark") as span:
+            pass
+
+    assert span.sampled is not parent_sampling_decision
 
 
 @pytest.mark.parametrize("explicit_decision", [True, False])
@@ -176,39 +182,38 @@ def test_inherits_parent_sampling_decision_when_traces_sampler_undefined(
     sentry_init(traces_sample_rate=0.5)
     mock_random_value = 0.25 if parent_sampling_decision is False else 0.75
 
-    with mock.patch.object(random, "random", return_value=mock_random_value):
-        transaction = start_transaction(
-            name="dogpark", parent_sampled=parent_sampling_decision
+    sentry_trace_header = (
+        "12312012123120121231201212312012-1121201211212012-{sampled}".format(
+            sampled=int(parent_sampling_decision)
         )
-        assert transaction.sampled is parent_sampling_decision
+    )
+    with mock.patch.object(random, "random", return_value=mock_random_value):
+        with sentry_sdk.continue_trace({"sentry-trace": sentry_trace_header}):
+            with start_span(name="dogpark") as span:
+                pass
+
+    assert span.sampled is parent_sampling_decision
 
 
 @pytest.mark.parametrize("parent_sampling_decision", [True, False])
 def test_passes_parent_sampling_decision_in_sampling_context(
     sentry_init, parent_sampling_decision
 ):
-    sentry_init(traces_sample_rate=1.0)
+    def dummy_traces_sampler(sampling_context):
+        assert sampling_context["parent_sampled"] is parent_sampling_decision
+        return 1.0
+
+    sentry_init(traces_sample_rate=1.0, traces_sampler=dummy_traces_sampler)
 
     sentry_trace_header = (
         "12312012123120121231201212312012-1121201211212012-{sampled}".format(
             sampled=int(parent_sampling_decision)
         )
     )
 
-    transaction = Transaction.continue_from_headers(
-        headers={"sentry-trace": sentry_trace_header}, name="dogpark"
-    )
-    spy = mock.Mock(wraps=transaction)
-    start_transaction(transaction=spy)
-
-    # there's only one call (so index at 0) and kwargs are always last in a call
-    # tuple (so index at -1)
-    sampling_context = spy._set_initial_sampling_decision.mock_calls[0][-1][
-        "sampling_context"
-    ]
-    assert "parent_sampled" in sampling_context
-    # because we passed in a spy, attribute access requires unwrapping
-    assert sampling_context["parent_sampled"]._mock_wraps is parent_sampling_decision
+    with sentry_sdk.continue_trace({"sentry-trace": sentry_trace_header}):
+        with sentry_sdk.start_span(name="dogpark"):
+            pass
 
 
 def test_passes_attributes_from_start_span_to_traces_sampler(