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

Author: sl0thentr0py

MIGRATION_GUIDE.md

@@ -11,6 +11,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.
 - 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.
@@ -19,11 +20,13 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
 - Redis integration: In Redis pipeline spans there is no `span["data"]["redis.commands"]` that contains a dict `{"count": 3, "first_ten": ["cmd1", "cmd2", ...]}` but instead `span["data"]["redis.commands.count"]` (containing `3`) and `span["data"]["redis.commands.first_ten"]` (containing `["cmd1", "cmd2", ...]`).
 - 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.
 
 ### Removed
 
 - Spans no longer have a `description`. Use `name` instead.
 - Dropped support for Python 3.6.
+- The `custom_sampling_context` parameter of `start_transaction` has been removed. Use `attributes` instead to set key-value pairs of data that should be accessible in the traces sampler. Note that span attributes need to conform to the [OpenTelemetry specification](https://opentelemetry.io/docs/concepts/signals/traces/#attributes), meaning only certain types can be set as values.
 - The PyMongo integration no longer sets tags. The data is still accessible via span attributes.
 - The PyMongo integration doesn't set `operation_ids` anymore. The individual IDs (`operation_id`, `request_id`, `session_id`) are now accessible as separate span attributes.
 - `sentry_sdk.metrics` and associated metrics APIs have been removed as Sentry no longer accepts metrics data in this form. See https://sentry.zendesk.com/hc/en-us/articles/26369339769883-Upcoming-API-Changes-to-Metrics

sentry_sdk/api.py

@@ -40,7 +40,6 @@
         ExcInfo,
         MeasurementUnit,
         LogLevelStr,
-        SamplingContext,
     )
     from sentry_sdk.tracing import Span, TransactionKwargs
 
