From 6ca981f08e532c700ff711c7d257ba002ed9642c Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Fri, 5 Jul 2024 22:08:12 +0000 Subject: [PATCH] Define HTTP request latency histogram timeseries in TOML --- nexus/src/context.rs | 2 +- openapi/nexus.json | 3 +- oximeter/impl/src/schema/codegen.rs | 1 + oximeter/impl/src/schema/mod.rs | 1 + oximeter/instruments/src/http.rs | 49 ++++++++-------------- oximeter/oximeter/schema/http-service.toml | 38 +++++++++++++++++ 6 files changed, 60 insertions(+), 34 deletions(-) create mode 100644 oximeter/oximeter/schema/http-service.toml diff --git a/nexus/src/context.rs b/nexus/src/context.rs index 1512671056..95d69e0c88 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -146,7 +146,7 @@ impl ServerContext { let authz = Arc::new(authz::Authz::new(&log)); let create_tracker = |name: &str| { let target = HttpService { - name: name.to_string(), + name: name.to_string().into(), id: config.deployment.id, }; const START_LATENCY_DECADE: i16 = -6; diff --git a/openapi/nexus.json b/openapi/nexus.json index ac38d1703a..4aacfcb420 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -19798,7 +19798,8 @@ "type": "string", "enum": [ "count", - "bytes" + "bytes", + "seconds" ] }, "User": { diff --git a/oximeter/impl/src/schema/codegen.rs b/oximeter/impl/src/schema/codegen.rs index 4aa09cf136..aaeadd7468 100644 --- a/oximeter/impl/src/schema/codegen.rs +++ b/oximeter/impl/src/schema/codegen.rs @@ -335,6 +335,7 @@ impl quote::ToTokens for Units { let toks = match self { Units::Count => quote! { ::oximeter::schema::Units::Count }, Units::Bytes => quote! { ::oximeter::schema::Units::Bytes }, + Units::Seconds => quote! { ::oximeter::schema::Units::Seconds }, }; toks.to_tokens(tokens); } diff --git a/oximeter/impl/src/schema/mod.rs b/oximeter/impl/src/schema/mod.rs index 28dbf38ab8..364b21503a 100644 --- a/oximeter/impl/src/schema/mod.rs +++ b/oximeter/impl/src/schema/mod.rs @@ -182,6 +182,7 @@ pub struct TimeseriesDescription { pub enum Units { Count, Bytes, + Seconds, } /// The schema for a timeseries. diff --git a/oximeter/instruments/src/http.rs b/oximeter/instruments/src/http.rs index 7db8bc27c3..a3852eb530 100644 --- a/oximeter/instruments/src/http.rs +++ b/oximeter/instruments/src/http.rs @@ -13,36 +13,24 @@ use futures::Future; use http::StatusCode; use http::Uri; use oximeter::{ - histogram::Histogram, histogram::Record, Metric, MetricsError, Producer, - Sample, Target, + histogram::Histogram, histogram::Record, MetricsError, Producer, Sample, }; +use std::borrow::Cow; use std::collections::BTreeMap; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; -use uuid::Uuid; -/// The [`HttpService`] is an [`oximeter::Target`] for monitoring HTTP servers. -#[derive(Debug, Clone, Target)] -pub struct HttpService { - pub name: String, - pub id: Uuid, -} - -/// An [`oximeter::Metric`] that tracks a histogram of the latency of requests to a specified HTTP -/// endpoint. -#[derive(Debug, Clone, Metric)] -pub struct RequestLatencyHistogram { - pub route: String, - pub method: String, - pub status_code: i64, - #[datum] - pub latency: Histogram, -} +oximeter::use_timeseries!("http-service.toml"); +pub use http_service::HttpService; +pub use http_service::RequestLatencyHistogram; // Return the route portion of the request, normalized to include a single // leading slash and no trailing slashes. -fn normalized_uri_path(uri: &Uri) -> String { - format!("/{}", uri.path().trim_end_matches('/').trim_start_matches('/')) +fn normalized_uri_path(uri: &Uri) -> Cow<'static, str> { + Cow::Owned(format!( + "/{}", + uri.path().trim_end_matches('/').trim_start_matches('/') + )) } impl RequestLatencyHistogram { @@ -56,9 +44,9 @@ impl RequestLatencyHistogram { ) -> Self { Self { route: normalized_uri_path(request.uri()), - method: request.method().to_string(), - status_code: status_code.as_u16().into(), - latency: histogram, + method: request.method().to_string().into(), + status_code: status_code.as_u16(), + datum: histogram, } } @@ -154,7 +142,7 @@ impl LatencyTracker { self.histogram.clone(), ) }); - entry.latency.sample(latency.as_secs_f64()).map_err(MetricsError::from) + entry.datum.sample(latency.as_secs_f64()).map_err(MetricsError::from) } /// Instrument the given Dropshot endpoint handler function. @@ -228,10 +216,8 @@ mod tests { #[test] fn test_latency_tracker() { - let service = HttpService { - name: String::from("my-service"), - id: ID.parse().unwrap(), - }; + let service = + HttpService { name: "my-service".into(), id: ID.parse().unwrap() }; let hist = Histogram::new(&[0.0, 1.0]).unwrap(); let tracker = LatencyTracker::new(service, hist); let request = http::request::Builder::new() @@ -249,8 +235,7 @@ mod tests { .unwrap(); let key = "/some/uri:GET:200"; - let actual_hist = - tracker.latencies.lock().unwrap()[key].latency.clone(); + let actual_hist = tracker.latencies.lock().unwrap()[key].datum.clone(); assert_eq!(actual_hist.n_samples(), 1); let bins = actual_hist.iter().collect::>(); assert_eq!(bins[1].count, 1); diff --git a/oximeter/oximeter/schema/http-service.toml b/oximeter/oximeter/schema/http-service.toml new file mode 100644 index 0000000000..c68e5b23e4 --- /dev/null +++ b/oximeter/oximeter/schema/http-service.toml @@ -0,0 +1,38 @@ +format_version = 1 + +[target] +name = "http_service" +description = "An Oxide HTTP server" +authz_scope = "fleet" +versions = [ + { version = 1, fields = [ "name", "id" ] }, +] + +[[metrics]] +name = "request_latency_histogram" +description = "Duration for the server to handle a request" +units = "seconds" +datum_type = "histogram_f64" +versions = [ + { added_in = 1, fields = [ "route", "method", "status_code" ] } +] + +[fields.name] +type = "string" +description = "The name of the HTTP server, or program running it" + +[fields.id] +type = "uuid" +description = "UUID of the HTTP server" + +[fields.route] +type = "string" +description = "HTTP route in the request" + +[fields.method] +type = "string" +description = "HTTP method in the request" + +[fields.status_code] +type = "u16" +description = "HTTP status code in the server's response"