Fix setting context tags on events captured by Spring (#2060)

* Fix setting context tags on events captured by Spring Fixes #2043 * Changelog. * Move ContextTagsEventProcessor from spring boot to spring module (#2065) * Fixes after merge Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com> Co-authored-by: Alexander Dinauer <alexander.dinauer@sentry.io>
getsentry · May 24, 2022 · 4835920 · 4835920
1 parent 6270232
commit 4835920
Show file tree

Hide file tree

Showing 7 changed files with 161 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Fixes
 
 - No longer close OutputStream that is passed into JsonSerializer (#2029)
+- Fix setting context tags on events captured by Spring (#2060)
 
 ### Features
 

diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java
@@ -9,6 +9,7 @@
 import io.sentry.Sentry;
 import io.sentry.SentryOptions;
 import io.sentry.protocol.SdkVersion;
+import io.sentry.spring.ContextTagsEventProcessor;
 import io.sentry.spring.SentryExceptionResolver;
 import io.sentry.spring.SentryRequestResolver;
 import io.sentry.spring.SentrySpringFilter;
@@ -29,6 +30,7 @@
 import javax.servlet.http.HttpServletRequest;
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.jetbrains.annotations.NotNull;
+import org.slf4j.MDC;
 import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.boot.autoconfigure.AutoConfigureBefore;
 import org.springframework.boot.autoconfigure.condition.AnyNestedCondition;
@@ -122,6 +124,18 @@ static class HubConfiguration {
       return HubAdapter.getInstance();
     }
 
+    @Configuration(proxyBeanMethods = false)
+    @ConditionalOnClass(MDC.class)
+    @Open
+    static class ContextTagsEventProcessorConfiguration {
+
+      @Bean
+      public @NotNull ContextTagsEventProcessor contextTagsEventProcessor(
+          final @NotNull SentryOptions sentryOptions) {
+        return new ContextTagsEventProcessor(sentryOptions);
+      }
+    }
+
     /** Registers beans specific to Spring MVC. */
     @Configuration(proxyBeanMethods = false)
     @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)

diff --git a/...-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/...-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt
@@ -21,6 +21,7 @@ import io.sentry.SentryLevel
 import io.sentry.SentryOptions
 import io.sentry.checkEvent
 import io.sentry.protocol.User
+import io.sentry.spring.ContextTagsEventProcessor
 import io.sentry.spring.HttpServletRequestSentryUserProvider
 import io.sentry.spring.SentryExceptionResolver
 import io.sentry.spring.SentryUserFilter
@@ -32,6 +33,7 @@ import io.sentry.transport.ITransportGate
 import io.sentry.transport.apache.ApacheHttpClientTransportFactory
 import org.aspectj.lang.ProceedingJoinPoint
 import org.assertj.core.api.Assertions.assertThat
+import org.slf4j.MDC
 import org.springframework.aop.support.NameMatchMethodPointcut
 import org.springframework.boot.autoconfigure.AutoConfigurations
 import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration
@@ -618,6 +620,27 @@ class SentryAutoConfigurationTest {
             }
     }
 
+    @Test
+    fun `when MDC is on the classpath, creates ContextTagsEventProcessor`() {
+        contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj")
+            .run {
+                assertThat(it).hasSingleBean(ContextTagsEventProcessor::class.java)
+                val options = it.getBean(SentryOptions::class.java)
+                assertThat(options.eventProcessors).anyMatch { processor -> processor.javaClass == ContextTagsEventProcessor::class.java }
+            }
+    }
+
+    @Test
+    fun `when MDC is not on the classpath, does not create ContextTagsEventProcessor`() {
+        contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj")
+            .withClassLoader(FilteredClassLoader(MDC::class.java))
+            .run {
+                assertThat(it).doesNotHaveBean(ContextTagsEventProcessor::class.java)
+                val options = it.getBean(SentryOptions::class.java)
+                assertThat(options.eventProcessors).noneMatch { processor -> processor.javaClass == ContextTagsEventProcessor::class.java }
+            }
+    }
+
     @Configuration(proxyBeanMethods = false)
     open class CustomOptionsConfigurationConfiguration {
 

diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api
@@ -3,6 +3,11 @@ public final class io/sentry/spring/BuildConfig {
 	public static final field VERSION_NAME Ljava/lang/String;
 }
 
+public final class io/sentry/spring/ContextTagsEventProcessor : io/sentry/EventProcessor {
+	public fun <init> (Lio/sentry/SentryOptions;)V
+	public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent;
+}
+
 public abstract interface annotation class io/sentry/spring/EnableSentry : java/lang/annotation/Annotation {
 	public abstract fun dsn ()Ljava/lang/String;
 	public abstract fun exceptionResolverOrder ()I

diff --git a/sentry-spring/build.gradle.kts b/sentry-spring/build.gradle.kts
@@ -37,6 +37,7 @@ dependencies {
     compileOnly(Config.Libs.springSecurityWeb)
     compileOnly(Config.Libs.aspectj)
     compileOnly(Config.Libs.servletApi)
+    compileOnly(Config.Libs.slf4jApi)
 
     compileOnly(Config.Libs.springWebflux)
 

diff --git a/sentry-spring/src/main/java/io/sentry/spring/ContextTagsEventProcessor.java b/sentry-spring/src/main/java/io/sentry/spring/ContextTagsEventProcessor.java
@@ -0,0 +1,41 @@
+package io.sentry.spring;
+
+import io.sentry.EventProcessor;
+import io.sentry.Hint;
+import io.sentry.SentryEvent;
+import io.sentry.SentryOptions;
+import io.sentry.util.CollectionUtils;
+import java.util.Map;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.slf4j.MDC;
+
+/**
+ * Attaches context tags defined in {@link SentryOptions#getContextTags()} from {@link MDC} to
+ * {@link SentryEvent#getTags()}.
+ */
+public final class ContextTagsEventProcessor implements EventProcessor {
+  private final SentryOptions options;
+
+  public ContextTagsEventProcessor(final @NotNull SentryOptions options) {
+    this.options = options;
+  }
+
+  @Override
+  public @NotNull SentryEvent process(@NotNull SentryEvent event, @Nullable Hint hint) {
+    final Map<String, String> contextMap = MDC.getCopyOfContextMap();
+    if (contextMap != null) {
+      final Map<String, String> mdcProperties =
+          CollectionUtils.filterMapEntries(contextMap, entry -> entry.getValue() != null);
+      if (!mdcProperties.isEmpty() && !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));
+          }
+        }
+      }
+    }
+    return event;
+  }
+}
diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/ContextTagsEventProcessorTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/ContextTagsEventProcessorTest.kt
@@ -0,0 +1,76 @@
+package io.sentry.spring
+
+import io.sentry.SentryEvent
+import io.sentry.SentryOptions
+import org.slf4j.MDC
+import kotlin.test.BeforeTest
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNotNull
+import kotlin.test.assertTrue
+
+class ContextTagsEventProcessorTest {
+
+    class Fixture {
+
+        fun getSut(contextTags: List<String> = emptyList(), mdcTags: Map<String, String> = emptyMap()): ContextTagsEventProcessor {
+            val options = SentryOptions().apply { contextTags.forEach { tag -> addContextTag(tag) } }
+            val sut = ContextTagsEventProcessor(options)
+            mdcTags.forEach { MDC.put(it.key, it.value) }
+            return sut
+        }
+    }
+
+    private val fixture = Fixture()
+
+    @BeforeTest
+    fun before() {
+        MDC.clear()
+    }
+
+    @Test
+    fun `does not copy tags if no tags are set on options`() {
+        val sut = fixture.getSut()
+
+        val result = sut.process(SentryEvent(), null)
+        val tags = result.tags
+        assertTrue(tags == null || tags.isEmpty())
+    }
+
+    @Test
+    fun `copies mdc tags`() {
+        val sut = fixture.getSut(contextTags = listOf("user-id"), mdcTags = mapOf("user-id" to "user-id-value"))
+
+        val result = sut.process(SentryEvent(), null)
+        val tags = result.tags
+        assertNotNull(tags) {
+            assertTrue(it.containsKey("user-id"))
+            assertEquals("user-id-value", it["user-id"])
+        }
+    }
+
+    @Test
+    fun `does not copy tags not defined in options`() {
+        val sut = fixture.getSut(contextTags = listOf("user-id"), mdcTags = mapOf("user-id" to "user-id-value", "request-id" to "request-id-value"))
+
+        val result = sut.process(SentryEvent(), null)
+        val tags = result.tags
+        assertNotNull(tags) {
+            assertTrue(it.containsKey("user-id"))
+            assertFalse(it.containsKey("request-id"))
+        }
+    }
+
+    @Test
+    fun `does not copy tag not set in MDC`() {
+        val sut = fixture.getSut(contextTags = listOf("user-id", "another-tag"), mdcTags = mapOf("user-id" to "user-id-value"))
+
+        val result = sut.process(SentryEvent(), null)
+        val tags = result.tags
+        assertNotNull(tags) {
+            assertTrue(it.containsKey("user-id"))
+            assertFalse(it.containsKey("another-tag"))
+        }
+    }
+}