feat(telemetry): add skill usage telemetry#66
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughAdds skill command telemetry to the OTel plugin: a new ChangesSkill command telemetry
Sequence Diagram(s)sequenceDiagram
participant OtelPlugin
participant createSkillCommandResolver
participant client as client.command.list
participant http as HTTP /command or /api/command
participant handleCommandExecuteBefore
participant ctx as ctx (skillCounter / emitLog)
OtelPlugin->>createSkillCommandResolver: initialize(client, serverUrl, directory)
OtelPlugin->>createSkillCommandResolver: refresh(true) on startup
createSkillCommandResolver->>client: command.list()
alt client returns source metadata
client-->>createSkillCommandResolver: commands filtered to source="skill"
else metadata missing or error
createSkillCommandResolver->>http: GET /command?directory=...
http-->>createSkillCommandResolver: catalog JSON
end
OtelPlugin->>handleCommandExecuteBefore: command.execute.before event
handleCommandExecuteBefore->>createSkillCommandResolver: resolve(command)
createSkillCommandResolver-->>handleCommandExecuteBefore: SkillCommandInfo or undefined
alt skill found
handleCommandExecuteBefore->>ctx: skillCounter.add(1)
handleCommandExecuteBefore->>ctx: emitLog(skill_invoked, attributes)
handleCommandExecuteBefore->>ctx: log(skill invoked)
else not a skill command
handleCommandExecuteBefore-->>OtelPlugin: no-op
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/handlers/disabled-metrics.test.ts (1)
248-248: ⚡ Quick winMake the log assertion order-independent.
Using
logger.records.at(0)is brittle if additional log records are emitted later. Assert presence of"skill_invoked"in the record set instead.Suggested diff
- expect(logger.records.at(0)!.body).toBe("skill_invoked") + expect(logger.records.some((r) => r.body === "skill_invoked")).toBe(true)🤖 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 `@tests/handlers/disabled-metrics.test.ts` at line 248, The assertion at line 248 uses `logger.records.at(0)!.body` to check for the "skill_invoked" log record, which is brittle because it depends on this being the first log record. If additional log records are emitted before or after, this assertion will fail. Instead of checking a specific position with `.at(0)`, refactor the assertion to check if "skill_invoked" exists anywhere in the logger.records array by using a method like `.some()` to verify that at least one record has a body matching "skill_invoked", making the test order-independent.
🤖 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.
Inline comments:
In `@src/handlers/skill.ts`:
- Line 1: The handler file is importing SeverityNumber directly from the
OpenTelemetry API, which violates the handler boundary contract that requires
all dependencies to be threaded through HandlerContext. Remove the
SeverityNumber import from src/handlers/skill.ts and instead add the severity
value (or a reference to it) to the HandlerContext interface. Then update the
handleCommandExecuteBefore handler and any other references (including the code
at line 190) to access the severity value from the ctx parameter instead of
using the direct import. This ensures all OTel globals are properly threaded
through context rather than imported directly in handler files.
---
Nitpick comments:
In `@tests/handlers/disabled-metrics.test.ts`:
- Line 248: The assertion at line 248 uses `logger.records.at(0)!.body` to check
for the "skill_invoked" log record, which is brittle because it depends on this
being the first log record. If additional log records are emitted before or
after, this assertion will fail. Instead of checking a specific position with
`.at(0)`, refactor the assertion to check if "skill_invoked" exists anywhere in
the logger.records array by using a method like `.some()` to verify that at
least one record has a body matching "skill_invoked", making the test
order-independent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6a9e9298-2c55-4a53-b2bd-5246baac1852
📒 Files selected for processing (8)
README.mdsrc/handlers/skill.tssrc/index.tssrc/otel.tssrc/types.tstests/handlers/disabled-metrics.test.tstests/handlers/skill.test.tstests/helpers.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d11123bf6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5d11123 to
7dded0f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dded0fa7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
7dded0f to
158debf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 158debf430
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
158debf to
8d20f29
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d20f2980f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8d20f29 to
2f77094
Compare
Summary
opencode.skill.countcounter for skill invocationsskill_invokedlog events with project/session/agent/skill metadata/commandand/api/commandfallbacksValidation
bun run lintbun run check:jsdoc-coveragebun run typecheckbun testSummary by CodeRabbit
Release Notes
New Features
opencode.skill.countmetric andskill_invokedlog events, covering both skill commands and native skill tool activity.serverUrlas part of the plugin configuration to enable skill-command resolution during startup.Documentation
opencode.skill.countand expandedOPENCODE_DISABLE_METRICSexamples.Tests
skill.countis disabled.