Skip to content

Commit 80e2109

Browse files
cursoragentclaude
andcommitted
fix(pyreqwest): prevent middleware stacking and dedupe middleware span logic
Co-Authored-By: gpt-5.3-codex-high <noreply@anthropic.com>
1 parent 2d5bf72 commit 80e2109

File tree

2 files changed

+120
-52
lines changed

2 files changed

+120
-52
lines changed

sentry_sdk/integrations/pyreqwest.py

Lines changed: 57 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import sentry_sdk
2+
from collections import deque
3+
from contextlib import contextmanager
24
from sentry_sdk import start_span
35
from sentry_sdk.consts import OP, SPANDATA
46
from sentry_sdk.integrations import Integration, DidNotEnable
@@ -16,16 +18,23 @@
1618
)
1719

1820
from typing import TYPE_CHECKING
21+
from weakref import WeakSet
1922

2023
if TYPE_CHECKING:
2124
from typing import Any
25+
from typing import Iterator
2226

2327

2428
import importlib.util
2529

2630
if importlib.util.find_spec("pyreqwest") is None:
2731
raise DidNotEnable("pyreqwest is not installed")
2832

33+
_MAX_TRACKED_NON_WEAKREF_BUILDERS = 2048
34+
_instrumented_builders = WeakSet() # type: "WeakSet[Any]"
35+
_instrumented_non_weakref_builder_ids = set() # type: "set[int]"
36+
_instrumented_non_weakref_builder_ids_order = deque() # type: "deque[int]"
37+
2938

3039
class PyreqwestIntegration(Integration):
3140
identifier = "pyreqwest"
@@ -74,24 +83,52 @@ def _patch_builder_method(cls: type, method_name: str, middleware: "Any") -> Non
7483
original_method = getattr(cls, method_name)
7584

7685
def sentry_patched_method(self: "Any", *args: "Any", **kwargs: "Any") -> "Any":
77-
if not getattr(self, "_sentry_instrumented", False):
86+
if not _builder_is_instrumented(self):
7887
integration = sentry_sdk.get_client().get_integration(PyreqwestIntegration)
7988
if integration is not None:
8089
self.with_middleware(middleware)
81-
try:
82-
self._sentry_instrumented = True
83-
except (TypeError, AttributeError):
84-
# In case the instance itself is immutable or doesn't allow extra attributes
85-
pass
90+
_mark_builder_instrumented(self)
8691
return original_method(self, *args, **kwargs)
8792

8893
setattr(cls, method_name, sentry_patched_method)
8994

9095

91-
async def sentry_async_middleware(request: "Any", next_handler: "Any") -> "Any":
92-
if sentry_sdk.get_client().get_integration(PyreqwestIntegration) is None:
93-
return await next_handler.run(request)
96+
def _builder_is_instrumented(builder: "Any") -> bool:
97+
if getattr(builder, "_sentry_instrumented", False):
98+
return True
99+
100+
if id(builder) in _instrumented_non_weakref_builder_ids:
101+
return True
102+
103+
try:
104+
return builder in _instrumented_builders
105+
except TypeError:
106+
return False
107+
94108

109+
def _mark_builder_instrumented(builder: "Any") -> None:
110+
try:
111+
_instrumented_builders.add(builder)
112+
except TypeError:
113+
builder_id = id(builder)
114+
if builder_id not in _instrumented_non_weakref_builder_ids:
115+
_instrumented_non_weakref_builder_ids.add(builder_id)
116+
_instrumented_non_weakref_builder_ids_order.append(builder_id)
117+
if (
118+
len(_instrumented_non_weakref_builder_ids_order)
119+
> _MAX_TRACKED_NON_WEAKREF_BUILDERS
120+
):
121+
old_builder_id = _instrumented_non_weakref_builder_ids_order.popleft()
122+
_instrumented_non_weakref_builder_ids.discard(old_builder_id)
123+
124+
try:
125+
builder._sentry_instrumented = True
126+
except (TypeError, AttributeError):
127+
pass
128+
129+
130+
@contextmanager
131+
def _sentry_middleware_span(request: "Any") -> "Iterator[Any]":
95132
parsed_url = None
96133
with capture_internal_exceptions():
97134
parsed_url = parse_url(str(request.url), sanitize=False)
@@ -127,60 +164,29 @@ async def sentry_async_middleware(request: "Any", next_handler: "Any") -> "Any":
127164
else:
128165
request.headers[key] = value
129166

130-
response = await next_handler.run(request)
131-
132-
span.set_http_status(response.status)
167+
yield span
133168

134169
with capture_internal_exceptions():
135170
add_http_request_source(span)
136171

137-
return response
138-
139172

140-
def sentry_sync_middleware(request: "Any", next_handler: "Any") -> "Any":
173+
async def sentry_async_middleware(request: "Any", next_handler: "Any") -> "Any":
141174
if sentry_sdk.get_client().get_integration(PyreqwestIntegration) is None:
142-
return next_handler.run(request)
175+
return await next_handler.run(request)
143176

144-
parsed_url = None
145-
with capture_internal_exceptions():
146-
parsed_url = parse_url(str(request.url), sanitize=False)
177+
with _sentry_middleware_span(request) as span:
178+
response = await next_handler.run(request)
179+
span.set_http_status(response.status)
147180

148-
with start_span(
149-
op=OP.HTTP_CLIENT,
150-
name="%s %s"
151-
% (
152-
request.method,
153-
parsed_url.url if parsed_url else SENSITIVE_DATA_SUBSTITUTE,
154-
),
155-
origin=PyreqwestIntegration.origin,
156-
) as span:
157-
span.set_data(SPANDATA.HTTP_METHOD, request.method)
158-
if parsed_url is not None:
159-
span.set_data("url", parsed_url.url)
160-
span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query)
161-
span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment)
181+
return response
162182

163-
if should_propagate_trace(sentry_sdk.get_client(), str(request.url)):
164-
for (
165-
key,
166-
value,
167-
) in sentry_sdk.get_current_scope().iter_trace_propagation_headers():
168-
logger.debug(
169-
"[Tracing] Adding `{key}` header {value} to outgoing request to {url}.".format(
170-
key=key, value=value, url=request.url
171-
)
172-
)
173183

174-
if key == BAGGAGE_HEADER_NAME:
175-
add_sentry_baggage_to_headers(request.headers, value)
176-
else:
177-
request.headers[key] = value
184+
def sentry_sync_middleware(request: "Any", next_handler: "Any") -> "Any":
185+
if sentry_sdk.get_client().get_integration(PyreqwestIntegration) is None:
186+
return next_handler.run(request)
178187

188+
with _sentry_middleware_span(request) as span:
179189
response = next_handler.run(request)
180-
181190
span.set_http_status(response.status)
182191

183-
with capture_internal_exceptions():
184-
add_http_request_source(span)
185-
186192
return response

tests/integrations/pyreqwest/test_pyreqwest.py

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99

1010
from sentry_sdk import start_transaction
1111
from sentry_sdk.consts import SPANDATA
12-
from sentry_sdk.integrations.pyreqwest import PyreqwestIntegration
12+
from sentry_sdk.integrations.pyreqwest import (
13+
PyreqwestIntegration,
14+
_patch_builder_method,
15+
sentry_sync_middleware,
16+
)
1317
from tests.conftest import get_free_port
1418

1519

@@ -55,6 +59,64 @@ def clear_captured_requests():
5559
PyreqwestMockHandler.captured_requests.clear()
5660

5761

62+
def test_patch_builder_method_only_adds_middleware_once(monkeypatch):
63+
class DummyClient:
64+
@staticmethod
65+
def get_integration(_integration):
66+
return object()
67+
68+
monkeypatch.setattr(
69+
"sentry_sdk.integrations.pyreqwest.sentry_sdk.get_client", lambda: DummyClient()
70+
)
71+
72+
class Builder:
73+
def __init__(self):
74+
self.middleware_count = 0
75+
76+
def with_middleware(self, _middleware):
77+
self.middleware_count += 1
78+
return self
79+
80+
def send(self):
81+
return self.middleware_count
82+
83+
_patch_builder_method(Builder, "send", sentry_sync_middleware)
84+
85+
builder = Builder()
86+
assert builder.send() == 1
87+
assert builder.send() == 1
88+
89+
90+
def test_patch_builder_method_instruments_immutable_builder_once(monkeypatch):
91+
class DummyClient:
92+
@staticmethod
93+
def get_integration(_integration):
94+
return object()
95+
96+
monkeypatch.setattr(
97+
"sentry_sdk.integrations.pyreqwest.sentry_sdk.get_client", lambda: DummyClient()
98+
)
99+
100+
class ImmutableBuilder:
101+
__slots__ = ("middleware_count",)
102+
103+
def __init__(self):
104+
self.middleware_count = 0
105+
106+
def with_middleware(self, _middleware):
107+
self.middleware_count += 1
108+
return self
109+
110+
def send(self):
111+
return self.middleware_count
112+
113+
_patch_builder_method(ImmutableBuilder, "send", sentry_sync_middleware)
114+
115+
builder = ImmutableBuilder()
116+
assert builder.send() == 1
117+
assert builder.send() == 1
118+
119+
58120
@pytest.mark.skipif(pyreqwest is None, reason="pyreqwest not installed")
59121
def test_sync_client_spans(sentry_init, capture_events, server_port):
60122
sentry_init(integrations=[PyreqwestIntegration()], traces_sample_rate=1.0)

0 commit comments

Comments
 (0)