Uh oh!
There was an error while loading. Please reload this page.
feat(trogonstack-otel): add metric and span naming skills - #28
Conversation
PR SummaryLow Risk Overview Registers the plugin in Written by Cursor Bugbot for commit 89c6596. This will update automatically on new commits. Configure here. |
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughA new OpenTelemetry plugin ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
120ae49 to
16f386bCompareThere was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
plugins/trogonstack-otel/skills/otel-name-metric/references/namespaces.md (1)
51-53: Normalize example metric names to one delimiter style.Several examples mix underscore-style metric tokens with dot-separated conventions, which can produce inconsistent naming recommendations from the skill. Prefer one style across all examples (ideally the same style used elsewhere in this plugin docs).
Also applies to: 107-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/trogonstack-otel/skills/otel-name-metric/references/namespaces.md` around lines 51 - 53, The examples mix underscore and dot delimiters; normalize to dot-separated metric tokens used elsewhere by replacing underscores with dots in the example names—e.g. change `faas.invoke_duration` to `faas.invoke.duration`, `gen_ai.client.token.usage` to `gen.ai.client.token.usage` (convert the namespace delimiter too if other docs use dots), and ensure `cicd.pipeline.run.duration` remains dot-delimited; apply the same normalization to the other examples referenced around lines 107-110 so all example metric names consistently use dot separators.plugins/trogonstack-otel/skills/otel-name-span/SKILL.md (1)
59-63: Clarify the*.system.nameattribute reference.Line 63 states "The system name MUST match the value in
*.system.nameattribute". The*.system.namenotation is ambiguous—it's unclear whether this refers to:
- A literal attribute name with an asterisk
- A wildcard pattern meaning any
{namespace}.system.nameattribute (e.g.,db.system.name,messaging.system.name)Consider clarifying this with a concrete example or rewording for clarity.
📝 Suggested clarification
### System-Specific Attributes - Pattern: `{system_name}.*.{property}` - Examples: `cassandra.consistency.level`, `aws.s3.key` -- The system name MUST match the value in `*.system.name` attribute+- The system name MUST match the value in the corresponding `{namespace}.system.name` attribute (e.g., if using `cassandra.*` attributes, the `db.system.name` attribute value should be `"cassandra"`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/trogonstack-otel/skills/otel-name-span/SKILL.md` around lines 59 - 63, Clarify the ambiguous `*.system.name` reference in the "System-Specific Attributes" section of SKILL.md by stating it is a wildcard pattern (not a literal asterisk) meaning any {namespace}.system.name attribute (e.g., `db.system.name`, `messaging.system.name`), and update the sentence "The system name MUST match the value in `*.system.name` attribute" to read something like "The system name MUST match the value in the `{namespace}.system.name` attribute (e.g., `db.system.name` for database systems)."plugins/trogonstack-otel/skills/otel-name-metric/SKILL.md (1)
80-90: Clarify the DB client pattern explanation.Line 84 states: "DB is always measured from client side, but convention uses
client". This explanation is somewhat circular. Consider clarifying why the convention explicitly includesclienteven though DB operations are always from the client perspective (e.g., for consistency with other patterns, explicitness, or future-proofing).📝 Suggested clarification
-| `db.client.operation.duration` | `{area}.{client}.{metric_name}` | DB is always measured from client side, but convention uses `client` |+| `db.client.operation.duration` | `{area}.{client}.{metric_name}` | Explicitly uses `client` for consistency with the pattern, even though DB operations are always client-side |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/trogonstack-otel/skills/otel-name-metric/SKILL.md` around lines 80 - 90, Clarify the DB client pattern text for the example `db.client.operation.duration` and the pattern `{area}.{client}.{metric_name}`: replace the circular sentence with a concise rationale saying the `client` token is included for explicitness and consistency with other area/client/server conventions (helps tooling and humans understand the side of the operation), supports future cases like proxies/replicas or server-side DB metrics, and keeps naming predictable across metric families; update the explanation next to `db.client.operation.duration` to reflect this.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/trogonstack-otel/.claude-plugin/plugin.json`:
- Around line 1-9: Add a marketplace registration for the new plugin
"trogonstack-otel" into the .claude-plugin/marketplace.json so the plugin can be
discovered/installed; specifically, open .claude-plugin/marketplace.json and add
an entry matching the pattern used by other siblings (e.g.,
trogonstack-diataxis) that includes the plugin key "trogonstack-otel",
repository identifier (TrogonStack/agentskills or the correct repo), version
"0.0.1" and author metadata consistent with the existing entries, saving the
file so the new plugin appears alongside trogonstack-diataxis, trogonstack-gh,
trogonstack-nats, trogonstack-datadog and trogonstack-ask.
In `@plugins/trogonstack-otel/skills/otel-name-span/references/anti-patterns.md`:
- Around line 37-38: Update the outdated OpenTelemetry attribute key in the
anti-patterns table by replacing occurrences of `db.system` with the canonical
`db.system.name`; specifically edit the table rows that read "Missing
`db.system` on DB calls" (and the other row referencing `db.system`) so they
instead say "Missing `db.system.name` on DB calls" and any inline code snippets
or cells use `db.system.name` to match protocols.md and the current semantic
conventions.
In `@plugins/trogonstack-otel/skills/otel-name-span/references/protocols.md`:
- Around line 39-46: The messaging examples use different operation names
("publish", "ack") than the operation table ("create", "send", "receive",
"process", "settle"); update the examples to use the table vocabulary so
terminology is consistent (replace "publish" with "send" and "ack" with "settle"
in the otel-name-span examples and any other occurrences referenced at lines
48-52), and ensure the span kind mappings in the examples match the table
entries (e.g., send -> PRODUCER/CLIENT, receive -> CLIENT, process -> CONSUMER,
settle -> CLIENT).
---
Nitpick comments:
In `@plugins/trogonstack-otel/skills/otel-name-metric/references/namespaces.md`:
- Around line 51-53: The examples mix underscore and dot delimiters; normalize
to dot-separated metric tokens used elsewhere by replacing underscores with dots
in the example names—e.g. change `faas.invoke_duration` to
`faas.invoke.duration`, `gen_ai.client.token.usage` to
`gen.ai.client.token.usage` (convert the namespace delimiter too if other docs
use dots), and ensure `cicd.pipeline.run.duration` remains dot-delimited; apply
the same normalization to the other examples referenced around lines 107-110 so
all example metric names consistently use dot separators.
In `@plugins/trogonstack-otel/skills/otel-name-metric/SKILL.md`:
- Around line 80-90: Clarify the DB client pattern text for the example
`db.client.operation.duration` and the pattern `{area}.{client}.{metric_name}`:
replace the circular sentence with a concise rationale saying the `client` token
is included for explicitness and consistency with other area/client/server
conventions (helps tooling and humans understand the side of the operation),
supports future cases like proxies/replicas or server-side DB metrics, and keeps
naming predictable across metric families; update the explanation next to
`db.client.operation.duration` to reflect this.
In `@plugins/trogonstack-otel/skills/otel-name-span/SKILL.md`:
- Around line 59-63: Clarify the ambiguous `*.system.name` reference in the
"System-Specific Attributes" section of SKILL.md by stating it is a wildcard
pattern (not a literal asterisk) meaning any {namespace}.system.name attribute
(e.g., `db.system.name`, `messaging.system.name`), and update the sentence "The
system name MUST match the value in `*.system.name` attribute" to read something
like "The system name MUST match the value in the `{namespace}.system.name`
attribute (e.g., `db.system.name` for database systems)."
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4715ac1a-96ac-4639-8a26-7b048bf16c84
📒 Files selected for processing (10)
.github/release-please-config.json.github/release-please-manifest.jsonplugins/trogonstack-otel/.claude-plugin/plugin.jsonplugins/trogonstack-otel/skills/otel-name-metric/SKILL.mdplugins/trogonstack-otel/skills/otel-name-metric/references/anti-patterns.mdplugins/trogonstack-otel/skills/otel-name-metric/references/instruments.mdplugins/trogonstack-otel/skills/otel-name-metric/references/namespaces.mdplugins/trogonstack-otel/skills/otel-name-span/SKILL.mdplugins/trogonstack-otel/skills/otel-name-span/references/anti-patterns.mdplugins/trogonstack-otel/skills/otel-name-span/references/protocols.md
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
3d71edd to
fdf5651CompareSigned-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Summary
trogonstack-otelplugin with two skills:otel-name-metricandotel-name-spanSkills
otel-name-metricReviews or creates metric names following OTel Semantic Conventions. Covers naming patterns (
{area}.{metric_name},{area}.{client|server}.{metric_name}), instrument selection, UCUM units, pluralization, attribute placement, and anti-patterns.Reference files:
namespaces.md,instruments.md,anti-patterns.mdotel-name-spanReviews or creates span names following OTel Semantic Conventions. Covers low-cardinality naming,
{verb} {object}pattern, protocol-specific patterns (HTTP, DB, messaging, RPC), span status mapping, and attribute placement.Reference files:
protocols.md,anti-patterns.mdTest plan
otel-name-metricagainst sample metric definitionsotel-name-spanagainst sample span definitions