Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,19 @@ When validating variables, we should use default values for object fields if app

By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/2003

### ([Issue #2036](https://github.com/apollographql/router/issues/2036))

Work around for [opentelemetry-rust#908](https://github.com/open-telemetry/opentelemetry-rust/issues/908)
The default URL currently incorrectly uses https causing errors when connecting to a default localhost OTel Collector.

The router will detect and work around this by explicitly setting the correct endpoint URLs when not specified in config.

In addition:
* basic TLS defaulting will occur when the endpoint scheme uses `https`.
* a warning will be raised if the endpoint port is 443 but no TLS config is specified.

By [@bryncooke](https://github.com/bryncooke) in https://github.com/apollographql/router/pull/#2048

## 🛠 Maintenance

### Apply tower best practice to inner service cloning ([PR #2030](https://github.com/apollographql/router/pull/2030))
Expand Down
32 changes: 27 additions & 5 deletions apollo-router/src/plugins/telemetry/otlp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,19 @@ impl Config {
pub(crate) fn exporter<T: From<HttpExporterBuilder> + From<TonicExporterBuilder>>(
&self,
) -> Result<T, BoxError> {
let endpoint = match &self.endpoint {
Endpoint::Default(_) => None,
Endpoint::Url(s) => Some(s),
let endpoint = match (self.endpoint.clone(), &self.protocol) {
// # https://github.com/apollographql/router/issues/2036
// Opentelemetry rust incorrectly defaults to https
// This will override the defaults to that of the spec
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md
(Endpoint::Default(_), Some(Protocol::Http)) => {
Some(Url::parse("http://localhost:4318").expect("default url is valid"))
}
// Default is GRPC
(Endpoint::Default(_), _) => {
Some(Url::parse("http://localhost:4317").expect("default url is valid"))
}
(Endpoint::Url(s), _) => Some(s),
};
match self.protocol.clone().unwrap_or_default() {
Protocol::Grpc => {
Expand All @@ -51,7 +61,7 @@ impl Config {
.with_env()
.with(&self.timeout, |b, t| b.with_timeout(*t))
.with(&endpoint, |b, e| b.with_endpoint(e.as_str()))
.try_with(&grpc.tls_config.defaulted(endpoint), |b, t| {
.try_with(&grpc.tls_config.defaulted(endpoint.as_ref()), |b, t| {
Ok(b.with_tls_config(t.try_into()?))
})?
.with(&grpc.metadata, |b, m| b.with_metadata(m.clone()))
Expand Down Expand Up @@ -139,7 +149,13 @@ impl TlsConfig {
pub(crate) fn defaulted(mut self, endpoint: Option<&Url>) -> Option<TlsConfig> {
if let Some(endpoint) = endpoint {
if self.domain_name.is_none() {
self.domain_name = endpoint.host_str().map(|s| s.to_string())
// If the URL contains the https scheme then default the tls config to use the domain from the URL. We know it's TLS.
// If the URL contains no scheme and the port is 443 emit a warning suggesting that they may have forgotten to configure TLS domain.
if endpoint.scheme() == "https" {
self.domain_name = endpoint.host_str().map(|s| s.to_string())
} else if endpoint.port() == Some(443) && endpoint.scheme() != "http" {
tracing::warn!("telemetry otlp exporter has been configured with port 443 but TLS domain has not been set. This is likely a configuration error")
}
}
}

Expand Down Expand Up @@ -297,6 +313,12 @@ mod tests {
assert_eq!(tls, None);
}

#[test]
fn default_endpoint() {
let tls = TlsConfig::default().defaulted(None);
assert_eq!(tls, None);
}

#[test]
fn endpoint_grpc_defaulting_scheme() {
let url = Url::parse("https://api.apm.com:433").unwrap();
Expand Down