Skip to content

Commit

Permalink
rename otlp-tonic to otlp-grpc
Browse files Browse the repository at this point in the history
* 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:
  • Loading branch information
Geal committed Nov 23, 2021
1 parent a1b20c9 commit f1e143c
Show file tree
Hide file tree
Showing 16 changed files with 37 additions and 66 deletions.
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 11 additions & 11 deletions crates/apollo-router/src/configuration/mod.rs
Original file line number Diff line number Diff line change
@@ -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::*;
Expand Down Expand Up @@ -199,7 +199,7 @@ impl Cors {
#[allow(clippy::large_enum_variant)]
pub enum OpenTelemetry {
Jaeger(Option<Jaeger>),
#[cfg(any(feature = "otlp-tonic", feature = "otlp-http"))]
#[cfg(any(feature = "otlp-grpc", feature = "otlp-http"))]
Otlp(otlp::Otlp),
}

Expand Down Expand Up @@ -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
Expand All @@ -324,18 +324,18 @@ mod tests {
))
.unwrap();

#[cfg(feature = "otlp-tonic")]
#[cfg(feature = "otlp-grpc")]
serde_yaml::from_str::<Configuration>(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")]
Expand All @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ 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<TlsConfig>,
#[serde(with = "header_map_serde", default)]
metadata: Option<MetadataMap>,
}

impl TonicExporter {
impl GrpcExporter {
pub fn exporter(&self) -> Result<opentelemetry_otlp::TonicExporterBuilder, ConfigurationError> {
let mut exporter = opentelemetry_otlp::new_exporter().tonic();
exporter = self.export_config.apply(exporter);
Expand Down
2 changes: 1 addition & 1 deletion crates/apollo-router/src/configuration/otlp/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
26 changes: 13 additions & 13 deletions crates/apollo-router/src/configuration/otlp/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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<GrpcExporter>),
#[cfg(feature = "otlp-http")]
Http(HttpExporter),
Http(Option<HttpExporter>),
}

impl Exporter {
pub fn exporter(&self) -> Result<opentelemetry_otlp::SpanExporterBuilder, ConfigurationError> {
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()),
}
}

Expand All @@ -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",
Expand Down Expand Up @@ -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)]
Expand Down
Original file line number Diff line number Diff line change
@@ -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()"

---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ opentelemetry:
otlp:
tracing:
exporter:
grpc:
endpoint: ~
metadata: ~
protocol: ~
timeout: ~
tls_config: ~
grpc: ~
trace_config: ~

Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ opentelemetry:
otlp:
tracing:
exporter:
http:
endpoint: ~
headers: ~
protocol: ~
timeout: ~
http: ~
trace_config: ~

Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,4 @@ opentelemetry:
otlp:
tracing:
exporter:
grpc:
endpoint:
grpc:
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,4 @@ opentelemetry:
otlp:
tracing:
exporter:
http:
endpoint:
http:
2 changes: 1 addition & 1 deletion crates/apollo-router/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?
Expand Down
20 changes: 1 addition & 19 deletions xtask/src/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(())
Expand Down

0 comments on commit f1e143c

Please sign in to comment.