feat: add streaming ATOF exporter#173
Conversation
Signed-off-by: Omkar Mehta <omehta@nvidia.com>
github-actions
Bot
added
size:L
|
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 (15)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (46){crates/python/src/py_api/**/*.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi}📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
python/nemo_relay/**/*⚙️ CodeRabbit configuration file
Files:
{crates/adaptive/**,python/nemo_relay/adaptive.py,python/nemo_relay/plugin.py,go/nemo_relay/adaptive/**,go/nemo_relay/!(adaptive)/**,**/node/**,**/wasm/**}📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Files:
{crates/adaptive/**,python/nemo_relay/plugin.py,go/nemo_relay/**,**/node/**,**/wasm/**}📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Files:
**/*.{wasm,js,ts}{,x}📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Files:
crates/node/**/*.{js,ts,jsx,tsx,json}📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Files:
crates/node/**/*.{ts,tsx,d.ts}📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Files:
**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,go,js,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
crates/node/**/*.{js,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
crates/{python,ffi,node,wasm}/**/*⚙️ CodeRabbit configuration file
Files:
**/*.rs📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
**/{Cargo.toml,**/*.rs}📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Files:
**/*.{h,hpp,c,cpp,rs}📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Files:
crates/**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
{docs/**,README.md,CONTRIBUTING.md}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
{docs/**,README.md,CONTRIBUTING.md,**/*.md}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
{docs/**,README.md}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
{docs/**,examples/**,README.md}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{md,mdx,py,sh,yaml,yml,toml,json}📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Files:
**/*.mdx📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Files:
**/*.{html,md,mdx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/**/*.{md,mdx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/**📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}⚙️ CodeRabbit configuration file
Files:
python/nemo_relay/**/*.py📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
{pyproject.toml,**/*.py}📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Files:
**/*.{py,txt,toml,cfg,yaml,yml}📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Files:
**/*.py📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
go/nemo_relay/**/*.go📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
go/**/*.go📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Files:
{go/nemo_relay/go.mod,go/**/*.go}📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Files:
**/*.go📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Files:
go/nemo_relay/**/*⚙️ CodeRabbit configuration file
Files:
**/test_*.{py,py}📄 CodeRabbit inference engine (.agents/skills/add-integration/SKILL.md)
Files:
python/**/*test*.py📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
Files:
python/**/{conftest.py,*test*.py}📄 CodeRabbit inference engine (.agents/skills/test-python-binding/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:
crates/{core,adaptive}/**📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
crates/{core,adaptive}/**/*.rs⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2026-05-07T18:04:44.387ZApplied to files:
🪛 LanguageTooldocs/observability-plugin/streaming-atof-design.mdx[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) [style] ~40-~40: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) [style] ~41-~41: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) [style] ~101-~101: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) [grammar] ~111-~111: Ensure spelling is correct (QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1) [style] ~112-~112: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) 🔇 Additional comments (24)
WalkthroughThis PR adds TCP-backed streaming ATOF event export: a new ChangesStreaming ATOF Export via TCP
🎯 4 (Complex) | ⏱️ ~45 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/atof.rs`:
- Around line 415-420: The exporter currently treats mpsc::TrySendError::Full as
a terminal error by calling store_stream_error(&state.last_error, ...) which
poisons the exporter; instead, in the match arm for
Err(mpsc::TrySendError::Full(_)) (around
sender.try_send(AtofStreamMessage::Event(value)) / AtofStreamMessage::Event), do
not call store_stream_error or mutate state.last_error: treat it as transient
backpressure by incrementing a transient-dropped counter (e.g.,
state.events_dropped or creating one), optionally log a debug/trace message, and
return/continue without setting last_error so future sends can succeed after the
queue drains. Ensure only genuine fatal errors (other TrySendError variants)
call store_stream_error and update state.last_error.
In `@crates/core/tests/unit/observability/atof_tests.rs`:
- Around line 127-129: The detached sink helpers start_atof_socket_sink and
start_atof_eof_sink spawn threads that can block forever on
TcpListener::accept() and BufReader reads (read_line/read_to_string), so change
them to enforce a bounded accept/read lifetime: make the listener nonblocking or
loop with a deadline using TEST_RECV_TIMEOUT, use TcpStream::set_read_timeout
(or set_read_timeout on accepted streams) before blocking reads, and
break/terminate the thread when the deadline elapses or when a shutdown signal
is received; ensure accepted sockets are closed on timeout so the helper thread
always returns instead of hanging CI.
In `@docs/observability-plugin/atof.mdx`:
- Around line 235-241: Add a short sentence to the AtofStreamingExporter docs
explaining the bounded queue backpressure behavior: state whether the
per-exporter bounded queue will block the producer, drop oldest/newest events,
or surface errors when the receiver cannot keep up, and describe any retry or
overflow policy (e.g., blocking until space, dropping with metric/log, or
returning errors) so users understand reliability/performance tradeoffs; place
this sentence in the paragraph that begins "Use `AtofStreamingExporter`..." and
reference the exporter name `AtofStreamingExporter` and "bounded queue" so
readers can correlate doc text with the implementation.
🪄 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: 9fb0c8b1-4c78-4ab2-a8d7-e2664edc8f37
📒 Files selected for processing (3)
crates/core/src/observability/atof.rscrates/core/tests/unit/observability/atof_tests.rsdocs/observability-plugin/atof.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (21)
{docs/**,README.md,CONTRIBUTING.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
{docs/**,README.md,CONTRIBUTING.md}: For docs-only changes, run targeted checks only if commands, package names, or examples changed. Usejust docsfor docs-site builds andjust docs-linkcheckwhen links changed
Run docs site build withjust docs
Files:
docs/observability-plugin/atof.mdx
{docs/**,README.md,CONTRIBUTING.md,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run docs link validation with
just docs-linkcheckwhen links change
Files:
docs/observability-plugin/atof.mdx
{docs/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify README and docs entry points still match current package names and paths for large or public-facing changes
Files:
docs/observability-plugin/atof.mdx
{docs/**,examples/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify examples still run with documented commands for large or public-facing changes
Files:
docs/observability-plugin/atof.mdx
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
docs/observability-plugin/atof.mdx
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
docs/observability-plugin/atof.mdx
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
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.
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
Files:
docs/observability-plugin/atof.mdx
**/*.{html,md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in HTML and Markdown files using HTML comment syntax
Files:
docs/observability-plugin/atof.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update embedded documentation snippets, patch docs, and binding-support notes if examples or supported bindings changed
Files:
docs/observability-plugin/atof.mdx
docs/**
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run
just docsor./scripts/build-docs.sh htmlto regenerate ignored Fern API reference pages before validation for documentation site changes
Files:
docs/observability-plugin/atof.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/observability-plugin/atof.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.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 work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.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
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Keep SPDX headers on Rust source files. The project is Apache-2.0.
Usesnake_casefor Rust binding naming conventions.
UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Preserve async behavior on the existing tokio-based model i...
Files:
crates/core/tests/unit/observability/atof_tests.rscrates/core/src/observability/atof.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/core/tests/unit/observability/atof_tests.rs
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/core/tests/unit/observability/atof_tests.rscrates/core/src/observability/atof.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/core/tests/unit/observability/atof_tests.rscrates/core/src/observability/atof.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/atof_tests.rscrates/core/src/observability/atof.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/core/tests/unit/observability/atof_tests.rscrates/core/src/observability/atof.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/atof_tests.rscrates/core/src/observability/atof.rs
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/tests/unit/observability/atof_tests.rscrates/core/src/observability/atof.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/atof_tests.rscrates/core/src/observability/atof.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/atof_tests.rs
🔇 Additional comments (4)
docs/observability-plugin/atof.mdx (3)
6-7: LGTM!
267-280: LGTM!
253-255: ⚡ Quick winStreaming exporter lifecycle example is already correct (no missing
force_flush())
AtofStreamingExporter::shutdown()callsforce_flush()internally, soderegister(...) -> shutdown() -> stats()already flushes buffered TCP writes.stats()is just a snapshot of counters/errors and is safe to read aftershutdown().crates/core/src/observability/atof.rs (1)
315-555: 🏗️ Heavy liftRun the full CI test matrix for
crates/corechanges (usejust test-all, notvalidate-change)
just test-allis the repo’s full Rust/Python/Go/Node/OpenClaw/WASM matrix (test-rust,test-python,test-go,test-node,test-openclaw,test-wasm). There is novalidate-changetask in the currentjustfile; runjust test-all(or the repo’s equivalent command if it’s named differently elsewhere) before merging.
|
/ok to test 232c587 |
willkill07
changed the title
Signed-off-by: Omkar Mehta <omehta@nvidia.com>
|
Actionable comments posted: 0 |
|
Current head is now e0835e5. CodeRabbit re-reviewed the update with 0 actionable comments; local validation passed cargo fmt, focused ATOF tests, just test-rust, docs-linkcheck, clippy, and diff-check. Could a vetter re-run validation (/ok to test) for this updated fork commit so the NVIDIA runner matrix can start? |
|
Additional local validation for current head e0835e5: I reran the full repo matrix with the repo-pinned Rust toolchain path first in PATH: |
|
/ok to test e0835e5 |
Signed-off-by: Omkar Mehta <omehta@nvidia.com>
|
Updated the PR head to What changed: the test socket sink now keeps the receiver side alive until exporter shutdown/EOF instead of dropping immediately after the expected event count. That preserves the separate-process streaming behavior under test and avoids the Windows late-flush race that surfaced as Local validation on the same tree passed:
Could a maintainer please re-run validation for the new head when convenient? |
|
/ok to test 42ccba9 |
|
You still have a CI failure: https://github.com/NVIDIA/NeMo-Relay/actions/runs/26613916449/job/78425763052 Additionally, Just having the exporter is one part. We need to expose this to the Observability Plugin... where multiple, named exporters can be defined in the plugin configuration file (and edited via I would really appreciate a design document for this feature rather than jumping straight to an implementation. |
Signed-off-by: Omkar Mehta <omehta@nvidia.com>
Signed-off-by: Omkar Mehta <omehta@nvidia.com>
Signed-off-by: Omkar Mehta <omehta@nvidia.com>
Signed-off-by: Omkar Mehta <omehta@nvidia.com>
Signed-off-by: Omkar Mehta <omehta@nvidia.com>
|
Pushed an update on top of 42ccba9. It keeps the accepted test sockets blocking through shutdown and treats platform-specific TCP close/reset as expected stream termination, which should address the Windows-arm64 10053 failure. Local validation:
|
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 `@docs/observability-plugin/local-cockpit-viewer.mdx`:
- Around line 21-24: Update the four bullet questions in the
docs/observability-plugin/local-cockpit-viewer.mdx list to vary their sentence
openings: keep "What session or prompt is active?" as-is, change the second
"Which model, tool, file, skill, memory, or subagent drove the result?" to
retain meaning but consider rephrasing (e.g., "What model, tool, file, skill,
memory, or subagent drove the result?"), rewrite the third "Which token or cost
fields are observed, and which are unavailable?" to a contrastive form like "Are
token or cost fields observed, or are they unavailable?", and rephrase the
fourth "Which hook or provider limitation explains missing trace detail?" to a
"What" form such as "What hook or provider limitation explains missing trace
detail?" to avoid three consecutive "Which" openings while preserving intent and
clarity.
🪄 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: 6283b078-1c76-48e2-94e9-a5f1fdb4c755
📒 Files selected for processing (3)
crates/core/tests/unit/observability/atof_tests.rsdocs/observability-plugin/about.mdxdocs/observability-plugin/local-cockpit-viewer.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (21)
{docs/**,README.md,CONTRIBUTING.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
{docs/**,README.md,CONTRIBUTING.md}: For docs-only changes, run targeted checks only if commands, package names, or examples changed. Usejust docsfor docs-site builds andjust docs-linkcheckwhen links changed
Run docs site build withjust docs
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
{docs/**,README.md,CONTRIBUTING.md,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run docs link validation with
just docs-linkcheckwhen links change
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
{docs/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify README and docs entry points still match current package names and paths for large or public-facing changes
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
{docs/**,examples/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify examples still run with documented commands for large or public-facing changes
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
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.
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
**/*.{html,md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in HTML and Markdown files using HTML comment syntax
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update embedded documentation snippets, patch docs, and binding-support notes if examples or supported bindings changed
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
docs/**
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run
just docsor./scripts/build-docs.sh htmlto regenerate ignored Fern API reference pages before validation for documentation site changes
Files:
docs/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.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/observability-plugin/local-cockpit-viewer.mdxdocs/observability-plugin/about.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.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 work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.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
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Keep SPDX headers on Rust source files. The project is Apache-2.0.
Usesnake_casefor Rust binding naming conventions.
UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Preserve async behavior on the existing tokio-based model i...
Files:
crates/core/tests/unit/observability/atof_tests.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/core/tests/unit/observability/atof_tests.rs
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/core/tests/unit/observability/atof_tests.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/core/tests/unit/observability/atof_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/atof_tests.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/core/tests/unit/observability/atof_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/atof_tests.rs
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/tests/unit/observability/atof_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/atof_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/atof_tests.rs
🪛 LanguageTool
docs/observability-plugin/local-cockpit-viewer.mdx
[style] ~24-~24: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... observed, and which are unavailable? - Which hook or provider limitation explains mi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~89-~89: You have already used ‘right’ in nearby sentences. Consider using an alternative word to let your writing stand out and sound more polished.
Context: ... preserves lifecycle order. ATIF is the right shape for post-run trajectory analysis,...
(REP_RIGHT)
🔇 Additional comments (4)
crates/core/tests/unit/observability/atof_tests.rs (2)
16-17: LGTM!Also applies to: 121-189, 214-223
336-347: LGTM!Also applies to: 467-524
docs/observability-plugin/about.mdx (1)
92-93: LGTM!docs/observability-plugin/local-cockpit-viewer.mdx (1)
1-235: Run docs link validation fordocs/observability-plugin/local-cockpit-viewer.mdx
Link checking couldn’t be executed here becausejustisn’t available (just: command not found); runjust docs-linkcheckto validate the new internal references.
There was a problem hiding this comment.
You misunderstood my request.
I need a design doc for this streaming ATOF support.
I don't want to conflate this PR (or documentation) with anything related to the local cockpit viewer. This file should not exist.
There was a problem hiding this comment.
The standalone ATOF streaming exporter seems okay, but what about the updates to the Observability plugin shape requested as part of a prior review comment?
Signed-off-by: Omkar Mehta <omehta@nvidia.com>
github-actions
Bot
added
size:XL
2 participants
Overview
Add a streaming ATOF exporter for NeMo Relay so downstream tools can consume canonical ATOF JSONL from a separate local process in real time.
Details
AtofStreamingExporterthat connects to a local receiver and writes newline-delimited canonical ATOF events.Where should the reviewer start?
Start with
crates/core/src/observability/atof.rs, especiallyAtofStreamingExporter, then review the socket receiver tests incrates/core/tests/unit/observability/atof_tests.rs.Validation
Already run locally:
cargo fmt --all --checkcargo clippy --workspace --all-targets -- -D warningscargo test -p nemo-relay streaming_exporter -- --test-threads=1just test-rustjust docsBreaking changes
None intended. This adds a new exporter surface and docs; existing ATOF/ATIF file export behavior remains unchanged.
Related Issues
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests