Address CR comments

Author: ericapisani

sentry_sdk/integrations/bottle.py

@@ -106,22 +106,9 @@ def _patched_handle(self: "Bottle", environ: "Dict[str, Any]") -> "Any":
             if has_span_streaming_enabled(sentry_sdk.get_client().options):
                 res = old_handle(self, environ)
 
-                try:
-                    if integration.transaction_style == "url":
-                        name = bottle_request.route.rule or "bottle"
-                    else:
-                        name = (
-                            bottle_request.route.name
-                            or transaction_from_function(bottle_request.route.callback)
-                            or "bottle"
-                        )
-
-                    sentry_sdk.get_current_scope().set_transaction_name(
-                        name,
-                        source=SEGMENT_SOURCE_FOR_STYLE[integration.transaction_style],
-                    )
-                except RuntimeError:
-                    pass
+                _set_segment_name_and_source(
+                    transaction_style=integration.transaction_style
+                )
 
             else:
                 res = old_handle(self, environ)
@@ -143,33 +130,17 @@ def patched_make_callback(
                 return prepared_callback
 
             def wrapped_callback(*args: object, **kwargs: object) -> "Any":
-                if has_span_streaming_enabled(sentry_sdk.get_client().options):
-                    with sentry_sdk.traces.start_span(name="bottle"):
-                        try:
-                            res = prepared_callback(*args, **kwargs)
-                        except Exception as exception:
-                            _capture_exception(exception, handled=False)
-                            raise exception
-
-                        if (
-                            isinstance(res, HTTPResponse)
-                            and res.status_code
-                            in integration.failed_request_status_codes
-                        ):
-                            _capture_exception(res, handled=True)
-
-                else:
-                    try:
-                        res = prepared_callback(*args, **kwargs)
-                    except Exception as exception:
-                        _capture_exception(exception, handled=False)
-                        raise exception
-
-                    if (
-                        isinstance(res, HTTPResponse)
-                        and res.status_code in integration.failed_request_status_codes
-                    ):
-                        _capture_exception(res, handled=True)
+                try:
+                    res = prepared_callback(*args, **kwargs)
+                except Exception as exception:
+                    _capture_exception(exception, handled=False)
+                    raise exception
+
+                if (
+                    isinstance(res, HTTPResponse)
+                    and res.status_code in integration.failed_request_status_codes
+                ):
+                    _capture_exception(res, handled=True)
 
                 return res
 
@@ -203,6 +174,25 @@ def size_of_file(self, file: "FileUpload") -> int:
         return file.content_length
 
 
+def _set_segment_name_and_source(transaction_style: str) -> None:
+    try:
+        if transaction_style == "url":
+            name = bottle_request.route.rule or "bottle request"
+        else:
+            name = (
+                bottle_request.route.name
+                or transaction_from_function(bottle_request.route.callback)
+                or "bottle request"
+            )
+
+        sentry_sdk.get_current_scope().set_transaction_name(
+            name,
+            source=SEGMENT_SOURCE_FOR_STYLE[transaction_style],
+        )
+    except RuntimeError:
+        pass
+
+
 def _set_transaction_name_and_source(
     event: "Event", transaction_style: str, request: "Any"
 ) -> None:

tests/integrations/bottle/test_bottle.py

@@ -593,10 +593,9 @@ def hi():
     sentry_sdk.flush()
 
     spans = [item.payload for item in items]
-    assert len(spans) == 2
+    assert len(spans) == 1
 
-    callback_span = spans[0]
-    segment = spans[1]
+    segment = spans[0]
 
     # Segment span (root, created by WSGI middleware)
     assert segment["is_segment"] is True
@@ -608,11 +607,6 @@ def hi():
     assert segment["attributes"]["http.response.status_code"] == 200
     assert segment["name"].endswith("hi")
 
-    # Callback span (created by wrapped_callback, child of segment)
-    assert callback_span["is_segment"] is False
-    assert callback_span["parent_span_id"] == segment["span_id"]
-    assert callback_span["name"] == "bottle"
-
 
 @pytest.mark.parametrize(
     "url,transaction_style,expected_name,expected_source",
@@ -658,13 +652,11 @@ def named_hi():
     sentry_sdk.flush()
 
     spans = [item.payload for item in items]
-    assert len(spans) == 2
+    assert len(spans) == 1
 
-    callback_span = spans[0]
-    segment = spans[1]
+    segment = spans[0]
 
     assert segment["is_segment"] is True
-    assert callback_span["parent_span_id"] == segment["span_id"]
 
     assert segment["name"].endswith(expected_name)
     assert segment["attributes"]["sentry.span.source"] == expected_source
@@ -695,27 +687,23 @@ def error():
     events = [item.payload for item in items if item.type == "event"]
     spans = [item.payload for item in items if item.type == "span"]
     assert len(events) == 1
-    assert len(spans) == 2
+    assert len(spans) == 1
 
     error_event = events[0]
-    callback_span = spans[0]
-    segment = spans[1]
+    segment = spans[0]
 
-    # All share the same trace
-    assert callback_span["trace_id"] == error_event["contexts"]["trace"]["trace_id"]
+    # Confirm the same trace is shared
     assert segment["trace_id"] == error_event["contexts"]["trace"]["trace_id"]
 
     # Span hierarchy
     assert segment["is_segment"] is True
     assert "parent_span_id" not in segment
-    assert callback_span["parent_span_id"] == segment["span_id"]
 
-    # Error event span_id points to the callback span (where the exception was raised)
-    assert error_event["contexts"]["trace"]["span_id"] == callback_span["span_id"]
+    # Error event span_id points to the segment span (where the exception was raised)
+    assert error_event["contexts"]["trace"]["span_id"] == segment["span_id"]
 
-    # Span statuses
+    # Span status
     assert segment["status"] == "error"
-    assert callback_span["status"] == "error"
 
     # Bottle mechanism on the error event
     assert error_event["exception"]["values"][0]["mechanism"]["type"] == "bottle"
@@ -755,13 +743,11 @@ def handle():
     sentry_sdk.flush()
 
     spans = [item.payload for item in items]
-    assert len(spans) == 2
+    assert len(spans) == 1
 
-    callback_span = spans[0]
-    segment = spans[1]
+    segment = spans[0]
 
     assert segment["is_segment"] is True
-    assert callback_span["parent_span_id"] == segment["span_id"]
 
     assert segment["status"] == expected_span_status
     assert segment["attributes"]["http.response.status_code"] == status_code
@@ -809,13 +795,11 @@ def handle():
 
     events = [item.payload for item in items if item.type == "event"]
     spans = [item.payload for item in items if item.type == "span"]
-    assert len(spans) == 2
+    assert len(spans) == 1
 
-    callback_span = spans[0]
-    segment = spans[1]
+    segment = spans[0]
 
     assert segment["is_segment"] is True
-    assert callback_span["parent_span_id"] == segment["span_id"]
 
     if should_capture:
         assert len(events) == 1