Skip to content

Commit a5739eb

Browse files
mikeblummikeblum
authored
Spec compliance: OTEL_PROPAGATORS should still work when OTEL_SDK_DISABLED (#7062)
1 parent d16cad3 commit a5739eb

File tree

2 files changed

+218
-62
lines changed

2 files changed

+218
-62
lines changed

Diff for: ‎sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java

+66-56
Original file line numberDiff line numberDiff line change
@@ -443,65 +443,19 @@ public AutoConfiguredOpenTelemetrySdk build() {
443443
List<Closeable> closeables = new ArrayList<>();
444444

445445
try {
446-
OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder().build();
447-
boolean sdkEnabled = !config.getBoolean("otel.sdk.disabled", false);
446+
OpenTelemetrySdkBuilder sdkBuilder = OpenTelemetrySdk.builder();
447+
448+
// The propagation system is part of the API and functions in the absence of an SDK.
449+
ContextPropagators propagators =
450+
PropagatorConfiguration.configurePropagators(config, spiHelper, propagatorCustomizer);
451+
sdkBuilder.setPropagators(propagators);
448452

453+
boolean sdkEnabled = !config.getBoolean("otel.sdk.disabled", false);
449454
if (sdkEnabled) {
450-
SdkMeterProviderBuilder meterProviderBuilder = SdkMeterProvider.builder();
451-
meterProviderBuilder.setResource(resource);
452-
MeterProviderConfiguration.configureMeterProvider(
453-
meterProviderBuilder,
454-
config,
455-
spiHelper,
456-
metricReaderCustomizer,
457-
metricExporterCustomizer,
458-
closeables);
459-
meterProviderBuilder = meterProviderCustomizer.apply(meterProviderBuilder, config);
460-
SdkMeterProvider meterProvider = meterProviderBuilder.build();
461-
closeables.add(meterProvider);
462-
463-
SdkTracerProviderBuilder tracerProviderBuilder = SdkTracerProvider.builder();
464-
tracerProviderBuilder.setResource(resource);
465-
TracerProviderConfiguration.configureTracerProvider(
466-
tracerProviderBuilder,
467-
config,
468-
spiHelper,
469-
meterProvider,
470-
spanExporterCustomizer,
471-
spanProcessorCustomizer,
472-
samplerCustomizer,
473-
closeables);
474-
tracerProviderBuilder = tracerProviderCustomizer.apply(tracerProviderBuilder, config);
475-
SdkTracerProvider tracerProvider = tracerProviderBuilder.build();
476-
closeables.add(tracerProvider);
477-
478-
SdkLoggerProviderBuilder loggerProviderBuilder = SdkLoggerProvider.builder();
479-
loggerProviderBuilder.setResource(resource);
480-
LoggerProviderConfiguration.configureLoggerProvider(
481-
loggerProviderBuilder,
482-
config,
483-
spiHelper,
484-
meterProvider,
485-
logRecordExporterCustomizer,
486-
logRecordProcessorCustomizer,
487-
closeables);
488-
loggerProviderBuilder = loggerProviderCustomizer.apply(loggerProviderBuilder, config);
489-
SdkLoggerProvider loggerProvider = loggerProviderBuilder.build();
490-
closeables.add(loggerProvider);
491-
492-
ContextPropagators propagators =
493-
PropagatorConfiguration.configurePropagators(config, spiHelper, propagatorCustomizer);
494-
495-
OpenTelemetrySdkBuilder sdkBuilder =
496-
OpenTelemetrySdk.builder()
497-
.setTracerProvider(tracerProvider)
498-
.setLoggerProvider(loggerProvider)
499-
.setMeterProvider(meterProvider)
500-
.setPropagators(propagators);
501-
502-
openTelemetrySdk = sdkBuilder.build();
455+
configureSdk(sdkBuilder, config, resource, spiHelper, closeables);
503456
}
504457

458+
OpenTelemetrySdk openTelemetrySdk = sdkBuilder.build();
505459
maybeRegisterShutdownHook(openTelemetrySdk);
506460
maybeSetAsGlobal(openTelemetrySdk);
507461
callAutoConfigureListeners(spiHelper, openTelemetrySdk);
@@ -526,6 +480,62 @@ public AutoConfiguredOpenTelemetrySdk build() {
526480
}
527481
}
528482

483+
// Visible for testing
484+
void configureSdk(
485+
OpenTelemetrySdkBuilder sdkBuilder,
486+
ConfigProperties config,
487+
Resource resource,
488+
SpiHelper spiHelper,
489+
List<Closeable> closeables) {
490+
SdkMeterProviderBuilder meterProviderBuilder = SdkMeterProvider.builder();
491+
meterProviderBuilder.setResource(resource);
492+
493+
MeterProviderConfiguration.configureMeterProvider(
494+
meterProviderBuilder,
495+
config,
496+
spiHelper,
497+
metricReaderCustomizer,
498+
metricExporterCustomizer,
499+
closeables);
500+
meterProviderBuilder = meterProviderCustomizer.apply(meterProviderBuilder, config);
501+
SdkMeterProvider meterProvider = meterProviderBuilder.build();
502+
closeables.add(meterProvider);
503+
504+
SdkTracerProviderBuilder tracerProviderBuilder = SdkTracerProvider.builder();
505+
tracerProviderBuilder.setResource(resource);
506+
TracerProviderConfiguration.configureTracerProvider(
507+
tracerProviderBuilder,
508+
config,
509+
spiHelper,
510+
meterProvider,
511+
spanExporterCustomizer,
512+
spanProcessorCustomizer,
513+
samplerCustomizer,
514+
closeables);
515+
tracerProviderBuilder = tracerProviderCustomizer.apply(tracerProviderBuilder, config);
516+
SdkTracerProvider tracerProvider = tracerProviderBuilder.build();
517+
closeables.add(tracerProvider);
518+
519+
SdkLoggerProviderBuilder loggerProviderBuilder = SdkLoggerProvider.builder();
520+
loggerProviderBuilder.setResource(resource);
521+
LoggerProviderConfiguration.configureLoggerProvider(
522+
loggerProviderBuilder,
523+
config,
524+
spiHelper,
525+
meterProvider,
526+
logRecordExporterCustomizer,
527+
logRecordProcessorCustomizer,
528+
closeables);
529+
loggerProviderBuilder = loggerProviderCustomizer.apply(loggerProviderBuilder, config);
530+
SdkLoggerProvider loggerProvider = loggerProviderBuilder.build();
531+
closeables.add(loggerProvider);
532+
533+
sdkBuilder
534+
.setTracerProvider(tracerProvider)
535+
.setLoggerProvider(loggerProvider)
536+
.setMeterProvider(meterProvider);
537+
}
538+
529539
@Nullable
530540
private static AutoConfiguredOpenTelemetrySdk maybeConfigureFromFile(
531541
ConfigProperties config, ComponentLoader componentLoader) {
@@ -607,7 +617,7 @@ void callAutoConfigureListeners(SpiHelper spiHelper, OpenTelemetrySdk openTeleme
607617
}
608618

609619
@SuppressWarnings("deprecation") // Support deprecated SdkTracerProviderConfigurer
610-
private void mergeSdkTracerProviderConfigurer() {
620+
void mergeSdkTracerProviderConfigurer() {
611621
for (io.opentelemetry.sdk.autoconfigure.spi.traces.SdkTracerProviderConfigurer configurer :
612622
componentLoader.load(
613623
io.opentelemetry.sdk.autoconfigure.spi.traces.SdkTracerProviderConfigurer.class)) {

Diff for: ‎sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java

+152-6
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@
6767
import java.util.Properties;
6868
import java.util.concurrent.TimeUnit;
6969
import java.util.function.BiFunction;
70+
import java.util.function.Consumer;
7071
import java.util.function.Supplier;
72+
import org.junit.jupiter.api.Assertions;
7173
import org.junit.jupiter.api.BeforeEach;
7274
import org.junit.jupiter.api.Test;
7375
import org.junit.jupiter.api.extension.ExtendWith;
@@ -391,7 +393,15 @@ void builder_registersShutdownHook() {
391393
}
392394

393395
@Test
394-
void shutdownHook() throws InterruptedException {
396+
void builder_customizes() {
397+
builder = spy(builder);
398+
OpenTelemetrySdk sdk = builder.build().getOpenTelemetrySdk();
399+
assertThat(sdk).isNotNull();
400+
verify(builder, times(1)).mergeSdkTracerProviderConfigurer();
401+
}
402+
403+
@Test
404+
void builder_shutdownHook() throws InterruptedException {
395405
OpenTelemetrySdk sdk = mock(OpenTelemetrySdk.class);
396406

397407
Thread thread = builder.shutdownHook(sdk);
@@ -411,7 +421,7 @@ void builder_CallAutoConfigureListeners() {
411421
}
412422

413423
@Test
414-
void callAutoConfigureListeners() {
424+
void builder_callAutoConfigureListeners() {
415425
AutoConfigureListener listener = mock(AutoConfigureListener.class);
416426
SpiHelper spiHelper = mock(SpiHelper.class);
417427
when(spiHelper.getListeners()).thenReturn(Collections.singleton(listener));
@@ -455,6 +465,39 @@ void disableSdk() {
455465
verify(logCustomizer, never()).apply(any(), any());
456466
}
457467

468+
@Test
469+
void disableSdk_PropagatorCustomizer() {
470+
Context extracted = Context.root().with(ContextKey.named("animal"), "bear");
471+
472+
when(propagator2.extract(any(), any(), any())).thenReturn(extracted);
473+
474+
AutoConfiguredOpenTelemetrySdk autoConfiguredSdk =
475+
AutoConfiguredOpenTelemetrySdk.builder()
476+
.addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "true"))
477+
.addPropertiesSupplier(() -> singletonMap("otel.propagators", "tracecontext"))
478+
.addPropagatorCustomizer(
479+
(previous, config) -> {
480+
assertThat(previous).isSameAs(W3CTraceContextPropagator.getInstance());
481+
return propagator1;
482+
})
483+
.addPropagatorCustomizer(
484+
(previous, config) -> {
485+
assertThat(previous).isSameAs(propagator1);
486+
return propagator2;
487+
})
488+
.build();
489+
490+
// When the SDK is disabled, propagators are still configured
491+
assertThat(autoConfiguredSdk.getOpenTelemetrySdk().getPropagators()).isNotNull();
492+
Consumer<TextMapPropagator> propagatorConsumer =
493+
propagator -> {
494+
assertThat(propagator.extract(Context.root(), Collections.emptyMap(), getter))
495+
.isEqualTo(extracted);
496+
};
497+
assertThat(autoConfiguredSdk.getOpenTelemetrySdk().getPropagators().getTextMapPropagator())
498+
.isInstanceOfSatisfying(TextMapPropagator.class, propagatorConsumer);
499+
}
500+
458501
@Test
459502
void tracerProviderCustomizer() {
460503
InMemorySpanExporter spanExporter = InMemorySpanExporter.create();
@@ -510,6 +553,88 @@ void testNonStringProperties() {
510553
});
511554
}
512555

556+
@Test
557+
@SuppressLogger(AutoConfiguredOpenTelemetrySdkBuilder.class)
558+
void configurationError_propagators() {
559+
BiFunction<SdkTracerProviderBuilder, ConfigProperties, SdkTracerProviderBuilder>
560+
traceCustomizer = getTracerProviderBuilderSpy();
561+
BiFunction<SdkMeterProviderBuilder, ConfigProperties, SdkMeterProviderBuilder>
562+
metricCustomizer = getMeterProviderBuilderSpy();
563+
BiFunction<SdkLoggerProviderBuilder, ConfigProperties, SdkLoggerProviderBuilder> logCustomizer =
564+
getLoggerProviderBuilderSpy();
565+
566+
assertThatThrownBy(
567+
() ->
568+
// Override the provider builders with mocks which we can verify are closed
569+
AutoConfiguredOpenTelemetrySdk.builder()
570+
.addTracerProviderCustomizer(traceCustomizer)
571+
.addMeterProviderCustomizer(metricCustomizer)
572+
.addLoggerProviderCustomizer(logCustomizer)
573+
.addPropertiesSupplier(() -> singletonMap("otel.metrics.exporter", "none"))
574+
.addPropertiesSupplier(() -> singletonMap("otel.traces.exporter", "none"))
575+
.addPropertiesSupplier(() -> singletonMap("otel.logs.exporter", "none"))
576+
.addPropertiesSupplier(() -> singletonMap("otel.propagators", "foo"))
577+
.addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "true"))
578+
.build())
579+
.isInstanceOf(ConfigurationException.class)
580+
.hasMessageContaining("Unrecognized value for otel.propagators");
581+
582+
// When the SDK is disabled and propagators are mis-configured, none of the customizers are
583+
// called
584+
verify(traceCustomizer, never()).apply(any(), any());
585+
verify(metricCustomizer, never()).apply(any(), any());
586+
verify(logCustomizer, never()).apply(any(), any());
587+
588+
assertThatThrownBy(
589+
() ->
590+
// Override the provider builders with mocks which we can verify are closed
591+
AutoConfiguredOpenTelemetrySdk.builder()
592+
.addTracerProviderCustomizer(traceCustomizer)
593+
.addMeterProviderCustomizer(metricCustomizer)
594+
.addLoggerProviderCustomizer(logCustomizer)
595+
.addPropertiesSupplier(() -> singletonMap("otel.metrics.exporter", "none"))
596+
.addPropertiesSupplier(() -> singletonMap("otel.traces.exporter", "none"))
597+
.addPropertiesSupplier(() -> singletonMap("otel.logs.exporter", "none"))
598+
.addPropertiesSupplier(() -> singletonMap("otel.propagators", "foo"))
599+
.addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "false"))
600+
.build())
601+
.isInstanceOf(ConfigurationException.class)
602+
.hasMessageContaining("Unrecognized value for otel.propagators");
603+
604+
// When the SDK is enabled and propagators are mis-configured, none of the customizers are
605+
// called
606+
verify(traceCustomizer, never()).apply(any(), any());
607+
verify(metricCustomizer, never()).apply(any(), any());
608+
verify(logCustomizer, never()).apply(any(), any());
609+
}
610+
611+
@Test
612+
@SuppressLogger(AutoConfiguredOpenTelemetrySdkBuilder.class)
613+
void configurationError_runtime() {
614+
BiFunction<SdkTracerProviderBuilder, ConfigProperties, SdkTracerProviderBuilder>
615+
traceCustomizer = getTracerProviderBuilderSpy();
616+
BiFunction<SdkMeterProviderBuilder, ConfigProperties, SdkMeterProviderBuilder>
617+
metricCustomizer = getMeterProviderBuilderSpy();
618+
BiFunction<SdkLoggerProviderBuilder, ConfigProperties, SdkLoggerProviderBuilder> logCustomizer =
619+
getLoggerProviderBuilderSpy();
620+
621+
doThrow(new RuntimeException()).when(traceCustomizer).apply(any(), any());
622+
623+
assertThatThrownBy(
624+
() ->
625+
AutoConfiguredOpenTelemetrySdk.builder()
626+
.addTracerProviderCustomizer(traceCustomizer)
627+
.addMeterProviderCustomizer(metricCustomizer)
628+
.addLoggerProviderCustomizer(logCustomizer)
629+
.addPropertiesSupplier(() -> singletonMap("otel.metrics.exporter", "none"))
630+
.addPropertiesSupplier(() -> singletonMap("otel.traces.exporter", "none"))
631+
.addPropertiesSupplier(() -> singletonMap("otel.logs.exporter", "none"))
632+
.addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "false"))
633+
.build())
634+
.isInstanceOf(ConfigurationException.class)
635+
.hasMessageContaining("Unexpected configuration error");
636+
}
637+
513638
@Test
514639
@SuppressLogger(AutoConfiguredOpenTelemetrySdkBuilder.class)
515640
void configurationError_ClosesResources() {
@@ -539,16 +664,37 @@ void configurationError_ClosesResources() {
539664
.addLoggerProviderCustomizer((u1, u2) -> loggerProviderBuilder)
540665
.addPropertiesSupplier(() -> singletonMap("otel.metrics.exporter", "none"))
541666
.addPropertiesSupplier(() -> singletonMap("otel.traces.exporter", "none"))
542-
.addPropertiesSupplier(() -> singletonMap("otel.logs.exporter", "none"))
543-
.addPropertiesSupplier(() -> singletonMap("otel.propagators", "foo"))
667+
.addPropertiesSupplier(() -> singletonMap("otel.logs.exporter", "foo"))
668+
.addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "false"))
544669
.build())
545670
.isInstanceOf(ConfigurationException.class)
546-
.hasMessageContaining("Unrecognized value for otel.propagators");
671+
.hasMessageContaining("Unrecognized value for otel.logs.exporter: foo");
547672

548673
verify(tracerProvider).close();
549674
verify(meterProvider).close();
550-
verify(loggerProvider).close();
551675

552676
logs.assertContains("Error closing io.opentelemetry.sdk.trace.SdkTracerProvider: Error!");
553677
}
678+
679+
@Test
680+
@SuppressLogger(AutoConfiguredOpenTelemetrySdkBuilder.class)
681+
void configurationError_fileNotFound() {
682+
assertThatThrownBy(
683+
() ->
684+
AutoConfiguredOpenTelemetrySdk.builder()
685+
.addPropertiesSupplier(() -> singletonMap("otel.config.file", "foo"))
686+
.addPropertiesSupplier(
687+
() -> singletonMap("otel.experimental.config.file", "foo"))
688+
.addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "true"))
689+
.build())
690+
.isInstanceOf(ConfigurationException.class)
691+
.hasMessageContaining("Configuration file not found");
692+
693+
Assertions.assertDoesNotThrow(
694+
() ->
695+
AutoConfiguredOpenTelemetrySdk.builder()
696+
.addPropertiesSupplier(() -> singletonMap("otel.experimental.config.file", ""))
697+
.addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "true"))
698+
.build());
699+
}
554700
}

0 commit comments

Comments
 (0)