Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions sentry_sdk/integrations/bottle.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
)
from sentry_sdk.integrations._wsgi_common import RequestExtractor
from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware
from sentry_sdk.tracing import SOURCE_FOR_STYLE
from sentry_sdk.traces import SOURCE_FOR_STYLE as SEGMENT_SOURCE_FOR_STYLE
from sentry_sdk.tracing import SOURCE_FOR_STYLE as TRANSACTION_SOURCE_FOR_STYLE
from sentry_sdk.tracing_utils import has_span_streaming_enabled
Comment thread
ericapisani marked this conversation as resolved.
from sentry_sdk.utils import (
capture_internal_exceptions,
ensure_integration_enabled,
Expand Down Expand Up @@ -100,7 +102,16 @@
scope.add_event_processor(
_make_request_event_processor(self, bottle_request, integration)
)
res = old_handle(self, environ)

if has_span_streaming_enabled(sentry_sdk.get_client().options):
res = old_handle(self, environ)

_set_segment_name_and_source(
transaction_style=integration.transaction_style
)

Check warning on line 111 in sentry_sdk/integrations/bottle.py

View check run for this annotation

@sentry/warden / warden: code-review

`_set_segment_name_and_source` skipped when `old_handle` raises an exception in streaming path

Wrap the streaming path in a `try/finally` so `_set_segment_name_and_source` is called even when `old_handle` raises (e.g. an unhandled `ZeroDivisionError`), otherwise the segment name is never set for error requests.

Check warning on line 111 in sentry_sdk/integrations/bottle.py

View check run for this annotation

@sentry/warden / warden: find-bugs

`_set_segment_name_and_source` skipped when `old_handle` raises in streaming mode

If `old_handle(self, environ)` raises an exception (e.g. a 404 or unhandled error), `_set_segment_name_and_source` is never called in the streaming path, so the error span will have no transaction name. Wrap the call in `try/finally` to guarantee the name is always set.
Comment thread
ericapisani marked this conversation as resolved.

else:
res = old_handle(self, environ)

return res

Expand Down Expand Up @@ -160,9 +171,28 @@
return self.request.files

def size_of_file(self, file: "FileUpload") -> int:
return file.content_length

Check warning on line 174 in sentry_sdk/integrations/bottle.py

View check run for this annotation

@sentry/warden / warden: code-review

[WWB-Y23] `_set_segment_name_and_source` skipped when `old_handle` raises an exception in streaming path (additional location)

Wrap the streaming path in a `try/finally` so `_set_segment_name_and_source` is called even when `old_handle` raises (e.g. an unhandled `ZeroDivisionError`), otherwise the segment name is never set for error requests.


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:
Expand All @@ -185,7 +215,9 @@
pass

event["transaction"] = name
event["transaction_info"] = {"source": SOURCE_FOR_STYLE[transaction_style]}
event["transaction_info"] = {
"source": TRANSACTION_SOURCE_FOR_STYLE[transaction_style]
}


def _make_request_event_processor(
Expand Down
259 changes: 255 additions & 4 deletions tests/integrations/bottle/test_bottle.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from werkzeug.test import Client
from werkzeug.wrappers import Response

import sentry_sdk
from sentry_sdk import capture_message
from sentry_sdk.integrations.bottle import BottleIntegration
from sentry_sdk.integrations.logging import LoggingIntegration
Expand Down Expand Up @@ -462,23 +463,37 @@
assert not events


@pytest.mark.parametrize("span_streaming", [True, False])
def test_span_origin(
sentry_init,
get_client,
capture_events,
capture_items,
span_streaming,
):
sentry_init(
integrations=[BottleIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream" if span_streaming else "static"},
)
events = capture_events()

if span_streaming:
items = capture_items("span")
else:
events = capture_events()

client = get_client()
client.get("/message")

(_, event) = events

assert event["contexts"]["trace"]["origin"] == "auto.http.bottle"
if span_streaming:
sentry_sdk.flush()
spans = [item.payload for item in items]
segment = spans[-1]
assert segment["is_segment"] is True
assert segment["attributes"]["sentry.origin"] == "auto.http.bottle"
else:
(_, event) = events
assert event["contexts"]["trace"]["origin"] == "auto.http.bottle"


@pytest.mark.parametrize("raise_error", [True, False])
Expand Down Expand Up @@ -556,3 +571,239 @@

(event,) = events
assert event["exception"]["values"][0]["type"] == "ZeroDivisionError"


def test_span_streaming_basic(sentry_init, capture_items):
sentry_init(
integrations=[BottleIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("span")

app = Bottle()

@app.route("/message")
def hi():
return "ok"

client = Client(app)
client.get("/message")

sentry_sdk.flush()

spans = [item.payload for item in items]
assert len(spans) == 1

segment = spans[0]

# Segment span (root, created by WSGI middleware)
assert segment["is_segment"] is True
assert "parent_span_id" not in segment
assert segment["status"] == "ok"
assert segment["attributes"]["sentry.op"] == "http.server"
assert segment["attributes"]["sentry.origin"] == "auto.http.bottle"
assert segment["attributes"]["http.request.method"] == "GET"
assert segment["attributes"]["http.response.status_code"] == 200
assert segment["name"].endswith("hi")


@pytest.mark.parametrize(
"url,transaction_style,expected_name,expected_source",
[
("/message", "endpoint", "hi", "component"),
("/message", "url", "/message", "route"),
("/message/123456", "url", "/message/<message_id>", "route"),
("/message-named-route", "endpoint", "hi", "component"),
],
)
def test_span_streaming_transaction_style(
sentry_init,
capture_items,
url,
transaction_style,
expected_name,
expected_source,
):
sentry_init(
integrations=[BottleIntegration(transaction_style=transaction_style)],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("span")

app = Bottle()

@app.route("/message")
def hi():
return "ok"

@app.route("/message/<message_id>")
def hi_with_id(message_id):
return "ok"

@app.route("/message-named-route", name="hi")
def named_hi():
return "ok"

client = Client(app)
client.get(url)

sentry_sdk.flush()

spans = [item.payload for item in items]
assert len(spans) == 1

segment = spans[0]

assert segment["is_segment"] is True

assert segment["name"].endswith(expected_name)
assert segment["attributes"]["sentry.span.source"] == expected_source


def test_span_streaming_with_error(sentry_init, capture_items):
sentry_init(
integrations=[BottleIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("event", "span")

app = Bottle()

@app.route("/error")
def error():
1 / 0

client = Client(app)
try:
client.get("/error")
except ZeroDivisionError:
pass

sentry_sdk.flush()

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) == 1

error_event = events[0]
segment = spans[0]

# 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

# 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 status
assert segment["status"] == "error"

# Bottle mechanism on the error event
assert error_event["exception"]["values"][0]["mechanism"]["type"] == "bottle"
assert error_event["exception"]["values"][0]["mechanism"]["handled"] is False

Check warning on line 711 in tests/integrations/bottle/test_bottle.py

View check run for this annotation

@sentry/warden / warden: find-bugs

[P2M-TYD] `_set_segment_name_and_source` skipped when `old_handle` raises in streaming mode (additional location)

If `old_handle(self, environ)` raises an exception (e.g. a 404 or unhandled error), `_set_segment_name_and_source` is never called in the streaming path, so the error span will have no transaction name. Wrap the call in `try/finally` to guarantee the name is always set.

@pytest.mark.parametrize(
"status_code,expected_span_status",
[
(200, "ok"),
(404, "error"),
(500, "error"),
],
)
def test_span_streaming_http_error_status(
sentry_init,
capture_items,
status_code,
expected_span_status,
):
sentry_init(
integrations=[BottleIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("span")

app = Bottle()

@app.route("/")
def handle():
return HTTPResponse(status=status_code, body="response")

client = Client(app)
client.get("/")

sentry_sdk.flush()

spans = [item.payload for item in items]
assert len(spans) == 1

segment = spans[0]

assert segment["is_segment"] is True

assert segment["status"] == expected_span_status
assert segment["attributes"]["http.response.status_code"] == status_code


@pytest.mark.parametrize("raise_error", [True, False])
@pytest.mark.parametrize(
("integration_kwargs", "status_code", "should_capture"),
(
({}, 500, True),
({}, 400, False),
({"failed_request_status_codes": set()}, 500, False),
({"failed_request_status_codes": {404, *range(500, 600)}}, 404, True),
({"failed_request_status_codes": {404, *range(500, 600)}}, 400, False),
),
)
def test_span_streaming_failed_request_status_codes(
sentry_init,
capture_items,
integration_kwargs,
status_code,
should_capture,
raise_error,
):
sentry_init(
integrations=[BottleIntegration(**integration_kwargs)],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("event", "span")

app = Bottle()

@app.route("/")
def handle():
response = HTTPResponse(status=status_code)
if raise_error:
raise response
return response

client = Client(app, Response)
client.get("/")

sentry_sdk.flush()

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) == 1

segment = spans[0]

assert segment["is_segment"] is True

if should_capture:
assert len(events) == 1
assert events[0]["exception"]["values"][0]["type"] == "HTTPResponse"
assert events[0]["exception"]["values"][0]["mechanism"]["handled"] is True
else:

Check warning on line 808 in tests/integrations/bottle/test_bottle.py

View check run for this annotation

@sentry/warden / warden: code-review

Test incorrectly asserts `handled=True` for the `raise_error=True` path

When `raise_error=True`, `wrapped_callback` captures the raised `HTTPResponse` via the `except Exception` branch with `handled=False`, so asserting `mechanism["handled"] is True` unconditionally will fail for the `raise_error=True` + `should_capture=True` parametrize combinations. The assertion should be conditioned on `raise_error`.
assert len(events) == 0

Check failure on line 809 in tests/integrations/bottle/test_bottle.py

View check run for this annotation

@sentry/warden / warden: find-bugs

`wrapped_callback` captures raised `HTTPResponse` with `handled=False`, bypassing `failed_request_status_codes`

In `sentry_sdk/integrations/bottle.py`, `wrapped_callback` (lines ~132-145) wraps the route callback. Bottle uses `raise HTTPResponse(...)` as a normal control-flow mechanism for early-exit responses, and `HTTPResponse` is an `Exception` subclass (via `BottleException`). The `except Exception` branch unconditionally invokes `_capture_exception(exception, handled=False)` and re-raises, so: 1. Raised `HTTPResponse` instances are captured even when their `status_code` is not in `integration.failed_request_status_codes` (e.g., 400 with the default config). 2. When the status code does match, the event is reported with `mechanism.handled = False` instead of `True`, contrary to the returned-`HTTPResponse` branch a few lines below which correctly uses `handled=True`. The newly-added test `test_span_streaming_failed_request_status_codes` parametrizes `raise_error=True` and asserts `events[0]['exception']['values'][0]['mechanism']['handled'] is True` for matching codes, and `len(events) == 0` for non-matching codes — both assertions fail against the current implementation. Fix: special-case `HTTPResponse` in `wrapped_callback` — catch it separately, check `res.status_code in integration.failed_request_status_codes`, and call `_capture_exception(res, handled=True)` only when matched, then re-raise (or return) as appropriate.
Loading