From 9389f49bd53ac65e5445d3a34d3ddb29c6e08c75 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Fri, 26 Jun 2020 18:09:31 +0200 Subject: [PATCH] feat: Introduce Transaction and Hub.start_transaction This aligns the tracing implementation with the current JS tracing implementation, up to a certain extent. Hub.start_transaction or start_transaction are meant to be used when starting transactions, replacing most uses of Hub.start_span / start_span. Spans are typically created from their parent transactions via transaction.start_child, or start_span relying on the transaction being in the current scope. 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. Span.continue_from_headers, Span.continue_from_environ, Span.from_traceparent and the equivalent methods on Transaction always return a Transaction and take kwargs to set attributes on the new Transaction. Rename Span.new_span to Span.start_child (and Transaction.start_child)a, aligning with JS / tracing API spec. The old name is kept for backwards compatibility. Co-authored-by: Markus Unterwaditzer --- sentry_sdk/api.py | 12 +- sentry_sdk/hub.py | 92 +++++-- sentry_sdk/integrations/aiohttp.py | 22 +- sentry_sdk/integrations/asgi.py | 16 +- sentry_sdk/integrations/celery.py | 16 +- sentry_sdk/integrations/rq.py | 13 +- sentry_sdk/integrations/wsgi.py | 18 +- sentry_sdk/scope.py | 20 +- sentry_sdk/tracing.py | 243 ++++++++++++------ tests/integrations/celery/test_celery.py | 22 +- .../sqlalchemy/test_sqlalchemy.py | 5 +- tests/integrations/stdlib/test_subprocess.py | 6 +- tests/test_tracing.py | 127 ++++++--- 13 files changed, 408 insertions(+), 204 deletions(-) diff --git a/sentry_sdk/api.py b/sentry_sdk/api.py index fc2b305716..9e12a2c94c 100644 --- a/sentry_sdk/api.py +++ b/sentry_sdk/api.py @@ -16,7 +16,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]) @@ -37,6 +37,7 @@ def overload(x): "flush", "last_event_id", "start_span", + "start_transaction", "set_tag", "set_context", "set_extra", @@ -201,3 +202,12 @@ def start_span( ): # type: (...) -> Span return Hub.current.start_span(span=span, **kwargs) + + +@hubmethod +def start_transaction( + transaction=None, # type: Optional[Transaction] + **kwargs # type: Any +): + # type: (...) -> Transaction + return Hub.current.start_transaction(transaction, **kwargs) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 6e77c93937..c8570c16a8 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, @@ -441,38 +441,88 @@ def start_span( ): # type: (...) -> Span """ - Create a new span whose parent span is the currently active - 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. + Create and start timing a new span whose parent is the currently active + span or transaction, if any. The return value is a span instance, + typically used as a context manager to start and stop timing in a `with` + block. + + Only spans contained in a transaction are sent to Sentry. Most + integrations start a transaction at the appropriate time, for example + for every incoming HTTP request. Use `start_transaction` to start a new + transaction when one is not already in progress. """ + # TODO: consider removing this in a future release. + # This is for backwards compatibility with releases before + # start_transaction existed, to allow for a smoother transition. + if isinstance(span, Transaction) or "transaction" in kwargs: + deprecation_msg = ( + "Deprecated: use start_transaction to start transactions and " + "Transaction.start_child to start spans." + ) + if isinstance(span, Transaction): + logger.warning(deprecation_msg) + return self.start_transaction(span) + if "transaction" in kwargs: + logger.warning(deprecation_msg) + name = kwargs.pop("transaction") + return self.start_transaction(name=name, **kwargs) - client, scope = self._stack[-1] + if span is not None: + return span kwargs.setdefault("hub", self) - if span is None: - span = scope.span - if span is not None: - span = span.new_span(**kwargs) - else: - span = Span(**kwargs) + span = self.scope.span + if span is not None: + return span.start_child(**kwargs) + + return Span(**kwargs) + + def start_transaction( + self, + transaction=None, # type: Optional[Transaction] + **kwargs # type: Any + ): + # type: (...) -> Transaction + """ + Start and return a transaction. + + Start an existing transaction if given, otherwise create and start a new + transaction with kwargs. + + This is the entry point to manual tracing instrumentation. + + A tree structure can be built by adding child spans to the transaction, + and child spans to other spans. To start a new child span within the + transaction or any span, call the respective `.start_child()` method. + + Every child span must be finished before the transaction is finished, + otherwise the unfinished spans are discarded. + + When used as context managers, spans and transactions are automatically + finished at the end of the `with` block. If not using context managers, + call the `.finish()` method. + + When the transaction is finished, it will be sent to Sentry with all its + finished child spans. + """ + if transaction is None: + kwargs.setdefault("hub", self) + transaction = Transaction(**kwargs) + + client, scope = self._stack[-1] - if span.sampled is None and span.transaction is not None: + 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 8bbb1670ee..61973ee9b6 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, @@ -87,27 +87,29 @@ 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" + transaction = 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(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 202c49025a..4b3e3fda07 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,16 @@ 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) + transaction = Transaction.continue_from_headers( + dict(scope["headers"]), op="{}.server".format(ty), + ) else: - span = Span() - span.op = "asgi.server" + transaction = Transaction(op="asgi.server") - span.set_tag("asgi.type", ty) - span.transaction = _DEFAULT_TRANSACTION_NAME + transaction.name = _DEFAULT_TRANSACTION_NAME + transaction.set_tag("asgi.type", ty) - with hub.start_span(span) as 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 5ac0d32f40..86714e2111 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,19 +130,21 @@ 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" + 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.transaction = task.name + transaction.name = task.name - with hub.start_span(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 fbe8cdda3d..1e51ec50cf 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 {} + transaction = 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 + transaction.name = job.func_name - with hub.start_span(span): + with hub.start_transaction(transaction): 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 2ac9f2f191..ee359c7925 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,15 +113,17 @@ 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" + transaction = Transaction.continue_from_environ( + environ, op="http.server", name="generic WSGI request" + ) - with hub.start_span(span) as span: + with hub.start_transaction(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)) @@ -133,7 +135,7 @@ def __call__(self, environ, start_response): def _sentry_start_response( old_start_response, # type: StartResponse - span, # type: Span + transaction, # type: Transaction status, # type: str response_headers, # type: WsgiResponseHeaders exc_info=None, # type: Optional[WsgiExcInfo] @@ -141,7 +143,7 @@ def _sentry_start_response( # type: (...) -> WsgiResponseIter with capture_internal_exceptions(): status_int = int(status.split(" ", 1)[0]) - span.set_http_status(status_int) + transaction.set_http_status(status_int) if exc_info is None: # The Django Rest Framework WSGI test client, and likely other diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index e5478cebc9..f928063920 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 @@ -137,8 +138,7 @@ def fingerprint(self, value): @property def transaction(self): # type: () -> Any - # would be type: () -> Optional[Span], see https://github.com/python/mypy/issues/3004 - # XXX: update return type to Optional[Transaction] + # would be type: () -> Optional[Transaction], see https://github.com/python/mypy/issues/3004 """Return the transaction (root span) in the scope.""" if self._span is None or self._span._span_recorder is None: return None @@ -163,8 +163,8 @@ def transaction(self, value): # the value argument. self._transaction = value span = self._span - if span: - span.transaction = value + if span and isinstance(span, Transaction): + span.name = value @_attr_setter def user(self, value): @@ -182,17 +182,19 @@ def set_user(self, value): @property def span(self): # type: () -> Optional[Span] - """Get/set current tracing span.""" + """Get/set current tracing span or transaction.""" return self._span @span.setter def span(self, span): # type: (Optional[Span]) -> None self._span = span - if span is not None: - span_transaction = span.transaction - if span_transaction: - self._transaction = span_transaction + # XXX: this differs from the implementation in JS, there Scope.setSpan + # does not set Scope._transactionName. + if isinstance(span, Transaction): + transaction = span + if transaction.name: + self._transaction = transaction.name def set_tag( self, diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 5e9ae8a0e0..ad409f1b91 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -96,7 +96,6 @@ class Span(object): "parent_span_id", "same_process_as_parent", "sampled", - "transaction", "op", "description", "start_timestamp", @@ -110,6 +109,15 @@ class Span(object): "_context_manager_state", ) + def __new__(cls, **kwargs): + # type: (**Any) -> Any + # TODO: consider removing this in a future release. + # This is for backwards compatibility with releases before Transaction + # existed, to allow for a smoother transition. + if "transaction" in kwargs: + return object.__new__(Transaction) + return object.__new__(cls) + def __init__( self, trace_id=None, # type: Optional[str] @@ -117,11 +125,11 @@ 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] status=None, # type: Optional[str] + transaction=None, # type: Optional[str] # deprecated ): # type: (...) -> None self.trace_id = trace_id or uuid.uuid4().hex @@ -129,7 +137,6 @@ def __init__( 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.description = description self.status = status @@ -151,7 +158,7 @@ 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) @@ -159,16 +166,12 @@ def init_finished_spans(self, maxlen): def __repr__(self): # type: () -> str - return ( - "<%s(transaction=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" - % ( - self.__class__.__name__, - self.transaction, - self.trace_id, - self.span_id, - self.parent_span_id, - self.sampled, - ) + return "<%s(trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( + self.__class__.__name__, + self.trace_id, + self.span_id, + self.parent_span_id, + self.sampled, ) def __enter__(self): @@ -192,27 +195,60 @@ 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 + def new_span(self, **kwargs): + # type: (**Any) -> Span + """Deprecated: use start_child instead.""" + logger.warning("Deprecated: use Span.start_child instead of Span.new_span.") + return self.start_child(**kwargs) + @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, + environ, # type: typing.Mapping[str, str] + **kwargs # type: Any + ): + # type: (...) -> Transaction + if cls is Span: + logger.warning( + "Deprecated: use Transaction.continue_from_environ " + "instead of Span.continue_from_environ." + ) + return Transaction.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, + headers, # type: typing.Mapping[str, str] + **kwargs # type: Any + ): + # type: (...) -> Transaction + if cls is Span: + logger.warning( + "Deprecated: use Transaction.continue_from_headers " + "instead of Span.continue_from_headers." + ) + parent = Transaction.from_traceparent(headers.get("sentry-trace"), **kwargs) if parent is None: - return cls() + parent = Transaction(**kwargs) parent.same_process_as_parent = False return parent @@ -221,8 +257,18 @@ 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, + traceparent, # type: Optional[str] + **kwargs # type: Any + ): + # type: (...) -> Optional[Transaction] + if cls is Span: + logger.warning( + "Deprecated: use Transaction.from_traceparent " + "instead of Span.from_traceparent." + ) + if not traceparent: return None @@ -245,7 +291,9 @@ def from_traceparent(cls, traceparent): else: sampled = None - return cls(trace_id=trace_id, parent_span_id=span_id, sampled=sampled) + return Transaction( + trace_id=trace_id, parent_span_id=span_id, sampled=sampled, **kwargs + ) def to_traceparent(self): # type: () -> str @@ -311,12 +359,14 @@ def is_success(self): def finish(self, hub=None): # type: (Optional[sentry_sdk.Hub]) -> Optional[str] - hub = hub or self.hub or sentry_sdk.Hub.current - + # XXX: would be type: (Optional[sentry_sdk.Hub]) -> None, but that leads + # to incompatible return types for Span.finish and Transaction.finish. if self.timestamp is not None: - # This transaction is already finished, so we should not flush it again. + # This span is already finished, ignore. return None + hub = hub or self.hub or sentry_sdk.Hub.current + try: duration_seconds = time.perf_counter() - self._start_timestamp_monotonic self.timestamp = self.start_timestamp + timedelta(seconds=duration_seconds) @@ -324,49 +374,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 - - 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 - - finished_spans = [ - span.to_json(client) - for span in self._span_recorder.spans - if span is not self and span.timestamp is not 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": finished_spans, - } - ) + return None def to_json(self, client): # type: (Optional[sentry_sdk.Client]) -> Dict[str, Any] @@ -381,10 +389,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 @@ -413,6 +417,91 @@ def get_trace_context(self): return rv +class Transaction(Span): + __slots__ = ("name",) + + def __init__( + self, + name="", # type: str + **kwargs # type: Any + ): + # type: (...) -> None + # TODO: consider removing this in a future release. + # This is for backwards compatibility with releases before Transaction + # existed, to allow for a smoother transition. + if not name and "transaction" in kwargs: + logger.warning( + "Deprecated: use Transaction(name=...) to create transactions " + "instead of Span(transaction=...)." + ) + name = kwargs.pop("transaction") + Span.__init__(self, **kwargs) + self.name = name + + def __repr__(self): + # type: () -> str + return ( + "<%s(name=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" + % ( + self.__class__.__name__, + self.name, + self.trace_id, + self.span_id, + self.parent_span_id, + self.sampled, + ) + ) + + def finish(self, hub=None): + # type: (Optional[sentry_sdk.Hub]) -> Optional[str] + if self.timestamp is not None: + # This transaction is already finished, ignore. + return None + + if self._span_recorder is None: + return None + + 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. + return None + + if not self.name: + logger.warning( + "Transaction has no name, falling back to ``." + ) + self.name = "" + + Span.finish(self, hub) + + if not self.sampled: + # At this point a `sampled = None` should have already been resolved + # to a concrete decision. + if self.sampled is None: + logger.warning("Discarding transaction without sampling decision.") + return None + + finished_spans = [ + span.to_json(client) + for span in self._span_recorder.spans + if span is not self and span.timestamp is not 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": finished_spans, + } + ) + + def _format_sql(cursor, sql): # type: (Any, str) -> Optional[str] diff --git a/tests/integrations/celery/test_celery.py b/tests/integrations/celery/test_celery.py index 3a4ad9895e..ed06e8f2b0 100644 --- a/tests/integrations/celery/test_celery.py +++ b/tests/integrations/celery/test_celery.py @@ -4,7 +4,7 @@ pytest.importorskip("celery") -from sentry_sdk import Hub, configure_scope +from sentry_sdk import Hub, configure_scope, start_transaction from sentry_sdk.integrations.celery import CeleryIntegration from sentry_sdk._compat import text_type @@ -74,14 +74,14 @@ def dummy_task(x, y): foo = 42 # noqa return x / y - with Hub.current.start_span() as span: + with start_transaction() as transaction: celery_invocation(dummy_task, 1, 2) _, expected_context = celery_invocation(dummy_task, 1, 0) (event,) = events - assert event["contexts"]["trace"]["trace_id"] == span.trace_id - assert event["contexts"]["trace"]["span_id"] != span.span_id + assert event["contexts"]["trace"]["trace_id"] == transaction.trace_id + assert event["contexts"]["trace"]["span_id"] != transaction.span_id assert event["transaction"] == "dummy_task" assert "celery_task_id" in event["tags"] assert event["extra"]["celery-job"] == dict( @@ -107,12 +107,12 @@ def dummy_task(x, y): events = capture_events() - with Hub.current.start_span(transaction="submission") as span: + with start_transaction(name="submission") as transaction: celery_invocation(dummy_task, 1, 0 if task_fails else 1) if task_fails: error_event = events.pop(0) - assert error_event["contexts"]["trace"]["trace_id"] == span.trace_id + assert error_event["contexts"]["trace"]["trace_id"] == transaction.trace_id assert error_event["exception"]["values"][0]["type"] == "ZeroDivisionError" execution_event, submission_event = events @@ -121,8 +121,8 @@ def dummy_task(x, y): assert submission_event["transaction"] == "submission" assert execution_event["type"] == submission_event["type"] == "transaction" - assert execution_event["contexts"]["trace"]["trace_id"] == span.trace_id - assert submission_event["contexts"]["trace"]["trace_id"] == span.trace_id + assert execution_event["contexts"]["trace"]["trace_id"] == transaction.trace_id + assert submission_event["contexts"]["trace"]["trace_id"] == transaction.trace_id if task_fails: assert execution_event["contexts"]["trace"]["status"] == "internal_error" @@ -139,7 +139,7 @@ def dummy_task(x, y): u"span_id": submission_event["spans"][0]["span_id"], u"start_timestamp": submission_event["spans"][0]["start_timestamp"], u"timestamp": submission_event["spans"][0]["timestamp"], - u"trace_id": text_type(span.trace_id), + u"trace_id": text_type(transaction.trace_id), } ] @@ -177,11 +177,11 @@ def test_simple_no_propagation(capture_events, init_celery): def dummy_task(): 1 / 0 - with Hub.current.start_span() as span: + with start_transaction() as transaction: dummy_task.delay() (event,) = events - assert event["contexts"]["trace"]["trace_id"] != span.trace_id + assert event["contexts"]["trace"]["trace_id"] != transaction.trace_id assert event["transaction"] == "dummy_task" (exception,) = event["exception"]["values"] assert exception["type"] == "ZeroDivisionError" diff --git a/tests/integrations/sqlalchemy/test_sqlalchemy.py b/tests/integrations/sqlalchemy/test_sqlalchemy.py index 3ef1b272de..5721f3f358 100644 --- a/tests/integrations/sqlalchemy/test_sqlalchemy.py +++ b/tests/integrations/sqlalchemy/test_sqlalchemy.py @@ -6,8 +6,7 @@ from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import relationship, sessionmaker -import sentry_sdk -from sentry_sdk import capture_message +from sentry_sdk import capture_message, start_transaction from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration @@ -101,7 +100,7 @@ class Address(Base): Session = sessionmaker(bind=engine) # noqa: N806 session = Session() - with sentry_sdk.start_span(transaction="test_transaction", sampled=True): + with start_transaction(name="test_transaction", sampled=True): with session.begin_nested(): session.query(Person).first() diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py index e2ae005d2a..4416e28b94 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 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 d49eeaf826..a46dd4359b 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -3,8 +3,14 @@ import pytest -from sentry_sdk import Hub, capture_message, start_span -from sentry_sdk.tracing import Span +from sentry_sdk import ( + capture_message, + configure_scope, + Hub, + start_span, + start_transaction, +) +from sentry_sdk.tracing import Span, 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: - span.set_status("ok") + with start_transaction(name="hi") as transaction: + transaction.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: @@ -40,13 +46,30 @@ def test_basic(sentry_init, capture_events, sample_rate): assert not events +def test_start_span_to_start_transaction(sentry_init, capture_events): + # XXX: this only exists for backwards compatibility with code before + # Transaction / start_transaction were introduced. + sentry_init(traces_sample_rate=1.0) + events = capture_events() + + with start_span(transaction="/1/"): + pass + + with start_span(Span(transaction="/2/")): + pass + + assert len(events) == 2 + assert events[0]["transaction"] == "/1/" + assert events[1]["transaction"] == "/2/" + + @pytest.mark.parametrize("sampled", [True, False, None]) 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()) @@ -58,17 +81,16 @@ 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" - 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 - - with Hub.current.start_span(span): - with Hub.current.configure_scope() as scope: + 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(transaction): + with configure_scope() as scope: scope.transaction = "ho" capture_message("hello") @@ -85,7 +107,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"] ) @@ -95,13 +117,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: - assert trace.sampled is not None + with start_transaction(name="hi") as transaction: + assert transaction.sampled is not None - with Hub.current.start_span() as span: - assert span.sampled == trace.sampled + with start_span() as span: + assert span.sampled == transaction.sampled - with Hub.current.start_span() as span: + with start_span() as span: assert span.sampled is None @@ -114,11 +136,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 @@ -140,9 +160,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,11 +171,38 @@ def test_span_trimming(sentry_init, capture_events): assert span2["op"] == "foo1" -def test_nested_span_sampling_override(): - with Hub.current.start_span(transaction="outer", sampled=True) as span: - assert span.sampled is True - with Hub.current.start_span(transaction="inner", sampled=False) as span: - assert span.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 outer_transaction.sampled is True + + +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") + 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 def test_no_double_sampling(sentry_init, capture_events): @@ -164,7 +211,7 @@ def test_no_double_sampling(sentry_init, capture_events): sentry_init(traces_sample_rate=1.0, sample_rate=0.0) events = capture_events() - with Hub.current.start_span(transaction="/"): + with start_transaction(name="/"): pass assert len(events) == 1 @@ -177,7 +224,7 @@ def before_send(event, hint): sentry_init(traces_sample_rate=1.0, before_send=before_send) events = capture_events() - with Hub.current.start_span(transaction="/"): + with start_transaction(name="/"): pass assert len(events) == 1 @@ -187,11 +234,11 @@ def test_get_transaction_from_scope(sentry_init, capture_events): sentry_init(traces_sample_rate=1.0) events = capture_events() - with start_span(transaction="/"): + with start_transaction(name="/"): with start_span(op="child-span"): with start_span(op="child-child-span"): scope = Hub.current.scope assert scope.span.op == "child-child-span" - assert scope.transaction.transaction == "/" + assert scope.transaction.name == "/" assert len(events) == 1