Documenting Spans and Transactions (#2358)

Added some docstrings and also did some minor cleanup for better readability of the code.

---------

Co-authored-by: Ivana Kellyerova <[email protected]>

Author: antonpirker

sentry_sdk/hub.py

@@ -479,6 +479,7 @@ def start_span(self, span=None, instrumenter=INSTRUMENTER.SENTRY, **kwargs):
         if instrumenter != configuration_instrumenter:
             return NoOpSpan()
 
+        # THIS BLOCK IS DEPRECATED
         # 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.
@@ -487,24 +488,33 @@ def start_span(self, span=None, instrumenter=INSTRUMENTER.SENTRY, **kwargs):
                 "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)
 
+        # THIS BLOCK IS DEPRECATED
+        # We do not pass a span into start_span in our code base, so I deprecate this.
         if span is not None:
+            deprecation_msg = "Deprecated: passing a span into `start_span` is deprecated and will be removed in the future."
+            logger.warning(deprecation_msg)
             return span
 
         kwargs.setdefault("hub", self)
 
-        span = self.scope.span
-        if span is not None:
-            return span.start_child(**kwargs)
+        active_span = self.scope.span
+        if active_span is not None:
+            new_child_span = active_span.start_child(**kwargs)
+            return new_child_span
 
         # If there is already a trace_id in the propagation context, use it.
+        # This does not need to be done for `start_child` above because it takes
+        # the trace_id from the parent span.
         if "trace_id" not in kwargs:
             traceparent = self.get_traceparent()
             trace_id = traceparent.split("-")[0] if traceparent else None

sentry_sdk/tracing.py

@@ -80,6 +80,9 @@ def add(self, span):
 
 
 class Span(object):
+    """A span holds timing information of a block of code.
+    Spans can have multiple child spans thus forming a span tree."""
+
     __slots__ = (
         "trace_id",
         "span_id",
@@ -201,6 +204,9 @@ def __exit__(self, ty, value, tb):
     @property
     def containing_transaction(self):
         # type: () -> Optional[Transaction]
+        """The ``Transaction`` that this span belongs to.
+        The ``Transaction`` is the root of the span tree,
+        so one could also think of this ``Transaction`` as the "root span"."""
 
         # this is a getter rather than a regular attribute so that transactions
         # can return `self` here instead (as a way to prevent them circularly
@@ -237,12 +243,15 @@ def start_child(self, instrumenter=INSTRUMENTER.SENTRY, **kwargs):
         )
         if span_recorder:
             span_recorder.add(child)
+
         return child
 
     def new_span(self, **kwargs):
         # type: (**Any) -> Span
-        """Deprecated: use :py:meth:`sentry_sdk.tracing.Span.start_child` instead."""
-        logger.warning("Deprecated: use Span.start_child instead of Span.new_span.")
+        """DEPRECATED: use :py:meth:`sentry_sdk.tracing.Span.start_child` instead."""
+        logger.warning(
+            "Deprecated: use Span.start_child instead of Span.new_span. This will be removed in the future."
+        )
         return self.start_child(**kwargs)
 
     @classmethod
@@ -254,12 +263,15 @@ def continue_from_environ(
         # type: (...) -> Transaction
         """
         Create a Transaction with the given params, then add in data pulled from
-        the 'sentry-trace' and 'baggage' headers from the environ (if any)
+        the ``sentry-trace`` and ``baggage`` headers from the environ (if any)
         before returning the Transaction.
 
-        This is different from `continue_from_headers` in that it assumes header
-        names in the form "HTTP_HEADER_NAME" - such as you would get from a wsgi
-        environ - rather than the form "header-name".
+        This is different from :py:meth:`~sentry_sdk.tracing.Span.continue_from_headers`
+        in that it assumes header names in the form ``HTTP_HEADER_NAME`` -
+        such as you would get from a WSGI/ASGI environ -
+        rather than the form ``header-name``.
+
+        :param environ: The ASGI/WSGI environ to pull information from.
         """
         if cls is Span:
             logger.warning(
@@ -277,7 +289,9 @@ def continue_from_headers(
         # type: (...) -> Transaction
         """
         Create a transaction with the given params (including any data pulled from
-        the 'sentry-trace' and 'baggage' headers).
+        the ``sentry-trace`` and ``baggage`` headers).
+
+        :param headers: The dictionary with the HTTP headers to pull information from.
         """
         # TODO move this to the Transaction class
         if cls is Span:
@@ -311,8 +325,8 @@ def continue_from_headers(
     def iter_headers(self):
         # type: () -> Iterator[Tuple[str, str]]
         """
-        Creates a generator which returns the span's `sentry-trace` and `baggage` headers.
-        If the span's containing transaction doesn't yet have a `baggage` value,
+        Creates a generator which returns the span's ``sentry-trace`` and ``baggage`` headers.
+        If the span's containing transaction doesn't yet have a ``baggage`` value,
         this will cause one to be generated and stored.
         """
         yield SENTRY_TRACE_HEADER_NAME, self.to_traceparent()
@@ -330,10 +344,10 @@ def from_traceparent(
     ):
         # type: (...) -> Optional[Transaction]
         """
-        DEPRECATED: Use :py:meth:`sentry_sdk.tracing.Transaction.continue_from_headers`.
+        DEPRECATED: Use :py:meth:`sentry_sdk.tracing.Span.continue_from_headers`.
 
-        Create a `Transaction` with the given params, then add in data pulled from
-        the given 'sentry-trace' header value before returning the `Transaction`.
+        Create a ``Transaction`` with the given params, then add in data pulled from
+        the given ``sentry-trace`` header value before returning the ``Transaction``.
         """
         logger.warning(
             "Deprecated: Use Transaction.continue_from_headers(headers, **kwargs) "
@@ -364,6 +378,9 @@ def to_traceparent(self):
 
     def to_baggage(self):
         # type: () -> Optional[Baggage]
+        """Returns the :py:class:`~sentry_sdk.tracing_utils.Baggage`
+        associated with this ``Span``, if any. (Taken from the root of the span tree.)
+        """
         if self.containing_transaction:
             return self.containing_transaction.get_baggage()
         return None
@@ -422,8 +439,21 @@ def is_success(self):
 
     def finish(self, hub=None, end_timestamp=None):
         # type: (Optional[sentry_sdk.Hub], Optional[datetime]) -> Optional[str]
-        # XXX: would be type: (Optional[sentry_sdk.Hub]) -> None, but that leads
+        # Note: would be type: (Optional[sentry_sdk.Hub]) -> None, but that leads
         # to incompatible return types for Span.finish and Transaction.finish.
+        """Sets the end timestamp of the span.
+        Additionally it also creates a breadcrumb from the span,
+        if the span represents a database or HTTP request.
+
+        :param hub: The hub to use for this transaction.
+            If not provided, the current hub will be used.
+        :param end_timestamp: Optional timestamp that should
+            be used as timestamp instead of the current time.
+
+        :return: Always ``None``. The type is ``Optional[str]`` to match
+            the return value of :py:meth:`sentry_sdk.tracing.Transaction.finish`.
+        """
+
         if self.timestamp is not None:
             # This span is already finished, ignore.
             return None
@@ -446,6 +476,8 @@ def finish(self, hub=None, end_timestamp=None):
 
     def to_json(self):
         # type: () -> Dict[str, Any]
+        """Returns a JSON-compatible representation of the span."""
+
         rv = {
             "trace_id": self.trace_id,
             "span_id": self.span_id,
@@ -491,6 +523,9 @@ def get_trace_context(self):
 
 
 class Transaction(Span):
+    """The Transaction is the root element that holds all the spans
+    for Sentry performance instrumentation."""
+
     __slots__ = (
         "name",
         "source",
@@ -512,6 +547,19 @@ def __init__(
         **kwargs  # type: Any
     ):
         # type: (...) -> None
+        """Constructs a new Transaction.
+
+        :param name: Identifier of the transaction.
+            Will show up in the Sentry UI.
+        :param parent_sampled: Whether the parent transaction was sampled.
+            If True this transaction will be kept, if False it will be discarded.
+        :param baggage: The W3C baggage header value.
+            (see https://www.w3.org/TR/baggage/)
+        :param source: A string describing the source of the transaction name.
+            This will be used to determine the transaction's type.
+            See https://develop.sentry.dev/sdk/event-payloads/transaction/#transaction-annotations
+            for more information. Default "custom".
+        """
         # TODO: consider removing this in a future release.
         # This is for backwards compatibility with releases before Transaction
         # existed, to allow for a smoother transition.
@@ -522,7 +570,7 @@ def __init__(
             )
             name = kwargs.pop("transaction")
 
-        Span.__init__(self, **kwargs)
+        super(Transaction, self).__init__(**kwargs)
 
         self.name = name
         self.source = source
@@ -568,6 +616,9 @@ def __exit__(self, ty, value, tb):
     @property
     def containing_transaction(self):
         # type: () -> Transaction
+        """The root element of the span tree.
+        In the case of a transaction it is the transaction itself.
+        """
 
         # Transactions (as spans) belong to themselves (as transactions). This
         # is a getter rather than a regular attribute to avoid having a circular
@@ -576,6 +627,17 @@ def containing_transaction(self):
 
     def finish(self, hub=None, end_timestamp=None):
         # type: (Optional[sentry_sdk.Hub], Optional[datetime]) -> Optional[str]
+        """Finishes the transaction and sends it to Sentry.
+        All finished spans in the transaction will also be sent to Sentry.
+
+        :param hub: The hub to use for this transaction.
+            If not provided, the current hub will be used.
+        :param end_timestamp: Optional timestamp that should
+            be used as timestamp instead of the current time.
+
+        :return: The event ID if the transaction was sent to Sentry,
+            otherwise None.
+        """
         if self.timestamp is not None:
             # This transaction is already finished, ignore.
             return None
@@ -610,7 +672,7 @@ def finish(self, hub=None, end_timestamp=None):
             )
             self.name = "<unlabeled transaction>"
 
-        Span.finish(self, hub, end_timestamp)
+        super(Transaction, self).finish(hub, end_timestamp)
 
         if not self.sampled:
             # At this point a `sampled = None` should have already been resolved
@@ -661,15 +723,26 @@ def set_measurement(self, name, value, unit=""):
 
     def set_context(self, key, value):
         # type: (str, Any) -> None
+        """Sets a context. Transactions can have multiple contexts
+        and they should follow the format described in the "Contexts Interface"
+        documentation.
+
+        :param key: The name of the context.
+        :param value: The information about the context.
+        """
         self._contexts[key] = value
 
     def set_http_status(self, http_status):
         # type: (int) -> None
+        """Sets the status of the Transaction according to the given HTTP status.
+
+        :param http_status: The HTTP status code."""
         super(Transaction, self).set_http_status(http_status)
         self.set_context("response", {"status_code": http_status})
 
     def to_json(self):
         # type: () -> Dict[str, Any]
+        """Returns a JSON-compatible representation of the transaction."""
         rv = super(Transaction, self).to_json()
 
         rv["name"] = self.name
@@ -680,10 +753,12 @@ def to_json(self):
 
     def get_baggage(self):
         # type: () -> Baggage
-        """
-        The first time a new baggage with sentry items is made,
-        it will be frozen.
-        """
+        """Returns the :py:class:`~sentry_sdk.tracing_utils.Baggage`
+        associated with the Transaction.
+
+        The first time a new baggage with Sentry items is made,
+        it will be frozen."""
+
         if not self._baggage or self._baggage.mutable:
             self._baggage = Baggage.populate_from_transaction(self)
 

sentry_sdk/tracing_utils.py

@@ -215,6 +215,10 @@ def _format_sql(cursor, sql):
 
 
 class Baggage(object):
+    """
+    The W3C Baggage header information (see https://www.w3.org/TR/baggage/).
+    """
+
     __slots__ = ("sentry_items", "third_party_items", "mutable")
 
     SENTRY_PREFIX = "sentry-"