feat(tracing): collect skill usage events - #6727
Conversation
PR #6652 added SkillUsedEvent but deliberately shipped no listener wiring, so the event reached no collector. The trace listener subscribed to the five setup events -- discovery, load, activation, failure -- and none of them can answer the question skills observability is for: activation is idempotent and fires once at setup, so an agent using a skill across twenty turns produces exactly one event. SkillUsedEvent is the only runtime signal and the only one that re-fires per execution. Subscribing to it lets a trace attribute skill usage to an agent and a task. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughSkill usage events are now registered with ChangesSkill usage tracing
Sequence Diagram(s)sequenceDiagram
participant SkillUsedEvent
participant CrewAIEventsBus
participant TraceCollectionListener
SkillUsedEvent->>CrewAIEventsBus: emit skill usage event
CrewAIEventsBus->>TraceCollectionListener: invoke registered handler
TraceCollectionListener->>TraceCollectionListener: handle as "skill_used"
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 |
![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 using default effort 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 c66d958. Configure here.
|
|
||
| with patch.object(TraceCollectionListener, "_handle_action_event") as handled: | ||
| listener._register_action_event_handlers(bus) | ||
| yield bus, handled |
There was a problem hiding this comment.
Test leaks handlers on shared bus
Medium Severity
The fixture claims an isolated bus, but CrewAIEventsBus() always returns the process-wide singleton. _register_action_event_handlers therefore attaches many handlers (including the new skill_used one) with no cleanup via scoped_handlers, so they remain after the patch ends and can fire against a listener created only with __new__ (no batch_manager). That risks flaky failures and handler errors in other tests.
Reviewed by Cursor Bugbot for commit c66d958. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/tests/tracing/test_skill_used_tracing.py (1)
21-33: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the attribution arguments are forwarded.
The single-use test emits
source=Noneand checks only the event type, so a regression that drops or replaces the source orSkillUsedEventwould still pass. Use a representative source and assert("skill_used", source, event)was forwarded to the action handler.Suggested assertion
- bus.emit( - None, - SkillUsedEvent( + source = object() + event = SkillUsedEvent( skill_name="pdf-processing", skill_path=Path("/skills/pdf-processing"), - ), ) + bus.emit(source, event) bus.flush() - assert "skill_used" in _event_types(handled), ( - "SkillUsedEvent was emitted but the trace listener ignored it" - ) + assert handled.call_args.args == ("skill_used", source, event)As per coding guidelines, tests should focus on observable behavior rather than only implementation details.
Also applies to: 40-51
🤖 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/tracing/test_skill_used_tracing.py` around lines 21 - 33, Update the single-use skill tracing test and the related test covering lines 40–51 to emit a representative non-None source and retain the SkillUsedEvent instance. Assert the mocked _handle_action_event receives ("skill_used", source, event), verifying both attribution arguments are forwarded unchanged rather than checking only the event type.Source: Coding guidelines
🤖 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/tracing/test_skill_used_tracing.py`:
- Around line 21-33: Update the single-use skill tracing test and the related
test covering lines 40–51 to emit a representative non-None source and retain
the SkillUsedEvent instance. Assert the mocked _handle_action_event receives
("skill_used", source, event), verifying both attribution arguments are
forwarded unchanged rather than checking only the event type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 84923888-8f08-4039-85a2-663823e763af
📒 Files selected for processing (2)
lib/crewai/src/crewai/events/listeners/tracing/trace_listener.pylib/crewai/tests/tracing/test_skill_used_tracing.py
The problem
#6652 added
SkillUsedEventand deliberately shipped no listener wiring ("No new listener/tracing wiring — the event flows through the existing bus"). Nothing subscribes to it, so it reaches no collector and never lands in a trace.The trace listener subscribes to the five setup events:
SkillDiscoveryStartedEventSkillDiscoveryCompletedEventSkillLoadedEventSkillActivatedEventSkillLoadFailedEventNone of them answers the question skills observability exists for. Activation is the closest proxy and it is idempotent, fired once at setup — an agent using a skill across twenty turns produces exactly one event.
The fix
Subscribe to
SkillUsedEvent. It is the only runtime signal and the only one that re-fires per execution, so a trace can attribute usage to an agent and a task.Tests
tests/tracing/test_skill_used_tracing.py— usage reaches the collector, every use is collected (not collapsed), and the setup events still are. Verified they fail with the subscription removed.Found while wiring these events through to AMP traces, where the same gap exists on the enterprise listener.
🤖 Generated with Claude Code
Note
Low Risk
Observability-only wiring plus unit tests; no changes to skill execution or auth/data paths.
Overview
Traces can now record per-execution skill usage, not just one-time setup signals like discovery, load, and activation.
TraceCollectionListenersubscribes toSkillUsedEventand forwards each emission as askill_usedaction event via_handle_action_event, matching the other skill handlers. That closes a gap whereSkillUsedEventwas emitted at runtime but never reached the trace collector, so repeated use of the same skill could not be attributed to tasks.New tests in
test_skill_used_tracing.pyassert thatskill_usedis collected, that multiple uses produce multiple trace events (unlike idempotent activation), and that existing setup events such asskill_activatedstill work.Reviewed by Cursor Bugbot for commit c66d958. Bugbot is set up for automated code reviews on this repo. Configure here.