ref(stdlib): Move crumbs to integration - #7161
Conversation
Sorry, something went wrong.
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.
@@ 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 |
Sorry, something went wrong.
| _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.
Sorry, something went wrong.
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.)
Sorry, something went wrong.
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.
Sorry, something went wrong.
| 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 adding parsed_url.fragment, parsed_url.url, and parsed_url.query to the breadcrumb dict.
- The else branch at lines 165-171 updates breadcrumb with SPANDATA.HTTP_FRAGMENT, "url", and SPANDATA.HTTP_QUERY without 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
Sorry, something went wrong.
Description
Migrate http.client breadcrumbs from maybe_create_breadcrumbs_from_span directly to the StdlibIntegration.
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