From 02980a81d180d84fa14f8fa386d871a396f3cd18 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Fri, 28 Apr 2023 17:22:21 +0100 Subject: [PATCH 01/14] Add basic prometheus metrics --- .gitignore | 5 +++- Cargo.toml | 2 ++ src/app.rs | 8 +++++- src/main.rs | 4 +++ src/metrics.rs | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 src/metrics.rs diff --git a/.gitignore b/.gitignore index d89f83f..2bb2839 100644 --- a/.gitignore +++ b/.gitignore @@ -19,4 +19,7 @@ Cargo.lock /target # Dev TLS certs -.certs/ \ No newline at end of file +.certs/ + +# Python venv for running helper scripts +/scripts/venv/ diff --git a/Cargo.toml b/Cargo.toml index 683d78f..ff4dd06 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,11 +20,13 @@ clap = { version = "4.2.1", features = ["derive", "env"] } expanduser = "1.2.2" http = "*" hyper = { version = "0.14", features = ["full"] } +lazy_static = "1.4" maligned = "0.2.1" mime = "0.3" ndarray = "0.15" ndarray-stats = "0.5" num-traits = "0.2.15" +prometheus = { version = "0.13", features = ["process"] } serde = { version = "1.0", features = ["derive"] } serde_json = "*" strum_macros = "0.24" diff --git a/src/app.rs b/src/app.rs index d88c147..088569e 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,6 +1,7 @@ //! Active Storage server API use crate::error::ActiveStorageError; +use crate::metrics::{metrics_handler, record_response_metrics, request_counter}; use crate::models; use crate::operation; use crate::operations; @@ -68,7 +69,11 @@ pub fn router() -> Router { .route("/:operation", post(unknown_operation_handler)) .layer( ServiceBuilder::new() - .layer(TraceLayer::new_for_http()) + .layer( + TraceLayer::new_for_http() + .on_request(request_counter) + .on_response(record_response_metrics), + ) .layer(ValidateRequestHeaderLayer::custom( // Validate that an authorization header has been provided. |request: &mut Request| { @@ -84,6 +89,7 @@ pub fn router() -> Router { Router::new() .route("/.well-known/s3-active-storage-schema", get(schema)) + .route("/metrics", get(metrics_handler)) .nest("/v1", v1()) .layer(NormalizePathLayer::trim_trailing_slash()) } diff --git a/src/main.rs b/src/main.rs index 564dfa2..bd65825 100644 --- a/src/main.rs +++ b/src/main.rs @@ -39,6 +39,9 @@ mod operations; mod s3_client; mod validated_json; +// TODO: Gate metrics module behind a cargo feature? +mod metrics; + /// S3 Active Storage Proxy command line interface #[derive(Debug, Parser)] struct CommandLineArgs { @@ -76,6 +79,7 @@ async fn main() { let args = CommandLineArgs::parse(); init_tracing(); + metrics::register_metrics(); let router = app::router(); let addr = SocketAddr::from_str(&format!("{}:{}", args.host, args.port)) diff --git a/src/metrics.rs b/src/metrics.rs new file mode 100644 index 0000000..6c63f16 --- /dev/null +++ b/src/metrics.rs @@ -0,0 +1,73 @@ +use axum::{body::Body, http::Request, response::Response}; +use lazy_static::lazy_static; +use prometheus::{self, Encoder, HistogramOpts, HistogramVec, IntCounterVec, Opts, Registry}; +use tracing::Span; + +lazy_static! { + // Registry for holding metric state + pub static ref REGISTRY: Registry = Registry::new(); + // Simple request counter + pub static ref INCOMING_REQUESTS: IntCounterVec = IntCounterVec::new( + Opts::new("incoming_requests", "The number of HTTP requests received"), + &["http_method"] + ).unwrap(); + // Request counter by status code + pub static ref RESPONSE_CODE_COLLECTOR: IntCounterVec = IntCounterVec::new( + Opts::new("outgoing_response", "The number of responses sent."), + &["status_code"] + ).unwrap(); + // Request histogram by response time + pub static ref RESPONSE_TIME_COLLECTOR: HistogramVec = HistogramVec::new( + HistogramOpts{ + common_opts: Opts::new("response_time", "The time taken to respond to each request"), + buckets: prometheus::DEFAULT_BUCKETS.to_vec(), // Change buckets here if desired + }, + &[], + ).unwrap(); +} + +pub fn register_metrics() { + REGISTRY + .register(Box::new(INCOMING_REQUESTS.clone())) + .unwrap(); + REGISTRY + .register(Box::new(RESPONSE_CODE_COLLECTOR.clone())) + .unwrap(); + REGISTRY + .register(Box::new(RESPONSE_TIME_COLLECTOR.clone())) + .unwrap(); +} + +pub async fn metrics_handler() -> String { + let encoder = prometheus::TextEncoder::new(); + let mut buffer = Vec::new(); + + encoder.encode(®ISTRY.gather(), &mut buffer).unwrap(); + + let output = String::from_utf8(buffer.clone()).unwrap(); + buffer.clear(); + + output +} + +/// Increments the prometheus counter on all incoming requests, labelled by http method +pub fn request_counter(request: &Request, _span: &Span) { + INCOMING_REQUESTS + .with_label_values(&[&request.method().to_string().to_ascii_uppercase()]) + .inc(); +} + +/// Increment the prometheus counter on all outgoing responses, labelled by status code +pub fn record_response_metrics( + response: &Response, + latency: std::time::Duration, + _span: &Span, +) { + RESPONSE_CODE_COLLECTOR + .with_label_values(&[response.status().as_str()]) + .inc(); + + RESPONSE_TIME_COLLECTOR + .with_label_values(&[]) + .observe(latency.as_secs_f64()); +} From b130fa30cc0b435d898be0422662765742effee7 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Tue, 2 May 2023 10:34:55 +0100 Subject: [PATCH 02/14] Error handling + comments and formatting --- src/app.rs | 4 ++-- src/metrics.rs | 28 ++++++++++++++++------------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/app.rs b/src/app.rs index 088569e..d9f4dd8 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,7 +1,7 @@ //! Active Storage server API use crate::error::ActiveStorageError; -use crate::metrics::{metrics_handler, record_response_metrics, request_counter}; +use crate::metrics::{metrics_handler, record_request_metrics, record_response_metrics}; use crate::models; use crate::operation; use crate::operations; @@ -71,7 +71,7 @@ pub fn router() -> Router { ServiceBuilder::new() .layer( TraceLayer::new_for_http() - .on_request(request_counter) + .on_request(record_request_metrics) .on_response(record_response_metrics), ) .layer(ValidateRequestHeaderLayer::custom( diff --git a/src/metrics.rs b/src/metrics.rs index 6c63f16..801a13c 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -10,34 +10,36 @@ lazy_static! { pub static ref INCOMING_REQUESTS: IntCounterVec = IntCounterVec::new( Opts::new("incoming_requests", "The number of HTTP requests received"), &["http_method"] - ).unwrap(); + ).expect("Prometheus metric initialization failed"); // Request counter by status code pub static ref RESPONSE_CODE_COLLECTOR: IntCounterVec = IntCounterVec::new( - Opts::new("outgoing_response", "The number of responses sent."), + Opts::new("outgoing_response", "The number of responses sent"), &["status_code"] - ).unwrap(); - // Request histogram by response time + ).expect("Prometheus metric initialization failed"); + // Response histogram by response time pub static ref RESPONSE_TIME_COLLECTOR: HistogramVec = HistogramVec::new( HistogramOpts{ common_opts: Opts::new("response_time", "The time taken to respond to each request"), buckets: prometheus::DEFAULT_BUCKETS.to_vec(), // Change buckets here if desired }, &[], - ).unwrap(); + ).expect("Prometheus metric initialization failed"); } +/// Registers various prometheus metrics with the global registry pub fn register_metrics() { REGISTRY .register(Box::new(INCOMING_REQUESTS.clone())) - .unwrap(); + .expect("registering prometheus metrics during initialization failed"); REGISTRY .register(Box::new(RESPONSE_CODE_COLLECTOR.clone())) - .unwrap(); + .expect("registering prometheus metrics during initialization failed"); REGISTRY .register(Box::new(RESPONSE_TIME_COLLECTOR.clone())) - .unwrap(); + .expect("registering prometheus metrics during initialization failed"); } +/// Returns currently gathered prometheus metrics pub async fn metrics_handler() -> String { let encoder = prometheus::TextEncoder::new(); let mut buffer = Vec::new(); @@ -50,23 +52,25 @@ pub async fn metrics_handler() -> String { output } -/// Increments the prometheus counter on all incoming requests, labelled by http method -pub fn request_counter(request: &Request, _span: &Span) { +/// Gather relevant prometheus metrics on all incoming requests +pub fn record_request_metrics(request: &Request, _span: &Span) { + // Increment request counter INCOMING_REQUESTS .with_label_values(&[&request.method().to_string().to_ascii_uppercase()]) .inc(); } -/// Increment the prometheus counter on all outgoing responses, labelled by status code +/// Gather relevant prometheus metrics on all outgoing responses pub fn record_response_metrics( response: &Response, latency: std::time::Duration, _span: &Span, ) { + // Record http status code RESPONSE_CODE_COLLECTOR .with_label_values(&[response.status().as_str()]) .inc(); - + // Record response time RESPONSE_TIME_COLLECTOR .with_label_values(&[]) .observe(latency.as_secs_f64()); From 091866ea3101094347f319064fcb2f3a15573064 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Thu, 4 May 2023 11:59:59 +0100 Subject: [PATCH 03/14] Bump release version --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 0347172..0f2105d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1578,7 +1578,7 @@ checksum = "f91339c0467de62360649f8d3e185ca8de4224ff281f66000de5eb2a77a79041" [[package]] name = "s3-active-storage" -version = "0.1.0" +version = "0.1.1" dependencies = [ "async-trait", "aws-credential-types", From 14419ad72eca63aa41fdac6b70a52282912ca565 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Thu, 4 May 2023 12:05:43 +0100 Subject: [PATCH 04/14] Re-add register_metrics call --- src/main.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 504cb99..0aeebe7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -26,8 +26,8 @@ mod app; mod array; mod cli; mod error; -mod models; mod metrics; +mod models; mod operation; mod operations; mod s3_client; @@ -40,6 +40,7 @@ mod validated_json; async fn main() { let args = cli::parse(); tracing::init_tracing(); + metrics::register_metrics(); let service = app::service(); server::serve(&args, service).await; } From 8750e0a59eef5e718bb92f8e0d99d0763b34936b Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Thu, 4 May 2023 12:08:21 +0100 Subject: [PATCH 05/14] Add prometheus deps --- Cargo.lock | 82 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0f2105d..232a975 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -525,9 +525,9 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "clap" -version = "4.2.5" +version = "4.2.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a1f23fa97e1d1641371b51f35535cb26959b8e27ab50d167a8b996b5bada819" +checksum = "34d21f9bf1b425d2968943631ec91202fe5e837264063503708b83013f8fc938" dependencies = [ "clap_builder", "clap_derive", @@ -536,9 +536,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.2.5" +version = "4.2.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fdc5d93c358224b4d6867ef1356d740de2303e9892edc06c5340daeccd96bab" +checksum = "914c8c79fb560f238ef6429439a30023c862f7a28e688c58f7203f12b29970bd" dependencies = [ "anstream", "anstyle", @@ -1024,7 +1024,7 @@ checksum = "adcf93614601c8129ddf72e2d5633df827ba6551541c6d8c59520a371475be1f" dependencies = [ "hermit-abi 0.3.1", "io-lifetimes", - "rustix", + "rustix 0.37.19", "windows-sys 0.48.0", ] @@ -1066,9 +1066,15 @@ checksum = "6a987beff54b60ffa6d51982e1aa1146bc42f19bd26be28b0586f252fccf5317" [[package]] name = "linux-raw-sys" -version = "0.3.6" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f051f77a7c8e6957c0696eac88f26b0117e54f52d3fc682ab19397a8812846a4" + +[[package]] +name = "linux-raw-sys" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b64f40e5e03e0d54f03845c8197d0291253cdbedfb1cb46b13c2c117554a9f4c" +checksum = "ece97ea872ece730aed82664c424eb4c8291e1ff2480247ccf7409044bc6479f" [[package]] name = "lock_api" @@ -1118,9 +1124,9 @@ checksum = "b87248edafb776e59e6ee64a79086f65890d3510f2c656c000bf2a7e8a0aea40" [[package]] name = "matrixmultiply" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fcce854c9e76cfd191182c280eb5460cb8efd2cb4e58a1fd56bc41102d7e5802" +checksum = "090126dc04f95dc0d1c1c91f61bdd474b3930ca064c1edc8a849da2c6cbe1e77" dependencies = [ "autocfg", "rawpointer", @@ -1368,6 +1374,42 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "procfs" +version = "0.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1de8dacb0873f77e6aefc6d71e044761fcc68060290f5b1089fcdf84626bb69" +dependencies = [ + "bitflags", + "byteorder", + "hex", + "lazy_static", + "rustix 0.36.13", +] + +[[package]] +name = "prometheus" +version = "0.13.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "449811d15fbdf5ceb5c1144416066429cf82316e2ec8ce0c1f6f8a02e7bbcf8c" +dependencies = [ + "cfg-if", + "fnv", + "lazy_static", + "libc", + "memchr", + "parking_lot", + "procfs", + "protobuf", + "thiserror", +] + +[[package]] +name = "protobuf" +version = "2.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "106dd99e98437432fed6519dedecfade6a06a73bb7b2a1e019fdd2bee5778d94" + [[package]] name = "pwd" version = "1.4.0" @@ -1519,15 +1561,29 @@ dependencies = [ [[package]] name = "rustix" -version = "0.37.18" +version = "0.36.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8bbfc1d1c7c40c01715f47d71444744a81669ca84e8b63e25a55e169b1f86433" +checksum = "3a38f9520be93aba504e8ca974197f46158de5dcaa9fa04b57c57cd6a679d658" dependencies = [ "bitflags", "errno", "io-lifetimes", "libc", - "linux-raw-sys", + "linux-raw-sys 0.1.4", + "windows-sys 0.45.0", +] + +[[package]] +name = "rustix" +version = "0.37.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acf8729d8542766f1b2cf77eb034d52f40d375bb8b615d0b147089946e16613d" +dependencies = [ + "bitflags", + "errno", + "io-lifetimes", + "libc", + "linux-raw-sys 0.3.7", "windows-sys 0.48.0", ] @@ -1592,11 +1648,13 @@ dependencies = [ "expanduser", "http", "hyper", + "lazy_static", "maligned", "mime", "ndarray", "ndarray-stats", "num-traits", + "prometheus", "regex", "serde", "serde_json", From 4f97972b1d675d8125411537d45247fd026a8db1 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Thu, 4 May 2023 12:40:31 +0100 Subject: [PATCH 06/14] Add more metric labels --- src/metrics.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index 801a13c..f0dd71e 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -9,7 +9,7 @@ lazy_static! { // Simple request counter pub static ref INCOMING_REQUESTS: IntCounterVec = IntCounterVec::new( Opts::new("incoming_requests", "The number of HTTP requests received"), - &["http_method"] + &["http_method", "path"] ).expect("Prometheus metric initialization failed"); // Request counter by status code pub static ref RESPONSE_CODE_COLLECTOR: IntCounterVec = IntCounterVec::new( @@ -22,7 +22,7 @@ lazy_static! { common_opts: Opts::new("response_time", "The time taken to respond to each request"), buckets: prometheus::DEFAULT_BUCKETS.to_vec(), // Change buckets here if desired }, - &[], + &["status_code"], ).expect("Prometheus metric initialization failed"); } @@ -44,9 +44,12 @@ pub async fn metrics_handler() -> String { let encoder = prometheus::TextEncoder::new(); let mut buffer = Vec::new(); - encoder.encode(®ISTRY.gather(), &mut buffer).unwrap(); + encoder + .encode(®ISTRY.gather(), &mut buffer) + .expect("could not encode gathered metrics into temporary buffer"); - let output = String::from_utf8(buffer.clone()).unwrap(); + let output = + String::from_utf8(buffer.clone()).expect("could not convert metrics buffer into string"); buffer.clear(); output @@ -55,8 +58,10 @@ pub async fn metrics_handler() -> String { /// Gather relevant prometheus metrics on all incoming requests pub fn record_request_metrics(request: &Request, _span: &Span) { // Increment request counter + let http_method = &request.method().to_string().to_ascii_uppercase(); + let request_path = &request.uri().to_string(); INCOMING_REQUESTS - .with_label_values(&[&request.method().to_string().to_ascii_uppercase()]) + .with_label_values(&[http_method, request_path]) .inc(); } @@ -66,12 +71,14 @@ pub fn record_response_metrics( latency: std::time::Duration, _span: &Span, ) { + let status_code = response.status(); + // let http_method // Record http status code RESPONSE_CODE_COLLECTOR - .with_label_values(&[response.status().as_str()]) + .with_label_values(&[status_code.as_str()]) .inc(); // Record response time RESPONSE_TIME_COLLECTOR - .with_label_values(&[]) + .with_label_values(&[status_code.as_str()]) .observe(latency.as_secs_f64()); } From 7b93cc5e97fb0e2b11ce5d2ed80663f434caaf01 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Thu, 4 May 2023 14:13:11 +0000 Subject: [PATCH 07/14] Gather system metrics too Switch to using default_registry so that system metrics (cpu time, open file descriptors etc.) are collected too. (Prometheus crate's system metrics feature only works on linux.) --- src/metrics.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index f0dd71e..11d680c 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -4,8 +4,6 @@ use prometheus::{self, Encoder, HistogramOpts, HistogramVec, IntCounterVec, Opts use tracing::Span; lazy_static! { - // Registry for holding metric state - pub static ref REGISTRY: Registry = Registry::new(); // Simple request counter pub static ref INCOMING_REQUESTS: IntCounterVec = IntCounterVec::new( Opts::new("incoming_requests", "The number of HTTP requests received"), @@ -28,13 +26,14 @@ lazy_static! { /// Registers various prometheus metrics with the global registry pub fn register_metrics() { - REGISTRY + let registry = prometheus::default_registry(); + registry .register(Box::new(INCOMING_REQUESTS.clone())) .expect("registering prometheus metrics during initialization failed"); - REGISTRY + registry .register(Box::new(RESPONSE_CODE_COLLECTOR.clone())) .expect("registering prometheus metrics during initialization failed"); - REGISTRY + registry .register(Box::new(RESPONSE_TIME_COLLECTOR.clone())) .expect("registering prometheus metrics during initialization failed"); } @@ -45,7 +44,7 @@ pub async fn metrics_handler() -> String { let mut buffer = Vec::new(); encoder - .encode(®ISTRY.gather(), &mut buffer) + .encode(&prometheus::gather(), &mut buffer) .expect("could not encode gathered metrics into temporary buffer"); let output = From 65c603082c5e90b99927e593cce588ec8410b5ad Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Thu, 4 May 2023 14:16:06 +0000 Subject: [PATCH 08/14] Remove unused import --- src/metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics.rs b/src/metrics.rs index 11d680c..bf4211f 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -1,6 +1,6 @@ use axum::{body::Body, http::Request, response::Response}; use lazy_static::lazy_static; -use prometheus::{self, Encoder, HistogramOpts, HistogramVec, IntCounterVec, Opts, Registry}; +use prometheus::{self, Encoder, HistogramOpts, HistogramVec, IntCounterVec, Opts}; use tracing::Span; lazy_static! { From b773d7a18e7316c64678b1df05156fd8fdbefcb3 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Thu, 4 May 2023 15:32:12 +0100 Subject: [PATCH 09/14] Move TraceLayer to capture all requests --- src/app.rs | 31 +++++++++++++++---------------- src/metrics.rs | 2 +- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/app.rs b/src/app.rs index 4b756ad..bf929ac 100644 --- a/src/app.rs +++ b/src/app.rs @@ -67,22 +67,16 @@ fn router() -> Router { .route("/sum", post(operation_handler::)) .route("/:operation", post(unknown_operation_handler)) .layer( - ServiceBuilder::new() - .layer( - TraceLayer::new_for_http() - .on_request(record_request_metrics) - .on_response(record_response_metrics), - ) - .layer(ValidateRequestHeaderLayer::custom( - // Validate that an authorization header has been provided. - |request: &mut Request| { - if request.headers().contains_key(header::AUTHORIZATION) { - Ok(()) - } else { - Err(StatusCode::UNAUTHORIZED.into_response()) - } - }, - )), + ServiceBuilder::new().layer(ValidateRequestHeaderLayer::custom( + // Validate that an authorization header has been provided. + |request: &mut Request| { + if request.headers().contains_key(header::AUTHORIZATION) { + Ok(()) + } else { + Err(StatusCode::UNAUTHORIZED.into_response()) + } + }, + )), ) } @@ -90,6 +84,11 @@ fn router() -> Router { .route("/.well-known/s3-active-storage-schema", get(schema)) .route("/metrics", get(metrics_handler)) .nest("/v1", v1()) + .layer( + TraceLayer::new_for_http() + .on_request(record_request_metrics) + .on_response(record_response_metrics), + ) } /// S3 Active Storage Server Service type alias diff --git a/src/metrics.rs b/src/metrics.rs index bf4211f..9604c0b 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -58,7 +58,7 @@ pub async fn metrics_handler() -> String { pub fn record_request_metrics(request: &Request, _span: &Span) { // Increment request counter let http_method = &request.method().to_string().to_ascii_uppercase(); - let request_path = &request.uri().to_string(); + let request_path = &request.uri().path(); INCOMING_REQUESTS .with_label_values(&[http_method, request_path]) .inc(); From 25bbac857d9df4c3fc801a4de7fe356f964100af Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Thu, 4 May 2023 15:33:20 +0100 Subject: [PATCH 10/14] Remove commented out code --- src/metrics.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/metrics.rs b/src/metrics.rs index 9604c0b..06bfa2a 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -71,7 +71,6 @@ pub fn record_response_metrics( _span: &Span, ) { let status_code = response.status(); - // let http_method // Record http status code RESPONSE_CODE_COLLECTOR .with_label_values(&[status_code.as_str()]) From 2dd0ac7d120c297e3217070b450e477f41474b58 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Thu, 4 May 2023 16:21:06 +0100 Subject: [PATCH 11/14] Switch to axum::middleware::from_fn --- src/app.rs | 31 +++++++++++++++---------------- src/metrics.rs | 47 ++++++++++++++++++++++++++--------------------- 2 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/app.rs b/src/app.rs index bf929ac..f3380fb 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,13 +1,14 @@ //! Active Storage server API use crate::error::ActiveStorageError; -use crate::metrics::{metrics_handler, record_request_metrics, record_response_metrics}; +use crate::metrics::{metrics_handler, track_metrics}; use crate::models; use crate::operation; use crate::operations; use crate::s3_client; use crate::validated_json::ValidatedJson; +use axum::middleware; use axum::{ body::{Body, Bytes}, extract::Path, @@ -67,16 +68,18 @@ fn router() -> Router { .route("/sum", post(operation_handler::)) .route("/:operation", post(unknown_operation_handler)) .layer( - ServiceBuilder::new().layer(ValidateRequestHeaderLayer::custom( - // Validate that an authorization header has been provided. - |request: &mut Request| { - if request.headers().contains_key(header::AUTHORIZATION) { - Ok(()) - } else { - Err(StatusCode::UNAUTHORIZED.into_response()) - } - }, - )), + ServiceBuilder::new() + .layer(TraceLayer::new_for_http()) + .layer(ValidateRequestHeaderLayer::custom( + // Validate that an authorization header has been provided. + |request: &mut Request| { + if request.headers().contains_key(header::AUTHORIZATION) { + Ok(()) + } else { + Err(StatusCode::UNAUTHORIZED.into_response()) + } + }, + )), ) } @@ -84,11 +87,7 @@ fn router() -> Router { .route("/.well-known/s3-active-storage-schema", get(schema)) .route("/metrics", get(metrics_handler)) .nest("/v1", v1()) - .layer( - TraceLayer::new_for_http() - .on_request(record_request_metrics) - .on_response(record_response_metrics), - ) + .route_layer(middleware::from_fn(track_metrics)) } /// S3 Active Storage Server Service type alias diff --git a/src/metrics.rs b/src/metrics.rs index 06bfa2a..831869c 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -1,7 +1,8 @@ -use axum::{body::Body, http::Request, response::Response}; +use std::time::Instant; + +use axum::{http::Request, middleware::Next, response::IntoResponse}; use lazy_static::lazy_static; use prometheus::{self, Encoder, HistogramOpts, HistogramVec, IntCounterVec, Opts}; -use tracing::Span; lazy_static! { // Simple request counter @@ -12,7 +13,7 @@ lazy_static! { // Request counter by status code pub static ref RESPONSE_CODE_COLLECTOR: IntCounterVec = IntCounterVec::new( Opts::new("outgoing_response", "The number of responses sent"), - &["status_code"] + &["status_code", "http_method", "path"] ).expect("Prometheus metric initialization failed"); // Response histogram by response time pub static ref RESPONSE_TIME_COLLECTOR: HistogramVec = HistogramVec::new( @@ -20,7 +21,7 @@ lazy_static! { common_opts: Opts::new("response_time", "The time taken to respond to each request"), buckets: prometheus::DEFAULT_BUCKETS.to_vec(), // Change buckets here if desired }, - &["status_code"], + &["status_code", "http_method", "path"], ).expect("Prometheus metric initialization failed"); } @@ -54,29 +55,33 @@ pub async fn metrics_handler() -> String { output } -/// Gather relevant prometheus metrics on all incoming requests -pub fn record_request_metrics(request: &Request, _span: &Span) { - // Increment request counter +pub async fn track_metrics(request: Request, next: Next) -> impl IntoResponse { + // Extract some useful quantities + let timer = Instant::now(); let http_method = &request.method().to_string().to_ascii_uppercase(); - let request_path = &request.uri().path(); + let request_path = request.uri().path().to_string(); + + // Increment request counter INCOMING_REQUESTS - .with_label_values(&[http_method, request_path]) + .with_label_values(&[http_method, &request_path]) .inc(); -} -/// Gather relevant prometheus metrics on all outgoing responses -pub fn record_response_metrics( - response: &Response, - latency: std::time::Duration, - _span: &Span, -) { + // Pass request onto next layer + let response = next.run(request).await; let status_code = response.status(); - // Record http status code + // Due to 'concentric shell model' for axum layers, + // latency is time taken to traverse all inner + // layers (including primary reduction operation) + // and then back up the layer stack. + let latency = timer.elapsed().as_secs_f64(); + + // Record response metrics RESPONSE_CODE_COLLECTOR - .with_label_values(&[status_code.as_str()]) + .with_label_values(&[status_code.as_str(), http_method, &request_path]) .inc(); - // Record response time RESPONSE_TIME_COLLECTOR - .with_label_values(&[status_code.as_str()]) - .observe(latency.as_secs_f64()); + .with_label_values(&[status_code.as_str(), http_method, &request_path]) + .observe(latency); + + response } From 964ed6f1ba754e89792f235642cefb9a83d9fa9e Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Fri, 7 Jul 2023 15:24:04 +0100 Subject: [PATCH 12/14] Ignore .vscode --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 43cb190..91aed39 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,5 @@ # Python venv for running helper scripts /scripts/venv/ + +.vscode \ No newline at end of file From 3233c69918b13e82cf26c253ffa561bc66cb1224 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Fri, 7 Jul 2023 15:26:10 +0100 Subject: [PATCH 13/14] Add prometheus helpers --- scripts/docker-compose.yml | 22 ++++++++++++++++++++++ scripts/prometheus.yml | 7 +++++++ 2 files changed, 29 insertions(+) create mode 100644 scripts/docker-compose.yml create mode 100644 scripts/prometheus.yml diff --git a/scripts/docker-compose.yml b/scripts/docker-compose.yml new file mode 100644 index 0000000..5413e9c --- /dev/null +++ b/scripts/docker-compose.yml @@ -0,0 +1,22 @@ +# NOTE: This should only be used for explicitly testing the prometheus metrics +# since it builds the Rust project from scratch within the container each time +# so is very slow and inefficient for regular development work. +services: + active-storage-proxy: + build: ../. + ports: + - "8080:8080" + minio: + image: minio/minio + command: ["server", "data", "--console-address", ":9001"] + ports: + - "9000:9000" + - "9001:9001" + prometheus: + image: prom/prometheus + volumes: + - type: bind + source: ./prometheus.yml + target: /etc/prometheus/prometheus.yml + ports: + - "9090:9090" \ No newline at end of file diff --git a/scripts/prometheus.yml b/scripts/prometheus.yml new file mode 100644 index 0000000..2b23068 --- /dev/null +++ b/scripts/prometheus.yml @@ -0,0 +1,7 @@ +global: + scrape_interval: 5s +scrape_configs: + - job_name: active-storage-proxy + static_configs: + - targets: + - "active-storage-proxy:8080" \ No newline at end of file From aa3e2ce957c2b0d9c1885e5d108868063292b7b0 Mon Sep 17 00:00:00 2001 From: Scott Davidson <49713135+sd109@users.noreply.github.com> Date: Mon, 10 Jul 2023 13:45:15 +0100 Subject: [PATCH 14/14] Remove unnecessary buffer clone/clear Co-authored-by: Mark Goddard --- src/metrics.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index 831869c..20d4d38 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -48,11 +48,7 @@ pub async fn metrics_handler() -> String { .encode(&prometheus::gather(), &mut buffer) .expect("could not encode gathered metrics into temporary buffer"); - let output = - String::from_utf8(buffer.clone()).expect("could not convert metrics buffer into string"); - buffer.clear(); - - output + String::from_utf8(buffer).expect("could not convert metrics buffer into string") } pub async fn track_metrics(request: Request, next: Next) -> impl IntoResponse {