feat(observability): Add OpenTelemetry GenAI Semantic Projection#490
feat(observability): Add OpenTelemetry GenAI Semantic Projection#490Mluckydwyer wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
License DiffCompared against Lockfile license changesLockfile License ChangesRustAdded
Removed
Updated/Changed
NodeAdded
Removed
Updated/Changed
PythonAdded
Removed
Updated/Changed
Status output |
Signed-off-by: Matt Dwyer <10600905+Mluckydwyer@users.noreply.github.com>
a21bb59 to
9873745
Compare
|
Local tests in e2e environments are still WIP. |
willkill07
left a comment
There was a problem hiding this comment.
Some minor comments. Also happy to get this across the finish line for you.
| /// # Safety | ||
| /// Any non-null C strings must be valid and `out` must be non-null. | ||
| #[unsafe(no_mangle)] | ||
| pub unsafe extern "C" fn nemo_relay_otel_subscriber_create_with_attribute_mappings( |
There was a problem hiding this comment.
We can get rid of this and only keep with_options.
Although a breaking API change, we haven't stabalized the FFI/Go APIs yet.
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] | ||
| #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub enum OpenTelemetrySemanticConvention { |
There was a problem hiding this comment.
If we do keep generic and also add GenAI, why not just create a new OtelGenAISemanticConvention exporter type? That way we keep all existing behavior/APIs and introduce the new one:
- as a new exporter type
- as a new configuration option under the observability plugin
| Each exporter projects `usage`/`cost` differently. Projections do not change the | ||
| canonical fields above. | ||
|
|
||
| | | ATOF | ATIF step / `final_metrics` | OpenInference | OpenTelemetry | |
There was a problem hiding this comment.
This table supports my suggestion on splitting out as a new type rather than overload OpenTelemetry
Overview
Add opt-in OpenTelemetry GenAI 1.37+ semantic-convention support to NeMo Relay’s existing OTLP exporter.
Generic OpenTelemetry remains the default, OpenInference behavior is unchanged, and no backend-specific transport or intake code is added.
Details
gen_aiprojection for agent, LLM, tool, embedding, retrieval, and reranking scopes.nemo_relay.*correlation attributes.
Validation passed: Rust formatting, Clippy, the serial Rust workspace suite, 35 focused OpenTelemetry tests, 536 Python tests, 266 Node.js tests, Go and FFI
suites, documentation checks, and the complete pre-commit suite.
Reviewer considerations:
rerankoperation./nvskills-ci.Where should the reviewer start?
Start with
crates/core/src/observability/otel.rs, the projection inotel_genai.rs, and the coverage incrates/core/tests/unit/observability/ otel_tests.rs.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)