From b49d2bb3c952a228449cd509edc13a8ff7c55ba7 Mon Sep 17 00:00:00 2001
From: Jean-Francois Breault <jfbreault@gmail.com>
Date: Tue, 18 Jun 2024 12:21:41 -0400
Subject: [PATCH] fix disable logback instraumentation

simply implementation

add test
---
 .../logging/LogbackAppenderInstaller.java     | 24 ++++++++-----
 .../logging/LogbackAppenderTest.java          | 36 +++++++++++++++++++
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/instrumentation/logging/LogbackAppenderInstaller.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/instrumentation/logging/LogbackAppenderInstaller.java
index 811dacb31bf7..a1e66e33fa17 100644
--- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/instrumentation/logging/LogbackAppenderInstaller.java
+++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/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 otelSdkDisableProperty =
+        evaluateBooleanProperty(applicationEnvironmentPreparedEvent, "otel.sdk.disabled", false);
+    boolean logbackInstrumentationEnabledProperty =
         evaluateBooleanProperty(
-            applicationEnvironmentPreparedEvent, "otel.instrumentation.logback-appender.enabled");
-    return otelSdkDisableProperty == null
-        || !otelSdkDisableProperty.booleanValue()
-        || logbackInstrumentationEnabledProperty == null
-        || logbackInstrumentationEnabledProperty.booleanValue();
+            applicationEnvironmentPreparedEvent,
+            "otel.instrumentation.logback-appender.enabled",
+            true);
+    return !otelSdkDisableProperty && logbackInstrumentationEnabledProperty;
   }
 
   private static void reInitializeOpenTelemetryAppender(
@@ -133,6 +132,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)) {
diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/testLogbackAppender/java/io/opentelemetry/instrumentation/spring/autoconfigure/instrumentation/logging/LogbackAppenderTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/testLogbackAppender/java/io/opentelemetry/instrumentation/spring/autoconfigure/instrumentation/logging/LogbackAppenderTest.java
index 9d1336b67ff6..c64cc6277a2e 100644
--- a/instrumentation/spring/spring-boot-autoconfigure/src/testLogbackAppender/java/io/opentelemetry/instrumentation/spring/autoconfigure/instrumentation/logging/LogbackAppenderTest.java
+++ b/instrumentation/spring/spring-boot-autoconfigure/src/testLogbackAppender/java/io/opentelemetry/instrumentation/spring/autoconfigure/instrumentation/logging/LogbackAppenderTest.java
@@ -110,4 +110,40 @@ void shouldNotInitializeAppenderWhenDisabled() {
 
     assertThat(testing.logRecords()).isEmpty();
   }
+
+  @Test
+  void shouldNotInitializeAppenderWhenSdkDisabled() {
+    Map<String, Object> properties = new HashMap<>();
+    properties.put("logging.config", "classpath:logback-test.xml");
+    properties.put("otel.sdk.disabled", "true");
+
+    SpringApplication app =
+        new SpringApplication(
+            TestingOpenTelemetryConfiguration.class, OpenTelemetryAppenderAutoConfiguration.class);
+    app.setDefaultProperties(properties);
+    ConfigurableApplicationContext context = app.run();
+    cleanup.deferCleanup(context);
+
+    LoggerFactory.getLogger("test").info("test log message");
+
+    assertThat(testing.logRecords()).isEmpty();
+  }
+
+  @Test
+  void shouldNotInitializeAppenderWhenInstrumentationDisabled() {
+    Map<String, Object> properties = new HashMap<>();
+    properties.put("logging.config", "classpath:logback-test.xml");
+    properties.put("otel.instrumentation.logback-appender.enabled", "false");
+
+    SpringApplication app =
+        new SpringApplication(
+            TestingOpenTelemetryConfiguration.class, OpenTelemetryAppenderAutoConfiguration.class);
+    app.setDefaultProperties(properties);
+    ConfigurableApplicationContext context = app.run();
+    cleanup.deferCleanup(context);
+
+    LoggerFactory.getLogger("test").info("test log message");
+
+    assertThat(testing.logRecords()).isEmpty();
+  }
 }