Skip to content

ref(stdlib): Move crumbs to integration - #7161

Merged
sentrivana merged 55 commits into
masterfrom
ivana/move-http-crumbs-4
Aug 12, 2026
Merged

ref(stdlib): Move crumbs to integration#7161
sentrivana merged 55 commits into
masterfrom
ivana/move-http-crumbs-4

Conversation

@sentrivana

@sentrivana sentrivana commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

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:

  • Add tests to both the standard library test suite and requests.

Issues

Part of #7067

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

112376 passed | ⏭️ 6759 skipped | Total: 119135 | Pass Rate: 94.33% | Execution Time: 405m 24s

📊 Comparison with Base Branch

Metric Change
Total Tests 📈 +242
Passed Tests 📈 +232
Failed Tests
Skipped Tests 📈 +10

All tests are passing successfully.

✅ Patch coverage is 100.00%. Project has 2494 uncovered lines.
❌ Project coverage is 90.17%. Comparing base (base) to head (head).

Files with missing lines (1)
File Patch % Lines
sentry_sdk/integrations/stdlib.py 100.00% ⚠️ 4 partials
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        +7

Generated by Codecov Action

_complete_span(span)
if (
breadcrumb
and "getresponse() got an unexpected keyword argument 'buffering'"

@sentrivana sentrivana Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.)

@sentrivana
sentrivana marked this pull request as ready for review August 11, 2026 08:56
@sentrivana
sentrivana requested a review from a team as a code owner August 11, 2026 08:56

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread sentry_sdk/integrations/stdlib.py
Comment thread sentry_sdk/tracing.py
@sentrivana
sentrivana marked this pull request as draft August 11, 2026 09:03
@sentrivana
sentrivana marked this pull request as ready for review August 11, 2026 09:26
Base automatically changed from ivana/move-http-crumbs-3 to master August 12, 2026 12:21
@sentrivana
sentrivana enabled auto-merge (squash) August 12, 2026 12:25
@sentrivana
sentrivana merged commit 0f0cd1f into master Aug 12, 2026
265 of 267 checks passed
@sentrivana
sentrivana deleted the ivana/move-http-crumbs-4 branch August 12, 2026 12:41
Comment on lines 165 to +171
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,

Copy link
Copy Markdown
Contributor

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
  • 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants