Skip to content

Set level based on status code for HTTP client breadcrumbs #4004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 11, 2025
Merged
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
18 changes: 16 additions & 2 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,27 @@ def record_sql_queries(

def maybe_create_breadcrumbs_from_span(scope, span):
# type: (sentry_sdk.Scope, sentry_sdk.tracing.Span) -> None

if span.op == OP.DB_REDIS:
scope.add_breadcrumb(
message=span.description, type="redis", category="redis", data=span._tags
)

elif span.op == OP.HTTP_CLIENT:
scope.add_breadcrumb(type="http", category="httplib", data=span._data)
level = None
status_code = span._data.get(SPANDATA.HTTP_STATUS_CODE)
if status_code:
if 500 <= status_code <= 599:
level = "error"
elif 400 <= status_code <= 499:
level = "warning"

if level:
scope.add_breadcrumb(
type="http", category="httplib", data=span._data, level=level
)
else:
scope.add_breadcrumb(type="http", category="httplib", data=span._data)
Comment on lines +173 to +178
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mess up the breadcrumb if we would add it with level=None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think but wasn't entirely sure so opted for the safe version 😬


elif span.op == "subprocess":
scope.add_breadcrumb(
type="subprocess",
Expand Down
10 changes: 8 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -587,8 +587,14 @@ def suppress_deprecation_warnings():

class MockServerRequestHandler(BaseHTTPRequestHandler):
def do_GET(self): # noqa: N802
# Process an HTTP GET request and return a response with an HTTP 200 status.
self.send_response(200)
# Process an HTTP GET request and return a response.
# If the path ends with /status/<number>, return status code <number>.
# Otherwise return a 200 response.
code = 200
if "/status/" in self.path:
code = int(self.path[-3:])

self.send_response(code)
self.end_headers()
return

Expand Down
55 changes: 55 additions & 0 deletions tests/integrations/aiohttp/test_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,61 @@ async def handler(request):
)


@pytest.mark.parametrize(
"status_code,level",
[
(200, None),
(301, None),
(403, "warning"),
(405, "warning"),
(500, "error"),
],
)
@pytest.mark.asyncio
async def test_crumb_capture_client_error(
sentry_init,
aiohttp_raw_server,
aiohttp_client,
event_loop,
capture_events,
status_code,
level,
):
sentry_init(integrations=[AioHttpIntegration()])

async def handler(request):
return web.Response(status=status_code)

raw_server = await aiohttp_raw_server(handler)

with start_transaction():
events = capture_events()

client = await aiohttp_client(raw_server)
resp = await client.get("/")
assert resp.status == status_code
capture_message("Testing!")

(event,) = events

crumb = event["breadcrumbs"]["values"][0]
assert crumb["type"] == "http"
if level is None:
assert "level" not in crumb
else:
assert crumb["level"] == level
assert crumb["category"] == "httplib"
assert crumb["data"] == ApproxDict(
{
"url": "http://127.0.0.1:{}/".format(raw_server.port),
"http.fragment": "",
"http.method": "GET",
"http.query": "",
"http.response.status_code": status_code,
}
)


@pytest.mark.asyncio
async def test_outgoing_trace_headers(sentry_init, aiohttp_raw_server, aiohttp_client):
sentry_init(
Expand Down
58 changes: 58 additions & 0 deletions tests/integrations/httpx/test_httpx.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,64 @@ def before_breadcrumb(crumb, hint):
)


@pytest.mark.parametrize(
"httpx_client",
(httpx.Client(), httpx.AsyncClient()),
)
@pytest.mark.parametrize(
"status_code,level",
[
(200, None),
(301, None),
(403, "warning"),
(405, "warning"),
(500, "error"),
],
)
def test_crumb_capture_client_error(
sentry_init, capture_events, httpx_client, httpx_mock, status_code, level
):
httpx_mock.add_response(status_code=status_code)

sentry_init(integrations=[HttpxIntegration()])

url = "http://example.com/"

with start_transaction():
events = capture_events()

if asyncio.iscoroutinefunction(httpx_client.get):
response = asyncio.get_event_loop().run_until_complete(
httpx_client.get(url)
)
else:
response = httpx_client.get(url)

assert response.status_code == status_code
capture_message("Testing!")

(event,) = events

crumb = event["breadcrumbs"]["values"][0]
assert crumb["type"] == "http"
assert crumb["category"] == "httplib"

if level is None:
assert "level" not in crumb
else:
assert crumb["level"] == level

assert crumb["data"] == ApproxDict(
{
"url": url,
SPANDATA.HTTP_METHOD: "GET",
SPANDATA.HTTP_FRAGMENT: "",
SPANDATA.HTTP_QUERY: "",
SPANDATA.HTTP_STATUS_CODE: status_code,
}
)


@pytest.mark.parametrize(
"httpx_client",
(httpx.Client(), httpx.AsyncClient()),
Expand Down
61 changes: 53 additions & 8 deletions tests/integrations/requests/test_requests.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,77 @@
import sys
from unittest import mock

import pytest
import requests
import responses

from sentry_sdk import capture_message
from sentry_sdk.consts import SPANDATA
from sentry_sdk.integrations.stdlib import StdlibIntegration
from tests.conftest import ApproxDict
from tests.conftest import ApproxDict, create_mock_http_server

PORT = create_mock_http_server()


def test_crumb_capture(sentry_init, capture_events):
sentry_init(integrations=[StdlibIntegration()])
events = capture_events()

url = "http://example.com/"
responses.add(responses.GET, url, status=200)
url = f"http://localhost:{PORT}/hello-world" # noqa:E231
response = requests.get(url)
capture_message("Testing!")

(event,) = events
(crumb,) = event["breadcrumbs"]["values"]
assert crumb["type"] == "http"
assert crumb["category"] == "httplib"
assert crumb["data"] == ApproxDict(
{
"url": url,
SPANDATA.HTTP_METHOD: "GET",
SPANDATA.HTTP_FRAGMENT: "",
SPANDATA.HTTP_QUERY: "",
SPANDATA.HTTP_STATUS_CODE: response.status_code,
"reason": response.reason,
}
)


@pytest.mark.skipif(
sys.version_info < (3, 7),
reason="The response status is not set on the span early enough in 3.6",
)
@pytest.mark.parametrize(
"status_code,level",
[
(200, None),
(301, None),
(403, "warning"),
(405, "warning"),
(500, "error"),
],
)
def test_crumb_capture_client_error(sentry_init, capture_events, status_code, level):
sentry_init(integrations=[StdlibIntegration()])

events = capture_events()

url = f"http://localhost:{PORT}/status/{status_code}" # noqa:E231
response = requests.get(url)

assert response.status_code == status_code

capture_message("Testing!")

(event,) = events
(crumb,) = event["breadcrumbs"]["values"]
assert crumb["type"] == "http"
assert crumb["category"] == "httplib"

if level is None:
assert "level" not in crumb
else:
assert crumb["level"] == level

assert crumb["data"] == ApproxDict(
{
"url": url,
Expand All @@ -41,11 +88,10 @@ def test_crumb_capture(sentry_init, capture_events):
def test_omit_url_data_if_parsing_fails(sentry_init, capture_events):
sentry_init(integrations=[StdlibIntegration()])

url = "https://example.com"
responses.add(responses.GET, url, status=200)

events = capture_events()

url = f"http://localhost:{PORT}/ok" # noqa:E231

with mock.patch(
"sentry_sdk.integrations.stdlib.parse_url",
side_effect=ValueError,
Expand All @@ -63,7 +109,6 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events):
# no url related data
}
)

assert "url" not in event["breadcrumbs"]["values"][0]["data"]
assert SPANDATA.HTTP_FRAGMENT not in event["breadcrumbs"]["values"][0]["data"]
assert SPANDATA.HTTP_QUERY not in event["breadcrumbs"]["values"][0]["data"]
45 changes: 45 additions & 0 deletions tests/integrations/stdlib/test_httplib.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import random
from http.client import HTTPConnection, HTTPSConnection
from socket import SocketIO
from urllib.error import HTTPError
from urllib.request import urlopen
from unittest import mock

Expand Down Expand Up @@ -42,6 +43,50 @@ def test_crumb_capture(sentry_init, capture_events):
)


@pytest.mark.parametrize(
"status_code,level",
[
(200, None),
(301, None),
(403, "warning"),
(405, "warning"),
(500, "error"),
],
)
def test_crumb_capture_client_error(sentry_init, capture_events, status_code, level):
sentry_init(integrations=[StdlibIntegration()])
events = capture_events()

url = f"http://localhost:{PORT}/status/{status_code}" # noqa:E231
try:
urlopen(url)
except HTTPError:
pass

capture_message("Testing!")

(event,) = events
(crumb,) = event["breadcrumbs"]["values"]

assert crumb["type"] == "http"
assert crumb["category"] == "httplib"

if level is None:
assert "level" not in crumb
else:
assert crumb["level"] == level

assert crumb["data"] == ApproxDict(
{
"url": url,
SPANDATA.HTTP_METHOD: "GET",
SPANDATA.HTTP_STATUS_CODE: status_code,
SPANDATA.HTTP_FRAGMENT: "",
SPANDATA.HTTP_QUERY: "",
}
)


def test_crumb_capture_hint(sentry_init, capture_events):
def before_breadcrumb(crumb, hint):
crumb["data"]["extra"] = "foo"
Expand Down
Loading