ref(stdlib): Move crumbs to integration - #7161
Conversation
Move subprocess breadcrumb creation from the centralized `maybe_create_breadcrumbs_from_span` hook into the stdlib integration's `Popen.__init__` wrapper. This makes breadcrumbs work for both legacy spans and streamed spans, and removes the dependency on span internals.
…-redis-breadcrumbs-to-integration
Codecov Results 📊✅ 112376 passed | ⏭️ 6759 skipped | Total: 119135 | Pass Rate: 94.33% | Execution Time: 405m 24s 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 2494 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 90.22% 90.17% -0.05%
==========================================
Files 193 193 —
Lines 25345 25375 +30
Branches 9286 9312 +26
==========================================
+ Hits 22866 22881 +15
- Misses 2479 2494 +15
- Partials 1427 1434 +7Generated by Codecov Action |
| _complete_span(span) | ||
| if ( | ||
| breadcrumb | ||
| and "getresponse() got an unexpected keyword argument 'buffering'" |
There was a problem hiding this comment.
This exception is basically used for control flow/compatibility in old urllib3. getresponse() will be called again afterwards without the extra arg, which is when we'll emit the breadcrumb. If this extra check were not here, we'd emit one crumb too many.
There was a problem hiding this comment.
(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.)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2e0281a. Configure here.
| 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, |
There was a problem hiding this comment.
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
- The span_streaming branch checks
should_send_default_pii()before addingparsed_url.fragment,parsed_url.url, andparsed_url.queryto the breadcrumb dict. - The
elsebranch at lines 165-171 updatesbreadcrumbwithSPANDATA.HTTP_FRAGMENT,"url", andSPANDATA.HTTP_QUERYwithout any PII guard. - The unguarded breadcrumb is later passed to
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

Description
Migrate
http.clientbreadcrumbs frommaybe_create_breadcrumbs_from_spandirectly to theStdlibIntegration.As the flow is scattered (one point where a request starts, but multiple points where it might end), we save the breadcrumb data alongside the span and emit a breadcrumb whenever we detect a response is finished.
Also:
Issues
Part of #7067