Skip to content

ref(httpx,httpx2): Move crumbs to integrations - #7149

Open
sentrivana wants to merge 8 commits into
ivana/move-http-crumbs-2from
ivana/move-http-crumbs-3
Open

ref(httpx,httpx2): Move crumbs to integrations#7149
sentrivana wants to merge 8 commits into
ivana/move-http-crumbs-2from
ivana/move-http-crumbs-3

Conversation

@sentrivana

@sentrivana sentrivana commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Description

Move breadcrumbs from the centralized, span-powered maybe_create_breadcrumbs_from_span directly to the HTTPX and HTTPX2 integrations. (Put the two together in one PR since they're the same changeset.)

Additional changes and context:

  • Running the async breadcrumb tests with pytest-asyncio, as that simulates how the scopes behave live better than setting up an ad-hoc event loop.
  • Split the crumb tests into span streaming/not span streaming (because of the difference in send_default_pii behavior).

Issues

Reminders

"reason": rv.reason_phrase,
}

if parsed_url and (not is_span_streaming_enabled or should_send_default_pii()):

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.

The is_not_span_streaming_enabled part is there for continuity in transaction mode, where we don't care about should_send_default_pii before setting breadcrumb data.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

105070 passed | ⏭️ 6677 skipped | Total: 111747 | Pass Rate: 94.02% | Execution Time: 358m 14s

📊 Comparison with Base Branch

Metric Change
Total Tests 📈 +1086
Passed Tests 📈 +1086
Failed Tests
Skipped Tests

All tests are passing successfully.

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

Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    90.15%    90.17%    +0.02%
==========================================
  Files          193       193         —
  Lines        25147     25207       +60
  Branches      9136      9160       +24
==========================================
+ Hits         22669     22728       +59
- Misses        2478      2479        +1
- Partials      1431      1431         —

Generated by Codecov Action

sentrivana added a commit that referenced this pull request Aug 10, 2026
Instead of parametrizing on sync/async httpx(2) client, split each test
case into a sync and async variant, with the async variant as a proper
`async def` function with `@pytest.mark.parametrize`.

I did this because the tests routinely fail for me locally, and
switching to using `pytest-asyncio` fixes that. The other reason is that
for #7149, the breadcrumb
tests don't simulate async very well, which leads to some scope problems
and ultimately breadcrumbs not appearing on events in tests.

I know this PR is not ideal since with the existing test duplication on
span streaming/transaction tracing, we already have a LOT of test cases,
and now many of them get an additional variant. But it does make them
more resilient to random local (and I believe also CI) failures and
we'll get rid of half of them on the new major branch.
@sentrivana
sentrivana marked this pull request as ready for review August 10, 2026 15:06
@sentrivana
sentrivana requested a review from a team as a code owner August 10, 2026 15:07
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