Clean the Logback instrumentation of the OTel starter (#12450)

Author: jeanbisutti

instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/logging/LogbackAppenderInstaller.java

@@ -30,15 +30,14 @@ static void install(ApplicationEnvironmentPreparedEvent applicationEnvironmentPr
 
   private static boolean isLogbackAppenderAddable(
       ApplicationEnvironmentPreparedEvent applicationEnvironmentPreparedEvent) {
-    Boolean otelSdkDisableProperty =
-        evaluateBooleanProperty(applicationEnvironmentPreparedEvent, "otel.sdk.disabled");
-    Boolean logbackInstrumentationEnabledProperty =
+    boolean otelSdkDisabled =
+        evaluateBooleanProperty(applicationEnvironmentPreparedEvent, "otel.sdk.disabled", false);
+    boolean logbackInstrumentationEnabled =
         evaluateBooleanProperty(
-            applicationEnvironmentPreparedEvent, "otel.instrumentation.logback-appender.enabled");
-    return otelSdkDisableProperty == null
-        || !otelSdkDisableProperty.booleanValue()
-        || logbackInstrumentationEnabledProperty == null
-        || logbackInstrumentationEnabledProperty.booleanValue();
+            applicationEnvironmentPreparedEvent,
+            "otel.instrumentation.logback-appender.enabled",
+            true);
+    return !otelSdkDisabled && logbackInstrumentationEnabled;
   }
 
   private static void reInitializeOpenTelemetryAppender(
@@ -141,6 +140,15 @@ private static Boolean evaluateBooleanProperty(
         .getProperty(property, Boolean.class);
   }
 
+  private static boolean evaluateBooleanProperty(
+      ApplicationEnvironmentPreparedEvent applicationEnvironmentPreparedEvent,
+      String property,
+      boolean defaultValue) {
+    return applicationEnvironmentPreparedEvent
+        .getEnvironment()
+        .getProperty(property, Boolean.class, defaultValue);
+  }
+
   private static Optional<OpenTelemetryAppender> findOpenTelemetryAppender() {
     ILoggerFactory loggerFactorySpi = LoggerFactory.getILoggerFactory();
     if (!(loggerFactorySpi instanceof LoggerContext)) {

smoke-tests-otel-starter/spring-boot-3/src/test/java/io/opentelemetry/spring/smoketest/OtelSpringStarterDisabledSmokeTest.java

@@ -5,22 +5,46 @@
 
 package io.opentelemetry.spring.smoketest;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.opentelemetry.sdk.logs.data.LogRecordData;
+import io.opentelemetry.sdk.metrics.data.MetricData;
+import io.opentelemetry.sdk.trace.data.SpanData;
+import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledInNativeImage;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.web.client.TestRestTemplate;
 
 @SpringBootTest(
     classes = {
       OtelSpringStarterSmokeTestApplication.class,
-      AbstractOtelSpringStarterSmokeTest.TestConfiguration.class
+      AbstractOtelSpringStarterSmokeTest.TestConfiguration.class,
+      SpringSmokeOtelConfiguration.class
     },
+    webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
     properties = {"otel.sdk.disabled=true"})
 @DisabledInNativeImage // Without this the native tests in the OtelSpringStarterSmokeTest class will
 // fail with org.h2.jdbc.JdbcSQLSyntaxErrorException: Table "CUSTOMER" already exists
-class OtelSpringStarterDisabledSmokeTest {
+class OtelSpringStarterDisabledSmokeTest extends AbstractSpringStarterSmokeTest {
+
+  @Autowired private TestRestTemplate testRestTemplate;
 
   @Test
-  void shouldStartApplication() {
-    // make sure we can still start the application with the disabled property
+  void shouldNotSendTelemetry() throws InterruptedException {
+    testRestTemplate.getForObject(OtelSpringStarterSmokeTestController.PING, String.class);
+
+    // See SpringSmokeOtelConfiguration
+    Thread.sleep(200);
+
+    List<SpanData> exportedSpans = testing.getExportedSpans();
+    assertThat(exportedSpans).isEmpty();
+
+    List<MetricData> exportedMetrics = testing.getExportedMetrics();
+    assertThat(exportedMetrics).isEmpty();
+
+    List<LogRecordData> exportedLogRecords = testing.getExportedLogRecords();
+    assertThat(exportedLogRecords).isEmpty();
   }
 }

smoke-tests-otel-starter/spring-boot-3/src/test/java/io/opentelemetry/spring/smoketest/OtelSpringStarterWithLogbackInstrumentationDisabledSmokeTest.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.spring.smoketest;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.opentelemetry.sdk.logs.data.LogRecordData;
+import java.util.List;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledInNativeImage;
+import org.springframework.boot.test.context.SpringBootTest;
+
+@SpringBootTest(
+    classes = {
+      OtelSpringStarterSmokeTestApplication.class,
+      AbstractOtelSpringStarterSmokeTest.TestConfiguration.class,
+      SpringSmokeOtelConfiguration.class
+    },
+    webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
+    properties = {"otel.instrumentation.logback-appender.enabled=false"})
+@DisabledInNativeImage // Without this the native tests in the OtelSpringStarterSmokeTest class will
+// fail with org.h2.jdbc.JdbcSQLSyntaxErrorException: Table "CUSTOMER" already exists
+class OtelSpringStarterWithLogbackInstrumentationDisabledSmokeTest
+    extends AbstractSpringStarterSmokeTest {
+
+  @Test
+  void shouldNotSendLogRecordTelemetry() throws InterruptedException {
+
+    // See SpringSmokeOtelConfiguration
+    Thread.sleep(200);
+
+    List<LogRecordData> exportedLogRecords = testing.getExportedLogRecords();
+    assertThat(exportedLogRecords).isEmpty();
+  }
+}