From f1e143cf2fe71ac70a426452b5dd0aee5ba699df Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 19 Nov 2021 11:23:48 +0100 Subject: [PATCH] rename otlp-tonic to otlp-grpc * The cargo feature name should match the actual feature, not the implementation detail * allow empty configuration under http or grpc exporter with this we can configure the HTTP or gRPC exporter while using default values, like: opentelemetry: otlp: tracing: exporter: http: --- .circleci/config.yml | 4 +-- crates/apollo-router/src/configuration/mod.rs | 22 ++++++++-------- .../configuration/otlp/{tonic.rs => grpc.rs} | 6 ++--- .../src/configuration/otlp/http.rs | 2 +- .../src/configuration/otlp/mod.rs | 26 +++++++++---------- ...serde__tests__serialize_metadata_map.snap} | 2 +- ...iguration_api_does_not_change_grpc-2.snap} | 0 ...nfiguration_api_does_not_change_grpc.snap} | 7 +---- ...onfiguration_api_does_not_change_http.snap | 6 +---- ...opentelemetry_otlp_tracing_grpc_basic.yml} | 3 +-- ...pentelemetry_otlp_tracing_grpc_common.yml} | 0 ..._opentelemetry_otlp_tracing_grpc_full.yml} | 0 ...g_opentelemetry_otlp_tracing_grpc_tls.yml} | 0 ..._opentelemetry_otlp_tracing_http_basic.yml | 3 +-- crates/apollo-router/src/lib.rs | 2 +- xtask/src/commands/test.rs | 20 +------------- 16 files changed, 37 insertions(+), 66 deletions(-) rename crates/apollo-router/src/configuration/otlp/{tonic.rs => grpc.rs} (97%) rename crates/apollo-router/src/configuration/otlp/snapshots/{apollo_router__configuration__otlp__tonic__header_map_serde__tests__serialize_metadata_map.snap => apollo_router__configuration__otlp__grpc__header_map_serde__tests__serialize_metadata_map.snap} (61%) rename crates/apollo-router/src/configuration/snapshots/{apollo_router__configuration__tests__ensure_configuration_api_does_not_change_tonic-2.snap => apollo_router__configuration__tests__ensure_configuration_api_does_not_change_grpc-2.snap} (100%) rename crates/apollo-router/src/configuration/snapshots/{apollo_router__configuration__tests__ensure_configuration_api_does_not_change_tonic.snap => apollo_router__configuration__tests__ensure_configuration_api_does_not_change_grpc.snap} (65%) rename crates/apollo-router/src/configuration/testdata/{config_opentelemetry_otlp_tracing_tonic_basic.yml => config_opentelemetry_otlp_tracing_grpc_basic.yml} (78%) rename crates/apollo-router/src/configuration/testdata/{config_opentelemetry_otlp_tracing_tonic_common.yml => config_opentelemetry_otlp_tracing_grpc_common.yml} (100%) rename crates/apollo-router/src/configuration/testdata/{config_opentelemetry_otlp_tracing_tonic_full.yml => config_opentelemetry_otlp_tracing_grpc_full.yml} (100%) rename crates/apollo-router/src/configuration/testdata/{config_opentelemetry_otlp_tracing_tonic_tls.yml => config_opentelemetry_otlp_tracing_grpc_tls.yml} (100%) diff --git a/.circleci/config.yml b/.circleci/config.yml index 773fd6d7fb..2df7bf1f37 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -233,10 +233,10 @@ commands: steps: - rust/build: with_cache: false - crate: --features otlp-tonic,tls -p apollo-router -p apollo-router-core + crate: --features otlp-grpc,tls -p apollo-router -p apollo-router-core - rust/build: with_cache: false - crate: --features otlp-tonic -p apollo-router -p apollo-router-core + crate: --features otlp-grpc -p apollo-router -p apollo-router-core - rust/build: with_cache: false crate: --no-default-features --features otlp-http -p apollo-router -p apollo-router-core diff --git a/crates/apollo-router/src/configuration/mod.rs b/crates/apollo-router/src/configuration/mod.rs index eeb184f075..8c89809f0d 100644 --- a/crates/apollo-router/src/configuration/mod.rs +++ b/crates/apollo-router/src/configuration/mod.rs @@ -1,6 +1,6 @@ //! Logic for loading configuration in to an object model -#[cfg(any(feature = "otlp-tonic", feature = "otlp-http"))] +#[cfg(any(feature = "otlp-grpc", feature = "otlp-http"))] pub mod otlp; use apollo_router_core::prelude::*; @@ -199,7 +199,7 @@ impl Cors { #[allow(clippy::large_enum_variant)] pub enum OpenTelemetry { Jaeger(Option), - #[cfg(any(feature = "otlp-tonic", feature = "otlp-http"))] + #[cfg(any(feature = "otlp-grpc", feature = "otlp-http"))] Otlp(otlp::Otlp), } @@ -312,7 +312,7 @@ mod tests { assert_config_snapshot!("testdata/config_opentelemetry_jaeger_full.yml"); } - #[cfg(any(feature = "otlp-tonic", feature = "otlp-http"))] + #[cfg(any(feature = "otlp-grpc", feature = "otlp-http"))] #[test] fn ensure_configuration_api_does_not_change_common() { // NOTE: don't take a snapshot here because the optional fields appear with ~ and they vary @@ -324,18 +324,18 @@ mod tests { )) .unwrap(); - #[cfg(feature = "otlp-tonic")] + #[cfg(feature = "otlp-grpc")] serde_yaml::from_str::(include_str!( - "testdata/config_opentelemetry_otlp_tracing_tonic_common.yml" + "testdata/config_opentelemetry_otlp_tracing_grpc_common.yml" )) .unwrap(); } - #[cfg(feature = "otlp-tonic")] + #[cfg(feature = "otlp-grpc")] #[test] - fn ensure_configuration_api_does_not_change_tonic() { - assert_config_snapshot!("testdata/config_opentelemetry_otlp_tracing_tonic_basic.yml"); - assert_config_snapshot!("testdata/config_opentelemetry_otlp_tracing_tonic_full.yml"); + fn ensure_configuration_api_does_not_change_grpc() { + assert_config_snapshot!("testdata/config_opentelemetry_otlp_tracing_grpc_basic.yml"); + assert_config_snapshot!("testdata/config_opentelemetry_otlp_tracing_grpc_full.yml"); } #[cfg(feature = "otlp-http")] @@ -345,10 +345,10 @@ mod tests { assert_config_snapshot!("testdata/config_opentelemetry_otlp_tracing_http_full.yml"); } - #[cfg(all(feature = "tls", feature = "otlp-tonic"))] + #[cfg(all(feature = "tls", feature = "otlp-grpc"))] #[test] fn ensure_configuration_api_does_not_change_tls_config() { - assert_config_snapshot!("testdata/config_opentelemetry_otlp_tracing_tonic_tls.yml"); + assert_config_snapshot!("testdata/config_opentelemetry_otlp_tracing_grpc_tls.yml"); } #[test] diff --git a/crates/apollo-router/src/configuration/otlp/tonic.rs b/crates/apollo-router/src/configuration/otlp/grpc.rs similarity index 97% rename from crates/apollo-router/src/configuration/otlp/tonic.rs rename to crates/apollo-router/src/configuration/otlp/grpc.rs index 5cff8ca301..310d86ec7d 100644 --- a/crates/apollo-router/src/configuration/otlp/tonic.rs +++ b/crates/apollo-router/src/configuration/otlp/grpc.rs @@ -5,9 +5,9 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use tonic::metadata::{KeyAndValueRef, MetadataKey, MetadataMap}; -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize, Default)] #[serde(deny_unknown_fields)] -pub struct TonicExporter { +pub struct GrpcExporter { #[serde(flatten)] export_config: ExportConfig, tls_config: Option, @@ -15,7 +15,7 @@ pub struct TonicExporter { metadata: Option, } -impl TonicExporter { +impl GrpcExporter { pub fn exporter(&self) -> Result { let mut exporter = opentelemetry_otlp::new_exporter().tonic(); exporter = self.export_config.apply(exporter); diff --git a/crates/apollo-router/src/configuration/otlp/http.rs b/crates/apollo-router/src/configuration/otlp/http.rs index 93769e0a86..a9f90cccfc 100644 --- a/crates/apollo-router/src/configuration/otlp/http.rs +++ b/crates/apollo-router/src/configuration/otlp/http.rs @@ -4,7 +4,7 @@ use opentelemetry_otlp::WithExportConfig; use serde::{Deserialize, Serialize}; use std::collections::HashMap; -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize, Default)] #[serde(deny_unknown_fields)] pub struct HttpExporter { #[serde(flatten)] diff --git a/crates/apollo-router/src/configuration/otlp/mod.rs b/crates/apollo-router/src/configuration/otlp/mod.rs index b7ba1b333c..90d94917d7 100644 --- a/crates/apollo-router/src/configuration/otlp/mod.rs +++ b/crates/apollo-router/src/configuration/otlp/mod.rs @@ -1,12 +1,12 @@ +#[cfg(feature = "otlp-grpc")] +mod grpc; #[cfg(feature = "otlp-http")] mod http; -#[cfg(feature = "otlp-tonic")] -mod tonic; +#[cfg(feature = "otlp-grpc")] +pub use self::grpc::*; #[cfg(feature = "otlp-http")] pub use self::http::*; -#[cfg(feature = "otlp-tonic")] -pub use self::tonic::*; use crate::configuration::ConfigurationError; use opentelemetry::sdk::resource::Resource; use opentelemetry::sdk::trace::{Sampler, Tracer}; @@ -32,19 +32,19 @@ pub struct Tracing { #[derive(Debug, Clone, Deserialize, Serialize)] #[serde(deny_unknown_fields, rename_all = "snake_case")] pub enum Exporter { - #[cfg(feature = "otlp-tonic")] - Grpc(TonicExporter), + #[cfg(feature = "otlp-grpc")] + Grpc(Option), #[cfg(feature = "otlp-http")] - Http(HttpExporter), + Http(Option), } impl Exporter { pub fn exporter(&self) -> Result { match &self { - #[cfg(feature = "otlp-tonic")] - Exporter::Grpc(exporter) => Ok(exporter.exporter()?.into()), + #[cfg(feature = "otlp-grpc")] + Exporter::Grpc(exporter) => Ok(exporter.clone().unwrap_or_default().exporter()?.into()), #[cfg(feature = "otlp-http")] - Exporter::Http(exporter) => Ok(exporter.exporter()?.into()), + Exporter::Http(exporter) => Ok(exporter.clone().unwrap_or_default().exporter()?.into()), } } @@ -53,8 +53,8 @@ impl Exporter { match std::env::var("ROUTER_TRACING").as_deref() { #[cfg(feature = "otlp-http")] Ok("http") => Ok(HttpExporter::exporter_from_env().into()), - #[cfg(feature = "otlp-tonic")] - Ok("tonic") => Ok(TonicExporter::exporter_from_env().into()), + #[cfg(feature = "otlp-grpc")] + Ok("tonic") => Ok(GrpcExporter::exporter_from_env().into()), #[cfg(not(any(feature = "otlp-http", feature = "otlp-grpc")))] Ok(val) => Err(ConfigurationError::InvalidEnvironmentVariable(format!( "unrecognized value for ROUTER_TRACING: {} - this router is built without support for OpenTelemetry", @@ -99,7 +99,7 @@ impl Tracing { } } -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, Deserialize, Serialize, Default)] #[serde(deny_unknown_fields, rename_all = "snake_case")] pub struct ExportConfig { #[serde(deserialize_with = "endpoint_url", default)] diff --git a/crates/apollo-router/src/configuration/otlp/snapshots/apollo_router__configuration__otlp__tonic__header_map_serde__tests__serialize_metadata_map.snap b/crates/apollo-router/src/configuration/otlp/snapshots/apollo_router__configuration__otlp__grpc__header_map_serde__tests__serialize_metadata_map.snap similarity index 61% rename from crates/apollo-router/src/configuration/otlp/snapshots/apollo_router__configuration__otlp__tonic__header_map_serde__tests__serialize_metadata_map.snap rename to crates/apollo-router/src/configuration/otlp/snapshots/apollo_router__configuration__otlp__grpc__header_map_serde__tests__serialize_metadata_map.snap index 87134e5d5a..eca9349e02 100644 --- a/crates/apollo-router/src/configuration/otlp/snapshots/apollo_router__configuration__otlp__tonic__header_map_serde__tests__serialize_metadata_map.snap +++ b/crates/apollo-router/src/configuration/otlp/snapshots/apollo_router__configuration__otlp__grpc__header_map_serde__tests__serialize_metadata_map.snap @@ -1,5 +1,5 @@ --- -source: crates/apollo-router/src/configuration/otlp/tonic.rs +source: crates/apollo-router/src/configuration/otlp/grpc.rs expression: "std::str::from_utf8(&buffer).unwrap()" --- diff --git a/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_tonic-2.snap b/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_grpc-2.snap similarity index 100% rename from crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_tonic-2.snap rename to crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_grpc-2.snap diff --git a/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_tonic.snap b/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_grpc.snap similarity index 65% rename from crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_tonic.snap rename to crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_grpc.snap index afc051f66a..f8f162a4f9 100644 --- a/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_tonic.snap +++ b/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_grpc.snap @@ -13,11 +13,6 @@ opentelemetry: otlp: tracing: exporter: - grpc: - endpoint: ~ - metadata: ~ - protocol: ~ - timeout: ~ - tls_config: ~ + grpc: ~ trace_config: ~ diff --git a/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_http.snap b/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_http.snap index 144f77a8ff..610e0d46d2 100644 --- a/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_http.snap +++ b/crates/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__ensure_configuration_api_does_not_change_http.snap @@ -13,10 +13,6 @@ opentelemetry: otlp: tracing: exporter: - http: - endpoint: ~ - headers: ~ - protocol: ~ - timeout: ~ + http: ~ trace_config: ~ diff --git a/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_tonic_basic.yml b/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_grpc_basic.yml similarity index 78% rename from crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_tonic_basic.yml rename to crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_grpc_basic.yml index 70a0a3cce3..81c50e00af 100644 --- a/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_tonic_basic.yml +++ b/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_grpc_basic.yml @@ -7,5 +7,4 @@ opentelemetry: otlp: tracing: exporter: - grpc: - endpoint: \ No newline at end of file + grpc: \ No newline at end of file diff --git a/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_tonic_common.yml b/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_grpc_common.yml similarity index 100% rename from crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_tonic_common.yml rename to crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_grpc_common.yml diff --git a/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_tonic_full.yml b/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_grpc_full.yml similarity index 100% rename from crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_tonic_full.yml rename to crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_grpc_full.yml diff --git a/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_tonic_tls.yml b/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_grpc_tls.yml similarity index 100% rename from crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_tonic_tls.yml rename to crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_grpc_tls.yml diff --git a/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_http_basic.yml b/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_http_basic.yml index a29d88f0e6..017a930caa 100644 --- a/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_http_basic.yml +++ b/crates/apollo-router/src/configuration/testdata/config_opentelemetry_otlp_tracing_http_basic.yml @@ -7,5 +7,4 @@ opentelemetry: otlp: tracing: exporter: - http: - endpoint: \ No newline at end of file + http: \ No newline at end of file diff --git a/crates/apollo-router/src/lib.rs b/crates/apollo-router/src/lib.rs index b2cd09e759..5e0b4197dc 100644 --- a/crates/apollo-router/src/lib.rs +++ b/crates/apollo-router/src/lib.rs @@ -306,7 +306,7 @@ fn try_initialize_subscriber( opentelemetry::global::set_error_handler(handle_error)?; return Ok(Some(Arc::new(subscriber.with(telemetry)))); } - #[cfg(any(feature = "otlp-tonic", feature = "otlp-http"))] + #[cfg(any(feature = "otlp-grpc", feature = "otlp-http"))] Some(OpenTelemetry::Otlp(configuration::otlp::Otlp::Tracing(tracing))) => { let tracer = if let Some(tracing) = tracing.as_ref() { tracing.tracer()? diff --git a/xtask/src/commands/test.rs b/xtask/src/commands/test.rs index 16764d6f13..c488358fd4 100644 --- a/xtask/src/commands/test.rs +++ b/xtask/src/commands/test.rs @@ -12,12 +12,7 @@ const TEST_DEFAULT_ARGS: &[&str] = &[ "--features", ]; -const FEATURE_SETS: &[&[&str]] = &[ - &["otlp-tonic", "tls"], - &["otlp-tonic"], - &["otlp-http"], - &[""], -]; +const FEATURE_SETS: &[&[&str]] = &[&["otlp-grpc", "tls"], &["otlp-grpc"], &["otlp-http"], &[""]]; #[derive(Debug, StructOpt)] pub struct Test { @@ -89,21 +84,8 @@ impl Test { }; for features in FEATURE_SETS { -<<<<<<< HEAD - if cfg!(windows) && features.contains(&"otlp-grpcio") { - // TODO: I couldn't make it build on Windows but it is supposed to build. - continue; - } - eprintln!("Running tests with features: {}", features.join(",")); cargo!(TEST_DEFAULT_ARGS, features.iter()); -======= - eprintln!("Running tests with features: {}", features.join(", ")); - cargo!( - ["test", "--workspace", "--locked", "--no-default-features"], - features.iter().flat_map(|feature| ["--features", feature]), - ); ->>>>>>> 4941eeb (remove grpcio) } Ok(())