fix(otel): skip OTLP signals when protocol=grpc to avoid background-thread panic (#9512)

Co-authored-by: Joah's AI agent <noreply@blockxyz.com>
Co-authored-by: Amp <amp@ampcode.com>

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",

crates/goose/src/otel/otlp.rs

@@ -24,6 +24,27 @@ static TRACER_PROVIDER: Mutex<Option<SdkTracerProvider>> = Mutex::new(None);
 static METER_PROVIDER: Mutex<Option<SdkMeterProvider>> = Mutex::new(None);
 static LOGGER_PROVIDER: Mutex<Option<SdkLoggerProvider>> = 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<OtlpTracingLayer> {
 
     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<OtlpMetricsLayer> {
 
     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<OtlpLogsLayer> {
 
     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) {