feat: add size-based operational log rotation#555
Conversation
Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.com>
github-actions
Bot
added
size:L
WalkthroughFile sinks gain optional size-based rotation and retention. Configuration validation, rotating file writing, sink path collision checks, runtime wiring, tests, and operational logging documentation are updated. ChangesOperational log rotation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TOML
participant build_logger
participant SizeRotatingFileWriter
participant FileSystem
TOML->>build_logger: resolve file sink rotation settings
build_logger->>SizeRotatingFileWriter: construct writer with size and retention
SizeRotatingFileWriter->>FileSystem: append active log records
SizeRotatingFileWriter->>FileSystem: rotate and rename retained files at size boundary
FileSystem-->>SizeRotatingFileWriter: return file operation results
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/src/logging/rotation.rs`:
- Around line 122-140: Remove the destination-existence check and
fs::remove_file call from rotate_files, leaving fs::rename to replace any
existing rotated log directly. Preserve the existing source selection,
missing-source skip, iteration order, and error propagation.
🪄 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: 1569cb30-c611-4484-ae42-964d45b073a1
📒 Files selected for processing (8)
crates/cli/src/configuration/logging.rscrates/cli/tests/coverage/shared/config_tests.rscrates/core/src/logging/config.rscrates/core/src/logging/mod.rscrates/core/src/logging/rotation.rscrates/core/src/logging/sink.rscrates/core/tests/coverage/logging_tests.rsdocs/reference/operational-logging.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (20)
**/*.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/src/logging/sink.rscrates/core/src/logging/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/core/src/logging/rotation.rscrates/cli/src/configuration/logging.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_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/src/logging/sink.rscrates/core/src/logging/mod.rscrates/core/src/logging/rotation.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_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/logging/sink.rscrates/core/src/logging/mod.rscrates/core/src/logging/rotation.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/logging/sink.rscrates/core/src/logging/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/core/src/logging/rotation.rscrates/cli/src/configuration/logging.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_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/src/logging/sink.rscrates/core/src/logging/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/core/src/logging/rotation.rscrates/cli/src/configuration/logging.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_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/src/logging/sink.rscrates/core/src/logging/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/core/src/logging/rotation.rscrates/cli/src/configuration/logging.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_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/src/logging/sink.rscrates/core/src/logging/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/core/src/logging/rotation.rscrates/cli/src/configuration/logging.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_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/logging/sink.rscrates/core/src/logging/mod.rscrates/core/src/logging/rotation.rscrates/cli/src/configuration/logging.rscrates/core/src/logging/config.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/src/logging/sink.rsdocs/reference/operational-logging.mdxcrates/core/src/logging/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/core/src/logging/rotation.rscrates/cli/src/configuration/logging.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_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/logging/sink.rscrates/core/src/logging/mod.rscrates/core/src/logging/rotation.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_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/src/logging/sink.rscrates/core/src/logging/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/core/src/logging/rotation.rscrates/cli/src/configuration/logging.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_tests.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/src/logging/sink.rscrates/core/src/logging/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/core/src/logging/rotation.rscrates/cli/src/configuration/logging.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_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/logging/sink.rscrates/core/src/logging/mod.rscrates/core/src/logging/rotation.rscrates/core/src/logging/config.rscrates/core/tests/coverage/logging_tests.rs
**/*.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/reference/operational-logging.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/reference/operational-logging.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/reference/operational-logging.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/reference/operational-logging.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/reference/operational-logging.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/reference/operational-logging.mdx
{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/cli/tests/coverage/shared/config_tests.rscrates/core/tests/coverage/logging_tests.rs
🔇 Additional comments (7)
crates/core/src/logging/config.rs (1)
31-37: LGTM!Also applies to: 229-289, 335-397
crates/core/src/logging/mod.rs (1)
11-11: LGTM!Also applies to: 25-27
crates/cli/src/configuration/logging.rs (1)
10-12: LGTM!Also applies to: 40-41, 106-125
crates/cli/tests/coverage/shared/config_tests.rs (1)
3274-3323: LGTM!crates/core/tests/coverage/logging_tests.rs (1)
5-12: LGTM!Also applies to: 243-243, 300-390, 548-673
docs/reference/operational-logging.mdx (1)
103-114: LGTM!crates/core/src/logging/sink.rs (1)
13-19: LGTM!Also applies to: 28-29, 49-112, 171-187
Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.com>
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
Just one question on maximum files. with rotation naming, .0 -> .9 makes sense (for up to 10) but anything beyond that isn't default sorted.
Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.com>
mnajafian-nv
left a comment
mnajafian-nv
left a comment
There was a problem hiding this comment.
LGTM, plus Will's nit
Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.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 (2)
crates/core/src/logging/rotation.rs (1)
37-45: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not rotate an empty active file for an oversized first record.
When the active file is empty and one record is larger than
max_file_size_bytes, the pre-write size check rotates the empty file into the first backup, then writes the oversized record to the new active file. This wastes a retained slot and can evict historical backups earlier than configured.Only rotate when
current_size > 0(or explicitly reject oversized records), and add a regression test.Proposed guard
- if self.current_size.saturating_add(incoming_len) > self.max_file_size_bytes { + if self.current_size > 0 + && self.current_size.saturating_add(incoming_len) > self.max_file_size_bytes + { self.rotate()?; }Also applies to: 85-91
🤖 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/logging/rotation.rs` around lines 37 - 45, Update rotate_if_needed so an empty active file is never rotated, including when incoming_bytes exceeds max_file_size_bytes; only invoke rotate when current_size is greater than zero and the combined size exceeds the limit. Add a regression test covering an oversized first record and verify no empty backup is created while the record is written successfully.crates/core/tests/coverage/logging_tests.rs (1)
560-673: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftCover runtime rotation failures through the sink error path.
These tests cover successful rotation, boundary handling, retention, and preflight collisions, but not a rename/open failure after the asynchronous sink is running. Add deterministic coverage asserting that the existing sink-error handler reports the failure and Relay remains shutdown-safe.
As per path instructions, tests under
crates/core/tests/**should cover promised behavior, including error paths and cross-request isolation where relevant.🤖 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/coverage/logging_tests.rs` around lines 560 - 673, Add a deterministic test near logging_rotation_* that starts the asynchronous file sink, forces a runtime rotation rename or reopen failure after logging begins, and asserts the existing sink-error handler reports the failure. Also verify the resulting Relay/runtime shutdown completes safely, reusing the established test helpers and error-observation mechanism rather than adding new production behavior.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.
Outside diff comments:
In `@crates/core/src/logging/rotation.rs`:
- Around line 37-45: Update rotate_if_needed so an empty active file is never
rotated, including when incoming_bytes exceeds max_file_size_bytes; only invoke
rotate when current_size is greater than zero and the combined size exceeds the
limit. Add a regression test covering an oversized first record and verify no
empty backup is created while the record is written successfully.
In `@crates/core/tests/coverage/logging_tests.rs`:
- Around line 560-673: Add a deterministic test near logging_rotation_* that
starts the asynchronous file sink, forces a runtime rotation rename or reopen
failure after logging begins, and asserts the existing sink-error handler
reports the failure. Also verify the resulting Relay/runtime shutdown completes
safely, reusing the established test helpers and error-observation mechanism
rather than adding new production behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 6b69dc11-d445-454f-be7f-d59dbf7c985b
📒 Files selected for processing (2)
crates/core/src/logging/rotation.rscrates/core/tests/coverage/logging_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (14)
**/*.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/src/logging/rotation.rscrates/core/tests/coverage/logging_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/src/logging/rotation.rscrates/core/tests/coverage/logging_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/logging/rotation.rscrates/core/tests/coverage/logging_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/logging/rotation.rscrates/core/tests/coverage/logging_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/src/logging/rotation.rscrates/core/tests/coverage/logging_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/src/logging/rotation.rscrates/core/tests/coverage/logging_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/src/logging/rotation.rscrates/core/tests/coverage/logging_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/logging/rotation.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/src/logging/rotation.rscrates/core/tests/coverage/logging_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/logging/rotation.rscrates/core/tests/coverage/logging_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/src/logging/rotation.rscrates/core/tests/coverage/logging_tests.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/src/logging/rotation.rscrates/core/tests/coverage/logging_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/logging/rotation.rscrates/core/tests/coverage/logging_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/coverage/logging_tests.rs
🔇 Additional comments (3)
crates/core/tests/coverage/logging_tests.rs (1)
5-12: LGTM!Also applies to: 243-243, 300-390, 548-558
crates/core/src/logging/rotation.rs (2)
1-16: 📐 Maintainability & Code QualityRun the required Rust/core validation before handoff.
Because this change touches
crates/core, runcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,validate-change, the full Rust/Python/Go/Node validation matrix, anduv run pre-commit run --all-files.As per coding guidelines, Rust changes require formatting, strict clippy, and
just test-rust; changes undercrates/corealso require broader validation.Source: Coding guidelines
18-35: LGTM!Also applies to: 47-80, 94-101, 103-148
|
/merge |
3 participants
Overview
Add optional size-based rotation and retention to Relay operational file-log sinks. File sinks remain append-only unless rotation is explicitly configured.
Details
max_file_size_bytesandretained_filessettings for file sinks.Where should the reviewer start?
Start with the rotating writer in
crates/core/src/logging/rotation.rsand its integration with the asynchronous file sink incrates/core/src/logging/sink.rs. Focused behavior coverage is incrates/core/tests/coverage/logging_tests.rs.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
max_file_size_bytesandretained_files(with aretained_filesmax of 9).