-
Notifications
You must be signed in to change notification settings - Fork 77
starknet_transaction_prover: HTTP request count + latency + in-flight metrics #14169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
avi-starkware
wants to merge
2
commits into
avi/prover-v3/job-metrics
Choose a base branch
from
avi/prover-v3/http-metrics
base: avi/prover-v3/job-metrics
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
138 changes: 138 additions & 0 deletions
138
crates/starknet_transaction_prover/src/server/http_metrics.rs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| //! tower middleware that records HTTP-level Prometheus metrics: | ||
| //! request count, latency histogram, and an RAII-guarded in-flight gauge. | ||
|
|
||
| use std::task::{Context, Poll}; | ||
| use std::time::Instant; | ||
|
|
||
| use http::{Method, Request, Response, StatusCode}; | ||
| use jsonrpsee::server::HttpBody; | ||
| use tower::{Layer, Service}; | ||
|
|
||
| use crate::server::metrics::GaugeGuard; | ||
|
|
||
| #[cfg(test)] | ||
| #[path = "http_metrics_test.rs"] | ||
| mod http_metrics_test; | ||
|
|
||
| pub mod names { | ||
| /// Counter of HTTP requests by method + status class. | ||
| pub const REQUESTS_TOTAL: &str = "prover_http_requests_total"; | ||
| /// Histogram of end-to-end HTTP request latency by method + status class. | ||
| pub const REQUEST_DURATION_SECONDS: &str = "prover_http_request_duration_seconds"; | ||
| /// Gauge of in-flight HTTP requests. | ||
| pub const IN_FLIGHT_REQUESTS: &str = "prover_http_inflight_requests"; | ||
| } | ||
|
|
||
| /// Pre-registers the HTTP metrics so the series exist before the first request. | ||
| /// This describes the histogram without recording into it. A phantom 0-second | ||
| /// observation would distort every quantile. | ||
| pub fn preregister_http_metrics() { | ||
| // Pre-register the 2xx/4xx/5xx series so an error-rate alert reads zero instead of an | ||
| // absent series before the first failure. This calls the same label helpers as the live | ||
| // path, so the pre-registered label values match the emitted ones. | ||
| for status in [StatusCode::OK, StatusCode::BAD_REQUEST, StatusCode::INTERNAL_SERVER_ERROR] { | ||
| metrics::counter!( | ||
| names::REQUESTS_TOTAL, | ||
| "method" => method_label(&Method::POST), | ||
| "status" => status_label(status), | ||
| ) | ||
| .increment(0); | ||
| } | ||
| metrics::describe_histogram!( | ||
| names::REQUEST_DURATION_SECONDS, | ||
| "HTTP request latency in seconds, by method and status class", | ||
| ); | ||
| metrics::gauge!(names::IN_FLIGHT_REQUESTS).set(0.0); | ||
| } | ||
|
|
||
| /// tower [`Layer`] that records request count, latency, and the in-flight gauge | ||
| /// for served requests. | ||
| #[derive(Clone, Copy)] | ||
| pub struct HttpMetricsLayer; | ||
|
|
||
| impl<S> Layer<S> for HttpMetricsLayer { | ||
| type Service = HttpMetricsService<S>; | ||
|
|
||
| fn layer(&self, inner: S) -> Self::Service { | ||
| HttpMetricsService { inner } | ||
| } | ||
| } | ||
|
|
||
| /// tower [`Service`] produced by [`HttpMetricsLayer`]. | ||
| #[derive(Clone)] | ||
| pub struct HttpMetricsService<S> { | ||
| inner: S, | ||
| } | ||
|
|
||
| impl<S, ReqB> Service<Request<ReqB>> for HttpMetricsService<S> | ||
| where | ||
| S: Service<Request<ReqB>, Response = Response<HttpBody>>, | ||
| S::Future: Send + 'static, | ||
| S::Error: Send + 'static, | ||
| { | ||
| type Response = Response<HttpBody>; | ||
| type Error = S::Error; | ||
| type Future = std::pin::Pin< | ||
| Box<dyn std::future::Future<Output = Result<Self::Response, Self::Error>> + Send>, | ||
| >; | ||
|
|
||
| fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
| self.inner.poll_ready(cx) | ||
| } | ||
|
|
||
| fn call(&mut self, request: Request<ReqB>) -> Self::Future { | ||
| let method = method_label(request.method()); | ||
| let start = Instant::now(); | ||
| let future = self.inner.call(request); | ||
|
|
||
| Box::pin(async move { | ||
| let _in_flight_guard = GaugeGuard::acquire(names::IN_FLIGHT_REQUESTS); | ||
| let result = future.await; | ||
| let duration_seconds = start.elapsed().as_secs_f64(); | ||
| let status = match &result { | ||
| Ok(response) => status_label(response.status()), | ||
| // The tower stack failed without producing an HTTP response. | ||
| Err(_) => "error", | ||
| }; | ||
| metrics::histogram!( | ||
| names::REQUEST_DURATION_SECONDS, | ||
| "method" => method, | ||
| "status" => status, | ||
| ) | ||
| .record(duration_seconds); | ||
| metrics::counter!( | ||
| names::REQUESTS_TOTAL, | ||
| "method" => method, | ||
| "status" => status, | ||
| ) | ||
| .increment(1); | ||
| result | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| /// Collapses HTTP statuses into a bounded set of label values to cap Prometheus series cardinality. | ||
| fn status_label(status: StatusCode) -> &'static str { | ||
| match status.as_u16() { | ||
| 100..=199 => "1xx", | ||
| 200..=299 => "2xx", | ||
| 300..=399 => "3xx", | ||
| 400..=499 => "4xx", | ||
| 500..=599 => "5xx", | ||
| _ => "other", | ||
| } | ||
| } | ||
|
|
||
| /// Collapses HTTP methods into a bounded set of label values to cap Prometheus series cardinality. | ||
| fn method_label(method: &Method) -> &'static str { | ||
| match *method { | ||
| Method::GET => "GET", | ||
| Method::POST => "POST", | ||
| Method::PUT => "PUT", | ||
| Method::DELETE => "DELETE", | ||
| Method::HEAD => "HEAD", | ||
| Method::OPTIONS => "OPTIONS", | ||
| Method::PATCH => "PATCH", | ||
| _ => "other", | ||
| } | ||
| } |
137 changes: 137 additions & 0 deletions
137
crates/starknet_transaction_prover/src/server/http_metrics_test.rs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| //! Unit tests for [`HttpMetricsLayer`]. | ||
| //! | ||
| //! All tests share one process-global Prometheus recorder ([`shared_handle`]), | ||
| //! so absolute sample values also reflect requests driven by other tests in this | ||
| //! binary. Each assertion reads a baseline before the action and compares the | ||
| //! delta. | ||
|
|
||
| use bytes::Bytes; | ||
| use http::{Method, Request, Response, StatusCode}; | ||
| use http_body_util::Full; | ||
| use jsonrpsee::server::HttpBody; | ||
| use tower::{Layer, ServiceBuilder, ServiceExt}; | ||
| use tower_http::compression::CompressionLayer; | ||
| use tower_http::cors::CorsLayer; | ||
| use tower_http::map_request_body::MapRequestBodyLayer; | ||
| use tower_http::map_response_body::MapResponseBodyLayer; | ||
|
|
||
| use crate::server::health::{HealthLayer, HEALTH_PATH}; | ||
| use crate::server::http_metrics::{names, HttpMetricsLayer}; | ||
| use crate::server::metrics::{MetricsLayer, METRICS_PATH}; | ||
| use crate::server::request_log::RequestLogLayer; | ||
| use crate::server::request_span::RequestSpanLayer; | ||
| use crate::server::test_recorder::{metric_value, shared_handle}; | ||
| use crate::server::OhttpJsonrpseeLayer; | ||
|
|
||
| fn ok_service() -> impl tower::Service< | ||
| Request<HttpBody>, | ||
| Response = Response<HttpBody>, | ||
| Error = std::convert::Infallible, | ||
| Future = futures::future::Ready<Result<Response<HttpBody>, std::convert::Infallible>>, | ||
| > + Clone { | ||
| tower::service_fn(|_req: Request<HttpBody>| { | ||
| let response = Response::builder() | ||
| .status(StatusCode::OK) | ||
| .body(HttpBody::new(Full::new(Bytes::new()))) | ||
| .expect("static body is infallible"); | ||
| futures::future::ready(Ok::<_, std::convert::Infallible>(response)) | ||
| }) | ||
| } | ||
|
|
||
| fn build_request(method: Method) -> Request<HttpBody> { | ||
| Request::builder() | ||
| .method(method) | ||
| .uri("/") | ||
| .body(HttpBody::new(Full::new(Bytes::new()))) | ||
| .expect("static body is infallible") | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn records_counter_histogram_and_returns_inflight_to_zero() { | ||
| let handle = shared_handle(); | ||
| let svc = HttpMetricsLayer.layer(ok_service()); | ||
|
|
||
| let scrape = handle.render(); | ||
| let before_counter = metric_value(&scrape, &post_2xx_counter_line()); | ||
| let before_histogram = metric_value(&scrape, &post_duration_count_line()); | ||
|
|
||
| for _ in 0..3 { | ||
| let response = svc.clone().oneshot(build_request(Method::POST)).await.unwrap(); | ||
| assert_eq!(response.status(), StatusCode::OK); | ||
| } | ||
|
|
||
| let scrape = handle.render(); | ||
| assert_eq!( | ||
| metric_value(&scrape, &post_2xx_counter_line()) - before_counter, | ||
| 3.0, | ||
| "counter delta" | ||
| ); | ||
| assert!( | ||
| scrape.lines().any(|line| line.starts_with(&post_duration_count_line())), | ||
| "latency histogram must carry both method and status labels; scrape:\n{scrape}" | ||
| ); | ||
| assert_eq!( | ||
| metric_value(&scrape, &post_duration_count_line()) - before_histogram, | ||
| 3.0, | ||
| "histogram delta" | ||
| ); | ||
|
|
||
| // The gauge is back at zero, so the guard ran for every request. | ||
| assert_eq!(metric_value(&scrape, names::IN_FLIGHT_REQUESTS), 0.0); | ||
| } | ||
|
|
||
| /// Sums the `prover_http_requests_total` series carrying `method="GET"`, across | ||
| /// statuses. Scoped to GET because the recorder is process-global and other | ||
| /// tests in this binary drive POSTs through the same layer in parallel. Only | ||
| /// this test sends a GET through it. | ||
| fn get_http_requests(scrape: &str) -> f64 { | ||
| scrape | ||
| .lines() | ||
| .filter(|line| !line.starts_with('#') && line.starts_with(names::REQUESTS_TOTAL)) | ||
| .filter(|line| line.contains("method=\"GET\"")) | ||
| .filter_map(|line| line.rsplit_once(' ').and_then(|(_, value)| value.parse::<f64>().ok())) | ||
| .sum() | ||
| } | ||
|
|
||
| fn post_2xx_counter_line() -> String { | ||
| format!("{}{{method=\"POST\",status=\"2xx\"}}", names::REQUESTS_TOTAL) | ||
| } | ||
|
|
||
| fn post_duration_count_line() -> String { | ||
| format!("{}_count{{method=\"POST\",status=\"2xx\"}}", names::REQUEST_DURATION_SECONDS) | ||
| } | ||
|
|
||
| /// `HttpMetricsLayer` sits below `HealthLayer` and `MetricsLayer` in the | ||
| /// production chain so probe and scrape traffic stays out of the request | ||
| /// distribution. Layer order is the only thing enforcing that, so this test | ||
| /// fails if the order changes. | ||
| #[tokio::test] | ||
| async fn probe_and_scrape_traffic_is_excluded_from_http_metrics() { | ||
| let handle = shared_handle(); | ||
| let svc = prover_http_middleware!( | ||
| MetricsLayer::new(handle.clone()), | ||
| None::<CorsLayer>, | ||
| None::<OhttpJsonrpseeLayer>, | ||
| ) | ||
| .service(ok_service()); | ||
|
|
||
| let before_get = get_http_requests(&handle.render()); | ||
|
|
||
| for path in [HEALTH_PATH, METRICS_PATH] { | ||
| let request = Request::builder() | ||
| .method(Method::GET) | ||
| .uri(path) | ||
| .body(HttpBody::new(Full::new(Bytes::new()))) | ||
| .expect("static body is infallible"); | ||
| let response = svc.clone().oneshot(request).await.unwrap(); | ||
| assert_eq!(response.status(), StatusCode::OK, "{path} should be served by its layer"); | ||
| } | ||
|
|
||
| let after_get = get_http_requests(&handle.render()); | ||
| assert_eq!( | ||
| after_get - before_get, | ||
| 0.0, | ||
| "short-circuited {HEALTH_PATH}/{METRICS_PATH} traffic must not reach the HTTP metrics \ | ||
| layer" | ||
| ); | ||
| } | ||
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flaky absolute in-flight gauge assert
Low Severity
The in-flight check uses an absolute scrape value of
0.0, while this file’s own header notes that the Prometheus recorder is process-global and other tests (including ones that driveHttpMetricsLayer) run in parallel. A concurrent in-flight request can make this assertion fail intermittently, and a steady0also does not proveGaugeGuardran because preregistration already sets the gauge to zero.Reviewed by Cursor Bugbot for commit ce52272. Configure here.