From c88c757b58fad412741b343a04e35418f9f2ad1b Mon Sep 17 00:00:00 2001 From: harscoet Date: Thu, 18 Jul 2024 23:37:10 +0200 Subject: [PATCH 1/2] zstd compression for tonic exporter --- opentelemetry-otlp/Cargo.toml | 1 + opentelemetry-otlp/src/exporter/mod.rs | 4 +++ opentelemetry-otlp/src/exporter/tonic/mod.rs | 36 +++++++++++++++++--- opentelemetry-otlp/src/lib.rs | 6 ++++ 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/opentelemetry-otlp/Cargo.toml b/opentelemetry-otlp/Cargo.toml index c7fe540089..0eac9430d1 100644 --- a/opentelemetry-otlp/Cargo.toml +++ b/opentelemetry-otlp/Cargo.toml @@ -65,6 +65,7 @@ default = ["grpc-tonic", "trace", "metrics", "logs"] # grpc using tonic grpc-tonic = ["tonic", "prost", "http", "tokio", "opentelemetry-proto/gen-tonic"] gzip-tonic = ["tonic/gzip"] +zstd-tonic = ["tonic/zstd"] tls = ["tonic/tls"] tls-roots = ["tls", "tonic/tls-roots"] tls-webpki-roots = ["tls", "tonic/tls-webpki-roots"] diff --git a/opentelemetry-otlp/src/exporter/mod.rs b/opentelemetry-otlp/src/exporter/mod.rs index dac2cd1daa..d138527d8f 100644 --- a/opentelemetry-otlp/src/exporter/mod.rs +++ b/opentelemetry-otlp/src/exporter/mod.rs @@ -98,12 +98,15 @@ impl Default for ExportConfig { pub enum Compression { /// Compresses data using gzip. Gzip, + /// Compresses data using zstd. + Zstd, } impl Display for Compression { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { Compression::Gzip => write!(f, "gzip"), + Compression::Zstd => write!(f, "zstd"), } } } @@ -114,6 +117,7 @@ impl FromStr for Compression { fn from_str(s: &str) -> Result { match s { "gzip" => Ok(Compression::Gzip), + "zstd" => Ok(Compression::Zstd), _ => Err(Error::UnsupportedCompressionAlgorithm(s.to_string())), } } diff --git a/opentelemetry-otlp/src/exporter/tonic/mod.rs b/opentelemetry-otlp/src/exporter/tonic/mod.rs index 244d16b62d..7deddcffe6 100644 --- a/opentelemetry-otlp/src/exporter/tonic/mod.rs +++ b/opentelemetry-otlp/src/exporter/tonic/mod.rs @@ -52,8 +52,16 @@ impl TryFrom for tonic::codec::CompressionEncoding { #[cfg(feature = "gzip-tonic")] Compression::Gzip => Ok(tonic::codec::CompressionEncoding::Gzip), #[cfg(not(feature = "gzip-tonic"))] - Compression::Gzip => Err(crate::Error::UnsupportedCompressionAlgorithm( - value.to_string(), + Compression::Gzip => Err(crate::Error::FeatureRequiredForCompressionAlgorithm( + "gzip-tonic", + "gzip", + )), + #[cfg(feature = "zstd-tonic")] + Compression::Zstd => Ok(tonic::codec::CompressionEncoding::Zstd), + #[cfg(not(feature = "zstd-tonic"))] + Compression::Zstd => Err(crate::Error::FeatureRequiredForCompressionAlgorithm( + "zstd-tonic", + "zstd", )), } } @@ -399,7 +407,7 @@ fn parse_headers_from_env(signal_headers_var: &str) -> HeaderMap { #[cfg(test)] mod tests { use crate::exporter::tests::run_env_test; - #[cfg(feature = "gzip-tonic")] + #[cfg(feature = "grpc-tonic")] use crate::exporter::Compression; use crate::TonicExporterBuilder; use crate::{OTEL_EXPORTER_OTLP_HEADERS, OTEL_EXPORTER_OTLP_TRACES_HEADERS}; @@ -438,7 +446,7 @@ mod tests { #[test] #[cfg(feature = "gzip-tonic")] - fn test_with_compression() { + fn test_with_gzip_compression() { // metadata should merge with the current one with priority instead of just replacing it let mut metadata = MetadataMap::new(); metadata.insert("foo", "bar".parse().unwrap()); @@ -446,6 +454,26 @@ mod tests { assert_eq!(builder.tonic_config.compression.unwrap(), Compression::Gzip); } + #[test] + #[cfg(feature = "zstd-tonic")] + fn test_with_zstd_compression() { + let builder = TonicExporterBuilder::default().with_compression(Compression::Zstd); + assert_eq!(builder.tonic_config.compression.unwrap(), Compression::Zstd); + } + + #[test] + #[cfg(feature = "grpc-tonic")] + fn test_convert_compression() { + #[cfg(feature = "gzip-tonic")] + assert!(tonic::codec::CompressionEncoding::try_from(Compression::Gzip).is_ok()); + #[cfg(not(feature = "gzip-tonic"))] + assert!(tonic::codec::CompressionEncoding::try_from(Compression::Gzip).is_err()); + #[cfg(feature = "zstd-tonic")] + assert!(tonic::codec::CompressionEncoding::try_from(Compression::Zstd).is_ok()); + #[cfg(not(feature = "zstd-tonic"))] + assert!(tonic::codec::CompressionEncoding::try_from(Compression::Zstd).is_err()); + } + #[test] fn test_parse_headers_from_env() { run_env_test( diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs index 28a13a2ca8..6405b143dc 100644 --- a/opentelemetry-otlp/src/lib.rs +++ b/opentelemetry-otlp/src/lib.rs @@ -97,6 +97,7 @@ //! For users uses `tonic` as grpc layer: //! * `grpc-tonic`: Use `tonic` as grpc layer. This is enabled by default. //! * `gzip-tonic`: Use gzip compression for `tonic` grpc layer. +//! * `zstd-tonic`: Use zstd compression for `tonic` grpc layer. //! * `tls-tonic`: Enable TLS. //! * `tls-roots`: Adds system trust roots to rustls-based gRPC clients using the rustls-native-certs crate //! * `tls-webkpi-roots`: Embeds Mozilla's trust roots to rustls-based gRPC clients using the webkpi-roots crate @@ -372,6 +373,11 @@ pub enum Error { /// Unsupported compression algorithm. #[error("unsupported compression algorithm '{0}'")] UnsupportedCompressionAlgorithm(String), + + /// Feature required to use the specified compression algorithm. + #[cfg(any(not(feature = "gzip-tonic"), not(feature = "zstd-tonic")))] + #[error("feature '{0}' is required to use the compression algorithm '{1}'")] + FeatureRequiredForCompressionAlgorithm(&'static str, &'static str), } #[cfg(feature = "grpc-tonic")] From 3d496e7cd6459b131ca9b03f9502e83926b62384 Mon Sep 17 00:00:00 2001 From: harscoet Date: Fri, 19 Jul 2024 15:49:14 +0200 Subject: [PATCH 2/2] use Compression enum instead of static src --- opentelemetry-otlp/src/exporter/tonic/mod.rs | 4 ++-- opentelemetry-otlp/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-otlp/src/exporter/tonic/mod.rs b/opentelemetry-otlp/src/exporter/tonic/mod.rs index 7deddcffe6..69ecb4fe79 100644 --- a/opentelemetry-otlp/src/exporter/tonic/mod.rs +++ b/opentelemetry-otlp/src/exporter/tonic/mod.rs @@ -54,14 +54,14 @@ impl TryFrom for tonic::codec::CompressionEncoding { #[cfg(not(feature = "gzip-tonic"))] Compression::Gzip => Err(crate::Error::FeatureRequiredForCompressionAlgorithm( "gzip-tonic", - "gzip", + Compression::Gzip, )), #[cfg(feature = "zstd-tonic")] Compression::Zstd => Ok(tonic::codec::CompressionEncoding::Zstd), #[cfg(not(feature = "zstd-tonic"))] Compression::Zstd => Err(crate::Error::FeatureRequiredForCompressionAlgorithm( "zstd-tonic", - "zstd", + Compression::Zstd, )), } } diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs index 6405b143dc..cb3077d23c 100644 --- a/opentelemetry-otlp/src/lib.rs +++ b/opentelemetry-otlp/src/lib.rs @@ -377,7 +377,7 @@ pub enum Error { /// Feature required to use the specified compression algorithm. #[cfg(any(not(feature = "gzip-tonic"), not(feature = "zstd-tonic")))] #[error("feature '{0}' is required to use the compression algorithm '{1}'")] - FeatureRequiredForCompressionAlgorithm(&'static str, &'static str), + FeatureRequiredForCompressionAlgorithm(&'static str, Compression), } #[cfg(feature = "grpc-tonic")]