ref: Move Redis breadcrumbs to integration - #7129
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.
Codecov Results 📊✅ 99265 passed | ⏭️ 6479 skipped | Total: 105744 | Pass Rate: 93.87% | Execution Time: 357m 31s 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 2501 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 89.98% 89.99% +0.01%
==========================================
Files 193 193 —
Lines 24975 24991 +16
Branches 9008 9012 +4
==========================================
+ Hits 22474 22490 +16
- Misses 2501 2501 —
- Partials 1437 1437 —Generated by Codecov Action |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 7472af9. Configure here.
| type="redis", | ||
| category="redis", | ||
| data=breadcrumb_data, | ||
| ) |
There was a problem hiding this comment.
Breadcrumb callbacks can block Redis operations
Medium Severity
The new add_breadcrumb calls run outside capture_internal_exceptions. If before_breadcrumb raises, the exception escapes the wrapper before the underlying Redis command executes, turning telemetry callback failures into application failures.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 7472af9. Configure here.
There was a problem hiding this comment.
This is a good point. Will take care of it in #7133
alexander-alderman-webb
left a comment
alexander-alderman-webb
left a comment
There was a problem hiding this comment.
Redis looks good to me
There was a problem hiding this comment.
These changes are already on master, might need to update the branch
2 participants
Description
Decouple Redis breadcrumbs from span data.
Issues
Closes #7065