Pass MDC tags as Sentry tags (#1954)

maciejwalkowiak · web-flow · commit 8bd08ba42c7e · 2022-03-28T17:27:47.000+02:00
* Pass MDC tags as Sentry tags in Logback integration.

* Pass MDC tags as Sentry tags in Log4j2 integration.

* Configure MDC tags through external configuration.

* Polish.

* Pass MDC tags as Sentry tags in JUL integration.

* Changelog.

* Update logback sample to include multiple tags.

* Update log4j2 sample to include multiple tags.

* Rename `mdcTags` to `contextTags`.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## 6.0.0-alpha.4
 
 * Ref: Remove not needed interface abstractions on Android (#1953)
+* Feat: Pass MDC tags as Sentry tags (#1954)
 
 ## 6.0.0-alpha.3
 
diff --git a/sentry-jul/src/main/java/io/sentry/jul/SentryHandler.java b/sentry-jul/src/main/java/io/sentry/jul/SentryHandler.java
@@ -42,6 +42,7 @@ public class SentryHandler extends Handler {
 
   private @NotNull Level minimumBreadcrumbLevel = Level.INFO;
   private @NotNull Level minimumEventLevel = Level.SEVERE;
+  private @NotNull SentryOptions options;
 
   /** Creates an instance of SentryHandler. */
   public SentryHandler() {
@@ -60,6 +61,7 @@ public SentryHandler(final @NotNull SentryOptions options) {
   /** Creates an instance of SentryHandler. */
   @TestOnly
   SentryHandler(final @NotNull SentryOptions options, final boolean configureFromLogManager) {
+    this.options = options;
     setFilter(new DropSentryFilter());
     if (configureFromLogManager) {
       retrieveProperties();
@@ -200,7 +202,20 @@ SentryEvent createEvent(final @NotNull LogRecord record) {
       mdcProperties =
           CollectionUtils.filterMapEntries(mdcProperties, entry -> entry.getValue() != null);
       if (!mdcProperties.isEmpty()) {
-        event.getContexts().put("MDC", mdcProperties);
+        if (!options.getContextTags().isEmpty()) {
+          for (final String contextTag : options.getContextTags()) {
+            // if mdc tag is listed in SentryOptions, apply as event tag
+            if (mdcProperties.containsKey(contextTag)) {
+              event.setTag(contextTag, mdcProperties.get(contextTag));
+              // remove from all tags applied to logging event
+              mdcProperties.remove(contextTag);
+            }
+          }
+        }
+        // put the rest of mdc tags in contexts
+        if (!mdcProperties.isEmpty()) {
+          event.getContexts().put("MDC", mdcProperties);
+        }
       }
     }
     event.setExtra(THREAD_ID, record.getThreadID());
diff --git a/sentry-jul/src/test/kotlin/io/sentry/jul/SentryHandlerTest.kt b/sentry-jul/src/test/kotlin/io/sentry/jul/SentryHandlerTest.kt
@@ -24,14 +24,15 @@ import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
 class SentryHandlerTest {
-    private class Fixture(minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, val configureWithLogManager: Boolean = false, val transport: ITransport = mock()) {
+    private class Fixture(minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, val configureWithLogManager: Boolean = false, val transport: ITransport = mock(), contextTags: List<String>? = null) {
         var logger: Logger
         var handler: SentryHandler
 
         init {
             val options = SentryOptions()
             options.dsn = "http://key@localhost/proj"
             options.setTransportFactory { _, _ -> transport }
+            contextTags?.forEach { options.addContextTag(it) }
             logger = Logger.getLogger("jul.SentryHandlerTest")
             handler = SentryHandler(options, configureWithLogManager)
             handler.setMinimumBreadcrumbLevel(minimumBreadcrumbLevel)
@@ -312,6 +313,22 @@ class SentryHandlerTest {
         )
     }
 
+    @Test
+    fun `sets tags as Sentry tags from MDC`() {
+        fixture = Fixture(minimumEventLevel = Level.WARNING, contextTags = listOf("contextTag1"))
+        MDC.put("key", "value")
+        MDC.put("contextTag1", "contextTag1Value")
+        fixture.logger.warning("testing MDC tags")
+
+        verify(fixture.transport).send(
+            checkEvent { event ->
+                assertEquals(mapOf("key" to "value"), event.contexts["MDC"])
+                assertEquals(mapOf("contextTag1" to "contextTag1Value"), event.tags)
+            },
+            anyOrNull()
+        )
+    }
+
     @Test
     fun `ignore set tags with null values from MDC`() {
         fixture = Fixture(minimumEventLevel = Level.WARNING)
diff --git a/sentry-log4j2/api/sentry-log4j2.api b/sentry-log4j2/api/sentry-log4j2.api
@@ -4,9 +4,9 @@ public final class io/sentry/log4j2/BuildConfig {
 }
 
 public class io/sentry/log4j2/SentryAppender : org/apache/logging/log4j/core/appender/AbstractAppender {
-	public fun <init> (Ljava/lang/String;Lorg/apache/logging/log4j/core/Filter;Ljava/lang/String;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Ljava/lang/Boolean;Lio/sentry/ITransportFactory;Lio/sentry/IHub;)V
+	public fun <init> (Ljava/lang/String;Lorg/apache/logging/log4j/core/Filter;Ljava/lang/String;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Ljava/lang/Boolean;Lio/sentry/ITransportFactory;Lio/sentry/IHub;[Ljava/lang/String;)V
 	public fun append (Lorg/apache/logging/log4j/core/LogEvent;)V
-	public static fun createAppender (Ljava/lang/String;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Ljava/lang/String;Ljava/lang/Boolean;Lorg/apache/logging/log4j/core/Filter;)Lio/sentry/log4j2/SentryAppender;
+	public static fun createAppender (Ljava/lang/String;Lorg/apache/logging/log4j/Level;Lorg/apache/logging/log4j/Level;Ljava/lang/String;Ljava/lang/Boolean;Lorg/apache/logging/log4j/core/Filter;Ljava/lang/String;)Lio/sentry/log4j2/SentryAppender;
 	protected fun createBreadcrumb (Lorg/apache/logging/log4j/core/LogEvent;)Lio/sentry/Breadcrumb;
 	protected fun createEvent (Lorg/apache/logging/log4j/core/LogEvent;)Lio/sentry/SentryEvent;
 	public fun start ()V
diff --git a/sentry-log4j2/src/main/java/io/sentry/log4j2/SentryAppender.java b/sentry-log4j2/src/main/java/io/sentry/log4j2/SentryAppender.java
@@ -46,6 +46,7 @@ public class SentryAppender extends AbstractAppender {
   private @NotNull Level minimumEventLevel = Level.ERROR;
   private final @Nullable Boolean debug;
   private final @NotNull IHub hub;
+  private final @Nullable List<String> contextTags;
 
   public SentryAppender(
       final @NotNull String name,
@@ -55,7 +56,8 @@ public SentryAppender(
       final @Nullable Level minimumEventLevel,
       final @Nullable Boolean debug,
       final @Nullable ITransportFactory transportFactory,
-      final @NotNull IHub hub) {
+      final @NotNull IHub hub,
+      final @Nullable String[] contextTags) {
     super(name, filter, null, true, null);
     this.dsn = dsn;
     if (minimumBreadcrumbLevel != null) {
@@ -67,6 +69,7 @@ public SentryAppender(
     this.debug = debug;
     this.transportFactory = transportFactory;
     this.hub = hub;
+    this.contextTags = contextTags != null ? Arrays.asList(contextTags) : null;
   }
 
   /**
@@ -87,7 +90,8 @@ public SentryAppender(
       @Nullable @PluginAttribute("minimumEventLevel") final Level minimumEventLevel,
       @Nullable @PluginAttribute("dsn") final String dsn,
       @Nullable @PluginAttribute("debug") final Boolean debug,
-      @Nullable @PluginElement("filter") final Filter filter) {
+      @Nullable @PluginElement("filter") final Filter filter,
+      @Nullable @PluginAttribute("contextTags") final String contextTags) {
 
     if (name == null) {
       LOGGER.error("No name provided for SentryAppender");
@@ -101,7 +105,8 @@ public SentryAppender(
         minimumEventLevel,
         debug,
         null,
-        HubAdapter.getInstance());
+        HubAdapter.getInstance(),
+        contextTags != null ? contextTags.split(",") : null);
   }
 
   @Override
@@ -117,6 +122,11 @@ public void start() {
               }
               options.setSentryClientName(BuildConfig.SENTRY_LOG4J2_SDK_NAME);
               options.setSdkVersion(createSdkVersion(options));
+              if (contextTags != null) {
+                for (final String contextTag : contextTags) {
+                  options.addContextTag(contextTag);
+                }
+              }
               Optional.ofNullable(transportFactory).ifPresent(options::setTransportFactory);
             });
       } catch (IllegalArgumentException e) {
@@ -177,7 +187,20 @@ public void append(final @NotNull LogEvent eventObject) {
         CollectionUtils.filterMapEntries(
             loggingEvent.getContextData().toMap(), entry -> entry.getValue() != null);
     if (!contextData.isEmpty()) {
-      event.getContexts().put("Context Data", contextData);
+      if (contextTags != null && !contextTags.isEmpty()) {
+        for (final String contextTag : contextTags) {
+          // if mdc tag is listed in SentryOptions, apply as event tag
+          if (contextData.containsKey(contextTag)) {
+            event.setTag(contextTag, contextData.get(contextTag));
+            // remove from all tags applied to logging event
+            contextData.remove(contextTag);
+          }
+        }
+      }
+      // put the rest of mdc tags in contexts
+      if (!contextData.isEmpty()) {
+        event.getContexts().put("Context Data", contextData);
+      }
     }
 
     return event;
diff --git a/sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt b/sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt
@@ -43,13 +43,13 @@ class SentryAppenderTest {
             whenever(transportFactory.create(any(), any())).thenReturn(transport)
         }
 
-        fun getSut(transportFactory: ITransportFactory? = null, minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, debug: Boolean? = null): ExtendedLogger {
+        fun getSut(transportFactory: ITransportFactory? = null, minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, debug: Boolean? = null, contextTags: List<String>? = null): ExtendedLogger {
             if (transportFactory != null) {
                 this.transportFactory = transportFactory
             }
             loggerContext.start()
             val config: Configuration = loggerContext.configuration
-            val appender = SentryAppender("sentry", null, "http://key@localhost/proj", minimumBreadcrumbLevel, minimumEventLevel, debug, this.transportFactory, HubAdapter.getInstance())
+            val appender = SentryAppender("sentry", null, "http://key@localhost/proj", minimumBreadcrumbLevel, minimumEventLevel, debug, this.transportFactory, HubAdapter.getInstance(), contextTags?.toTypedArray())
             config.addAppender(appender)
 
             val ref = AppenderRef.createAppenderRef("sentry", null, null)
@@ -241,6 +241,22 @@ class SentryAppenderTest {
         )
     }
 
+    @Test
+    fun `sets tags from ThreadContext as Sentry tags`() {
+        val logger = fixture.getSut(minimumEventLevel = Level.WARN, contextTags = listOf("contextTag1"))
+        ThreadContext.put("key", "value")
+        ThreadContext.put("contextTag1", "contextTag1Value")
+        logger.warn("testing MDC tags")
+
+        verify(fixture.transport).send(
+            checkEvent { event ->
+                assertEquals(mapOf("key" to "value"), event.contexts["Context Data"])
+                assertEquals(mapOf("contextTag1" to "contextTag1Value"), event.tags)
+            },
+            anyOrNull()
+        )
+    }
+
     @Test
     fun `ignore set tags with null values from ThreadContext`() {
         val logger = fixture.getSut(minimumEventLevel = Level.WARN)
diff --git a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java
@@ -112,7 +112,20 @@ protected void append(@NotNull ILoggingEvent eventObject) {
         CollectionUtils.filterMapEntries(
             loggingEvent.getMDCPropertyMap(), entry -> entry.getValue() != null);
     if (!mdcProperties.isEmpty()) {
-      event.getContexts().put("MDC", mdcProperties);
+      if (!options.getContextTags().isEmpty()) {
+        for (final String contextTag : options.getContextTags()) {
+          // if mdc tag is listed in SentryOptions, apply as event tag
+          if (mdcProperties.containsKey(contextTag)) {
+            event.setTag(contextTag, mdcProperties.get(contextTag));
+            // remove from all tags applied to logging event
+            mdcProperties.remove(contextTag);
+          }
+        }
+      }
+      // put the rest of mdc tags in contexts
+      if (!mdcProperties.isEmpty()) {
+        event.getContexts().put("MDC", mdcProperties);
+      }
     }
 
     return event;
diff --git a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt
@@ -31,7 +31,7 @@ import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
 class SentryAppenderTest {
-    private class Fixture(dsn: String? = "http://key@localhost/proj", minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null) {
+    private class Fixture(dsn: String? = "http://key@localhost/proj", minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, contextTags: List<String>? = null) {
         val logger: Logger = LoggerFactory.getLogger(SentryAppenderTest::class.java)
         val loggerContext = LoggerFactory.getILoggerFactory() as LoggerContext
         val transportFactory = mock<ITransportFactory>()
@@ -43,6 +43,7 @@ class SentryAppenderTest {
             val appender = SentryAppender()
             val options = SentryOptions()
             options.dsn = dsn
+            contextTags?.forEach { options.addContextTag(it) }
             appender.setOptions(options)
             appender.setMinimumBreadcrumbLevel(minimumBreadcrumbLevel)
             appender.setMinimumEventLevel(minimumEventLevel)
@@ -217,6 +218,22 @@ class SentryAppenderTest {
         )
     }
 
+    @Test
+    fun `sets tags as sentry tags from MDC`() {
+        fixture = Fixture(minimumEventLevel = Level.WARN, contextTags = listOf("contextTag1"))
+        MDC.put("key", "value")
+        MDC.put("contextTag1", "contextTag1Value")
+        fixture.logger.warn("testing MDC tags")
+
+        verify(fixture.transport).send(
+            checkEvent { event ->
+                assertEquals(mapOf("key" to "value"), event.contexts["MDC"])
+                assertEquals(mapOf("contextTag1" to "contextTag1Value"), event.tags)
+            },
+            anyOrNull()
+        )
+    }
+
     @Test
     fun `ignore set tags with null values from MDC`() {
         fixture = Fixture(minimumEventLevel = Level.WARN)
diff --git a/sentry-samples/sentry-samples-jul/src/main/resources/sentry.properties b/sentry-samples/sentry-samples-jul/src/main/resources/sentry.properties
@@ -3,3 +3,4 @@ dsn=https://502f25099c204a2fbf4cb16edc5975d1@o447951.ingest.sentry.io/5428563
 debug=true
 environment=staging
 in-app-includes=io.sentry.samples
+context-tags=userId,requestId
diff --git a/sentry-samples/sentry-samples-log4j2/src/main/java/io/sentry/samples/log4j2/Main.java b/sentry-samples/sentry-samples-log4j2/src/main/java/io/sentry/samples/log4j2/Main.java
@@ -13,8 +13,11 @@ public static void main(String[] args) {
     // Update the DSN in log4j2.xml to see these events in your Sentry dashboard.
     LOGGER.debug("Hello Sentry!");
 
-    // ThreadContext parameters are converted to Sentry Event tags
+    // ThreadContext tags listed in log4j2.xml are converted to Sentry Event tags
     ThreadContext.put("userId", UUID.randomUUID().toString());
+    ThreadContext.put("requestId", UUID.randomUUID().toString());
+    // ThreadContext tag not listed in log4j2.xml
+    ThreadContext.put("context-tag", "context-tag-value");
 
     // logging arguments are converted to Sentry Event parameters
     LOGGER.info("User has made a purchase of product: {}", 445);
diff --git a/sentry-samples/sentry-samples-log4j2/src/main/resources/log4j2.xml b/sentry-samples/sentry-samples-log4j2/src/main/resources/log4j2.xml
@@ -11,6 +11,8 @@
             dsn="https://502f25099c204a2fbf4cb16edc5975d1@o447951.ingest.sentry.io/5428563"
             minimumBreadcrumbLevel="DEBUG"
             minimumEventLevel="WARN"
+            debug="true"
+            contextTags="userId,requestId"
     />
   </Appenders>
   <Loggers>
diff --git a/sentry-samples/sentry-samples-logback/src/main/java/io/sentry/samples/logback/Main.java b/sentry-samples/sentry-samples-logback/src/main/java/io/sentry/samples/logback/Main.java
@@ -11,8 +11,11 @@ public class Main {
   public static void main(String[] args) {
     LOGGER.debug("Hello Sentry!");
 
-    // MDC parameters are converted to Sentry Event tags
+    // MDC tags listed in logback.xml are converted to Sentry Event tags
     MDC.put("userId", UUID.randomUUID().toString());
+    MDC.put("requestId", UUID.randomUUID().toString());
+    // MDC tag not listed in logback.xml
+    MDC.put("context-tag", "context-tag-value");
 
     // logging arguments are converted to Sentry Event parameters
     LOGGER.info("User has made a purchase of product: {}", 445);
diff --git a/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml b/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml
@@ -11,6 +11,8 @@
       <debug>true</debug>
       <!-- NOTE: Replace the test DSN below with YOUR OWN DSN to see the events from this app in your Sentry project/dashboard -->
       <dsn>https://502f25099c204a2fbf4cb16edc5975d1@o447951.ingest.sentry.io/5428563</dsn>
+      <contextTag>userId</contextTag>
+      <contextTag>requestId</contextTag>
     </options>
     <!-- Demonstrates how to modify the minimum values -->
     <!-- Default for Events is ERROR -->
diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api
@@ -124,11 +124,13 @@ public abstract interface class io/sentry/EventProcessor {
 
 public final class io/sentry/ExternalOptions {
 	public fun <init> ()V
+	public fun addContextTag (Ljava/lang/String;)V
 	public fun addIgnoredExceptionForType (Ljava/lang/Class;)V
 	public fun addInAppExclude (Ljava/lang/String;)V
 	public fun addInAppInclude (Ljava/lang/String;)V
 	public fun addTracingOrigin (Ljava/lang/String;)V
 	public static fun from (Lio/sentry/config/PropertiesProvider;Lio/sentry/ILogger;)Lio/sentry/ExternalOptions;
+	public fun getContextTags ()Ljava/util/List;
 	public fun getDebug ()Ljava/lang/Boolean;
 	public fun getDist ()Ljava/lang/String;
 	public fun getDsn ()Ljava/lang/String;
@@ -993,6 +995,7 @@ public final class io/sentry/SentryLevel : java/lang/Enum, io/sentry/JsonSeriali
 
 public class io/sentry/SentryOptions {
 	public fun <init> ()V
+	public fun addContextTag (Ljava/lang/String;)V
 	public fun addEventProcessor (Lio/sentry/EventProcessor;)V
 	public fun addIgnoredExceptionForType (Ljava/lang/Class;)V
 	public fun addInAppExclude (Ljava/lang/String;)V
@@ -1004,6 +1007,7 @@ public class io/sentry/SentryOptions {
 	public fun getBeforeSend ()Lio/sentry/SentryOptions$BeforeSendCallback;
 	public fun getCacheDirPath ()Ljava/lang/String;
 	public fun getConnectionTimeoutMillis ()I
+	public fun getContextTags ()Ljava/util/List;
 	public fun getDiagnosticLevel ()Lio/sentry/SentryLevel;
 	public fun getDist ()Ljava/lang/String;
 	public fun getDistinctId ()Ljava/lang/String;
diff --git a/sentry/src/main/java/io/sentry/ExternalOptions.java b/sentry/src/main/java/io/sentry/ExternalOptions.java
@@ -32,6 +32,7 @@ public final class ExternalOptions {
   private final @NotNull List<String> inAppExcludes = new CopyOnWriteArrayList<>();
   private final @NotNull List<String> inAppIncludes = new CopyOnWriteArrayList<>();
   private final @NotNull List<String> tracingOrigins = new CopyOnWriteArrayList<>();
+  private final @NotNull List<String> contextTags = new CopyOnWriteArrayList<>();
   private @Nullable String proguardUuid;
   private final @NotNull Set<Class<? extends Throwable>> ignoredExceptionsForType =
       new CopyOnWriteArraySet<>();
@@ -81,6 +82,9 @@ public final class ExternalOptions {
     for (final String tracingOrigin : propertiesProvider.getList("tracing-origins")) {
       options.addTracingOrigin(tracingOrigin);
     }
+    for (final String contextTag : propertiesProvider.getList("context-tags")) {
+      options.addContextTag(contextTag);
+    }
     options.setProguardUuid(propertiesProvider.getProperty("proguard-uuid"));
 
     for (final String ignoredExceptionType :
@@ -212,6 +216,10 @@ public void setProxy(final @Nullable SentryOptions.Proxy proxy) {
     return inAppIncludes;
   }
 
+  public @NotNull List<String> getContextTags() {
+    return contextTags;
+  }
+
   public @Nullable String getProguardUuid() {
     return proguardUuid;
   }
@@ -236,6 +244,10 @@ public void addTracingOrigin(final @NotNull String tracingOrigin) {
     this.tracingOrigins.add(tracingOrigin);
   }
 
+  public void addContextTag(final @NotNull String contextTag) {
+    this.contextTags.add(contextTag);
+  }
+
   public void addIgnoredExceptionForType(final @NotNull Class<? extends Throwable> exceptionType) {
     this.ignoredExceptionsForType.add(exceptionType);
   }
diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java
diff --git a/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt b/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt
diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt
