Skip to content

Commit 25a953e

Browse files
ericapisaniericapisani
committed
Address code review comments
1 parent bf39690 commit 25a953e

3 files changed

Lines changed: 159 additions & 125 deletions

File tree

‎sentry_sdk/integrations/httpx.py‎

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
import sentry_sdk
2-
from sentry_sdk import start_span as legacy_start_span
32
from sentry_sdk.consts import OP, SPANDATA
43
from sentry_sdk.integrations import Integration, DidNotEnable
5-
from sentry_sdk.traces import start_span
64
from sentry_sdk.tracing import BAGGAGE_HEADER_NAME
75
from sentry_sdk.tracing_utils import (
8-
add_http_request_source_for_streamed_span,
9-
should_propagate_trace,
106
add_http_request_source,
7+
should_propagate_trace,
118
add_sentry_baggage_to_headers,
129
has_span_streaming_enabled,
1310
)
@@ -61,7 +58,7 @@ def send(self: "Client", request: "Request", **kwargs: "Any") -> "Response":
6158
parsed_url = parse_url(str(request.url), sanitize=False)
6259

6360
if is_span_streaming_enabled:
64-
with start_span(
61+
with sentry_sdk.traces.start_span(
6562
name="%s %s"
6663
% (
6764
request.method,
@@ -70,15 +67,15 @@ def send(self: "Client", request: "Request", **kwargs: "Any") -> "Response":
7067
attributes={
7168
"sentry.op": OP.HTTP_CLIENT,
7269
"sentry.origin": HttpxIntegration.origin,
73-
SPANDATA.HTTP_METHOD: request.method,
70+
"http.request.method": request.method,
7471
},
75-
) as segment:
72+
) as streamed_span:
7673
attributes: "Attributes" = {}
7774

7875
if parsed_url is not None:
79-
attributes["url"] = parsed_url.url
80-
attributes[SPANDATA.HTTP_QUERY] = parsed_url.query
81-
attributes[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment
76+
attributes["url.full"] = parsed_url.url
77+
attributes["url.query"] = parsed_url.query
78+
attributes["url.fragment"] = parsed_url.fragment
8279

8380
if should_propagate_trace(client, str(request.url)):
8481
for (
@@ -98,17 +95,17 @@ def send(self: "Client", request: "Request", **kwargs: "Any") -> "Response":
9895

9996
rv = real_send(self, request, **kwargs)
10097

101-
segment.status = "error" if rv.status_code >= 400 else "ok"
102-
attributes[SPANDATA.HTTP_STATUS_CODE] = rv.status_code
98+
streamed_span.status = "error" if rv.status_code >= 400 else "ok"
99+
attributes["http.response.status_code"] = rv.status_code
103100

104-
segment.set_attributes(attributes)
101+
streamed_span.set_attributes(attributes)
105102

106103
# Needs to happen within the context manager as we want to attach the
107104
# final data before the span finishes and is sent for ingesting.
108105
with capture_internal_exceptions():
109-
add_http_request_source_for_streamed_span(segment)
106+
add_http_request_source(streamed_span)
110107
else:
111-
with legacy_start_span(
108+
with sentry_sdk.start_span(
112109
op=OP.HTTP_CLIENT,
113110
name="%s %s"
114111
% (
@@ -168,7 +165,7 @@ async def send(
168165
parsed_url = parse_url(str(request.url), sanitize=False)
169166

170167
if is_span_streaming_enabled:
171-
with start_span(
168+
with sentry_sdk.traces.start_span(
172169
name="%s %s"
173170
% (
174171
request.method,
@@ -177,15 +174,17 @@ async def send(
177174
attributes={
178175
"sentry.op": OP.HTTP_CLIENT,
179176
"sentry.origin": HttpxIntegration.origin,
180-
SPANDATA.HTTP_METHOD: request.method,
177+
"http.request.method": request.method,
181178
},
182-
) as segment:
179+
) as streamed_span:
183180
attributes: "Attributes" = {}
184181

185182
if parsed_url is not None:
186-
attributes["url"] = parsed_url.url
187-
attributes[SPANDATA.HTTP_QUERY] = parsed_url.query
188-
attributes[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment
183+
attributes["url.full"] = parsed_url.url
184+
if parsed_url.query:
185+
attributes["url.query"] = parsed_url.query
186+
if parsed_url.fragment:
187+
attributes["url.fragment"] = parsed_url.fragment
189188

190189
if should_propagate_trace(client, str(request.url)):
191190
for (
@@ -205,17 +204,17 @@ async def send(
205204

206205
rv = await real_send(self, request, **kwargs)
207206

208-
segment.status = "error" if rv.status_code >= 400 else "ok"
209-
attributes[SPANDATA.HTTP_STATUS_CODE] = rv.status_code
207+
streamed_span.status = "error" if rv.status_code >= 400 else "ok"
208+
attributes["http.response.status_code"] = rv.status_code
210209

211-
segment.set_attributes(attributes)
210+
streamed_span.set_attributes(attributes)
212211

213212
# Needs to happen within the context manager as we want to attach the
214213
# final data before the span finishes and is sent for ingesting.
215214
with capture_internal_exceptions():
216-
add_http_request_source_for_streamed_span(segment)
215+
add_http_request_source(streamed_span)
217216
else:
218-
with legacy_start_span(
217+
with sentry_sdk.start_span(
219218
op=OP.HTTP_CLIENT,
220219
name="%s %s"
221220
% (

‎sentry_sdk/tracing_utils.py‎

Lines changed: 30 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -278,17 +278,14 @@ def add_source(
278278
if isinstance(span, LegacySpan):
279279
span.set_data(SPANDATA.CODE_LINENO, lineno)
280280
else:
281-
span.set_attribute(SPANDATA.CODE_LINENO, lineno)
281+
span.set_attribute("code.line.number", lineno)
282282

283283
try:
284284
namespace = frame.f_globals.get("__name__")
285285
except Exception:
286286
namespace = None
287-
if namespace is not None:
288-
if isinstance(span, LegacySpan):
289-
span.set_data(SPANDATA.CODE_NAMESPACE, namespace)
290-
else:
291-
span.set_attribute(SPANDATA.CODE_NAMESPACE, namespace)
287+
if namespace is not None and isinstance(span, LegacySpan):
288+
span.set_data(SPANDATA.CODE_NAMESPACE, namespace)
292289

293290
filepath = _get_frame_module_abs_path(frame)
294291
if filepath is not None:
@@ -303,7 +300,7 @@ def add_source(
303300
span.set_data(SPANDATA.CODE_FILEPATH, in_app_path)
304301
else:
305302
if in_app_path is not None:
306-
span.set_attribute(SPANDATA.CODE_FILEPATH, in_app_path)
303+
span.set_attribute("code.file.path", in_app_path)
307304

308305
try:
309306
code_function = frame.f_code.co_name
@@ -314,7 +311,7 @@ def add_source(
314311
if isinstance(span, LegacySpan):
315312
span.set_data(SPANDATA.CODE_FUNCTION, frame.f_code.co_name)
316313
else:
317-
span.set_attribute(SPANDATA.CODE_FUNCTION, frame.f_code.co_name)
314+
span.set_attribute("code.function.name", frame.f_code.co_name)
318315

319316

320317
def add_query_source(span: "sentry_sdk.tracing.Span") -> None:
@@ -347,31 +344,44 @@ def add_query_source(span: "sentry_sdk.tracing.Span") -> None:
347344
)
348345

349346

350-
def add_http_request_source_for_streamed_span(
351-
span: "sentry_sdk.traces.StreamedSpan",
347+
def add_http_request_source(
348+
span: "Union[sentry_sdk.tracing.Span, sentry_sdk.traces.StreamedSpan]",
352349
) -> None:
353350
"""
354-
Adds OTel compatible source code information to a span for an outgoing HTTP request.
355-
356-
This is intended to be used with StreamedSpans, not legacy Spans.
357-
358-
StreamedSpans need to have this information added before the span finishes, which
359-
is why some of the checks that exist in `add_http_request_source` are not present here.
351+
Adds OTel compatible source code information to a span for an outgoing HTTP request
360352
"""
361353
client = sentry_sdk.get_client()
362354

355+
if isinstance(span, LegacySpan):
356+
if not client.is_active():
357+
return
358+
359+
# In the StreamedSpan case, we need to add the extra span information before
360+
# the span finishes, so it's expected that this will be None. In the LegacySpan case,
361+
# it should already be finished.
362+
if span.timestamp is None:
363+
return
364+
363365
if span.start_timestamp is None:
364366
return
365367

366368
should_add_request_source = client.options.get("enable_http_request_source", True)
367369
if not should_add_request_source:
368370
return
369371

370-
if span.start_timestamp_monotonic_ns is not None:
371-
elapsed = nanosecond_time() - span.start_timestamp_monotonic_ns
372-
end_timestamp = span.start_timestamp + timedelta(microseconds=elapsed / 1000)
372+
if span.timestamp is None:
373+
if (
374+
isinstance(span, sentry_sdk.traces.StreamedSpan)
375+
and span.start_timestamp_monotonic_ns is not None
376+
):
377+
elapsed = nanosecond_time() - span.start_timestamp_monotonic_ns
378+
end_timestamp = span.start_timestamp + timedelta(
379+
microseconds=elapsed / 1000
380+
)
381+
else:
382+
end_timestamp = datetime.now(timezone.utc)
373383
else:
374-
end_timestamp = datetime.now(timezone.utc)
384+
end_timestamp = span.timestamp
375385

376386
duration = end_timestamp - span.start_timestamp
377387
threshold = client.options.get("http_request_source_threshold_ms", 0)
@@ -388,36 +398,6 @@ def add_http_request_source_for_streamed_span(
388398
)
389399

390400

391-
def add_http_request_source(span: "sentry_sdk.tracing.Span") -> None:
392-
"""
393-
Adds OTel compatible source code information to a span for an outgoing HTTP request
394-
"""
395-
client = sentry_sdk.get_client()
396-
if not client.is_active():
397-
return
398-
399-
if span.timestamp is None or span.start_timestamp is None:
400-
return
401-
402-
should_add_request_source = client.options.get("enable_http_request_source", True)
403-
if not should_add_request_source:
404-
return
405-
406-
duration = span.timestamp - span.start_timestamp
407-
threshold = client.options.get("http_request_source_threshold_ms", 0)
408-
slow_query = duration / timedelta(milliseconds=1) > threshold
409-
410-
if not slow_query:
411-
return
412-
413-
add_source(
414-
span=span,
415-
project_root=client.options["project_root"],
416-
in_app_include=client.options.get("in_app_include"),
417-
in_app_exclude=client.options.get("in_app_exclude"),
418-
)
419-
420-
421401
def extract_sentrytrace_data(
422402
header: "Optional[str]",
423403
) -> "Optional[Dict[str, Union[str, bool, None]]]":

0 commit comments

Comments
 (0)