Skip to content

Commit 95af815

Browse files
scottgerringscottgerring
authored
chore(otlp): prevent invalid combinations of exporter and protocol (#3230)
1 parent 5c8dbc8 commit 95af815

File tree

6 files changed

+200
-45
lines changed

6 files changed

+200
-45
lines changed

‎opentelemetry-otlp/src/exporter/http/logs.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::Protocol;
33
use opentelemetry::{otel_debug, otel_warn};
44
use opentelemetry_sdk::error::{OTelSdkError, OTelSdkResult};
55
use opentelemetry_sdk::logs::{LogBatch, LogExporter};
6+
#[cfg(feature = "http-proto")]
67
use prost::Message;
78
use std::time;
89

@@ -51,13 +52,18 @@ fn handle_partial_success(response_body: &[u8], protocol: Protocol) {
5152
return;
5253
}
5354
},
54-
_ => match Message::decode(response_body) {
55+
#[cfg(feature = "http-proto")]
56+
Protocol::HttpBinary => match Message::decode(response_body) {
5557
Ok(r) => r,
5658
Err(e) => {
5759
otel_debug!(name: "HttpLogsClient.ResponseParseError", error = e.to_string());
5860
return;
5961
}
6062
},
63+
#[cfg(feature = "grpc-tonic")]
64+
Protocol::Grpc => {
65+
unreachable!("HTTP client should not receive Grpc protocol")
66+
}
6167
};
6268

6369
if let Some(partial_success) = response.partial_success {

‎opentelemetry-otlp/src/exporter/http/metrics.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::Protocol;
33
use opentelemetry::{otel_debug, otel_warn};
44
use opentelemetry_sdk::error::{OTelSdkError, OTelSdkResult};
55
use opentelemetry_sdk::metrics::data::ResourceMetrics;
6+
#[cfg(feature = "http-proto")]
67
use prost::Message;
78

89
use super::OtlpHttpClient;
@@ -47,13 +48,18 @@ fn handle_partial_success(response_body: &[u8], protocol: Protocol) {
4748
return;
4849
}
4950
},
50-
_ => match Message::decode(response_body) {
51+
#[cfg(feature = "http-proto")]
52+
Protocol::HttpBinary => match Message::decode(response_body) {
5153
Ok(r) => r,
5254
Err(e) => {
5355
otel_debug!(name: "HttpMetricsClient.ResponseParseError", error = e.to_string());
5456
return;
5557
}
5658
},
59+
#[cfg(feature = "grpc-tonic")]
60+
Protocol::Grpc => {
61+
unreachable!("HTTP client should not receive Grpc protocol")
62+
}
5763
};
5864

5965
if let Some(partial_success) = response.partial_success {

‎opentelemetry-otlp/src/exporter/http/mod.rs‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use super::{
2-
default_headers, default_protocol, parse_header_string, resolve_timeout, ExporterBuildError,
2+
default_headers, parse_header_string, resolve_timeout, ExporterBuildError,
33
OTEL_EXPORTER_OTLP_HTTP_ENDPOINT_DEFAULT,
44
};
55
use crate::{ExportConfig, Protocol, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS};
@@ -15,6 +15,7 @@ use opentelemetry_proto::transform::trace::tonic::group_spans_by_resource_and_sc
1515
use opentelemetry_sdk::logs::LogBatch;
1616
#[cfg(feature = "trace")]
1717
use opentelemetry_sdk::trace::SpanData;
18+
#[cfg(feature = "http-proto")]
1819
use prost::Message;
1920
use std::collections::HashMap;
2021
use std::env;
@@ -154,7 +155,7 @@ impl Default for HttpExporterBuilder {
154155
fn default() -> Self {
155156
HttpExporterBuilder {
156157
exporter_config: ExportConfig {
157-
protocol: default_protocol(),
158+
protocol: Protocol::default(),
158159
..ExportConfig::default()
159160
},
160161
http_config: HttpConfig {
@@ -595,7 +596,12 @@ impl OtlpHttpClient {
595596
Ok(json) => (json.into_bytes(), "application/json"),
596597
Err(e) => return Err(e.to_string()),
597598
},
598-
_ => (req.encode_to_vec(), "application/x-protobuf"),
599+
#[cfg(feature = "http-proto")]
600+
Protocol::HttpBinary => (req.encode_to_vec(), "application/x-protobuf"),
601+
#[cfg(feature = "grpc-tonic")]
602+
Protocol::Grpc => {
603+
unreachable!("HTTP client should not receive Grpc protocol")
604+
}
599605
};
600606

601607
let (processed_body, content_encoding) = self.process_body(body)?;
@@ -617,7 +623,12 @@ impl OtlpHttpClient {
617623
Ok(json) => (json.into_bytes(), "application/json"),
618624
Err(e) => return Err(e.to_string()),
619625
},
620-
_ => (req.encode_to_vec(), "application/x-protobuf"),
626+
#[cfg(feature = "http-proto")]
627+
Protocol::HttpBinary => (req.encode_to_vec(), "application/x-protobuf"),
628+
#[cfg(feature = "grpc-tonic")]
629+
Protocol::Grpc => {
630+
unreachable!("HTTP client should not receive Grpc protocol")
631+
}
621632
};
622633

623634
let (processed_body, content_encoding) = self.process_body(body)?;
@@ -642,7 +653,12 @@ impl OtlpHttpClient {
642653
return None;
643654
}
644655
},
645-
_ => (req.encode_to_vec(), "application/x-protobuf"),
656+
#[cfg(feature = "http-proto")]
657+
Protocol::HttpBinary => (req.encode_to_vec(), "application/x-protobuf"),
658+
#[cfg(feature = "grpc-tonic")]
659+
Protocol::Grpc => {
660+
unreachable!("HTTP client should not receive Grpc protocol")
661+
}
646662
};
647663

648664
match self.process_body(body) {

‎opentelemetry-otlp/src/exporter/http/trace.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use opentelemetry_sdk::{
55
error::{OTelSdkError, OTelSdkResult},
66
trace::{SpanData, SpanExporter},
77
};
8+
#[cfg(feature = "http-proto")]
89
use prost::Message;
910

1011
impl SpanExporter for OtlpHttpClient {
@@ -52,13 +53,18 @@ fn handle_partial_success(response_body: &[u8], protocol: Protocol) {
5253
return;
5354
}
5455
},
55-
_ => match Message::decode(response_body) {
56+
#[cfg(feature = "http-proto")]
57+
Protocol::HttpBinary => match Message::decode(response_body) {
5658
Ok(r) => r,
5759
Err(e) => {
5860
otel_debug!(name: "HttpTraceClient.ResponseParseError", error = e.to_string());
5961
return;
6062
}
6163
},
64+
#[cfg(feature = "grpc-tonic")]
65+
Protocol::Grpc => {
66+
unreachable!("HTTP client should not receive Grpc protocol")
67+
}
6268
};
6369

6470
if let Some(partial_success) = response.partial_success {

‎opentelemetry-otlp/src/exporter/mod.rs‎

Lines changed: 89 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -29,26 +29,12 @@ pub const OTEL_EXPORTER_OTLP_PROTOCOL: &str = "OTEL_EXPORTER_OTLP_PROTOCOL";
2929
/// Compression algorithm to use, defaults to none.
3030
pub const OTEL_EXPORTER_OTLP_COMPRESSION: &str = "OTEL_EXPORTER_OTLP_COMPRESSION";
3131

32-
#[cfg(feature = "http-json")]
33-
/// Default protocol, using http-json.
34-
pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON;
35-
#[cfg(all(feature = "http-proto", not(feature = "http-json")))]
36-
/// Default protocol, using http-proto.
37-
pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF;
38-
#[cfg(all(
39-
feature = "grpc-tonic",
40-
not(any(feature = "http-proto", feature = "http-json"))
41-
))]
42-
/// Default protocol, using grpc
43-
pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = OTEL_EXPORTER_OTLP_PROTOCOL_GRPC;
44-
45-
#[cfg(not(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json")))]
46-
/// Default protocol if no features are enabled.
47-
pub const OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT: &str = "";
48-
49-
const OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF: &str = "http/protobuf";
50-
const OTEL_EXPORTER_OTLP_PROTOCOL_GRPC: &str = "grpc";
51-
const OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON: &str = "http/json";
32+
/// Protocol value for HTTP with protobuf encoding
33+
pub const OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF: &str = "http/protobuf";
34+
/// Protocol value for gRPC
35+
pub const OTEL_EXPORTER_OTLP_PROTOCOL_GRPC: &str = "grpc";
36+
/// Protocol value for HTTP with JSON encoding
37+
pub const OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON: &str = "http/json";
5238

5339
/// Max waiting time for the backend to process each signal batch, defaults to 10 seconds.
5440
pub const OTEL_EXPORTER_OTLP_TIMEOUT: &str = "OTEL_EXPORTER_OTLP_TIMEOUT";
@@ -84,9 +70,10 @@ pub struct ExportConfig {
8470
pub timeout: Option<Duration>,
8571
}
8672

