Skip to content

Commit 9716916

Browse files
scottgerringscottgerring
committed
address review comments
1 parent 52938a4 commit 9716916

File tree

6 files changed

+115
-88
lines changed

6 files changed

+115
-88
lines changed

‎opentelemetry-otlp/CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## vNext
44

5+
- Add HTTP compression support with `gzip-http` and `zstd-http` feature flags
6+
57
## 0.30.0
68

79
Released 2025-May-23

‎opentelemetry-otlp/examples/basic-otlp-http/Cargo.toml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ bench = false
1515
[features]
1616
default = ["reqwest-blocking"]
1717
reqwest-blocking = ["opentelemetry-otlp/reqwest-blocking-client"]
18-
gzip = ["opentelemetry-otlp/gzip-http"]
19-
zstd = ["opentelemetry-otlp/zstd-http"]
2018

2119
[dependencies]
2220
opentelemetry = { path = "../../../opentelemetry" }

‎opentelemetry-otlp/examples/basic-otlp-http/src/main.rs‎

Lines changed: 6 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ use opentelemetry::{
55
};
66
use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
77
use opentelemetry_otlp::WithExportConfig;
8-
#[cfg(any(feature = "gzip", feature = "zstd"))]
9-
use opentelemetry_otlp::{Compression, WithHttpConfig};
108
use opentelemetry_otlp::{LogExporter, MetricExporter, Protocol, SpanExporter};
119
use opentelemetry_sdk::Resource;
1210
use opentelemetry_sdk::{
@@ -29,29 +27,9 @@ fn get_resource() -> Resource {
2927
}
3028

3129
fn init_logs() -> SdkLoggerProvider {
32-
#[cfg(any(feature = "gzip", feature = "zstd"))]
33-
let mut exporter_builder = LogExporter::builder()
30+
let exporter = LogExporter::builder()
3431
.with_http()
35-
.with_protocol(Protocol::HttpBinary);
36-
37-
#[cfg(not(any(feature = "gzip", feature = "zstd")))]
38-
let exporter_builder = LogExporter::builder()
39-
.with_http()
40-
.with_protocol(Protocol::HttpBinary);
41-
42-
#[cfg(feature = "gzip")]
43-
{
44-
exporter_builder = exporter_builder.with_compression(Compression::Gzip);
45-
println!("Using gzip compression for logs");
46-
}
47-
48-
#[cfg(all(feature = "zstd", not(feature = "gzip")))]
49-
{
50-
exporter_builder = exporter_builder.with_compression(Compression::Zstd);
51-
println!("Using zstd compression for logs");
52-
}
53-
54-
let exporter = exporter_builder
32+
.with_protocol(Protocol::HttpBinary)
5533
.build()
5634
.expect("Failed to create log exporter");
5735

@@ -62,29 +40,9 @@ fn init_logs() -> SdkLoggerProvider {
6240
}
6341

6442
fn init_traces() -> SdkTracerProvider {
65-
#[cfg(any(feature = "gzip", feature = "zstd"))]
66-
let mut exporter_builder = SpanExporter::builder()
67-
.with_http()
68-
.with_protocol(Protocol::HttpBinary); //can be changed to `Protocol::HttpJson` to export in JSON format
69-
70-
#[cfg(not(any(feature = "gzip", feature = "zstd")))]
71-
let exporter_builder = SpanExporter::builder()
43+
let exporter = SpanExporter::builder()
7244
.with_http()
73-
.with_protocol(Protocol::HttpBinary); //can be changed to `Protocol::HttpJson` to export in JSON format
74-
75-
#[cfg(feature = "gzip")]
76-
{
77-
exporter_builder = exporter_builder.with_compression(Compression::Gzip);
78-
println!("Using gzip compression for traces");
79-
}
80-
81-
#[cfg(all(feature = "zstd", not(feature = "gzip")))]
82-
{
83-
exporter_builder = exporter_builder.with_compression(Compression::Zstd);
84-
println!("Using zstd compression for traces");
85-
}
86-
87-
let exporter = exporter_builder
45+
.with_protocol(Protocol::HttpBinary) //can be changed to `Protocol::HttpJson` to export in JSON format
8846
.build()
8947
.expect("Failed to create trace exporter");
9048

@@ -95,29 +53,9 @@ fn init_traces() -> SdkTracerProvider {
9553
}
9654

9755
fn init_metrics() -> SdkMeterProvider {
98-
#[cfg(any(feature = "gzip", feature = "zstd"))]
99-
let mut exporter_builder = MetricExporter::builder()
100-
.with_http()
101-
.with_protocol(Protocol::HttpBinary); //can be changed to `Protocol::HttpJson` to export in JSON format
102-
103-
#[cfg(not(any(feature = "gzip", feature = "zstd")))]
104-
let exporter_builder = MetricExporter::builder()
56+
let exporter = MetricExporter::builder()
10557
.with_http()
106-
.with_protocol(Protocol::HttpBinary); //can be changed to `Protocol::HttpJson` to export in JSON format
107-
108-
#[cfg(feature = "gzip")]
109-
{
110-
exporter_builder = exporter_builder.with_compression(Compression::Gzip);
111-
println!("Using gzip compression for metrics");
112-
}
113-
114-
#[cfg(all(feature = "zstd", not(feature = "gzip")))]
115-
{
116-
exporter_builder = exporter_builder.with_compression(Compression::Zstd);
117-
println!("Using zstd compression for metrics");
118-
}
119-
120-
let exporter = exporter_builder
58+
.with_protocol(Protocol::HttpBinary) //can be changed to `Protocol::HttpJson` to export in JSON format
12159
.build()
12260
.expect("Failed to create metric exporter");
12361

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use super::OtlpHttpClient;
2+
use http::header::CONTENT_ENCODING;
23
use http::{header::CONTENT_TYPE, Method};
34
use opentelemetry::otel_debug;
45
use opentelemetry_sdk::error::{OTelSdkError, OTelSdkResult};
@@ -24,7 +25,7 @@ impl LogExporter for OtlpHttpClient {
2425
.header(CONTENT_TYPE, content_type);
2526

2627
if let Some(encoding) = content_encoding {
27-
request_builder = request_builder.header("Content-Encoding", encoding);
28+
request_builder = request_builder.header(CONTENT_ENCODING, encoding);
2829
}
2930

3031
let mut request = request_builder

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

Lines changed: 74 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ pub struct HttpConfig {
4747
/// Select the HTTP client
4848
client: Option<Arc<dyn HttpClient>>,
4949

50-
/// Additional headers to send to the collector.
50+
/// Additional headers to send to the OTLP endpoint.
5151
headers: Option<HashMap<String, String>>,
5252

53-
/// The compression algorithm to use when communicating with the collector.
53+
/// The compression algorithm to use when communicating with the OTLP endpoint.
5454
compression: Option<crate::Compression>,
5555
}
5656

@@ -119,6 +119,30 @@ impl HttpExporterBuilder {
119119

120120
let compression = self.resolve_compression(signal_compression_var)?;
121121

122+
// Validate compression is supported at build time
123+
if let Some(compression_alg) = &compression {
124+
match compression_alg {
125+
crate::Compression::Gzip => {
126+
#[cfg(not(feature = "gzip-http"))]
127+
{
128+
return Err(ExporterBuildError::UnsupportedCompressionAlgorithm(
129+
"gzip compression requested but gzip-http feature not enabled"
130+
.to_string(),
131+
));
132+
}
133+
}
134+
crate::Compression::Zstd => {
135+
#[cfg(not(feature = "zstd-http"))]
136+
{
137+
return Err(ExporterBuildError::UnsupportedCompressionAlgorithm(
138+
"zstd compression requested but zstd-http feature not enabled"
139+
.to_string(),
140+
));
141+
}
142+
}
143+
}
144+
}
145+
122146
let timeout = resolve_timeout(signal_timeout_var, self.exporter_config.timeout.as_ref());
123147

124148
#[allow(unused_mut)] // TODO - clippy thinks mut is not needed, but it is
@@ -284,8 +308,10 @@ pub(crate) struct OtlpHttpClient {
284308
}
285309

286310
impl OtlpHttpClient {
287-
/// Compress data using gzip if compression is enabled and the feature is available
288-
fn compress_body(&self, body: Vec<u8>) -> Result<(Vec<u8>, Option<&'static str>), String> {
311+
/// Compress data using gzip or zstd if the user has requested it and the relevant feature
312+
/// has been enabled. If the user has requested it but the feature has not been enabled,
313+
/// we should catch this at exporter build time and never get here.
314+
fn process_body(&self, body: Vec<u8>) -> Result<(Vec<u8>, Option<&'static str>), String> {
289315
match self.compression {
290316
#[cfg(feature = "gzip-http")]
291317
Some(crate::Compression::Gzip) => {
@@ -352,8 +378,8 @@ impl OtlpHttpClient {
352378
_ => (req.encode_to_vec(), "application/x-protobuf"),
353379
};
354380

355-
let (compressed_body, content_encoding) = self.compress_body(body)?;
356-
Ok((compressed_body, content_type, content_encoding))
381+
let (processed_body, content_encoding) = self.process_body(body)?;
382+
Ok((processed_body, content_type, content_encoding))
357383
}
358384

359385
#[cfg(feature = "logs")]
@@ -374,8 +400,8 @@ impl OtlpHttpClient {
374400
_ => (req.encode_to_vec(), "application/x-protobuf"),
375401
};
376402

377-
let (compressed_body, content_encoding) = self.compress_body(body)?;
378-
Ok((compressed_body, content_type, content_encoding))
403+
let (processed_body, content_encoding) = self.process_body(body)?;
404+
Ok((processed_body, content_type, content_encoding))
379405
}
380406

381407
#[cfg(feature = "metrics")]
@@ -399,9 +425,9 @@ impl OtlpHttpClient {
399425
_ => (req.encode_to_vec(), "application/x-protobuf"),
400426
};
401427

402-
match self.compress_body(body) {
403-
Ok((compressed_body, content_encoding)) => {
404-
Some((compressed_body, content_type, content_encoding))
428+
match self.process_body(body) {
429+
Ok((processed_body, content_encoding)) => {
430+
Some((processed_body, content_type, content_encoding))
405431
}
406432
Err(e) => {
407433
otel_debug!(name: "CompressionFailed", error = e);
@@ -839,7 +865,7 @@ mod tests {
839865

840866
// Test with some sample data
841867
let test_data = b"Hello, world! This is test data for compression.";
842-
let result = client.compress_body(test_data.to_vec()).unwrap();
868+
let result = client.process_body(test_data.to_vec()).unwrap();
843869
let (compressed_body, content_encoding) = result;
844870

845871
// Verify encoding header is set
@@ -870,7 +896,7 @@ mod tests {
870896

871897
// Test with some sample data
872898
let test_data = b"Hello, world! This is test data for zstd compression.";
873-
let result = client.compress_body(test_data.to_vec()).unwrap();
899+
let result = client.process_body(test_data.to_vec()).unwrap();
874900
let (compressed_body, content_encoding) = result;
875901

876902
// Verify encoding header is set
@@ -897,7 +923,7 @@ mod tests {
897923
);
898924

899925
let body = vec![1, 2, 3, 4];
900-
let result = client.compress_body(body.clone()).unwrap();
926+
let result = client.process_body(body.clone()).unwrap();
901927
let (result_body, content_encoding) = result;
902928

903929
// Body should be unchanged and no encoding header
@@ -918,7 +944,7 @@ mod tests {
918944
);
919945

920946
let body = vec![1, 2, 3, 4];
921-
let result = client.compress_body(body);
947+
let result = client.process_body(body);
922948

923949
// Should return error when gzip requested but feature not enabled
924950
assert!(result.is_err());
@@ -940,7 +966,7 @@ mod tests {
940966
);
941967

942968
let body = vec![1, 2, 3, 4];
943-
let result = client.compress_body(body);
969+
let result = client.process_body(body);
944970

945971
// Should return error when zstd requested but feature not enabled
946972
assert!(result.is_err());
@@ -1199,5 +1225,37 @@ mod tests {
11991225
},
12001226
);
12011227
}
1228+
1229+
#[cfg(all(feature = "trace", not(feature = "gzip-http")))]
1230+
#[test]
1231+
fn test_build_span_exporter_with_gzip_without_feature() {
1232+
use super::super::HttpExporterBuilder;
1233+
use crate::{ExporterBuildError, WithHttpConfig};
1234+
1235+
let builder = HttpExporterBuilder::default().with_compression(crate::Compression::Gzip);
1236+
1237+
let result = builder.build_span_exporter();
1238+
// This test will fail until the issue is fixed: compression validation should happen at build time
1239+
assert!(matches!(
1240+
result,
1241+
Err(ExporterBuildError::UnsupportedCompressionAlgorithm(_))
1242+
));
1243+
}
1244+
1245+
#[cfg(all(feature = "trace", not(feature = "zstd-http")))]
1246+
#[test]
1247+
fn test_build_span_exporter_with_zstd_without_feature() {
1248+
use super::super::HttpExporterBuilder;
1249+
use crate::{ExporterBuildError, WithHttpConfig};
1250+
1251+
let builder = HttpExporterBuilder::default().with_compression(crate::Compression::Zstd);
1252+
1253+
let result = builder.build_span_exporter();
1254+
// This test will fail until the issue is fixed: compression validation should happen at build time
1255+
assert!(matches!(
1256+
result,
1257+
Err(ExporterBuildError::UnsupportedCompressionAlgorithm(_))
1258+
));
1259+
}
12021260
}
12031261
}

‎opentelemetry-otlp/src/lib.rs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,8 @@
261261
//! The following feature flags offer additional configurations on http:
262262
//!
263263
//! * `http-proto`: Use http as transport layer, protobuf as body format. This feature is enabled by default.
264+
//! * `gzip-http`: Use gzip compression for HTTP transport.
265+
//! * `zstd-http`: Use zstd compression for HTTP transport.
264266
//! * `reqwest-blocking-client`: Use reqwest blocking http client. This feature is enabled by default.
265267
//! * `reqwest-client`: Use reqwest http client.
266268
//! * `reqwest-rustls`: Use reqwest with TLS with system trust roots via `rustls-native-certs` crate.
@@ -281,9 +283,11 @@
281283
//! use opentelemetry_sdk::{trace::{self, RandomIdGenerator, Sampler}, Resource};
282284
//! # #[cfg(feature = "metrics")]
283285
//! use opentelemetry_sdk::metrics::Temporality;
284-
//! use opentelemetry_otlp::{Protocol, WithExportConfig};
286+
//! use opentelemetry_otlp::{Protocol, WithExportConfig, Compression};
285287
//! # #[cfg(feature = "grpc-tonic")]
286288
//! use opentelemetry_otlp::WithTonicConfig;
289+
//! # #[cfg(any(feature = "http-proto", feature = "http-json"))]
290+
//! use opentelemetry_otlp::WithHttpConfig;
287291
//! use std::time::Duration;
288292
//! # #[cfg(feature = "grpc-tonic")]
289293
//! use tonic::metadata::*;
@@ -316,6 +320,19 @@
316320
//! # tracer
317321
//! # };
318322
//!
323+
//! // HTTP exporter example with compression
324+
//! # #[cfg(all(feature = "trace", feature = "http-proto"))]
325+
//! # let _http_tracer = {
326+
//! let exporter = opentelemetry_otlp::SpanExporter::builder()
327+
//! .with_http()
328+
//! .with_endpoint("http://localhost:4318/v1/traces")
329+
//! .with_timeout(Duration::from_secs(3))
330+
//! .with_protocol(Protocol::HttpBinary)
331+
//! .with_compression(Compression::Gzip) // Requires gzip-http feature
332+
//! .build()?;
333+
//! # exporter
334+
//! # };
335+
//!
319336
//! # #[cfg(all(feature = "metrics", feature = "grpc-tonic"))]
320337
//! # {
321338
//! let exporter = opentelemetry_otlp::MetricExporter::builder()
@@ -332,6 +349,19 @@
332349
//! .build();
333350
//! # }
334351
//!
352+
//! // HTTP metrics exporter example with compression
353+
//! # #[cfg(all(feature = "metrics", feature = "http-proto"))]
354+
//! # {
355+
//! let exporter = opentelemetry_otlp::MetricExporter::builder()
356+
//! .with_http()
357+
//! .with_endpoint("http://localhost:4318/v1/metrics")
358+
//! .with_protocol(Protocol::HttpBinary)
359+
//! .with_timeout(Duration::from_secs(3))
360+
//! .with_compression(Compression::Zstd) // Requires zstd-http feature
361+
//! .build()
362+
//! .unwrap();
363+
//! # }
364+
//!
335365
//! # #[cfg(all(feature = "trace", feature = "grpc-tonic"))]
336366
//! # {
337367
//! tracer.in_span("doing_work", |cx| {

0 commit comments

Comments
 (0)