From bfdd27182ab1e4fde9d41022f85b85ed326bc568 Mon Sep 17 00:00:00 2001 From: bryn Date: Tue, 17 Oct 2023 12:23:37 +0100 Subject: [PATCH 1/2] Rename `trace_config` to `common` Currently, we have `metrics.common` and `tracing.trace_config`. Bring tracing into alignment with metrics by renaming `trace_config` to `common`. --- .../config_bryn_telemetry_config_fixes.md | 14 + .../0013-telemetry-exporters-enabled.yaml | 5 + ...> 0014-telemetry_trace_config_common.yaml} | 0 .../src/configuration/migrations/README.md | 2 +- ...nfiguration__tests__schema_generation.snap | 298 +++++++++--------- ...onfiguration@trace_config.router.yaml.snap | 10 + .../migrations/trace_config.router.yaml | 5 + .../testdata/tracing_config.router.yaml | 2 +- .../testdata/zipkin-address.router.yaml | 2 +- .../testdata/zipkin-url.router.yaml | 2 +- .../testdata/zipkin-url_env.router.yaml | 2 +- .../configuration/testdata/zipkin.router.yaml | 2 +- apollo-router/src/plugins/telemetry/config.rs | 22 +- apollo-router/src/plugins/telemetry/mod.rs | 20 +- .../telemetry/testdata/config.router.yaml | 2 +- .../src/plugins/telemetry/tracing/apollo.rs | 2 +- .../src/plugins/telemetry/tracing/datadog.rs | 8 +- .../src/plugins/telemetry/tracing/jaeger.rs | 6 +- .../src/plugins/telemetry/tracing/mod.rs | 2 +- .../src/plugins/telemetry/tracing/otlp.rs | 2 +- .../src/plugins/telemetry/tracing/zipkin.rs | 4 +- .../src/testdata/datadog.router.yaml | 2 +- apollo-router/src/testdata/jaeger.router.yaml | 2 +- apollo-router/src/testdata/otlp.router.yaml | 2 +- apollo-router/src/testdata/zipkin.router.yaml | 2 +- .../fixtures/apollo_reports.batch_router.yaml | 2 +- .../tests/fixtures/apollo_reports.router.yaml | 2 +- .../tests/fixtures/broken_plugin.router.yaml | 2 +- .../tests/fixtures/datadog.router.yaml | 2 +- .../fixtures/jaeger-0.5-sample.router.yaml | 2 +- .../fixtures/jaeger-no-sample.router.yaml | 2 +- .../tests/fixtures/jaeger.router.yaml | 2 +- .../tests/fixtures/no-telemetry.router.yaml | 4 +- apollo-router/tests/fixtures/otlp.router.yaml | 2 +- .../tests/fixtures/zipkin.router.yaml | 2 +- apollo-router/tests/telemetry_test.rs | 2 +- .../tracing/router/datadog.router.yaml | 2 +- dockerfiles/tracing/router/jaeger.router.yaml | 2 +- dockerfiles/tracing/router/zipkin.router.yaml | 2 +- .../source/configuration/apollo-telemetry.mdx | 10 +- docs/source/configuration/metrics.mdx | 2 +- docs/source/configuration/tracing.mdx | 8 +- examples/telemetry/datadog.router.yaml | 2 +- examples/telemetry/jaeger-agent.router.yaml | 2 +- .../telemetry/jaeger-collector.router.yaml | 2 +- examples/telemetry/otlp.router.yaml | 2 +- examples/telemetry/zipkin-agent.router.yaml | 2 +- .../telemetry/zipkin-collector.router.yaml | 2 +- 48 files changed, 258 insertions(+), 224 deletions(-) create mode 100644 .changesets/config_bryn_telemetry_config_fixes.md create mode 100644 apollo-router/src/configuration/migrations/0013-telemetry-exporters-enabled.yaml rename apollo-router/src/configuration/migrations/{0012-telemetry-exporters-enabled.yaml => 0014-telemetry_trace_config_common.yaml} (100%) create mode 100644 apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@trace_config.router.yaml.snap create mode 100644 apollo-router/src/configuration/testdata/migrations/trace_config.router.yaml diff --git a/.changesets/config_bryn_telemetry_config_fixes.md b/.changesets/config_bryn_telemetry_config_fixes.md new file mode 100644 index 0000000000..383e18efc8 --- /dev/null +++ b/.changesets/config_bryn_telemetry_config_fixes.md @@ -0,0 +1,14 @@ +### Bring telemetry tracing config and metrics config into alignment ([Issue #4043](https://github.com/apollographql/router/issues/4043)) + +Configuration between tracing and metrics was inconsistent and did not align with otel spec terminology. The following changes have been made to router.yaml configuration: + +`trace_config` has been renamed to `common` + + ```diff +telemetry + tracing: +- trace_config: ++ common: + ``` + +By [@BrynCooke](https://github.com/BrynCooke) in https://github.com/apollographql/router/pull/4044 diff --git a/apollo-router/src/configuration/migrations/0013-telemetry-exporters-enabled.yaml b/apollo-router/src/configuration/migrations/0013-telemetry-exporters-enabled.yaml new file mode 100644 index 0000000000..cfdbc456b6 --- /dev/null +++ b/apollo-router/src/configuration/migrations/0013-telemetry-exporters-enabled.yaml @@ -0,0 +1,5 @@ +description: telemetry.tracing.trace_config has been renamed to telemetry.tracing.common +actions: + - type: move + from: telemetry.tracing.trace_config + to: telemetry.tracing.common diff --git a/apollo-router/src/configuration/migrations/0012-telemetry-exporters-enabled.yaml b/apollo-router/src/configuration/migrations/0014-telemetry_trace_config_common.yaml similarity index 100% rename from apollo-router/src/configuration/migrations/0012-telemetry-exporters-enabled.yaml rename to apollo-router/src/configuration/migrations/0014-telemetry_trace_config_common.yaml diff --git a/apollo-router/src/configuration/migrations/README.md b/apollo-router/src/configuration/migrations/README.md index 3913561c21..b9175f85cf 100644 --- a/apollo-router/src/configuration/migrations/README.md +++ b/apollo-router/src/configuration/migrations/README.md @@ -10,7 +10,7 @@ The filename should begin with a 4 digit numerical prefix. This allows us to app The yaml consists of a description and a number of actions: ```yaml -description: telemetry.tracing.trace_config.attributes.router has been renamed to 'supergraph' for consistency +description: telemetry.tracing.common.attributes.router has been renamed to 'supergraph' for consistency actions: - type: move from: some.source diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index 04c81a9a1d..c4ee9cc3a7 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -4693,6 +4693,155 @@ expression: "&schema" "description": "Tracing configuration", "type": "object", "properties": { + "common": { + "description": "Common configuration", + "type": "object", + "properties": { + "attributes": { + "description": "The resources configured on the tracing pipeline", + "default": {}, + "type": "object", + "additionalProperties": { + "anyOf": [ + { + "description": "bool values", + "type": "boolean" + }, + { + "description": "i64 values", + "type": "integer", + "format": "int64" + }, + { + "description": "f64 values", + "type": "number", + "format": "double" + }, + { + "description": "String values", + "type": "string" + }, + { + "description": "Array of homogeneous values", + "anyOf": [ + { + "description": "Array of bools", + "type": "array", + "items": { + "type": "boolean" + } + }, + { + "description": "Array of integers", + "type": "array", + "items": { + "type": "integer", + "format": "int64" + } + }, + { + "description": "Array of floats", + "type": "array", + "items": { + "type": "number", + "format": "double" + } + }, + { + "description": "Array of strings", + "type": "array", + "items": { + "type": "string" + } + } + ] + } + ] + } + }, + "max_attributes_per_event": { + "description": "The maximum attributes per event before discarding", + "default": 128, + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, + "max_attributes_per_link": { + "description": "The maximum attributes per link before discarding", + "default": 128, + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, + "max_attributes_per_span": { + "description": "The maximum attributes per span before discarding", + "default": 128, + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, + "max_events_per_span": { + "description": "The maximum events per span before discarding", + "default": 128, + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, + "max_links_per_span": { + "description": "The maximum links per span before discarding", + "default": 128, + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, + "parent_based_sampler": { + "description": "Whether to use parent based sampling", + "default": true, + "type": "boolean" + }, + "sampler": { + "description": "The sampler, always_on, always_off or a decimal between 0.0 and 1.0", + "anyOf": [ + { + "description": "Sample a given fraction. Fractions >= 1 will always sample.", + "type": "number", + "format": "double" + }, + { + "oneOf": [ + { + "description": "Always sample", + "type": "string", + "enum": [ + "always_on" + ] + }, + { + "description": "Never sample", + "type": "string", + "enum": [ + "always_off" + ] + } + ] + } + ] + }, + "service_name": { + "description": "The trace service name", + "default": null, + "type": "string", + "nullable": true + }, + "service_namespace": { + "description": "The trace service namespace", + "default": null, + "type": "string", + "nullable": true + } + }, + "additionalProperties": false + }, "datadog": { "description": "Datadog exporter configuration", "type": "object", @@ -5130,155 +5279,6 @@ expression: "&schema" }, "additionalProperties": false }, - "trace_config": { - "description": "Common configuration", - "type": "object", - "properties": { - "attributes": { - "description": "The resources configured on the tracing pipeline", - "default": {}, - "type": "object", - "additionalProperties": { - "anyOf": [ - { - "description": "bool values", - "type": "boolean" - }, - { - "description": "i64 values", - "type": "integer", - "format": "int64" - }, - { - "description": "f64 values", - "type": "number", - "format": "double" - }, - { - "description": "String values", - "type": "string" - }, - { - "description": "Array of homogeneous values", - "anyOf": [ - { - "description": "Array of bools", - "type": "array", - "items": { - "type": "boolean" - } - }, - { - "description": "Array of integers", - "type": "array", - "items": { - "type": "integer", - "format": "int64" - } - }, - { - "description": "Array of floats", - "type": "array", - "items": { - "type": "number", - "format": "double" - } - }, - { - "description": "Array of strings", - "type": "array", - "items": { - "type": "string" - } - } - ] - } - ] - } - }, - "max_attributes_per_event": { - "description": "The maximum attributes per event before discarding", - "default": 128, - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "max_attributes_per_link": { - "description": "The maximum attributes per link before discarding", - "default": 128, - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "max_attributes_per_span": { - "description": "The maximum attributes per span before discarding", - "default": 128, - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "max_events_per_span": { - "description": "The maximum events per span before discarding", - "default": 128, - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "max_links_per_span": { - "description": "The maximum links per span before discarding", - "default": 128, - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "parent_based_sampler": { - "description": "Whether to use parent based sampling", - "default": true, - "type": "boolean" - }, - "sampler": { - "description": "The sampler, always_on, always_off or a decimal between 0.0 and 1.0", - "anyOf": [ - { - "description": "Sample a given fraction. Fractions >= 1 will always sample.", - "type": "number", - "format": "double" - }, - { - "oneOf": [ - { - "description": "Always sample", - "type": "string", - "enum": [ - "always_on" - ] - }, - { - "description": "Never sample", - "type": "string", - "enum": [ - "always_off" - ] - } - ] - } - ] - }, - "service_name": { - "description": "The trace service name", - "default": null, - "type": "string", - "nullable": true - }, - "service_namespace": { - "description": "The trace service namespace", - "default": null, - "type": "string", - "nullable": true - } - }, - "additionalProperties": false - }, "zipkin": { "description": "Zipkin exporter configuration", "type": "object", diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@trace_config.router.yaml.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@trace_config.router.yaml.snap new file mode 100644 index 0000000000..f8f0a77977 --- /dev/null +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@trace_config.router.yaml.snap @@ -0,0 +1,10 @@ +--- +source: apollo-router/src/configuration/tests.rs +expression: new_config +--- +--- +telemetry: + tracing: + common: + service_name: foo + diff --git a/apollo-router/src/configuration/testdata/migrations/trace_config.router.yaml b/apollo-router/src/configuration/testdata/migrations/trace_config.router.yaml new file mode 100644 index 0000000000..94dfe94181 --- /dev/null +++ b/apollo-router/src/configuration/testdata/migrations/trace_config.router.yaml @@ -0,0 +1,5 @@ +telemetry: + tracing: + trace_config: + service_name: "foo" + diff --git a/apollo-router/src/configuration/testdata/tracing_config.router.yaml b/apollo-router/src/configuration/testdata/tracing_config.router.yaml index 9b071d72ad..fee8cc5469 100644 --- a/apollo-router/src/configuration/testdata/tracing_config.router.yaml +++ b/apollo-router/src/configuration/testdata/tracing_config.router.yaml @@ -2,7 +2,7 @@ supergraph: listen: 1.2.3.4:5 telemetry: tracing: - trace_config: + common: attributes: foo: bar max_attributes_per_event: 2 diff --git a/apollo-router/src/configuration/testdata/zipkin-address.router.yaml b/apollo-router/src/configuration/testdata/zipkin-address.router.yaml index 3f8e0d60e2..991fbf83fe 100644 --- a/apollo-router/src/configuration/testdata/zipkin-address.router.yaml +++ b/apollo-router/src/configuration/testdata/zipkin-address.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router zipkin: enabled: true diff --git a/apollo-router/src/configuration/testdata/zipkin-url.router.yaml b/apollo-router/src/configuration/testdata/zipkin-url.router.yaml index d756550677..113362a93a 100644 --- a/apollo-router/src/configuration/testdata/zipkin-url.router.yaml +++ b/apollo-router/src/configuration/testdata/zipkin-url.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router zipkin: enabled: true diff --git a/apollo-router/src/configuration/testdata/zipkin-url_env.router.yaml b/apollo-router/src/configuration/testdata/zipkin-url_env.router.yaml index 548cdef2b7..571b8bcc55 100644 --- a/apollo-router/src/configuration/testdata/zipkin-url_env.router.yaml +++ b/apollo-router/src/configuration/testdata/zipkin-url_env.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router zipkin: enabled: true diff --git a/apollo-router/src/configuration/testdata/zipkin.router.yaml b/apollo-router/src/configuration/testdata/zipkin.router.yaml index b042bd602e..44cd6878eb 100644 --- a/apollo-router/src/configuration/testdata/zipkin.router.yaml +++ b/apollo-router/src/configuration/testdata/zipkin.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router zipkin: enabled: true diff --git a/apollo-router/src/plugins/telemetry/config.rs b/apollo-router/src/plugins/telemetry/config.rs index a197085e45..08c2574d36 100644 --- a/apollo-router/src/plugins/telemetry/config.rs +++ b/apollo-router/src/plugins/telemetry/config.rs @@ -140,7 +140,7 @@ pub(crate) struct Tracing { /// Propagation configuration pub(crate) propagation: Propagation, /// Common configuration - pub(crate) trace_config: Trace, + pub(crate) common: Trace, /// OpenTelemetry native exporter configuration pub(crate) otlp: otlp::Config, /// Jaeger exporter configuration @@ -569,23 +569,23 @@ impl From for opentelemetry::sdk::trace::Sampler { impl From<&Trace> for opentelemetry::sdk::trace::Config { fn from(config: &Trace) -> Self { - let mut trace_config = opentelemetry::sdk::trace::config(); + let mut common = opentelemetry::sdk::trace::config(); let mut sampler: opentelemetry::sdk::trace::Sampler = config.sampler.clone().into(); if config.parent_based_sampler { sampler = parent_based(sampler); } - trace_config = trace_config.with_sampler(sampler); - trace_config = trace_config.with_max_events_per_span(config.max_events_per_span); - trace_config = trace_config.with_max_attributes_per_span(config.max_attributes_per_span); - trace_config = trace_config.with_max_links_per_span(config.max_links_per_span); - trace_config = trace_config.with_max_attributes_per_event(config.max_attributes_per_event); - trace_config = trace_config.with_max_attributes_per_link(config.max_attributes_per_link); + common = common.with_sampler(sampler); + common = common.with_max_events_per_span(config.max_events_per_span); + common = common.with_max_attributes_per_span(config.max_attributes_per_span); + common = common.with_max_links_per_span(config.max_links_per_span); + common = common.with_max_attributes_per_event(config.max_attributes_per_event); + common = common.with_max_attributes_per_link(config.max_attributes_per_link); // Take the default first, then config, then env resources, then env variable. Last entry wins - trace_config = trace_config.with_resource(config.to_resource()); - trace_config + common = common.with_resource(config.to_resource()); + common } } @@ -597,7 +597,7 @@ impl Conf { pub(crate) fn calculate_field_level_instrumentation_ratio(&self) -> Result { Ok( match ( - &self.tracing.trace_config.sampler, + &self.tracing.common.sampler, &self.apollo.field_level_instrumentation_sampler, ) { // Error conditions diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index cb8eb2fdde..89ca63c67f 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -624,20 +624,20 @@ impl Telemetry { config: &config::Conf, ) -> Result<(SamplerOption, opentelemetry::sdk::trace::TracerProvider), BoxError> { let tracing_config = &config.tracing; - let mut trace_config = tracing_config.trace_config.clone(); - let mut sampler = trace_config.sampler.clone(); + let mut common = tracing_config.common.clone(); + let mut sampler = common.sampler.clone(); // set it to AlwaysOn: it is now done in the SamplingFilter, so whatever is sent to an exporter // should be accepted - trace_config.sampler = SamplerOption::Always(Sampler::AlwaysOn); + common.sampler = SamplerOption::Always(Sampler::AlwaysOn); - let mut builder = opentelemetry::sdk::trace::TracerProvider::builder() - .with_config((&trace_config).into()); + let mut builder = + opentelemetry::sdk::trace::TracerProvider::builder().with_config((&common).into()); - builder = setup_tracing(builder, &tracing_config.jaeger, &trace_config)?; - builder = setup_tracing(builder, &tracing_config.zipkin, &trace_config)?; - builder = setup_tracing(builder, &tracing_config.datadog, &trace_config)?; - builder = setup_tracing(builder, &tracing_config.otlp, &trace_config)?; - builder = setup_tracing(builder, &config.apollo, &trace_config)?; + builder = setup_tracing(builder, &tracing_config.jaeger, &common)?; + builder = setup_tracing(builder, &tracing_config.zipkin, &common)?; + builder = setup_tracing(builder, &tracing_config.datadog, &common)?; + builder = setup_tracing(builder, &tracing_config.otlp, &common)?; + builder = setup_tracing(builder, &config.apollo, &common)?; if !tracing_config.jaeger.enabled() && !tracing_config.zipkin.enabled() diff --git a/apollo-router/src/plugins/telemetry/testdata/config.router.yaml b/apollo-router/src/plugins/telemetry/testdata/config.router.yaml index 598d8d80ed..f7c0bc5566 100644 --- a/apollo-router/src/plugins/telemetry/testdata/config.router.yaml +++ b/apollo-router/src/plugins/telemetry/testdata/config.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router attributes: str: a diff --git a/apollo-router/src/plugins/telemetry/tracing/apollo.rs b/apollo-router/src/plugins/telemetry/tracing/apollo.rs index 19538cdd04..d2041aa381 100644 --- a/apollo-router/src/plugins/telemetry/tracing/apollo.rs +++ b/apollo-router/src/plugins/telemetry/tracing/apollo.rs @@ -15,7 +15,7 @@ impl TracingConfigurator for Config { self.apollo_key.is_some() && self.apollo_graph_ref.is_some() } - fn apply(&self, builder: Builder, _trace_config: &config::Trace) -> Result { + fn apply(&self, builder: Builder, _common: &config::Trace) -> Result { tracing::debug!("configuring Apollo tracing"); let exporter = apollo_telemetry::Exporter::builder() .endpoint(&self.endpoint) diff --git a/apollo-router/src/plugins/telemetry/tracing/datadog.rs b/apollo-router/src/plugins/telemetry/tracing/datadog.rs index aab2120ef6..ad761bb18b 100644 --- a/apollo-router/src/plugins/telemetry/tracing/datadog.rs +++ b/apollo-router/src/plugins/telemetry/tracing/datadog.rs @@ -62,7 +62,7 @@ impl TracingConfigurator for Config { fn apply(&self, builder: Builder, trace: &Trace) -> Result { tracing::info!("Configuring Datadog tracing: {}", self.batch_processor); let enable_span_mapping = self.enable_span_mapping.then_some(true); - let trace_config: sdk::trace::Config = trace.into(); + let common: sdk::trace::Config = trace.into(); let exporter = opentelemetry_datadog::new_pipeline() .with(&self.endpoint.to_uri(&DEFAULT_ENDPOINT), |builder, e| { builder.with_agent_endpoint(e.to_string().trim_end_matches('/')) @@ -82,7 +82,7 @@ impl TracingConfigurator for Config { }) }) .with( - &trace_config.resource.get(SERVICE_NAME), + &common.resource.get(SERVICE_NAME), |builder, service_name| { // Datadog exporter incorrectly ignores the service name in the resource // Set it explicitly here @@ -90,13 +90,13 @@ impl TracingConfigurator for Config { }, ) .with_version( - trace_config + common .resource .get(SERVICE_VERSION) .expect("cargo version is set as a resource default;qed") .to_string(), ) - .with_trace_config(trace_config) + .with_trace_config(common) .build_exporter()?; Ok(builder.with_span_processor( BatchSpanProcessor::builder(exporter, opentelemetry::runtime::Tokio) diff --git a/apollo-router/src/plugins/telemetry/tracing/jaeger.rs b/apollo-router/src/plugins/telemetry/tracing/jaeger.rs index b06ab7472a..c5c35f0a8d 100644 --- a/apollo-router/src/plugins/telemetry/tracing/jaeger.rs +++ b/apollo-router/src/plugins/telemetry/tracing/jaeger.rs @@ -86,7 +86,7 @@ impl TracingConfigurator for Config { ) } - fn apply(&self, builder: Builder, trace_config: &Trace) -> Result { + fn apply(&self, builder: Builder, common: &Trace) -> Result { match &self { Config::Agent { enabled, @@ -95,7 +95,7 @@ impl TracingConfigurator for Config { } if *enabled => { tracing::info!("Configuring Jaeger tracing: {} (agent)", batch_processor); let exporter = opentelemetry_jaeger::new_agent_pipeline() - .with_trace_config(trace_config.into()) + .with_trace_config(common.into()) .with(&agent.endpoint.to_socket(), |b, s| b.with_endpoint(s)) .build_async_agent_exporter(opentelemetry::runtime::Tokio)?; Ok(builder.with_span_processor( @@ -116,7 +116,7 @@ impl TracingConfigurator for Config { ); let exporter = opentelemetry_jaeger::new_collector_pipeline() - .with_trace_config(trace_config.into()) + .with_trace_config(common.into()) .with(&collector.username, |b, u| b.with_username(u)) .with(&collector.password, |b, p| b.with_password(p)) .with( diff --git a/apollo-router/src/plugins/telemetry/tracing/mod.rs b/apollo-router/src/plugins/telemetry/tracing/mod.rs index 8300d15f73..eedb32d13f 100644 --- a/apollo-router/src/plugins/telemetry/tracing/mod.rs +++ b/apollo-router/src/plugins/telemetry/tracing/mod.rs @@ -27,7 +27,7 @@ pub(crate) mod zipkin; pub(crate) trait TracingConfigurator { fn enabled(&self) -> bool; - fn apply(&self, builder: Builder, trace_config: &Trace) -> Result; + fn apply(&self, builder: Builder, common: &Trace) -> Result; } #[derive(Debug)] diff --git a/apollo-router/src/plugins/telemetry/tracing/otlp.rs b/apollo-router/src/plugins/telemetry/tracing/otlp.rs index 79e23271d4..d7fba0a9ba 100644 --- a/apollo-router/src/plugins/telemetry/tracing/otlp.rs +++ b/apollo-router/src/plugins/telemetry/tracing/otlp.rs @@ -15,7 +15,7 @@ impl TracingConfigurator for super::super::otlp::Config { self.enabled } - fn apply(&self, builder: Builder, _trace_config: &Trace) -> Result { + fn apply(&self, builder: Builder, _common: &Trace) -> Result { tracing::info!("Configuring Otlp tracing: {}", self.batch_processor); let exporter: SpanExporterBuilder = self.exporter()?; Ok(builder.with_span_processor( diff --git a/apollo-router/src/plugins/telemetry/tracing/zipkin.rs b/apollo-router/src/plugins/telemetry/tracing/zipkin.rs index e34632d8c3..381afc2376 100644 --- a/apollo-router/src/plugins/telemetry/tracing/zipkin.rs +++ b/apollo-router/src/plugins/telemetry/tracing/zipkin.rs @@ -38,11 +38,11 @@ impl TracingConfigurator for Config { self.enabled } - fn apply(&self, builder: Builder, trace_config: &Trace) -> Result { + fn apply(&self, builder: Builder, common: &Trace) -> Result { tracing::info!("configuring Zipkin tracing: {}", self.batch_processor); let exporter = opentelemetry_zipkin::new_pipeline() - .with_trace_config(trace_config.into()) + .with_trace_config(common.into()) .with(&self.endpoint.to_uri(&DEFAULT_ENDPOINT), |b, endpoint| { b.with_collector_endpoint(endpoint.to_string()) }) diff --git a/apollo-router/src/testdata/datadog.router.yaml b/apollo-router/src/testdata/datadog.router.yaml index 26790f8d37..17c3c72986 100644 --- a/apollo-router/src/testdata/datadog.router.yaml +++ b/apollo-router/src/testdata/datadog.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router datadog: enabled: true diff --git a/apollo-router/src/testdata/jaeger.router.yaml b/apollo-router/src/testdata/jaeger.router.yaml index 8fbdf4c7af..2140731a35 100644 --- a/apollo-router/src/testdata/jaeger.router.yaml +++ b/apollo-router/src/testdata/jaeger.router.yaml @@ -10,7 +10,7 @@ telemetry: header_name: apollo-custom-trace-id propagation: jaeger: true - trace_config: + common: service_name: router jaeger: enabled: true diff --git a/apollo-router/src/testdata/otlp.router.yaml b/apollo-router/src/testdata/otlp.router.yaml index 3550c97644..bbde177920 100644 --- a/apollo-router/src/testdata/otlp.router.yaml +++ b/apollo-router/src/testdata/otlp.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router otlp: enabled: true diff --git a/apollo-router/src/testdata/zipkin.router.yaml b/apollo-router/src/testdata/zipkin.router.yaml index b042bd602e..44cd6878eb 100644 --- a/apollo-router/src/testdata/zipkin.router.yaml +++ b/apollo-router/src/testdata/zipkin.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router zipkin: enabled: true diff --git a/apollo-router/tests/fixtures/apollo_reports.batch_router.yaml b/apollo-router/tests/fixtures/apollo_reports.batch_router.yaml index bf22811ba2..7fdf278a8b 100644 --- a/apollo-router/tests/fixtures/apollo_reports.batch_router.yaml +++ b/apollo-router/tests/fixtures/apollo_reports.batch_router.yaml @@ -8,7 +8,7 @@ telemetry: experimental_response_trace_id: enabled: true header_name: "my_trace_id" - trace_config: + common: sampler: always_on apollo: diff --git a/apollo-router/tests/fixtures/apollo_reports.router.yaml b/apollo-router/tests/fixtures/apollo_reports.router.yaml index 0e0a2b5f03..f7e1ee724d 100644 --- a/apollo-router/tests/fixtures/apollo_reports.router.yaml +++ b/apollo-router/tests/fixtures/apollo_reports.router.yaml @@ -5,7 +5,7 @@ telemetry: experimental_response_trace_id: enabled: true header_name: "my_trace_id" - trace_config: + common: sampler: always_on apollo: diff --git a/apollo-router/tests/fixtures/broken_plugin.router.yaml b/apollo-router/tests/fixtures/broken_plugin.router.yaml index 784b4481b0..a8b0cc4821 100644 --- a/apollo-router/tests/fixtures/broken_plugin.router.yaml +++ b/apollo-router/tests/fixtures/broken_plugin.router.yaml @@ -10,7 +10,7 @@ telemetry: header_name: apollo-custom-trace-id propagation: jaeger: true - trace_config: + common: service_name: router jaeger: enabled: true diff --git a/apollo-router/tests/fixtures/datadog.router.yaml b/apollo-router/tests/fixtures/datadog.router.yaml index 26790f8d37..17c3c72986 100644 --- a/apollo-router/tests/fixtures/datadog.router.yaml +++ b/apollo-router/tests/fixtures/datadog.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router datadog: enabled: true diff --git a/apollo-router/tests/fixtures/jaeger-0.5-sample.router.yaml b/apollo-router/tests/fixtures/jaeger-0.5-sample.router.yaml index b7c7a0dfcd..b0c0273633 100644 --- a/apollo-router/tests/fixtures/jaeger-0.5-sample.router.yaml +++ b/apollo-router/tests/fixtures/jaeger-0.5-sample.router.yaml @@ -5,7 +5,7 @@ telemetry: header_name: apollo-custom-trace-id propagation: jaeger: true - trace_config: + common: service_name: router sampler: 0.5 jaeger: diff --git a/apollo-router/tests/fixtures/jaeger-no-sample.router.yaml b/apollo-router/tests/fixtures/jaeger-no-sample.router.yaml index 6ec3390e23..ca676ec379 100644 --- a/apollo-router/tests/fixtures/jaeger-no-sample.router.yaml +++ b/apollo-router/tests/fixtures/jaeger-no-sample.router.yaml @@ -8,7 +8,7 @@ telemetry: header_name: apollo-custom-trace-id propagation: jaeger: true - trace_config: + common: service_name: router sampler: always_off jaeger: diff --git a/apollo-router/tests/fixtures/jaeger.router.yaml b/apollo-router/tests/fixtures/jaeger.router.yaml index e5ee9ba59e..2d916776f2 100644 --- a/apollo-router/tests/fixtures/jaeger.router.yaml +++ b/apollo-router/tests/fixtures/jaeger.router.yaml @@ -5,7 +5,7 @@ telemetry: header_name: apollo-custom-trace-id propagation: jaeger: true - trace_config: + common: service_name: router sampler: always_on jaeger: diff --git a/apollo-router/tests/fixtures/no-telemetry.router.yaml b/apollo-router/tests/fixtures/no-telemetry.router.yaml index c8219e505b..99629baae5 100644 --- a/apollo-router/tests/fixtures/no-telemetry.router.yaml +++ b/apollo-router/tests/fixtures/no-telemetry.router.yaml @@ -3,7 +3,7 @@ telemetry: experimental_response_trace_id: enabled: true header_name: apollo-custom-trace-id - trace_config: + common: service_name: router sampler: always_on experimental_logging: @@ -19,4 +19,4 @@ telemetry: override_subgraph_url: products: http://localhost:4005 include_subgraph_errors: - all: true \ No newline at end of file + all: true diff --git a/apollo-router/tests/fixtures/otlp.router.yaml b/apollo-router/tests/fixtures/otlp.router.yaml index 3550c97644..bbde177920 100644 --- a/apollo-router/tests/fixtures/otlp.router.yaml +++ b/apollo-router/tests/fixtures/otlp.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router otlp: enabled: true diff --git a/apollo-router/tests/fixtures/zipkin.router.yaml b/apollo-router/tests/fixtures/zipkin.router.yaml index b042bd602e..44cd6878eb 100644 --- a/apollo-router/tests/fixtures/zipkin.router.yaml +++ b/apollo-router/tests/fixtures/zipkin.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router zipkin: enabled: true diff --git a/apollo-router/tests/telemetry_test.rs b/apollo-router/tests/telemetry_test.rs index d8dcb6966f..af32e8482e 100644 --- a/apollo-router/tests/telemetry_test.rs +++ b/apollo-router/tests/telemetry_test.rs @@ -17,7 +17,7 @@ async fn test_telemetry_doesnt_hang_with_invalid_schema() { r#" telemetry: tracing: - trace_config: + common: service_name: router otlp: endpoint: default diff --git a/dockerfiles/tracing/router/datadog.router.yaml b/dockerfiles/tracing/router/datadog.router.yaml index edaaaf6417..0a43a7c298 100644 --- a/dockerfiles/tracing/router/datadog.router.yaml +++ b/dockerfiles/tracing/router/datadog.router.yaml @@ -6,7 +6,7 @@ cors: telemetry: tracing: - trace_config: + common: service_name: router datadog: enabled: true diff --git a/dockerfiles/tracing/router/jaeger.router.yaml b/dockerfiles/tracing/router/jaeger.router.yaml index 8f2065830e..9a2cda75f8 100644 --- a/dockerfiles/tracing/router/jaeger.router.yaml +++ b/dockerfiles/tracing/router/jaeger.router.yaml @@ -6,7 +6,7 @@ cors: telemetry: tracing: - trace_config: + common: service_name: router jaeger: enabled: true diff --git a/dockerfiles/tracing/router/zipkin.router.yaml b/dockerfiles/tracing/router/zipkin.router.yaml index e76b3a134e..defe9d86f8 100644 --- a/dockerfiles/tracing/router/zipkin.router.yaml +++ b/dockerfiles/tracing/router/zipkin.router.yaml @@ -6,7 +6,7 @@ cors: telemetry: tracing: - trace_config: + common: service_name: router zipkin: enabled: true diff --git a/docs/source/configuration/apollo-telemetry.mdx b/docs/source/configuration/apollo-telemetry.mdx index 19a71c3a28..900615bafd 100644 --- a/docs/source/configuration/apollo-telemetry.mdx +++ b/docs/source/configuration/apollo-telemetry.mdx @@ -46,17 +46,17 @@ You can customize your router's trace sampling rate by setting the following opt telemetry: apollo: # In this example, the router will request traces for 50% of requests. - # This value can't exceed the value of tracing.trace_config.sampler. + # This value can't exceed the value of tracing.common.sampler. field_level_instrumentation_sampler: 0.5 tracing: - trace_config: + common: # FTV1 uses the same trace sampling as other tracing options, # so this value is also required. sampler: 0.5 ``` -> **Note:** Because field-level instrumentation is dependent on general-purpose [OpenTelemetry tracing](./tracing/), the value of `apollo.field_level_instrumentation_sampler` cannot exceed the value of `tracing.trace_config.sampler`. +> **Note:** Because field-level instrumentation is dependent on general-purpose [OpenTelemetry tracing](./tracing/), the value of `apollo.field_level_instrumentation_sampler` cannot exceed the value of `tracing.common.sampler`. ### Disabling field-level traces @@ -278,7 +278,7 @@ telemetry: apollo: # The percentage of requests will include HTTP request and response headers in traces sent to Apollo Studio. # This is expensive and should be left at a low value. - # This cannot be higher than tracing->trace_config->sampler + # This cannot be higher than tracing->common->sampler field_level_instrumentation_sampler: 0.01 # (default) # Include HTTP request and response headers in traces sent to Apollo Studio @@ -291,7 +291,7 @@ telemetry: except: # Send all variable values except for variable named first - first tracing: - trace_config: + common: sampler: 0.5 # The percentage of requests that will generate traces (a rate or `always_on` or `always_off`) ``` diff --git a/docs/source/configuration/metrics.mdx b/docs/source/configuration/metrics.mdx index e017f80985..4eec93aeac 100644 --- a/docs/source/configuration/metrics.mdx +++ b/docs/source/configuration/metrics.mdx @@ -31,7 +31,7 @@ A Resource is a set of key-value pairs that provide additional information to an ```yaml title="router.yaml" telemetry: tracing: - trace_config: + common: attributes: "environment.name": "production" "environment.namespace": "{env.MY_K8_NAMESPACE_ENV_VARIABLE}" diff --git a/docs/source/configuration/tracing.mdx b/docs/source/configuration/tracing.mdx index 07827534fd..c7a51479a7 100644 --- a/docs/source/configuration/tracing.mdx +++ b/docs/source/configuration/tracing.mdx @@ -29,7 +29,7 @@ Span data is sent to a collector such as [Jaeger](https://www.jaegertracing.io/) ```yaml title="router.yaml" telemetry: tracing: - trace_config: + common: # (Optional) Set the service name to easily find traces related to the apollo-router in your metrics dashboards service_name: "router" ``` @@ -49,7 +49,7 @@ A Resource is a set of key-value pairs that provide additional global informatio ```yaml title="router.yaml" telemetry: tracing: - trace_config: + common: attributes: "environment.name": "production" "environment.namespace": "{env.MY_K8_NAMESPACE_ENV_VARIABLE}" @@ -64,7 +64,7 @@ To prevent sending to many traces to your APM you may want to enable sampling. ```yaml title="router.yaml" telemetry: tracing: - trace_config: + common: sampler: always_on # (default) all requests are sampled (always_on|always_off|<0.0-1.0>) parent_based_sampler: true # (default) If an incoming span has OpenTelemetry headers then the request will always be sampled. ``` @@ -113,7 +113,7 @@ You may set limits on spans to prevent sending too much data to your APM. ```yaml title="router.yaml" telemetry: tracing: - trace_config: + common: # Optional limits max_attributes_per_event: 10 diff --git a/examples/telemetry/datadog.router.yaml b/examples/telemetry/datadog.router.yaml index 26790f8d37..17c3c72986 100644 --- a/examples/telemetry/datadog.router.yaml +++ b/examples/telemetry/datadog.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router datadog: enabled: true diff --git a/examples/telemetry/jaeger-agent.router.yaml b/examples/telemetry/jaeger-agent.router.yaml index 365ecb376a..0121d6f1cd 100644 --- a/examples/telemetry/jaeger-agent.router.yaml +++ b/examples/telemetry/jaeger-agent.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router jaeger: enabled: true diff --git a/examples/telemetry/jaeger-collector.router.yaml b/examples/telemetry/jaeger-collector.router.yaml index a31a69b854..6c170ec617 100644 --- a/examples/telemetry/jaeger-collector.router.yaml +++ b/examples/telemetry/jaeger-collector.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router jaeger: enabled: true diff --git a/examples/telemetry/otlp.router.yaml b/examples/telemetry/otlp.router.yaml index 3550c97644..bbde177920 100644 --- a/examples/telemetry/otlp.router.yaml +++ b/examples/telemetry/otlp.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router otlp: enabled: true diff --git a/examples/telemetry/zipkin-agent.router.yaml b/examples/telemetry/zipkin-agent.router.yaml index b042bd602e..44cd6878eb 100644 --- a/examples/telemetry/zipkin-agent.router.yaml +++ b/examples/telemetry/zipkin-agent.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router zipkin: enabled: true diff --git a/examples/telemetry/zipkin-collector.router.yaml b/examples/telemetry/zipkin-collector.router.yaml index 7335367285..90301c3ff3 100644 --- a/examples/telemetry/zipkin-collector.router.yaml +++ b/examples/telemetry/zipkin-collector.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - trace_config: + common: service_name: router zipkin: enabled: true From 53b8c08ad33ae19a9ca344e0d920bbce7c1ced96 Mon Sep 17 00:00:00 2001 From: bryn Date: Tue, 17 Oct 2023 13:17:06 +0100 Subject: [PATCH 2/2] Swap ordering of migrations, I edited the wrong files. --- .../0013-telemetry-exporters-enabled.yaml | 25 ++++++++++++++++--- .../0014-telemetry_trace_config_common.yaml | 25 +++---------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/apollo-router/src/configuration/migrations/0013-telemetry-exporters-enabled.yaml b/apollo-router/src/configuration/migrations/0013-telemetry-exporters-enabled.yaml index cfdbc456b6..606a63551e 100644 --- a/apollo-router/src/configuration/migrations/0013-telemetry-exporters-enabled.yaml +++ b/apollo-router/src/configuration/migrations/0013-telemetry-exporters-enabled.yaml @@ -1,5 +1,22 @@ -description: telemetry.tracing.trace_config has been renamed to telemetry.tracing.common +description: Telemetry exporters now have an enabled field actions: - - type: move - from: telemetry.tracing.trace_config - to: telemetry.tracing.common + - type: add + path: telemetry.tracing.jaeger + name: enabled + value: true + - type: add + path: telemetry.tracing.otlp + name: enabled + value: true + - type: add + path: telemetry.tracing.zipkin + name: enabled + value: true + - type: add + path: telemetry.tracing.datadog + name: enabled + value: true + - type: add + path: telemetry.metrics.otlp + name: enabled + value: true diff --git a/apollo-router/src/configuration/migrations/0014-telemetry_trace_config_common.yaml b/apollo-router/src/configuration/migrations/0014-telemetry_trace_config_common.yaml index 606a63551e..cfdbc456b6 100644 --- a/apollo-router/src/configuration/migrations/0014-telemetry_trace_config_common.yaml +++ b/apollo-router/src/configuration/migrations/0014-telemetry_trace_config_common.yaml @@ -1,22 +1,5 @@ -description: Telemetry exporters now have an enabled field +description: telemetry.tracing.trace_config has been renamed to telemetry.tracing.common actions: - - type: add - path: telemetry.tracing.jaeger - name: enabled - value: true - - type: add - path: telemetry.tracing.otlp - name: enabled - value: true - - type: add - path: telemetry.tracing.zipkin - name: enabled - value: true - - type: add - path: telemetry.tracing.datadog - name: enabled - value: true - - type: add - path: telemetry.metrics.otlp - name: enabled - value: true + - type: move + from: telemetry.tracing.trace_config + to: telemetry.tracing.common