feat(observability)!: project typed OTLP attributes#414
Conversation
willkill07
added
the
breaking
ChatGPT Codex Connector
github-actions
Bot
added
size:XL
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (12)**/*.rs📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
{crates/core,crates/adaptive}/**/*📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
crates/core/**/*.rs📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
Files:
**/*.{rs,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,js,mjs,cjs,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,go,js,ts,c,h}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,go,js,ts}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
crates/{core,adaptive}/**/*📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,go,js,ts}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
crates/{core,adaptive}/**/*.rs⚙️ CodeRabbit configuration file
Files:
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
WalkthroughAdds typed OTLP attribute projection and configurable aliases across OpenTelemetry and OpenInference. Validation, plugin configuration, FFI, Node, Python, and Go bindings are updated, while tests adopt flattened typed attribute keys. ChangesOTLP attribute mapping
Language and FFI integrations
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Config as Exporter configuration
participant Subscriber as OTLP subscriber
participant Processor as OtelEventProcessor
participant Exporter as OTLP exporter
Config->>Subscriber: validate and pass attribute_mappings
Subscriber->>Processor: store mappings
Processor->>Processor: project attributes and apply aliases
Processor->>Exporter: emit typed OTLP attributes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
0620b48 to
328a0fa
Compare
328a0fa to
7fadff2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@crates/core/tests/unit/observability/exporter_parity_tests.rs`:
- Around line 697-710: Remove the stale
`otel.get("nemo_relay.start.input_json")` deserialization and equality assertion
from the OTel parity test. Keep validating the projected
`nemo_relay.start.input.headers` attribute and the existing end-output keys in
the test.
In `@crates/ffi/src/api/observability.rs`:
- Around line 533-551: Update parse_attribute_mappings_json so syntactically
valid JSON that fails deserialization due to an invalid mapping shape, including
{} null or a non-string key, returns NemoRelayStatus::InvalidArg while
preserving InvalidJson for malformed JSON syntax. Keep the existing error
reporting and validation behavior consistent with parse_string_map_json.
In `@crates/ffi/tests/unit/api/plugin_tests.rs`:
- Around line 1302-1381: Extend the positive mapping coverage so configured
aliases are observed in exported spans: in
crates/ffi/tests/unit/api/plugin_tests.rs lines 1302-1381, emit an event through
each subscriber created by
test_ffi_typed_attribute_mapping_constructors_validate_and_accept_mappings and
assert tenant.id contains the expected typed value; in
crates/node/tests/openinference_tests.mjs line 35 and
crates/node/tests/otel_tests.mjs line 31, add runtime/exporter assertions for
the respective aliases; in go/nemo_relay/openinference_test.go lines 55-58 and
go/nemo_relay/otel_test.go lines 48-51, add focused projection assertions for
tenant.id. Preserve the existing constructor validation assertions.
In `@crates/node/tests/observability_plugin_tests.mjs`:
- Line 34: Update the Node-facing configuration around attribute_mappings to
expose the camelCase option attributeMappings, and translate it to Rust’s
internal attribute_mappings field before constructing the plugin configuration.
Remove or avoid exposing the snake_case key in the JavaScript API while
preserving the existing empty-mapping behavior.
In `@docs/configure-plugins/observability/opentelemetry.mdx`:
- Around line 108-112: The OpenTelemetry `attribute_mappings` example is missing
its required configuration scope. Place it inside the
`[components.config.opentelemetry]` table, or provide the complete table header
with the snippet, while keeping the alias mapping unchanged.
In `@go/nemo_relay/nemo_relay.go`:
- Around line 1840-1846: Add focused tests for both the OpenTelemetry and
OpenInference mapping paths using AttributeMappings and OtlpAttributeMapping.
Assert that a valid mapping projects the source attribute to the alias while
preserving its correct typed value, and that invalid mappings are rejected.
Verify these behaviors through the Go API/FFI path so an ignored
AttributeMappings argument cannot pass unnoticed.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 5cde0522-c538-446e-b972-cdfc94cc4cc6
📒 Files selected for processing (30)
crates/core/src/observability/mod.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rscrates/core/src/observability/plugin_component.rscrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/observability.d.tscrates/node/observability.jscrates/node/src/api/mod.rscrates/node/tests/observability_plugin_tests.mjscrates/node/tests/openinference_tests.mjscrates/node/tests/otel_tests.mjscrates/python/src/py_types/observability.rsdocs/configure-plugins/observability/openinference.mdxdocs/configure-plugins/observability/opentelemetry.mdxgo/nemo_relay/nemo_relay.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gopython/nemo_relay/_native.pyipython/nemo_relay/observability.pypython/nemo_relay/observability.pyipython/tests/test_observability_plugin.pypython/tests/test_types.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (39)
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/tests/test_observability_plugin.pycrates/core/tests/integration/middleware_tests.rscrates/ffi/tests/unit/api/plugin_tests.rspython/nemo_relay/observability.pycrates/core/tests/unit/observability/plugin_component_tests.rspython/tests/test_types.pycrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/tests/test_observability_plugin.pycrates/core/tests/integration/middleware_tests.rscrates/ffi/tests/unit/api/plugin_tests.rspython/nemo_relay/observability.pycrates/node/tests/observability_plugin_tests.mjscrates/node/tests/otel_tests.mjscrates/node/observability.d.tscrates/node/observability.jscrates/core/tests/unit/observability/plugin_component_tests.rspython/tests/test_types.pycrates/node/tests/openinference_tests.mjscrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/tests/test_observability_plugin.pypython/nemo_relay/observability.pypython/tests/test_types.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/tests/test_observability_plugin.pygo/nemo_relay/observability_plugin.gocrates/ffi/nemo_relay.hgo/nemo_relay/otel_test.gocrates/core/tests/integration/middleware_tests.rscrates/ffi/tests/unit/api/plugin_tests.rspython/nemo_relay/observability.pycrates/node/observability.d.tscrates/node/observability.jscrates/core/tests/unit/observability/plugin_component_tests.rspython/tests/test_types.pygo/nemo_relay/openinference_test.gocrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rsgo/nemo_relay/nemo_relay.gocrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/tests/test_observability_plugin.pypython/nemo_relay/observability.pypython/tests/test_types.pycrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/src/observability/mod.rscrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
python/tests/test_observability_plugin.pygo/nemo_relay/observability_plugin.gogo/nemo_relay/otel_test.gopython/nemo_relay/observability.pycrates/node/observability.d.tscrates/node/observability.jspython/tests/test_types.pygo/nemo_relay/openinference_test.gogo/nemo_relay/nemo_relay.go
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_observability_plugin.pypython/tests/test_types.py
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
python/tests/test_observability_plugin.pygo/nemo_relay/observability_plugin.gocrates/ffi/nemo_relay.hgo/nemo_relay/otel_test.gopython/nemo_relay/observability.pyidocs/configure-plugins/observability/opentelemetry.mdxdocs/configure-plugins/observability/openinference.mdxpython/nemo_relay/_native.pyicrates/core/tests/integration/middleware_tests.rscrates/ffi/tests/unit/api/plugin_tests.rspython/nemo_relay/observability.pycrates/node/tests/observability_plugin_tests.mjscrates/node/tests/otel_tests.mjscrates/node/observability.d.tscrates/node/observability.jscrates/core/tests/unit/observability/plugin_component_tests.rspython/tests/test_types.pycrates/node/tests/openinference_tests.mjsgo/nemo_relay/openinference_test.gocrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rsgo/nemo_relay/nemo_relay.gocrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
python/tests/test_observability_plugin.pygo/nemo_relay/observability_plugin.gogo/nemo_relay/otel_test.gocrates/core/tests/integration/middleware_tests.rscrates/ffi/tests/unit/api/plugin_tests.rspython/nemo_relay/observability.pycrates/node/observability.d.tscrates/node/observability.jscrates/core/tests/unit/observability/plugin_component_tests.rspython/tests/test_types.pygo/nemo_relay/openinference_test.gocrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rsgo/nemo_relay/nemo_relay.gocrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
python/tests/test_observability_plugin.pygo/nemo_relay/otel_test.gocrates/core/tests/integration/middleware_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/tests/observability_plugin_tests.mjscrates/node/tests/otel_tests.mjscrates/core/tests/unit/observability/plugin_component_tests.rspython/tests/test_types.pycrates/node/tests/openinference_tests.mjsgo/nemo_relay/openinference_test.gocrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/nemo_relay.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/nemo_relay.go
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/otel_test.gocrates/core/tests/integration/middleware_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/observability.d.tscrates/node/observability.jscrates/core/tests/unit/observability/plugin_component_tests.rsgo/nemo_relay/openinference_test.gocrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rsgo/nemo_relay/nemo_relay.gocrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/otel_test.gopython/nemo_relay/observability.pyipython/nemo_relay/_native.pyipython/nemo_relay/observability.pygo/nemo_relay/openinference_test.gocrates/node/src/api/mod.rsgo/nemo_relay/nemo_relay.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/otel_test.gopython/nemo_relay/observability.pyipython/nemo_relay/_native.pyipython/nemo_relay/observability.pygo/nemo_relay/openinference_test.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/nemo_relay.go
crates/ffi/**
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Rebuild the FFI crate in release mode so the shared library and header stay in sync when making changes to crates/ffi
Files:
crates/ffi/nemo_relay.hcrates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
crates/ffi/nemo_relay.h
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Check the generated header diff when any exported symbol or type changed in the FFI surface
Update generated or generated-from-build surfaces such as
crates/ffi/nemo_relay.hthrough the proper build step.
Files:
crates/ffi/nemo_relay.h
{crates/ffi/src/api/*.rs,crates/ffi/nemo_relay.h}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Add or update the shared C/FFI surface in the relevant
crates/ffi/src/api/*.rsmodule, re-export it throughcrates/ffi/src/api/mod.rs, and keep the generatedcrates/ffi/nemo_relay.hheader correct.
Files:
crates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/ffi/nemo_relay.hcrates/ffi/tests/unit/api/plugin_tests.rscrates/node/tests/observability_plugin_tests.mjscrates/node/tests/otel_tests.mjscrates/node/observability.d.tscrates/node/observability.jscrates/node/tests/openinference_tests.mjscrates/node/src/api/mod.rscrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rs
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/observability.pyipython/nemo_relay/_native.pyipython/nemo_relay/observability.py
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/configure-plugins/observability/opentelemetry.mdxdocs/configure-plugins/observability/openinference.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/configure-plugins/observability/opentelemetry.mdxdocs/configure-plugins/observability/openinference.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/configure-plugins/observability/opentelemetry.mdxdocs/configure-plugins/observability/openinference.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/configure-plugins/observability/opentelemetry.mdxdocs/configure-plugins/observability/openinference.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/configure-plugins/observability/opentelemetry.mdxdocs/configure-plugins/observability/openinference.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/configure-plugins/observability/opentelemetry.mdxdocs/configure-plugins/observability/openinference.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/integration/middleware_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/ffi/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/ffi, also usetest-ffi-surfacefor validationUse C FFI export names prefixed with
nemo_relay_in the raw C FFI layer.
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/observability.py
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/tests/observability_plugin_tests.mjscrates/node/tests/otel_tests.mjscrates/node/observability.d.tscrates/node/observability.jscrates/node/tests/openinference_tests.mjs
crates/node/**/*.{js,ts,jsx,tsx,json}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Format changed Node files with
npm run format --workspace=nemo-relay-node
Files:
crates/node/observability.d.tscrates/node/observability.js
crates/node/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Use
npm run check:docstrings --workspace=nemo-relay-nodeto validate public API docstring checks when surface docs changed
Files:
crates/node/observability.d.ts
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.
Files:
crates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
🧠 Learnings (1)
📚 Learning: 2026-05-07T18:04:44.387Z
Learnt from: mnajafian-nv
Repo: NVIDIA/NeMo-Flow PR: 67
File: integrations/openclaw/src/modules.ts:1-2
Timestamp: 2026-05-07T18:04:44.387Z
Learning: In NVIDIA/NeMo-Flow, TypeScript source files should use `//` line comments for SPDX headers (e.g., `// SPDX-FileCopyrightText: ...` and `// SPDX-License-Identifier: ...`) rather than C-style block comments (`/* ... */`). The repo’s copyright checker enforces this mapping, so `//` SPDX headers in `.ts` files should not be flagged as a style violation.
Applied to files:
crates/node/observability.d.ts
🔇 Additional comments (34)
docs/configure-plugins/observability/openinference.mdx (1)
66-66: LGTM!Also applies to: 116-127
docs/configure-plugins/observability/opentelemetry.mdx (1)
60-60: LGTM!crates/core/src/observability/plugin_component.rs (2)
54-56: LGTM!Also applies to: 387-389, 426-426, 508-508
1378-1379: 🗄️ Data Integrity & IntegrationNo mapping-validation gap here
validate_otlp_valuesalready rejects invalidattribute_mappingsbefore both builders consume them;unsupported_valueonly affects diagnostic severity.> Likely an incorrect or invalid review comment.crates/node/observability.d.ts (2)
73-77: LGTM!
61-61: 📐 Maintainability & Code QualityKeep
observabilityconfig in snake_case. This module is a schema-shaped helper, and its public API already usesattribute_mappings; camelCase is used on the subscriber constructors instead.> Likely an incorrect or invalid review comment.crates/node/src/api/mod.rs (1)
112-127: LGTM!Also applies to: 187-187, 297-297, 3474-3480
go/nemo_relay/nemo_relay.go (1)
254-254: LGTM!Also applies to: 263-263, 1910-1918, 1939-1950, 2086-2094, 2115-2125
go/nemo_relay/observability_plugin.go (1)
146-146: LGTM!crates/ffi/nemo_relay.h (1)
1324-1343: LGTM!Also applies to: 1398-1417
crates/ffi/src/api/observability.rs (1)
19-19: LGTM!Also applies to: 689-725, 783-787, 920-956, 1014-1018
crates/node/tests/openinference_tests.mjs (1)
72-78: LGTM!crates/node/tests/otel_tests.mjs (1)
68-74: LGTM!crates/python/src/py_types/observability.rs (1)
17-30: LGTM!Also applies to: 398-398, 436-436, 455-455, 487-503, 597-597, 635-635, 654-654, 686-702
python/nemo_relay/_native.pyi (1)
967-974: LGTM!Also applies to: 1047-1054
python/nemo_relay/observability.py (1)
201-201: LGTM!Also applies to: 219-219
python/nemo_relay/observability.pyi (1)
78-78: LGTM!python/tests/test_observability_plugin.py (1)
43-43: LGTM!python/tests/test_types.py (1)
572-573: LGTM!Also applies to: 585-586, 666-667, 679-680
crates/core/src/observability/mod.rs (1)
9-31: LGTM!Also applies to: 75-95, 97-169, 170-205, 357-435
crates/core/src/observability/openinference.rs (3)
24-28: LGTM!Also applies to: 82-84, 112-129, 217-236, 257-353
501-512: LGTM!Also applies to: 535-569, 591-629
631-691: LGTM!Also applies to: 780-877, 1447-1515, 1532-1533
crates/core/src/observability/otel.rs (3)
24-28: LGTM!Also applies to: 43-44, 75-77, 96-123, 211-230, 246-347
494-506: LGTM!Also applies to: 528-562, 584-623
625-678: LGTM!Also applies to: 763-799, 979-1044
crates/core/tests/integration/middleware_tests.rs (1)
958-969: LGTM!Also applies to: 989-1000
crates/core/tests/unit/observability/exporter_parity_tests.rs (2)
445-448: LGTM!Also applies to: 622-623, 655-668
711-722: LGTM!crates/core/tests/unit/observability/openinference_tests.rs (2)
690-794: LGTM!
851-878: LGTM!Also applies to: 979-984, 1054-1075, 1254-1254, 1962-1967, 2400-2409, 2771-2776, 2818-2823, 2960-2963, 3035-3040, 3947-3953, 4043-4061
crates/core/tests/unit/observability/otel_tests.rs (2)
587-691: LGTM!
742-758: LGTM!Also applies to: 855-861, 1085-1095, 1348-1353, 1442-1442, 1567-1589, 1881-1887
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
225-225: LGTM!Also applies to: 246-254, 300-315, 339-339
Signed-off-by: Will Killian <wkillian@nvidia.com>
7fadff2 to
0c33e81
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/tests/test_observability_plugin.py (1)
39-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest non-empty mapping serialization.
This assertion covers only the default
[]. Add anOtlpConfig(attribute_mappings=[...])case and assert the mapping survivesto_dict()so the new Python serialization path is protected.As per path instructions, Python tests should cover behavior promised by the changed API surface.
🤖 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 `@python/tests/test_observability_plugin.py` around lines 39 - 43, Add a separate assertion in the OtlpConfig serialization tests using a non-empty attribute_mappings value, and verify that OtlpConfig.to_dict() preserves the supplied mapping unchanged while retaining the existing default-configuration assertion.Source: Path instructions
♻️ Duplicate comments (2)
crates/ffi/tests/unit/api/plugin_tests.rs (1)
1302-1382: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConstructor validation is covered, but the mapping is never observed on an exported span.
Same gap flagged previously: this test only checks
NemoRelayStatus::Ok/InvalidArgfrom the constructors, never emits an event through the created subscribers to confirmtenant.id(or similar) actually appears with the expected typed value in the exported span. A regression in attribute-mapping forwarding through the FFI layer could still pass this test.🤖 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 `@crates/ffi/tests/unit/api/plugin_tests.rs` around lines 1302 - 1382, Extend test_ffi_typed_attribute_mapping_constructors_validate_and_accept_mappings to emit an event through each successfully created subscriber and inspect the exported span, asserting the configured tenant.id mapping appears with the expected typed value. Reuse the existing test helpers and export/flush inspection path, and retain the current valid and invalid constructor status assertions.crates/core/tests/unit/observability/exporter_parity_tests.rs (1)
697-721: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winStale
nemo_relay.start.input_jsonkey is still present and will panic.This is the same issue flagged on a previous commit:
start_attributes()now only projectsevent.input()into dotted top-level keys (e.g.nemo_relay.start.input.headers, confirmed two lines below in this same block); it no longer emits anemo_relay.start.input_jsonenvelope. Line 701'sotel.get("nemo_relay.start.input_json").unwrap()will panic withNone.unwrap().🐛 Proposed fix
- let otel_input: Json = - serde_json::from_str(otel.get("nemo_relay.start.input_json").unwrap()).unwrap(); - assert_eq!(otel_input, request_content); - assert_eq!( + assert_eq!( otel.get("nemo_relay.start.input.headers"), Some(&"{}".to_string()) );🤖 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 `@crates/core/tests/unit/observability/exporter_parity_tests.rs` around lines 697 - 721, Update the OTel assertions in the model-call parity test to stop reading the removed nemo_relay.start.input_json envelope and instead validate the projected dotted start-input attributes produced by start_attributes(), including the existing nemo_relay.start.input.headers assertion. Remove the serde_json parsing and request_content equality check tied to that stale key while preserving the output-attribute assertions.
🤖 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 `@crates/core/src/observability/mod.rs`:
- Around line 75-95: Update validate_attribute_mappings so duplicate detection
uses each mapping.alias trimmed of surrounding whitespace, while preserving the
existing blank-alias validation and duplicate error behavior. Store or compare
the normalized alias in aliases so values such as "foo" and " foo" are rejected
as duplicates.
In `@crates/core/tests/unit/observability/otel_tests.rs`:
- Around line 742-748: Correct the assertion in the affected observability test
to use the input attribute key for the task value: update the key from
nemo_relay.start.data.task to nemo_relay.start.input.task, while leaving the
metadata.phase assertion unchanged.
- Around line 1567-1573: Update the query assertion in the event observability
test to use the nemo_relay.start.data.query attribute key, matching the event’s
BaseEvent::builder().data(...) construction; leave the metadata assertion
unchanged.
In `@crates/node/observability.d.ts`:
- Line 61: Rename the public option from attribute_mappings to attributeMappings
in crates/node/observability.d.ts at line 61 and initialize the same camelCase
property in crates/node/observability.js at line 62; preserve the single
translation to the internal snake_case schema at the native boundary.
In `@crates/node/tests/openinference_tests.mjs`:
- Line 35: Extend the positive binding tests to verify emitted attributes, not
just successful construction: in crates/node/tests/openinference_tests.mjs at
line 35, assert the OpenInference span contains the mapped tenant alias and its
typed value; in crates/node/tests/otel_tests.mjs at line 31, assert the emitted
OpenTelemetry span contains tenant.id and its typed value. Use each test’s
existing event/export assertion helpers and preserve the current mapping setup.
In `@python/nemo_relay/observability.py`:
- Line 201: Preserve positional dataclass constructor compatibility by moving
attribute_mappings after timeout_millis in both
python/nemo_relay/observability.py lines 201-201 and
python/nemo_relay/observability.pyi lines 78-78. Keep the runtime declaration
and public stub field order identical so existing positional arguments continue
binding to transport and subsequent fields correctly.
In `@python/tests/test_types.py`:
- Around line 572-573: Extend the existing OpenTelemetry and OpenInference
collector-backed tests around their attribute-mapping setup to verify
binding-level forwarding: configure the mapping with the tenant key and alias,
emit telemetry, and assert the alias appears in the resulting OTLP attributes.
Keep the existing local getter/setter round-trip assertions, and apply the same
forwarding assertion to both referenced test locations.
- Around line 585-587: Add test cases in the attribute_mappings validation tests
for both native configuration types, covering a blank alias and duplicate
aliases in addition to the existing blank key case. Assert each raises
ValueError with the corresponding validation message, preserving coverage of the
shared mapping contract.
---
Outside diff comments:
In `@python/tests/test_observability_plugin.py`:
- Around line 39-43: Add a separate assertion in the OtlpConfig serialization
tests using a non-empty attribute_mappings value, and verify that
OtlpConfig.to_dict() preserves the supplied mapping unchanged while retaining
the existing default-configuration assertion.
---
Duplicate comments:
In `@crates/core/tests/unit/observability/exporter_parity_tests.rs`:
- Around line 697-721: Update the OTel assertions in the model-call parity test
to stop reading the removed nemo_relay.start.input_json envelope and instead
validate the projected dotted start-input attributes produced by
start_attributes(), including the existing nemo_relay.start.input.headers
assertion. Remove the serde_json parsing and request_content equality check tied
to that stale key while preserving the output-attribute assertions.
In `@crates/ffi/tests/unit/api/plugin_tests.rs`:
- Around line 1302-1382: Extend
test_ffi_typed_attribute_mapping_constructors_validate_and_accept_mappings to
emit an event through each successfully created subscriber and inspect the
exported span, asserting the configured tenant.id mapping appears with the
expected typed value. Reuse the existing test helpers and export/flush
inspection path, and retain the current valid and invalid constructor status
assertions.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: ff608ed6-2dc2-4a9d-9daf-b674c823c5cd
📒 Files selected for processing (28)
crates/core/src/observability/mod.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rscrates/core/src/observability/plugin_component.rscrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/observability.d.tscrates/node/observability.jscrates/node/src/api/mod.rscrates/node/tests/observability_plugin_tests.mjscrates/node/tests/openinference_tests.mjscrates/node/tests/otel_tests.mjscrates/python/src/py_types/observability.rsgo/nemo_relay/nemo_relay.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gopython/nemo_relay/_native.pyipython/nemo_relay/observability.pypython/nemo_relay/observability.pyipython/tests/test_observability_plugin.pypython/tests/test_types.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (33)
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/nemo_relay/observability.pypython/tests/test_observability_plugin.pycrates/core/tests/integration/middleware_tests.rspython/tests/test_types.pycrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/node/src/api/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/ffi/src/api/observability.rscrates/core/src/observability/plugin_component.rscrates/python/src/py_types/observability.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/nemo_relay/observability.pycrates/node/tests/observability_plugin_tests.mjspython/tests/test_observability_plugin.pycrates/node/tests/otel_tests.mjscrates/core/tests/integration/middleware_tests.rscrates/node/observability.jscrates/node/observability.d.tscrates/node/tests/openinference_tests.mjspython/tests/test_types.pycrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/node/src/api/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/ffi/src/api/observability.rscrates/core/src/observability/plugin_component.rscrates/python/src/py_types/observability.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/observability.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/nemo_relay/observability.pypython/tests/test_observability_plugin.pypython/tests/test_types.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/nemo_relay/observability.pypython/tests/test_observability_plugin.pygo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/core/tests/integration/middleware_tests.rscrates/node/observability.jsgo/nemo_relay/observability_plugin.gocrates/node/observability.d.tspython/tests/test_types.pycrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/ffi/nemo_relay.hcrates/core/src/observability/mod.rscrates/node/src/api/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/ffi/src/api/observability.rscrates/core/src/observability/plugin_component.rscrates/python/src/py_types/observability.rscrates/core/src/observability/otel.rsgo/nemo_relay/nemo_relay.gocrates/core/src/observability/openinference.rs
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyigo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gopython/nemo_relay/_native.pyicrates/node/src/api/mod.rsgo/nemo_relay/nemo_relay.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyigo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gopython/nemo_relay/_native.pyigo/nemo_relay/nemo_relay.go
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/nemo_relay/observability.pypython/tests/test_observability_plugin.pypython/tests/test_types.pycrates/core/src/observability/mod.rscrates/node/src/api/mod.rscrates/ffi/src/api/observability.rscrates/core/src/observability/plugin_component.rscrates/python/src/py_types/observability.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
python/nemo_relay/observability.pypython/tests/test_observability_plugin.pygo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/node/observability.jsgo/nemo_relay/observability_plugin.gocrates/node/observability.d.tspython/tests/test_types.pygo/nemo_relay/nemo_relay.go
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
python/nemo_relay/observability.pycrates/node/tests/observability_plugin_tests.mjspython/nemo_relay/observability.pyipython/tests/test_observability_plugin.pygo/nemo_relay/otel_test.gocrates/node/tests/otel_tests.mjsgo/nemo_relay/openinference_test.gocrates/core/tests/integration/middleware_tests.rscrates/node/observability.jsgo/nemo_relay/observability_plugin.gocrates/node/observability.d.tscrates/node/tests/openinference_tests.mjspython/tests/test_types.pypython/nemo_relay/_native.pyicrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/ffi/nemo_relay.hcrates/core/src/observability/mod.rscrates/node/src/api/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/ffi/src/api/observability.rscrates/core/src/observability/plugin_component.rscrates/python/src/py_types/observability.rscrates/core/src/observability/otel.rsgo/nemo_relay/nemo_relay.gocrates/core/src/observability/openinference.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
python/nemo_relay/observability.pypython/tests/test_observability_plugin.pygo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/core/tests/integration/middleware_tests.rscrates/node/observability.jsgo/nemo_relay/observability_plugin.gocrates/node/observability.d.tspython/tests/test_types.pycrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/node/src/api/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/ffi/src/api/observability.rscrates/core/src/observability/plugin_component.rscrates/python/src/py_types/observability.rscrates/core/src/observability/otel.rsgo/nemo_relay/nemo_relay.gocrates/core/src/observability/openinference.rs
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyipython/nemo_relay/_native.pyi
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/tests/observability_plugin_tests.mjscrates/node/tests/otel_tests.mjscrates/node/observability.jscrates/node/observability.d.tscrates/node/tests/openinference_tests.mjs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/tests/observability_plugin_tests.mjscrates/node/tests/otel_tests.mjscrates/node/observability.jscrates/node/observability.d.tscrates/node/tests/openinference_tests.mjscrates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/nemo_relay.hcrates/node/src/api/mod.rscrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/node/tests/observability_plugin_tests.mjspython/tests/test_observability_plugin.pygo/nemo_relay/otel_test.gocrates/node/tests/otel_tests.mjsgo/nemo_relay/openinference_test.gocrates/core/tests/integration/middleware_tests.rscrates/node/tests/openinference_tests.mjspython/tests/test_types.pycrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rs
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_observability_plugin.pypython/tests/test_types.py
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/core/tests/integration/middleware_tests.rscrates/node/observability.jsgo/nemo_relay/observability_plugin.gocrates/node/observability.d.tscrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/node/src/api/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/ffi/src/api/observability.rscrates/core/src/observability/plugin_component.rscrates/python/src/py_types/observability.rscrates/core/src/observability/otel.rsgo/nemo_relay/nemo_relay.gocrates/core/src/observability/openinference.rs
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/integration/middleware_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/node/src/api/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/ffi/src/api/observability.rscrates/core/src/observability/plugin_component.rscrates/python/src/py_types/observability.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/plugin_component.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/node/**/*.{js,ts,jsx,tsx,json}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Format changed Node files with
npm run format --workspace=nemo-relay-node
Files:
crates/node/observability.jscrates/node/observability.d.ts
crates/node/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Use
npm run check:docstrings --workspace=nemo-relay-nodeto validate public API docstring checks when surface docs changed
Files:
crates/node/observability.d.ts
crates/ffi/**
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Rebuild the FFI crate in release mode so the shared library and header stay in sync when making changes to crates/ffi
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rs
crates/ffi/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/ffi, also usetest-ffi-surfacefor validationUse C FFI export names prefixed with
nemo_relay_in the raw C FFI layer.
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
crates/ffi/nemo_relay.h
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Check the generated header diff when any exported symbol or type changed in the FFI surface
Update generated or generated-from-build surfaces such as
crates/ffi/nemo_relay.hthrough the proper build step.
Files:
crates/ffi/nemo_relay.h
{crates/ffi/src/api/*.rs,crates/ffi/nemo_relay.h}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Add or update the shared C/FFI surface in the relevant
crates/ffi/src/api/*.rsmodule, re-export it throughcrates/ffi/src/api/mod.rs, and keep the generatedcrates/ffi/nemo_relay.hheader correct.
Files:
crates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rs
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.
Files:
crates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
🧠 Learnings (1)
📚 Learning: 2026-05-07T18:04:44.387Z
Learnt from: mnajafian-nv
Repo: NVIDIA/NeMo-Flow PR: 67
File: integrations/openclaw/src/modules.ts:1-2
Timestamp: 2026-05-07T18:04:44.387Z
Learning: In NVIDIA/NeMo-Flow, TypeScript source files should use `//` line comments for SPDX headers (e.g., `// SPDX-FileCopyrightText: ...` and `// SPDX-License-Identifier: ...`) rather than C-style block comments (`/* ... */`). The repo’s copyright checker enforces this mapping, so `//` SPDX headers in `.ts` files should not be flagged as a style violation.
Applied to files:
crates/node/observability.d.ts
🔇 Additional comments (43)
crates/node/tests/observability_plugin_tests.mjs (1)
34-34: The previously reported Node casing issue remains.This assertion still codifies
attribute_mappingsin the Node-facing helper shape. Update it when the public helper and native-boundary translation are changed.crates/node/observability.d.ts (1)
73-77: LGTM!crates/node/src/api/mod.rs (1)
112-126: LGTM!Also applies to: 187-187, 297-297, 3470-3480, 3505-3506
crates/node/tests/openinference_tests.mjs (1)
72-78: LGTM!crates/node/tests/otel_tests.mjs (1)
68-74: LGTM!crates/ffi/src/api/observability.rs (2)
533-551: 🎯 Functional Correctness | ⚡ Quick winShape errors still return
InvalidJsoninstead ofInvalidArg.
serde_json::from_strdeserializes directly intoVec<OtlpAttributeMapping>, so syntactically valid JSON with the wrong shape ({},null, a non-stringkey/alias) is classified asInvalidJsonrather thanInvalidArg, inconsistent withparse_string_map_json's documented status codes. This was already flagged on a previous revision and remains unresolved.🐛 Proposed fix
- let mappings: Vec<OtlpAttributeMapping> = - serde_json::from_str(&json_string).map_err(|error| { - set_last_error(&format!("invalid attribute_mappings JSON: {error}")); - NemoRelayStatus::InvalidJson - })?; + let value: serde_json::Value = serde_json::from_str(&json_string).map_err(|error| { + set_last_error(&format!("invalid attribute_mappings JSON: {error}")); + NemoRelayStatus::InvalidJson + })?; + let mappings: Vec<OtlpAttributeMapping> = serde_json::from_value(value).map_err(|error| { + set_last_error(&format!("invalid attribute_mappings: {error}")); + NemoRelayStatus::InvalidArg + })?;
689-725: LGTM!Also applies to: 783-787, 920-956, 1014-1018
go/nemo_relay/nemo_relay.go (2)
1840-1846: 📐 Maintainability & Code Quality | 🏗️ Heavy liftGo tests still lack behavior assertions for attribute mappings.
AttributeMappingsis wired through correctly, but neither Go test (otel_test.go,openinference_test.go) asserts the mapped alias is actually emitted with the correct value, or that an invalid mapping is rejected — only subscriber lifecycle success is proven. Flagged previously and still unresolved.As per path instructions, "Any API change should include focused Go tests and consider race-test behavior."
Source: Path instructions
254-254: LGTM!Also applies to: 263-263, 1910-1949, 2086-2125
go/nemo_relay/openinference_test.go (1)
55-58: 📐 Maintainability & Code Quality | 🏗️ Heavy liftMapping is configured but never asserted.
AttributeMappingsis set but the test only checks lifecycle success, not thattenant.idalias is actually emitted or that malformed mappings are rejected via this Go→FFI path. Same gap flagged previously onnemo_relay.go.go/nemo_relay/otel_test.go (1)
48-51: 📐 Maintainability & Code Quality | 🏗️ Heavy liftMapping is configured but never asserted.
Same gap as the OpenInference test:
AttributeMappingsis set but no assertion proves the aliased attribute is emitted correctly or that invalid mappings are rejected through this path. Same gap flagged previously onnemo_relay.go.crates/core/src/observability/plugin_component.rs (1)
54-56: LGTM!Also applies to: 387-389, 426-426, 508-508, 1378-1379, 1418-1419, 2129-2138
crates/ffi/nemo_relay.h (1)
1324-1343: LGTM!Also applies to: 1398-1417
go/nemo_relay/observability_plugin.go (1)
142-156: LGTM!crates/python/src/py_types/observability.rs (1)
17-30: LGTM!Also applies to: 398-398, 436-436, 455-455, 487-503, 597-597, 635-635, 654-654, 686-702
python/nemo_relay/_native.pyi (1)
967-974: LGTM!Also applies to: 1047-1054
python/nemo_relay/observability.py (1)
219-219: LGTM!crates/core/src/observability/mod.rs (4)
9-31: LGTM!
97-169: LGTM! Top-level-only projection with typed scalar handling (including the u64-overflow-to-string fallback) matches the documented contract and is covered by the accompanying test.
170-205: LGTM! Alias resolution correctly protects existing keys and prefers the most recently pushed match, consistent with the doc comment and cross-file lifecycle tests.
357-435: LGTM!crates/core/src/observability/openinference.rs (7)
24-28: LGTM!Also applies to: 82-84, 112-129, 217-236
257-266: LGTM! Validation is correctly enforced at every construction path that accepts mappings, and empty-vec forwarding is consistent for the mapping-less convenience constructors.Also applies to: 280-280, 295-295, 315-353
501-501: LGTM!Also applies to: 512-512, 535-567
599-609: LGTM! Storingprojected_attributesat start and merging with end attributes before alias resolution correctly ensures aliases can reference either start- or end-projected fields while never overriding an already-present key, matching the OpenInference span lifecycle tests.Also applies to: 618-625
640-646: LGTM! Mappings are applied after intrinsic mark fields (orphan flag, span kind) are pushed, so intrinsic fields can't be clobbered by a misconfigured alias.Also applies to: 659-665, 681-681
780-797: LGTM! The expanded LLM detection and metadata retain-filter correctly ensure the final span reflects the completed event's metadata for LLM spans.Also applies to: 832-840
1447-1477: LGTM!Also applies to: 1512-1512
crates/core/src/observability/otel.rs (6)
24-28: LGTM!Also applies to: 43-44, 75-77, 96-123, 211-230
246-260: LGTM! Mirrors the OpenInference subscriber's validation-on-construction pattern.As per coding guidelines,
crates/core/src/observability/{atif,otel,openinference}.rsshould keep ATIF/OTel/OpenInference implementations in sync when the core event model or lifecycle changes — this segment stays consistent withopeninference.rs.Also applies to: 270-291, 313-347
Source: Coding guidelines
494-506: LGTM!Also applies to: 528-562
584-602: LGTM! Same start/end attribute-merge-then-alias pattern asopeninference.rs, verified by the OTel unit tests for typed alias copying and precedence.Also applies to: 605-619
625-640: LGTM! Mapping application ordering (after intrinsic fields are pushed) matchesopeninference.rs, preserving intrinsic-field protection parity.Also applies to: 653-656, 659-669
763-784: LGTM!Also applies to: 979-1012
crates/core/tests/integration/middleware_tests.rs (1)
958-969: LGTM!Also applies to: 987-1000
crates/core/tests/unit/observability/exporter_parity_tests.rs (2)
445-448: LGTM!
655-668: 🎯 Functional CorrectnessUse
nemo_relay.end.output.reasoninghere.llm_end()storesreasoninginevent.output(), and both exporters flatten that payload undernemo_relay.end.output.*;nemo_relay.end.data.reasoningis not emitted for this case.> Likely an incorrect or invalid review comment.crates/core/tests/unit/observability/openinference_tests.rs (1)
690-794: LGTM!Also applies to: 851-880, 979-984, 1054-1075, 1254-1254, 1962-1967, 2400-2409, 2771-2776, 2818-2823, 2960-2963, 3035-3040, 3947-3953, 4043-4061
crates/core/tests/unit/observability/otel_tests.rs (4)
587-691: LGTM!
855-861: LGTM!
1085-1094: LGTM!Also applies to: 1348-1353, 1442-1442
1587-1589: LGTM!Also applies to: 1881-1887
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
225-225: LGTM!Also applies to: 246-254, 300-315, 339-339
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/tests/unit/observability/openinference_tests.rs (1)
3987-4053: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the data key for
status_code
status_codeis supplied via.data(...), so this assertion should checknemo_relay.end.data.status_codeinstead ofnemo_relay.end.output.status_code.🤖 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 `@crates/core/tests/unit/observability/openinference_tests.rs` around lines 3987 - 4053, The assertion for the status code in the observability test uses the wrong attribute namespace. Update the assertion near the end of the test to check nemo_relay.end.data.status_code, matching the status_code field supplied through BaseEvent::builder().data(...), while leaving the expected value unchanged.
♻️ Duplicate comments (4)
python/nemo_relay/observability.py (1)
201-201:⚠️ Potential issue | 🟠 MajorPreserve positional
OtlpConfigcompatibility. Insertingattribute_mappingsbeforetransportshifts all later positional arguments in the public dataclass constructor.
python/nemo_relay/observability.py#L201-L201: moveattribute_mappingsaftertimeout_millis.python/nemo_relay/observability.pyi#L78-L78: mirror the corrected field order.
As per path instructions, binding changes are public API changes and should preserve existing behavior unless a migration path is provided.🤖 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 `@python/nemo_relay/observability.py` at line 201, Restore positional OtlpConfig compatibility by moving attribute_mappings after timeout_millis in python/nemo_relay/observability.py at lines 201-201, and mirror the same field order in python/nemo_relay/observability.pyi at lines 78-78. Preserve all existing later positional constructor arguments and behavior.Source: Path instructions
crates/ffi/tests/unit/api/plugin_tests.rs (1)
1302-1381:⚠️ Potential issue | 🟡 MinorAssert positive attribute projection, not just constructor acceptance. A mapping can be accepted but lost during forwarding or export serialization without these tests detecting it.
crates/ffi/tests/unit/api/plugin_tests.rs#L1302-L1381: emit through the created subscribers and assert the configuredtenant.idalias.crates/node/tests/otel_tests.mjs#L31-L31: add a runtime/exporter assertion that the alias is emitted with the expected typed value.
As per path instructions, tests should cover the behavior promised by the changed API surface.🤖 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 `@crates/ffi/tests/unit/api/plugin_tests.rs` around lines 1302 - 1381, Extend test_ffi_typed_attribute_mapping_constructors_validate_and_accept_mappings in crates/ffi/tests/unit/api/plugin_tests.rs (lines 1302-1381) to emit through both created subscribers and assert that the configured tenant.id alias is forwarded/exported with the expected value. Add a runtime/exporter assertion in crates/node/tests/otel_tests.mjs (line 31) verifying the alias is emitted with the expected typed value; retain the existing constructor validation checks.Source: Path instructions
crates/core/tests/unit/observability/otel_tests.rs (2)
740-758: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winStill asserting the wrong attribute key:
taskcame from.input(...), not.data(...).Unresolved from a previous review round.
.data(json!({"scope": true}))and.input(json!({"task": "scope-start"}))are separate fields; perstart_attributes(),taskshould project undernemo_relay.start.input.task, notnemo_relay.start.data.task. As written this assertion will fail.🐛 Proposed fix
let attributes = attr_map(&span.attributes); assert_eq!( - attributes.get("nemo_relay.start.data.task"), + attributes.get("nemo_relay.start.input.task"), Some(&"scope-start".to_string()) );🤖 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 `@crates/core/tests/unit/observability/otel_tests.rs` around lines 740 - 758, Update the span attribute assertion in the observability test to use nemo_relay.start.input.task for the task value from .input(...), while preserving the expected scope-start value and leaving the data and event attribute assertions unchanged.Source: Path instructions
1565-1573: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winStill asserting the wrong attribute key (mirror-image of the L742 bug):
querycame from.data(...), no.input()was called.Unresolved from a previous review round.
tool_eventis built with.data(json!({"query": "hello"}))only;event.input()isNone, so nonemo_relay.start.input.*keys are emitted. The assertion should targetnemo_relay.start.data.query.🐛 Proposed fix
let start_attributes = attr_map(&start_attributes(&tool_event)); assert_eq!( - start_attributes.get("nemo_relay.start.input.query"), + start_attributes.get("nemo_relay.start.data.query"), Some(&"hello".to_string()) );🤖 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 `@crates/core/tests/unit/observability/otel_tests.rs` around lines 1565 - 1573, Update the query assertion in the tool_event observability test to use the emitted key nemo_relay.start.data.query, since tool_event is populated via data rather than input. Leave the metadata assertion unchanged.
🤖 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 `@crates/core/src/observability/otel.rs`:
- Around line 611-619: Guard every alias/application mapping call in the
event-processing methods with !self.attribute_mappings.is_empty(). Update both
call sites in process_mark and the call site in process_mark_as_tool, including
the shown span-end path, so attribute_mapping_aliases/apply_attribute_mappings
is skipped when no mappings are configured while preserving existing behavior
when mappings exist.
In `@go/nemo_relay/otel_test.go`:
- Around line 48-51: Extend the lifecycle test in go/nemo_relay/otel_test.go at
lines 48-51 to send spans through a mock OTLP collector and assert the emitted
span contains the tenant.id alias attribute; reuse any existing collector-test
helper. Apply the same collector-backed assertion to
go/nemo_relay/openinference_test.go at lines 55-58, ensuring both exporter paths
verify forwarding rather than only local AttributeMappings configuration.
---
Outside diff comments:
In `@crates/core/tests/unit/observability/openinference_tests.rs`:
- Around line 3987-4053: The assertion for the status code in the observability
test uses the wrong attribute namespace. Update the assertion near the end of
the test to check nemo_relay.end.data.status_code, matching the status_code
field supplied through BaseEvent::builder().data(...), while leaving the
expected value unchanged.
---
Duplicate comments:
In `@crates/core/tests/unit/observability/otel_tests.rs`:
- Around line 740-758: Update the span attribute assertion in the observability
test to use nemo_relay.start.input.task for the task value from .input(...),
while preserving the expected scope-start value and leaving the data and event
attribute assertions unchanged.
- Around line 1565-1573: Update the query assertion in the tool_event
observability test to use the emitted key nemo_relay.start.data.query, since
tool_event is populated via data rather than input. Leave the metadata assertion
unchanged.
In `@crates/ffi/tests/unit/api/plugin_tests.rs`:
- Around line 1302-1381: Extend
test_ffi_typed_attribute_mapping_constructors_validate_and_accept_mappings in
crates/ffi/tests/unit/api/plugin_tests.rs (lines 1302-1381) to emit through both
created subscribers and assert that the configured tenant.id alias is
forwarded/exported with the expected value. Add a runtime/exporter assertion in
crates/node/tests/otel_tests.mjs (line 31) verifying the alias is emitted with
the expected typed value; retain the existing constructor validation checks.
In `@python/nemo_relay/observability.py`:
- Line 201: Restore positional OtlpConfig compatibility by moving
attribute_mappings after timeout_millis in python/nemo_relay/observability.py at
lines 201-201, and mirror the same field order in
python/nemo_relay/observability.pyi at lines 78-78. Preserve all existing later
positional constructor arguments and behavior.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 2023d811-9d24-40e4-9bdc-198426659382
📒 Files selected for processing (29)
crates/cli/tests/coverage/plugins_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rscrates/core/src/observability/plugin_component.rscrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/observability.d.tscrates/node/observability.jscrates/node/src/api/mod.rscrates/node/tests/observability_plugin_tests.mjscrates/node/tests/openinference_tests.mjscrates/node/tests/otel_tests.mjscrates/python/src/py_types/observability.rsgo/nemo_relay/nemo_relay.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gopython/nemo_relay/_native.pyipython/nemo_relay/observability.pypython/nemo_relay/observability.pyipython/tests/test_observability_plugin.pypython/tests/test_types.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (33)
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/tests/test_observability_plugin.pycrates/cli/tests/coverage/plugins_tests.rscrates/ffi/tests/unit/api/plugin_tests.rspython/tests/test_types.pypython/nemo_relay/observability.pycrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/python/src/py_types/observability.rscrates/core/src/observability/plugin_component.rscrates/node/src/api/mod.rscrates/ffi/src/api/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/tests/test_observability_plugin.pycrates/node/tests/observability_plugin_tests.mjscrates/node/observability.jscrates/cli/tests/coverage/plugins_tests.rscrates/node/observability.d.tscrates/ffi/tests/unit/api/plugin_tests.rspython/tests/test_types.pycrates/node/tests/otel_tests.mjspython/nemo_relay/observability.pycrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/node/tests/openinference_tests.mjscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/python/src/py_types/observability.rscrates/core/src/observability/plugin_component.rscrates/node/src/api/mod.rscrates/ffi/src/api/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/tests/test_observability_plugin.pypython/tests/test_types.pypython/nemo_relay/observability.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/tests/test_observability_plugin.pygo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/node/observability.jscrates/cli/tests/coverage/plugins_tests.rscrates/node/observability.d.tscrates/ffi/tests/unit/api/plugin_tests.rsgo/nemo_relay/observability_plugin.gopython/tests/test_types.pypython/nemo_relay/observability.pycrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/nemo_relay.hcrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/python/src/py_types/observability.rscrates/core/src/observability/plugin_component.rscrates/node/src/api/mod.rsgo/nemo_relay/nemo_relay.gocrates/ffi/src/api/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/tests/test_observability_plugin.pypython/tests/test_types.pypython/nemo_relay/observability.pycrates/core/src/observability/mod.rscrates/python/src/py_types/observability.rscrates/core/src/observability/plugin_component.rscrates/node/src/api/mod.rscrates/ffi/src/api/observability.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
python/tests/test_observability_plugin.pygo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/node/observability.jscrates/node/observability.d.tsgo/nemo_relay/observability_plugin.gopython/tests/test_types.pypython/nemo_relay/observability.pygo/nemo_relay/nemo_relay.go
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_observability_plugin.pypython/tests/test_types.py
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
python/tests/test_observability_plugin.pycrates/node/tests/observability_plugin_tests.mjsgo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/node/observability.jscrates/cli/tests/coverage/plugins_tests.rscrates/node/observability.d.tscrates/ffi/tests/unit/api/plugin_tests.rsgo/nemo_relay/observability_plugin.gopython/tests/test_types.pycrates/node/tests/otel_tests.mjspython/nemo_relay/observability.pyipython/nemo_relay/observability.pycrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/nemo_relay.hpython/nemo_relay/_native.pyicrates/node/tests/openinference_tests.mjscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/python/src/py_types/observability.rscrates/core/src/observability/plugin_component.rscrates/node/src/api/mod.rsgo/nemo_relay/nemo_relay.gocrates/ffi/src/api/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
python/tests/test_observability_plugin.pygo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/node/observability.jscrates/cli/tests/coverage/plugins_tests.rscrates/node/observability.d.tscrates/ffi/tests/unit/api/plugin_tests.rsgo/nemo_relay/observability_plugin.gopython/tests/test_types.pypython/nemo_relay/observability.pycrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/python/src/py_types/observability.rscrates/core/src/observability/plugin_component.rscrates/node/src/api/mod.rsgo/nemo_relay/nemo_relay.gocrates/ffi/src/api/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
python/tests/test_observability_plugin.pycrates/node/tests/observability_plugin_tests.mjsgo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/cli/tests/coverage/plugins_tests.rscrates/ffi/tests/unit/api/plugin_tests.rspython/tests/test_types.pycrates/node/tests/otel_tests.mjscrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/node/tests/openinference_tests.mjscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/tests/unit/observability/openinference_tests.rs
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/tests/observability_plugin_tests.mjscrates/node/observability.jscrates/node/observability.d.tscrates/node/tests/otel_tests.mjscrates/node/tests/openinference_tests.mjs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/tests/observability_plugin_tests.mjscrates/node/observability.jscrates/node/observability.d.tscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/tests/otel_tests.mjscrates/ffi/nemo_relay.hcrates/node/tests/openinference_tests.mjscrates/python/src/py_types/observability.rscrates/node/src/api/mod.rscrates/ffi/src/api/observability.rs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/node/observability.jscrates/cli/tests/coverage/plugins_tests.rscrates/node/observability.d.tscrates/ffi/tests/unit/api/plugin_tests.rsgo/nemo_relay/observability_plugin.gocrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/python/src/py_types/observability.rscrates/core/src/observability/plugin_component.rscrates/node/src/api/mod.rsgo/nemo_relay/nemo_relay.gocrates/ffi/src/api/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gopython/nemo_relay/observability.pyipython/nemo_relay/observability.pypython/nemo_relay/_native.pyicrates/node/src/api/mod.rsgo/nemo_relay/nemo_relay.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gopython/nemo_relay/observability.pyipython/nemo_relay/observability.pypython/nemo_relay/_native.pyigo/nemo_relay/nemo_relay.go
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gogo/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
crates/node/**/*.{js,ts,jsx,tsx,json}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Format changed Node files with
npm run format --workspace=nemo-relay-node
Files:
crates/node/observability.jscrates/node/observability.d.ts
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/tests/coverage/plugins_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/python/src/py_types/observability.rscrates/core/src/observability/plugin_component.rscrates/node/src/api/mod.rscrates/ffi/src/api/observability.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/node/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Use
npm run check:docstrings --workspace=nemo-relay-nodeto validate public API docstring checks when surface docs changed
Files:
crates/node/observability.d.ts
crates/ffi/**
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Rebuild the FFI crate in release mode so the shared library and header stay in sync when making changes to crates/ffi
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rs
crates/ffi/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/ffi, also usetest-ffi-surfacefor validationUse C FFI export names prefixed with
nemo_relay_in the raw C FFI layer.
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/observability.pyipython/nemo_relay/observability.pypython/nemo_relay/_native.pyi
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/observability.py
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/integration/middleware_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/exporter_parity_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/ffi/nemo_relay.h
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Check the generated header diff when any exported symbol or type changed in the FFI surface
Update generated or generated-from-build surfaces such as
crates/ffi/nemo_relay.hthrough the proper build step.
Files:
crates/ffi/nemo_relay.h
{crates/ffi/src/api/*.rs,crates/ffi/nemo_relay.h}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Add or update the shared C/FFI surface in the relevant
crates/ffi/src/api/*.rsmodule, re-export it throughcrates/ffi/src/api/mod.rs, and keep the generatedcrates/ffi/nemo_relay.hheader correct.
Files:
crates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rs
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.
Files:
crates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
🧠 Learnings (1)
📚 Learning: 2026-05-07T18:04:44.387Z
Learnt from: mnajafian-nv
Repo: NVIDIA/NeMo-Flow PR: 67
File: integrations/openclaw/src/modules.ts:1-2
Timestamp: 2026-05-07T18:04:44.387Z
Learning: In NVIDIA/NeMo-Flow, TypeScript source files should use `//` line comments for SPDX headers (e.g., `// SPDX-FileCopyrightText: ...` and `// SPDX-License-Identifier: ...`) rather than C-style block comments (`/* ... */`). The repo’s copyright checker enforces this mapping, so `//` SPDX headers in `.ts` files should not be flagged as a style violation.
Applied to files:
crates/node/observability.d.ts
🔇 Additional comments (32)
python/tests/test_observability_plugin.py (1)
43-43: LGTM!crates/node/observability.js (1)
62-62: LGTM!crates/cli/tests/coverage/plugins_tests.rs (2)
9-11: LGTM!Also applies to: 203-209, 230-230, 325-325, 1177-1178, 1191-1192, 1207-1208, 1223-1224, 2332-2333
2267-2320: LGTM!Also applies to: 2348-2476
crates/node/tests/otel_tests.mjs (1)
68-74: LGTM!python/nemo_relay/observability.py (1)
219-219: LGTM!crates/core/tests/unit/observability/exporter_parity_tests.rs (1)
445-448: LGTM!Also applies to: 622-624, 655-668, 697-725
crates/node/tests/observability_plugin_tests.mjs (1)
34-34: 📐 Maintainability & Code Quality | ⚡ Quick winStill snake_case:
attribute_mappingsshould beattributeMappingsin the Node API.This test still encodes the snake_case
attribute_mappingsshape. Per coding guidelines, Node.js public APIs must use camelCase; exposeattributeMappingsto JS and translate internally.Source: Coding guidelines
crates/node/tests/openinference_tests.mjs (2)
35-35: 🎯 Functional Correctness | ⚡ Quick winStill missing emitted-attribute assertions for
attributeMappings.Construction with
attributeMappingssucceeds but nothing asserts the mappedtenant.idalias and its typed value actually appear on the emitted OpenInference span, so a regression dropping the mapping before it reaches core config would go undetected.Source: Path instructions
72-78: LGTM!crates/core/tests/integration/middleware_tests.rs (1)
958-969: LGTM!Also applies to: 989-1000
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
60-96: LGTM!Also applies to: 202-254, 257-328, 332-373, 652-714
crates/ffi/nemo_relay.h (1)
1324-1343: LGTM!Also applies to: 1398-1417
python/nemo_relay/_native.pyi (1)
967-974: LGTM!Also applies to: 1047-1054
crates/core/src/observability/openinference.rs (1)
24-28: LGTM!Also applies to: 82-129, 217-353, 498-629, 631-877, 1447-1515
python/tests/test_types.py (2)
572-573: Still missing binding-level forwarding assertion (unresolved from prior review).These assertions only prove local getter/setter round-tripping. The collector-backed end-to-end tests (
test_subscriber_exports_scope_and_mark_events_end_to_end) never configureattribute_mappingsor assert the alias appears in the emitted OTLP attributes. Also applies to lines 666-667.
585-587: Still missing blank-alias/duplicate-alias validation coverage (unresolved from prior review).Only the blank-
keybranch ofvalidate_attribute_mappingsis exercised. Blankaliasand duplicate-alias cases remain untested for bothOpenTelemetryConfigandOpenInferenceConfig. Also applies to lines 679-681.crates/ffi/src/api/observability.rs (2)
533-551: Unresolved from prior review: shape errors still misclassified asInvalidJson.Syntactically-valid-but-wrong-shape payloads (
{},null, non-stringkey) still returnNemoRelayStatus::InvalidJsoninstead ofInvalidArg, inconsistent withparse_string_map_json's documented status codes.🐛 Previously proposed fix (still applicable)
- let mappings: Vec<OtlpAttributeMapping> = - serde_json::from_str(&json_string).map_err(|error| { - set_last_error(&format!("invalid attribute_mappings JSON: {error}")); - NemoRelayStatus::InvalidJson - })?; + let value: serde_json::Value = serde_json::from_str(&json_string).map_err(|error| { + set_last_error(&format!("invalid attribute_mappings JSON: {error}")); + NemoRelayStatus::InvalidJson + })?; + let mappings: Vec<OtlpAttributeMapping> = serde_json::from_value(value).map_err(|error| { + set_last_error(&format!("invalid attribute_mappings: {error}")); + NemoRelayStatus::InvalidArg + })?;
19-19: LGTM!Also applies to: 689-726, 783-804, 909-936, 1014-1035
go/nemo_relay/observability_plugin.go (1)
142-156: LGTM!crates/python/src/py_types/observability.rs (1)
17-31: LGTM!Also applies to: 398-398, 436-436, 455-455, 487-503, 597-597, 635-635, 654-654, 686-702
crates/core/src/observability/otel.rs (1)
24-28: LGTM!Also applies to: 57-81, 95-127, 129-231, 244-402, 491-562, 584-609, 763-799, 979-1013
crates/node/observability.d.ts (2)
61-61: 🗄️ Data Integrity & Integration | ⚡ Quick winStill snake_case:
attribute_mappingsshould beattributeMappings.This is the same mismatch flagged previously — the public Node config still exposes
attribute_mappingswhile the intended convention is camelCase, translated to the internal snake_case schema only at the native boundary. Unresolved since last review.As per coding guidelines, Node.js public APIs use camelCase.
Source: Coding guidelines
73-78: LGTM!crates/core/src/observability/mod.rs (2)
79-93: 🎯 Functional Correctness | ⚡ Quick winAlias duplicate check still doesn't trim before comparing.
mapping.alias.trim().is_empty()validates the trimmed value, butaliases.insert(mapping.alias.as_str())inserts the raw alias, so"foo"and" foo"still pass as distinct. This was flagged previously and remains unresolved.🐛 Proposed fix
- if !aliases.insert(mapping.alias.as_str()) { + if !aliases.insert(mapping.alias.trim()) { return Err(format!( "attribute mapping alias {:?} is duplicated", mapping.alias )); }
97-204: LGTM!Also applies to: 357-435
go/nemo_relay/nemo_relay.go (2)
1840-1847: 📐 Maintainability & Code Quality | 🏗️ Heavy liftStill no behavior test for either Go mapping path.
Per the referenced Go test evidence,
TestOpenTelemetrySubscriberLifecycle/TestOpenInferenceSubscriberLifecycleonly setAttributeMappingsand assert lifecycle success — they don't verify the alias is actually projected with the correct typed value, nor that invalid mappings are rejected. Unresolved from the previous review round.As per path instructions, "Any API change should include focused Go tests and consider race-test behavior."
Also applies to: 2022-2022
Source: Path instructions
1830-1847: LGTM on the CGo marshaling/FFI wiring itself — nil-checked marshaling, correctly scopeddefer C.free, and FFI argument order matches the extern declarations.Also applies to: 1910-1949, 2012-2023, 2086-2125
crates/core/tests/unit/observability/otel_tests.rs (1)
587-691: LGTM!Also applies to: 855-860, 1085-1094, 1348-1353, 1442-1442, 1881-1886
crates/core/src/observability/plugin_component.rs (1)
54-56: LGTM!Also applies to: 390-392, 429-429, 506-536, 707-731, 1401-1404, 1441-1444, 2154-2163
crates/node/src/api/mod.rs (1)
112-127: LGTM!Also applies to: 187-187, 297-297, 3451-3481, 3486-3507
crates/core/tests/unit/observability/openinference_tests.rs (1)
690-794: LGTM!Also applies to: 851-878, 979-985, 1054-1075, 1254-1254, 1962-1968, 2400-2410, 2771-2776, 2818-2824, 2960-2967, 3035-3041, 3947-3954
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/node/tests/otel_tests.mjs`:
- Around line 110-111: Replace the independent byte checks with decoded OTLP
request assertions that locate the tenant.id attribute and verify its exact
string value. Update crates/node/tests/otel_tests.mjs lines 110-111 to expect
the mapped node value; crates/node/tests/openinference_tests.mjs lines 114-115
to expect the scope-end tenant node value; go/nemo_relay/otel_test.go lines
171-173 to expect mark metadata go; and go/nemo_relay/openinference_test.go
lines 150-152 to expect scope-end tenant go.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 1c4d45eb-eb42-4241-8093-2ae3674408e6
📒 Files selected for processing (6)
crates/ffi/src/api/observability.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/tests/openinference_tests.mjscrates/node/tests/otel_tests.mjsgo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (21)
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/tests/otel_tests.mjscrates/node/tests/openinference_tests.mjs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/node/tests/otel_tests.mjscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/tests/openinference_tests.mjscrates/ffi/src/api/observability.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/node/tests/otel_tests.mjscrates/ffi/tests/unit/api/plugin_tests.rsgo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/node/tests/openinference_tests.mjscrates/ffi/src/api/observability.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/tests/otel_tests.mjscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/tests/openinference_tests.mjscrates/ffi/src/api/observability.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/node/tests/otel_tests.mjscrates/ffi/tests/unit/api/plugin_tests.rsgo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/node/tests/openinference_tests.mjs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
crates/ffi/**
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Rebuild the FFI crate in release mode so the shared library and header stay in sync when making changes to crates/ffi
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
crates/ffi/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/ffi, also usetest-ffi-surfacefor validationUse C FFI export names prefixed with
nemo_relay_in the raw C FFI layer.
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/ffi/tests/unit/api/plugin_tests.rsgo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/ffi/src/api/observability.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/ffi/tests/unit/api/plugin_tests.rsgo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/ffi/src/api/observability.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/ffi/tests/unit/api/plugin_tests.rsgo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/ffi/src/api/observability.rs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
{crates/ffi/src/api/*.rs,crates/ffi/nemo_relay.h}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Add or update the shared C/FFI surface in the relevant
crates/ffi/src/api/*.rsmodule, re-export it throughcrates/ffi/src/api/mod.rs, and keep the generatedcrates/ffi/nemo_relay.hheader correct.
Files:
crates/ffi/src/api/observability.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/ffi/src/api/observability.rs
🔇 Additional comments (4)
crates/node/tests/otel_tests.mjs (1)
16-18: LGTM!Also applies to: 86-92
go/nemo_relay/otel_test.go (1)
88-95: 📐 Maintainability & Code QualityRun the required Go binding validation before handoff.
No Go validation results are provided. Run
cd go/nemo_relay && go fmt ./...,go vet ./...,just test-go, anduv run pre-commit run --all-files.
go/nemo_relay/otel_test.go#L88-L95: validate the OpenTelemetry invalid-mapping path.go/nemo_relay/openinference_test.go#L94-L101: validate the OpenInference invalid-mapping path.As per coding guidelines: “Format changed Go packages with
cd go/nemo_relay && go fmt ./...” and “Run Go tests withjust test-go.”Source: Coding guidelines
crates/node/tests/openinference_tests.mjs (1)
86-103: LGTM!crates/ffi/src/api/observability.rs (1)
540-547: 📐 Maintainability & Code QualityRun the required Rust and FFI validation before handoff.
No validation results are provided for these Rust/FFI changes. Run
cargo fmt --all,just test-rust,cargo clippy --workspace --all-targets -- -D warnings,cargo deny check, the repository’stest-ffi-surfacetarget, a release FFI rebuild, anduv run pre-commit run --all-files.
crates/ffi/src/api/observability.rs#L540-L547: validate the public FFI parsing behavior and generated surface.crates/ffi/tests/unit/api/plugin_tests.rs#L1381-L1415: validate the new invalid-shape coverage against the same FFI artifact.As per coding guidelines: “Any Rust change must run
just test-rust,” “runcargo fmt --all,” and “runcargo clippy --workspace --all-targets -- -D warnings”;crates/ffichanges also require FFI-surface validation and a release rebuild.Source: Coding guidelines
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/observability/mod.rs (1)
425-444: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test for blank-alias branch.
rejects_invalid_attribute_mappingscovers blankkey, duplicate alias, and whitespace-duplicate alias, but never exercises the blank-aliasvalidation branch (Line 84-86 ofvalidate_attribute_mappings).✅ Suggested addition
assert!( super::validate_attribute_mappings(&[OtlpAttributeMapping::new("", "alias")]).is_err() ); + assert!( + super::validate_attribute_mappings(&[OtlpAttributeMapping::new("key", "")]).is_err() + );🤖 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 `@crates/core/src/observability/mod.rs` around lines 425 - 444, Add a case to the rejects_invalid_attribute_mappings test that passes an OtlpAttributeMapping with a valid key and blank alias, and assert that validate_attribute_mappings returns an error, covering the blank-alias branch.
🤖 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 `@crates/node/tests/openinference_tests.mjs`:
- Around line 20-28: The protobuf encoding helper assertStringAttribute is
duplicated across Node test files. Remove the duplicate definition from the
current test and reuse the shared or existing assertStringAttribute helper from
otel_tests.mjs, preserving all current assertions and call behavior.
In `@crates/node/tests/otel_tests.mjs`:
- Around line 20-28: Remove the duplicate assertStringAttribute
protobuf-encoding helper from the OTLP test and reuse the existing consolidated
helper for string attribute assertions. Update its call sites as needed while
preserving the current assertion behavior and error context.
In `@go/nemo_relay/otel_test.go`:
- Around line 16-25: Update assertOtlpStringAttribute to encode protobuf field
and length values using proper varint encoding rather than byte(len(...)) casts,
including the key length, value length, and enclosing attribute length. Preserve
the existing OTLP byte layout and assertion behavior while ensuring keys and
values of 128 bytes or more are encoded correctly.
In `@python/tests/test_types.py`:
- Line 602: Update the pytest.raises match patterns at both referenced locations
in test_types.py to raw strings, preserving the intentional .* wildcard while
satisfying Ruff RUF043.
---
Outside diff comments:
In `@crates/core/src/observability/mod.rs`:
- Around line 425-444: Add a case to the rejects_invalid_attribute_mappings test
that passes an OtlpAttributeMapping with a valid key and blank alias, and assert
that validate_attribute_mappings returns an error, covering the blank-alias
branch.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 5ad57d28-9691-4c12-8f7c-59e3998d9bf2
📒 Files selected for processing (9)
crates/core/src/observability/mod.rscrates/core/tests/unit/observability/otel_tests.rscrates/node/tests/openinference_tests.mjscrates/node/tests/otel_tests.mjsgo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gopython/nemo_relay/observability.pypython/nemo_relay/observability.pyipython/tests/test_types.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (26)
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/nemo_relay/observability.pypython/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/nemo_relay/observability.pycrates/node/tests/openinference_tests.mjspython/tests/test_types.pycrates/node/tests/otel_tests.mjscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/observability.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/nemo_relay/observability.pypython/tests/test_types.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/nemo_relay/observability.pygo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gopython/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyigo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyigo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.go
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/nemo_relay/observability.pypython/tests/test_types.pycrates/core/src/observability/mod.rs
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
python/nemo_relay/observability.pygo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gopython/tests/test_types.py
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyigo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gocrates/node/tests/openinference_tests.mjspython/tests/test_types.pycrates/node/tests/otel_tests.mjscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
python/nemo_relay/observability.pygo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gopython/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyi
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.go
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
go/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gocrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.go
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
go/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gocrates/node/tests/openinference_tests.mjspython/tests/test_types.pycrates/node/tests/otel_tests.mjscrates/core/tests/unit/observability/otel_tests.rs
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/tests/openinference_tests.mjscrates/node/tests/otel_tests.mjs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/tests/openinference_tests.mjscrates/node/tests/otel_tests.mjs
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_types.py
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rs
🪛 Ruff (0.15.21)
python/tests/test_types.py
[warning] 602-602: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
[warning] 707-707: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
🔇 Additional comments (11)
crates/core/tests/unit/observability/otel_tests.rs (2)
742-757: LGTM!Also applies to: 855-860, 1085-1094, 1348-1353, 1442-1442, 1567-1572, 1587-1588, 1881-1886
587-691: 🎯 Functional CorrectnessNo change needed here. These assertions already inspect the raw
span.attributesvector, so duplicate aliases would not be hidden byattr_map.> Likely an incorrect or invalid review comment.crates/core/src/observability/mod.rs (2)
76-95: LGTM!
97-208: LGTM!crates/node/tests/openinference_tests.mjs (1)
45-124: LGTM!crates/node/tests/otel_tests.mjs (1)
45-120: LGTM!python/nemo_relay/observability.py (1)
195-230: LGTM!python/nemo_relay/observability.pyi (1)
74-88: LGTM!python/tests/test_types.py (1)
95-104: LGTM!Also applies to: 583-584, 596-607, 636-659, 688-689, 701-712, 748-775
go/nemo_relay/openinference_test.go (1)
94-102: LGTM!Also applies to: 103-153
go/nemo_relay/otel_test.go (1)
59-62: LGTM! Strengthened assertion at Line 181 now verifies the mapped alias actually reaches the emitted OTLP body, resolving the previously flagged gap.Also applies to: 132-135, 181-181
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/tests/unit/observability/otel_tests.rs (1)
742-757: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that the legacy
*_jsonattributes are absent.These updates only prove that the new flattened keys exist; a regression emitting both typed and legacy attributes would still pass. Add negative assertions for the removed raw JSON keys, following the explicit
handle_attributes_jsonabsence check at Lines 1348-1353.As per path instructions, tests should cover the behavior promised by the changed API surface, including the flattened schema migration.
Also applies to: 855-860, 1085-1094, 1567-1572, 1587-1588, 1881-1886
🤖 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 `@crates/core/tests/unit/observability/otel_tests.rs` around lines 742 - 757, The observability tests only verify flattened attributes and must also verify legacy JSON attributes are absent. Update the affected assertions in the relevant test cases around the span and event attribute maps to add negative checks for each corresponding *_json key, following the existing handle_attributes_json absence assertion pattern and covering all listed locations.Source: Path instructions
🤖 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 `@python/tests/test_types.py`:
- Around line 95-103: Update _otlp_string_attribute to calculate field lengths
from the UTF-8 encoded key and value bytes, then encode all protobuf length
prefixes using varint encoding rather than single-byte bytes([...]) values.
Preserve the existing OTLP field structure while supporting non-ASCII strings
and payloads of 128 bytes or more.
---
Outside diff comments:
In `@crates/core/tests/unit/observability/otel_tests.rs`:
- Around line 742-757: The observability tests only verify flattened attributes
and must also verify legacy JSON attributes are absent. Update the affected
assertions in the relevant test cases around the span and event attribute maps
to add negative checks for each corresponding *_json key, following the existing
handle_attributes_json absence assertion pattern and covering all listed
locations.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: de3b41b7-1453-407b-bd2e-9c0432caa7fe
📒 Files selected for processing (12)
crates/core/src/observability/mod.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rscrates/core/tests/unit/observability/otel_tests.rscrates/node/tests/openinference_tests.mjscrates/node/tests/otel_tests.mjsgo/nemo_relay/openinference_test.gogo/nemo_relay/otel_test.gopython/nemo_relay/observability.pypython/nemo_relay/observability.pyipython/tests/test_types.pyscripts/test-support/otel_test_utils.mjs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (28)
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/observability.pyipython/nemo_relay/observability.pygo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/observability.pyipython/nemo_relay/observability.pygo/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
python/nemo_relay/observability.pyipython/nemo_relay/observability.pyscripts/test-support/otel_test_utils.mjsgo/nemo_relay/otel_test.gopython/tests/test_types.pycrates/node/tests/otel_tests.mjsgo/nemo_relay/openinference_test.gocrates/node/tests/openinference_tests.mjscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/observability.pyipython/nemo_relay/observability.py
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/nemo_relay/observability.pypython/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/nemo_relay/observability.pyscripts/test-support/otel_test_utils.mjspython/tests/test_types.pycrates/node/tests/otel_tests.mjscrates/node/tests/openinference_tests.mjscrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/observability.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/nemo_relay/observability.pypython/tests/test_types.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/nemo_relay/observability.pygo/nemo_relay/otel_test.gopython/tests/test_types.pygo/nemo_relay/openinference_test.gocrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/nemo_relay/observability.pypython/tests/test_types.pycrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
python/nemo_relay/observability.pygo/nemo_relay/otel_test.gopython/tests/test_types.pygo/nemo_relay/openinference_test.go
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
python/nemo_relay/observability.pygo/nemo_relay/otel_test.gopython/tests/test_types.pygo/nemo_relay/openinference_test.gocrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}
⚙️ CodeRabbit configuration file
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.
Files:
scripts/test-support/otel_test_utils.mjs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.gocrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/otel_test.gogo/nemo_relay/openinference_test.go
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
go/nemo_relay/otel_test.gopython/tests/test_types.pycrates/node/tests/otel_tests.mjsgo/nemo_relay/openinference_test.gocrates/node/tests/openinference_tests.mjscrates/core/tests/unit/observability/otel_tests.rs
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_types.py
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/tests/otel_tests.mjscrates/node/tests/openinference_tests.mjs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/tests/otel_tests.mjscrates/node/tests/openinference_tests.mjs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.
Files:
crates/core/src/observability/otel.rscrates/core/src/observability/openinference.rs
🔇 Additional comments (17)
crates/node/tests/openinference_tests.mjs (1)
7-7: LGTM!Also applies to: 35-35, 72-79, 82-92, 103-103, 114-114
crates/node/tests/otel_tests.mjs (1)
7-7: LGTM!Also applies to: 16-19, 35-35, 72-79, 81-92, 110-110
crates/core/tests/unit/observability/otel_tests.rs (1)
587-691: LGTM!Also applies to: 1348-1353, 1442-1442
scripts/test-support/otel_test_utils.mjs (1)
8-34: LGTM!Also applies to: 98-100, 118-120
go/nemo_relay/openinference_test.go (1)
55-58: LGTM!Also applies to: 94-102, 103-150
go/nemo_relay/otel_test.go (1)
7-8: LGTM!Also applies to: 17-30, 64-67, 103-111, 137-140, 183-186
python/nemo_relay/observability.py (1)
210-210: LGTM!Also applies to: 219-219
python/nemo_relay/observability.pyi (1)
87-87: LGTM!python/tests/test_types.py (1)
583-584: LGTM!Also applies to: 596-607, 631-662, 688-689, 701-712, 743-778
crates/core/src/observability/mod.rs (3)
76-95: LGTM!
183-207: LGTM!
425-445: LGTM!crates/core/src/observability/openinference.rs (3)
599-615: LGTM!
617-636: LGTM!
617-636: 🗄️ Data Integrity & IntegrationATIF does not need a parallel
attribute_mappingssurface.crates/core/src/observability/atif.rsonly exposesAtifExporter::new(session_id, agent_info)and exports structured ATIF trajectory fields, so the OTLP mapping/config changes in OpenTelemetry/OpenInference have no ATIF counterpart here.> Likely an incorrect or invalid review comment.crates/core/src/observability/otel.rs (2)
592-608: LGTM!
610-630: LGTM!
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/core/src/observability/otel.rs (2)
239-370: 📐 Maintainability & Code Quality | 🔵 TrivialConstructor surface keeps growing alongside the new composable options API.
Same pattern as
openinference.rs:from_tracer_provider_with_optionsis additive, but the four older constructors remain, continuing the constructor-sprawl concern raised in an earlier review round on this file (line 334).See consolidated comment for the cross-file fix.
🤖 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 `@crates/core/src/observability/otel.rs` around lines 239 - 370, Consolidate the OpenTelemetry subscriber constructors around from_tracer_provider_with_options: remove the older convenience constructors from_tracer_provider, from_tracer_provider_with_mark_projection, from_tracer_provider_with_mark_projection_and_exclusions, and from_tracer_provider_with_attribute_mappings, and have callers construct OpenTelemetrySubscriberOptions explicitly while preserving their existing defaults and validation behavior.
650-659: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
process_endclones the full end-attribute set instead of filtering likeprocess_startdoes.
projected_attributes.extend(attributes.iter().cloned())(line 653) clones every end attribute, undoing the exact memory-bloat fix (attribute_mapping_inputs) applied one call earlier on the start path (line 630).See consolidated comment for the proposed fix (shared with
openinference.rs).🤖 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 `@crates/core/src/observability/otel.rs` around lines 650 - 659, Update process_end so projected_attributes receives only the filtered attribute-mapping inputs, matching process_start’s use of attribute_mapping_inputs, rather than cloning every end attribute via attributes.iter().cloned(). Preserve alias generation through attribute_mapping_aliases and continue applying the complete end attributes to the span.crates/core/src/observability/openinference.rs (2)
656-665: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
process_endclones the full end-attribute set instead of filtering likeprocess_startdoes.
projected_attributes.extend(attributes.iter().cloned())clones every end attribute (including large payloads) to compute aliases, undoing the memory-bloat fix (attribute_mapping_inputs) just applied on the start side.See consolidated comment for the proposed fix (shared with
otel.rs).🤖 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 `@crates/core/src/observability/openinference.rs` around lines 656 - 665, Update process_end to compute attribute-mapping aliases using only the required attribute-mapping inputs, matching process_start, instead of cloning the full end_attributes set into projected_attributes. Reuse the existing attribute_mapping_inputs filtering approach and preserve setting the complete end attribute set on the span.
245-375: 📐 Maintainability & Code Quality | 🔵 TrivialConstructor surface keeps growing alongside the new composable options API.
from_tracer_provider_with_optionsis a good addition, butfrom_tracer_provider,_with_mark_projection,_with_mark_projection_and_exclusions, and_with_attribute_mappingsremain as separate public constructors doing overlapping work. This is a continuation of the earlier open discussion about constructor sprawl.See consolidated comment for the cross-file fix.
🤖 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 `@crates/core/src/observability/openinference.rs` around lines 245 - 375, Consolidate the overlapping constructors into from_tracer_provider_with_options and remove the separate public from_tracer_provider, from_tracer_provider_with_mark_projection, from_tracer_provider_with_mark_projection_and_exclusions, and from_tracer_provider_with_attribute_mappings methods. Update their callers to construct OpenInferenceSubscriberOptions with equivalent defaults and settings, preserving validation and existing behavior.
🤖 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.
Outside diff comments:
In `@crates/core/src/observability/openinference.rs`:
- Around line 656-665: Update process_end to compute attribute-mapping aliases
using only the required attribute-mapping inputs, matching process_start,
instead of cloning the full end_attributes set into projected_attributes. Reuse
the existing attribute_mapping_inputs filtering approach and preserve setting
the complete end attribute set on the span.
- Around line 245-375: Consolidate the overlapping constructors into
from_tracer_provider_with_options and remove the separate public
from_tracer_provider, from_tracer_provider_with_mark_projection,
from_tracer_provider_with_mark_projection_and_exclusions, and
from_tracer_provider_with_attribute_mappings methods. Update their callers to
construct OpenInferenceSubscriberOptions with equivalent defaults and settings,
preserving validation and existing behavior.
In `@crates/core/src/observability/otel.rs`:
- Around line 239-370: Consolidate the OpenTelemetry subscriber constructors
around from_tracer_provider_with_options: remove the older convenience
constructors from_tracer_provider, from_tracer_provider_with_mark_projection,
from_tracer_provider_with_mark_projection_and_exclusions, and
from_tracer_provider_with_attribute_mappings, and have callers construct
OpenTelemetrySubscriberOptions explicitly while preserving their existing
defaults and validation behavior.
- Around line 650-659: Update process_end so projected_attributes receives only
the filtered attribute-mapping inputs, matching process_start’s use of
attribute_mapping_inputs, rather than cloning every end attribute via
attributes.iter().cloned(). Preserve alias generation through
attribute_mapping_aliases and continue applying the complete end attributes to
the span.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 390f9ef2-968e-4423-bb9e-bcfff6aa9575
📒 Files selected for processing (6)
crates/core/src/observability/mod.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/tests/unit/observability/otel_tests.rspython/tests/test_types.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/tests/test_types.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/tests/test_types.pycrates/core/src/observability/mod.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
python/tests/test_types.py
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_types.py
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
python/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
python/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
python/tests/test_types.pycrates/core/tests/unit/observability/otel_tests.rscrates/core/tests/unit/observability/openinference_tests.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/observability/otel_tests.rscrates/core/src/observability/mod.rscrates/core/tests/unit/observability/openinference_tests.rscrates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
crates/core/src/observability/{atif,otel,openinference}.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
When changing the core event model, emitted fields, exporter behavior, subscriber config, or lifecycle, keep the ATIF, OpenTelemetry, and OpenInference implementations in sync.
Files:
crates/core/src/observability/openinference.rscrates/core/src/observability/otel.rs
🔇 Additional comments (6)
python/tests/test_types.py (1)
95-108: LGTM!Also applies to: 588-612, 636-668, 694-718, 749-782
crates/core/tests/unit/observability/otel_tests.rs (1)
587-671: LGTM!Also applies to: 764-779, 877-882, 1107-1116, 1370-1375, 1464-1464, 1589-1594, 1609-1610, 1903-1908
crates/core/src/observability/mod.rs (1)
178-198: LGTM! The newattribute_mapping_inputsfilter is exactly the targeted retention fix requested in earlier feedback (only mapping-relevant start attributes are kept instead of the full attribute set), and the new test directly verifies it.Also applies to: 384-494
crates/core/tests/unit/observability/openinference_tests.rs (1)
690-816: LGTM!Also applies to: 873-900, 1001-1006, 1076-1097, 1276-1276, 1984-1989, 2422-2431, 2793-2798, 2840-2845, 2982-2985, 3057-3062, 3969-3975, 4065-4083
crates/core/src/observability/openinference.rs (1)
628-648: LGTM!process_start's filtered retention and the mark-projection paths' ordering (intrinsic fields pushed beforeapply_attribute_mappings) correctly prevent mapped aliases from overwriting intrinsic fields.Also applies to: 671-731
crates/core/src/observability/otel.rs (1)
621-641: LGTM!Also applies to: 665-718
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/core/tests/unit/observability/attribute_projection_tests.rs`:
- Around line 36-112: Add coverage in
projects_typed_json_and_copies_configured_aliases for a top-level boolean
attribute and confirm a null-valued attribute is omitted from the projected
results. Extend rejects_invalid_attribute_mappings to assert that a
whitespace-only alias is rejected, using the existing validation and projection
helpers without changing their behavior.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: e4657fee-10ea-4e2a-b084-de1d8bece58f
📒 Files selected for processing (4)
crates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/observability/mod.rscrates/core/src/observability/plugin_component.rs
🔇 Additional comments (3)
crates/core/src/observability/mod.rs (1)
76-95: 📐 Maintainability & Code QualityRun the required core validation matrix.
Please provide successful results for
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,cargo deny check,validate-change, the Rust/Python/Go/Node matrix, and finaluv run pre-commit run --all-files. As per coding guidelines, “Ifcrates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.”Also applies to: 171-197
Source: Coding guidelines
crates/core/src/observability/plugin_component.rs (1)
392-392: LGTM!Also applies to: 424-439, 506-536, 1390-1425, 1428-1465, 1561-1597, 2140-2165
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
84-95: LGTM!Also applies to: 233-237, 257-348
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
/ok to test e24f692 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/core/tests/unit/observability/attribute_projection_tests.rs`:
- Around line 73-77: Update the assertion for nemo_relay.start.metadata.enabled
in the observability attribute projection test to compare against
opentelemetry::Value::Bool(true) rather than a stringified "true" value. Keep
the existing unset-attribute assertion unchanged.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 62fb72a5-ae8b-41f2-af10-3baa1fdd03f1
📒 Files selected for processing (1)
crates/core/tests/unit/observability/attribute_projection_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/observability/attribute_projection_tests.rs
🔇 Additional comments (2)
crates/core/tests/unit/observability/attribute_projection_tests.rs (2)
44-45: LGTM!Also applies to: 119-121
44-45: 📐 Maintainability & Code QualityVerify the required Rust validation before handoff.
Run
cargo fmt --all,just test-rust,cargo clippy --workspace --all-targets -- -D warnings,uv run pre-commit run --all-files, and the requiredcrates/corebroader validation/full language matrix. As per coding guidelines, Rust changes require these formatting, lint, test, and core validation checks.Also applies to: 73-77, 119-121
Source: Coding guidelines
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/ffi/tests/unit/api/plugin_tests.rs (1)
1548-1582: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover malformed mapping JSON separately.
Line 1548 covers valid JSON with invalid shapes, but neither constructor is tested with malformed JSON and expected
InvalidJson. Add"not-json"coverage for both APIs so the status distinction cannot regress.🤖 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 `@crates/ffi/tests/unit/api/plugin_tests.rs` around lines 1548 - 1582, Extend the tests around nemo_relay_otel_subscriber_create_with_attribute_mappings and nemo_relay_openinference_subscriber_create_with_attribute_mappings with a separate malformed JSON input of "not-json". Assert both constructors return NemoRelayStatus::InvalidJson, while preserving the existing InvalidArg assertions for valid JSON with invalid shapes.Source: Path instructions
python/tests/test_observability_plugin.py (1)
40-50: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover non-empty attribute-mapping serialization.
The current assertion verifies only the empty default. A regression that drops configured mappings from
OtlpConfig.to_dict()would still pass.Suggested test
assert OtlpConfig().to_dict() == { "enabled": False, "mark_projection": "inherit", "mark_exclude_names": ["llm.chunk"], "attribute_mappings": [], "transport": "http_binary", "headers": {}, "resource_attributes": {}, "service_name": "nemo-relay", "timeout_millis": 3000, } + mappings = [{"key": "nemo_relay.start.metadata.tenant", "alias": "tenant.id"}] + assert OtlpConfig(attribute_mappings=mappings).to_dict()["attribute_mappings"] == mappingsAs per path instructions, tests should cover the behavior promised by the changed API surface, including error paths.
🤖 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 `@python/tests/test_observability_plugin.py` around lines 40 - 50, Extend the OtlpConfig serialization test around OtlpConfig.to_dict() with a configuration containing at least one non-empty attribute mapping, then assert the serialized attribute_mappings preserves that mapping and its values. Keep the existing empty-default assertion unchanged.Source: Path instructions
🤖 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 `@python/nemo_relay/observability.py`:
- Line 88: Update the sinks annotation in the relevant observability
configuration to remove the unnecessary quotes around the union type, using the
native union expression directly while preserving the existing list and None
typing.
---
Outside diff comments:
In `@crates/ffi/tests/unit/api/plugin_tests.rs`:
- Around line 1548-1582: Extend the tests around
nemo_relay_otel_subscriber_create_with_attribute_mappings and
nemo_relay_openinference_subscriber_create_with_attribute_mappings with a
separate malformed JSON input of "not-json". Assert both constructors return
NemoRelayStatus::InvalidJson, while preserving the existing InvalidArg
assertions for valid JSON with invalid shapes.
In `@python/tests/test_observability_plugin.py`:
- Around line 40-50: Extend the OtlpConfig serialization test around
OtlpConfig.to_dict() with a configuration containing at least one non-empty
attribute mapping, then assert the serialized attribute_mappings preserves that
mapping and its values. Keep the existing empty-default assertion unchanged.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 95527e4b-910e-4be8-b15b-90b5e3b15d0f
📒 Files selected for processing (18)
crates/cli/tests/coverage/plugins_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/observability.d.tscrates/node/observability.jscrates/node/src/api/mod.rscrates/node/tests/observability_plugin_tests.mjscrates/python/src/py_types/observability.rsgo/nemo_relay/nemo_relay.gogo/nemo_relay/observability_plugin.gopython/nemo_relay/_native.pyipython/nemo_relay/observability.pypython/nemo_relay/observability.pyipython/tests/test_observability_plugin.pypython/tests/test_types.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (32)
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/tests/test_observability_plugin.pypython/nemo_relay/observability.pycrates/cli/tests/coverage/plugins_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/src/api/observability.rspython/tests/test_types.pycrates/python/src/py_types/observability.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/tests/test_observability_plugin.pycrates/node/observability.jscrates/node/tests/observability_plugin_tests.mjscrates/node/observability.d.tspython/nemo_relay/observability.pycrates/cli/tests/coverage/plugins_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/src/api/observability.rspython/tests/test_types.pycrates/python/src/py_types/observability.rs
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/tests/test_observability_plugin.pypython/nemo_relay/observability.pypython/tests/test_types.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/tests/test_observability_plugin.pycrates/node/observability.jscrates/node/observability.d.tspython/nemo_relay/observability.pygo/nemo_relay/observability_plugin.gocrates/cli/tests/coverage/plugins_tests.rscrates/ffi/nemo_relay.hcrates/ffi/tests/unit/api/plugin_tests.rscrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/src/api/observability.rspython/tests/test_types.pygo/nemo_relay/nemo_relay.gocrates/python/src/py_types/observability.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/tests/test_observability_plugin.pypython/nemo_relay/observability.pycrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/ffi/src/api/observability.rspython/tests/test_types.pycrates/python/src/py_types/observability.rs
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
python/tests/test_observability_plugin.pycrates/node/observability.jscrates/node/observability.d.tspython/nemo_relay/observability.pygo/nemo_relay/observability_plugin.gopython/tests/test_types.pygo/nemo_relay/nemo_relay.go
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/test_observability_plugin.pypython/tests/test_types.py
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
python/tests/test_observability_plugin.pycrates/node/observability.jscrates/node/tests/observability_plugin_tests.mjscrates/node/observability.d.tspython/nemo_relay/observability.pypython/nemo_relay/observability.pyigo/nemo_relay/observability_plugin.gocrates/cli/tests/coverage/plugins_tests.rscrates/ffi/nemo_relay.hcrates/ffi/tests/unit/api/plugin_tests.rscrates/node/src/api/mod.rspython/nemo_relay/_native.pyicrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/src/api/observability.rspython/tests/test_types.pygo/nemo_relay/nemo_relay.gocrates/python/src/py_types/observability.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
python/tests/test_observability_plugin.pycrates/node/observability.jscrates/node/observability.d.tspython/nemo_relay/observability.pygo/nemo_relay/observability_plugin.gocrates/cli/tests/coverage/plugins_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/src/api/observability.rspython/tests/test_types.pygo/nemo_relay/nemo_relay.gocrates/python/src/py_types/observability.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
python/tests/test_observability_plugin.pycrates/node/tests/observability_plugin_tests.mjscrates/cli/tests/coverage/plugins_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/core/tests/unit/observability/plugin_component_tests.rspython/tests/test_types.py
crates/node/**/*.{js,ts,jsx,tsx,json}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Format changed Node files with
npm run format --workspace=nemo-relay-node
Files:
crates/node/observability.jscrates/node/observability.d.ts
crates/node/**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
camelCasefor Node.js public APIs.
Files:
crates/node/observability.jscrates/node/tests/observability_plugin_tests.mjscrates/node/observability.d.ts
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/node/observability.jscrates/node/observability.d.tsgo/nemo_relay/observability_plugin.gocrates/cli/tests/coverage/plugins_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/src/api/observability.rsgo/nemo_relay/nemo_relay.gocrates/python/src/py_types/observability.rs
crates/{python,ffi,node}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/node/observability.jscrates/node/tests/observability_plugin_tests.mjscrates/node/observability.d.tscrates/ffi/nemo_relay.hcrates/ffi/tests/unit/api/plugin_tests.rscrates/node/src/api/mod.rscrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rs
crates/node/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Use
npm run check:docstrings --workspace=nemo-relay-nodeto validate public API docstring checks when surface docs changed
Files:
crates/node/observability.d.ts
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/observability.py
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyigo/nemo_relay/observability_plugin.gocrates/node/src/api/mod.rspython/nemo_relay/_native.pyigo/nemo_relay/nemo_relay.go
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyigo/nemo_relay/observability_plugin.gopython/nemo_relay/_native.pyigo/nemo_relay/nemo_relay.go
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/observability.pypython/nemo_relay/observability.pyipython/nemo_relay/_native.pyi
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directlyUse
PascalCasefor public Go APIs.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing the experimental Go binding, format Go code with
gofmtand keepgo vet ./...passing.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep shared plugin helpers in
go/nemo_relayaligned with plugin registration, composition, and lifecycle behavior.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_relay/observability_plugin.gogo/nemo_relay/nemo_relay.go
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/tests/coverage/plugins_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/src/api/mod.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/src/api/observability.rscrates/python/src/py_types/observability.rs
crates/ffi/**
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Rebuild the FFI crate in release mode so the shared library and header stay in sync when making changes to crates/ffi
Files:
crates/ffi/nemo_relay.hcrates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
crates/ffi/nemo_relay.h
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Check the generated header diff when any exported symbol or type changed in the FFI surface
Update generated or generated-from-build surfaces such as
crates/ffi/nemo_relay.hthrough the proper build step.
Files:
crates/ffi/nemo_relay.h
{crates/ffi/src/api/*.rs,crates/ffi/nemo_relay.h}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Add or update the shared C/FFI surface in the relevant
crates/ffi/src/api/*.rsmodule, re-export it throughcrates/ffi/src/api/mod.rs, and keep the generatedcrates/ffi/nemo_relay.hheader correct.
Files:
crates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rs
crates/ffi/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/ffi, also usetest-ffi-surfacefor validationUse C FFI export names prefixed with
nemo_relay_in the raw C FFI layer.
Files:
crates/ffi/tests/unit/api/plugin_tests.rscrates/ffi/src/api/observability.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rs
🧠 Learnings (3)
📚 Learning: 2026-07-14T02:53:44.529Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 414
File: crates/node/tests/observability_plugin_tests.mjs:34-34
Timestamp: 2026-07-14T02:53:44.529Z
Learning: Do not flag camelCase style violations for keys returned by observability plugin configuration helpers (e.g., `observability.otlpConfig()` and similar helpers like `atofConfig()` / `atifConfig()`) in the Node observability module and its tests. These helpers intentionally return the snake_case plugin configuration schema consumed by `plugin.initialize()` and written/read via TOML. This is distinct from the Node public API / native subscriber options (e.g., fields like `attributeMappings`) which follow the camelCase guideline; only the plugin-config schema helpers should be exempt.
Applied to files:
crates/node/observability.jscrates/node/tests/observability_plugin_tests.mjs
📚 Learning: 2026-07-14T02:53:55.471Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 414
File: crates/node/observability.d.ts:61-61
Timestamp: 2026-07-14T02:53:55.471Z
Learning: In `crates/node/observability.d.ts` and `crates/node/observability.js`, treat `OtlpConfig`/`otlpConfig` and related helpers as an intentional mirror of the snake_case TOML/plugin configuration schema consumed by `plugin.initialize()`. Do not apply the usual “Node.js public APIs use camelCase” naming review expectation to this plugin-config schema surface. Instead, camelCase review expectations should apply to the native binding surface (e.g., `OpenTelemetrySubscriber`/`OpenInferenceSubscriber` constructors and their `attributeMappings`), which expose camelCase separately.
Applied to files:
crates/node/observability.jscrates/node/observability.d.ts
📚 Learning: 2026-05-07T18:04:44.387Z
Learnt from: mnajafian-nv
Repo: NVIDIA/NeMo-Flow PR: 67
File: integrations/openclaw/src/modules.ts:1-2
Timestamp: 2026-05-07T18:04:44.387Z
Learning: In NVIDIA/NeMo-Flow, TypeScript source files should use `//` line comments for SPDX headers (e.g., `// SPDX-FileCopyrightText: ...` and `// SPDX-License-Identifier: ...`) rather than C-style block comments (`/* ... */`). The repo’s copyright checker enforces this mapping, so `//` SPDX headers in `.ts` files should not be flagged as a style violation.
Applied to files:
crates/node/observability.d.ts
🪛 ast-grep (0.44.1)
go/nemo_relay/nemo_relay.go
[warning] 347-347: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(status)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
🪛 Ruff (0.15.21)
python/nemo_relay/observability.py
[warning] 88-88: Remove quotes from type annotation
Remove quotes
(UP037)
🔇 Additional comments (22)
crates/core/src/observability/plugin_component.rs (2)
38-46: 📐 Maintainability & Code QualityRun the required Rust validation matrix before merge.
No validation output is included for this
crates/corechange. Confirmcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,cargo deny check,validate-change, the full language matrix, and final pre-commit pass. As per coding guidelines, “Changes tocrates/core... must run the full validation matrix across Rust, Python, Go, and Node.js.”Source: Coding guidelines
152-220: LGTM!Also applies to: 389-446, 492-571, 738-826, 1498-1499, 1538-1539, 1617-1704, 1734-1753, 1877-1889, 1926-2174, 2427-2436, 2490-2491
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
21-90: LGTM!Also applies to: 119-159, 265-330, 376-412, 417-488, 534-758, 760-986
crates/cli/tests/coverage/plugins_tests.rs (1)
12-14: LGTM!Also applies to: 188-220, 659-679, 682-702, 775-801, 824-889, 1485-1513, 1992-2021, 2291-2304, 2582-2599
crates/ffi/nemo_relay.h (1)
1332-1350: LGTM!Also applies to: 1406-1424
crates/ffi/src/api/observability.rs (1)
537-557: LGTM!Also applies to: 790-794, 1021-1025
go/nemo_relay/nemo_relay.go (1)
254-263: LGTM!Also applies to: 1873-1879, 1943-1952, 1972-1982, 2055-2055, 2119-2128, 2148-2158
go/nemo_relay/observability_plugin.go (1)
187-220: LGTM!crates/node/observability.d.ts (1)
9-34: LGTM!Also applies to: 72-72, 84-89
crates/node/observability.js (1)
15-18: LGTM!Also applies to: 61-61
crates/node/src/api/mod.rs (5)
92-105: 🩺 Stability & AvailabilityGuard optional
Symbol.asyncDisposesupport.Module initialization currently errors if the host runtime does not expose
Symbol.asyncDispose, preventing the native module from loading even when dynamic disposal is unused. Guard the property installation or enforce an engine requirement that guarantees the symbol exists.
235-248: 🩺 Stability & AvailabilityReject blank stream URLs at the Node boundary.
This checks only that
urlis present, so an empty or whitespace-only URL reachesAtofStreamSinkConfig::new. The Python binding explicitly rejects blank URLs; mirror that validation here or verify thatAtofExporter::newguarantees identical behavior.Source: Path instructions
49-52: LGTM!Also applies to: 131-145, 206-206, 324-324, 4066-4089, 4092-4306
215-234: LGTM!Also applies to: 249-274, 3432-3437
3389-3412: 📐 Maintainability & Code QualityNo change needed — the JS-facing
AtofExporterAPI already uses camelCase options (outputDirectory,timeoutMillis,headerEnv,fieldNamePolicy,attributeMappings); the snake_case Rust fields here are binding internals.> Likely an incorrect or invalid review comment.crates/node/tests/observability_plugin_tests.mjs (1)
22-23: LGTM!Also applies to: 34-34, 43-43, 54-60, 63-67, 77-79, 112-115, 164-164
crates/python/src/py_types/observability.rs (1)
17-31: LGTM!Also applies to: 445-445, 483-483, 502-502, 534-550, 644-644, 682-682, 701-701, 733-749
python/nemo_relay/_native.pyi (1)
833-898: LGTM!Also applies to: 975-982, 1055-1062, 2276-2312
python/nemo_relay/observability.py (1)
58-87: LGTM!Also applies to: 89-120, 230-230, 239-239, 257-257, 297-301
python/nemo_relay/observability.pyi (1)
23-46: LGTM!Also applies to: 94-94
python/tests/test_observability_plugin.py (1)
18-19: LGTM!Also applies to: 31-39, 53-58, 60-75, 105-123, 145-155
python/tests/test_types.py (1)
94-109: LGTM!Also applies to: 591-592, 604-615, 641-668, 697-698, 710-721, 754-785
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/ffi/tests/unit/api/plugin_tests.rs (1)
1548-1582: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover malformed mapping JSON separately.
Line 1548 covers valid JSON with invalid shapes, but neither constructor is tested with malformed JSON and expected
InvalidJson. Add"not-json"coverage for both APIs so the status distinction cannot regress.🤖 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 `@crates/ffi/tests/unit/api/plugin_tests.rs` around lines 1548 - 1582, Extend the tests around nemo_relay_otel_subscriber_create_with_attribute_mappings and nemo_relay_openinference_subscriber_create_with_attribute_mappings with a separate malformed JSON input of "not-json". Assert both constructors return NemoRelayStatus::InvalidJson, while preserving the existing InvalidArg assertions for valid JSON with invalid shapes.Source: Path instructions
python/tests/test_observability_plugin.py (1)
40-50: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover non-empty attribute-mapping serialization.
The current assertion verifies only the empty default. A regression that drops configured mappings from
OtlpConfig.to_dict()would still pass.Suggested test
assert OtlpConfig().to_dict() == { "enabled": False, "mark_projection": "inherit", "mark_exclude_names": ["llm.chunk"], "attribute_mappings": [], "transport": "http_binary", "headers": {}, "resource_attributes": {}, "service_name": "nemo-relay", "timeout_millis": 3000, } + mappings = [{"key": "nemo_relay.start.metadata.tenant", "alias": "tenant.id"}] + assert OtlpConfig(attribute_mappings=mappings).to_dict()["attribute_mappings"] == mappingsAs per path instructions, tests should cover the behavior promised by the changed API surface, including error paths.
🤖 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 `@python/tests/test_observability_plugin.py` around lines 40 - 50, Extend the OtlpConfig serialization test around OtlpConfig.to_dict() with a configuration containing at least one non-empty attribute mapping, then assert the serialized attribute_mappings preserves that mapping and its values. Keep the existing empty-default assertion unchanged.Source: Path instructions
🤖 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 `@python/nemo_relay/observability.py`:
- Line 88: Update the sinks annotation in the relevant observability
configuration to remove the unnecessary quotes around the union type, using the
native union expression directly while preserving the existing list and None
typing.
---
Outside diff comments:
In `@crates/ffi/tests/unit/api/plugin_tests.rs`:
- Around line 1548-1582: Extend the tests around
nemo_relay_otel_subscriber_create_with_attribute_mappings and
nemo_relay_openinference_subscriber_create_with_attribute_mappings with a
separate malformed JSON input of "not-json". Assert both constructors return
NemoRelayStatus::InvalidJson, while preserving the existing InvalidArg
assertions for valid JSON with invalid shapes.
In `@python/tests/test_observability_plugin.py`:
- Around line 40-50: Extend the OtlpConfig serialization test around
OtlpConfig.to_dict() with a configuration containing at least one non-empty
attribute mapping, then assert the serialized attribute_mappings preserves that
mapping and its values. Keep the existing empty-default assertion unchanged.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 95527e4b-910e-4be8-b15b-90b5e3b15d0f
📒 Files selected for processing (18)
crates/cli/tests/coverage/plugins_tests.rscrates/core/src/observability/plugin_component.rscrates/core/tests/unit/observability/plugin_component_tests.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/observability.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/observability.d.tscrates/node/observability.jscrates/node/src/api/mod.rscrates/node/tests/observability_plugin_tests.mjscrates/python/src/py_types/observability.rsgo/nemo_relay/nemo_relay.gogo/nemo_relay/observability_plugin.gopython/nemo_relay/_native.pyipython/nemo_relay/observability.pypython/nemo_relay/observability.pyipython/tests/test_observability_plugin.pypython/tests/test_types.py
📜 Review details
🔇 Additional comments (22)
crates/core/src/observability/plugin_component.rs (2)
38-46: 📐 Maintainability & Code QualityRun the required Rust validation matrix before merge.
No validation output is included for this
crates/corechange. Confirmcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,cargo deny check,validate-change, the full language matrix, and final pre-commit pass. As per coding guidelines, “Changes tocrates/core... must run the full validation matrix across Rust, Python, Go, and Node.js.”Source: Coding guidelines
152-220: LGTM!Also applies to: 389-446, 492-571, 738-826, 1498-1499, 1538-1539, 1617-1704, 1734-1753, 1877-1889, 1926-2174, 2427-2436, 2490-2491
crates/core/tests/unit/observability/plugin_component_tests.rs (1)
21-90: LGTM!Also applies to: 119-159, 265-330, 376-412, 417-488, 534-758, 760-986
crates/cli/tests/coverage/plugins_tests.rs (1)
12-14: LGTM!Also applies to: 188-220, 659-679, 682-702, 775-801, 824-889, 1485-1513, 1992-2021, 2291-2304, 2582-2599
crates/ffi/nemo_relay.h (1)
1332-1350: LGTM!Also applies to: 1406-1424
crates/ffi/src/api/observability.rs (1)
537-557: LGTM!Also applies to: 790-794, 1021-1025
go/nemo_relay/nemo_relay.go (1)
254-263: LGTM!Also applies to: 1873-1879, 1943-1952, 1972-1982, 2055-2055, 2119-2128, 2148-2158
go/nemo_relay/observability_plugin.go (1)
187-220: LGTM!crates/node/observability.d.ts (1)
9-34: LGTM!Also applies to: 72-72, 84-89
crates/node/observability.js (1)
15-18: LGTM!Also applies to: 61-61
crates/node/src/api/mod.rs (5)
92-105: 🩺 Stability & AvailabilityGuard optional
Symbol.asyncDisposesupport.Module initialization currently errors if the host runtime does not expose
Symbol.asyncDispose, preventing the native module from loading even when dynamic disposal is unused. Guard the property installation or enforce an engine requirement that guarantees the symbol exists.
235-248: 🩺 Stability & AvailabilityReject blank stream URLs at the Node boundary.
This checks only that
urlis present, so an empty or whitespace-only URL reachesAtofStreamSinkConfig::new. The Python binding explicitly rejects blank URLs; mirror that validation here or verify thatAtofExporter::newguarantees identical behavior.Source: Path instructions
49-52: LGTM!Also applies to: 131-145, 206-206, 324-324, 4066-4089, 4092-4306
215-234: LGTM!Also applies to: 249-274, 3432-3437
3389-3412: 📐 Maintainability & Code QualityNo change needed — the JS-facing
AtofExporterAPI already uses camelCase options (outputDirectory,timeoutMillis,headerEnv,fieldNamePolicy,attributeMappings); the snake_case Rust fields here are binding internals.> Likely an incorrect or invalid review comment.crates/node/tests/observability_plugin_tests.mjs (1)
22-23: LGTM!Also applies to: 34-34, 43-43, 54-60, 63-67, 77-79, 112-115, 164-164
crates/python/src/py_types/observability.rs (1)
17-31: LGTM!Also applies to: 445-445, 483-483, 502-502, 534-550, 644-644, 682-682, 701-701, 733-749
python/nemo_relay/_native.pyi (1)
833-898: LGTM!Also applies to: 975-982, 1055-1062, 2276-2312
python/nemo_relay/observability.py (1)
58-87: LGTM!Also applies to: 89-120, 230-230, 239-239, 257-257, 297-301
python/nemo_relay/observability.pyi (1)
23-46: LGTM!Also applies to: 94-94
python/tests/test_observability_plugin.py (1)
18-19: LGTM!Also applies to: 31-39, 53-58, 60-75, 105-123, 145-155
python/tests/test_types.py (1)
94-109: LGTM!Also applies to: 591-592, 604-615, 641-668, 697-698, 710-721, 754-785
🛑 Comments failed to post (1)
python/nemo_relay/observability.py (1)
88-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Remove the unnecessary quoted union annotation.
Ruff UP037 flags this annotation. Use the native union expression directly:
Suggested fix
- sinks: list["AtofFileSinkConfig | AtofStreamSinkConfig"] | None = None + sinks: list[AtofFileSinkConfig | AtofStreamSinkConfig] | None = None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.sinks: list[AtofFileSinkConfig | AtofStreamSinkConfig] | None = None🧰 Tools
🪛 Ruff (0.15.21)
[warning] 88-88: Remove quotes from type annotation
Remove quotes
(UP037)
🤖 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 `@python/nemo_relay/observability.py` at line 88, Update the sinks annotation in the relevant observability configuration to remove the unnecessary quotes around the union type, using the native union expression directly while preserving the existing list and None typing.Source: Linters/SAST tools
|
/ok to test 15e1656 |
|
/merge |
#### Overview Documents the typed OTLP attribute projection implemented by merged PR #414 for the OpenTelemetry and OpenInference exporters. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Documents the ordered `attribute_mappings` configuration for remapping fully qualified projected attributes to backend aliases without changing their OTLP types. - Describes typed dotted projection for lifecycle metadata, start/end fields, handle attributes, and mark data. - Explains top-level JSON encoding for nested objects and arrays, omission of `null` fields, and the retained OpenInference metadata compatibility attribute. - Updates the OpenTelemetry and OpenInference documentation and their observability-skill references. - Contains no implementation changes. - Breaking changes: none. Validation: - `just docs` - Targeted pre-commit hooks for all four changed documentation and skill files - `git diff --check` #### Where should the reviewer start? Start with `docs/configure-plugins/observability/opentelemetry.mdx`, then compare the compatibility behavior in `docs/configure-plugins/observability/openinference.mdx`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: #414 Authors: - Will Killian (https://github.com/willkill07) Approvers: - Maryam Najafian (https://github.com/mnajafian-nv) URL: #415
3 participants
Warning
BREAKING CHANGE: [Observability schema] OpenTelemetry and OpenInference exporters no longer emit raw
*_jsonevent-payload attributes. They now project top-level fields as typed OTLP attributes (nested values remain JSON strings). Update trace queries and dashboards to the new attribute paths; useattribute_mappingswhere a backend needs a compatibility alias.Overview
Project OpenTelemetry and OpenInference event payloads into typed OTLP attributes, with optional backend-specific aliases.
Details
OtlpAttributeMappingand mapping validation to the Rust core and observability plugin configuration.attribute_mappingsan interactive typed list in the built-in configuration editor, with editablekeyandaliasfields.Where should the reviewer start?
Start with
crates/core/src/observability/mod.rs, then review the OpenTelemetry and OpenInference exporters, the configuration component editor schema, and their parity tests.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Closes RELAY-472
Summary by CodeRabbit
New Features
attribute_mappingsin Python, Node.js, Go, and C APIs, including new subscriber constructors, plus editor/UI support.Bug Fixes
*_jsonattributes).Tests