feat: decouple instrumentation span level from log verbosity#157
feat: decouple instrumentation span level from log verbosity#157
Conversation
Add EV_TRACE_LEVEL env var to control OTLP span export level independently from RUST_LOG stdout log verbosity. When OTLP is enabled, per-layer filtering applies separate filters to fmt layer (RUST_LOG) and OTLP layer (EV_TRACE_LEVEL). Allows operators to run with RUST_LOG=info (clean logs) while exporting debug-level spans to observability backends like Jaeger.
📝 WalkthroughWalkthroughThe pull request decouples OTLP span export verbosity from stdout log verbosity by introducing a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
bin/ev-reth/src/main.rs (1)
85-96:⚠️ Potential issue | 🟡 MinorLog statement before tracing initialization will be silently dropped.
The
info!macro at line 86 is called beforeinit_tracing()at line 96. Since no subscriber is registered yet, this log will be silently discarded and never appear in output.Consider moving the log after
init_tracing()or removing it if redundant with the launch success message at line 116.🔧 Suggested fix
fn main() { - info!("=== EV-RETH NODE STARTING ==="); - reth_cli_util::sigsegv_handler::install(); // Enable backtraces unless a RUST_BACKTRACE value has already been explicitly provided. if std::env::var_os("RUST_BACKTRACE").is_none() { std::env::set_var("RUST_BACKTRACE", "1"); } // initialize tracing (with optional OTLP support based on env vars) init_tracing(); + info!("=== EV-RETH NODE STARTING ==="); + if let Err(err) =🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/ev-reth/src/main.rs` around lines 85 - 96, The initial info! log is emitted before tracing is initialized, so it will be dropped; move the info!("=== EV-RETH NODE STARTING ===") call to after init_tracing() (or remove it if redundant with the later launch success message) so that the message is logged by the subscriber; locate the log invocation in main() and the init_tracing() call and reorder accordingly to ensure tracing is initialized before emitting the info! message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@bin/ev-reth/src/main.rs`:
- Around line 85-96: The initial info! log is emitted before tracing is
initialized, so it will be dropped; move the info!("=== EV-RETH NODE STARTING
===") call to after init_tracing() (or remove it if redundant with the later
launch success message) so that the message is logged by the subscriber; locate
the log invocation in main() and the init_tracing() call and reorder accordingly
to ensure tracing is initialized before emitting the info! message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc22b28f-8693-4b65-bf66-cca0a14ce571
📒 Files selected for processing (3)
CHANGELOG.mdREADME.mdbin/ev-reth/src/main.rs
|
Claude finished @randygrok's task —— View job PR Review: EV_TRACE_LEVEL FeatureReview Checklist:
📋 SummaryThis PR successfully implements the ✅ Strengths
|
| Aspect | Rating | Notes |
|---|---|---|
| Implementation | ✅ Good | Clean separation of concerns, proper error handling |
| Documentation | ✅ Excellent | Comprehensive README with examples and reference table |
| Error Handling | ✅ Good | Proper fallback logic for environment variable parsing |
| Testing | Only manual verification mentioned, no unit tests for tracing config |
🔍 Detailed Analysis
Security Considerations
- ✅ No security concerns - environment variables are read-only and used for configuration
- ✅ No sensitive data exposure through tracing configuration
Performance Considerations
- ✅ Minimal performance impact - configuration only happens at startup
- ✅ Per-layer filtering is more efficient than global filtering for mixed verbosity needs
Potential Improvements
-
Add Unit Tests: Consider adding tests for the
init_tracing()function to verify:- Proper fallback behavior when
EV_TRACE_LEVELis not set - Correct layer configuration with various environment variable combinations
- OTLP config parsing edge cases
- Proper fallback behavior when
-
Environment Variable Validation: Consider validating
EV_TRACE_LEVELformat at startup and providing helpful error messages for invalid filter expressions
Documentation Quality
The README updates are exemplary:
- Clear explanation of the new feature
- Practical usage examples
- Reference table showing variable interactions
- Good integration with existing documentation structure
🏁 Final Recommendation
Approve with Minor Fix Required
This is a solid implementation that addresses a real operational need. The only blocking issue is the misplaced log statement that will be silently dropped. Once that's fixed, this PR is ready to merge.
The feature successfully:
- ✅ Decouples OTLP span verbosity from stdout logging
- ✅ Maintains backward compatibility
- ✅ Provides clear documentation and examples
- ✅ Uses proper Rust tracing patterns
2 participants
Description
Add
EV_TRACE_LEVELenv var to control OTLP span export level independently fromRUST_LOGstdout log verbosity. When OTLP is enabled, per-layer filtering applies separate filters to fmt layer (RUST_LOG) and OTLP layer (EV_TRACE_LEVEL). This allows operators to run withRUST_LOG=info(clean logs) while exporting debug-level spans to observability backends like Jaeger.Type of Change
Related Issues
Closes #156
Checklist
Testing
just build✓just lint✓just fmt-check✓just test✓ (all 17 test suites pass)EV_TRACE_LEVEL=debugand clean logs withRUST_LOG=infoSummary by CodeRabbit
New Features
EV_TRACE_LEVELenvironment variable to independently control OTLP span export verbosity, separate from stdout logging controlled byRUST_LOG.Documentation