diff --git a/crates/goose-test-support/src/otel.rs b/crates/goose-test-support/src/otel.rs index 4eed3370..1b7459f3 100644 --- a/crates/goose-test-support/src/otel.rs +++ b/crates/goose-test-support/src/otel.rs @@ -32,10 +32,14 @@ pub fn clear_otel_env(overrides: &[(&'static str, &'static str)]) -> OtelTestGua let mut keys: Vec<&'static str> = vec![ "OTEL_EXPORTER_OTLP_ENDPOINT", "OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", + "OTEL_EXPORTER_OTLP_LOGS_PROTOCOL", "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", + "OTEL_EXPORTER_OTLP_METRICS_PROTOCOL", "OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE", + "OTEL_EXPORTER_OTLP_PROTOCOL", "OTEL_EXPORTER_OTLP_TIMEOUT", "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", + "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL", "OTEL_LOG_LEVEL", "OTEL_LOGS_EXPORTER", "OTEL_METRICS_EXPORTER", diff --git a/crates/goose/src/otel/otlp.rs b/crates/goose/src/otel/otlp.rs index c9304967..e86479d0 100644 --- a/crates/goose/src/otel/otlp.rs +++ b/crates/goose/src/otel/otlp.rs @@ -24,6 +24,27 @@ static TRACER_PROVIDER: Mutex> = Mutex::new(None); static METER_PROVIDER: Mutex> = Mutex::new(None); static LOGGER_PROVIDER: Mutex> = Mutex::new(None); +static GRPC_PROTOCOL_WARNING_EMITTED: std::sync::Once = std::sync::Once::new(); + +/// One-shot stderr warning when `OTEL_EXPORTER_OTLP_PROTOCOL=grpc` is set +/// in an environment where goose was built without the `grpc-tonic` +/// transport feature. Using `tracing::warn!` here would race the OTel +/// subscriber that is being initialized; eprintln keeps it visible +/// regardless of subscriber state. +fn warn_grpc_protocol_skipped_once() { + GRPC_PROTOCOL_WARNING_EMITTED.call_once(|| { + eprintln!( + "goose otel: OTEL_EXPORTER_OTLP_PROTOCOL is set to a gRPC \ + variant, but this goose build only includes the HTTP \ + transport (http-proto). OTLP signals are disabled to \ + avoid background-thread panics. Set \ + OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf and point \ + OTEL_EXPORTER_OTLP_ENDPOINT at an http://…:4318 collector \ + to re-enable export." + ); + }); +} + #[derive(Debug, Clone, PartialEq)] pub enum ExporterType { Otlp, @@ -41,6 +62,35 @@ impl ExporterType { } } +/// Resolved transport protocol for an OTLP signal, per OTel spec: +/// signal-specific `OTEL_EXPORTER_OTLP_{SIGNAL}_PROTOCOL` overrides the +/// shared `OTEL_EXPORTER_OTLP_PROTOCOL`, and the default is `http/protobuf` +/// (matching what `.with_http()` produces in this build). +/// +/// goose's `opentelemetry-otlp` build only enables the `http-proto` / +/// `reqwest-client` transport features — not `grpc-tonic`. If the caller's +/// environment sets `…_PROTOCOL=grpc`, the `.with_http()` exporter still +/// builds successfully but its background batch / metric reader threads +/// panic on the first export with +/// `internal error: entered unreachable code: HTTP client should not +/// receive Grpc protocol`. We honour the env var by skipping the signal +/// rather than crashing detached threads. +fn signal_protocol_is_http(signal: &str) -> bool { + let signal_var = format!("OTEL_EXPORTER_OTLP_{}_PROTOCOL", signal.to_uppercase()); + let raw = env::var(&signal_var) + .ok() + .filter(|v| !v.is_empty()) + .or_else(|| env::var("OTEL_EXPORTER_OTLP_PROTOCOL").ok()) + .unwrap_or_default(); + match raw.trim().to_lowercase().as_str() { + // Default per spec when unset — matches `.with_http()`. + "" | "http/protobuf" | "http/json" => true, + // gRPC variants require the `grpc-tonic` feature, which goose + // does not enable. + _ => false, + } +} + /// Returns the exporter type for a signal, or None if disabled. /// /// Checks in order: @@ -148,6 +198,10 @@ fn create_otlp_tracing_layer() -> OtlpResult { let tracer_provider = match exporter { ExporterType::Otlp => { + if !signal_protocol_is_http("traces") { + warn_grpc_protocol_skipped_once(); + return Err("OTLP traces protocol is grpc but goose was built without grpc-tonic; skipping traces exporter".into()); + } let exporter = opentelemetry_otlp::SpanExporter::builder() .with_http() .build()?; @@ -192,6 +246,10 @@ fn create_otlp_metrics_layer() -> OtlpResult { let meter_provider = match exporter { ExporterType::Otlp => { + if !signal_protocol_is_http("metrics") { + warn_grpc_protocol_skipped_once(); + return Err("OTLP metrics protocol is grpc but goose was built without grpc-tonic; skipping metrics exporter".into()); + } let exporter = opentelemetry_otlp::MetricExporter::builder() .with_http() .with_temporality(temporality_preference()) @@ -223,6 +281,10 @@ fn create_otlp_logs_layer() -> OtlpResult { let logger_provider = match exporter { ExporterType::Otlp => { + if !signal_protocol_is_http("logs") { + warn_grpc_protocol_skipped_once(); + return Err("OTLP logs protocol is grpc but goose was built without grpc-tonic; skipping logs exporter".into()); + } let exporter = opentelemetry_otlp::LogExporter::builder() .with_http() .build()?; @@ -458,6 +520,67 @@ mod tests { assert_eq!(signal_exporter(signal), expected); } + #[test_case(&[], true; "default unset is http")] + #[test_case(&[("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf")], true; "http/protobuf shared")] + #[test_case(&[("OTEL_EXPORTER_OTLP_PROTOCOL", "http/json")], true; "http/json shared")] + #[test_case(&[("OTEL_EXPORTER_OTLP_PROTOCOL", "HTTP/PROTOBUF")], true; "case insensitive")] + #[test_case(&[("OTEL_EXPORTER_OTLP_PROTOCOL", " http/protobuf ")], true; "trimmed")] + #[test_case(&[("OTEL_EXPORTER_OTLP_PROTOCOL", "")], true; "empty falls through to default")] + #[test_case(&[("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc")], false; "shared grpc is not http")] + #[test_case(&[("OTEL_EXPORTER_OTLP_PROTOCOL", "GRPC")], false; "grpc case insensitive")] + fn signal_protocol_is_http_shared(env: &[(&'static str, &'static str)], expected: bool) { + let _guard = clear_otel_env(env); + assert_eq!(signal_protocol_is_http("traces"), expected); + assert_eq!(signal_protocol_is_http("metrics"), expected); + assert_eq!(signal_protocol_is_http("logs"), expected); + } + + #[test_case( + &[("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc"), ("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL", "http/protobuf")], + true, false, false; + "signal override beats shared grpc for traces only" + )] + #[test_case( + &[("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf"), ("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL", "grpc")], + true, false, true; + "signal override flips metrics to grpc" + )] + #[test_case( + &[("OTEL_EXPORTER_OTLP_LOGS_PROTOCOL", "")], + true, true, true; + "empty signal override falls through to default http" + )] + fn signal_protocol_is_http_per_signal( + env: &[(&'static str, &'static str)], + traces: bool, + metrics: bool, + logs: bool, + ) { + let _guard = clear_otel_env(env); + assert_eq!(signal_protocol_is_http("traces"), traces); + assert_eq!(signal_protocol_is_http("metrics"), metrics); + assert_eq!(signal_protocol_is_http("logs"), logs); + } + + /// When `OTEL_EXPORTER_OTLP_PROTOCOL=grpc` is set (matching the + /// Blox + Datadog Agent environment that produced the + /// "HTTP client should not receive Grpc protocol" panic), each + /// signal layer must short-circuit with an error instead of + /// building a panic-prone exporter. + #[test] + fn grpc_protocol_skips_layers() { + let rt = tokio::runtime::Runtime::new().unwrap(); + let _guard = rt.enter(); + let _env = clear_otel_env(&[ + ("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317"), + ("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc"), + ]); + assert!(create_otlp_tracing_layer().is_err()); + assert!(create_otlp_metrics_layer().is_err()); + assert!(create_otlp_logs_layer().is_err()); + shutdown_otlp(); + } + #[test_case("console"; "console")] #[test_case("otlp"; "otlp")] fn test_all_layers_ok(exporter: &'static str) {