feat(huey): Migrate Huey integration to spans-first tracing#6399
feat(huey): Migrate Huey integration to spans-first tracing#6399ericapisani wants to merge 12 commits into
Conversation
The Huey integration was not properly handling task groups and chords when enqueuing. When a group/chord is enqueued, we would attempt to access the `name` attribute of the group/chord object for the span. They don't have one, causing an AttributeError. Fixes PY-2426 Fixes #6310
Groups and chords weren't introduced until Huey 3.0, so handle that gracefully.
…s set correctly on segments in _capture_exception method
ericapisani
changed the base branch from
master
to
py-2426-fix-group-and-chord-handling
Codecov Results 📊✅ 282 passed | Total: 282 | Pass Rate: 100% | Execution Time: 43.96s All tests are passing successfully. ❌ Patch coverage is 20.00%. Project has 14790 uncovered lines. Files with missing lines (2)
Generated by Codecov Action |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
…to test order of spans generated since that's stable
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 97fb86c. Configure here.
| # Huey also has exceptions that are raised for control flow reasons, not | ||
| # because there's an actual error. This check, similar to the aiohttp one above, | ||
| # is to prevent accidentally overwriting a status of "ok" with "error" | ||
| if HueyControlFlowExceptions and isinstance(value, HueyControlFlowExceptions): | ||
| return False |
There was a problem hiding this comment.
Can you break out handling of HueyControlFlowExceptions into it's own PR?
Span first changes look good and it'd be worth reviewing the other changes separately.
| assert event["transaction"] == "maybe_locked_task" | ||
| assert event["tags"]["huey_task_id"] == result.task.id | ||
| assert ( | ||
| event["contexts"]["trace"]["status"] == "aborted" | ||
| if should_be_locked |
There was a problem hiding this comment.
Vacuous assert for unlocked status due to operator precedence in ternary expression
The assertion event["contexts"]["trace"]["status"] == "aborted" if should_be_locked else "ok" is parsed as (event[...]["status"] == "aborted") if should_be_locked else "ok". When should_be_locked=False, it asserts the string literal "ok", which is always truthy — the test never actually checks that the trace status equals "ok" for the unlocked case.
Evidence
- Python parses
X == Y if C else Zas(X == Y) if C else Z, notX == (Y if C else Z). - When
should_be_locked=False, the ternary resolves to the string"ok", soassert "ok"is trivially true. - The
lock_name=="lock.b"/should_be_locked=Falseparametrize branch therefore passes even if the integration returns the wrong status (e.g."aborted"). - The correct form is
event[...]["status"] == ("aborted" if should_be_locked else "ok").
Identified by Warden find-bugs · YVJ-ZBD
There was a problem hiding this comment.
Fix attempt detected (commit 2814292)
The after code still has the same operator precedence bug: assert event[...]['status'] == 'aborted' if should_be_locked else 'ok' parses identically to the buggy version, missing the required parentheses around the ternary.
The original issue appears unresolved. Please review and try again.
Evaluated by Warden
…t huey has in earlier versions that results in more events being emitted than a later version)
2 participants
Builds on a bug fix (PY-2426/ #6392 )
Migrates the Huey integration to support the new spans-first tracing lifecycle (
trace_lifecycle: "stream"), while keeping full backwards compatibility with the existing transaction-based path.Fixes PY-2331 and #6029