diff --git a/mypy.ini b/mypy.ini index 6165796502..7ad5ce7148 100644 --- a/mypy.ini +++ b/mypy.ini @@ -22,9 +22,6 @@ warn_redundant_casts = True ; Relaxations: -[mypy-sentry_sdk.tracing] -disallow_untyped_defs = False - [mypy-sentry_sdk._compat] disallow_untyped_defs = False diff --git a/sentry_sdk/api.py b/sentry_sdk/api.py index 93d8137236..873ea96dce 100644 --- a/sentry_sdk/api.py +++ b/sentry_sdk/api.py @@ -15,6 +15,7 @@ from typing import ContextManager from sentry_sdk._types import Event, Hint, Breadcrumb, BreadcrumbHint + from sentry_sdk.tracing import Span T = TypeVar("T") F = TypeVar("F", bound=Callable[..., Any]) @@ -34,6 +35,7 @@ def overload(x): "push_scope", "flush", "last_event_id", + "start_span", ] @@ -179,3 +181,15 @@ def last_event_id(): if hub is not None: return hub.last_event_id() return None + + +@hubmethod +def start_span( + span=None, # type: Optional[Span] + **kwargs # type: Any +): + # type: (...) -> 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) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 0d149bfce1..fa72a42113 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -10,7 +10,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, maybe_create_breadcrumbs_from_span +from sentry_sdk.tracing import Span from sentry_sdk.utils import ( exc_info_from_error, event_from_exception, @@ -128,17 +128,6 @@ def main(self): return GLOBAL_HUB -class _HubManager(object): - def __init__(self, hub): - # type: (Hub) -> None - self._old = Hub.current - _local.set(hub) - - def __exit__(self, exc_type, exc_value, tb): - # type: (Any, Any, Any) -> None - _local.set(self._old) - - class _ScopeManager(object): def __init__(self, hub): # type: (Hub) -> None @@ -429,44 +418,27 @@ def add_breadcrumb( while len(scope._breadcrumbs) > max_breadcrumbs: scope._breadcrumbs.popleft() - @contextmanager - def span( - self, - span=None, # type: Optional[Span] - **kwargs # type: Any - ): - # type: (...) -> Generator[Span, None, None] - # TODO: Document - span = self.start_span(span=span, **kwargs) - - _, scope = self._stack[-1] - old_span = scope.span - scope.span = span - - try: - yield span - except Exception: - span.set_failure() - raise - finally: - try: - span.finish() - maybe_create_breadcrumbs_from_span(self, span) - self.finish_span(span) - except Exception: - self._capture_internal_exception(sys.exc_info()) - scope.span = old_span - def start_span( self, span=None, # type: Optional[Span] **kwargs # type: Any ): # type: (...) -> Span - # TODO: Document + """ + 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. + """ client, scope = self._stack[-1] + kwargs.setdefault("hub", self) + if span is None: if scope.span is not None: span = scope.span.new_span(**kwargs) @@ -482,48 +454,6 @@ def start_span( return span - def finish_span( - self, span # type: Span - ): - # type: (...) -> Optional[str] - # TODO: Document - if span.timestamp is None: - # This transaction is not yet finished so we just finish it. - span.finish() - - if span.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 - - if self.client is None: - # We have no client and therefore nowhere to send this transaction - # event. - return None - - if not span.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 Hub.span(..)` on a - # non-transaction span and later decided to make it a transaction. - assert ( - span.sampled is not None - ), "Need to set transaction when entering span!" - return None - - return self.capture_event( - { - "type": "transaction", - "transaction": span.transaction, - "contexts": {"trace": span.get_trace_context()}, - "timestamp": span.timestamp, - "start_timestamp": span.start_timestamp, - "spans": [ - s.to_json() for s in (span._finished_spans or ()) if s is not span - ], - } - ) - @overload # noqa def push_scope( self, callback=None # type: Optional[None] diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 8f2840feb5..aeef62e67a 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -65,7 +65,7 @@ async def inner(): # If this transaction name makes it to the UI, AIOHTTP's # URL resolver did not find a route or died trying. - with hub.span(transaction="generic AIOHTTP request"): + with hub.start_span(transaction="generic AIOHTTP request"): try: response = await old_handle(self, request) except HTTPException: diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py index 272139279a..c95be9eb8b 100644 --- a/sentry_sdk/integrations/celery.py +++ b/sentry_sdk/integrations/celery.py @@ -79,7 +79,7 @@ def apply_async(*args, **kwargs): if headers is not None: kwargs["headers"] = headers - with hub.span(op="celery.submit", description=task.name): + with hub.start_span(op="celery.submit", description=task.name): return f(*args, **kwargs) else: return f(*args, **kwargs) @@ -114,7 +114,7 @@ def _inner(*args, **kwargs): # something such as attribute access can fail. span.transaction = task.name - with hub.span(span): + with hub.start_span(span): return f(*args, **kwargs) return _inner diff --git a/sentry_sdk/integrations/redis.py b/sentry_sdk/integrations/redis.py index 5e10d3bd91..0f23210b99 100644 --- a/sentry_sdk/integrations/redis.py +++ b/sentry_sdk/integrations/redis.py @@ -32,7 +32,7 @@ def sentry_patched_execute_command(self, name, *args, **kwargs): description = " ".join(description_parts) - with hub.span(op="redis", description=description) as span: + with hub.start_span(op="redis", description=description) as span: if name and args and name.lower() in ("get", "set", "setex", "setnx"): span.set_tag("redis.key", args[0]) diff --git a/sentry_sdk/integrations/rq.py b/sentry_sdk/integrations/rq.py index 011811c0a3..a32ec57f5b 100644 --- a/sentry_sdk/integrations/rq.py +++ b/sentry_sdk/integrations/rq.py @@ -55,7 +55,7 @@ def sentry_patched_perform_job(self, job, *args, **kwargs): with capture_internal_exceptions(): span.transaction = job.func_name - with hub.span(span): + with hub.start_span(span): rv = old_perform_job(self, job, *args, **kwargs) if self.is_horse: diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index da81ca91bc..5e83faaab8 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -181,7 +181,7 @@ def sentry_patched_popen_init(self, *a, **kw): env = _init_argument(a, kw, "env", 10, lambda x: dict(x or os.environ)) env["SUBPROCESS_" + k.upper().replace("-", "_")] = v - with hub.span(op="subprocess", description=description) as span: + with hub.start_span(op="subprocess", description=description) as span: span.set_data("subprocess.cwd", cwd) return old_popen_init(self, *a, **kw) diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 4ca51e33cc..aebacb4ef6 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -96,7 +96,7 @@ def __call__(self, environ, start_response): span.op = "http.server" span.transaction = "generic WSGI request" - with hub.span(span) as span: + with hub.start_span(span) as span: try: rv = self.app( environ, diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index acb25e365f..57dd0d9f25 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -4,6 +4,7 @@ from datetime import datetime +import sentry_sdk from sentry_sdk.utils import capture_internal_exceptions from sentry_sdk._compat import PY2 from sentry_sdk._types import MYPY @@ -21,8 +22,7 @@ from typing import Any from typing import Dict from typing import List - - import sentry_sdk + from typing import Tuple _traceparent_header_format_re = re.compile( "^[ \t]*" # whitespace @@ -44,12 +44,15 @@ def __init__( self.prefix = prefix def __getitem__(self, key): + # type: (str) -> Optional[Any] return self.environ[self.prefix + key.replace("-", "_").upper()] def __len__(self): + # type: () -> int return sum(1 for _ in iter(self)) def __iter__(self): + # type: () -> Generator[str, None, None] for k in self.environ: if not isinstance(k, str): continue @@ -76,6 +79,8 @@ class Span(object): "_tags", "_data", "_finished_spans", + "hub", + "_context_manager_state", ) def __init__( @@ -88,6 +93,7 @@ def __init__( transaction=None, # type: Optional[str] op=None, # type: Optional[str] description=None, # type: Optional[str] + hub=None, # type: Optional[sentry_sdk.Hub] ): # type: (...) -> None self.trace_id = trace_id or uuid.uuid4().hex @@ -98,19 +104,22 @@ def __init__( self.transaction = transaction self.op = op self.description = description + self.hub = hub self._tags = {} # type: Dict[str, str] self._data = {} # type: Dict[str, Any] self._finished_spans = None # type: Optional[List[Span]] self.start_timestamp = datetime.now() #: End timestamp of span - self.timestamp = None + self.timestamp = None # type: Optional[datetime] def init_finished_spans(self): + # type: () -> None if self._finished_spans is None: self._finished_spans = [] def __repr__(self): + # type: () -> str return ( "<%s(transaction=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( @@ -123,7 +132,29 @@ def __repr__(self): ) ) + def __enter__(self): + # type: () -> Span + hub = self.hub or sentry_sdk.Hub.current + + _, scope = hub._stack[-1] + old_span = scope.span + scope.span = self + self._context_manager_state = (hub, scope, old_span) + return self + + def __exit__(self, ty, value, tb): + # type: (Optional[Any], Optional[Any], Optional[Any]) -> None + if value is not None: + self.set_failure() + + hub, scope, old_span = self._context_manager_state + del self._context_manager_state + + self.finish(hub) + scope.span = old_span + def new_span(self, **kwargs): + # type: (**Any) -> Span rv = type(self)( trace_id=self.trace_id, span_id=None, @@ -136,20 +167,24 @@ def new_span(self, **kwargs): @classmethod def continue_from_environ(cls, environ): + # type: (typing.Mapping[str, str]) -> Span return cls.continue_from_headers(EnvironHeaders(environ)) @classmethod def continue_from_headers(cls, headers): + # type: (typing.Mapping[str, str]) -> Span parent = cls.from_traceparent(headers.get("sentry-trace")) if parent is None: return cls() return parent.new_span(same_process_as_parent=False) def iter_headers(self): + # type: () -> Generator[Tuple[str, str], None, None] yield "sentry-trace", self.to_traceparent() @classmethod def from_traceparent(cls, traceparent): + # type: (Optional[str]) -> Optional[Span] if not traceparent: return None @@ -175,6 +210,7 @@ def from_traceparent(cls, traceparent): return cls(trace_id=trace_id, span_id=span_id, sampled=sampled) def to_traceparent(self): + # type: () -> str sampled = "" if self.sampled is True: sampled = "1" @@ -183,29 +219,79 @@ def to_traceparent(self): return "%s-%s-%s" % (self.trace_id, self.span_id, sampled) def to_legacy_traceparent(self): + # type: () -> str return "00-%s-%s-00" % (self.trace_id, self.span_id) def set_tag(self, key, value): + # type: (str, Any) -> None self._tags[key] = value def set_data(self, key, value): + # type: (str, Any) -> None self._data[key] = value def set_failure(self): + # type: () -> None self.set_tag("status", "failure") def set_success(self): + # type: () -> None self.set_tag("status", "success") def is_success(self): + # type: () -> bool return self._tags.get("status") in (None, "success") - def finish(self): + def finish(self, hub=None): + # type: (Optional[sentry_sdk.Hub]) -> Optional[str] + hub = hub or self.hub or sentry_sdk.Hub.current + + if self.timestamp is not None: + # This transaction is already finished, so we should not flush it again. + return None + self.timestamp = datetime.now() + if self._finished_spans is not None: self._finished_spans.append(self) + _maybe_create_breadcrumbs_from_span(hub, 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 + + if hub.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. + assert ( + self.sampled is not None + ), "Need to set transaction when entering span!" + return None + + return hub.capture_event( + { + "type": "transaction", + "transaction": self.transaction, + "contexts": {"trace": self.get_trace_context()}, + "timestamp": self.timestamp, + "start_timestamp": self.start_timestamp, + "spans": [ + s.to_json() for s in (self._finished_spans or ()) if s is not self + ], + } + ) + def to_json(self): + # type: () -> Any rv = { "trace_id": self.trace_id, "span_id": self.span_id, @@ -223,6 +309,7 @@ def to_json(self): return rv def get_trace_context(self): + # type: () -> Any rv = { "trace_id": self.trace_id, "span_id": self.span_id, @@ -266,7 +353,7 @@ def record_sql_queries( paramstyle, # type: Optional[str] executemany, # type: bool ): - # type: (...) -> Generator[Optional[Span], None, None] + # type: (...) -> Generator[Span, None, None] if not params_list or params_list == [None]: params_list = None @@ -282,7 +369,7 @@ def record_sql_queries( with capture_internal_exceptions(): hub.add_breadcrumb(message=query, category="query", data=data) - with hub.span(op="db", description=query) as span: + with hub.start_span(op="db", description=query) as span: for k, v in data.items(): span.set_data(k, v) yield span @@ -290,9 +377,10 @@ def record_sql_queries( @contextlib.contextmanager def record_http_request(hub, url, method): + # type: (sentry_sdk.Hub, str, str) -> Generator[Dict[str, str], None, None] data_dict = {"url": url, "method": method} - with hub.span(op="http", description="%s %s" % (url, method)) as span: + with hub.start_span(op="http", description="%s %s" % (url, method)) as span: try: yield data_dict finally: @@ -303,7 +391,8 @@ def record_http_request(hub, url, method): span.set_data(k, v) -def maybe_create_breadcrumbs_from_span(hub, span): +def _maybe_create_breadcrumbs_from_span(hub, span): + # type: (sentry_sdk.Hub, Span) -> None if span.op == "redis": hub.add_breadcrumb(type="redis", category="redis", data=span._tags) elif span.op == "http" and span.is_success(): diff --git a/tests/integrations/celery/test_celery.py b/tests/integrations/celery/test_celery.py index 32a82c2a96..3e3c436b87 100644 --- a/tests/integrations/celery/test_celery.py +++ b/tests/integrations/celery/test_celery.py @@ -74,7 +74,7 @@ def dummy_task(x, y): foo = 42 # noqa return x / y - with Hub.current.span() as span: + with Hub.current.start_span() as span: celery_invocation(dummy_task, 1, 2) _, expected_context = celery_invocation(dummy_task, 1, 0) @@ -106,7 +106,7 @@ def dummy_task(x, y): events = capture_events() - with Hub.current.span(transaction="submission") as span: + with Hub.current.start_span(transaction="submission") as span: celery_invocation(dummy_task, 1, 0 if task_fails else 1) if task_fails: @@ -178,7 +178,7 @@ def test_simple_no_propagation(capture_events, init_celery): def dummy_task(): 1 / 0 - with Hub.current.span() as span: + with Hub.current.start_span() as span: dummy_task.delay() event, = events diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py index f7765c4cee..00a0d1d8bc 100644 --- a/tests/integrations/stdlib/test_subprocess.py +++ b/tests/integrations/stdlib/test_subprocess.py @@ -62,7 +62,7 @@ def test_subprocess_basic( sentry_init(integrations=[StdlibIntegration()], traces_sample_rate=1.0) events = capture_events() - with Hub.current.span(transaction="foo", op="foo") as span: + with Hub.current.start_span(transaction="foo", op="foo") as span: args = [ sys.executable, "-c", diff --git a/tests/test_tracing.py b/tests/test_tracing.py index ce9b48b0fa..2e118cd2d9 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -12,12 +12,12 @@ def test_basic(sentry_init, capture_events, sample_rate): sentry_init(traces_sample_rate=sample_rate) events = capture_events() - with Hub.current.span(transaction="hi"): + with Hub.current.start_span(transaction="hi"): with pytest.raises(ZeroDivisionError): - with Hub.current.span(op="foo", description="foodesc"): + with Hub.current.start_span(op="foo", description="foodesc"): 1 / 0 - with Hub.current.span(op="bar", description="bardesc"): + with Hub.current.start_span(op="bar", description="bardesc"): pass if sample_rate: @@ -41,8 +41,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.span(transaction="hi"): - with Hub.current.span() as old_span: + with Hub.current.start_span(transaction="hi"): + with Hub.current.start_span() as old_span: old_span.sampled = sampled headers = dict(Hub.current.iter_trace_propagation_headers()) @@ -60,7 +60,7 @@ def test_continue_from_headers(sentry_init, capture_events, sampled): assert span.sampled == sampled assert span.trace_id == old_span.trace_id - with Hub.current.span(span): + with Hub.current.start_span(span): with Hub.current.configure_scope() as scope: scope.transaction = "ho" capture_message("hello") @@ -88,13 +88,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.span(transaction="hi") as trace: + with Hub.current.start_span(transaction="hi") as trace: assert trace.sampled is not None - with Hub.current.span() as span: + with Hub.current.start_span() as span: assert span.sampled == trace.sampled - with Hub.current.span() as span: + with Hub.current.start_span() as span: assert span.sampled is None @@ -107,9 +107,9 @@ def test_memory_usage(sentry_init, capture_events, args, expected_refcount): references = weakref.WeakSet() - with Hub.current.span(transaction="hi"): + with Hub.current.start_span(transaction="hi"): for i in range(100): - with Hub.current.span( + with Hub.current.start_span( op="helloworld", description="hi {}".format(i) ) as span: