refactor: configureSdk + fix fuzz tests

open-telemetry · Feb 2, 2025 · 3bc9876 · 3bc9876
1 parent 88ce1b2
commit 3bc9876
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 53 deletions.
diff --git a/...c/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java b/...c/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java
@@ -451,54 +451,8 @@ public AutoConfiguredOpenTelemetrySdk build() {
       sdkBuilder.setPropagators(propagators);
 
       boolean sdkEnabled = !config.getBoolean("otel.sdk.disabled", false);
-
       if (sdkEnabled) {
-        SdkMeterProviderBuilder meterProviderBuilder = SdkMeterProvider.builder();
-        meterProviderBuilder.setResource(resource);
-        MeterProviderConfiguration.configureMeterProvider(
-            meterProviderBuilder,
-            config,
-            spiHelper,
-            metricReaderCustomizer,
-            metricExporterCustomizer,
-            closeables);
-        meterProviderBuilder = meterProviderCustomizer.apply(meterProviderBuilder, config);
-        SdkMeterProvider meterProvider = meterProviderBuilder.build();
-        closeables.add(meterProvider);
-
-        SdkTracerProviderBuilder tracerProviderBuilder = SdkTracerProvider.builder();
-        tracerProviderBuilder.setResource(resource);
-        TracerProviderConfiguration.configureTracerProvider(
-            tracerProviderBuilder,
-            config,
-            spiHelper,
-            meterProvider,
-            spanExporterCustomizer,
-            spanProcessorCustomizer,
-            samplerCustomizer,
-            closeables);
-        tracerProviderBuilder = tracerProviderCustomizer.apply(tracerProviderBuilder, config);
-        SdkTracerProvider tracerProvider = tracerProviderBuilder.build();
-        closeables.add(tracerProvider);
-
-        SdkLoggerProviderBuilder loggerProviderBuilder = SdkLoggerProvider.builder();
-        loggerProviderBuilder.setResource(resource);
-        LoggerProviderConfiguration.configureLoggerProvider(
-            loggerProviderBuilder,
-            config,
-            spiHelper,
-            meterProvider,
-            logRecordExporterCustomizer,
-            logRecordProcessorCustomizer,
-            closeables);
-        loggerProviderBuilder = loggerProviderCustomizer.apply(loggerProviderBuilder, config);
-        SdkLoggerProvider loggerProvider = loggerProviderBuilder.build();
-        closeables.add(loggerProvider);
-
-        sdkBuilder
-            .setTracerProvider(tracerProvider)
-            .setLoggerProvider(loggerProvider)
-            .setMeterProvider(meterProvider);
+        sdkBuilder = configureSdk(sdkBuilder, config, resource, spiHelper, closeables);
       }
 
       OpenTelemetrySdk openTelemetrySdk = sdkBuilder.build();
@@ -527,6 +481,61 @@ public AutoConfiguredOpenTelemetrySdk build() {
     }
   }
 
+  public OpenTelemetrySdkBuilder configureSdk(
+      OpenTelemetrySdkBuilder sdkBuilder,
+      ConfigProperties config,
+      Resource resource,
+      SpiHelper spiHelper,
+      List<Closeable> closeables) {
+    SdkMeterProviderBuilder meterProviderBuilder = SdkMeterProvider.builder();
+    meterProviderBuilder.setResource(resource);
+
+    MeterProviderConfiguration.configureMeterProvider(
+        meterProviderBuilder,
+        config,
+        spiHelper,
+        metricReaderCustomizer,
+        metricExporterCustomizer,
+        closeables);
+    meterProviderBuilder = meterProviderCustomizer.apply(meterProviderBuilder, config);
+    SdkMeterProvider meterProvider = meterProviderBuilder.build();
+    closeables.add(meterProvider);
+
+    SdkTracerProviderBuilder tracerProviderBuilder = SdkTracerProvider.builder();
+    tracerProviderBuilder.setResource(resource);
+    TracerProviderConfiguration.configureTracerProvider(
+        tracerProviderBuilder,
+        config,
+        spiHelper,
+        meterProvider,
+        spanExporterCustomizer,
+        spanProcessorCustomizer,
+        samplerCustomizer,
+        closeables);
+    tracerProviderBuilder = tracerProviderCustomizer.apply(tracerProviderBuilder, config);
+    SdkTracerProvider tracerProvider = tracerProviderBuilder.build();
+    closeables.add(tracerProvider);
+
+    SdkLoggerProviderBuilder loggerProviderBuilder = SdkLoggerProvider.builder();
+    loggerProviderBuilder.setResource(resource);
+    LoggerProviderConfiguration.configureLoggerProvider(
+        loggerProviderBuilder,
+        config,
+        spiHelper,
+        meterProvider,
+        logRecordExporterCustomizer,
+        logRecordProcessorCustomizer,
+        closeables);
+    loggerProviderBuilder = loggerProviderCustomizer.apply(loggerProviderBuilder, config);
+    SdkLoggerProvider loggerProvider = loggerProviderBuilder.build();
+    closeables.add(loggerProvider);
+
+    return sdkBuilder
+        .setTracerProvider(tracerProvider)
+        .setLoggerProvider(loggerProvider)
+        .setMeterProvider(meterProvider);
+  }
+
   @Nullable
   private static AutoConfiguredOpenTelemetrySdk maybeConfigureFromFile(
       ConfigProperties config, ComponentLoader componentLoader) {

diff --git a/.../src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java b/.../src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java
@@ -546,7 +546,7 @@ void testNonStringProperties() {
 
   @Test
   @SuppressLogger(AutoConfiguredOpenTelemetrySdkBuilder.class)
-  void configurationError_ClosesResources() {
+  void configurationError_propagators() {
     // AutoConfiguredOpenTelemetrySdk should close partially configured resources if an exception
     // short circuits configuration. Verify that SdkTracerProvider, SdkMeterProvider, and
     // SdkLoggerProvider
@@ -575,14 +575,25 @@ void configurationError_ClosesResources() {
                     .addPropertiesSupplier(() -> singletonMap("otel.traces.exporter", "none"))
                     .addPropertiesSupplier(() -> singletonMap("otel.logs.exporter", "none"))
                     .addPropertiesSupplier(() -> singletonMap("otel.propagators", "foo"))
+                    .addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "true"))
                     .build())
         .isInstanceOf(ConfigurationException.class)
         .hasMessageContaining("Unrecognized value for otel.propagators");
 
-    verify(tracerProvider).close();
-    verify(meterProvider).close();
-    verify(loggerProvider).close();
-
-    logs.assertContains("Error closing io.opentelemetry.sdk.trace.SdkTracerProvider: Error!");
+    assertThatThrownBy(
+            () ->
+                // Override the provider builders with mocks which we can verify are closed
+                AutoConfiguredOpenTelemetrySdk.builder()
+                    .addTracerProviderCustomizer((u1, u2) -> tracerProviderBuilder)
+                    .addMeterProviderCustomizer((u1, u2) -> meterProviderBuilder)
+                    .addLoggerProviderCustomizer((u1, u2) -> loggerProviderBuilder)
+                    .addPropertiesSupplier(() -> singletonMap("otel.metrics.exporter", "none"))
+                    .addPropertiesSupplier(() -> singletonMap("otel.traces.exporter", "none"))
+                    .addPropertiesSupplier(() -> singletonMap("otel.logs.exporter", "none"))
+                    .addPropertiesSupplier(() -> singletonMap("otel.propagators", "foo"))
+                    .addPropertiesSupplier(() -> singletonMap("otel.sdk.disabled", "false"))
+                    .build())
+        .isInstanceOf(ConfigurationException.class)
+        .hasMessageContaining("Unrecognized value for otel.propagators");
   }
 }