73+
#[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))]
8774
impl Default for ExportConfig {
8875
fn default() -> Self {
89-
let protocol = default_protocol();
76+
let protocol = Protocol::default();
9077

9178
Self {
9279
endpoint: None,
@@ -190,13 +177,33 @@ fn resolve_compression_from_env(
190177
}
191178
}
192179

193-
/// default protocol based on enabled features
194-
fn default_protocol() -> Protocol {
195-
match OTEL_EXPORTER_OTLP_PROTOCOL_DEFAULT {
196-
OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_PROTOBUF => Protocol::HttpBinary,
197-
OTEL_EXPORTER_OTLP_PROTOCOL_GRPC => Protocol::Grpc,
198-
OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON => Protocol::HttpJson,
199-
_ => Protocol::HttpBinary,
180+
/// Returns the default protocol based on environment variable or enabled features.
181+
///
182+
/// Priority order (first available wins):
183+
/// 1. OTEL_EXPORTER_OTLP_PROTOCOL environment variable (if set and feature is enabled)
184+
/// 2. http-json (if enabled)
185+
/// 3. http-proto (if enabled)
186+
/// 4. grpc-tonic (if enabled)
187+
#[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))]
188+
impl Default for Protocol {
189+
fn default() -> Self {
190+
// Check environment variable first
191+
if let Some(protocol) = Protocol::from_env() {
192+
return protocol;
193+
}
194+
195+
// Fall back to feature-based defaults
196+
#[cfg(feature = "http-json")]
197+
return Protocol::HttpJson;
198+
199+
#[cfg(all(feature = "http-proto", not(feature = "http-json")))]
200+
return Protocol::HttpBinary;
201+
202+
#[cfg(all(
203+
feature = "grpc-tonic",
204+
not(any(feature = "http-proto", feature = "http-json"))
205+
))]
206+
return Protocol::Grpc;
200207
}
201208
}
202209

@@ -460,21 +467,15 @@ mod tests {
460467
not(any(feature = "grpc-tonic", feature = "http-proto"))
461468
))]
462469
{
463-
assert_eq!(
464-
crate::exporter::default_protocol(),
465-
crate::Protocol::HttpJson
466-
);
470+
assert_eq!(crate::Protocol::default(), crate::Protocol::HttpJson);
467471
}
468472

469473
#[cfg(all(
470474
feature = "http-proto",
471475
not(any(feature = "grpc-tonic", feature = "http-json"))
472476
))]
473477
{
474-
assert_eq!(
475-
crate::exporter::default_protocol(),
476-
crate::Protocol::HttpBinary
477-
);
478+
assert_eq!(crate::Protocol::default(), crate::Protocol::HttpBinary);
478479
}
479480

480481
#[cfg(all(
@@ -486,6 +487,58 @@ mod tests {
486487
}
487488
}
488489

490+
#[test]
491+
fn test_protocol_from_env() {
492+
use crate::{Protocol, OTEL_EXPORTER_OTLP_PROTOCOL};
493+
494+
// Test with no env var set - should return None
495+
temp_env::with_var_unset(OTEL_EXPORTER_OTLP_PROTOCOL, || {
496+
assert_eq!(Protocol::from_env(), None);
497+
});
498+
499+
// Test with grpc protocol
500+
#[cfg(feature = "grpc-tonic")]
501+
run_env_test(vec![(OTEL_EXPORTER_OTLP_PROTOCOL, "grpc")], || {
502+
assert_eq!(Protocol::from_env(), Some(Protocol::Grpc));
503+
});
504+
505+
// Test with http/protobuf protocol
506+
#[cfg(feature = "http-proto")]
507+
run_env_test(vec![(OTEL_EXPORTER_OTLP_PROTOCOL, "http/protobuf")], || {
508+
assert_eq!(Protocol::from_env(), Some(Protocol::HttpBinary));
509+
});
510+
511+
// Test with http/json protocol
512+
#[cfg(feature = "http-json")]
513+
run_env_test(vec![(OTEL_EXPORTER_OTLP_PROTOCOL, "http/json")], || {
514+
assert_eq!(Protocol::from_env(), Some(Protocol::HttpJson));
515+
});
516+
517+
// Test with invalid protocol - should return None
518+
run_env_test(vec![(OTEL_EXPORTER_OTLP_PROTOCOL, "invalid")], || {
519+
assert_eq!(Protocol::from_env(), None);
520+
});
521+
}
522+
523+
#[test]
524+
fn test_default_protocol_respects_env() {
525+
// Test that env var takes precedence over feature-based defaults
526+
#[cfg(all(feature = "http-json", feature = "http-proto"))]
527+
run_env_test(
528+
vec![(crate::OTEL_EXPORTER_OTLP_PROTOCOL, "http/protobuf")],
529+
|| {
530+
// Even though http-json would be the default, env var should override
531+
assert_eq!(crate::Protocol::default(), crate::Protocol::HttpBinary);
532+
},
533+
);
534+
535+
#[cfg(all(feature = "grpc-tonic", feature = "http-json"))]
536+
run_env_test(vec![(crate::OTEL_EXPORTER_OTLP_PROTOCOL, "grpc")], || {
537+
// Even though http-json would be the default, env var should override
538+
assert_eq!(crate::Protocol::default(), crate::Protocol::Grpc);
539+
});
540+
}
541+
489542
#[test]
490543
fn test_url_decode() {
491544
let test_cases = vec![

0 commit comments

Comments
 (0)