add some more comments

Author: pabloDeputter

sentry_sdk/integrations/boto3/_client.py

@@ -29,7 +29,19 @@
 def _activate_client_span(
     span: "Union[Span, StreamedSpan]",
 ) -> "Iterator[Union[Span, StreamedSpan]]":
-    """Temporarily activate an inactive boto span without ending it."""
+    """
+    Activate the boto span temporarily during `_make_api_call()` without ending it.
+
+    Botocore returns a `StreamingBody` before its bytes are consumed. Using the
+    context manager would finish it as soon as `_make_api_call()` returns, so
+    restore the caller's span here and let the `StreamingBody` wrapper finish
+    the boto span when body is consumed/closed.
+
+    faulty:                               desired:
+           boto3  [_make_api_call]                boto3  [_make_api_call------]
+           http     [request]                     http       [request]
+           stream               [read]            stream                [read]
+    """
     if isinstance(span, NoOpStreamedSpan):
         yield span
         return

sentry_sdk/integrations/boto3/_instrumentation.py

@@ -106,12 +106,11 @@ def _instrument_streaming_body(
     if isinstance(span, StreamedSpan):
         streaming_span = sentry_sdk.traces.start_span(
             name=span.name,
-            # `parent_span` is set explicitly to the boto span.
+            # keep stream span under the boto span after `_make_api_call()` returns.
             parent_span=span,
-            # avoid making the streaming span the current span on the scope since the application might
-            # keep `StreamingBody` open before reading it. Otherwise: 1. when the streamingspan ends it
-            # could restore the parent span on the scope, breaking the parent-child relation of newly
-            # created spans; 2. newly created spans would be attached to the streaming span.
+            # the body may outlive the api call, so keep it inactive. Otherwise it
+            # 1. could restore the already-finished boto span when it ends; 2. make
+            # unrelated new spans attach to the stream span since it's the current span.
             active=False,
             attributes={
                 "sentry.op": OP.HTTP_CLIENT_STREAM,
@@ -134,6 +133,7 @@ def finish_span(error: "Optional[BaseException]" = None) -> None:
             return
 
         finished = True
+        # finish stream span before boto span, and only once across read/close.
         _finish_span(streaming_span, error)
         _finish_span(span, error)
 
@@ -153,6 +153,7 @@ def sentry_streaming_body_read(*args: "Any", **kwargs: "Any") -> bytes:
             read_return_value = orig_read(*args, **kwargs)
             with capture_internal_exceptions():
                 amount_of_bytes_requested = args[0] if args else kwargs.get("amt")
+                # detect read-to-end, eof, or the known content length being consumed.
                 if (
                     amount_of_bytes_requested is None
                     or amount_of_bytes_requested < 0
@@ -190,8 +191,6 @@ def sentry_raw_stream_close(*args: "Any", **kwargs: "Any") -> None:
         raw_stream = body._raw_stream  # type: ignore[attr-defined]
         orig_raw_close = raw_stream.close
 
-        # StreamingBody.__exit__ closes `_raw_stream` directly, bypassing
-        # StreamingBody.close(), so both levels need to be instrumented.
         raw_stream.close = sentry_raw_stream_close
         body.read = sentry_streaming_body_read  # type: ignore
         body.close = sentry_streaming_body_close  # type: ignore