Skip to content

Commit

Permalink
feat: Introduce Transaction and Hub.start_transaction
Browse files Browse the repository at this point in the history
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.

Co-authored-by: Markus Unterwaditzer <markus@unterwaditzer.net>
  • Loading branch information
rhcarvalho and untitaker committed Jun 27, 2020
1 parent 2c0b5ec commit ecafc4a
Show file tree
Hide file tree
Showing 14 changed files with 472 additions and 209 deletions.
5 changes: 5 additions & 0 deletions sentry_sdk/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from typing import Optional
from typing import Tuple
from typing import Type
from typing import Union
from typing_extensions import Literal

ExcInfo = Tuple[
Expand All @@ -36,3 +37,7 @@
]
SessionStatus = Literal["ok", "exited", "crashed", "abnormal"]
EndpointType = Literal["store", "envelope"]

from sentry_sdk.tracing import Span, Transaction # noqa

SpanLike = Union[Span, Transaction]
12 changes: 11 additions & 1 deletion sentry_sdk/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -37,6 +37,7 @@ def overload(x):
"flush",
"last_event_id",
"start_span",
"start_transaction",
"set_tag",
"set_context",
"set_extra",
Expand Down Expand Up @@ -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)
144 changes: 122 additions & 22 deletions sentry_sdk/hub.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -441,38 +441,138 @@ 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.
"""

client, scope = self._stack[-1]
# XXX: if the only way to use start_span correctly is when there
# is already an existing transaction/span in the scope, then
# this is a hard to use API. We don't document nor support
# appending an existing span to a new transaction created to
# contain the span.

# 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)

# XXX: this is not very useful because
#
# with hub.start_span(Span(...)):
# pass
#
# is equivalent to the simpler
#
# with Span(...):
# pass
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)

# XXX: returning a detached span here means whatever span tree built
# from it will be eventually discarded.
#
# This is also not very useful because
#
# with hub.start_span(op="..."):
# pass
#
# is equivalent, when there is no span in the scope, to the simpler
#
# with Span(op="..."):
# pass
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.
"""
# 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:
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(
Expand Down
22 changes: 12 additions & 10 deletions sentry_sdk/integrations/aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions sentry_sdk/integrations/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
16 changes: 9 additions & 7 deletions sentry_sdk/integrations/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions sentry_sdk/integrations/rq.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit ecafc4a

Please sign in to comment.