feat(metrics): implement prometheus metrics endpoint#283
Open
ns212 wants to merge 1 commit intodevelopmentfrom
Open
feat(metrics): implement prometheus metrics endpoint#283ns212 wants to merge 1 commit intodevelopmentfrom
ns212 wants to merge 1 commit intodevelopmentfrom
Conversation
Contributor
WalkthroughThis update removes Visual Studio Code theme configurations, adds the Prometheus dependency to the Rust project's dependency file, and introduces a new metrics endpoint. The metrics endpoint is implemented by adding a route in the HTTP server and corresponding asynchronous handler functions for collecting and encoding Prometheus metrics data. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant S as HTTP Server
participant H as Metrics Handler
participant R as Prometheus Registry
participant E as TextEncoder
C->>S: GET /metrics
S->>H: handle_metrics(request)
H->>R: Create & register metrics
H->>H: Call get_metrics_data()
H->>E: Encode collected metrics
E-->>H: Encoded metrics data
H->>S: Return HTTP 200 with metrics payload
S-->>C: HTTP 200 Response with Prometheus metrics
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
p2pool/Cargo.toml (1)
72-72: Confirm optimal Prometheus version.
Addingprometheus = "0.13.4"is fine, but ensure it won't cause dependency resolution or security issues.If you're unsure, consider verifying any open issues or advisories for this specific version.
p2pool/src/server/http/stats/handlers.rs (1)
367-406: Metrics handler flow looks good.
This async function properly collects, encodes, and returns metrics via a Prometheus-friendly format. Consider adding a brief doccomment explaining usage and ensure you have tests verifying proper response on success/failure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.vscode/settings.json(0 hunks)p2pool/Cargo.toml(1 hunks)p2pool/src/server/http/server.rs(1 hunks)p2pool/src/server/http/stats/handlers.rs(3 hunks)
💤 Files with no reviewable changes (1)
- .vscode/settings.json
🧰 Additional context used
🧬 Code Definitions (1)
p2pool/src/server/http/stats/handlers.rs (7)
p2pool/src/server/http/server.rs (1) (1)
new(53-65)p2pool/src/sharechain/in_memory.rs (1) (1)
new(74-181)p2pool/src/server/server.rs (1) (1)
new(52-139)p2pool/src/server/http/stats_collector.rs (2) (2)
new(53-87)new(390-392)p2pool/src/sharechain/mod.rs (1) (1)
new(63-73)p2pool/src/sharechain/p2chain_level.rs (1) (1)
height(127-129)p2pool/src/server/p2p/peer_store.rs (4) (4)
whitelist_peers(192-194)whitelist_peers(211-211)whitelist_peers(226-226)greylist_peers(196-198)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test (esmeralda)
- GitHub Check: ci
- GitHub Check: Cucumber tests / Base Layer
- GitHub Check: cargo check with stable
- GitHub Check: security_audit
🔇 Additional comments (4)
p2pool/src/server/http/server.rs (1)
77-77: Route addition is correct.
Adding/metricsendpoint aligns well with standard convention for Prometheus scraping. Great job.p2pool/src/server/http/stats/handlers.rs (3)
9-9: No issues with the new import path.
Theresponse::Responseimport is needed for returning the new type from the handler.
13-13: Prometheus imports are valid.
These imports appropriately cover encoding, gauge, and registry usage.
26-26: New log target definition is fine.
DefiningMETRICS_LOG_TARGETensures logs for metrics are clearly separated.
Comment on lines
+408
to
+525
| async fn get_metrics_data(state: AppState, registry: &Registry) -> Result<(), StatusCode> { | ||
| // Get full stats which includes chain stats and connection info | ||
| let stats = handle_get_stats(State(state.clone())).await?.0; | ||
|
|
||
| // Get peer info | ||
| let (tx, rx) = oneshot::channel(); | ||
| state | ||
| .p2p_service_client | ||
| .send(P2pServiceQuery::GetPeers(tx)) | ||
| .await | ||
| .map_err(|error| { | ||
| error!(target: METRICS_LOG_TARGET, "Failed to get peers info: {error:?}"); | ||
| StatusCode::INTERNAL_SERVER_ERROR | ||
| })?; | ||
|
|
||
| let peers_info = rx.await.map_err(|error| { | ||
| error!(target: METRICS_LOG_TARGET, "Failed to receive peers info: {error:?}"); | ||
| StatusCode::INTERNAL_SERVER_ERROR | ||
| })?; | ||
|
|
||
| // Register and set metrics | ||
|
|
||
| // We'll use serde_json to convert the stats to a value we can extract fields from | ||
| let rx_stats_json = serde_json::to_value(&stats.randomx_stats).unwrap_or_default(); | ||
| let sha3x_stats_json = serde_json::to_value(&stats.sha3x_stats).unwrap_or_default(); | ||
|
|
||
| // Chain height metrics | ||
| let chain_height = GaugeVec::new(Opts::new("p2pool_chain_height", "Current chain height"), &["algorithm"]).unwrap(); | ||
| registry.register(Box::new(chain_height.clone())).unwrap(); | ||
|
|
||
| if let Some(height) = rx_stats_json.get("height").and_then(|v| v.as_u64()) { | ||
| chain_height.with_label_values(&["randomx"]).set(height as f64); | ||
| } | ||
|
|
||
| if let Some(height) = sha3x_stats_json.get("height").and_then(|v| v.as_u64()) { | ||
| chain_height.with_label_values(&["sha3x"]).set(height as f64); | ||
| } | ||
|
|
||
| // Shares metrics | ||
| let total_shares = GaugeVec::new(Opts::new("p2pool_total_shares", "Total number of shares"), &[ | ||
| "algorithm", | ||
| ]) | ||
| .unwrap(); | ||
| registry.register(Box::new(total_shares.clone())).unwrap(); | ||
|
|
||
| if let Some(shares) = rx_stats_json.get("total_shares").and_then(|v| v.as_u64()) { | ||
| total_shares.with_label_values(&["randomx"]).set(shares as f64); | ||
| } | ||
|
|
||
| if let Some(shares) = sha3x_stats_json.get("total_shares").and_then(|v| v.as_u64()) { | ||
| total_shares.with_label_values(&["sha3x"]).set(shares as f64); | ||
| } | ||
|
|
||
| // My shares metrics | ||
| let my_shares = GaugeVec::new(Opts::new("p2pool_my_shares", "Number of my shares"), &["algorithm"]).unwrap(); | ||
| registry.register(Box::new(my_shares.clone())).unwrap(); | ||
|
|
||
| if let Some(shares) = rx_stats_json.get("num_my_shares").and_then(|v| v.as_u64()) { | ||
| my_shares.with_label_values(&["randomx"]).set(shares as f64); | ||
| } | ||
|
|
||
| if let Some(shares) = sha3x_stats_json.get("num_my_shares").and_then(|v| v.as_u64()) { | ||
| my_shares.with_label_values(&["sha3x"]).set(shares as f64); | ||
| } | ||
|
|
||
| // Connection metrics | ||
| let connected_peers = Gauge::new("p2pool_connected_peers", "Number of connected peers").unwrap(); | ||
| registry.register(Box::new(connected_peers.clone())).unwrap(); | ||
| connected_peers.set(stats.connection_info.connected_peers as f64); | ||
|
|
||
| // Connection counters | ||
| let pending_incoming = Gauge::new("p2pool_pending_incoming", "Number of pending incoming connections").unwrap(); | ||
| registry.register(Box::new(pending_incoming.clone())).unwrap(); | ||
| pending_incoming.set(stats.connection_info.network_info.connection_counters.pending_incoming as f64); | ||
|
|
||
| let pending_outgoing = Gauge::new("p2pool_pending_outgoing", "Number of pending outgoing connections").unwrap(); | ||
| registry.register(Box::new(pending_outgoing.clone())).unwrap(); | ||
| pending_outgoing.set(stats.connection_info.network_info.connection_counters.pending_outgoing as f64); | ||
|
|
||
| let established_incoming = Gauge::new( | ||
| "p2pool_established_incoming", | ||
| "Number of established incoming connections", | ||
| ) | ||
| .unwrap(); | ||
| registry.register(Box::new(established_incoming.clone())).unwrap(); | ||
| established_incoming.set( | ||
| stats | ||
| .connection_info | ||
| .network_info | ||
| .connection_counters | ||
| .established_incoming as f64, | ||
| ); | ||
|
|
||
| let established_outgoing = Gauge::new( | ||
| "p2pool_established_outgoing", | ||
| "Number of established outgoing connections", | ||
| ) | ||
| .unwrap(); | ||
| registry.register(Box::new(established_outgoing.clone())).unwrap(); | ||
| established_outgoing.set( | ||
| stats | ||
| .connection_info | ||
| .network_info | ||
| .connection_counters | ||
| .established_outgoing as f64, | ||
| ); | ||
|
|
||
| // Peer list metrics | ||
| let whitelist_peers = IntGauge::new("p2pool_whitelist_peers", "Number of whitelisted peers").unwrap(); | ||
| registry.register(Box::new(whitelist_peers.clone())).unwrap(); | ||
| whitelist_peers.set(peers_info.0.len() as i64); | ||
|
|
||
| let greylist_peers = IntGauge::new("p2pool_greylist_peers", "Number of greylisted peers").unwrap(); | ||
| registry.register(Box::new(greylist_peers.clone())).unwrap(); | ||
| greylist_peers.set(peers_info.1.len() as i64); | ||
|
|
||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid duplicate metric registrations on every request call.
Each time handle_metrics is invoked, get_metrics_data re-registers multiple metrics. Typically, we'd define and register shared metrics once at startup, then just update them within each request. Repeated registration can lead to duplicates or memory overhead. A recommended approach is to keep the Gauges in a static or higher scope, updating them instead of re-registering.
-async fn get_metrics_data(state: AppState, registry: &Registry) -> Result<(), StatusCode> {
- // Register and set metrics
- let chain_height = GaugeVec::new(Opts::new("p2pool_chain_height", "Current chain height"), &["algorithm"]).unwrap();
- registry.register(Box::new(chain_height.clone())).unwrap();
- // ...
+async fn get_metrics_data(state: AppState) -> Result<(), StatusCode> {
+ // Instead of registering metrics here, update previously registered metrics:
+ // chain_height.with_label_values(&["randomx"]).set(..);
+ // ...
}Committable suggestion skipped: line range outside the PR's diff.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores