From 93b8283b0a423693162e35308c848b8b68ac11d3 Mon Sep 17 00:00:00 2001
From: Jean Bisutti <jean.bisutti@gmail.com>
Date: Thu, 22 Sep 2022 18:54:55 +0200
Subject: [PATCH] Add marker attribute for Logback (#6652)

---
 .../javaagent/build.gradle.kts                |  1 +
 .../appender/v1_0/LogbackSingletons.java      |  8 ++++++-
 .../logback/appender/v1_0/LogbackTest.java    | 21 +++++++++++++++++++
 .../appender/v1_0/OpenTelemetryAppender.java  | 15 ++++++++++++-
 .../v1_0/internal/LoggingEventMapper.java     | 16 +++++++++++++-
 .../v1_0/OpenTelemetryAppenderConfigTest.java | 12 +++++++++--
 .../v1_0/internal/LoggingEventMapperTest.java |  6 +++---
 .../src/test/resources/logback-test.xml       |  1 +
 8 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts b/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts
index 0f6b34623c90..753bb054d8c7 100644
--- a/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts
+++ b/instrumentation/logback/logback-appender-1.0/javaagent/build.gradle.kts
@@ -56,4 +56,5 @@ tasks.withType<Test>().configureEach {
   jvmArgs("-Dotel.instrumentation.logback-appender.experimental.capture-mdc-attributes=*")
   jvmArgs("-Dotel.instrumentation.logback-appender.experimental-log-attributes=true")
   jvmArgs("-Dotel.instrumentation.logback-appender.experimental.capture-code-attributes=true")
+  jvmArgs("-Dotel.instrumentation.logback-appender.experimental.capture-marker-attribute=true")
 }
diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java b/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java
index 6789d3567ab0..a91bf7fcf742 100644
--- a/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java
+++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/appender/v1_0/LogbackSingletons.java
@@ -24,6 +24,9 @@ public final class LogbackSingletons {
     boolean captureCodeAttributes =
         config.getBoolean(
             "otel.instrumentation.logback-appender.experimental.capture-code-attributes", false);
+    boolean captureMarkerAttribute =
+        config.getBoolean(
+            "otel.instrumentation.logback-appender.experimental.capture-marker-attribute", false);
     List<String> captureMdcAttributes =
         config.getList(
             "otel.instrumentation.logback-appender.experimental.capture-mdc-attributes",
@@ -31,7 +34,10 @@ public final class LogbackSingletons {
 
     mapper =
         new LoggingEventMapper(
-            captureExperimentalAttributes, captureMdcAttributes, captureCodeAttributes);
+            captureExperimentalAttributes,
+            captureMdcAttributes,
+            captureCodeAttributes,
+            captureMarkerAttribute);
   }
 
   public static LoggingEventMapper mapper() {
diff --git a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java
index 3c0fa46f8023..fc2c995aba2c 100644
--- a/instrumentation/logback/logback-appender-1.0/javaagent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java
+++ b/instrumentation/logback/logback-appender-1.0/javaagent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java
@@ -26,6 +26,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.MDC;
+import org.slf4j.Marker;
+import org.slf4j.MarkerFactory;
 
 class LogbackTest extends AgentInstrumentationSpecification {
 
@@ -219,6 +221,25 @@ void testMdc() {
                     .containsEntry(SemanticAttributes.CODE_FILEPATH, "LogbackTest.java"));
   }
 
+  @Test
+  public void testMarker() {
+
+    String markerName = "aMarker";
+    Marker marker = MarkerFactory.getMarker(markerName);
+
+    abcLogger.info(marker, "Message");
+
+    await().untilAsserted(() -> assertThat(testing.logs().size()).isEqualTo(1));
+
+    LogData log = getLogs().get(0);
+
+    assertThat(log)
+        .hasAttributesSatisfying(
+            attributes ->
+                assertThat(attributes)
+                    .containsEntry(AttributeKey.stringKey("logback.marker"), markerName));
+  }
+
   private static void performLogging(
       Logger logger,
       OneArgLoggerMethod oneArgLoggerMethod,
diff --git a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java
index 2f2c13760f6e..1e9a2107be59 100644
--- a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java
+++ b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppender.java
@@ -26,6 +26,7 @@ public class OpenTelemetryAppender extends UnsynchronizedAppenderBase<ILoggingEv
 
   private volatile boolean captureExperimentalAttributes = false;
   private volatile boolean captureCodeAttributes = false;
+  private volatile boolean captureMarkerAttribute = false;
   private volatile List<String> captureMdcAttributes = emptyList();
 
   private volatile LoggingEventMapper mapper;
@@ -36,7 +37,10 @@ public OpenTelemetryAppender() {}
   public void start() {
     mapper =
         new LoggingEventMapper(
-            captureExperimentalAttributes, captureMdcAttributes, captureCodeAttributes);
+            captureExperimentalAttributes,
+            captureMdcAttributes,
+            captureCodeAttributes,
+            captureMarkerAttribute);
     super.start();
   }
 
@@ -77,6 +81,15 @@ public void setCaptureCodeAttributes(boolean captureCodeAttributes) {
     this.captureCodeAttributes = captureCodeAttributes;
   }
 
+  /**
+   * Sets whether the marker attribute should be set to logs.
+   *
+   * @param captureMarkerAttribute To enable or disable the marker attribute
+   */
+  public void setCaptureMarkerAttribute(boolean captureMarkerAttribute) {
+    this.captureMarkerAttribute = captureMarkerAttribute;
+  }
+
   /** Configures the {@link MDC} attributes that will be copied to logs. */
   public void setCaptureMdcAttributes(String attributes) {
     if (attributes != null) {
diff --git a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java
index 576e32395237..ab2a50be7acb 100644
--- a/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java
+++ b/instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java
@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import org.slf4j.Marker;
 
 /**
  * This class is internal and is hence not for public use. Its APIs are unstable and can change at
@@ -31,18 +32,23 @@ public final class LoggingEventMapper {
 
   private static final Cache<String, AttributeKey<String>> mdcAttributeKeys = Cache.bounded(100);
 
+  private static final AttributeKey<String> LOG_MARKER = AttributeKey.stringKey("logback.marker");
+
   private final boolean captureExperimentalAttributes;
   private final List<String> captureMdcAttributes;
   private final boolean captureAllMdcAttributes;
   private final boolean captureCodeAttributes;
+  private final boolean captureMarkerAttribute;
 
   public LoggingEventMapper(
       boolean captureExperimentalAttributes,
       List<String> captureMdcAttributes,
-      boolean captureCodeAttributes) {
+      boolean captureCodeAttributes,
+      boolean captureMarkerAttribute) {
     this.captureExperimentalAttributes = captureExperimentalAttributes;
     this.captureCodeAttributes = captureCodeAttributes;
     this.captureMdcAttributes = captureMdcAttributes;
+    this.captureMarkerAttribute = captureMarkerAttribute;
     this.captureAllMdcAttributes =
         captureMdcAttributes.size() == 1 && captureMdcAttributes.get(0).equals("*");
   }
@@ -125,6 +131,14 @@ private void mapLoggingEvent(LogRecordBuilder builder, ILoggingEvent loggingEven
       }
     }
 
+    if (captureMarkerAttribute) {
+      Marker marker = loggingEvent.getMarker();
+      if (marker != null) {
+        String markerName = marker.getName();
+        attributes.put(LOG_MARKER, markerName);
+      }
+    }
+
     builder.setAllAttributes(attributes.build());
 
     // span context
diff --git a/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppenderConfigTest.java b/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppenderConfigTest.java
index 06853ba75784..0c7c83bce3ff 100644
--- a/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppenderConfigTest.java
+++ b/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/OpenTelemetryAppenderConfigTest.java
@@ -30,6 +30,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.MDC;
+import org.slf4j.Marker;
+import org.slf4j.MarkerFactory;
 
 class OpenTelemetryAppenderConfigTest {
 
@@ -101,7 +103,9 @@ private static Span runWithSpan(String spanName, Runnable runnable) {
   @Test
   void logWithExtras() {
     Instant start = Instant.now();
-    logger.info("log message 1", new IllegalStateException("Error!"));
+    String markerName = "aMarker";
+    Marker marker = MarkerFactory.getMarker(markerName);
+    logger.info(marker, "log message 1", new IllegalStateException("Error!"));
 
     List<LogData> logDataList = logExporter.getFinishedLogItems();
     assertThat(logDataList).hasSize(1);
@@ -114,7 +118,8 @@ void logWithExtras() {
         .isLessThan(TimeUnit.MILLISECONDS.toNanos(Instant.now().toEpochMilli()));
     assertThat(logData.getSeverity()).isEqualTo(Severity.INFO);
     assertThat(logData.getSeverityText()).isEqualTo("INFO");
-    assertThat(logData.getAttributes().size()).isEqualTo(3 + 4); // 4 code attributes
+    assertThat(logData.getAttributes().size())
+        .isEqualTo(3 + 4 + 1); // 3 exception attributes, 4 code attributes, 1 marker attribute
     assertThat(logData.getAttributes().get(SemanticAttributes.EXCEPTION_TYPE))
         .isEqualTo(IllegalStateException.class.getName());
     assertThat(logData.getAttributes().get(SemanticAttributes.EXCEPTION_MESSAGE))
@@ -135,6 +140,9 @@ void logWithExtras() {
 
     Long lineNumber = logData.getAttributes().get(SemanticAttributes.CODE_LINENO);
     assertThat(lineNumber).isGreaterThan(1);
+
+    String logMarker = logData.getAttributes().get(AttributeKey.stringKey("logback.marker"));
+    assertThat(logMarker).isEqualTo(markerName);
   }
 
   @Test
diff --git a/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapperTest.java b/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapperTest.java
index f4b6ac530412..5c0567f6b6d6 100644
--- a/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapperTest.java
+++ b/instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapperTest.java
@@ -22,7 +22,7 @@ class LoggingEventMapperTest {
   @Test
   void testDefault() {
     // given
-    LoggingEventMapper mapper = new LoggingEventMapper(false, emptyList(), false);
+    LoggingEventMapper mapper = new LoggingEventMapper(false, emptyList(), false, false);
     Map<String, String> contextData = new HashMap<>();
     contextData.put("key1", "value1");
     contextData.put("key2", "value2");
@@ -38,7 +38,7 @@ void testDefault() {
   @Test
   void testSome() {
     // given
-    LoggingEventMapper mapper = new LoggingEventMapper(false, singletonList("key2"), false);
+    LoggingEventMapper mapper = new LoggingEventMapper(false, singletonList("key2"), false, false);
     Map<String, String> contextData = new HashMap<>();
     contextData.put("key1", "value1");
     contextData.put("key2", "value2");
@@ -55,7 +55,7 @@ void testSome() {
   @Test
   void testAll() {
     // given
-    LoggingEventMapper mapper = new LoggingEventMapper(false, singletonList("*"), false);
+    LoggingEventMapper mapper = new LoggingEventMapper(false, singletonList("*"), false, false);
     Map<String, String> contextData = new HashMap<>();
     contextData.put("key1", "value1");
     contextData.put("key2", "value2");
diff --git a/instrumentation/logback/logback-appender-1.0/library/src/test/resources/logback-test.xml b/instrumentation/logback/logback-appender-1.0/library/src/test/resources/logback-test.xml
index d213483122ee..a3a4273a1222 100644
--- a/instrumentation/logback/logback-appender-1.0/library/src/test/resources/logback-test.xml
+++ b/instrumentation/logback/logback-appender-1.0/library/src/test/resources/logback-test.xml
@@ -12,6 +12,7 @@
     class="io.opentelemetry.instrumentation.logback.appender.v1_0.OpenTelemetryAppender">
     <captureExperimentalAttributes>false</captureExperimentalAttributes>
     <captureCodeAttributes>true</captureCodeAttributes>
+    <captureMarkerAttribute>true</captureMarkerAttribute>
     <captureMdcAttributes>*</captureMdcAttributes>
   </appender>