From bf588da3c8559ec8f48d1eae52c568c1a99ab8ab Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Thu, 4 Jul 2024 21:19:02 +0000 Subject: [PATCH] Don't fail requests if we cannot track their latencies - Fixes #5998 - Add a helper trait to extract the status code from a generic kind of response, so that we can use the latency tracker for methods that return a `http::Response` directly. Implement this for the extant response types we need to instrument. - Add latency tracking to the device OAuth endpoints that were previously untracked. --- Cargo.lock | 97 ++++++++++++++------------- nexus/src/external_api/device_auth.rs | 16 +++-- oximeter/instruments/Cargo.toml | 6 ++ oximeter/instruments/src/http.rs | 24 ++++--- workspace-hack/Cargo.toml | 18 ++--- 5 files changed, 92 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d79933cd87..86963d2c67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -859,7 +859,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4895c018bb228aa6b3ba1a0285543fcb4b704734c3fb1f72afaa75aa769500c1" dependencies = [ "serde", - "toml 0.8.13", + "toml 0.8.14", ] [[package]] @@ -1951,7 +1951,7 @@ dependencies = [ "tempfile", "thiserror", "tokio", - "toml 0.8.13", + "toml 0.8.14", "trust-dns-client", "trust-dns-proto", "trust-dns-resolver", @@ -2022,14 +2022,14 @@ dependencies = [ "serde", "serde_json", "slog", - "toml 0.8.13", + "toml 0.8.14", "uuid", ] [[package]] name = "dropshot" version = "0.10.2-dev" -source = "git+https://github.com/oxidecomputer/dropshot?branch=main#0cd0e828d096578392b6a5524334d44fd10ef6da" +source = "git+https://github.com/oxidecomputer/dropshot?branch=main#6a3f84ca5fd8d0c5c010cfe837efbe6b5d117d9d" dependencies = [ "async-stream", "async-trait", @@ -2065,7 +2065,7 @@ dependencies = [ "slog-term", "tokio", "tokio-rustls 0.25.0", - "toml 0.8.13", + "toml 0.8.14", "usdt", "uuid", "version_check", @@ -2075,8 +2075,9 @@ dependencies = [ [[package]] name = "dropshot_endpoint" version = "0.10.2-dev" -source = "git+https://github.com/oxidecomputer/dropshot?branch=main#0cd0e828d096578392b6a5524334d44fd10ef6da" +source = "git+https://github.com/oxidecomputer/dropshot?branch=main#6a3f84ca5fd8d0c5c010cfe837efbe6b5d117d9d" dependencies = [ + "heck 0.5.0", "proc-macro2", "quote", "serde", @@ -2236,7 +2237,7 @@ dependencies = [ "serde_json", "socket2 0.5.7", "tokio", - "toml 0.8.13", + "toml 0.8.14", "trust-dns-resolver", "uuid", ] @@ -3296,7 +3297,7 @@ dependencies = [ "httpdate", "itoa", "pin-project-lite", - "socket2 0.5.7", + "socket2 0.4.10", "tokio", "tower-service", "tracing", @@ -3514,7 +3515,7 @@ dependencies = [ "smf", "thiserror", "tokio", - "toml 0.8.13", + "toml 0.8.14", "uuid", "whoami", "zone 0.3.0", @@ -4563,7 +4564,7 @@ dependencies = [ "serde_json", "serde_with", "tokio-postgres", - "toml 0.8.13", + "toml 0.8.14", "uuid", ] @@ -5314,7 +5315,7 @@ dependencies = [ "thiserror", "tokio", "tokio-postgres", - "toml 0.8.13", + "toml 0.8.14", "url", ] @@ -5362,7 +5363,7 @@ dependencies = [ "test-strategy", "thiserror", "tokio", - "toml 0.8.13", + "toml 0.8.14", "uuid", ] @@ -5387,7 +5388,7 @@ dependencies = [ "slog", "thiserror", "tokio", - "toml 0.8.13", + "toml 0.8.14", "uuid", ] @@ -5422,7 +5423,7 @@ dependencies = [ "subprocess", "tokio", "tokio-postgres", - "toml 0.8.13", + "toml 0.8.14", ] [[package]] @@ -5464,7 +5465,7 @@ dependencies = [ "tokio", "tokio-stream", "tokio-tungstenite 0.20.1", - "toml 0.8.13", + "toml 0.8.14", "uuid", ] @@ -5686,7 +5687,7 @@ dependencies = [ "tar", "thiserror", "tokio", - "toml 0.8.13", + "toml 0.8.14", "walkdir", ] @@ -5734,7 +5735,7 @@ dependencies = [ "slog-term", "tar", "tokio", - "toml 0.8.13", + "toml 0.8.14", "tufaceous-lib", ] @@ -5837,7 +5838,7 @@ dependencies = [ "tokio", "tokio-stream", "tokio-util", - "toml 0.8.13", + "toml 0.8.14", "usdt", "uuid", "zeroize", @@ -5981,6 +5982,7 @@ dependencies = [ "similar", "slog", "smallvec 1.13.2", + "socket2 0.5.7", "spin 0.9.8", "string_cache", "subtle", @@ -5995,7 +5997,7 @@ dependencies = [ "toml 0.7.8", "toml_datetime", "toml_edit 0.19.15", - "toml_edit 0.22.13", + "toml_edit 0.22.14", "tracing", "trust-dns-proto", "unicode-bidi", @@ -6257,7 +6259,7 @@ dependencies = [ "oximeter-timeseries-macro", "prettyplease", "syn 2.0.68", - "toml 0.8.13", + "toml 0.8.14", "uuid", ] @@ -6312,7 +6314,7 @@ dependencies = [ "subprocess", "thiserror", "tokio", - "toml 0.8.13", + "toml 0.8.14", "uuid", ] @@ -6390,7 +6392,7 @@ dependencies = [ "strum", "syn 2.0.68", "thiserror", - "toml 0.8.13", + "toml 0.8.14", "trybuild", "uuid", ] @@ -6404,11 +6406,14 @@ dependencies = [ "dropshot", "futures", "http 0.2.12", + "hyper 0.14.28", "kstat-rs", "libc", "omicron-workspace-hack", "oximeter", "rand 0.8.5", + "schemars", + "serde", "slog", "slog-async", "slog-term", @@ -8126,7 +8131,7 @@ dependencies = [ "serde", "tempfile", "thiserror", - "toml 0.8.13", + "toml 0.8.14", "toolchain_find", ] @@ -8561,9 +8566,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.118" +version = "1.0.120" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d947f6b3163d8857ea16c4fa0dd4840d52f3041039a85decd46867eb1abef2e4" +checksum = "4e0d21c9a8cae1235ad58a00c11cb40d4b1e5c784f1ef2c537876ed6ffd8b7c5" dependencies = [ "itoa", "ryu", @@ -8611,9 +8616,9 @@ dependencies = [ [[package]] name = "serde_tokenstream" -version = "0.2.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a00ffd23fd882d096f09fcaae2a9de8329a328628e86027e049ee051dc1621f" +checksum = "8790a7c3fe883e443eaa2af6f705952bc5d6e8671a220b9335c8cae92c037e74" dependencies = [ "proc-macro2", "quote", @@ -9178,7 +9183,7 @@ dependencies = [ "sprockets-rot", "thiserror", "tokio", - "toml 0.8.13", + "toml 0.8.14", ] [[package]] @@ -9877,9 +9882,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.37.0" +version = "1.38.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1adbebffeca75fcfd058afa480fb6c0b81e165a0323f9c9d39c9697e37c46787" +checksum = "ba4f4a02a7a80d6f274636f0aa95c7e383b912d41fe721a31f29e29698585a4a" dependencies = [ "backtrace", "bytes", @@ -9896,9 +9901,9 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.2.0" +version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b8a1e28f2deaa14e508979454cb3a223b10b938b45af148bc0986de36f1923b" +checksum = "5f5ae998a069d4b5aba8ee9dad856af7d520c3699e6159b185c2acd48155d39a" dependencies = [ "proc-macro2", "quote", @@ -10033,14 +10038,14 @@ dependencies = [ [[package]] name = "toml" -version = "0.8.13" +version = "0.8.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4e43f8cc456c9704c851ae29c67e17ef65d2c30017c17a9765b89c382dc8bba" +checksum = "6f49eb2ab21d2f26bd6db7bf383edc527a7ebaee412d17af4d40fdccd442f335" dependencies = [ "serde", "serde_spanned", "toml_datetime", - "toml_edit 0.22.13", + "toml_edit 0.22.14", ] [[package]] @@ -10067,9 +10072,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.22.13" +version = "0.22.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c127785850e8c20836d49732ae6abfa47616e60bf9d9f57c43c250361a9db96c" +checksum = "f21c7aaf97f1bd9ca9d4f9e73b0a6c74bd5afef56f2bc931943a6e1c37e04e38" dependencies = [ "indexmap 2.2.6", "serde", @@ -10295,7 +10300,7 @@ dependencies = [ "serde_derive", "serde_json", "termcolor", - "toml 0.8.13", + "toml 0.8.14", ] [[package]] @@ -10355,7 +10360,7 @@ dependencies = [ "slog", "tar", "tokio", - "toml 0.8.13", + "toml 0.8.14", "tough", "url", "zip", @@ -10732,9 +10737,9 @@ checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" [[package]] name = "uuid" -version = "1.8.0" +version = "1.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a183cf7feeba97b4dd1c0d46788634f6221d87fa961b305bed08c851829efcc0" +checksum = "5de17fd2f7da591098415cff336e12965a28061ddace43b59cb3c430179c9439" dependencies = [ "getrandom 0.2.14", "serde", @@ -11030,8 +11035,8 @@ dependencies = [ "textwrap", "tokio", "tokio-util", - "toml 0.8.13", - "toml_edit 0.22.13", + "toml 0.8.14", + "toml_edit 0.22.14", "tui-tree-widget", "unicode-width", "update-engine", @@ -11057,7 +11062,7 @@ dependencies = [ "sha2", "sled-hardware-types", "thiserror", - "toml 0.8.13", + "toml 0.8.14", "update-engine", ] @@ -11145,7 +11150,7 @@ dependencies = [ "tokio", "tokio-stream", "tokio-util", - "toml 0.8.13", + "toml 0.8.14", "tough", "trust-dns-resolver", "tufaceous", @@ -11459,7 +11464,7 @@ dependencies = [ "tabled", "tar", "tokio", - "toml 0.8.13", + "toml 0.8.14", "usdt", ] diff --git a/nexus/src/external_api/device_auth.rs b/nexus/src/external_api/device_auth.rs index 2aa1965e79..883dbf4e19 100644 --- a/nexus/src/external_api/device_auth.rs +++ b/nexus/src/external_api/device_auth.rs @@ -92,9 +92,11 @@ pub(crate) async fn device_auth_request( &model.into_response(rqctx.server.using_tls(), host), ) }; - // TODO: instrumentation doesn't work because we use `Response` - //apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await - handler.await + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await } #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] @@ -250,7 +252,9 @@ pub(crate) async fn device_access_token( ), } }; - // TODO: instrumentation doesn't work because we use `Response` - //apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await - handler.await + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await } diff --git a/oximeter/instruments/Cargo.toml b/oximeter/instruments/Cargo.toml index f51278f794..d5dcac411a 100644 --- a/oximeter/instruments/Cargo.toml +++ b/oximeter/instruments/Cargo.toml @@ -13,9 +13,12 @@ chrono = { workspace = true, optional = true } dropshot = { workspace = true, optional = true } futures = { workspace = true, optional = true } http = { workspace = true, optional = true } +hyper = { workspace = true, optional = true } kstat-rs = { workspace = true, optional = true } libc = { workspace = true, optional = true } oximeter = { workspace = true, optional = true } +schemars = { workspace = true, optional = true } +serde = { workspace = true, optional = true } slog = { workspace = true, optional = true } tokio = { workspace = true, optional = true } thiserror = { workspace = true, optional = true } @@ -29,7 +32,10 @@ http-instruments = [ "dep:dropshot", "dep:futures", "dep:http", + "dep:hyper", "dep:oximeter", + "dep:schemars", + "dep:serde", "dep:uuid" ] kstat = [ diff --git a/oximeter/instruments/src/http.rs b/oximeter/instruments/src/http.rs index 4bc6cf8677..7db8bc27c3 100644 --- a/oximeter/instruments/src/http.rs +++ b/oximeter/instruments/src/http.rs @@ -4,7 +4,7 @@ //! Instrumentation tools for HTTP services. -// Copyright 2021 Oxide Computer Company +// Copyright 2024 Oxide Computer Company use dropshot::{ HttpError, HttpResponse, RequestContext, RequestInfo, ServerContext, @@ -178,16 +178,22 @@ impl LatencyTracker { let start = Instant::now(); let result = handler.await; let latency = start.elapsed(); - let status_code = match result { - Ok(_) => R::response_metadata().success.unwrap(), + let status_code = match &result { + Ok(response) => response.status_code(), Err(ref e) => e.status_code, }; - self.update(&context.request, status_code, latency).map_err(|e| { - HttpError::for_internal_error(format!( - "error instrumenting dropshot request handler: {}", - e - )) - })?; + if let Err(e) = self.update(&context.request, status_code, latency) { + slog::error!( + &context.log, + "error instrumenting dropshot handler"; + "error" => ?e, + "status_code" => status_code.as_u16(), + "method" => %context.request.method(), + "uri" => %context.request.uri(), + "remote_addr" => context.request.remote_addr(), + "latency" => ?latency, + ); + } result } } diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index adcc511763..2d9a4731f9 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -93,29 +93,30 @@ schemars = { version = "0.8.21", features = ["bytes", "chrono", "uuid1"] } scopeguard = { version = "1.2.0" } semver = { version = "1.0.23", features = ["serde"] } serde = { version = "1.0.203", features = ["alloc", "derive", "rc"] } -serde_json = { version = "1.0.118", features = ["raw_value", "unbounded_depth"] } +serde_json = { version = "1.0.120", features = ["raw_value", "unbounded_depth"] } sha2 = { version = "0.10.8", features = ["oid"] } similar = { version = "2.5.0", features = ["inline", "unicode"] } slog = { version = "2.7.0", features = ["dynamic-keys", "max_level_trace", "release_max_level_debug", "release_max_level_trace"] } smallvec = { version = "1.13.2", default-features = false, features = ["const_new"] } +socket2 = { version = "0.5.7", default-features = false, features = ["all"] } spin = { version = "0.9.8" } string_cache = { version = "0.8.7" } subtle = { version = "2.5.0" } syn-f595c2ba2a3f28df = { package = "syn", version = "2.0.68", features = ["extra-traits", "fold", "full", "visit", "visit-mut"] } time = { version = "0.3.36", features = ["formatting", "local-offset", "macros", "parsing"] } -tokio = { version = "1.37.0", features = ["full", "test-util"] } +tokio = { version = "1.38.0", features = ["full", "test-util"] } tokio-postgres = { version = "0.7.10", features = ["with-chrono-0_4", "with-serde_json-1", "with-uuid-1"] } tokio-stream = { version = "0.1.15", features = ["net"] } tokio-util = { version = "0.7.11", features = ["codec", "io-util"] } toml = { version = "0.7.8" } -toml_edit-3c51e837cfc5589a = { package = "toml_edit", version = "0.22.13", features = ["serde"] } +toml_edit-3c51e837cfc5589a = { package = "toml_edit", version = "0.22.14", features = ["serde"] } tracing = { version = "0.1.40", features = ["log"] } trust-dns-proto = { version = "0.22.0" } unicode-bidi = { version = "0.3.15" } unicode-normalization = { version = "0.1.23" } usdt = { version = "0.5.0" } usdt-impl = { version = "0.5.0", default-features = false, features = ["asm", "des"] } -uuid = { version = "1.8.0", features = ["serde", "v4"] } +uuid = { version = "1.9.1", features = ["serde", "v4"] } yasna = { version = "0.5.2", features = ["bit-vec", "num-bigint", "std", "time"] } zerocopy = { version = "0.7.34", features = ["derive", "simd"] } zeroize = { version = "1.7.0", features = ["std", "zeroize_derive"] } @@ -197,11 +198,12 @@ schemars = { version = "0.8.21", features = ["bytes", "chrono", "uuid1"] } scopeguard = { version = "1.2.0" } semver = { version = "1.0.23", features = ["serde"] } serde = { version = "1.0.203", features = ["alloc", "derive", "rc"] } -serde_json = { version = "1.0.118", features = ["raw_value", "unbounded_depth"] } +serde_json = { version = "1.0.120", features = ["raw_value", "unbounded_depth"] } sha2 = { version = "0.10.8", features = ["oid"] } similar = { version = "2.5.0", features = ["inline", "unicode"] } slog = { version = "2.7.0", features = ["dynamic-keys", "max_level_trace", "release_max_level_debug", "release_max_level_trace"] } smallvec = { version = "1.13.2", default-features = false, features = ["const_new"] } +socket2 = { version = "0.5.7", default-features = false, features = ["all"] } spin = { version = "0.9.8" } string_cache = { version = "0.8.7" } subtle = { version = "2.5.0" } @@ -209,12 +211,12 @@ syn-dff4ba8e3ae991db = { package = "syn", version = "1.0.109", features = ["extr syn-f595c2ba2a3f28df = { package = "syn", version = "2.0.68", features = ["extra-traits", "fold", "full", "visit", "visit-mut"] } time = { version = "0.3.36", features = ["formatting", "local-offset", "macros", "parsing"] } time-macros = { version = "0.2.18", default-features = false, features = ["formatting", "parsing"] } -tokio = { version = "1.37.0", features = ["full", "test-util"] } +tokio = { version = "1.38.0", features = ["full", "test-util"] } tokio-postgres = { version = "0.7.10", features = ["with-chrono-0_4", "with-serde_json-1", "with-uuid-1"] } tokio-stream = { version = "0.1.15", features = ["net"] } tokio-util = { version = "0.7.11", features = ["codec", "io-util"] } toml = { version = "0.7.8" } -toml_edit-3c51e837cfc5589a = { package = "toml_edit", version = "0.22.13", features = ["serde"] } +toml_edit-3c51e837cfc5589a = { package = "toml_edit", version = "0.22.14", features = ["serde"] } tracing = { version = "0.1.40", features = ["log"] } trust-dns-proto = { version = "0.22.0" } unicode-bidi = { version = "0.3.15" } @@ -222,7 +224,7 @@ unicode-normalization = { version = "0.1.23" } unicode-xid = { version = "0.2.4" } usdt = { version = "0.5.0" } usdt-impl = { version = "0.5.0", default-features = false, features = ["asm", "des"] } -uuid = { version = "1.8.0", features = ["serde", "v4"] } +uuid = { version = "1.9.1", features = ["serde", "v4"] } yasna = { version = "0.5.2", features = ["bit-vec", "num-bigint", "std", "time"] } zerocopy = { version = "0.7.34", features = ["derive", "simd"] } zeroize = { version = "1.7.0", features = ["std", "zeroize_derive"] }