feat: add grpc_total_streams_gauge metric#1037
Conversation
Amp-Thread-ID: https://ampcode.com/threads/T-019cd75a-a0b2-73fd-a77c-8ad8076a0708 Co-authored-by: Amp <amp@ampcode.com>
Manual Deploy AvailableYou can trigger a manual deploy of this PR branch to testnet: Alternative: Comment
Comment updated automatically when the PR is synchronized. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces stream spike metrics tracking for GRPC connections. A new Assessment against linked issues
Suggested reviewers
✨ 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 |
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)
magicblock-chainlink/src/remote_account_provider/chain_laser_actor/stream_manager.rs (1)
360-375:⚠️ Potential issue | 🟠 MajorRefresh the total gauge on every stream-topology change.
update_stream_metrics_with_extra()now countscurrent_new_handleandprogram_sub, but theNone -> Some/Some -> Nonetransitions for those streams still do not call it. As written,grpc_total_streams_gaugestays stale after the first account stream is created, after a program stream is created, and after program subscriptions are cleared.Suggested follow-up
async fn insert_current_new_stream( &mut self, pubkeys: &[&Pubkey], commitment: &CommitmentLevel, from_slot: Option<u64>, ) -> RemoteAccountProviderResult<()> { let request = Self::build_account_request(pubkeys, commitment, from_slot); let LaserStreamWithHandle { stream, handle } = self.stream_factory.subscribe(request).await?; self.stream_map.insert(StreamKey::CurrentNew, stream); self.current_new_handle = Some(handle); + self.update_stream_metrics(); Ok(()) }} else { let mut subscribed_programs = HashSet::new(); subscribed_programs.insert(program_id); let LaserStreamWithHandle { stream, handle } = self .create_program_stream(&subscribed_programs, commitment) .await?; self.stream_map.insert(StreamKey::Program, stream); self.program_sub = Some((subscribed_programs, handle)); + self.update_stream_metrics(); }pub fn clear_program_subscriptions(&mut self) { self.stream_map.remove(&StreamKey::Program); self.program_sub = None; + self.update_stream_metrics(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@magicblock-chainlink/src/remote_account_provider/chain_laser_actor/stream_manager.rs` around lines 360 - 375, The total-streams gauge is not refreshed when current_new_handle or program_sub transition between None and Some; after any assignment or clear of self.current_new_handle or self.program_sub you must call self.update_stream_metrics_with_extra(0) so grpc_total_streams_gauge is recomputed — find the places that set/clear current_new_handle and program_sub in StreamManager (e.g., where new account streams or program subscriptions are created or removed) and add a call to update_stream_metrics_with_extra(0) right after each state change to ensure the gauge stays up-to-date.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@magicblock-metrics/src/metrics/mod.rs`:
- Around line 523-531: The static initializer GRPC_TOTAL_STREAMS_GAUGE currently
calls IntGaugeVec::new(...).unwrap(), introducing a panic; remove the unwrap and
make the metric creation fallible instead — call IntGaugeVec::new(...).map_err
or .ok() and propagate the Result/Option up to your startup/init path (e.g.,
return a Result from your metrics init function) or store an Option/Result in
the static so callers can handle failure; specifically modify the static ref
GRPC_TOTAL_STREAMS_GAUGE initialization to not unwrap, propagate the error from
IntGaugeVec::new, and update the metrics initialization code to log/return the
creation error rather than panicking.
---
Outside diff comments:
In
`@magicblock-chainlink/src/remote_account_provider/chain_laser_actor/stream_manager.rs`:
- Around line 360-375: The total-streams gauge is not refreshed when
current_new_handle or program_sub transition between None and Some; after any
assignment or clear of self.current_new_handle or self.program_sub you must call
self.update_stream_metrics_with_extra(0) so grpc_total_streams_gauge is
recomputed — find the places that set/clear current_new_handle and program_sub
in StreamManager (e.g., where new account streams or program subscriptions are
created or removed) and add a call to update_stream_metrics_with_extra(0) right
after each state change to ensure the gauge stays up-to-date.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23aa04b1-8e06-4583-b291-528383df8285
📒 Files selected for processing (2)
magicblock-chainlink/src/remote_account_provider/chain_laser_actor/stream_manager.rsmagicblock-metrics/src/metrics/mod.rs
Amp-Thread-ID: https://ampcode.com/threads/T-019ce11b-3012-717b-93ec-bf04804122d9 Co-authored-by: Amp <amp@ampcode.com>
Summary
Add
grpc_total_streams_gaugemetric to track total GRPC streams across optimized,unoptimized, current, and program streams. This provides better visibility into stream
lifecycle during reconnections when old and new streams coexist temporarily.
CLOSES: #1035
Details
grpc_total_streams_gauge Metric
Added a new metric that measures the total count of active GRPC streams, including:
This metric captures the actual stream count at any point, including temporary spikes when old streams are being replaced with new ones during optimization cycles.
Summary by CodeRabbit
New Features
Improvements