Optimize SentryTracingFilter when hub is disabled. (#1275)

Fixes #1264

CHANGELOG.md

@@ -13,6 +13,7 @@
 * Fix: Make the ANR Atomic flags immutable
 * Enchancement: Integration interface better compatibility with Kotlin null-safety
 * Enchancement: Simplify Sentry configuration in Spring integration (#1259)
+* Enchancement: Optimize SentryTracingFilter when hub is disabled.
 
 Breaking Changes:
 * Enchancement: SentryExceptionResolver should not send handled errors by default (#1248).

sentry-spring/api/sentry-spring.api

@@ -105,6 +105,7 @@ public class io/sentry/spring/tracing/SentryTracingConfiguration {
 public class io/sentry/spring/tracing/SentryTracingFilter : org/springframework/web/filter/OncePerRequestFilter {
 	public fun <init> ()V
 	public fun <init> (Lio/sentry/IHub;Lio/sentry/spring/SentryRequestResolver;)V
+	public fun <init> (Lio/sentry/IHub;Lio/sentry/spring/SentryRequestResolver;Lio/sentry/spring/tracing/TransactionNameProvider;)V
 	protected fun doFilterInternal (Ljavax/servlet/http/HttpServletRequest;Ljavax/servlet/http/HttpServletResponse;Ljavax/servlet/FilterChain;)V
 }
 

sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java

@@ -36,8 +36,7 @@ public class SentryTracingFilter extends OncePerRequestFilter {
   /** Operation used by {@link SentryTransaction} created in {@link SentryTracingFilter}. */
   private static final String TRANSACTION_OP = "http.server";
 
-  private final @NotNull TransactionNameProvider transactionNameProvider =
-      new TransactionNameProvider();
+  private final @NotNull TransactionNameProvider transactionNameProvider;
   private final @NotNull IHub hub;
   private final @NotNull SentryRequestResolver requestResolver;
 
@@ -47,12 +46,21 @@ public SentryTracingFilter() {
 
   public SentryTracingFilter(
       final @NotNull IHub hub, final @NotNull SentryRequestResolver requestResolver) {
+    this(hub, requestResolver, new TransactionNameProvider());
+  }
+
+  public SentryTracingFilter(
+      final @NotNull IHub hub,
+      final @NotNull SentryRequestResolver requestResolver,
+      final @NotNull TransactionNameProvider transactionNameProvider) {
     this.hub = Objects.requireNonNull(hub, "hub is required");
     this.requestResolver = Objects.requireNonNull(requestResolver, "requestResolver is required");
+    this.transactionNameProvider =
+        Objects.requireNonNull(transactionNameProvider, "transactionNameProvider is required");
   }
 
   SentryTracingFilter(final @NotNull IHub hub) {
-    this(hub, new SentryRequestResolver(hub));
+    this(hub, new SentryRequestResolver(hub), new TransactionNameProvider());
   }
 
   @Override
@@ -62,24 +70,29 @@ protected void doFilterInternal(
       final @NotNull FilterChain filterChain)
       throws ServletException, IOException {
 
-    final String sentryTraceHeader = httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER);
+    if (hub.isEnabled()) {
+      final String sentryTraceHeader = httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER);
 
-    // at this stage we are not able to get real transaction name
-    final ITransaction transaction = startTransaction(httpRequest, sentryTraceHeader);
-    try {
-      filterChain.doFilter(httpRequest, httpResponse);
-    } finally {
-      // after all filters run, templated path pattern is available in request attribute
-      final String transactionName = transactionNameProvider.provideTransactionName(httpRequest);
-      // if transaction name is not resolved, the request has not been processed by a controller and
-      // we should not report it to Sentry
-      if (transactionName != null) {
-        transaction.setName(transactionName);
-        transaction.setOperation(TRANSACTION_OP);
-        transaction.setRequest(requestResolver.resolveSentryRequest(httpRequest));
-        transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus()));
-        transaction.finish();
+      // at this stage we are not able to get real transaction name
+      final ITransaction transaction = startTransaction(httpRequest, sentryTraceHeader);
+      try {
+        filterChain.doFilter(httpRequest, httpResponse);
+      } finally {
+        // after all filters run, templated path pattern is available in request attribute
+        final String transactionName = transactionNameProvider.provideTransactionName(httpRequest);
+        // if transaction name is not resolved, the request has not been processed by a controller
+        // and
+        // we should not report it to Sentry
+        if (transactionName != null) {
+          transaction.setName(transactionName);
+          transaction.setOperation(TRANSACTION_OP);
+          transaction.setRequest(requestResolver.resolveSentryRequest(httpRequest));
+          transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus()));
+          transaction.finish();
+        }
       }
+    } else {
+      filterChain.doFilter(httpRequest, httpResponse);
     }
   }
 

sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt

@@ -4,7 +4,10 @@ import com.nhaarman.mockitokotlin2.any
 import com.nhaarman.mockitokotlin2.check
 import com.nhaarman.mockitokotlin2.eq
 import com.nhaarman.mockitokotlin2.mock
+import com.nhaarman.mockitokotlin2.spy
 import com.nhaarman.mockitokotlin2.verify
+import com.nhaarman.mockitokotlin2.verifyNoMoreInteractions
+import com.nhaarman.mockitokotlin2.verifyZeroInteractions
 import com.nhaarman.mockitokotlin2.whenever
 import io.sentry.IHub
 import io.sentry.SentryOptions
@@ -14,6 +17,7 @@ import io.sentry.SpanId
 import io.sentry.SpanStatus
 import io.sentry.TransactionContext
 import io.sentry.protocol.SentryId
+import io.sentry.spring.SentryRequestResolver
 import javax.servlet.FilterChain
 import javax.servlet.http.HttpServletRequest
 import kotlin.test.Test
@@ -30,12 +34,14 @@ class SentryTracingFilterTest {
         val request = MockHttpServletRequest()
         val response = MockHttpServletResponse()
         val chain = mock<FilterChain>()
+        val sentryRequestResolver = spy(SentryRequestResolver(hub))
+        val transactionNameProvider = spy(TransactionNameProvider())
 
         init {
             whenever(hub.options).thenReturn(SentryOptions())
         }
 
-        fun getSut(sentryTraceHeader: String? = null): SentryTracingFilter {
+        fun getSut(isEnabled: Boolean = true, sentryTraceHeader: String? = null): SentryTracingFilter {
             request.requestURI = "/product/12"
             request.method = "POST"
             request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}")
@@ -45,7 +51,8 @@ class SentryTracingFilterTest {
             }
             response.status = 200
             whenever(hub.startTransaction(any<String>(), any(), any())).thenAnswer { SentryTransaction(it.arguments[0] as String, SpanContext(it.arguments[1] as String), hub) }
-            return SentryTracingFilter(hub)
+            whenever(hub.isEnabled).thenReturn(isEnabled)
+            return SentryTracingFilter(hub, sentryRequestResolver, transactionNameProvider)
         }
     }
 
@@ -92,4 +99,18 @@ class SentryTracingFilterTest {
             assertThat(it.contexts.trace!!.parentSpanId).isEqualTo(parentSpanId)
         }, eq(null))
     }
+
+    @Test
+    fun `when hub is disabled, components are not invoked`() {
+        val filter = fixture.getSut(isEnabled = false)
+
+        filter.doFilter(fixture.request, fixture.response, fixture.chain)
+
+        verify(fixture.chain).doFilter(fixture.request, fixture.response)
+
+        verify(fixture.hub).isEnabled
+        verifyNoMoreInteractions(fixture.hub)
+        verifyZeroInteractions(fixture.sentryRequestResolver)
+        verifyZeroInteractions(fixture.transactionNameProvider)
+    }
 }

sentry-spring/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker

@@ -0,0 +1 @@
+mock-maker-inline