Metric tap agent #161
Merged
Metric tap agent #161
Conversation
This file contains 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
Pull Request Test Coverage Report for Build 9067937779Details
💛 - Coveralls |
gusinacio
requested changes
May 9, 2024
gusinacio
requested changes
May 9, 2024
tap-agent/src/metrics.rs
Outdated
| @@ -34,7 +34,8 @@ async fn _run_server(port: u16) { | |||
| .route("/metrics", get(handler_metrics)) | |||
| .fallback(handler_404); | |||
| let addr = SocketAddr::from(([0, 0, 0, 0], port)); | |||
| let server = Server::bind(&addr).serve(app.into_make_service()); | |||
| let listener = tokio::net::TcpListener::bind(addr).await.unwrap(); | |||
There was a problem hiding this comment.
use expect instead of unwrap and add information: "Failed to bind metrics address"
aasseman
requested changes
May 10, 2024
tap-agent/src/metrics.rs
Outdated
|
|
||
| use axum::{http::StatusCode, response::IntoResponse, routing::get, Router}; | ||
| use futures_util::FutureExt; | ||
| use jsonrpsee::tracing::error; |
There was a problem hiding this comment.
Hmm I can't remember why I used jsonrpsee::tracing::error here https://github.com/semiotic-ai/timeline-aggregation-protocol/blob/c179dfedee1ed8078b358de3d35f4d051f74c9c1/tap_aggregator/src/metrics.rs#L8
But in any case, it should look less weird with tracing::error instead.
|
|
||
| lazy_static! { | ||
| static ref RAVS_PER_SENDER_ALLOCATION: CounterVec = register_counter_vec!( | ||
| format!("ravs_created_per_sender_allocation"), |
There was a problem hiding this comment.
Inconsistent with other labels. Sometimes you use sender_x_allocation and sometimes sender_allocation. Just pick either one and use it everywhere
Comment on lines
60
to
66
| lazy_static! { | ||
| static ref CLOSED_ALLOCATIONS: Counter = register_counter!( | ||
| format!("amount_of_closed_allocations"), | ||
| "allocations closed", | ||
| ) | ||
| .unwrap(); | ||
| } |
There was a problem hiding this comment.
I'd be more specific as to what this is counting:
Suggested change
| lazy_static! { | |
| static ref CLOSED_ALLOCATIONS: Counter = register_counter!( | |
| format!("amount_of_closed_allocations"), | |
| "allocations closed", | |
| ) | |
| .unwrap(); | |
| } | |
| lazy_static! { | |
| static ref CLOSED_SENDER_ALLOCATIONS: Counter = register_counter!( | |
| format!("closed_sender_allocation_total"), | |
| "Total count of sender-allocation managers closed.", | |
| ) | |
| .unwrap(); | |
| } |
Comment on lines
55
to
58
| lazy_static! { | ||
| static ref RAV_VALUE: GaugeVec = | ||
| register_gauge_vec!(format!("rav_value"), "RAV Updated value", &["allocation"]).unwrap(); | ||
| } |
There was a problem hiding this comment.
It's missing the sender label.
Suggested change
| lazy_static! { | |
| static ref RAV_VALUE: GaugeVec = | |
| register_gauge_vec!(format!("rav_value"), "RAV Updated value", &["allocation"]).unwrap(); | |
| } | |
| lazy_static! { | |
| static ref RAV_VALUE: GaugeVec = | |
| register_gauge_vec!(format!("rav_value"), "Value of the last RAV per sender-allocation", &["sender", "allocation"]).unwrap(); | |
| } |
aasseman
reviewed
May 13, 2024
aasseman
approved these changes
May 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
fixes #156