-
Notifications
You must be signed in to change notification settings - Fork 931
Instrumentations and GenAI: Add copilot review instructions, update agents.md #4457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
aabmass
merged 10 commits into
open-telemetry:main
from
lmolkova:genai-review-instructions
Apr 23, 2026
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
71512f2
add copilot reivew instructions for GenAI libs
lmolkova 50368d1
up
lmolkova db6291a
up
lmolkova 1711efa
expand beyond genai, create common files
lmolkova c977a29
up
lmolkova 542a991
Merge branch 'main' into genai-review-instructions
lmolkova 416644d
changelog
lmolkova 2e0b826
Merge branch 'main' into genai-review-instructions
lmolkova 46b6c16
Merge branch 'main' into genai-review-instructions
lmolkova bf7a22c
Merge branch 'main' into genai-review-instructions
lmolkova File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
66 changes: 66 additions & 0 deletions
66
.github/instructions/instrumentation-genai.instructions.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| --- | ||
| applyTo: "instrumentation-genai/**" | ||
| --- | ||
|
|
||
| Review rules for PRs touching `instrumentation-genai/**`. Flag violations with a link to the rule. | ||
|
|
||
| These rules are additive to | ||
| [`instrumentation.instructions.md`](instrumentation.instructions.md), which applies to all | ||
| instrumentation packages. | ||
|
|
||
| ## 1. Scope of `instrumentation-genai/` | ||
|
|
||
| Only for: | ||
|
|
||
| - Generative AI inference providers, | ||
| - Agentic frameworks, | ||
| - Libraries directly supporting the above (e.g., MCP, GenAI protocols). | ||
|
|
||
| Database clients (including vector DBs used outside a GenAI-specific client) and CLI libs belong | ||
| in `instrumentation/`, not here. | ||
|
|
||
| ## 2. GenAI component ownership | ||
|
|
||
| See [`CONTRIBUTING.md#guideline-for-genai-instrumentations`](../../CONTRIBUTING.md#guideline-for-genai-instrumentations) | ||
| for GenAI-specific maintenance expectations on top of the general | ||
| [instrumentation checklist](../../CONTRIBUTING.md#guideline-for-instrumentations). | ||
|
|
||
| ## 3. Telemetry and configuration via `opentelemetry-util-genai` | ||
|
|
||
| - Spans, logs, metrics, and events must go through `opentelemetry-util-genai`. Direct use of | ||
| `Tracer`, `Meter`, `Logger`, or event APIs is not allowed. | ||
| - Content capture, hooks, and other cross-cutting configuration are owned by the util. | ||
| Instrumentations must not introduce their own env vars, settings, or hook interfaces. | ||
| - Message content, prompts, and tool call arguments must only be set through the util's content | ||
| capture path — never as unconditional span/log attributes. | ||
| - Adding attributes to invocations produced by the util is fine. | ||
| - If a capability is missing in `opentelemetry-util-genai`, land it in the util first. | ||
|
|
||
| ## 4. GenAI semantic conventions | ||
|
|
||
| - Attributes, spans, events, and metrics must match the | ||
| [GenAI semantic conventions](https://github.com/open-telemetry/semantic-conventions/tree/main/docs/gen-ai). | ||
| - `gen_ai.*` attribute names must come from | ||
| `opentelemetry.semconv._incubating.attributes.gen_ai_attributes`. | ||
| - For attributes with a well-known value set, use the generated enum from the same module | ||
| (e.g. `GenAiOutputTypeValues` for `gen_ai.output.type`) instead of string literals. | ||
|
|
||
| ## 5. Tests | ||
|
|
||
| - Use recorded VCR cassettes for provider calls. No live-key-only tests; skipping on missing key | ||
| is not acceptable. | ||
| - Cover streaming and non-streaming variants when both exist. | ||
| - For error scenarios, at minimum include: provider error / endpoint unavailable, stream | ||
| interrupted by network, stream closed early by the caller. | ||
|
|
||
| ## 6. Examples | ||
|
|
||
| New instrumentations must ship a minimal example under the package's `examples/`, with both a | ||
| `manual/` and a `zero-code/` (auto-instrumentation) variant. | ||
|
|
||
| ## 7. PR description | ||
|
|
||
| - Cover which part of the GenAI semconv the change implements or follows (when applicable) and | ||
| how instrumentations should consume it. | ||
|
|
||
| See also [AGENTS.md](../../AGENTS.md) for general repo rules. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| --- | ||
| applyTo: "{instrumentation,instrumentation-genai}/**" | ||
| --- | ||
|
|
||
| Review rules for PRs touching `instrumentation/**` and `instrumentation-genai/**`. Flag violations | ||
| with a link to the rule. | ||
|
|
||
| ## 0. Reviewer mindset | ||
|
|
||
| Review as long-term maintainer. | ||
|
|
||
| For new instrumentations, consult upstream library docs and judge: | ||
|
|
||
| - Does the library already emit its own telemetry, making this instrumentation redundant? | ||
| - Is the library used widely enough to warrant a package in this repo? | ||
| - Does it avoid unbounded in-memory accumulation or other side-effects? | ||
|
|
||
| For changes to existing instrumentations: prefer back-compat. Break users only for a real reason; | ||
| prefer opt-in or additive. Breaking changes need explicit justification in the PR. | ||
|
|
||
| ## 1. Component ownership & maintenance commitment | ||
|
|
||
| - New instrumentations must add an entry under the correct folder in | ||
| [`component_owners.yml`](../component_owners.yml) in the same PR. Contributor must commit to | ||
| long-term maintenance. See | ||
| [Expectations from contributors](../../CONTRIBUTING.md#expectations-from-contributors) and the | ||
| general [instrumentation checklist](../../CONTRIBUTING.md#guideline-for-instrumentations). | ||
|
|
||
| ## 2. Semantic conventions | ||
|
|
||
| - Attribute names must come from the semconv attribute modules, not hardcoded strings. Use the | ||
| module matching the namespace under `opentelemetry.semconv` (e.g. `server_attributes`, | ||
| `error_attributes`, `http_attributes`, `db_attributes`, …). | ||
| - For attributes with a well-known value set in semconv, use the generated enum from the same | ||
| modules instead of string literals. | ||
| - If a signal is not in semconv, wait until semconv lands. | ||
|
|
||
| ## 3. Exception handling | ||
|
|
||
| - When catching exceptions from the underlying library to record telemetry, always re-raise the | ||
| original exception unmodified. | ||
| - Do not raise **new** exceptions in instrumentation/telemetry code. | ||
|
|
||
| ## 4. Tests | ||
|
|
||
| - For every public API instrumented, cover sync/async variants when both exist. | ||
| - Cover happy path and error scenarios. | ||
| - Tests must verify exact attribute names **and value types**, checked against the semconv spec. | ||
| - Test against oldest and latest supported library versions via `tests/requirements.{oldest,latest}.txt` | ||
| and `{oldest,latest}` `tox.ini` factors. | ||
|
|
||
| See also [AGENTS.md](../../AGENTS.md) for general repo rules. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| --- | ||
| applyTo: "util/opentelemetry-util-genai/**" | ||
| --- | ||
|
|
||
| Review rules for PRs touching `util/opentelemetry-util-genai/**`. Flag violations with a link to | ||
| the rule. | ||
|
|
||
| ## 0. Reviewer mindset | ||
|
|
||
| Review as long-term maintainer. Every GenAI instrumentation in the repo depends on this package; | ||
| churn breaks them. Default to back-compat changes. Every public addition is a long-term | ||
| commitment — limit public API. | ||
|
|
||
| ## 1. Semconv first | ||
|
|
||
| No code without semconv. If a signal, attribute, or operation is not in the | ||
| [GenAI semconv](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/), | ||
| land the semconv change first. | ||
|
|
||
| ## 2. Semantic conventions | ||
|
|
||
| - Names (attributes, operations, spans) must match semconv exactly. | ||
| - Use matching `opentelemetry.semconv` modules per namespace (`gen_ai`, `server`, `error`, …). | ||
| Do not hardcode name strings if a constant exists. | ||
| - Shared attributes must behave consistently across invocation types (same conditions, same | ||
| defaults). If semconv marks an attribute for multiple invocations, set it on all. | ||
|
|
||
| ## 3. API backwards compatibility | ||
|
|
||
| - Do not remove or rename public objects. Deprecate first via a docstring note pointing to the | ||
| replacement (not `@deprecated` — unreliable). | ||
| - Private modules and module-private objects start with `_`. | ||
| - Default to internal (`_`-prefixed) unless instrumentations need it public. | ||
|
|
||
| ## 4. Invocation shape | ||
|
|
||
| - `start_*()` factories must accept all sampling-relevant semconv attributes as parameters. | ||
| Attributes also marked required by semconv must be required parameters (no default value). | ||
| - `start_*()` factories must map 1:1 to distinct semconv operation types (inference, embeddings, | ||
| tool execution, agent invocation, workflow invocation). Names must match the operation | ||
| unambiguously — e.g., `create_agent` vs `invoke_agent` are distinct ops; `start_agent()` alone | ||
| is ambiguous. | ||
| - Each operation exposes both a factory (`start_inference(...)`) and a context-manager | ||
| (`inference(...)`) form. | ||
| - Never construct invocation types directly (`InferenceInvocation(...)`) — skips span creation, | ||
| silent no-ops. Always use `handler.start_*()` or the context manager. | ||
|
|
||
| ## 5. Exception handling | ||
|
|
||
| - When catching exceptions from the underlying library to record telemetry, always re-raise the | ||
| original exception unmodified. | ||
| - Do not raise **new** exceptions in utils code. | ||
|
|
||
| ## 6. Performance | ||
|
|
||
| Keep the hot path tight: | ||
|
|
||
| - Avoid per-invocation allocations; do not accumulate state unboundedly. | ||
| - Skip content capture when content capture is disabled. | ||
| - Skip setting span attributes when the span is not recording. | ||
| - Still record attributes that feed metrics — metric recording is independent of span sampling. | ||
|
|
||
| ## 7. DRY | ||
|
|
||
| Do not copy-paste logic across invocation types. Extract shared helpers. | ||
|
|
||
| ## 8. Tests | ||
|
|
||
| - Every new operation type or attribute change needs tests asserting exact attribute names **and | ||
| value types**, checked against semconv. | ||
| - Cover success (`invocation.stop()`), failure (`invocation.fail(exc)`), and conditional | ||
| attribute logic. | ||
| - Prefer public API in tests. | ||
|
|
||
| ## 9. Documentation | ||
|
|
||
| - Docstrings for invocation types and span/event helpers must link to the corresponding semconv | ||
| op. | ||
| - When adding/changing attributes, update the docstring with what is set and when. | ||
|
|
||
| ## 10. PR description | ||
|
|
||
| - Cover which part of the GenAI semconv the change implements or follows (when applicable) and | ||
| how instrumentations should consume it. | ||
|
|
||
| See also [AGENTS.md](../../AGENTS.md) for general repo rules. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.