From 8b9cc5ca3fea35ee54e309bde5a0a651c3b31322 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Wed, 2 Aug 2023 14:30:04 +0100 Subject: [PATCH 1/2] web_server.rs: Added bucket domain to observability. --- src/web/web_server.rs | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/web/web_server.rs b/src/web/web_server.rs index 48dcb5b1..2ab3f53b 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -120,18 +120,34 @@ impl WebServer { req: Request, addr: String, ) -> Result>, http::Error> { + let request_host_bucket = req + .headers() + .get(HOST) + .expect("No host header found") + .to_str() + .expect("Error converting host header to string") + .to_string(); + if let Ok(forwarded_for_ip_addr) = forwarded_headers::handle_forwarded_for_headers(req.headers()) { + // uri() below has a preceding '/', so no space with host info!( - "{} (via {}) {} {}", + "{} (via {}) {} {}{}", forwarded_for_ip_addr, addr, req.method(), + request_host_bucket, req.uri() ); } else { - info!("{} {} {}", addr, req.method(), req.uri()); + info!( + "{} {} {}{}", + addr, + req.method(), + request_host_bucket, + req.uri() + ); } // Lots of instrumentation @@ -140,12 +156,16 @@ impl WebServer { .span_builder(format!("Web {} request", req.method())) .with_trace_id(gen_trace_id()) .with_attributes(vec![ + KeyValue::new("domain", format!("{}", request_host_bucket.to_string())), KeyValue::new("method", format!("{}", req.method())), KeyValue::new("uri", req.uri().to_string()), ]) .start(&tracer); - let metrics_tags = &[KeyValue::new("method", req.method().to_string())]; + let metrics_tags = &[ + KeyValue::new("domain", request_host_bucket.to_string()), + KeyValue::new("method", req.method().to_string()), + ]; // The actual handler let res = self @@ -160,22 +180,30 @@ impl WebServer { // Returning the result match res { Ok(res) => { - debug!("{} {} {}", req.method(), res.status(), req.uri()); + debug!( + "{} {} {}{}", + req.method(), + res.status(), + request_host_bucket, + req.uri() + ); Ok(res .map(|body| BoxBody::new(http_body_util::BodyExt::map_err(body, Error::from)))) } Err(error) => { info!( - "{} {} {} {}", + "{} {} {}{} {}", req.method(), error.http_status_code(), + request_host_bucket, req.uri(), error ); self.metrics.error_counter.add( 1, &[ - metrics_tags[0].clone(), + KeyValue::new("domain", request_host_bucket.to_string()), + KeyValue::new("method", req.method().to_string()), KeyValue::new("status_code", error.http_status_code().to_string()), ], ); From 2f558898354a668b73579d7e09a0ecc922e9dcaf Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 14:54:06 +0100 Subject: [PATCH 2/2] add configuration option to enable/disable monitoring bucket in web metrics --- doc/book/reference-manual/configuration.md | 9 ++++ src/garage/server.rs | 2 +- src/util/config.rs | 3 ++ src/web/web_server.rs | 51 ++++++++++------------ 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/doc/book/reference-manual/configuration.md b/doc/book/reference-manual/configuration.md index f0a3b438..f545de29 100644 --- a/doc/book/reference-manual/configuration.md +++ b/doc/book/reference-manual/configuration.md @@ -75,6 +75,7 @@ root_domain = ".s3.garage" [s3_web] bind_addr = "[::]:3902" root_domain = ".web.garage" +add_host_to_metrics = true [admin] api_bind_addr = "0.0.0.0:3903" @@ -138,6 +139,7 @@ The `[s3_api]` section: [`s3_region`](#s3_region). The `[s3_web]` section: +[`add_host_to_metrics`](#web_add_host_to_metrics), [`bind_addr`](#web_bind_addr), [`root_domain`](#web_root_domain). @@ -744,6 +746,13 @@ For instance, if `root_domain` is `web.garage.eu`, a bucket called `deuxfleurs.f will be accessible either with hostname `deuxfleurs.fr.web.garage.eu` or with hostname `deuxfleurs.fr`. +#### `add_host_to_metrics` {#web_add_host_to_metrics} + +Whether to include the requested domain name (HTTP `Host` header) in the +Prometheus metrics of the web endpoint. This is disabled by default as the +number of possible values is not bounded and can be a source of cardinality +explosion in the exported metrics. + ### The `[admin]` section diff --git a/src/garage/server.rs b/src/garage/server.rs index 9e58fa6d..1dc86fd3 100644 --- a/src/garage/server.rs +++ b/src/garage/server.rs @@ -113,7 +113,7 @@ pub async fn run_server(config_file: PathBuf, secrets: Secrets) -> Result<(), Er if let Some(web_config) = &config.s3_web { info!("Initializing web server..."); - let web_server = WebServer::new(garage.clone(), web_config.root_domain.clone()); + let web_server = WebServer::new(garage.clone(), &web_config); servers.push(( "Web", tokio::spawn(web_server.run(web_config.bind_addr.clone(), watch_cancel.clone())), diff --git a/src/util/config.rs b/src/util/config.rs index b4e2b008..73fc4ff4 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -183,6 +183,9 @@ pub struct WebConfig { pub bind_addr: UnixOrTCPSocketAddress, /// Suffix to remove from domain name to find bucket pub root_domain: String, + /// Whether to add the requested domain to exported Prometheus metrics + #[serde(default)] + pub add_host_to_metrics: bool, } /// Configuration for the admin and monitoring HTTP API diff --git a/src/web/web_server.rs b/src/web/web_server.rs index 2ab3f53b..e73dab48 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -33,6 +33,7 @@ use garage_api_s3::get::{handle_get_without_ctx, handle_head_without_ctx}; use garage_model::garage::Garage; use garage_table::*; +use garage_util::config::WebConfig; use garage_util::data::Uuid; use garage_util::error::Error as GarageError; use garage_util::forwarded_headers; @@ -69,16 +70,18 @@ pub struct WebServer { garage: Arc, metrics: Arc, root_domain: String, + add_host_to_metrics: bool, } impl WebServer { /// Run a web server - pub fn new(garage: Arc, root_domain: String) -> Arc { + pub fn new(garage: Arc, config: &WebConfig) -> Arc { let metrics = Arc::new(WebMetrics::new()); Arc::new(WebServer { garage, metrics, - root_domain, + root_domain: config.root_domain.clone(), + add_host_to_metrics: config.add_host_to_metrics, }) } @@ -120,12 +123,11 @@ impl WebServer { req: Request, addr: String, ) -> Result>, http::Error> { - let request_host_bucket = req + let host_header = req .headers() .get(HOST) - .expect("No host header found") - .to_str() - .expect("Error converting host header to string") + .and_then(|x| x.to_str().ok()) + .unwrap_or("") .to_string(); if let Ok(forwarded_for_ip_addr) = @@ -137,17 +139,11 @@ impl WebServer { forwarded_for_ip_addr, addr, req.method(), - request_host_bucket, + host_header, req.uri() ); } else { - info!( - "{} {} {}{}", - addr, - req.method(), - request_host_bucket, - req.uri() - ); + info!("{} {} {}{}", addr, req.method(), host_header, req.uri()); } // Lots of instrumentation @@ -156,16 +152,16 @@ impl WebServer { .span_builder(format!("Web {} request", req.method())) .with_trace_id(gen_trace_id()) .with_attributes(vec![ - KeyValue::new("domain", format!("{}", request_host_bucket.to_string())), + KeyValue::new("host", format!("{}", host_header.clone())), KeyValue::new("method", format!("{}", req.method())), KeyValue::new("uri", req.uri().to_string()), ]) .start(&tracer); - let metrics_tags = &[ - KeyValue::new("domain", request_host_bucket.to_string()), - KeyValue::new("method", req.method().to_string()), - ]; + let mut metrics_tags = vec![KeyValue::new("method", req.method().to_string())]; + if self.add_host_to_metrics { + metrics_tags.push(KeyValue::new("host", host_header.clone())); + } // The actual handler let res = self @@ -184,7 +180,7 @@ impl WebServer { "{} {} {}{}", req.method(), res.status(), - request_host_bucket, + host_header, req.uri() ); Ok(res @@ -195,18 +191,15 @@ impl WebServer { "{} {} {}{} {}", req.method(), error.http_status_code(), - request_host_bucket, + host_header, req.uri(), error ); - self.metrics.error_counter.add( - 1, - &[ - KeyValue::new("domain", request_host_bucket.to_string()), - KeyValue::new("method", req.method().to_string()), - KeyValue::new("status_code", error.http_status_code().to_string()), - ], - ); + metrics_tags.push(KeyValue::new( + "status_code", + error.http_status_code().to_string(), + )); + self.metrics.error_counter.add(1, &metrics_tags); Ok(error_to_res(error)) } }