fix(telemetry): record task and crew failures instead of reporting them as OK - #6781
fix(telemetry): record task and crew failures instead of reporting them as OK#6781joaomdmoura wants to merge 1 commit into
Conversation
…em as OK
Task and crew failures were indistinguishable from successes in telemetry,
which is why error_count is zero for every month in the downstream
aggregates rather than merely low.
Three separate defects:
1. Task failures were recorded as successes. TaskFailedEvent routed to
Telemetry.task_ended, which calls close_span() - and close_span
unconditionally sets StatusCode.OK. Every failed task was exported as
OK, so no downstream query could ever count one.
2. Crew failures were not recorded at all, and leaked their span.
on_crew_failed never touched telemetry, so a crew that raised left
_execution_span open: never ended, never exported. The failure was
invisible and the span was lost entirely.
3. Some task failures leaked their span too. on_task_failed only ended the
span when source.agent.crew was present, so a task failing without one
was popped from the span map and never closed.
Changes:
- Add close_span_with_error(), which sets StatusCode.ERROR and optionally
records an error_type attribute.
- Add Telemetry.task_failed() and Telemetry.crew_failed(); crew_failed
clears _execution_span so it cannot be double-closed.
- Wire TaskFailedEvent and CrewKickoffFailedEvent to them, closing spans
unconditionally so neither can leak.
- Add optional error_type to TaskFailedEvent and CrewKickoffFailedEvent,
populated with type(e).__name__ at the four emit sites. Defaults to None,
so existing callers are unaffected.
PII: only the exception *class name* is recorded, never the message, which
routinely contains prompts, model output, and credentials. close_span_with_error
drops any value failing str.isidentifier(), so a message cannot be recorded
even if passed by mistake. Tests assert this against six message-shaped
inputs.
Tests: new tests/telemetry/test_failure_instrumentation.py (16 tests) covering
error status, the success/failure distinction, the PII guard, span-leak
regressions for both task and crew, and event backwards compatibility. The
module sets OTEL_SDK_DISABLED explicitly - the suite runs with the SDK
disabled and the root conftest pops the variable on teardown, so tests that
need real spans must not rely on that leak.
Note: total_duration_ms is a separate, pipeline-side issue. The raw `duration`
column is a Go-style string ("2.026641s"), so toInt64OrZero() yields 0 for
99.99% of rows. That fix belongs in the ClickHouse materialized views, not here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UNumDnNbiyw3pv1WakAe6t
📝 WalkthroughWalkthroughCrew and task failure events now include exception class names. Telemetry marks failed spans with error status, records valid error types, clears crew spans, and avoids exception messages. Regression tests cover propagation, filtering, cleanup, and compatibility. ChangesFailure telemetry instrumentation
Sequence Diagram(s)sequenceDiagram
participant CrewOrTaskExecution
participant FailureEvent
participant EventListener
participant Telemetry
participant Span
CrewOrTaskExecution->>FailureEvent: emit failure with error_type
FailureEvent->>EventListener: deliver failure event
EventListener->>Telemetry: call crew_failed or task_failed
Telemetry->>Span: set error status and record valid error_type
Telemetry->>Span: end failed span
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes telemetry instrumentation so task and crew failures are exported as failures (ERROR) instead of being reported as OK or not exported due to leaked spans, enabling downstream error counting and safer failure attribution via error_type.
Changes:
- Add
close_span_with_error()and route task/crew failure paths to close spans withStatusCode.ERROR, optionally recording a sanitizederror_type. - Extend failure events (
TaskFailedEvent,CrewKickoffFailedEvent) with optionalerror_typeand populate it at emit sites. - Add regression tests covering failure status, PII guard behavior, and span-leak prevention.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/crewai/src/crewai/telemetry/utils.py | Adds close_span_with_error() to end spans with ERROR and optionally record sanitized error_type. |
| lib/crewai/src/crewai/telemetry/telemetry.py | Adds Telemetry.task_failed() / Telemetry.crew_failed() to close spans correctly on failures and prevent crew span leaks. |
| lib/crewai/src/crewai/events/event_listener.py | Wires crew/task failure events to the new telemetry failure methods and closes task spans unconditionally to avoid leaks. |
| lib/crewai/src/crewai/events/types/task_events.py | Adds optional error_type field to TaskFailedEvent for safe telemetry categorization. |
| lib/crewai/src/crewai/events/types/crew_events.py | Adds optional error_type field to CrewKickoffFailedEvent for safe telemetry categorization. |
| lib/crewai/src/crewai/task.py | Populates error_type when emitting TaskFailedEvent from sync/async execution paths. |
| lib/crewai/src/crewai/crew.py | Populates error_type when emitting CrewKickoffFailedEvent from sync/async kickoff paths. |
| lib/crewai/tests/telemetry/test_failure_instrumentation.py | Adds regression coverage for failure vs success spans, PII guard, and span-leak scenarios. |
Suppressed comments (1)
lib/crewai/src/crewai/task.py:963
- Using
raise ere-raises the exception with a new traceback starting at this line, which can hide the original failure location. Prefer a bareraiseto preserve the original traceback when propagating the caught exception.
raise e
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self, | ||
| TaskFailedEvent(error=str(e), error_type=type(e).__name__, task=self), | ||
| ) | ||
| raise e |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/tests/telemetry/test_failure_instrumentation.py (1)
1-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a wiring-level test for
on_task_failedwithout an agent crew.The suite tests
Telemetry.task_failedin isolation with aMocktask, and it testsclose_span_with_errorin isolation. It does not testEventListener.on_task_faileditself. This handler is the exact place where the PR removes thesource.agent.crewgate, so add a test that emits aTaskFailedEventfor a task whoseagent.crewisNone(or falsy) through the event bus and confirmsEventListener.execution_spansno longer holds the span, and that the span is closed. This closes the coverage gap for the specific regression this PR fixes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/telemetry/test_failure_instrumentation.py` around lines 1 - 211, Add a wiring-level test for EventListener.on_task_failed that publishes a TaskFailedEvent through the event bus for a task whose agent.crew is None or falsy. Assert the associated span is removed from EventListener.execution_spans and finished with error status, covering the handler path without relying on Telemetry.task_failed directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/crewai/tests/telemetry/test_failure_instrumentation.py`:
- Around line 1-211: Add a wiring-level test for EventListener.on_task_failed
that publishes a TaskFailedEvent through the event bus for a task whose
agent.crew is None or falsy. Assert the associated span is removed from
EventListener.execution_spans and finished with error status, covering the
handler path without relying on Telemetry.task_failed directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e8ff7c9-33c8-47ef-8d75-b6e1a3d291b4
📒 Files selected for processing (8)
lib/crewai/src/crewai/crew.pylib/crewai/src/crewai/events/event_listener.pylib/crewai/src/crewai/events/types/crew_events.pylib/crewai/src/crewai/events/types/task_events.pylib/crewai/src/crewai/task.pylib/crewai/src/crewai/telemetry/telemetry.pylib/crewai/src/crewai/telemetry/utils.pylib/crewai/tests/telemetry/test_failure_instrumentation.py
Why
error_countis zero for every month increw_task_executions_daily_target— going back to at least Sept 2025, across ~230M task executions. That isn't a low error rate, it's broken instrumentation.Root cause: failures were being recorded as successes.
Three separate defects
1. Task failures were recorded as
OK.TaskFailedEventrouted toTelemetry.task_ended, which callsclose_span():Every failed task was exported with
otel.status_code: OK. No downstream query could ever count one — which is exactly what the zeros show.2. Crew failures weren't recorded at all, and leaked their span.
on_crew_failednever touched telemetry. A crew that raised left_execution_spanopen — never ended, never exported. The failure was invisible and the span was lost.3. Some task failures leaked their span too.
on_task_failedonly ended the span whensource.agent.crewwas present. A task failing without one got popped from the span map and never closed.Changes
close_span_with_error()— setsStatusCode.ERROR, optionally recordserror_type.Telemetry.task_failed()andTelemetry.crew_failed().crew_failedclears_execution_spanso it can't be double-closed, and no-ops safely when no span exists (share_crew=False).error_typeonTaskFailedEventandCrewKickoffFailedEvent, populated withtype(e).__name__at the four emit sites. Defaults toNone— existing callers unaffected.Privacy
Only the exception class name is recorded, never the message.
TaskFailedEvent.erroralready carriesstr(e), which routinely contains prompts, model output, and credentials — that field is untouched and never reaches telemetry.close_span_with_errordrops any value failingstr.isidentifier(), so a message can't be recorded even if passed by mistake. Every builtin exception name is a valid identifier, so nothing legitimate is lost. Tested against six message-shaped inputs including one containingsk-live-1234.Verification
End-to-end through the real telemetry path — two successful tasks and one
TimeoutErrorwhose message contained a fake API key:Testing
New
tests/telemetry/test_failure_instrumentation.py— 16 tests covering error status, the success/failure distinction, the PII guard, span-leak regressions for both task and crew, and event backwards compatibility.tests/telemetry/: 47 passed, 1 skipped, stable across repeated randomized runs.tests/events/: 184 passed, 2 failed — both pre-existing onmain(verified by stashing).tests/utilities/test_events.py: 46 passed.tests/a2a/: collection error from a missing optionala2amodule, pre-existing.One thing worth knowing for future telemetry tests: the suite runs with
OTEL_SDK_DISABLED=true, and the root conftest pops it on teardown. So only the first test in a session sees a disabled SDK and silently gets non-recording spans. This module sets it explicitly rather than depending on that leak.Not included:
total_duration_msAlso zero, but it's a pipeline-side bug, not an SDK one. The data is there — the raw
durationcolumn is a Go-style string:toInt64OrZero("2.026641s")returns0, so the MV sums to nothing. The fix belongs in the ClickHouse materialized views:I can't deploy that from here — flagging it so it gets picked up alongside this.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UNumDnNbiyw3pv1WakAe6t
Note
Medium Risk
Changes how execution spans are closed on failure paths in telemetry and the event listener; behavior is well-tested but affects observability pipelines that depend on span status and export.
Overview
Fixes broken failure metrics by closing OpenTelemetry spans with ERROR instead of OK, and by ending spans that were previously leaked or never touched on crew kickoff failure.
Adds
close_span_with_error(), which setsStatusCode.ERRORand optionally recordserror_typeonly when the value passesisidentifier()(so error messages with prompts or secrets never reach telemetry).Telemetry.task_failed()andTelemetry.crew_failed()use this helper; crew failure also clears_execution_spanafter close.TaskFailedEventandCrewKickoffFailedEventgain optionalerror_type(defaultsNone); kickoff and task emit sites settype(e).__name__. The event listener routes task failures totask_failedunconditionally (no longer gated onsource.agent.crew) and callscrew_failedonCrewKickoffFailedEvent.New
tests/telemetry/test_failure_instrumentation.pycovers error vs success spans, PII guard, span-leak regressions, and backwards compatibility.Reviewed by Cursor Bugbot for commit 328e0e3. Bugbot is set up for automated code reviews on this repo. Configure here.