@@ -239,12 +238,8 @@ def flush(
     return get_client().flush(timeout=timeout, callback=callback)
 
 
-def start_span(
-    *,
-    custom_sampling_context=None,
-    **kwargs,  # type: Any
-):
-    # type: (...) -> POTelSpan
+def start_span(**kwargs):
+    # type: (type.Any) -> POTelSpan
     """
     Start and return a span.
 
@@ -257,13 +252,11 @@ def start_span(
     of the `with` block. If not using context managers, call the `finish()`
     method.
     """
-    # TODO: Consider adding type hints to the method signature.
-    return get_current_scope().start_span(custom_sampling_context, **kwargs)
+    return get_current_scope().start_span(**kwargs)
 
 
 def start_transaction(
     transaction=None,  # type: Optional[Transaction]
-    custom_sampling_context=None,  # type: Optional[SamplingContext]
     **kwargs,  # type: Unpack[TransactionKwargs]
 ):
     # type: (...) -> POTelSpan
@@ -295,14 +288,12 @@ def start_transaction(
 
     :param transaction: The transaction to start. If omitted, we create and
         start a new transaction.
-    :param custom_sampling_context: The transaction's custom sampling context.
     :param kwargs: Optional keyword arguments to be passed to the Transaction
         constructor. See :py:class:`sentry_sdk.tracing.Transaction` for
         available arguments.
     """
     return start_span(
         span=transaction,
-        custom_sampling_context=custom_sampling_context,
         **kwargs,
     )
 

sentry_sdk/integrations/opentelemetry/consts.py

@@ -30,3 +30,4 @@ class SentrySpanAttribute:
     NAME = "sentry.name"
     SOURCE = "sentry.source"
     CONTEXT = "sentry.context"
+    CUSTOM_SAMPLED = "sentry.custom_sampled"  # used for saving start_span(sampled=X)

sentry_sdk/integrations/opentelemetry/sampler.py

@@ -2,7 +2,6 @@
 from typing import cast
 
 from opentelemetry import trace
-
 from opentelemetry.sdk.trace.sampling import Sampler, SamplingResult, Decision
 from opentelemetry.trace.span import TraceState
 
@@ -12,6 +11,7 @@
 from sentry_sdk.integrations.opentelemetry.consts import (
     TRACESTATE_SAMPLED_KEY,
     TRACESTATE_SAMPLE_RATE_KEY,
+    SentrySpanAttribute,
 )
 
 from typing import TYPE_CHECKING
@@ -114,28 +114,34 @@ def should_sample(
 
         parent_span_context = trace.get_current_span(parent_context).get_span_context()
 
+        attributes = attributes or {}
+
         # No tracing enabled, thus no sampling
         if not has_tracing_enabled(client.options):
             return dropped_result(parent_span_context, attributes)
 
-        sample_rate = None
+        # 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)
+            else:
+                return dropped_result(parent_span_context, attributes)
 
-        # Check if sampled=True was passed to start_transaction
-        # TODO-anton: Do we want to keep the start_transaction(sampled=True) thing?
+        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:
-            # TODO-anton: Make proper sampling_context
-            # TODO-neel-potel: Make proper sampling_context
             sampling_context = {
                 "transaction_context": {
                     "name": name,
+                    "op": attributes.get(SentrySpanAttribute.OP),
                 },
                 "parent_sampled": get_parent_sampled(parent_span_context, trace_id),
             }
-
+            sampling_context.update(attributes)
             sample_rate = client.options["traces_sampler"](sampling_context)
 
         else:

sentry_sdk/integrations/opentelemetry/scope.py

@@ -24,7 +24,6 @@
     from typing import Tuple, Optional, Generator, Dict, Any
     from typing_extensions import Unpack
 
-    from sentry_sdk._types import SamplingContext
     from sentry_sdk.tracing import TransactionKwargs
 
 
@@ -112,22 +111,31 @@ def _incoming_otel_span_context(self):
 
         return span_context
 
-    def start_transaction(self, custom_sampling_context=None, **kwargs):
-        # type: (Optional[SamplingContext], Unpack[TransactionKwargs]) -> POTelSpan
+    def start_transaction(self, **kwargs):
+        # type: (Unpack[TransactionKwargs]) -> POTelSpan
         """
         .. deprecated:: 3.0.0
             This function is deprecated and will be removed in a future release.
             Use :py:meth:`sentry_sdk.start_span` instead.
         """
-        return self.start_span(custom_sampling_context=custom_sampling_context)
+        return self.start_span(**kwargs)
 
-    def start_span(self, custom_sampling_context=None, **kwargs):
-        # type: (Optional[SamplingContext], Any) -> POTelSpan
+    def start_span(self, **kwargs):
+        # type: (Any) -> POTelSpan
         return POTelSpan(**kwargs, scope=self)
 
 
-_INITIAL_CURRENT_SCOPE = PotelScope(ty=ScopeType.CURRENT)
-_INITIAL_ISOLATION_SCOPE = PotelScope(ty=ScopeType.ISOLATION)
+_INITIAL_CURRENT_SCOPE = None
+_INITIAL_ISOLATION_SCOPE = None
+
+
+def _setup_initial_scopes():
+    global _INITIAL_CURRENT_SCOPE, _INITIAL_ISOLATION_SCOPE
+    _INITIAL_CURRENT_SCOPE = PotelScope(ty=ScopeType.CURRENT)
+    _INITIAL_ISOLATION_SCOPE = PotelScope(ty=ScopeType.ISOLATION)
+
+
+_setup_initial_scopes()
 
 
 @contextmanager

sentry_sdk/integrations/rq.py

@@ -108,11 +108,9 @@ def sentry_patched_handle_exception(self, job, *exc_info, **kwargs):
         @ensure_integration_enabled(RqIntegration, old_enqueue_job)
         def sentry_patched_enqueue_job(self, job, **kwargs):
             # type: (Queue, Any, **Any) -> Any
-            scope = sentry_sdk.get_current_scope()
-            if scope.span is not None:
-                job.meta["_sentry_trace_headers"] = dict(
-                    scope.iter_trace_propagation_headers()
-                )
+            job.meta["_sentry_trace_headers"] = dict(
+                sentry_sdk.get_current_scope().iter_trace_propagation_headers()
+            )
 
             return old_enqueue_job(self, job, **kwargs)
 

sentry_sdk/scope.py

@@ -497,7 +497,11 @@ def get_traceparent(self, *args, **kwargs):
         client = self.get_client()
 
         # If we have an active span, return traceparent from there
-        if has_tracing_enabled(client.options) and self.span is not None:
+        if (
+            has_tracing_enabled(client.options)
+            and self.span is not None
+            and self.span.is_valid
+        ):
             return self.span.to_traceparent()
 
         # If this scope has a propagation context, return traceparent from there
@@ -521,7 +525,11 @@ def get_baggage(self, *args, **kwargs):
         client = self.get_client()
 
         # If we have an active span, return baggage from there
-        if has_tracing_enabled(client.options) and self.span is not None:
+        if (
+            has_tracing_enabled(client.options)
+            and self.span is not None
+            and self.span.is_valid
+        ):
             return self.span.to_baggage()
 
         # If this scope has a propagation context, return baggage from there
@@ -610,7 +618,7 @@ def iter_trace_propagation_headers(self, *args, **kwargs):
         span = kwargs.pop("span", None)
         span = span or self.span
 
-        if has_tracing_enabled(client.options) and span is not None:
+        if has_tracing_enabled(client.options) and span is not None and span.is_valid:
             for header in span.iter_headers():
                 yield header
         else:
@@ -946,9 +954,7 @@ def add_breadcrumb(self, crumb=None, hint=None, **kwargs):
         while len(self._breadcrumbs) > max_breadcrumbs:
             self._breadcrumbs.popleft()
 
-    def start_transaction(
-        self, transaction=None, custom_sampling_context=None, **kwargs
-    ):
+    def start_transaction(self, transaction=None, **kwargs):
         # type: (Optional[Transaction], Optional[SamplingContext], Unpack[TransactionKwargs]) -> Union[Transaction, NoOpSpan]
         """
         Start and return a transaction.
@@ -974,7 +980,6 @@ def start_transaction(
 
         :param transaction: The transaction to start. If omitted, we create and
             start a new transaction.
-        :param custom_sampling_context: The transaction's custom sampling context.
         :param kwargs: Optional keyword arguments to be passed to the Transaction
             constructor. See :py:class:`sentry_sdk.tracing.Transaction` for
             available arguments.
@@ -985,8 +990,6 @@ def start_transaction(
 
         try_autostart_continuous_profiler()
 
-        custom_sampling_context = custom_sampling_context or {}
-
         # if we haven't been given a transaction, make one
         transaction = Transaction(**kwargs)
 
@@ -996,7 +999,6 @@ def start_transaction(
             "transaction_context": transaction.to_json(),
             "parent_sampled": transaction.parent_sampled,
         }
-        sampling_context.update(custom_sampling_context)
         transaction._set_initial_sampling_decision(sampling_context=sampling_context)
 
         if transaction.sampled:
@@ -1317,7 +1319,11 @@ def _apply_contexts_to_event(self, event, hint, options):
 
         # Add "trace" context
         if contexts.get("trace") is None:
-            if has_tracing_enabled(options) and self._span is not None:
+            if (
+                has_tracing_enabled(options)
+                and self._span is not None
+                and self._span.is_valid
+            ):
                 contexts["trace"] = self._span.get_trace_context()
             else:
                 contexts["trace"] = self.get_trace_context()

sentry_sdk/tracing.py

@@ -43,6 +43,8 @@
 
     from typing_extensions import TypedDict, Unpack
 
+    from opentelemetry.utils import types as OTelSpanAttributes
+
     P = ParamSpec("P")
     R = TypeVar("R")
 
@@ -1202,10 +1204,12 @@ def __init__(
         op=None,  # type: Optional[str]
         description=None,  # type: Optional[str]
         status=None,  # type: Optional[str]
+        sampled=None,  # type: Optional[bool]
         start_timestamp=None,  # type: Optional[Union[datetime, float]]
         origin=None,  # type: Optional[str]
         name=None,  # type: Optional[str]
         source=TRANSACTION_SOURCE_CUSTOM,  # type: str
+        attributes=None,  # type: OTelSpanAttributes
         only_if_parent=False,  # type: bool
         otel_span=None,  # type: Optional[OtelSpan]
         **_,  # type: dict[str, object]
@@ -1230,6 +1234,9 @@ def __init__(
             if skip_span:
                 self._otel_span = INVALID_SPAN
             else:
+                from sentry_sdk.integrations.opentelemetry.consts import (
+                    SentrySpanAttribute,
+                )
                 from sentry_sdk.integrations.opentelemetry.utils import (
                     convert_to_otel_timestamp,
                 )
@@ -1239,12 +1246,18 @@ def __init__(
                     start_timestamp = convert_to_otel_timestamp(start_timestamp)
 
                 span_name = name or description or op or ""
+
+                # Prepopulate some attrs so that they're accessible in traces_sampler
+                attributes = attributes or {}
+                attributes[SentrySpanAttribute.OP] = op
+                if sampled is not None:
+                    attributes[SentrySpanAttribute.CUSTOM_SAMPLED] = sampled
+
                 self._otel_span = tracer.start_span(
-                    span_name, start_time=start_timestamp
+                    span_name, start_time=start_timestamp, attributes=attributes
                 )
 
                 self.origin = origin or DEFAULT_SPAN_ORIGIN
-                self.op = op
                 self.description = description
                 self.name = span_name
                 self.source = source
@@ -1376,6 +1389,11 @@ def span_id(self):
         # type: () -> str
         return format_span_id(self._otel_span.get_span_context().span_id)
 
+    @property
+    def is_valid(self):
+        # type: () -> bool
+        return self._otel_span.get_span_context().is_valid
+
     @property
     def sampled(self):
         # type: () -> Optional[bool]

tests/conftest.py

@@ -74,8 +74,7 @@ def clean_scopes():
     scope._isolation_scope.set(None)
     scope._current_scope.set(None)
 
-    potel_scope._INITIAL_CURRENT_SCOPE.clear()
-    potel_scope._INITIAL_ISOLATION_SCOPE.clear()
+    potel_scope._setup_initial_scopes()
 
 
 @pytest.fixture(autouse=True)

tests/integrations/aiohttp/test_aiohttp.py

@@ -535,8 +535,6 @@ async def handler(request):
     with start_transaction(
         name="/interactions/other-dogs/new-dog",
         op="greeting.sniff",
-        # make trace_id difference between transactions
-        trace_id="0123456789012345678901234567890",
     ) as transaction:
         client = await aiohttp_client(raw_server)
         resp = await client.get("/")
@@ -572,14 +570,21 @@ async def handler(request):
     with start_transaction(
         name="/interactions/other-dogs/new-dog",
         op="greeting.sniff",
-        trace_id="0123456789012345678901234567890",
-    ):
+    ) as transaction:
         client = await aiohttp_client(raw_server)
         resp = await client.get("/", headers={"bagGage": "custom=value"})
 
         assert (
-            resp.request_info.headers["baggage"]
-            == "custom=value,sentry-trace_id=0123456789012345678901234567890,sentry-environment=production,sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42,sentry-transaction=/interactions/other-dogs/new-dog,sentry-sample_rate=1.0,sentry-sampled=true"
+            sorted(resp.request_info.headers["baggage"].split(","))
+            == sorted([
+                "custom=value",
+                f"sentry-trace_id={transaction.trace_id}",
+                "sentry-environment=production",
+                "sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42",
+                "sentry-transaction=/interactions/other-dogs/new-dog",
+                "sentry-sample_rate=1.0",
+                "sentry-sampled=true",
+            ])
         )