From 2e37add0e1b7b152f7791b6c5d106036f2691e27 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 8 Jun 2020 13:20:14 +0200 Subject: [PATCH 01/14] ref(am): Introduce start_transaction --- sentry_sdk/api.py | 15 +- sentry_sdk/hub.py | 58 +++++- sentry_sdk/integrations/aiohttp.py | 16 +- sentry_sdk/integrations/asgi.py | 15 +- sentry_sdk/integrations/celery.py | 14 +- sentry_sdk/integrations/rq.py | 13 +- sentry_sdk/integrations/wsgi.py | 12 +- sentry_sdk/scope.py | 9 +- sentry_sdk/tracing.py | 191 +++++++++++-------- tests/integrations/stdlib/test_subprocess.py | 4 +- tests/test_tracing.py | 73 ++++--- 11 files changed, 265 insertions(+), 155 deletions(-) diff --git a/sentry_sdk/api.py b/sentry_sdk/api.py index 9224a0aeca..7cf690ed69 100644 --- a/sentry_sdk/api.py +++ b/sentry_sdk/api.py @@ -17,7 +17,7 @@ from typing import Union from sentry_sdk._types import Event, Hint, Breadcrumb, BreadcrumbHint, ExcInfo - from sentry_sdk.tracing import Span + from sentry_sdk.tracing import Span, Transaction T = TypeVar("T") F = TypeVar("F", bound=Callable[..., Any]) @@ -38,6 +38,7 @@ def overload(x): "flush", "last_event_id", "start_span", + "start_transaction", "set_tag", "set_context", "set_extra", @@ -255,3 +256,15 @@ def start_span( # TODO: All other functions in this module check for # `Hub.current is None`. That actually should never happen? return Hub.current.start_span(span=span, **kwargs) + + +@hubmethod +def start_transaction( + span_or_name=None, # type: Optional[Union[Span, str]] + **kwargs # type: Any +): + # type: (...) -> Transaction + + # TODO: All other functions in this module check for + # `Hub.current is None`. That actually should never happen? + return Hub.current.start_transaction(span_or_name=span_or_name, **kwargs) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 18558761cf..2ca37b7701 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -8,7 +8,7 @@ from sentry_sdk._compat import with_metaclass from sentry_sdk.scope import Scope from sentry_sdk.client import Client -from sentry_sdk.tracing import Span +from sentry_sdk.tracing import Span, Transaction from sentry_sdk.sessions import Session from sentry_sdk.utils import ( exc_info_from_error, @@ -445,10 +445,9 @@ def start_span( span, if any. The return value is the span object that can be used as a context manager to start and stop timing. - Note that you will not see any span that is not contained - within a transaction. Create a transaction with - ``start_span(transaction="my transaction")`` if an - integration doesn't already do this for you. + Note that you will not see any span that is not contained within a + transaction. Most integrations already do this for you, but create a + transaction with `start_transaction` otherwise. """ client, scope = self._stack[-1] @@ -462,17 +461,56 @@ def start_span( else: span = Span(**kwargs) - if span.sampled is None and span.transaction is not None: + elif isinstance(span, Transaction): + raise ValueError("Pass transactions to start_transaction instead") + + return span + + def start_transaction( + self, + span_or_name=None, # type: Optional[Union[Span, str]] + **kwargs # type: Any + ): + # type: (...) -> Transaction + """ + Create a new transaction detached from the current span. The return + value is the `Transaction` object which for the most part works like a + span. + """ + + kwargs.setdefault("hub", self) + + if isinstance(span_or_name, str): + if "name" in kwargs: + raise ValueError("Cannot specify transaction name twice.") + kwargs["name"] = span_or_name + transaction = Transaction(**kwargs) + + elif span_or_name is None and "name" in kwargs: + transaction = Transaction(**kwargs) + + elif isinstance(span_or_name, Transaction): + transaction = span_or_name + + elif span_or_name is None and "span" in kwargs: + transaction = kwargs.pop("span") + + else: + raise ValueError("transaction object or name required.") + + client, scope = self._stack[-1] + + if transaction.sampled is None: sample_rate = client and client.options["traces_sample_rate"] or 0 - span.sampled = random.random() < sample_rate + transaction.sampled = random.random() < sample_rate - if span.sampled: + if transaction.sampled: max_spans = ( client and client.options["_experiments"].get("max_spans") or 1000 ) - span.init_finished_spans(maxlen=max_spans) + transaction.init_span_recorder(maxlen=max_spans) - return span + return transaction @overload # noqa def push_scope( diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 63bd827669..08afd2ae26 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -9,7 +9,7 @@ _filter_headers, request_body_within_bounds, ) -from sentry_sdk.tracing import Span +from sentry_sdk.tracing import Transaction from sentry_sdk.utils import ( capture_internal_exceptions, event_from_exception, @@ -85,13 +85,15 @@ async def sentry_app_handle(self, request, *args, **kwargs): scope.clear_breadcrumbs() scope.add_event_processor(_make_request_processor(weak_request)) - span = Span.continue_from_headers(request.headers) - span.op = "http.server" - # If this transaction name makes it to the UI, AIOHTTP's - # URL resolver did not find a route or died trying. - span.transaction = "generic AIOHTTP request" + span = Transaction.continue_from_headers( + request.headers, + op="http.server", + # If this transaction name makes it to the UI, AIOHTTP's + # URL resolver did not find a route or died trying. + name="generic AIOHTTP request", + ) - with hub.start_span(span): + with hub.start_transaction(span): try: response = await old_handle(self, request) except HTTPException as e: diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index 202c49025a..07b9575741 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -19,7 +19,7 @@ HAS_REAL_CONTEXTVARS, CONTEXTVARS_ERROR_MESSAGE, ) -from sentry_sdk.tracing import Span +from sentry_sdk.tracing import Transaction if MYPY: from typing import Dict @@ -123,16 +123,17 @@ async def _run_app(self, scope, callback): ty = scope["type"] if ty in ("http", "websocket"): - span = Span.continue_from_headers(dict(scope["headers"])) - span.op = "{}.server".format(ty) + span = Transaction.continue_from_headers( + dict(scope["headers"]), + name=_DEFAULT_TRANSACTION_NAME, + op="{}.server".format(ty), + ) else: - span = Span() - span.op = "asgi.server" + span = Transaction(name=_DEFAULT_TRANSACTION_NAME, op="asgi.server") span.set_tag("asgi.type", ty) - span.transaction = _DEFAULT_TRANSACTION_NAME - with hub.start_span(span) as span: + with hub.start_transaction(span): # XXX: Would be cool to have correct span status, but we # would have to wrap send(). That is a bit hard to do with # the current abstraction over ASGI 2/3. diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py index 5ac0d32f40..5f70657f2f 100644 --- a/sentry_sdk/integrations/celery.py +++ b/sentry_sdk/integrations/celery.py @@ -4,7 +4,7 @@ from sentry_sdk.hub import Hub from sentry_sdk.utils import capture_internal_exceptions, event_from_exception -from sentry_sdk.tracing import Span +from sentry_sdk.tracing import Transaction from sentry_sdk._compat import reraise from sentry_sdk.integrations import Integration, DidNotEnable from sentry_sdk.integrations.logging import ignore_logger @@ -130,9 +130,11 @@ def _inner(*args, **kwargs): scope.clear_breadcrumbs() scope.add_event_processor(_make_event_processor(task, *args, **kwargs)) - span = Span.continue_from_headers(args[3].get("headers") or {}) - span.op = "celery.task" - span.transaction = "unknown celery task" + span = Transaction.continue_from_headers( + args[3].get("headers") or {}, + op="celery.task", + name="unknown celery task", + ) # Could possibly use a better hook than this one span.set_status("ok") @@ -140,9 +142,9 @@ def _inner(*args, **kwargs): with capture_internal_exceptions(): # Celery task objects are not a thing to be trusted. Even # something such as attribute access can fail. - span.transaction = task.name + span.name = task.name - with hub.start_span(span): + with hub.start_transaction(span): return f(*args, **kwargs) return _inner # type: ignore diff --git a/sentry_sdk/integrations/rq.py b/sentry_sdk/integrations/rq.py index fbe8cdda3d..d950f7fbf7 100644 --- a/sentry_sdk/integrations/rq.py +++ b/sentry_sdk/integrations/rq.py @@ -4,7 +4,7 @@ from sentry_sdk.hub import Hub from sentry_sdk.integrations import Integration, DidNotEnable -from sentry_sdk.tracing import Span +from sentry_sdk.tracing import Transaction from sentry_sdk.utils import capture_internal_exceptions, event_from_exception @@ -61,15 +61,16 @@ def sentry_patched_perform_job(self, job, *args, **kwargs): scope.clear_breadcrumbs() scope.add_event_processor(_make_event_processor(weakref.ref(job))) - span = Span.continue_from_headers( - job.meta.get("_sentry_trace_headers") or {} + span = Transaction.continue_from_headers( + job.meta.get("_sentry_trace_headers") or {}, + op="rq.task", + name="unknown RQ task", ) - span.op = "rq.task" with capture_internal_exceptions(): - span.transaction = job.func_name + span.name = job.func_name - with hub.start_span(span): + with hub.start_transaction(span): rv = old_perform_job(self, job, *args, **kwargs) if self.is_horse: diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index bd87663896..b28d36b259 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -8,7 +8,7 @@ event_from_exception, ) from sentry_sdk._compat import PY2, reraise, iteritems -from sentry_sdk.tracing import Span +from sentry_sdk.tracing import Transaction from sentry_sdk.sessions import auto_session_tracking from sentry_sdk.integrations._wsgi_common import _filter_headers @@ -113,11 +113,11 @@ def __call__(self, environ, start_response): _make_wsgi_event_processor(environ) ) - span = Span.continue_from_environ(environ) - span.op = "http.server" - span.transaction = "generic WSGI request" + span = Transaction.continue_from_environ( + environ, op="http.server", name="generic WSGI request" + ) - with hub.start_span(span) as span: + with hub.start_transaction(span) as span: try: rv = self.app( environ, @@ -133,7 +133,7 @@ def __call__(self, environ, start_response): def _sentry_start_response( old_start_response, # type: StartResponse - span, # type: Span + span, # type: Transaction status, # type: str response_headers, # type: WsgiResponseHeaders exc_info=None, # type: Optional[WsgiExcInfo] diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index c721b56505..614661f438 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -5,6 +5,7 @@ from sentry_sdk._functools import wraps from sentry_sdk._types import MYPY from sentry_sdk.utils import logger, capture_internal_exceptions +from sentry_sdk.tracing import Transaction if MYPY: from typing import Any @@ -140,8 +141,8 @@ def transaction(self, value): """When set this forces a specific transaction name to be set.""" self._transaction = value span = self._span - if span: - span.transaction = value + if span and isinstance(span, Transaction) and value: + span.name = value @_attr_setter def user(self, value): @@ -166,8 +167,8 @@ def span(self): def span(self, span): # type: (Optional[Span]) -> None self._span = span - if span is not None: - span_transaction = span.transaction + if isinstance(span, Transaction): + span_transaction = span.name if span_transaction: self._transaction = span_transaction diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index b3dbde6f65..021444bc6b 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -25,6 +25,10 @@ from typing import Dict from typing import List from typing import Tuple + from typing import TypeVar + from typing import Type + + SpanTy = TypeVar("SpanTy", bound="Span") _traceparent_header_format_re = re.compile( "^[ \t]*" # whitespace @@ -67,28 +71,20 @@ def __iter__(self): class _SpanRecorder(object): - __slots__ = ("maxlen", "finished_spans", "open_span_count") + __slots__ = ("maxlen", "spans") def __init__(self, maxlen): # type: (int) -> None self.maxlen = maxlen - self.open_span_count = 0 # type: int - self.finished_spans = [] # type: List[Span] + self.spans = [] # type: List[Span] - def start_span(self, span): + def add(self, span): # type: (Span) -> None - # This is just so that we don't run out of memory while recording a lot - # of spans. At some point we just stop and flush out the start of the - # trace tree (i.e. the first n spans with the smallest - # start_timestamp). - self.open_span_count += 1 - if self.open_span_count > self.maxlen: + if len(self.spans) >= self.maxlen: span._span_recorder = None - - def finish_span(self, span): - # type: (Span) -> None - self.finished_spans.append(span) + else: + self.spans.append(span) class Span(object): @@ -98,7 +94,6 @@ class Span(object): "parent_span_id", "same_process_as_parent", "sampled", - "transaction", "op", "description", "start_timestamp", @@ -119,7 +114,6 @@ def __init__( parent_span_id=None, # type: Optional[str] same_process_as_parent=True, # type: bool sampled=None, # type: Optional[bool] - transaction=None, # type: Optional[str] op=None, # type: Optional[str] description=None, # type: Optional[str] hub=None, # type: Optional[sentry_sdk.Hub] @@ -130,9 +124,8 @@ def __init__( self.span_id = span_id or uuid.uuid4().hex[16:] self.parent_span_id = parent_span_id self.same_process_as_parent = same_process_as_parent - self.sampled = sampled - self.transaction = transaction self.op = op + self.sampled = sampled self.description = description self.status = status self.hub = hub @@ -153,19 +146,19 @@ def __init__( self._span_recorder = None # type: Optional[_SpanRecorder] - def init_finished_spans(self, maxlen): + def init_span_recorder(self, maxlen): # type: (int) -> None if self._span_recorder is None: self._span_recorder = _SpanRecorder(maxlen) - self._span_recorder.start_span(self) + self._span_recorder.add(self) def __repr__(self): # type: () -> str return ( - "<%s(transaction=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" + "<%s(name=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( self.__class__.__name__, - self.transaction, + getattr(self, "name", None), self.trace_id, self.span_id, self.parent_span_id, @@ -173,8 +166,10 @@ def __repr__(self): ) ) - def __enter__(self): - # type: () -> Span + def __enter__( + self, # type: SpanTy + ): + # type: (...) -> SpanTy hub = self.hub or sentry_sdk.Hub.current _, scope = hub._stack[-1] @@ -194,25 +189,45 @@ def __exit__(self, ty, value, tb): self.finish(hub) scope.span = old_span - def new_span(self, **kwargs): + def start_child(self, **kwargs): # type: (**Any) -> Span + """ + Start a sub-span from the current span or transaction. + + Takes the same arguments as the initializer of :py:class:`Span`. No + attributes other than the sample rate are inherited. + """ kwargs.setdefault("sampled", self.sampled) - rv = type(self)( + + rv = Span( trace_id=self.trace_id, span_id=None, parent_span_id=self.span_id, **kwargs ) - rv._span_recorder = self._span_recorder + rv._span_recorder = recorder = self._span_recorder + if recorder: + recorder.add(rv) return rv + # deprecated + new_span = start_child + @classmethod - def continue_from_environ(cls, environ): - # type: (typing.Mapping[str, str]) -> Span - return cls.continue_from_headers(EnvironHeaders(environ)) + def continue_from_environ( + cls, # type: Type[SpanTy] + environ, # type: typing.Mapping[str, str] + **kwargs # type: Any + ): + # type: (...) -> SpanTy + return cls.continue_from_headers(EnvironHeaders(environ), **kwargs) @classmethod - def continue_from_headers(cls, headers): - # type: (typing.Mapping[str, str]) -> Span - parent = cls.from_traceparent(headers.get("sentry-trace")) + def continue_from_headers( + cls, # type: Type[SpanTy] + headers, # type: typing.Mapping[str, str] + **kwargs # type: Any + ): + # type: (...) -> SpanTy + parent = cls.from_traceparent(headers.get("sentry-trace"), **kwargs) if parent is None: return cls() parent.same_process_as_parent = False @@ -223,8 +238,12 @@ def iter_headers(self): yield "sentry-trace", self.to_traceparent() @classmethod - def from_traceparent(cls, traceparent): - # type: (Optional[str]) -> Optional[Span] + def from_traceparent( + cls, # type: Type[SpanTy] + traceparent, # type: Optional[str] + **kwargs # type: Any + ): + # type: (...) -> Optional[SpanTy] if not traceparent: return None @@ -247,7 +266,7 @@ def from_traceparent(cls, traceparent): else: sampled = None - return cls(trace_id=trace_id, parent_span_id=span_id, sampled=sampled) + return cls(trace_id=trace_id, parent_span_id=span_id, sampled=sampled, **kwargs) def to_traceparent(self): # type: () -> str @@ -326,49 +345,7 @@ def finish(self, hub=None): self.timestamp = datetime.utcnow() _maybe_create_breadcrumbs_from_span(hub, self) - - if self._span_recorder is None: - return None - - self._span_recorder.finish_span(self) - - if self.transaction is None: - # If this has no transaction set we assume there's a parent - # transaction for this span that would be flushed out eventually. - return None - - client = hub.client - - if client is None: - # We have no client and therefore nowhere to send this transaction - # event. - return None - - if not self.sampled: - # At this point a `sampled = None` should have already been - # resolved to a concrete decision. If `sampled` is `None`, it's - # likely that somebody used `with sentry_sdk.Hub.start_span(..)` on a - # non-transaction span and later decided to make it a transaction. - if self.sampled is None: - logger.warning("Discarding transaction Span without sampling decision") - - return None - - return hub.capture_event( - { - "type": "transaction", - "transaction": self.transaction, - "contexts": {"trace": self.get_trace_context()}, - "tags": self._tags, - "timestamp": self.timestamp, - "start_timestamp": self.start_timestamp, - "spans": [ - s.to_json(client) - for s in self._span_recorder.finished_spans - if s is not self - ], - } - ) + return None def to_json(self, client): # type: (Optional[sentry_sdk.Client]) -> Dict[str, Any] @@ -383,10 +360,6 @@ def to_json(self, client): "timestamp": self.timestamp, } # type: Dict[str, Any] - transaction = self.transaction - if transaction: - rv["transaction"] = transaction - if self.status: self._tags["status"] = self.status @@ -415,6 +388,58 @@ def get_trace_context(self): return rv +class Transaction(Span): + __slots__ = ("name",) + + def __init__( + self, + name, # type: str + **kwargs # type: Any + ): + # type: (...) -> None + self.name = name + + Span.__init__(self, **kwargs) + + def finish(self, hub=None): + # type: (Optional[sentry_sdk.Hub]) -> Optional[str] + Span.finish(self, hub) + + hub = hub or self.hub or sentry_sdk.Hub.current + client = hub.client + + if client is None: + # We have no client and therefore nowhere to send this transaction + # event. + return None + + if not self.sampled: + # At this point a `sampled = None` should have already been + # resolved to a concrete decision. If `sampled` is `None`, it's + # likely that somebody used `with sentry_sdk.Hub.start_span(..)` on a + # non-transaction span and later decided to make it a transaction. + if self.sampled is None: + logger.warning("Discarding transaction Span without sampling decision") + + return None + + return hub.capture_event( + { + "type": "transaction", + "transaction": self.name, + "contexts": {"trace": self.get_trace_context()}, + "tags": self._tags, + "timestamp": self.timestamp, + "start_timestamp": self.start_timestamp, + "spans": [ + s.to_json(client) + for s in (self._span_recorder and self._span_recorder.spans or ()) + if s is not self and s.timestamp is not None + ], + } + ) + + def _format_sql(cursor, sql): # type: (Any, str) -> Optional[str] diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py index ee6e7c8c60..44821efde4 100644 --- a/tests/integrations/stdlib/test_subprocess.py +++ b/tests/integrations/stdlib/test_subprocess.py @@ -5,7 +5,7 @@ import pytest -from sentry_sdk import Hub, capture_message +from sentry_sdk import capture_message, start_transaction from sentry_sdk._compat import PY2 from sentry_sdk.integrations.stdlib import StdlibIntegration @@ -63,7 +63,7 @@ def test_subprocess_basic( sentry_init(integrations=[StdlibIntegration()], traces_sample_rate=1.0) events = capture_events() - with Hub.current.start_span(transaction="foo", op="foo") as span: + with start_transaction(name="foo") as span: args = [ sys.executable, "-c", diff --git a/tests/test_tracing.py b/tests/test_tracing.py index d68f815bd2..ab45ef15e8 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -3,8 +3,14 @@ import pytest -from sentry_sdk import Hub, capture_message -from sentry_sdk.tracing import Span +from sentry_sdk import ( + Hub, + capture_message, + start_span, + start_transaction, + configure_scope, +) +from sentry_sdk.tracing import Transaction @pytest.mark.parametrize("sample_rate", [0.0, 1.0]) @@ -12,13 +18,13 @@ def test_basic(sentry_init, capture_events, sample_rate): sentry_init(traces_sample_rate=sample_rate) events = capture_events() - with Hub.current.start_span(transaction="hi") as span: + with start_transaction(name="hi") as span: span.set_status("ok") with pytest.raises(ZeroDivisionError): - with Hub.current.start_span(op="foo", description="foodesc"): + with start_span(op="foo", description="foodesc"): 1 / 0 - with Hub.current.start_span(op="bar", description="bardesc"): + with start_span(op="bar", description="bardesc"): pass if sample_rate: @@ -44,8 +50,8 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): sentry_init(traces_sample_rate=1.0, traceparent_v2=True) events = capture_events() - with Hub.current.start_span(transaction="hi"): - with Hub.current.start_span() as old_span: + with start_transaction(name="hi"): + with start_span() as old_span: old_span.sampled = sampled headers = dict(Hub.current.iter_trace_propagation_headers()) @@ -57,8 +63,7 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): if sampled is None: assert header.endswith("-") - span = Span.continue_from_headers(headers) - span.transaction = "WRONG" + span = Transaction.continue_from_headers(headers, name="WRONG") assert span is not None assert span.sampled == sampled assert span.trace_id == old_span.trace_id @@ -66,8 +71,8 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): assert span.parent_span_id == old_span.span_id assert span.span_id != old_span.span_id - with Hub.current.start_span(span): - with Hub.current.configure_scope() as scope: + with start_transaction(span): + with configure_scope() as scope: scope.transaction = "ho" capture_message("hello") @@ -94,13 +99,13 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): def test_sampling_decided_only_for_transactions(sentry_init, capture_events): sentry_init(traces_sample_rate=0.5) - with Hub.current.start_span(transaction="hi") as trace: + with start_transaction(name="hi") as trace: assert trace.sampled is not None - with Hub.current.start_span() as span: + with start_span() as span: assert span.sampled == trace.sampled - with Hub.current.start_span() as span: + with start_span() as span: assert span.sampled is None @@ -113,11 +118,9 @@ def test_memory_usage(sentry_init, capture_events, args, expected_refcount): references = weakref.WeakSet() - with Hub.current.start_span(transaction="hi"): + with start_transaction(name="hi"): for i in range(100): - with Hub.current.start_span( - op="helloworld", description="hi {}".format(i) - ) as span: + with start_span(op="helloworld", description="hi {}".format(i)) as span: def foo(): pass @@ -139,9 +142,9 @@ def test_span_trimming(sentry_init, capture_events): sentry_init(traces_sample_rate=1.0, _experiments={"max_spans": 3}) events = capture_events() - with Hub.current.start_span(transaction="hi"): + with start_transaction(name="hi"): for i in range(10): - with Hub.current.start_span(op="foo{}".format(i)): + with start_span(op="foo{}".format(i)): pass (event,) = events @@ -151,7 +154,31 @@ def test_span_trimming(sentry_init, capture_events): def test_nested_span_sampling_override(): - with Hub.current.start_span(transaction="outer", sampled=True) as span: + with start_transaction(name="outer", sampled=True) as span: + assert span.sampled is True + with start_transaction(name="inner", sampled=False) as span2: + assert span2.sampled is False + assert span.sampled is True - with Hub.current.start_span(transaction="inner", sampled=False) as span: - assert span.sampled is False + + +def test_transaction_method_signature(sentry_init, capture_events): + sentry_init(traces_sample_rate=1.0) + events = capture_events() + + with pytest.raises(TypeError): + start_span(name="foo") + + with pytest.raises(ValueError): + start_transaction() + + with start_transaction(name="a"): + pass + + with start_transaction("b"): + pass + + with start_transaction(Transaction(name="c")): + pass + + assert len(events) == 3 From 275648793d5f34a5ee70ed65210da2534778bf0c Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 8 Jun 2020 20:33:21 +0200 Subject: [PATCH 02/14] Apply suggestions from code review Co-authored-by: Rodolfo Carvalho --- tests/test_tracing.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_tracing.py b/tests/test_tracing.py index ab45ef15e8..1dce1fd8a4 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -153,11 +153,11 @@ def test_span_trimming(sentry_init, capture_events): assert span2["op"] == "foo1" -def test_nested_span_sampling_override(): - with start_transaction(name="outer", sampled=True) as span: - assert span.sampled is True - with start_transaction(name="inner", sampled=False) as span2: - assert span2.sampled is False +def test_nested_transaction_sampling_override(): + with start_transaction(name="outer", sampled=True) as outer_transaction: + assert outer_transaction.sampled is True + with start_transaction(name="inner", sampled=False) as inner_transaction: + assert inner_transaction.sampled is False assert span.sampled is True From bdfc417e6a81ff491973722adcdcf74bbefec4d1 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 10:15:25 +0200 Subject: [PATCH 03/14] misc: Remove TODO Hub.current is never None. Fixing other occurrences is left for a separate commit. --- sentry_sdk/api.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sentry_sdk/api.py b/sentry_sdk/api.py index 7cf690ed69..104b2f75ca 100644 --- a/sentry_sdk/api.py +++ b/sentry_sdk/api.py @@ -264,7 +264,4 @@ def start_transaction( **kwargs # type: Any ): # type: (...) -> Transaction - - # TODO: All other functions in this module check for - # `Hub.current is None`. That actually should never happen? return Hub.current.start_transaction(span_or_name=span_or_name, **kwargs) From 7ce76f0aa9c7c25903ddce6c19707219f01e59e4 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 13:37:06 +0200 Subject: [PATCH 04/14] fixup! Apply suggestions from code review --- tests/test_tracing.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_tracing.py b/tests/test_tracing.py index 1dce1fd8a4..c857d54b48 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -158,8 +158,7 @@ def test_nested_transaction_sampling_override(): assert outer_transaction.sampled is True with start_transaction(name="inner", sampled=False) as inner_transaction: assert inner_transaction.sampled is False - - assert span.sampled is True + assert outer_transaction.sampled is True def test_transaction_method_signature(sentry_init, capture_events): From dc8ffc307dfb07449caaee9fa41884a63726d05f Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 15:20:00 +0200 Subject: [PATCH 05/14] ref: Rename span -> transaction --- sentry_sdk/integrations/aiohttp.py | 10 +++++----- sentry_sdk/integrations/asgi.py | 8 ++++---- sentry_sdk/integrations/celery.py | 8 ++++---- sentry_sdk/integrations/rq.py | 6 +++--- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index c5aee56d62..61973ee9b6 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -87,7 +87,7 @@ async def sentry_app_handle(self, request, *args, **kwargs): scope.clear_breadcrumbs() scope.add_event_processor(_make_request_processor(weak_request)) - span = Transaction.continue_from_headers( + transaction = Transaction.continue_from_headers( request.headers, op="http.server", # If this transaction name makes it to the UI, AIOHTTP's @@ -95,21 +95,21 @@ async def sentry_app_handle(self, request, *args, **kwargs): name="generic AIOHTTP request", ) - with hub.start_transaction(span): + with hub.start_transaction(transaction): try: response = await old_handle(self, request) except HTTPException as e: - span.set_http_status(e.status_code) + transaction.set_http_status(e.status_code) raise except asyncio.CancelledError: - span.set_status("cancelled") + transaction.set_status("cancelled") raise except Exception: # This will probably map to a 500 but seems like we # have no way to tell. Do not set span status. reraise(*_capture_exception(hub)) - span.set_http_status(response.status) + transaction.set_http_status(response.status) return response Application._handle = sentry_app_handle diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index 07b9575741..58cf10c487 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -123,17 +123,17 @@ async def _run_app(self, scope, callback): ty = scope["type"] if ty in ("http", "websocket"): - span = Transaction.continue_from_headers( + transaction = Transaction.continue_from_headers( dict(scope["headers"]), name=_DEFAULT_TRANSACTION_NAME, op="{}.server".format(ty), ) else: - span = Transaction(name=_DEFAULT_TRANSACTION_NAME, op="asgi.server") + transaction = Transaction(name=_DEFAULT_TRANSACTION_NAME, op="asgi.server") - span.set_tag("asgi.type", ty) + transaction.set_tag("asgi.type", ty) - with hub.start_transaction(span): + with hub.start_transaction(transaction): # XXX: Would be cool to have correct span status, but we # would have to wrap send(). That is a bit hard to do with # the current abstraction over ASGI 2/3. diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py index 5f70657f2f..86714e2111 100644 --- a/sentry_sdk/integrations/celery.py +++ b/sentry_sdk/integrations/celery.py @@ -130,21 +130,21 @@ def _inner(*args, **kwargs): scope.clear_breadcrumbs() scope.add_event_processor(_make_event_processor(task, *args, **kwargs)) - span = Transaction.continue_from_headers( + transaction = Transaction.continue_from_headers( args[3].get("headers") or {}, op="celery.task", name="unknown celery task", ) # Could possibly use a better hook than this one - span.set_status("ok") + transaction.set_status("ok") with capture_internal_exceptions(): # Celery task objects are not a thing to be trusted. Even # something such as attribute access can fail. - span.name = task.name + transaction.name = task.name - with hub.start_transaction(span): + with hub.start_transaction(transaction): return f(*args, **kwargs) return _inner # type: ignore diff --git a/sentry_sdk/integrations/rq.py b/sentry_sdk/integrations/rq.py index d950f7fbf7..1e51ec50cf 100644 --- a/sentry_sdk/integrations/rq.py +++ b/sentry_sdk/integrations/rq.py @@ -61,16 +61,16 @@ def sentry_patched_perform_job(self, job, *args, **kwargs): scope.clear_breadcrumbs() scope.add_event_processor(_make_event_processor(weakref.ref(job))) - span = Transaction.continue_from_headers( + transaction = Transaction.continue_from_headers( job.meta.get("_sentry_trace_headers") or {}, op="rq.task", name="unknown RQ task", ) with capture_internal_exceptions(): - span.name = job.func_name + transaction.name = job.func_name - with hub.start_transaction(span): + with hub.start_transaction(transaction): rv = old_perform_job(self, job, *args, **kwargs) if self.is_horse: From 80275ef0531d398fec2fc2cc6276a48f6ea19607 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 15:22:27 +0200 Subject: [PATCH 06/14] fixup! ref: Rename span -> transaction --- sentry_sdk/integrations/wsgi.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index f21e88db2e..ec4ea43d8a 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -113,15 +113,15 @@ def __call__(self, environ, start_response): _make_wsgi_event_processor(environ) ) - span = Transaction.continue_from_environ( + transaction = Transaction.continue_from_environ( environ, op="http.server", name="generic WSGI request" ) - with hub.start_transaction(span) as span: + with hub.start_transaction(transaction) as transaction: try: rv = self.app( environ, - partial(_sentry_start_response, start_response, span), + partial(_sentry_start_response, start_response, transaction), ) except BaseException: reraise(*_capture_exception(hub)) From 63a46818bd7097ca7ba84b6109f35275eb92a4fe Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 16:37:02 +0200 Subject: [PATCH 07/14] test: Nested spans in scope --- tests/test_tracing.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_tracing.py b/tests/test_tracing.py index c857d54b48..fde54ea6ff 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -181,3 +181,25 @@ def test_transaction_method_signature(sentry_init, capture_events): pass assert len(events) == 3 + + +def test_nested_spans_in_scope(sentry_init, capture_events): + sentry_init(traces_sample_rate=1.0) + events = capture_events() + + # XXX: nesting of spans may not work when async code is involved (data race + # accessing scope._span) + with start_transaction(name="/status/") as transaction: + assert Hub.current.scope._span is transaction + with start_span(op="check1", description="desc1") as span1: + assert Hub.current.scope._span is span1 + with start_span(op="check1_1", description="desc1_1") as span1_1: + assert Hub.current.scope._span is span1_1 + assert Hub.current.scope._span is span1 + with start_span(op="check2", description="desc2") as span2: + assert Hub.current.scope._span is span2 + assert Hub.current.scope._span is transaction + transaction.set_status("ok") + + assert len(events) == 1 + assert len(events[0]["spans"]) == 3 From e8349eabe8100fea535a86e930290ebdea470e01 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 16:45:38 +0200 Subject: [PATCH 08/14] fixup! ref: Rename span -> transaction --- tests/integrations/stdlib/test_subprocess.py | 4 ++-- tests/test_tracing.py | 22 ++++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py index 44821efde4..4dc2137255 100644 --- a/tests/integrations/stdlib/test_subprocess.py +++ b/tests/integrations/stdlib/test_subprocess.py @@ -63,7 +63,7 @@ def test_subprocess_basic( sentry_init(integrations=[StdlibIntegration()], traces_sample_rate=1.0) events = capture_events() - with start_transaction(name="foo") as span: + with start_transaction(name="foo") as transaction: args = [ sys.executable, "-c", @@ -114,7 +114,7 @@ def test_subprocess_basic( assert os.environ == old_environ - assert span.trace_id in str(output) + assert transaction.trace_id in str(output) capture_message("hi") diff --git a/tests/test_tracing.py b/tests/test_tracing.py index fde54ea6ff..0367860117 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -18,8 +18,8 @@ def test_basic(sentry_init, capture_events, sample_rate): sentry_init(traces_sample_rate=sample_rate) events = capture_events() - with start_transaction(name="hi") as span: - span.set_status("ok") + with start_transaction(name="hi") as transaction: + transaction.set_status("ok") with pytest.raises(ZeroDivisionError): with start_span(op="foo", description="foodesc"): 1 / 0 @@ -63,15 +63,15 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): if sampled is None: assert header.endswith("-") - span = Transaction.continue_from_headers(headers, name="WRONG") - assert span is not None - assert span.sampled == sampled - assert span.trace_id == old_span.trace_id - assert span.same_process_as_parent is False - assert span.parent_span_id == old_span.span_id - assert span.span_id != old_span.span_id + transaction = Transaction.continue_from_headers(headers, name="WRONG") + assert transaction is not None + assert transaction.sampled == sampled + assert transaction.trace_id == old_span.trace_id + assert transaction.same_process_as_parent is False + assert transaction.parent_span_id == old_span.span_id + assert transaction.span_id != old_span.span_id - with start_transaction(span): + with start_transaction(transaction): with configure_scope() as scope: scope.transaction = "ho" capture_message("hello") @@ -89,7 +89,7 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): assert ( trace1["contexts"]["trace"]["trace_id"] == trace2["contexts"]["trace"]["trace_id"] - == span.trace_id + == transaction.trace_id == message["contexts"]["trace"]["trace_id"] ) From 5036f285196713e1493d4e83272c38a714edc2cc Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 23:50:51 +0200 Subject: [PATCH 09/14] fixup! ref: Rename span -> transaction --- sentry_sdk/integrations/asgi.py | 4 +++- sentry_sdk/integrations/wsgi.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/asgi.py b/sentry_sdk/integrations/asgi.py index 58cf10c487..3db4fa1dfc 100644 --- a/sentry_sdk/integrations/asgi.py +++ b/sentry_sdk/integrations/asgi.py @@ -129,7 +129,9 @@ async def _run_app(self, scope, callback): op="{}.server".format(ty), ) else: - transaction = Transaction(name=_DEFAULT_TRANSACTION_NAME, op="asgi.server") + transaction = Transaction( + name=_DEFAULT_TRANSACTION_NAME, op="asgi.server" + ) transaction.set_tag("asgi.type", ty) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index ec4ea43d8a..505cdd6bea 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -121,7 +121,9 @@ def __call__(self, environ, start_response): try: rv = self.app( environ, - partial(_sentry_start_response, start_response, transaction), + partial( + _sentry_start_response, start_response, transaction + ), ) except BaseException: reraise(*_capture_exception(hub)) From 429c84214f999aaca8039d4f84fa0df2a53f8698 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 16:59:46 +0200 Subject: [PATCH 10/14] ref: Rename trace -> transaction --- tests/test_tracing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_tracing.py b/tests/test_tracing.py index 0367860117..9be69d47c4 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -99,11 +99,11 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): def test_sampling_decided_only_for_transactions(sentry_init, capture_events): sentry_init(traces_sample_rate=0.5) - with start_transaction(name="hi") as trace: - assert trace.sampled is not None + with start_transaction(name="hi") as transaction: + assert transaction.sampled is not None with start_span() as span: - assert span.sampled == trace.sampled + assert span.sampled == transaction.sampled with start_span() as span: assert span.sampled is None From af2b1e391b81aff8c20e0c2dfa95dd301d177a63 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 17:14:28 +0200 Subject: [PATCH 11/14] fix: Remove span from start_transaction kwargs Not possible to start a transaction from a span, let's not confuse the API more. --- sentry_sdk/hub.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 2ca37b7701..f8a62fe10f 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -492,9 +492,6 @@ def start_transaction( elif isinstance(span_or_name, Transaction): transaction = span_or_name - elif span_or_name is None and "span" in kwargs: - transaction = kwargs.pop("span") - else: raise ValueError("transaction object or name required.") From 986c9f12698084771f6bb25e3a137a118f2738f8 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 23:05:28 +0200 Subject: [PATCH 12/14] fix: Remove ambiguous positional argument In favor of replicating start_span's signature. Example usages: Hub.start_transaction(transaction) or Hub.start_transaction(name="...", ...) --- sentry_sdk/api.py | 4 ++-- sentry_sdk/hub.py | 52 ++++++++++++++++++++++++++----------------- tests/test_tracing.py | 5 +---- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/sentry_sdk/api.py b/sentry_sdk/api.py index 699cc01426..9e12a2c94c 100644 --- a/sentry_sdk/api.py +++ b/sentry_sdk/api.py @@ -206,8 +206,8 @@ def start_span( @hubmethod def start_transaction( - span_or_name=None, # type: Optional[Union[Span, str]] + transaction=None, # type: Optional[Transaction] **kwargs # type: Any ): # type: (...) -> Transaction - return Hub.current.start_transaction(span_or_name=span_or_name, **kwargs) + return Hub.current.start_transaction(transaction, **kwargs) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index f8a62fe10f..e20cb810f6 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -468,33 +468,45 @@ def start_span( def start_transaction( self, - span_or_name=None, # type: Optional[Union[Span, str]] + transaction=None, # type: Optional[Transaction] **kwargs # type: Any ): # type: (...) -> Transaction """ - Create a new transaction detached from the current span. The return - value is the `Transaction` object which for the most part works like a - span. - """ - - kwargs.setdefault("hub", self) - - if isinstance(span_or_name, str): - if "name" in kwargs: - raise ValueError("Cannot specify transaction name twice.") - kwargs["name"] = span_or_name - transaction = Transaction(**kwargs) + Start and return a transaction. - elif span_or_name is None and "name" in kwargs: + Start an existing transaction if given, otherwise create and start a new + transaction with kwargs. + """ + # XXX: should we always set transaction.hub = self? + # In a multi-hub program, what does it mean to write + # hub1.start_transaction(Transaction(hub=hub2)) + # ? Should the transaction be on hub1 or hub2? + + # XXX: it is strange that both start_span and start_transaction take + # kwargs, but those are ignored if span/transaction are not None. + # Code such as: + # hub.start_transaction(Transaction(name="foo"), name="bar") + # + # is clearly weird, but not so weird if we intentionally want to rename + # a transaction we got from somewhere else: + # hub.start_transaction(transaction, name="new_name") + # + # Would be clearer if Hub was not involved: + # transaction.name = "new_name" + # with transaction: # __enter__ => start, __exit__ => finish + # with transaction.start_child(...): + # pass + # # alternatively, rely on transaction being in the current scope + # with Span(...): + # pass + + if transaction is None: + if "name" not in kwargs: + raise ValueError("missing name") + kwargs.setdefault("hub", self) transaction = Transaction(**kwargs) - elif isinstance(span_or_name, Transaction): - transaction = span_or_name - - else: - raise ValueError("transaction object or name required.") - client, scope = self._stack[-1] if transaction.sampled is None: diff --git a/tests/test_tracing.py b/tests/test_tracing.py index 9be69d47c4..c470cab39c 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -174,13 +174,10 @@ def test_transaction_method_signature(sentry_init, capture_events): with start_transaction(name="a"): pass - with start_transaction("b"): - pass - with start_transaction(Transaction(name="c")): pass - assert len(events) == 3 + assert len(events) == 2 def test_nested_spans_in_scope(sentry_init, capture_events): From a32e2d083e083b1ec61fa56c734f1616bd659d82 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 22 Jun 2020 23:42:42 +0200 Subject: [PATCH 13/14] feat: Allow transactions with no name It is okay to start a transaction without a name and set it later. Sometimes the proper name is not known until after the transaction has started. We could fail the transaction if it has no name when calling the finish method. Instead, set a default name that will prompt users to give a name to their transactions. This is the same behavior as implemented in JS. --- sentry_sdk/hub.py | 2 -- sentry_sdk/tracing.py | 8 +++++++- tests/test_tracing.py | 10 +++++++--- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index e20cb810f6..8bd9e14c77 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -502,8 +502,6 @@ def start_transaction( # pass if transaction is None: - if "name" not in kwargs: - raise ValueError("missing name") kwargs.setdefault("hub", self) transaction = Transaction(**kwargs) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 021444bc6b..2cb6e1e0ab 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -393,7 +393,7 @@ class Transaction(Span): def __init__( self, - name, # type: str + name="", # type: str **kwargs # type: Any ): # type: (...) -> None @@ -403,6 +403,12 @@ def __init__( def finish(self, hub=None): # type: (Optional[sentry_sdk.Hub]) -> Optional[str] + if not self.name: + logger.warning( + "Transaction has no name, falling back to ``." + ) + self.name = "" + Span.finish(self, hub) hub = hub or self.hub or sentry_sdk.Hub.current diff --git a/tests/test_tracing.py b/tests/test_tracing.py index c470cab39c..f009c044ec 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -168,8 +168,12 @@ def test_transaction_method_signature(sentry_init, capture_events): with pytest.raises(TypeError): start_span(name="foo") - with pytest.raises(ValueError): - start_transaction() + with start_transaction() as transaction: + pass + assert transaction.name == "" + + with start_transaction() as transaction: + transaction.name = "name-known-after-transaction-started" with start_transaction(name="a"): pass @@ -177,7 +181,7 @@ def test_transaction_method_signature(sentry_init, capture_events): with start_transaction(Transaction(name="c")): pass - assert len(events) == 2 + assert len(events) == 4 def test_nested_spans_in_scope(sentry_init, capture_events): From a845863d4cadae87bbbca554dd695d6ff51b9b5f Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Tue, 23 Jun 2020 00:02:05 +0200 Subject: [PATCH 14/14] test: Assert each block creates new event Asserting that each start_transaction context contributes with one and only one new event makes the overall test more robust -- though far from perfect, e.g. we make no assertions about the event itself. --- tests/test_tracing.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_tracing.py b/tests/test_tracing.py index f009c044ec..9a3d41460e 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -167,20 +167,23 @@ def test_transaction_method_signature(sentry_init, capture_events): with pytest.raises(TypeError): start_span(name="foo") + assert len(events) == 0 with start_transaction() as transaction: pass assert transaction.name == "" + assert len(events) == 1 with start_transaction() as transaction: transaction.name = "name-known-after-transaction-started" + assert len(events) == 2 with start_transaction(name="a"): pass + assert len(events) == 3 with start_transaction(Transaction(name="c")): pass - assert len(events) == 4