diff --git a/pytest.ini b/pytest.ini index 4e440e2a47..c00b03296c 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,5 +2,5 @@ DJANGO_SETTINGS_MODULE = tests.integrations.django.myapp.settings addopts = --tb=short markers = - tests_internal_exceptions + tests_internal_exceptions: Handle internal exceptions just as the SDK does, to test it. (Otherwise internal exceptions are recorded and reraised.) only: A temporary marker, to make pytest only run the tests with the mark, similar to jest's `it.only`. To use, run `pytest -v -m only`. diff --git a/sentry_sdk/integrations/flask.py b/sentry_sdk/integrations/flask.py index f6306e5a41..fe630ea50a 100644 --- a/sentry_sdk/integrations/flask.py +++ b/sentry_sdk/integrations/flask.py @@ -104,7 +104,8 @@ def _request_started(sender, **kwargs): with hub.configure_scope() as scope: request = _request_ctx_stack.top.request - # Rely on WSGI middleware to start a trace + # Set the transaction name here, but rely on WSGI middleware to actually + # start the transaction try: if integration.transaction_style == "endpoint": scope.transaction = request.url_rule.endpoint diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index 30bf014068..bc3df8b97b 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -77,6 +77,8 @@ class Scope(object): "_level", "_name", "_fingerprint", + # note that for legacy reasons, _transaction is the transaction *name*, + # not a Transaction object (the object is stored in _span) "_transaction", "_user", "_tags", diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 3028284ac3..af256d583e 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -111,6 +111,11 @@ class Span(object): def __new__(cls, **kwargs): # type: (**Any) -> Any + """ + Backwards-compatible implementation of Span and Transaction + creation. + """ + # TODO: consider removing this in a future release. # This is for backwards compatibility with releases before Transaction # existed, to allow for a smoother transition. @@ -166,8 +171,10 @@ def init_span_recorder(self, maxlen): def __repr__(self): # type: () -> str - return "<%s(trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( + return "<%s(op=%r, description:%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( self.__class__.__name__, + self.op, + self.description, self.trace_id, self.span_id, self.parent_span_id, @@ -200,8 +207,9 @@ def start_child(self, **kwargs): """ 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. + Takes the same arguments as the initializer of :py:class:`Span`. The + trace id, sampling decision, and span recorder are inherited from the + current span/transaction. """ kwargs.setdefault("sampled", self.sampled) @@ -227,6 +235,14 @@ def continue_from_environ( **kwargs # type: Any ): # type: (...) -> Transaction + """ + Create a Transaction with the given params, then add in data pulled from + the 'sentry-trace' header in the environ (if any) before returning the + Transaction. + + If the 'sentry-trace' header is malformed or missing, just create and + return a Transaction instance with the given params. + """ if cls is Span: logger.warning( "Deprecated: use Transaction.continue_from_environ " @@ -241,16 +257,25 @@ def continue_from_headers( **kwargs # type: Any ): # type: (...) -> Transaction + """ + Create a Transaction with the given params, then add in data pulled from + the 'sentry-trace' header (if any) before returning the Transaction. + + If the 'sentry-trace' header is malformed or missing, just create and + return a Transaction instance with the given params. + """ 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: - parent = Transaction(**kwargs) - parent.same_process_as_parent = False - return parent + transaction = Transaction.from_traceparent( + headers.get("sentry-trace"), **kwargs + ) + if transaction is None: + transaction = Transaction(**kwargs) + transaction.same_process_as_parent = False + return transaction def iter_headers(self): # type: () -> Generator[Tuple[str, str], None, None] @@ -263,6 +288,13 @@ def from_traceparent( **kwargs # type: Any ): # type: (...) -> Optional[Transaction] + """ + Create a Transaction with the given params, then add in data pulled from + the given 'sentry-trace' header value before returning the Transaction. + + If the header value is malformed or missing, just create and return a + Transaction instance with the given params. + """ if cls is Span: logger.warning( "Deprecated: use Transaction.from_traceparent " @@ -279,20 +311,23 @@ def from_traceparent( if match is None: return None - trace_id, span_id, sampled_str = match.groups() + trace_id, parent_span_id, sampled_str = match.groups() if trace_id is not None: trace_id = "{:032x}".format(int(trace_id, 16)) - if span_id is not None: - span_id = "{:016x}".format(int(span_id, 16)) + if parent_span_id is not None: + parent_span_id = "{:016x}".format(int(parent_span_id, 16)) if sampled_str: - sampled = sampled_str != "0" # type: Optional[bool] + parent_sampled = sampled_str != "0" # type: Optional[bool] else: - sampled = None + parent_sampled = None return Transaction( - trace_id=trace_id, parent_span_id=span_id, sampled=sampled, **kwargs + trace_id=trace_id, + parent_span_id=parent_span_id, + sampled=parent_sampled, + **kwargs ) def to_traceparent(self): @@ -436,16 +471,14 @@ def __init__( 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, - ) + return "<%s(name=%r, op=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( + self.__class__.__name__, + self.name, + self.op, + self.trace_id, + self.span_id, + self.parent_span_id, + self.sampled, ) def finish(self, hub=None): @@ -454,7 +487,9 @@ def finish(self, hub=None): # This transaction is already finished, ignore. return None + # This is a de facto proxy for checking if sampled = False if self._span_recorder is None: + logger.debug("Discarding transaction because sampled = False") return None hub = hub or self.hub or sentry_sdk.Hub.current diff --git a/tests/conftest.py b/tests/conftest.py index 1c368a5b14..d5589238b5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -48,6 +48,8 @@ def _capture_internal_exception(self, exc_info): @request.addfinalizer def _(): + # rerasise the errors so that this just acts as a pass-through (that + # happens to keep track of the errors which pass through it) for e in errors: reraise(*e) diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index a8d9a6a458..ed062761bb 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -4,13 +4,17 @@ import pytest try: + # py3 from urllib.request import urlopen except ImportError: + # py2 from urllib import urlopen try: + # py2 from httplib import HTTPSConnection except ImportError: + # py3 from http.client import HTTPSConnection from sentry_sdk import capture_message diff --git a/tests/tracing/test_integration_tests.py b/tests/tracing/test_integration_tests.py index 7423e4bd1e..3f5025e41f 100644 --- a/tests/tracing/test_integration_tests.py +++ b/tests/tracing/test_integration_tests.py @@ -51,11 +51,13 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): sentry_init(traces_sample_rate=1.0) events = capture_events() + # make a parent transaction (normally this would be in a different service) with start_transaction(name="hi"): with start_span() as old_span: old_span.sampled = sampled headers = dict(Hub.current.iter_trace_propagation_headers()) + # test that the sampling decision is getting encoded in the header correctly header = headers["sentry-trace"] if sampled is True: assert header.endswith("-1") @@ -64,6 +66,8 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): if sampled is None: assert header.endswith("-") + # child transaction, to prove that we can read 'sentry-trace' header data + # correctly transaction = Transaction.continue_from_headers(headers, name="WRONG") assert transaction is not None assert transaction.sampled == sampled @@ -72,6 +76,9 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): assert transaction.parent_span_id == old_span.span_id assert transaction.span_id != old_span.span_id + # add child transaction to the scope, to show that the captured message will + # be tagged with the trace id (since it happens while the transaction is + # open) with start_transaction(transaction): with configure_scope() as scope: scope.transaction = "ho" diff --git a/tests/tracing/test_misc.py b/tests/tracing/test_misc.py index ce717437ea..8cb4988f2a 100644 --- a/tests/tracing/test_misc.py +++ b/tests/tracing/test_misc.py @@ -14,6 +14,12 @@ def test_span_trimming(sentry_init, capture_events): pass (event,) = events + + # the transaction is its own first span (which counts for max_spans) but it + # doesn't show up in the span list in the event, so this is 1 less than our + # max_spans value + assert len(event["spans"]) == 2 + span1, span2 = event["spans"] assert span1["op"] == "foo0" assert span2["op"] == "foo1"