From 618aacaa61972c3e25b8c996abfa1d4dc475154e Mon Sep 17 00:00:00 2001 From: Simon Hauser Date: Thu, 27 Jun 2024 10:27:29 +0200 Subject: [PATCH] feat(tvix/tracing): http trace propagation Introduces a helper function within tvix-tracing that returns a reqwest tracing middleware that will ingest the traceparent if otlp is enabled. It is feature flagged in tvix-tracing so not every consumer of that library automatically has reqwest in its dependencies. Tested using netcat to verify that the `traceparent` header is there if otlp is enabled and missing if otlp feature is disabled. Change-Id: I5abccae777b725f5ff7382e3686165383c477a39 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11886 Tested-by: BuildkiteCI Reviewed-by: flokli --- tvix/Cargo.lock | 57 ++++++ tvix/Cargo.nix | 218 ++++++++++++++++++++- tvix/docs/src/TODO.md | 7 +- tvix/store/Cargo.toml | 3 +- tvix/store/src/pathinfoservice/nix_http.rs | 6 +- tvix/tracing/Cargo.toml | 8 +- tvix/tracing/default.nix | 2 +- tvix/tracing/src/lib.rs | 1 - tvix/tracing/src/propagate/mod.rs | 4 +- tvix/tracing/src/propagate/reqwest.rs | 13 ++ 10 files changed, 302 insertions(+), 17 deletions(-) create mode 100644 tvix/tracing/src/propagate/reqwest.rs diff --git a/tvix/Cargo.lock b/tvix/Cargo.lock index e758b3597..731731e72 100644 --- a/tvix/Cargo.lock +++ b/tvix/Cargo.lock @@ -1432,8 +1432,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "190092ea657667030ac6a35e305e62fc4dd69fd98ac98631e5d3a2b1575a12b5" dependencies = [ "cfg-if", + "js-sys", "libc", "wasi", + "wasm-bindgen", ] [[package]] @@ -2032,6 +2034,16 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" +[[package]] +name = "mime_guess" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4192263c238a5f0d0c6bfd21f336a313a4ce1c450542449ca191bb657b4642ef" +dependencies = [ + "mime", + "unicase", +] + [[package]] name = "minimal-lexical" version = "0.2.1" @@ -2961,6 +2973,7 @@ dependencies = [ "js-sys", "log", "mime", + "mime_guess", "once_cell", "percent-encoding", "pin-project-lite", @@ -2983,6 +2996,39 @@ dependencies = [ "winreg", ] +[[package]] +name = "reqwest-middleware" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a735987236a8e238bf0296c7e351b999c188ccc11477f311b82b55c93984216" +dependencies = [ + "anyhow", + "async-trait", + "http", + "reqwest", + "serde", + "task-local-extensions", + "thiserror", +] + +[[package]] +name = "reqwest-tracing" +version = "0.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "190838e54153d7a7e2ea98851304b3ce92daeabf14c54d32b01b84a3e636f683" +dependencies = [ + "anyhow", + "async-trait", + "getrandom", + "matchit", + "opentelemetry", + "reqwest", + "reqwest-middleware", + "task-local-extensions", + "tracing", + "tracing-opentelemetry", +] + [[package]] name = "ring" version = "0.17.7" @@ -3642,6 +3688,15 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "task-local-extensions" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba323866e5d033818e3240feeb9f7db2c4296674e4d9e16b97b7bf8f490434e8" +dependencies = [ + "pin-utils", +] + [[package]] name = "tempfile" version = "3.9.0" @@ -4450,6 +4505,7 @@ dependencies = [ "prost", "prost-build", "reqwest", + "reqwest-middleware", "rstest", "rstest_reuse", "serde", @@ -4489,6 +4545,7 @@ dependencies = [ "opentelemetry-http", "opentelemetry-otlp", "opentelemetry_sdk", + "reqwest-tracing", "thiserror", "tokio", "tonic", diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix index 25a9a4e6b..4d6e1d946 100644 --- a/tvix/Cargo.nix +++ b/tvix/Cargo.nix @@ -4325,6 +4325,12 @@ rec { name = "cfg-if"; packageId = "cfg-if"; } + { + name = "js-sys"; + packageId = "js-sys"; + optional = true; + target = { target, features }: ((("wasm32" == target."arch" or null) || ("wasm64" == target."arch" or null)) && ("unknown" == target."os" or null)); + } { name = "libc"; packageId = "libc"; @@ -4337,6 +4343,13 @@ rec { usesDefaultFeatures = false; target = { target, features }: ("wasi" == target."os" or null); } + { + name = "wasm-bindgen"; + packageId = "wasm-bindgen"; + optional = true; + usesDefaultFeatures = false; + target = { target, features }: ((("wasm32" == target."arch" or null) || ("wasm64" == target."arch" or null)) && ("unknown" == target."os" or null)); + } ]; features = { "compiler_builtins" = [ "dep:compiler_builtins" ]; @@ -4346,7 +4359,7 @@ rec { "rustc-dep-of-std" = [ "compiler_builtins" "core" "libc/rustc-dep-of-std" "wasi/rustc-dep-of-std" ]; "wasm-bindgen" = [ "dep:wasm-bindgen" ]; }; - resolvedDefaultFeatures = [ "std" ]; + resolvedDefaultFeatures = [ "js" "js-sys" "std" "wasm-bindgen" ]; }; "gimli" = rec { crateName = "gimli"; @@ -6084,6 +6097,34 @@ rec { ]; }; + "mime_guess" = rec { + crateName = "mime_guess"; + version = "2.0.4"; + edition = "2015"; + sha256 = "1vs28rxnbfwil6f48hh58lfcx90klcvg68gxdc60spwa4cy2d4j1"; + authors = [ + "Austin Bonander " + ]; + dependencies = [ + { + name = "mime"; + packageId = "mime"; + } + { + name = "unicase"; + packageId = "unicase"; + } + ]; + buildDependencies = [ + { + name = "unicase"; + packageId = "unicase"; + } + ]; + features = { + "default" = [ "rev-mappings" ]; + }; + }; "minimal-lexical" = rec { crateName = "minimal-lexical"; version = "0.2.1"; @@ -8991,6 +9032,12 @@ rec { packageId = "mime"; target = { target, features }: (!("wasm32" == target."arch" or null)); } + { + name = "mime_guess"; + packageId = "mime_guess"; + optional = true; + usesDefaultFeatures = false; + } { name = "once_cell"; packageId = "once_cell"; @@ -9178,7 +9225,140 @@ rec { "wasm-streams" = [ "dep:wasm-streams" ]; "webpki-roots" = [ "dep:webpki-roots" ]; }; - resolvedDefaultFeatures = [ "__rustls" "__tls" "hyper-rustls" "json" "rustls" "rustls-native-certs" "rustls-pemfile" "rustls-tls-native-roots" "serde_json" "stream" "tokio-rustls" "tokio-util" "wasm-streams" ]; + resolvedDefaultFeatures = [ "__rustls" "__tls" "hyper-rustls" "json" "mime_guess" "multipart" "rustls" "rustls-native-certs" "rustls-pemfile" "rustls-tls-native-roots" "serde_json" "stream" "tokio-rustls" "tokio-util" "wasm-streams" ]; + }; + "reqwest-middleware" = rec { + crateName = "reqwest-middleware"; + version = "0.2.5"; + edition = "2018"; + sha256 = "05j2k29mrdc23cqpyiqirj61i74r3cspwv19y25j73ka4f3mjwss"; + authors = [ + "Rodrigo Gryzinski " + ]; + dependencies = [ + { + name = "anyhow"; + packageId = "anyhow"; + } + { + name = "async-trait"; + packageId = "async-trait"; + } + { + name = "http"; + packageId = "http"; + } + { + name = "reqwest"; + packageId = "reqwest"; + usesDefaultFeatures = false; + features = [ "json" "multipart" ]; + } + { + name = "serde"; + packageId = "serde"; + } + { + name = "task-local-extensions"; + packageId = "task-local-extensions"; + } + { + name = "thiserror"; + packageId = "thiserror"; + } + ]; + + }; + "reqwest-tracing" = rec { + crateName = "reqwest-tracing"; + version = "0.4.8"; + edition = "2018"; + sha256 = "10zn6vka710vn0r4vi8lpzmdm4nfnc2171cqxbiagmsk87jkh20r"; + authors = [ + "Rodrigo Gryzinski " + ]; + dependencies = [ + { + name = "anyhow"; + packageId = "anyhow"; + } + { + name = "async-trait"; + packageId = "async-trait"; + } + { + name = "getrandom"; + packageId = "getrandom"; + target = { target, features }: ("wasm32" == target."arch" or null); + features = [ "js" ]; + } + { + name = "matchit"; + packageId = "matchit"; + } + { + name = "opentelemetry"; + packageId = "opentelemetry"; + rename = "opentelemetry_0_22_pkg"; + optional = true; + } + { + name = "reqwest"; + packageId = "reqwest"; + usesDefaultFeatures = false; + } + { + name = "reqwest-middleware"; + packageId = "reqwest-middleware"; + } + { + name = "task-local-extensions"; + packageId = "task-local-extensions"; + } + { + name = "tracing"; + packageId = "tracing"; + } + { + name = "tracing-opentelemetry"; + packageId = "tracing-opentelemetry"; + rename = "tracing-opentelemetry_0_23_pkg"; + optional = true; + } + ]; + features = { + "opentelemetry_0_13" = [ "opentelemetry_0_13_pkg" "tracing-opentelemetry_0_12_pkg" ]; + "opentelemetry_0_13_pkg" = [ "dep:opentelemetry_0_13_pkg" ]; + "opentelemetry_0_14" = [ "opentelemetry_0_14_pkg" "tracing-opentelemetry_0_13_pkg" ]; + "opentelemetry_0_14_pkg" = [ "dep:opentelemetry_0_14_pkg" ]; + "opentelemetry_0_15" = [ "opentelemetry_0_15_pkg" "tracing-opentelemetry_0_14_pkg" ]; + "opentelemetry_0_15_pkg" = [ "dep:opentelemetry_0_15_pkg" ]; + "opentelemetry_0_16" = [ "opentelemetry_0_16_pkg" "tracing-opentelemetry_0_16_pkg" ]; + "opentelemetry_0_16_pkg" = [ "dep:opentelemetry_0_16_pkg" ]; + "opentelemetry_0_17" = [ "opentelemetry_0_17_pkg" "tracing-opentelemetry_0_17_pkg" ]; + "opentelemetry_0_17_pkg" = [ "dep:opentelemetry_0_17_pkg" ]; + "opentelemetry_0_18" = [ "opentelemetry_0_18_pkg" "tracing-opentelemetry_0_18_pkg" ]; + "opentelemetry_0_18_pkg" = [ "dep:opentelemetry_0_18_pkg" ]; + "opentelemetry_0_19" = [ "opentelemetry_0_19_pkg" "tracing-opentelemetry_0_19_pkg" ]; + "opentelemetry_0_19_pkg" = [ "dep:opentelemetry_0_19_pkg" ]; + "opentelemetry_0_20" = [ "opentelemetry_0_20_pkg" "tracing-opentelemetry_0_20_pkg" ]; + "opentelemetry_0_20_pkg" = [ "dep:opentelemetry_0_20_pkg" ]; + "opentelemetry_0_21" = [ "opentelemetry_0_21_pkg" "tracing-opentelemetry_0_22_pkg" ]; + "opentelemetry_0_21_pkg" = [ "dep:opentelemetry_0_21_pkg" ]; + "opentelemetry_0_22" = [ "opentelemetry_0_22_pkg" "tracing-opentelemetry_0_23_pkg" ]; + "opentelemetry_0_22_pkg" = [ "dep:opentelemetry_0_22_pkg" ]; + "tracing-opentelemetry_0_12_pkg" = [ "dep:tracing-opentelemetry_0_12_pkg" ]; + "tracing-opentelemetry_0_13_pkg" = [ "dep:tracing-opentelemetry_0_13_pkg" ]; + "tracing-opentelemetry_0_14_pkg" = [ "dep:tracing-opentelemetry_0_14_pkg" ]; + "tracing-opentelemetry_0_16_pkg" = [ "dep:tracing-opentelemetry_0_16_pkg" ]; + "tracing-opentelemetry_0_17_pkg" = [ "dep:tracing-opentelemetry_0_17_pkg" ]; + "tracing-opentelemetry_0_18_pkg" = [ "dep:tracing-opentelemetry_0_18_pkg" ]; + "tracing-opentelemetry_0_19_pkg" = [ "dep:tracing-opentelemetry_0_19_pkg" ]; + "tracing-opentelemetry_0_20_pkg" = [ "dep:tracing-opentelemetry_0_20_pkg" ]; + "tracing-opentelemetry_0_22_pkg" = [ "dep:tracing-opentelemetry_0_22_pkg" ]; + "tracing-opentelemetry_0_23_pkg" = [ "dep:tracing-opentelemetry_0_23_pkg" ]; + }; + resolvedDefaultFeatures = [ "opentelemetry_0_22" "opentelemetry_0_22_pkg" "tracing-opentelemetry_0_23_pkg" ]; }; "ring" = rec { crateName = "ring"; @@ -11164,6 +11344,23 @@ rec { features = { }; resolvedDefaultFeatures = [ "default" ]; }; + "task-local-extensions" = rec { + crateName = "task-local-extensions"; + version = "0.1.4"; + edition = "2018"; + sha256 = "1s1l0i4qzgxpjxmy3ng4fik2ki5jgngypzj06a782cyhwmk3hcms"; + authors = [ + "Conrad Ludgate " + "Rodrigo Gryzinski " + ]; + dependencies = [ + { + name = "pin-utils"; + packageId = "pin-utils"; + } + ]; + + }; "tempfile" = rec { crateName = "tempfile"; version = "3.9.0"; @@ -14184,6 +14381,10 @@ rec { usesDefaultFeatures = false; features = [ "rustls-tls-native-roots" "stream" ]; } + { + name = "reqwest-middleware"; + packageId = "reqwest-middleware"; + } { name = "serde"; packageId = "serde"; @@ -14267,7 +14468,7 @@ rec { { name = "tvix-tracing"; packageId = "tvix-tracing"; - features = [ "tonic" ]; + features = [ "tonic" "reqwest" ]; } { name = "url"; @@ -14361,6 +14562,12 @@ rec { optional = true; features = [ "rt-tokio" ]; } + { + name = "reqwest-tracing"; + packageId = "reqwest-tracing"; + optional = true; + usesDefaultFeatures = false; + } { name = "thiserror"; packageId = "thiserror"; @@ -14402,11 +14609,12 @@ rec { } ]; features = { - "otlp" = [ "dep:tracing-opentelemetry" "dep:opentelemetry" "dep:opentelemetry-otlp" "dep:opentelemetry_sdk" "dep:opentelemetry-http" ]; + "otlp" = [ "dep:tracing-opentelemetry" "dep:opentelemetry" "dep:opentelemetry-otlp" "dep:opentelemetry_sdk" "dep:opentelemetry-http" "reqwest-tracing?/opentelemetry_0_22" ]; + "reqwest" = [ "dep:reqwest-tracing" ]; "tonic" = [ "dep:tonic" "dep:http" ]; "tracy" = [ "dep:tracing-tracy" ]; }; - resolvedDefaultFeatures = [ "default" "otlp" "tonic" "tracy" ]; + resolvedDefaultFeatures = [ "default" "otlp" "reqwest" "tonic" "tracy" ]; }; "typenum" = rec { crateName = "typenum"; diff --git a/tvix/docs/src/TODO.md b/tvix/docs/src/TODO.md index 127fb6f4d..52fa0a9e7 100644 --- a/tvix/docs/src/TODO.md +++ b/tvix/docs/src/TODO.md @@ -224,7 +224,6 @@ logs etc, but this is something requiring a lot of designing. - Maybe drop `--log-level` entirely, and only use `RUST_LOG` env exclusively? `debug`,`trace` level across all crates is a bit useless, and `RUST_LOG` can be much more granular… - - Trace propagation for HTTP clients too, using - https://www.w3.org/TR/trace-context/ or https://www.w3.org/TR/baggage/, - whichever makes more sense. - Candidates: nix+http(s) protocol, object_store crates. + - Trace propagation for object_store once they support a way to register a + middleware, so we can use that to register a tracing middleware. + https://github.com/apache/arrow-rs/issues/5990 diff --git a/tvix/store/Cargo.toml b/tvix/store/Cargo.toml index e8fbaf02c..8063a0e87 100644 --- a/tvix/store/Cargo.toml +++ b/tvix/store/Cargo.toml @@ -36,9 +36,10 @@ tvix-castore = { path = "../castore" } url = "2.4.0" walkdir = "2.4.0" reqwest = { version = "0.11.22", features = ["rustls-tls-native-roots", "stream"], default-features = false } +reqwest-middleware = "0.2.5" lru = "0.12.3" parking_lot = "0.12.2" -tvix-tracing = { path = "../tracing", features = ["tonic"] } +tvix-tracing = { path = "../tracing", features = ["tonic", "reqwest"] } tracing = "0.1.40" tracing-indicatif = "0.3.6" diff --git a/tvix/store/src/pathinfoservice/nix_http.rs b/tvix/store/src/pathinfoservice/nix_http.rs index 1dd7da483..57fe37f44 100644 --- a/tvix/store/src/pathinfoservice/nix_http.rs +++ b/tvix/store/src/pathinfoservice/nix_http.rs @@ -31,7 +31,7 @@ use tvix_castore::{ /// TODO: what about reading from nix-cache-info? pub struct NixHTTPPathInfoService { base_url: url::Url, - http_client: reqwest::Client, + http_client: reqwest_middleware::ClientWithMiddleware, blob_service: BS, directory_service: DS, @@ -45,7 +45,9 @@ impl NixHTTPPathInfoService { pub fn new(base_url: url::Url, blob_service: BS, directory_service: DS) -> Self { Self { base_url, - http_client: reqwest::Client::new(), + http_client: reqwest_middleware::ClientBuilder::new(reqwest::Client::new()) + .with(tvix_tracing::propagate::reqwest::tracing_middleware()) + .build(), blob_service, directory_service, diff --git a/tvix/tracing/Cargo.toml b/tvix/tracing/Cargo.toml index bc9a8c3c7..80892bf7a 100644 --- a/tvix/tracing/Cargo.toml +++ b/tvix/tracing/Cargo.toml @@ -22,6 +22,8 @@ opentelemetry-http = { version = "0.11.0", optional = true } tonic = { version = "0.11.0", optional = true } http = { version = "0.2.11", optional = true } +reqwest-tracing = { version = "0.4.8", default-features = false, optional = true } + [features] default = [] otlp = [ @@ -29,7 +31,8 @@ otlp = [ "dep:opentelemetry", "dep:opentelemetry-otlp", "dep:opentelemetry_sdk", - "dep:opentelemetry-http" + "dep:opentelemetry-http", + "reqwest-tracing?/opentelemetry_0_22", ] tracy = [ "dep:tracing-tracy" @@ -38,6 +41,9 @@ tonic = [ "dep:tonic", "dep:http", ] +reqwest = [ + "dep:reqwest-tracing", +] [lints] workspace = true diff --git a/tvix/tracing/default.nix b/tvix/tracing/default.nix index dd7dc200f..a4ac9de1c 100644 --- a/tvix/tracing/default.nix +++ b/tvix/tracing/default.nix @@ -6,6 +6,6 @@ meta.ci.targets = lib.filter (x: lib.hasPrefix "with-features" x || x == "no-features") (lib.attrNames passthru); passthru = depot.tvix.utils.mkFeaturePowerset { inherit (old) crateName; - features = [ "otlp" "tracy" ]; + features = [ "otlp" "tracy" "tonic" "reqwest" ]; }; }) diff --git a/tvix/tracing/src/lib.rs b/tvix/tracing/src/lib.rs index b914408ff..35fdcda5a 100644 --- a/tvix/tracing/src/lib.rs +++ b/tvix/tracing/src/lib.rs @@ -17,7 +17,6 @@ use opentelemetry_sdk::{ #[cfg(feature = "tracy")] use tracing_tracy::TracyLayer; -#[cfg(feature = "tonic")] // TODO or http pub mod propagate; lazy_static! { diff --git a/tvix/tracing/src/propagate/mod.rs b/tvix/tracing/src/propagate/mod.rs index 42c532e9d..9a7e4332b 100644 --- a/tvix/tracing/src/propagate/mod.rs +++ b/tvix/tracing/src/propagate/mod.rs @@ -1,8 +1,8 @@ #[cfg(feature = "tonic")] pub mod tonic; -// TODO: Helper library for reqwest. We could use -// https://github.com/TrueLayer/reqwest-middleware/tree/main/reqwest-tracing to realise this +#[cfg(feature = "reqwest")] +pub mod reqwest; // TODO: Helper library for axum or another http server, see // https://github.com/hseeberger/hello-tracing-rs/blob/main/hello-tracing-common/src/otel/http.rs diff --git a/tvix/tracing/src/propagate/reqwest.rs b/tvix/tracing/src/propagate/reqwest.rs new file mode 100644 index 000000000..e2afb3e94 --- /dev/null +++ b/tvix/tracing/src/propagate/reqwest.rs @@ -0,0 +1,13 @@ +use reqwest_tracing::{SpanBackendWithUrl, TracingMiddleware}; + +/// Returns a new tracing middleware which can be used with reqwest_middleware. +/// It will then write the `traceparent` in the header on the request and additionally records the +/// `url` into `http.url`. +/// +/// If otlp feature is disabled, this will not insert a `traceparent` into the header. It will +/// basically function as a noop. +/// +/// `traceparent` => https://www.w3.org/TR/trace-context/#trace-context-http-headers-format +pub fn tracing_middleware() -> TracingMiddleware { + TracingMiddleware::::new() +}