Skip to content

Commit b890c1e

Browse files
rohan-b99rohan-b99
authored
Rohan b99/datadog propagator should be exclusively active (warn if not) (#8677)
1 parent 49ecbde commit b890c1e

File tree

6 files changed

+341
-8
lines changed

6 files changed

+341
-8
lines changed

‎.changesets/maint_rohan_b99_datadog_propagator_must_be_exclusively_active.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
### Warn if datadog propagator is not exclusively active ([PR #8677](https://github.com/apollographql/router/pull/8677))
2+
3+
Adds validation for propagator configuration, ensuring that a warning log is emitted if:
4+
- The datadog propagator is enabled, and any other propagators are enabled (except baggage)
5+
- Datadog tracing is enabled and other propagators are enabled (except for baggage)
6+
7+
By [@rohan-b99](https://github.com/rohan-b99) in https://github.com/apollographql/router/pull/8677

‎apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7366,9 +7366,12 @@ expression: "&schema"
73667366
"type": "boolean"
73677367
},
73687368
"datadog": {
7369-
"default": false,
7369+
"default": null,
73707370
"description": "Propagate Datadog",
7371-
"type": "boolean"
7371+
"type": [
7372+
"boolean",
7373+
"null"
7374+
]
73727375
},
73737376
"jaeger": {
73747377
"default": false,

‎apollo-router/src/plugins/telemetry/config.rs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,32 @@ pub(crate) struct Tracing {
225225
pub(crate) datadog: tracing::datadog::Config,
226226
}
227227

228+
impl Tracing {
229+
pub(crate) fn is_baggage_propagation_enabled(&self) -> bool {
230+
self.propagation.baggage
231+
}
232+
233+
pub(crate) fn is_trace_context_propagation_enabled(&self) -> bool {
234+
self.propagation.trace_context || self.otlp.enabled
235+
}
236+
237+
pub(crate) fn is_jaeger_propagation_enabled(&self) -> bool {
238+
self.propagation.jaeger
239+
}
240+
241+
pub(crate) fn is_datadog_propagation_enabled(&self) -> bool {
242+
self.propagation.datadog.unwrap_or(false) || self.datadog.enabled
243+
}
244+
245+
pub(crate) fn is_zipkin_propagation_enabled(&self) -> bool {
246+
self.propagation.zipkin || self.zipkin.enabled
247+
}
248+
249+
pub(crate) fn is_aws_xray_propagation_enabled(&self) -> bool {
250+
self.propagation.aws_xray
251+
}
252+
}
253+
228254
#[derive(Clone, Default, Debug, Deserialize, JsonSchema, PartialEq)]
229255
#[serde(deny_unknown_fields, default)]
230256
pub(crate) struct ExposeTraceId {
@@ -314,7 +340,7 @@ pub(crate) struct Propagation {
314340
/// Propagate Jaeger
315341
pub(crate) jaeger: bool,
316342
/// Propagate Datadog
317-
pub(crate) datadog: bool,
343+
pub(crate) datadog: Option<bool>,
318344
/// Propagate Zipkin
319345
pub(crate) zipkin: bool,
320346
/// Propagate AWS X-Ray

‎apollo-router/src/plugins/telemetry/reload/builder.rs‎

Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ mod tests {
245245
use crate::plugins::telemetry::config::Exporters;
246246
use crate::plugins::telemetry::config::Instrumentation;
247247
use crate::plugins::telemetry::config::Metrics;
248+
use crate::plugins::telemetry::config::Propagation;
248249
use crate::plugins::telemetry::config::Tracing;
249250

250251
fn create_default_config() -> Conf {
@@ -626,4 +627,273 @@ mod tests {
626627
"Tracer provider should reload when span limits change"
627628
);
628629
}
630+
631+
#[tokio::test(flavor = "multi_thread")]
632+
async fn test_datadog_propagation_only_passes() {
633+
let mut config = create_config_with_apollo_enabled();
634+
config.exporters.tracing.propagation = Propagation {
635+
datadog: Some(true),
636+
..Default::default()
637+
};
638+
639+
let builder = Builder::new(&None, &config);
640+
assert!(builder.build().is_ok());
641+
}
642+
643+
#[tokio::test(flavor = "multi_thread")]
644+
async fn test_datadog_with_baggage_propagation_passes() {
645+
let mut config = create_config_with_apollo_enabled();
646+
config.exporters.tracing.propagation = Propagation {
647+
datadog: Some(true),
648+
baggage: true,
649+
..Default::default()
650+
};
651+
652+
let builder = Builder::new(&None, &config);
653+
assert!(builder.build().is_ok());
654+
}
655+
656+
#[tokio::test(flavor = "multi_thread")]
657+
async fn test_jaeger_propagation_only_passes() {
658+
let mut config = create_config_with_apollo_enabled();
659+
config.exporters.tracing.propagation = Propagation {
660+
jaeger: true,
661+
..Default::default()
662+
};
663+
664+
let builder = Builder::new(&None, &config);
665+
assert!(builder.build().is_ok());
666+
}
667+
668+
#[tokio::test(flavor = "multi_thread")]
669+
async fn test_datadog_with_jaeger_propagation_fails() {
670+
use crate::test_harness::tracing_test;
671+
let _guard = tracing_test::dispatcher_guard();
672+
let mut config = create_config_with_apollo_enabled();
673+
config.exporters.tracing.propagation = Propagation {
674+
datadog: Some(true),
675+
jaeger: true,
676+
..Default::default()
677+
};
678+
679+
let builder = Builder::new(&None, &config);
680+
assert!(builder.build().is_ok());
681+
assert!(tracing_test::logs_contain(
682+
"datadog propagation should not be used with any other propagator except for baggage to avoid trace id conflicts",
683+
));
684+
}
685+
686+
#[tokio::test(flavor = "multi_thread")]
687+
async fn test_datadog_with_trace_context_propagation_fails() {
688+
use crate::test_harness::tracing_test;
689+
let _guard = tracing_test::dispatcher_guard();
690+
let mut config = create_config_with_apollo_enabled();
691+
config.exporters.tracing.propagation = Propagation {
692+
datadog: Some(true),
693+
trace_context: true,
694+
..Default::default()
695+
};
696+
697+
let builder = Builder::new(&None, &config);
698+
assert!(builder.build().is_ok());
699+
assert!(tracing_test::logs_contain(
700+
"datadog propagation should not be used with any other propagator except for baggage to avoid trace id conflicts",
701+
));
702+
}
703+
704+
#[tokio::test(flavor = "multi_thread")]
705+
async fn test_datadog_with_zipkin_propagation_fails() {
706+
use crate::test_harness::tracing_test;
707+
let _guard = tracing_test::dispatcher_guard();
708+
let mut config = create_config_with_apollo_enabled();
709+
config.exporters.tracing.propagation = Propagation {
710+
datadog: Some(true),
711+
zipkin: true,
712+
..Default::default()
713+
};
714+
715+
let builder = Builder::new(&None, &config);
716+
assert!(builder.build().is_ok());
717+
assert!(tracing_test::logs_contain(
718+
"datadog propagation should not be used with any other propagator except for baggage to avoid trace id conflicts",
719+
));
720+
}
721+
722+
#[tokio::test(flavor = "multi_thread")]
723+
async fn test_datadog_with_aws_xray_propagation_fails() {
724+
use crate::test_harness::tracing_test;
725+
let _guard = tracing_test::dispatcher_guard();
726+
let mut config = create_config_with_apollo_enabled();
727+
config.exporters.tracing.propagation = Propagation {
728+
datadog: Some(true),
729+
aws_xray: true,
730+
..Default::default()
731+
};
732+
733+
let builder = Builder::new(&None, &config);
734+
assert!(builder.build().is_ok());
735+
assert!(tracing_test::logs_contain(
736+
"datadog propagation should not be used with any other propagator except for baggage to avoid trace id conflicts",
737+
));
738+
}
739+
740+
#[tokio::test(flavor = "multi_thread")]
741+
async fn test_datadog_exporter_enabled_with_datadog_propagation_only_passes() {
742+
use crate::test_harness::tracing_test;
743+
let _guard = tracing_test::dispatcher_guard();
744+
let mut config = create_config_with_apollo_enabled();
745+
config.exporters.tracing.datadog.enabled = true;
746+
config.exporters.tracing.propagation = Propagation {
747+
datadog: Some(true),
748+
..Default::default()
749+
};
750+
751+
let builder = Builder::new(&None, &config);
752+
assert!(builder.build().is_ok());
753+
}
754+
755+
#[tokio::test(flavor = "multi_thread")]
756+
async fn test_datadog_exporter_enabled_with_datadog_baggage_propagation_passes() {
757+
use crate::test_harness::tracing_test;
758+
let _guard = tracing_test::dispatcher_guard();
759+
let mut config = create_config_with_apollo_enabled();
760+
config.exporters.tracing.datadog.enabled = true;
761+
config.exporters.tracing.propagation = Propagation {
762+
datadog: Some(true),
763+
baggage: true,
764+
..Default::default()
765+
};
766+
767+
let builder = Builder::new(&None, &config);
768+
assert!(builder.build().is_ok());
769+
}
770+
771+
#[tokio::test(flavor = "multi_thread")]
772+
async fn test_datadog_exporter_enabled_with_jaeger_propagation_only_fails() {
773+
use crate::test_harness::tracing_test;
774+
let _guard = tracing_test::dispatcher_guard();
775+
let mut config = create_config_with_apollo_enabled();
776+
config.exporters.tracing.datadog.enabled = true;
777+
config.exporters.tracing.propagation = Propagation {
778+
jaeger: true,
779+
..Default::default()
780+
};
781+
782+
let builder = Builder::new(&None, &config);
783+
assert!(builder.build().is_ok());
784+
assert!(tracing_test::logs_contain(
785+
"datadog propagation should be explicitly disabled if the datadog exporter is enabled and any propagator other than baggage is enabled to avoid trace id conflicts",
786+
));
787+
}
788+
789+
#[tokio::test(flavor = "multi_thread")]
790+
async fn test_datadog_exporter_enabled_with_datadog_jaeger_propagation_fails() {
791+
use crate::test_harness::tracing_test;
792+
let _guard = tracing_test::dispatcher_guard();
793+
let mut config = create_config_with_apollo_enabled();
794+
config.exporters.tracing.datadog.enabled = true;
795+
config.exporters.tracing.propagation = Propagation {
796+
datadog: Some(true),
797+
jaeger: true,
798+
..Default::default()
799+
};
800+
801+
let builder = Builder::new(&None, &config);
802+
assert!(builder.build().is_ok());
803+
assert!(tracing_test::logs_contain(
804+
"if the datadog exporter is enabled and any other propagator except for baggage is enabled, the datadog propagator should be disabled to avoid trace id conflicts",
805+
));
806+
}
807+
808+
#[tokio::test(flavor = "multi_thread")]
809+
async fn test_datadog_exporter_enabled_with_datadog_trace_context_propagation_fails() {
810+
use crate::test_harness::tracing_test;
811+
let _guard = tracing_test::dispatcher_guard();
812+
let mut config = create_config_with_apollo_enabled();
813+
config.exporters.tracing.datadog.enabled = true;
814+
config.exporters.tracing.propagation = Propagation {
815+
datadog: Some(true),
816+
trace_context: true,
817+
..Default::default()
818+
};
819+
820+
let builder = Builder::new(&None, &config);
821+
assert!(builder.build().is_ok());
822+
assert!(tracing_test::logs_contain(
823+
"if the datadog exporter is enabled and any other propagator except for baggage is enabled, the datadog propagator should be disabled to avoid trace id conflicts",
824+
));
825+
}
826+
827+
#[tokio::test(flavor = "multi_thread")]
828+
async fn test_datadog_exporter_enabled_with_datadog_zipkin_propagation_fails() {
829+
use crate::test_harness::tracing_test;
830+
let _guard = tracing_test::dispatcher_guard();
831+
let mut config = create_config_with_apollo_enabled();
832+
config.exporters.tracing.datadog.enabled = true;
833+
config.exporters.tracing.propagation = Propagation {
834+
datadog: Some(true),
835+
zipkin: true,
836+
..Default::default()
837+
};
838+
839+
let builder = Builder::new(&None, &config);
840+
assert!(builder.build().is_ok());
841+
assert!(tracing_test::logs_contain(
842+
"if the datadog exporter is enabled and any other propagator except for baggage is enabled, the datadog propagator should be disabled to avoid trace id conflicts",
843+
));
844+
}
845+
846+
#[tokio::test(flavor = "multi_thread")]
847+
async fn test_datadog_exporter_enabled_with_datadog_aws_xray_propagation_fails() {
848+
use crate::test_harness::tracing_test;
849+
let _guard = tracing_test::dispatcher_guard();
850+
let mut config = create_config_with_apollo_enabled();
851+
config.exporters.tracing.datadog.enabled = true;
852+
config.exporters.tracing.propagation = Propagation {
853+
datadog: Some(true),
854+
aws_xray: true,
855+
..Default::default()
856+
};
857+
858+
let builder = Builder::new(&None, &config);
859+
assert!(builder.build().is_ok());
860+
assert!(tracing_test::logs_contain(
861+
"if the datadog exporter is enabled and any other propagator except for baggage is enabled, the datadog propagator should be disabled to avoid trace id conflicts",
862+
));
863+
}
864+
865+
#[tokio::test(flavor = "multi_thread")]
866+
async fn test_datadog_exporter_enabled_with_otlp_exporter_enabled_passes() {
867+
let mut config = create_config_with_apollo_enabled();
868+
config.exporters.tracing.datadog.enabled = true;
869+
config.exporters.tracing.otlp.enabled = true;
870+
871+
let builder = Builder::new(&None, &config);
872+
assert!(builder.build().is_ok());
873+
}
874+
875+
#[tokio::test(flavor = "multi_thread")]
876+
async fn test_datadog_propagation_with_otlp_exporter_enabled_passes() {
877+
let mut config = create_config_with_apollo_enabled();
878+
config.exporters.tracing.otlp.enabled = true;
879+
config.exporters.tracing.propagation.datadog = Some(true);
880+
881+
let builder = Builder::new(&None, &config);
882+
assert!(builder.build().is_ok());
883+
}
884+
885+
#[tokio::test(flavor = "multi_thread")]
886+
async fn test_datadog_exporter_enabled_with_zipkin_exporter_enabled_fails() {
887+
use crate::test_harness::tracing_test;
888+
let _guard = tracing_test::dispatcher_guard();
889+
let mut config = create_config_with_apollo_enabled();
890+
config.exporters.tracing.datadog.enabled = true;
891+
config.exporters.tracing.zipkin.enabled = true;
892+
893+
let builder = Builder::new(&None, &config);
894+
assert!(builder.build().is_ok());
895+
assert!(tracing_test::logs_contain(
896+
"datadog propagation should be explicitly disabled if the datadog exporter is enabled and any propagator other than baggage is enabled to avoid trace id conflicts",
897+
));
898+
}
629899
}

‎apollo-router/src/plugins/telemetry/reload/tracing.rs‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,40 @@ pub(crate) fn create_propagator(
8080
tracing: &Tracing,
8181
) -> TextMapCompositePropagator {
8282
let mut propagators: Vec<Box<dyn TextMapPropagator + Send + Sync + 'static>> = Vec::new();
83-
if propagation.jaeger {
83+
84+
if tracing.is_jaeger_propagation_enabled() {
8485
propagators.push(Box::<opentelemetry_jaeger_propagator::Propagator>::default());
8586
}
86-
if propagation.baggage {
87+
if tracing.is_baggage_propagation_enabled() {
8788
propagators.push(Box::<opentelemetry_sdk::propagation::BaggagePropagator>::default());
8889
}
89-
if propagation.trace_context || tracing.otlp.enabled {
90+
if tracing.is_trace_context_propagation_enabled() {
9091
propagators.push(Box::<opentelemetry_sdk::propagation::TraceContextPropagator>::default());
9192
}
92-
if propagation.zipkin || tracing.zipkin.enabled {
93+
if tracing.is_zipkin_propagation_enabled() {
9394
propagators.push(Box::<opentelemetry_zipkin::Propagator>::default());
9495
}
95-
if propagation.datadog || tracing.datadog.enabled {
96+
if tracing.is_datadog_propagation_enabled() {
97+
if tracing.is_jaeger_propagation_enabled()
98+
|| tracing.is_trace_context_propagation_enabled()
99+
|| tracing.is_zipkin_propagation_enabled()
100+
|| tracing.is_aws_xray_propagation_enabled()
101+
{
102+
if tracing.datadog.enabled && propagation.datadog.unwrap_or(false) {
103+
tracing::warn!(
104+
"if the datadog exporter is enabled and any other propagator except for baggage is enabled, the datadog propagator should be disabled to avoid trace id conflicts"
105+
);
106+
} else if let Some(true) = propagation.datadog {
107+
tracing::warn!(
108+
"datadog propagation should not be used with any other propagator except for baggage to avoid trace id conflicts"
109+
);
110+
} else if propagation.datadog.is_none() {
111+
tracing::warn!(
112+
"datadog propagation should be explicitly disabled if the datadog exporter is enabled and any propagator other than baggage is enabled to avoid trace id conflicts"
113+
);
114+
}
115+
}
116+
96117
propagators.push(Box::<
97118
crate::plugins::telemetry::tracing::datadog_exporter::DatadogPropagator,
98119
>::default());

0 commit comments

Comments
 (0)