From 829a910e84ba57fe42d12321ee087078ebbcf5d2 Mon Sep 17 00:00:00 2001 From: Scott Gerring Date: Mon, 10 Nov 2025 10:36:04 +0100 Subject: [PATCH] chore(otlp): Make invalid proto combos unrepresentable --- opentelemetry-otlp/CHANGELOG.md | 22 +++ .../examples/basic-otlp-http/src/main.rs | 10 +- opentelemetry-otlp/src/exporter/http/logs.rs | 17 +- .../src/exporter/http/metrics.rs | 17 +- opentelemetry-otlp/src/exporter/http/mod.rs | 125 ++++++++++----- opentelemetry-otlp/src/exporter/http/trace.rs | 17 +- opentelemetry-otlp/src/exporter/mod.rs | 145 ++++++------------ opentelemetry-otlp/src/exporter/tonic/mod.rs | 5 +- opentelemetry-otlp/src/lib.rs | 40 ++--- 9 files changed, 211 insertions(+), 187 deletions(-) diff --git a/opentelemetry-otlp/CHANGELOG.md b/opentelemetry-otlp/CHANGELOG.md index 6f91c6c6ca..1bc381e7c1 100644 --- a/opentelemetry-otlp/CHANGELOG.md +++ b/opentelemetry-otlp/CHANGELOG.md @@ -4,6 +4,28 @@ - Add partial success response handling for OTLP exporters (traces, metrics, logs) per OTLP spec. Exporters now log warnings when the server returns partial success responses with rejected items and error messages. [#865](https://github.com/open-telemetry/opentelemetry-rust/issues/865) - Refactor `internal-logs` feature in `opentelemetry-otlp` to reduce unnecessary dependencies[3191](https://github.com/open-telemetry/opentelemetry-rust/pull/3192) +- **Breaking** Make invalid protocol/transport combinations unrepresentable [#3082](https://github.com/open-telemetry/opentelemetry-rust/issues/3082) + - Removed `protocol` field from `ExportConfig` and `with_protocol()` method from `WithExportConfig` trait + - Added new HTTP-specific `Protocol` enum with `HttpProtobuf` and `HttpJson` variants + - Added `with_protocol()` method to `WithHttpConfig` trait that accepts the HTTP-specific `Protocol` enum + - The `Protocol` enum is now only available and only needed when using HTTP transport + - This change makes invalid protocol/transport combinations (like using HTTP protocol with gRPC transport) impossible to express + - Migration example: + ```rust + // Before: + SpanExporter::builder() + .with_http() + .with_protocol(Protocol::HttpBinary) // old Protocol from ExportConfig + .build() + + // After: + use opentelemetry_otlp::Protocol; + + SpanExporter::builder() + .with_http() + .with_protocol(Protocol::HttpProtobuf) // HTTP-specific Protocol enum + .build() + ``` ## 0.31.0 diff --git a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs index 8cffe0c6b1..f94790f1a0 100644 --- a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs @@ -4,8 +4,8 @@ use opentelemetry::{ InstrumentationScope, KeyValue, }; use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge; -use opentelemetry_otlp::WithExportConfig; -use opentelemetry_otlp::{LogExporter, MetricExporter, Protocol, SpanExporter}; +use opentelemetry_otlp::{LogExporter, MetricExporter, SpanExporter}; +use opentelemetry_otlp::{Protocol, WithHttpConfig}; use opentelemetry_sdk::Resource; use opentelemetry_sdk::{ logs::SdkLoggerProvider, metrics::SdkMeterProvider, trace::SdkTracerProvider, @@ -29,7 +29,7 @@ fn get_resource() -> Resource { fn init_logs() -> SdkLoggerProvider { let exporter = LogExporter::builder() .with_http() - .with_protocol(Protocol::HttpBinary) + .with_protocol(Protocol::HttpProtobuf) .build() .expect("Failed to create log exporter"); @@ -42,7 +42,7 @@ fn init_logs() -> SdkLoggerProvider { fn init_traces() -> SdkTracerProvider { let exporter = SpanExporter::builder() .with_http() - .with_protocol(Protocol::HttpBinary) //can be changed to `Protocol::HttpJson` to export in JSON format + .with_protocol(Protocol::HttpProtobuf) .build() .expect("Failed to create trace exporter"); @@ -55,7 +55,7 @@ fn init_traces() -> SdkTracerProvider { fn init_metrics() -> SdkMeterProvider { let exporter = MetricExporter::builder() .with_http() - .with_protocol(Protocol::HttpBinary) //can be changed to `Protocol::HttpJson` to export in JSON format + .with_protocol(Protocol::HttpProtobuf) .build() .expect("Failed to create metric exporter"); diff --git a/opentelemetry-otlp/src/exporter/http/logs.rs b/opentelemetry-otlp/src/exporter/http/logs.rs index 7e19bdf7a4..e974a27ade 100644 --- a/opentelemetry-otlp/src/exporter/http/logs.rs +++ b/opentelemetry-otlp/src/exporter/http/logs.rs @@ -1,5 +1,4 @@ -use super::OtlpHttpClient; -use crate::Protocol; +use super::{OtlpHttpClient, Protocol}; use opentelemetry::{otel_debug, otel_warn}; use opentelemetry_sdk::error::{OTelSdkError, OTelSdkResult}; use opentelemetry_sdk::logs::{LogBatch, LogExporter}; @@ -51,7 +50,15 @@ fn handle_partial_success(response_body: &[u8], protocol: Protocol) { return; } }, - _ => match Message::decode(response_body) { + Protocol::HttpProtobuf => match Message::decode(response_body) { + Ok(r) => r, + Err(e) => { + otel_debug!(name: "HttpLogsClient.ResponseParseError", error = e.to_string()); + return; + } + }, + #[cfg(not(feature = "http-json"))] + Protocol::HttpJson => match Message::decode(response_body) { Ok(r) => r, Err(e) => { otel_debug!(name: "HttpLogsClient.ResponseParseError", error = e.to_string()); @@ -81,7 +88,7 @@ mod tests { let invalid = vec![0xFF, 0xFF, 0xFF, 0xFF]; // Should not panic - logs debug and returns early - handle_partial_success(&invalid, Protocol::HttpBinary); + handle_partial_success(&invalid, Protocol::HttpProtobuf); } #[test] @@ -89,7 +96,7 @@ mod tests { let empty = vec![]; // Should not panic - handle_partial_success(&empty, Protocol::HttpBinary); + handle_partial_success(&empty, Protocol::HttpProtobuf); } #[cfg(feature = "http-json")] diff --git a/opentelemetry-otlp/src/exporter/http/metrics.rs b/opentelemetry-otlp/src/exporter/http/metrics.rs index 0be05ccc32..77bc3c6362 100644 --- a/opentelemetry-otlp/src/exporter/http/metrics.rs +++ b/opentelemetry-otlp/src/exporter/http/metrics.rs @@ -1,11 +1,10 @@ use crate::metric::MetricsClient; -use crate::Protocol; use opentelemetry::{otel_debug, otel_warn}; use opentelemetry_sdk::error::{OTelSdkError, OTelSdkResult}; use opentelemetry_sdk::metrics::data::ResourceMetrics; use prost::Message; -use super::OtlpHttpClient; +use super::{OtlpHttpClient, Protocol}; impl MetricsClient for OtlpHttpClient { async fn export(&self, metrics: &ResourceMetrics) -> OTelSdkResult { @@ -47,7 +46,15 @@ fn handle_partial_success(response_body: &[u8], protocol: Protocol) { return; } }, - _ => match Message::decode(response_body) { + Protocol::HttpProtobuf => match Message::decode(response_body) { + Ok(r) => r, + Err(e) => { + otel_debug!(name: "HttpMetricsClient.ResponseParseError", error = e.to_string()); + return; + } + }, + #[cfg(not(feature = "http-json"))] + Protocol::HttpJson => match Message::decode(response_body) { Ok(r) => r, Err(e) => { otel_debug!(name: "HttpMetricsClient.ResponseParseError", error = e.to_string()); @@ -77,7 +84,7 @@ mod tests { let invalid = vec![0xFF, 0xFF, 0xFF, 0xFF]; // Should not panic - logs debug and returns early - handle_partial_success(&invalid, Protocol::HttpBinary); + handle_partial_success(&invalid, Protocol::HttpProtobuf); } #[test] @@ -85,7 +92,7 @@ mod tests { let empty = vec![]; // Should not panic - handle_partial_success(&empty, Protocol::HttpBinary); + handle_partial_success(&empty, Protocol::HttpProtobuf); } #[cfg(feature = "http-json")] diff --git a/opentelemetry-otlp/src/exporter/http/mod.rs b/opentelemetry-otlp/src/exporter/http/mod.rs index 2bf427546b..d1a0df3cf7 100644 --- a/opentelemetry-otlp/src/exporter/http/mod.rs +++ b/opentelemetry-otlp/src/exporter/http/mod.rs @@ -1,8 +1,8 @@ use super::{ - default_headers, default_protocol, parse_header_string, resolve_timeout, ExporterBuildError, + default_headers, parse_header_string, resolve_timeout, ExporterBuildError, OTEL_EXPORTER_OTLP_HTTP_ENDPOINT_DEFAULT, }; -use crate::{ExportConfig, Protocol, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS}; +use crate::{ExportConfig, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS}; use http::{HeaderName, HeaderValue, Uri}; use opentelemetry::otel_debug; use opentelemetry_http::{Bytes, HttpClient}; @@ -100,6 +100,32 @@ mod trace; ))] use opentelemetry_http::hyper::HyperClient; +/// The protocol to used for HTTP requests +/// +/// This combination of transport (HTTP) and encoding (protobuf / JSON) is +/// used in the OTel spec itself. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum Protocol { + /// Binary protobuf encoding over HTTP + HttpProtobuf, + /// JSON encoding over HTTP + HttpJson, +} + +#[allow(clippy::derivable_impls)] +impl Default for Protocol { + fn default() -> Self { + #[cfg(feature = "http-json")] + { + Protocol::HttpJson + } + #[cfg(not(feature = "http-json"))] + { + Protocol::HttpProtobuf + } + } +} + /// Configuration of the http transport #[derive(Debug, Default)] pub struct HttpConfig { @@ -115,6 +141,9 @@ pub struct HttpConfig { /// The retry policy to use for HTTP requests. #[cfg(feature = "experimental-http-retry")] retry_policy: Option, + + /// The encoding format for HTTP payloads. + protocol: Protocol, } /// Configuration for the OTLP HTTP exporter. @@ -153,10 +182,7 @@ pub struct HttpExporterBuilder { impl Default for HttpExporterBuilder { fn default() -> Self { HttpExporterBuilder { - exporter_config: ExportConfig { - protocol: default_protocol(), - ..ExportConfig::default() - }, + exporter_config: ExportConfig::default(), http_config: HttpConfig { headers: Some(default_headers()), ..HttpConfig::default() @@ -283,7 +309,7 @@ impl HttpExporterBuilder { http_client, endpoint, headers, - self.exporter_config.protocol, + self.http_config.protocol, timeout, compression, #[cfg(feature = "experimental-http-retry")] @@ -557,7 +583,7 @@ impl OtlpHttpClient { client: Arc, collector_endpoint: Uri, headers: HashMap, - protocol: Protocol, + encoding: Protocol, timeout: Duration, compression: Option, #[cfg(feature = "experimental-http-retry")] retry_policy: Option, @@ -566,7 +592,7 @@ impl OtlpHttpClient { client: Mutex::new(Some(client)), collector_endpoint, headers: Arc::new(headers), - protocol, + protocol: encoding, _timeout: timeout, compression, #[cfg(feature = "experimental-http-retry")] @@ -595,7 +621,12 @@ impl OtlpHttpClient { Ok(json) => (json.into_bytes(), "application/json"), Err(e) => return Err(e.to_string()), }, - _ => (req.encode_to_vec(), "application/x-protobuf"), + Protocol::HttpProtobuf => (req.encode_to_vec(), "application/x-protobuf"), + #[cfg(not(feature = "http-json"))] + Protocol::HttpJson => { + // This should never happen as we validate at build time, but handle it gracefully + (req.encode_to_vec(), "application/x-protobuf") + } }; let (processed_body, content_encoding) = self.process_body(body)?; @@ -617,7 +648,12 @@ impl OtlpHttpClient { Ok(json) => (json.into_bytes(), "application/json"), Err(e) => return Err(e.to_string()), }, - _ => (req.encode_to_vec(), "application/x-protobuf"), + Protocol::HttpProtobuf => (req.encode_to_vec(), "application/x-protobuf"), + #[cfg(not(feature = "http-json"))] + Protocol::HttpJson => { + // This should never happen as we validate at build time, but handle it gracefully + (req.encode_to_vec(), "application/x-protobuf") + } }; let (processed_body, content_encoding) = self.process_body(body)?; @@ -642,7 +678,13 @@ impl OtlpHttpClient { return None; } }, - _ => (req.encode_to_vec(), "application/x-protobuf"), + Protocol::HttpProtobuf => (req.encode_to_vec(), "application/x-protobuf"), + #[cfg(not(feature = "http-json"))] + Protocol::HttpJson => { + // This should never happen as we validate at build time, but handle it gracefully + otel_debug!(name: "JsonNotAvailable", message = "http-json feature not enabled, falling back to protobuf"); + (req.encode_to_vec(), "application/x-protobuf") + } }; match self.process_body(body) { @@ -750,6 +792,11 @@ pub trait WithHttpConfig { /// Set the retry policy for HTTP requests. #[cfg(feature = "experimental-http-retry")] fn with_retry_policy(self, policy: RetryPolicy) -> Self; + + /// Set the payload encoding format. + /// + /// Note: JSON encoding requires the `http-json` feature to be enabled. + fn with_protocol(self, protocol: Protocol) -> Self; } impl WithHttpConfig for B { @@ -780,11 +827,16 @@ impl WithHttpConfig for B { self.http_client_config().retry_policy = Some(policy); self } + + fn with_protocol(mut self, protocol: Protocol) -> Self { + self.http_client_config().protocol = protocol; + self + } } #[cfg(test)] mod tests { - use crate::exporter::http::HttpConfig; + use crate::exporter::http::{HttpConfig, Protocol}; use crate::exporter::tests::run_env_test; use crate::{ HttpExporterBuilder, WithExportConfig, WithHttpConfig, OTEL_EXPORTER_OTLP_ENDPOINT, @@ -1027,6 +1079,7 @@ mod tests { compression: None, #[cfg(feature = "experimental-http-retry")] retry_policy: None, + protocol: Protocol::default(), }, exporter_config: crate::ExportConfig::default(), }; @@ -1079,7 +1132,7 @@ mod tests { #[cfg(feature = "gzip-http")] mod compression_tests { - use super::super::OtlpHttpClient; + use super::super::{OtlpHttpClient, Protocol}; use flate2::read::GzDecoder; use opentelemetry_http::{Bytes, HttpClient}; use std::io::Read; @@ -1090,7 +1143,7 @@ mod tests { std::sync::Arc::new(MockHttpClient), "http://localhost:4318".parse().unwrap(), std::collections::HashMap::new(), - crate::Protocol::HttpBinary, + Protocol::HttpProtobuf, std::time::Duration::from_secs(10), Some(crate::Compression::Gzip), #[cfg(feature = "experimental-http-retry")] @@ -1123,7 +1176,7 @@ mod tests { std::sync::Arc::new(MockHttpClient), "http://localhost:4318".parse().unwrap(), std::collections::HashMap::new(), - crate::Protocol::HttpBinary, + Protocol::HttpProtobuf, std::time::Duration::from_secs(10), Some(crate::Compression::Zstd), #[cfg(feature = "experimental-http-retry")] @@ -1153,7 +1206,7 @@ mod tests { std::sync::Arc::new(MockHttpClient), "http://localhost:4318".parse().unwrap(), std::collections::HashMap::new(), - crate::Protocol::HttpBinary, + Protocol::HttpProtobuf, std::time::Duration::from_secs(10), None, // No compression #[cfg(feature = "experimental-http-retry")] @@ -1176,7 +1229,7 @@ mod tests { std::sync::Arc::new(MockHttpClient), "http://localhost:4318".parse().unwrap(), std::collections::HashMap::new(), - crate::Protocol::HttpBinary, + Protocol::HttpProtobuf, std::time::Duration::from_secs(10), Some(crate::Compression::Gzip), #[cfg(feature = "experimental-http-retry")] @@ -1200,7 +1253,7 @@ mod tests { std::sync::Arc::new(MockHttpClient), "http://localhost:4318".parse().unwrap(), std::collections::HashMap::new(), - crate::Protocol::HttpBinary, + Protocol::HttpProtobuf, std::time::Duration::from_secs(10), Some(crate::Compression::Zstd), #[cfg(feature = "experimental-http-retry")] @@ -1236,7 +1289,7 @@ mod tests { } mod export_body_tests { - use super::super::OtlpHttpClient; + use super::super::{OtlpHttpClient, Protocol}; use opentelemetry_http::{Bytes, HttpClient}; use std::collections::HashMap; @@ -1257,14 +1310,14 @@ mod tests { } fn create_test_client( - protocol: crate::Protocol, + encoding: Protocol, compression: Option, ) -> OtlpHttpClient { OtlpHttpClient::new( std::sync::Arc::new(MockHttpClient), "http://localhost:4318".parse().unwrap(), HashMap::new(), - protocol, + encoding, std::time::Duration::from_secs(10), compression, #[cfg(feature = "experimental-http-retry")] @@ -1308,7 +1361,7 @@ mod tests { #[cfg(feature = "trace")] #[test] fn test_build_trace_export_body_binary_protocol() { - let client = create_test_client(crate::Protocol::HttpBinary, None); + let client = create_test_client(Protocol::HttpProtobuf, None); let span_data = create_test_span_data(); let result = client.build_trace_export_body(vec![span_data]).unwrap(); @@ -1321,7 +1374,7 @@ mod tests { #[cfg(all(feature = "trace", feature = "http-json"))] #[test] fn test_build_trace_export_body_json_protocol() { - let client = create_test_client(crate::Protocol::HttpJson, None); + let client = create_test_client(Protocol::HttpJson, None); let span_data = create_test_span_data(); let result = client.build_trace_export_body(vec![span_data]).unwrap(); @@ -1334,8 +1387,7 @@ mod tests { #[cfg(all(feature = "trace", feature = "gzip-http"))] #[test] fn test_build_trace_export_body_with_compression() { - let client = - create_test_client(crate::Protocol::HttpBinary, Some(crate::Compression::Gzip)); + let client = create_test_client(Protocol::HttpProtobuf, Some(crate::Compression::Gzip)); let span_data = create_test_span_data(); let result = client.build_trace_export_body(vec![span_data]).unwrap(); @@ -1356,7 +1408,7 @@ mod tests { #[cfg(feature = "logs")] #[test] fn test_build_logs_export_body_binary_protocol() { - let client = create_test_client(crate::Protocol::HttpBinary, None); + let client = create_test_client(Protocol::HttpProtobuf, None); let batch = create_test_log_batch(); let result = client.build_logs_export_body(batch).unwrap(); @@ -1369,7 +1421,7 @@ mod tests { #[cfg(all(feature = "logs", feature = "http-json"))] #[test] fn test_build_logs_export_body_json_protocol() { - let client = create_test_client(crate::Protocol::HttpJson, None); + let client = create_test_client(Protocol::HttpJson, None); let batch = create_test_log_batch(); let result = client.build_logs_export_body(batch).unwrap(); @@ -1382,8 +1434,7 @@ mod tests { #[cfg(all(feature = "logs", feature = "gzip-http"))] #[test] fn test_build_logs_export_body_with_compression() { - let client = - create_test_client(crate::Protocol::HttpBinary, Some(crate::Compression::Gzip)); + let client = create_test_client(Protocol::HttpProtobuf, Some(crate::Compression::Gzip)); let batch = create_test_log_batch(); let result = client.build_logs_export_body(batch).unwrap(); @@ -1398,7 +1449,7 @@ mod tests { fn test_build_metrics_export_body_binary_protocol() { use opentelemetry_sdk::metrics::data::ResourceMetrics; - let client = create_test_client(crate::Protocol::HttpBinary, None); + let client = create_test_client(Protocol::HttpProtobuf, None); let metrics = ResourceMetrics::default(); let result = client.build_metrics_export_body(&metrics).unwrap(); @@ -1413,7 +1464,7 @@ mod tests { fn test_build_metrics_export_body_json_protocol() { use opentelemetry_sdk::metrics::data::ResourceMetrics; - let client = create_test_client(crate::Protocol::HttpJson, None); + let client = create_test_client(Protocol::HttpJson, None); let metrics = ResourceMetrics::default(); let result = client.build_metrics_export_body(&metrics).unwrap(); @@ -1428,8 +1479,7 @@ mod tests { fn test_build_metrics_export_body_with_compression() { use opentelemetry_sdk::metrics::data::ResourceMetrics; - let client = - create_test_client(crate::Protocol::HttpBinary, Some(crate::Compression::Gzip)); + let client = create_test_client(Protocol::HttpProtobuf, Some(crate::Compression::Gzip)); let metrics = ResourceMetrics::default(); let result = client.build_metrics_export_body(&metrics).unwrap(); @@ -1444,8 +1494,7 @@ mod tests { fn test_build_metrics_export_body_compression_error_returns_none() { use opentelemetry_sdk::metrics::data::ResourceMetrics; - let client = - create_test_client(crate::Protocol::HttpBinary, Some(crate::Compression::Gzip)); + let client = create_test_client(Protocol::HttpProtobuf, Some(crate::Compression::Gzip)); let metrics = ResourceMetrics::default(); // Should return None when compression fails (feature not enabled) @@ -1530,7 +1579,7 @@ mod tests { #[cfg(feature = "experimental-http-retry")] #[test] fn test_default_retry_policy_when_none_configured() { - let client = create_test_client(crate::Protocol::HttpBinary, None); + let client = create_test_client(Protocol::HttpProtobuf, None); // Verify default values are used assert_eq!(client.retry_policy.max_retries, 3); @@ -1555,7 +1604,7 @@ mod tests { std::sync::Arc::new(MockHttpClient), "http://localhost:4318".parse().unwrap(), HashMap::new(), - crate::Protocol::HttpBinary, + Protocol::HttpProtobuf, std::time::Duration::from_secs(10), None, Some(custom_policy), diff --git a/opentelemetry-otlp/src/exporter/http/trace.rs b/opentelemetry-otlp/src/exporter/http/trace.rs index 17d4747ecc..45e836f16c 100644 --- a/opentelemetry-otlp/src/exporter/http/trace.rs +++ b/opentelemetry-otlp/src/exporter/http/trace.rs @@ -1,5 +1,4 @@ -use super::OtlpHttpClient; -use crate::Protocol; +use super::{OtlpHttpClient, Protocol}; use opentelemetry::{otel_debug, otel_warn}; use opentelemetry_sdk::{ error::{OTelSdkError, OTelSdkResult}, @@ -52,7 +51,15 @@ fn handle_partial_success(response_body: &[u8], protocol: Protocol) { return; } }, - _ => match Message::decode(response_body) { + Protocol::HttpProtobuf => match Message::decode(response_body) { + Ok(r) => r, + Err(e) => { + otel_debug!(name: "HttpTraceClient.ResponseParseError", error = e.to_string()); + return; + } + }, + #[cfg(not(feature = "http-json"))] + Protocol::HttpJson => match Message::decode(response_body) { Ok(r) => r, Err(e) => { otel_debug!(name: "HttpTraceClient.ResponseParseError", error = e.to_string()); @@ -82,7 +89,7 @@ mod tests { let invalid = vec![0xFF, 0xFF, 0xFF, 0xFF]; // Should not panic - logs debug and returns early - handle_partial_success(&invalid, Protocol::HttpBinary); + handle_partial_success(&invalid, Protocol::HttpProtobuf); } #[test] @@ -90,7 +97,7 @@ mod tests { let empty = vec![]; // Should not panic - handle_partial_success(&empty, Protocol::HttpBinary); + handle_partial_success(&empty, Protocol::HttpProtobuf); } #[cfg(feature = "http-json")] diff --git a/opentelemetry-otlp/src/exporter/mod.rs b/opentelemetry-otlp/src/exporter/mod.rs index 2ce25ba15d..dc40119ee7 100644 --- a/opentelemetry-otlp/src/exporter/mod.rs +++ b/opentelemetry-otlp/src/exporter/mod.rs @@ -6,7 +6,6 @@ use crate::exporter::http::HttpExporterBuilder; #[cfg(feature = "grpc-tonic")] use crate::exporter::tonic::TonicExporterBuilder; -use crate::Protocol; #[cfg(feature = "serialize")] use serde::{Deserialize, Serialize}; use std::fmt::{Display, Formatter}; @@ -31,25 +30,21 @@ pub const OTEL_EXPORTER_OTLP_COMPRESSION: &str = "OTEL_EXPORTER_OTLP_COMPRESSION #[cfg(feature = "http-json")] /// Default protocol, using http-json. -pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON; +pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = "http/json"; #[cfg(all(feature = "http-proto", not(feature = "http-json")))] /// Default protocol, using http-proto. -pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF; +pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = "http/protobuf"; #[cfg(all( feature = "grpc-tonic", not(any(feature = "http-proto", feature = "http-json")) ))] /// Default protocol, using grpc -pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = OTEL_EXPORTER_OTLP_PROTOCOL_GRPC; +pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = "grpc"; #[cfg(not(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json")))] /// Default protocol if no features are enabled. pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = ""; -const OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF: &str = "http/protobuf"; -const OTEL_EXPORTER_OTLP_PROTOCOL_GRPC: &str = "grpc"; -const OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON: &str = "http/json"; - /// Max waiting time for the backend to process each signal batch, defaults to 10 seconds. pub const OTEL_EXPORTER_OTLP_TIMEOUT: &str = "OTEL_EXPORTER_OTLP_TIMEOUT"; /// Default max waiting time for the backend to process each signal batch. @@ -66,17 +61,13 @@ pub(crate) mod http; pub(crate) mod tonic; /// Configuration for the OTLP exporter. -#[derive(Debug)] +#[derive(Debug, Default)] pub struct ExportConfig { /// The address of the OTLP collector. - /// Default address will be used based on the protocol. /// /// Note: Programmatically setting this will override any value set via the environment variable. pub endpoint: Option, - /// The protocol to use when communicating with the collector. - pub protocol: Protocol, - /// The timeout to the collector. /// The default value is 10 seconds. /// @@ -84,20 +75,6 @@ pub struct ExportConfig { pub timeout: Option, } -impl Default for ExportConfig { - fn default() -> Self { - let protocol = default_protocol(); - - Self { - endpoint: None, - // don't use default_endpoint(protocol) here otherwise we - // won't know if user provided a value - protocol, - timeout: None, - } - } -} - #[derive(Error, Debug)] /// Errors that can occur while building an exporter. // TODO: Refine and polish this. @@ -190,16 +167,6 @@ fn resolve_compression_from_env( } } -/// default protocol based on enabled features -fn default_protocol() -> Protocol { - match OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT { - OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF => Protocol::HttpBinary, - OTEL_EXPORTER_OTLP_PROTOCOL_GRPC => Protocol::Grpc, - OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON => Protocol::HttpJson, - _ => Protocol::HttpBinary, - } -} - /// default user-agent headers #[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))] fn default_headers() -> std::collections::HashMap { @@ -252,14 +219,6 @@ pub trait WithExportConfig { /// /// Note: Programmatically setting this will override any value set via the environment variable. fn with_endpoint>(self, endpoint: T) -> Self; - /// Set the protocol to use when communicating with the collector. - /// - /// Note that protocols that are not supported by exporters will be ignored. The exporter - /// will use default protocol in this case. - /// - /// ## Note - /// All exporters in this crate only support one protocol, thus choosing the protocol is a no-op at the moment. - fn with_protocol(self, protocol: Protocol) -> Self; /// Set the timeout to the collector. /// /// Note: Programmatically setting this will override any value set via the environment variable. @@ -276,11 +235,6 @@ impl WithExportConfig for B { self } - fn with_protocol(mut self, protocol: Protocol) -> Self { - self.export_config().protocol = protocol; - self - } - fn with_timeout(mut self, timeout: Duration) -> Self { self.export_config().timeout = Some(timeout); self @@ -288,7 +242,6 @@ impl WithExportConfig for B { fn with_export_config(mut self, exporter_config: ExportConfig) -> Self { self.export_config().endpoint = exporter_config.endpoint; - self.export_config().protocol = exporter_config.protocol; self.export_config().timeout = exporter_config.timeout; self } @@ -370,19 +323,54 @@ fn parse_header_key_value_string(key_value_string: &str) -> Option<(&str, String #[cfg(test)] #[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))] mod tests { + use super::*; + pub(crate) fn run_env_test(env_vars: T, f: F) where F: FnOnce(), T: Into>, { - temp_env::with_vars( - env_vars - .into() - .iter() - .map(|&(k, v)| (k, Some(v))) - .collect::)>>(), - f, - ) + // List of all OTEL environment variables that could interfere with tests + #[cfg_attr(not(feature = "logs"), allow(unused_mut))] + let mut otel_env_vars = vec![ + OTEL_EXPORTER_OTLP_ENDPOINT, + OTEL_EXPORTER_OTLP_PROTOCOL, + OTEL_EXPORTER_OTLP_HEADERS, + OTEL_EXPORTER_OTLP_COMPRESSION, + OTEL_EXPORTER_OTLP_TIMEOUT, + crate::OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, + crate::OTEL_EXPORTER_OTLP_TRACES_TIMEOUT, + crate::OTEL_EXPORTER_OTLP_TRACES_COMPRESSION, + crate::OTEL_EXPORTER_OTLP_TRACES_HEADERS, + crate::OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, + crate::OTEL_EXPORTER_OTLP_METRICS_TIMEOUT, + crate::OTEL_EXPORTER_OTLP_METRICS_COMPRESSION, + crate::OTEL_EXPORTER_OTLP_METRICS_HEADERS, + ]; + + #[cfg(feature = "logs")] + { + otel_env_vars.extend_from_slice(&[ + crate::OTEL_EXPORTER_OTLP_LOGS_ENDPOINT, + crate::OTEL_EXPORTER_OTLP_LOGS_TIMEOUT, + crate::OTEL_EXPORTER_OTLP_LOGS_COMPRESSION, + crate::OTEL_EXPORTER_OTLP_LOGS_HEADERS, + ]); + } + + let mut test_env_vars: Vec<(&str, Option<&str>)> = + env_vars.into().iter().map(|&(k, v)| (k, Some(v))).collect(); + + // Clear all OTEL env vars that aren't explicitly set in the test + let test_env_keys: std::collections::HashSet<&str> = + test_env_vars.iter().map(|(k, _)| *k).collect(); + for &var in &otel_env_vars { + if !test_env_keys.contains(var) { + test_env_vars.push((var, None)); + } + } + + temp_env::with_vars(test_env_vars, f) } #[cfg(any(feature = "http-proto", feature = "http-json"))] @@ -399,11 +387,10 @@ mod tests { fn export_builder_error_invalid_http_endpoint() { use std::time::Duration; - use crate::{ExportConfig, LogExporter, Protocol, WithExportConfig}; + use crate::{ExportConfig, LogExporter, WithExportConfig}; let ex_config = ExportConfig { endpoint: Some("invalid_uri/something".to_string()), - protocol: Protocol::HttpBinary, timeout: Some(Duration::from_secs(10)), }; @@ -426,11 +413,10 @@ mod tests { async fn export_builder_error_invalid_grpc_endpoint() { use std::time::Duration; - use crate::{ExportConfig, LogExporter, Protocol, WithExportConfig}; + use crate::{ExportConfig, LogExporter, WithExportConfig}; let ex_config = ExportConfig { endpoint: Some("invalid_uri/something".to_string()), - protocol: Protocol::Grpc, timeout: Some(Duration::from_secs(10)), }; @@ -453,39 +439,6 @@ mod tests { assert_eq!(exporter_builder.exporter_config.endpoint, None); } - #[test] - fn test_default_protocol() { - #[cfg(all( - feature = "http-json", - not(any(feature = "grpc-tonic", feature = "http-proto")) - ))] - { - assert_eq!( - crate::exporter::default_protocol(), - crate::Protocol::HttpJson - ); - } - - #[cfg(all( - feature = "http-proto", - not(any(feature = "grpc-tonic", feature = "http-json")) - ))] - { - assert_eq!( - crate::exporter::default_protocol(), - crate::Protocol::HttpBinary - ); - } - - #[cfg(all( - feature = "grpc-tonic", - not(any(feature = "http-proto", feature = "http-json")) - ))] - { - assert_eq!(crate::exporter::default_protocol(), crate::Protocol::Grpc); - } - } - #[test] fn test_url_decode() { let test_cases = vec![ diff --git a/opentelemetry-otlp/src/exporter/tonic/mod.rs b/opentelemetry-otlp/src/exporter/tonic/mod.rs index 15c098f60b..339fb93416 100644 --- a/opentelemetry-otlp/src/exporter/tonic/mod.rs +++ b/opentelemetry-otlp/src/exporter/tonic/mod.rs @@ -156,10 +156,7 @@ impl Default for TonicExporterBuilder { #[cfg(feature = "experimental-grpc-retry")] retry_policy: None, }, - exporter_config: ExportConfig { - protocol: crate::Protocol::Grpc, - ..Default::default() - }, + exporter_config: ExportConfig::default(), } } } diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs index 7bcc535939..098b31c72f 100644 --- a/opentelemetry-otlp/src/lib.rs +++ b/opentelemetry-otlp/src/lib.rs @@ -28,14 +28,13 @@ //! # { //! use opentelemetry::global; //! use opentelemetry::trace::Tracer; -//! use opentelemetry_otlp::Protocol; -//! use opentelemetry_otlp::WithExportConfig; +//! use opentelemetry_otlp::{Protocol, WithExportConfig, WithHttpConfig}; //! //! fn main() -> Result<(), Box> { -//! // Initialize OTLP exporter using HTTP binary protocol +//! // Initialize OTLP exporter with protobuf encoding and HTTP transport //! let otlp_exporter = opentelemetry_otlp::SpanExporter::builder() //! .with_http() -//! .with_protocol(Protocol::HttpBinary) +//! .with_protocol(Protocol::HttpProtobuf) //! .build()?; //! //! // Create a tracer provider with the exporter @@ -167,14 +166,13 @@ //! use opentelemetry::global; //! use opentelemetry::metrics::Meter; //! use opentelemetry::KeyValue; -//! use opentelemetry_otlp::Protocol; -//! use opentelemetry_otlp::WithExportConfig; +//! use opentelemetry_otlp::{Protocol, WithExportConfig, WithHttpConfig}; //! //! fn main() -> Result<(), Box> { -//! // Initialize OTLP exporter using HTTP binary protocol +//! // Initialize OTLP exporter with protobuf encoding and HTTP transport //! let exporter = opentelemetry_otlp::MetricExporter::builder() //! .with_http() -//! .with_protocol(Protocol::HttpBinary) +//! .with_protocol(Protocol::HttpProtobuf) //! .with_endpoint("http://localhost:9090/api/v1/otlp/v1/metrics") //! .build()?; //! @@ -249,11 +247,11 @@ //! use opentelemetry_sdk::{trace::{self, RandomIdGenerator, Sampler}, Resource}; //! # #[cfg(feature = "metrics")] //! use opentelemetry_sdk::metrics::Temporality; -//! use opentelemetry_otlp::{Protocol, WithExportConfig, Compression}; +//! use opentelemetry_otlp::{WithExportConfig, Compression}; //! # #[cfg(feature = "grpc-tonic")] //! use opentelemetry_otlp::WithTonicConfig; //! # #[cfg(any(feature = "http-proto", feature = "http-json"))] -//! use opentelemetry_otlp::WithHttpConfig; +//! use opentelemetry_otlp::{Protocol, WithHttpConfig}; //! use std::time::Duration; //! # #[cfg(feature = "grpc-tonic")] //! use tonic::metadata::*; @@ -293,7 +291,7 @@ //! .with_http() //! .with_endpoint("http://localhost:4318/v1/traces") //! .with_timeout(Duration::from_secs(3)) -//! .with_protocol(Protocol::HttpBinary) +//! .with_protocol(Protocol::HttpProtobuf) //! .with_compression(Compression::Gzip) // Requires gzip-http feature //! .build()?; //! # exporter @@ -304,7 +302,6 @@ //! let exporter = opentelemetry_otlp::MetricExporter::builder() //! .with_tonic() //! .with_endpoint("http://localhost:4318/v1/metrics") -//! .with_protocol(Protocol::Grpc) //! .with_timeout(Duration::from_secs(3)) //! .build() //! .unwrap(); @@ -321,7 +318,7 @@ //! let exporter = opentelemetry_otlp::MetricExporter::builder() //! .with_http() //! .with_endpoint("http://localhost:4318/v1/metrics") -//! .with_protocol(Protocol::HttpBinary) +//! .with_protocol(Protocol::HttpProtobuf) //! .with_timeout(Duration::from_secs(3)) //! .with_compression(Compression::Zstd) // Requires zstd-http feature //! .build() @@ -401,7 +398,7 @@ pub use crate::logs::{ }; #[cfg(any(feature = "http-proto", feature = "http-json"))] -pub use crate::exporter::http::{HasHttpConfig, WithHttpConfig}; +pub use crate::exporter::http::{HasHttpConfig, Protocol, WithHttpConfig}; #[cfg(feature = "grpc-tonic")] pub use crate::exporter::tonic::{HasTonicConfig, WithTonicConfig}; @@ -442,21 +439,6 @@ pub use crate::exporter::http::HttpExporterBuilder; #[cfg(feature = "grpc-tonic")] pub use crate::exporter::tonic::{TonicConfig, TonicExporterBuilder}; -#[cfg(feature = "serialize")] -use serde::{Deserialize, Serialize}; - -/// The communication protocol to use when exporting data. -#[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))] -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum Protocol { - /// GRPC protocol - Grpc, - /// HTTP protocol with binary protobuf - HttpBinary, - /// HTTP protocol with JSON payload - HttpJson, -} - #[derive(Debug, Default)] #[doc(hidden)] /// Placeholder type when no exporter pipeline has been configured in telemetry pipeline.