-
Notifications
You must be signed in to change notification settings - Fork 649
ref(stdlib): Move crumbs to integration #7161
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
Changes from all commits
07a3ea3
2f91234
c3aea86
15d2e2f
53ab7f0
0ea13df
2f54487
8e6087f
91fe6a4
5858c87
906838b
4e093aa
040f223
7472af9
8ace993
f492ba4
8e3adaf
2a172c1
1239d84
9adbede
abdc32b
d03072b
689d9d1
8bfb879
405fb12
31486cf
ccf1de9
0de51ad
1bdf216
62ca144
86c8163
579d224
3e992fb
8ca14b3
720071a
f1312f7
58c435a
e31a3aa
e32ac38
9c2b924
0d8e4ca
a640d16
d68e635
a793322
0db0743
be9520a
a44e3e1
1e27316
5df68a9
2af3694
2e0281a
60870a2
db40334
33a8c0c
9c63035
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| from sentry_sdk.tracing import Span | ||
| from sentry_sdk.tracing_utils import ( | ||
| EnvironHeaders, | ||
| add_http_breadcrumb, | ||
| add_http_request_source, | ||
| has_span_streaming_enabled, | ||
| should_propagate_trace, | ||
|
|
@@ -112,11 +113,21 @@ | |
| parsed_url = parse_url(real_url, sanitize=False) | ||
|
|
||
| span_streaming = has_span_streaming_enabled(client.options) | ||
| span: "Union[Span, StreamedSpan, None]" | ||
| span: "Union[Span, StreamedSpan, None]" = None | ||
| breadcrumb: "dict[str, Any]" = {} | ||
|
|
||
| if span_streaming: | ||
| if sentry_sdk.traces.get_current_span() is None: | ||
| span = None | ||
| else: | ||
| breadcrumb[SPANDATA.HTTP_REQUEST_METHOD] = method | ||
| if parsed_url is not None and should_send_default_pii(): | ||
| breadcrumb.update( | ||
| { | ||
| SPANDATA.URL_FRAGMENT: parsed_url.fragment, | ||
| SPANDATA.URL_FULL: parsed_url.url, | ||
| SPANDATA.URL_QUERY: parsed_url.query, | ||
| } | ||
| ) | ||
|
|
||
| if sentry_sdk.traces.get_current_span() is not None: | ||
| span = sentry_sdk.traces.start_span( | ||
| name="%s %s" | ||
| % ( | ||
|
|
@@ -136,6 +147,7 @@ | |
| span.set_attribute(SPANDATA.URL_QUERY, parsed_url.query) | ||
|
|
||
| set_on_span = span.set_attribute | ||
|
|
||
| else: | ||
| span = sentry_sdk.start_span( | ||
| op=OP.HTTP_CLIENT, | ||
|
|
@@ -145,17 +157,35 @@ | |
| ) | ||
|
|
||
| span.set_data(SPANDATA.HTTP_METHOD, method) | ||
| breadcrumb[SPANDATA.HTTP_METHOD] = method | ||
|
|
||
| if parsed_url is not None: | ||
| span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment) | ||
| span.set_data("url", parsed_url.url) | ||
| span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query) | ||
|
|
||
| breadcrumb.update( | ||
| { | ||
| SPANDATA.HTTP_FRAGMENT: parsed_url.fragment, | ||
| "url": parsed_url.url, | ||
| SPANDATA.HTTP_QUERY: parsed_url.query, | ||
|
Check failure on line 171 in sentry_sdk/integrations/stdlib.py
|
||
| } | ||
| ) | ||
|
|
||
| set_on_span = span.set_data | ||
|
|
||
| # for proxies, these point to the proxy host/port | ||
| if span and tunnel_host: | ||
| set_on_span(SPANDATA.NETWORK_PEER_ADDRESS, self.host) | ||
| set_on_span(SPANDATA.NETWORK_PEER_PORT, self.port) | ||
| if tunnel_host: | ||
| if span: | ||
| set_on_span(SPANDATA.NETWORK_PEER_ADDRESS, self.host) | ||
| set_on_span(SPANDATA.NETWORK_PEER_PORT, self.port) | ||
|
|
||
| breadcrumb.update( | ||
| { | ||
| SPANDATA.NETWORK_PEER_ADDRESS: self.host, | ||
| SPANDATA.NETWORK_PEER_PORT: self.port, | ||
| } | ||
| ) | ||
|
|
||
| rv = real_putrequest(self, method, url, *args, **kwargs) | ||
|
|
||
|
|
@@ -174,28 +204,46 @@ | |
| self.putheader(key, value) | ||
|
|
||
| self._sentrysdk_span = span # type: ignore[attr-defined] | ||
| self._sentrysdk_breadcrumb = breadcrumb # type: ignore[attr-defined] | ||
|
|
||
| return rv | ||
|
|
||
| def getresponse(self: "HTTPConnection", *args: "Any", **kwargs: "Any") -> "Any": | ||
| span = getattr(self, "_sentrysdk_span", None) | ||
|
|
||
| if span is None: | ||
| return real_getresponse(self, *args, **kwargs) | ||
| breadcrumb = getattr(self, "_sentrysdk_breadcrumb", None) | ||
|
|
||
| try: | ||
| rv = real_getresponse(self, *args, **kwargs) | ||
| except BaseException: | ||
| _complete_span(span) | ||
| except BaseException as ex: | ||
| if span: | ||
| _complete_span(span) | ||
| if ( | ||
| breadcrumb | ||
| and "getresponse() got an unexpected keyword argument 'buffering'" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This exception is basically used for control flow/compatibility in old urllib3.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Also, we can get rid of this once we drop 3.6, it's only really a problem in super old requests on Python 3.6.) |
||
| not in str(ex) | ||
| ): | ||
| # the exception msg check is needed for Python 3.6/requests compat | ||
| add_http_breadcrumb(None, breadcrumb) | ||
| raise | ||
|
|
||
| status_code = int(rv.status) | ||
|
|
||
| if breadcrumb: | ||
| breadcrumb[SPANDATA.HTTP_STATUS_CODE] = status_code | ||
|
|
||
| if span is None: | ||
| if breadcrumb: | ||
| add_http_breadcrumb(status_code, breadcrumb) | ||
| return rv | ||
|
sentrivana marked this conversation as resolved.
|
||
|
|
||
| if isinstance(span, StreamedSpan): | ||
| status_code = int(rv.status) | ||
| span.status = "error" if status_code >= 400 else "ok" | ||
| span.set_attribute("http.response.status_code", status_code) | ||
| else: | ||
| span.set_http_status(int(rv.status)) | ||
| span.set_attribute(SPANDATA.HTTP_STATUS_CODE, status_code) | ||
| elif isinstance(span, Span): | ||
| span.set_http_status(status_code) | ||
| span.set_data("reason", rv.reason) | ||
| if breadcrumb: | ||
| breadcrumb["reason"] = rv.reason | ||
|
|
||
| # getresponse doesn't include actually reading the response body. This | ||
| # is done in read(). So if the metadata/headers suggest there's a body to | ||
|
|
@@ -206,6 +254,11 @@ | |
| else: | ||
| _complete_span(span) | ||
|
|
||
| if breadcrumb: | ||
| # Regardless of whether the response itself has been fully read or not, | ||
| # the breadcrumb can now be emitted since we now have the status code. | ||
| add_http_breadcrumb(status_code, breadcrumb) | ||
|
|
||
| return rv | ||
|
|
||
| def read(self: "HTTPResponse", *args: "Any", **kwargs: "Any") -> "Any": | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sensitive URL data added to breadcrumbs without PII check
The non-span_streaming path adds URL fragment, full URL, and query to breadcrumbs without checking should_send_default_pii(), while the span_streaming path gates the same fields behind that check.
Evidence
should_send_default_pii()before addingparsed_url.fragment,parsed_url.url, andparsed_url.queryto the breadcrumb dict.elsebranch at lines 165-171 updatesbreadcrumbwithSPANDATA.HTTP_FRAGMENT,"url", andSPANDATA.HTTP_QUERYwithout any PII guard.add_http_breadcrumb, emitting potentially sensitive query parameters and URL fragments even when default PII collection is disabled.Identified by Warden · skill-scanner · DBV-UKK