Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRemoved tracing/instrumentation spans from connection send/recv, Muxer methods, and the stage.poll span; preserved original control flow and public signatures. Added an explicit drop(poll) after polling in the async runtime. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
44461e4 to
c22f853
Compare
There was a problem hiding this comment.
Pull request overview
This PR reduces OpenTelemetry verbosity by removing trace-level span instrumentation from high-frequency operations that were creating thousands of spans per second. The focus shifts OTEL tracing to concentrate on internal block processing flows, making traces more practical and cost-effective.
Changes:
- Removed trace span from stage polling loop in pure-stage executor
- Removed trace-level instrumentation from 4 high-frequency multiplexer methods
- Removed trace spans from network send/recv operations
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/pure-stage/src/tokio.rs | Removed trace span from high-frequency stage poll operation in interpreter loop |
| crates/amaru-protocols/src/mux.rs | Removed trace-level instrument attributes from 4 high-frequency network multiplexer methods (outgoing, next_segment, received, want_next) while retaining debug-level instrumentation for less frequent operations |
| crates/amaru-network/src/connection.rs | Removed trace spans from high-frequency send/recv connection operations while preserving instrumentation on less frequent operations like close |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
c22f853 to
d0122bd
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/pure-stage/src/tokio.rs (1)
417-421: Explicitdrop(poll)is safe and readable, though technically redundant.Reaching line 421 guarantees
poll == Poll::Pending— a unit variant carrying no data — so thedropis a runtime no-op. That said, it's a bonzer signal: it makes the intent clear that poll's lifetime deliberately ends before theeffect.lock().take()dance below. No concerns here, just a heads-up in case a future reader wonders why it's there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/pure-stage/src/tokio.rs` around lines 417 - 421, The explicit drop(poll) is technically redundant because Poll::Pending is unit, but please keep it and add a short comment clarifying intent so future readers don't wonder; locate the poll created by stage.as_mut().poll(&mut Context::from_waker(Waker::noop())) and add a one-line comment immediately before or after drop(poll) explaining that the drop intentionally ends poll's borrow/lifetime before the subsequent effect.lock().take() work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/pure-stage/src/tokio.rs`:
- Around line 417-421: The explicit drop(poll) is technically redundant because
Poll::Pending is unit, but please keep it and add a short comment clarifying
intent so future readers don't wonder; locate the poll created by
stage.as_mut().poll(&mut Context::from_waker(Waker::noop())) and add a one-line
comment immediately before or after drop(poll) explaining that the drop
intentionally ends poll's borrow/lifetime before the subsequent
effect.lock().take() work.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/amaru-network/src/connection.rscrates/amaru-protocols/src/mux.rscrates/pure-stage/src/tokio.rs
💤 Files with no reviewable changes (1)
- crates/amaru-protocols/src/mux.rs
Signed-off-by: jeluard <jeluard@users.noreply.github.com>
d0122bd to
fb1c6f3
Compare
|
Superseded by #723 |
OTEL main focus is to give insights to internal block processing. This PR removes some span creations outside of this flow that creates thousands of spans per second, making it impractical and costly to follow all traces.
Summary by CodeRabbit