docs: add telemetry implementation guidance to AGENTS.md - #1461
Conversation
Adds a §5 coding-standards bullet directing contributors to the hook-fired plugin pattern for spans and metrics. Recurring confusion in telemetry PRs has come from reading mellea/telemetry/tracing.py, matching start_*_span helper names, and calling them directly from core code — instead of emitting through a *TracingPlugin / *MetricsPlugin subscribing to lifecycle hooks. The bullet states the rule, the sync-only exception, and that a span needs paired open/close hooks while a completion-only hook can feed a metric. Assisted-by: Claude Code Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
github-actions
Bot
added
the
documentation
|
@planetf1 I decided to add this to try and address the thrash I've seen you both of our PRs that touch telemetry recently |
markstur
left a comment
markstur
left a comment
There was a problem hiding this comment.
LGTM
Inline is some very optional grammar advice just FYI
| - **Friendly Dependency Errors**: Wraps optional backend imports in `try/except ImportError` with a helpful message (e.g., "Please pip install mellea[hf]"). See `mellea/stdlib/session.py` for examples. | ||
| - **CLI command docstrings**: Typer command functions in `cli/` follow an enriched convention with `Prerequisites:` and `See Also:` sections — these feed the auto-generated CLI reference page. See [`docs/CONTRIBUTING_DOCS.md`](docs/CONTRIBUTING_DOCS.md) for the full pattern. Regenerate after changes: `uv run poe clidocs`. Test the generator: `uv run pytest tooling/docs-autogen/test_cli_reference.py -v`. Full pipeline docs: [`tooling/docs-autogen/README.md`](tooling/docs-autogen/README.md). | ||
| - **Backend telemetry fields**: All backends must populate `mot.generation.usage` (dict with `prompt_tokens`, `completion_tokens`, `total_tokens`), `mot.generation.model` (str), and `mot.generation.provider` (str) in their `post_processing()` method. These fields live on `mot.generation`, a `GenerationMetadata` dataclass. `mot.generation.streaming` (bool) and `mot.generation.ttfb_ms` (float | None) are set automatically in `astream()` — backends do not need to set them. Metrics are automatically recorded by `TokenMetricsPlugin`, `LatencyMetricsPlugin`, and `ErrorMetricsPlugin` — don't add manual `record_token_usage_metrics()`, `record_request_duration()`, or `record_error()` calls. | ||
| - **Adding or editing telemetry (spans/metrics)**: Telemetry is emitted by **hook-fired plugins**, not direct calls. Core fires lifecycle hooks; a `*TracingPlugin` in `mellea/telemetry/tracing_plugins.py` emits spans and a `*MetricsPlugin` in `mellea/telemetry/metrics_plugins.py` emits metrics, both subscribing to those hooks. Core does **not** call `start_*_span`/`finish_*_span` from `mellea/telemetry/tracing.py` directly (the only exception is sync code that can't fire paired hooks). Matching helper names in `tracing.py` is not enough — read the plugins, and use the existing one whose span shape matches yours as the template. Emitting a span needs a start/pre hook to open it and a matching end/post hook to close it; a hook that only fires at completion, with no paired opener, can feed a metric but cannot anchor a span. |
There was a problem hiding this comment.
grammar nitty-nit?
FYI -- grammar on "start/pre hook" looks suspect (but not bad) so I tried to figure out what would be recommended (in particular hyphenating with that slash).
I came up with this:
The Best Revision for your PR/DocumentationSince they are concepts and not literal function strings, you should use standard technical hyphenation:"Emitting a span needs a start/pre-hook to open it and a matching end/post-hook to close it;"
If you want to be completely aligned with Mellea's precise engineering terminology, you could also write it as:"Emitting a span needs a pre-call hook to open it and a matching post-call hook to close it;"
BUT your version reads just as well to me (or close enough). Of course it is for agents to ready anyway. I'd recommend you stick with whatever you are more comfortable with, but wanted to share the FYI with what I found.
Note: you could also replace the slash with "or", but I like the slash.
There was a problem hiding this comment.
the primary intent of this block is to force the AI to look in the right place for how telemetry is shaped, the rest is just to more forcefully guide it there. I was intentionally vague on key words to prevent the AI from reading this block as the guide instead of reading the code
6530575
3 participants
Pull Request
Issue
N/A — docs-only clarification, no tracking issue.
Description
Adds a coding-standards bullet to AGENTS.md §5 documenting the telemetry implementation pattern.
Recurring confusion in telemetry PRs has come from reading
mellea/telemetry/tracing.py, matchingstart_*_spanhelper names, and calling them directly from core code — instead of emitting through a*TracingPlugin/*MetricsPluginthat subscribes to lifecycle hooks. The new bullet states:start_*_span/finish_*_spandirectly (the only exception is sync code that can't fire paired hooks).tracing.pyisn't enough.Testing
Attribution
Adding a new component, requirement, sampling strategy, or tool?