Merge branch 'main' into gh-1262

CHANGELOG.md

@@ -14,9 +14,14 @@
 * 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: Simplify configuring Logback integration when environment variable with the DSN is not set (#1271)
+* Fix: Prevent NoOpHub from creating heavy SentryOptions objects (#1272)
+* Enchancement: Add Request to the Scope. #1270
 * Fix: Fix SentryTransaction#getStatus NPE (#1273)
 * Enchancement: Optimize SentryTracingFilter when hub is disabled.
 * Fix: Discard unfinished Spans before sending them over to Sentry (#1279)
+* Fix: Interrupt the thread in QueuedThreadPoolExecutor (#1276)
+* Fix: SentryTransaction#finish should not clear another transaction from the scope (#1278)
 
 Breaking Changes:
 * Enchancement: SentryExceptionResolver should not send handled errors by default (#1248).

sentry-apache-http-client-5/src/test/kotlin/io/sentry/transport/apache/ApacheHttpClientTransportTest.kt

@@ -18,6 +18,8 @@ import io.sentry.SentryOptions
 import io.sentry.transport.RateLimiter
 import io.sentry.transport.ReusableCountLatch
 import java.util.concurrent.CompletableFuture
+import java.util.concurrent.Executors
+import kotlin.test.AfterTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import org.apache.hc.client5.http.async.methods.SimpleHttpResponse
@@ -34,6 +36,7 @@ class ApacheHttpClientTransportTest {
         val requestDetails = RequestDetails("http://key@localhost/proj", mapOf("header-name" to "header-value"))
         val client = mock<CloseableHttpAsyncClient>()
         val currentlyRunning = spy<ReusableCountLatch>()
+        val executorService = Executors.newFixedThreadPool(2)
 
         init {
             whenever(rateLimiter.filter(any(), anyOrNull())).thenAnswer { it.arguments[0] }
@@ -64,6 +67,11 @@ class ApacheHttpClientTransportTest {
 
     private val fixture = Fixture()
 
+    @AfterTest
+    fun `shutdown executor`() {
+        fixture.executorService.shutdownNow()
+    }
+
     @Test
     fun `updates retry on rate limiter`() {
         val response = SimpleHttpResponse(200)
@@ -113,7 +121,7 @@ class ApacheHttpClientTransportTest {
     fun `flush waits till all requests are finished`() {
         val sut = fixture.getSut()
         whenever(fixture.client.execute(any(), any())).then {
-            CompletableFuture.runAsync {
+            fixture.executorService.submit {
                 Thread.sleep(5)
                 (it.arguments[1] as FutureCallback<SimpleHttpResponse>).completed(SimpleHttpResponse(200))
             }
@@ -131,7 +139,7 @@ class ApacheHttpClientTransportTest {
     fun `keeps sending events after flush`() {
         val sut = fixture.getSut()
         whenever(fixture.client.execute(any(), any())).then {
-            CompletableFuture.runAsync {
+            fixture.executorService.submit {
                 Thread.sleep(5)
                 (it.arguments[1] as FutureCallback<SimpleHttpResponse>).completed(SimpleHttpResponse(200))
             }
@@ -150,22 +158,22 @@ class ApacheHttpClientTransportTest {
     fun `logs warning when flush timeout was lower than time needed to execute all events`() {
         val sut = fixture.getSut()
         whenever(fixture.client.execute(any(), any())).then {
-            CompletableFuture.runAsync {
-                Thread.sleep(100)
+            fixture.executorService.submit {
+                Thread.sleep(1000)
                 (it.arguments[1] as FutureCallback<SimpleHttpResponse>).completed(SimpleHttpResponse(200))
             }
         }.then {
-            CompletableFuture.runAsync {
-                Thread.sleep(5)
+            fixture.executorService.submit {
+                Thread.sleep(20)
                 (it.arguments[1] as FutureCallback<SimpleHttpResponse>).completed(SimpleHttpResponse(200))
             }
         }
         sut.send(SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null))
         sut.send(SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null))
 
-        sut.flush(10)
+        sut.flush(200)
 
+        verify(fixture.logger).log(SentryLevel.WARNING, "Failed to flush all events within %s ms", 200L)
         verify(fixture.currentlyRunning, times(1)).decrement()
-        verify(fixture.logger).log(SentryLevel.WARNING, "Failed to flush all events within %s ms", 10L)
     }
 }

sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java

@@ -35,14 +35,20 @@ public final class SentryAppender extends UnsynchronizedAppenderBase<ILoggingEve
   @Override
   public void start() {
     if (!Sentry.isEnabled()) {
-      options.setEnableExternalConfiguration(true);
-      options.setSentryClientName(BuildConfig.SENTRY_LOGBACK_SDK_NAME);
-      options.setSdkVersion(createSdkVersion(options));
-      Optional.ofNullable(transportFactory).ifPresent(options::setTransportFactory);
-      try {
-        Sentry.init(options);
-      } catch (IllegalArgumentException e) {
-        addWarn("Failed to init Sentry during appender initialization: " + e.getMessage());
+      if (options.getDsn() != null && !options.getDsn().endsWith("_IS_UNDEFINED")) {
+        options.setEnableExternalConfiguration(true);
+        options.setSentryClientName(BuildConfig.SENTRY_LOGBACK_SDK_NAME);
+        options.setSdkVersion(createSdkVersion(options));
+        Optional.ofNullable(transportFactory).ifPresent(options::setTransportFactory);
+        try {
+          Sentry.init(options);
+        } catch (IllegalArgumentException e) {
+          addWarn("Failed to init Sentry during appender initialization: " + e.getMessage());
+        }
+      } else {
+        options
+            .getLogger()
+            .log(SentryLevel.WARNING, "DSN is null. SentryAppender is not being initialized");
       }
     }
     super.start();

sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt

@@ -2,6 +2,7 @@ package io.sentry.logback
 
 import ch.qos.logback.classic.Level
 import ch.qos.logback.classic.LoggerContext
+import ch.qos.logback.core.status.Status
 import com.nhaarman.mockitokotlin2.any
 import com.nhaarman.mockitokotlin2.anyOrNull
 import com.nhaarman.mockitokotlin2.mock
@@ -29,7 +30,7 @@ import org.slf4j.LoggerFactory
 import org.slf4j.MDC
 
 class SentryAppenderTest {
-    private class Fixture(minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null) {
+    private class Fixture(dsn: String? = "http://key@localhost/proj", minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null) {
         val logger: Logger = LoggerFactory.getLogger(SentryAppenderTest::class.java)
         val loggerContext = LoggerFactory.getILoggerFactory() as LoggerContext
         val transportFactory = mock<ITransportFactory>()
@@ -40,7 +41,7 @@ class SentryAppenderTest {
             whenever(this.transportFactory.create(any(), any())).thenReturn(transport)
             val appender = SentryAppender()
             val options = SentryOptions()
-            options.dsn = "http://key@localhost/proj"
+            options.dsn = dsn
             appender.setOptions(options)
             appender.setMinimumBreadcrumbLevel(minimumBreadcrumbLevel)
             appender.setMinimumEventLevel(minimumEventLevel)
@@ -305,4 +306,12 @@ class SentryAppenderTest {
             }, anyOrNull())
         }
     }
+
+    @Test
+    fun `does not initialize Sentry when environment variable with DSN is not set`() {
+        // environment variables referenced in the logback.xml that are not set in the OS, have value "ENV_NAME_IS_UNDEFINED"
+        fixture = Fixture(dsn = "DSN_IS_UNDEFINED", minimumEventLevel = Level.DEBUG)
+
+        assertTrue(fixture.loggerContext.statusManager.copyOfStatusList.none { it.level == Status.WARN })
+    }
 }

sentry-samples/sentry-samples-logback/src/main/resources/logback.xml

@@ -8,6 +8,7 @@
 
   <appender name="sentry" class="io.sentry.logback.SentryAppender">
     <options>
+      <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>
     </options>

sentry/api/sentry.api

@@ -436,6 +436,7 @@ public final class io/sentry/Scope : java/lang/Cloneable {
 	public synthetic fun clone ()Ljava/lang/Object;
 	public fun getContexts ()Lio/sentry/protocol/Contexts;
 	public fun getLevel ()Lio/sentry/SentryLevel;
+	public fun getRequest ()Lio/sentry/protocol/Request;
 	public fun getSpan ()Lio/sentry/ISpan;
 	public fun getTransaction ()Lio/sentry/ITransaction;
 	public fun getTransactionName ()Ljava/lang/String;
@@ -450,6 +451,7 @@ public final class io/sentry/Scope : java/lang/Cloneable {
 	public fun setExtra (Ljava/lang/String;Ljava/lang/String;)V
 	public fun setFingerprint (Ljava/util/List;)V
 	public fun setLevel (Lio/sentry/SentryLevel;)V
+	public fun setRequest (Lio/sentry/protocol/Request;)V
 	public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 	public fun setTransaction (Lio/sentry/ITransaction;)V
 	public fun setTransaction (Ljava/lang/String;)V
@@ -1465,9 +1467,11 @@ public final class io/sentry/protocol/OperatingSystem : io/sentry/IUnknownProper
 	public fun setVersion (Ljava/lang/String;)V
 }
 
-public final class io/sentry/protocol/Request : io/sentry/IUnknownPropertiesConsumer {
+public final class io/sentry/protocol/Request : io/sentry/IUnknownPropertiesConsumer, java/lang/Cloneable {
 	public fun <init> ()V
 	public fun acceptUnknownProperties (Ljava/util/Map;)V
+	public fun clone ()Lio/sentry/protocol/Request;
+	public synthetic fun clone ()Ljava/lang/Object;
 	public fun getCookies ()Ljava/lang/String;
 	public fun getData ()Ljava/lang/Object;
 	public fun getEnvs ()Ljava/util/Map;

sentry/src/main/java/io/sentry/Hub.java

@@ -556,7 +556,10 @@ public void flush(long timeoutMillis) {
                   e);
         } finally {
           if (item != null) {
-            item.getScope().clearTransaction();
+            final Scope scope = item.getScope();
+            if (scope.getTransaction() == transaction) {
+              scope.clearTransaction();
+            }
           }
         }
       }

sentry/src/main/java/io/sentry/NoOpHub.java

@@ -10,6 +10,8 @@ final class NoOpHub implements IHub {
 
   private static final NoOpHub instance = new NoOpHub();
 
+  private final @NotNull SentryOptions emptyOptions = SentryOptions.empty();
+
   private NoOpHub() {}
 
   public static NoOpHub getInstance() {
@@ -144,6 +146,6 @@ public void setSpanContext(
 
   @Override
   public @NotNull SentryOptions getOptions() {
-    return new SentryOptions();
+    return emptyOptions;
   }
 }

sentry/src/main/java/io/sentry/Scope.java

@@ -1,6 +1,7 @@
 package io.sentry;
 
 import io.sentry.protocol.Contexts;
+import io.sentry.protocol.Request;
 import io.sentry.protocol.User;
 import io.sentry.util.Objects;
 import java.util.ArrayList;
@@ -29,6 +30,9 @@ public final class Scope implements Cloneable {
   /** Scope's user */
   private @Nullable User user;
 
+  /** Scope's request */
+  private @Nullable Request request;
+
   /** Scope's fingerprint */
   private @NotNull List<String> fingerprint = new ArrayList<>();
 
@@ -163,6 +167,24 @@ public void setUser(final @Nullable User user) {
     }
   }
 
+  /**
+   * Returns the Scope's request
+   *
+   * @return the request
+   */
+  public @Nullable Request getRequest() {
+    return request;
+  }
+
+  /**
+   * Sets the Scope's request
+   *
+   * @param request the request
+   */
+  public void setRequest(final @Nullable Request request) {
+    this.request = request;
+  }
+
   /**
    * Returns the Scope's fingerprint list
    *
@@ -287,6 +309,7 @@ public void clear() {
     level = null;
     transaction = null;
     user = null;
+    request = null;
     fingerprint.clear();
     breadcrumbs.clear();
     tags.clear();
@@ -489,6 +512,9 @@ public void addAttachment(final @NotNull Attachment attachment) {
     final User userRef = user;
     clone.user = userRef != null ? userRef.clone() : null;
 
+    final Request requestRef = request;
+    clone.request = requestRef != null ? requestRef.clone() : null;
+
     clone.fingerprint = new ArrayList<>(fingerprint);
     clone.eventProcessors = new CopyOnWriteArrayList<>(eventProcessors);
 

sentry/src/main/java/io/sentry/SentryClient.java

@@ -433,6 +433,9 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec
       if (event.getUser() == null) {
         event.setUser(scope.getUser());
       }
+      if (event.getRequest() == null) {
+        event.setRequest(scope.getRequest());
+      }
       if (event.getFingerprints() == null) {
         event.setFingerprints(scope.getFingerprint());
       }

sentry/src/main/java/io/sentry/SentryOptions.java

@@ -1301,22 +1301,45 @@ public interface TracesSamplerCallback {
     Double sample(@NotNull SamplingContext samplingContext);
   }
 
+  /**
+   * Creates SentryOptions instance without initializing any of the internal parts.
+   *
+   * <p>Used by {@link NoOpHub}.
+   *
+   * @return SentryOptions
+   */
+  static @NotNull SentryOptions empty() {
+    return new SentryOptions(true);
+  }
+
   /** SentryOptions ctor It adds and set default things */
   public SentryOptions() {
-    // SentryExecutorService should be inited before any SendCachedEventFireAndForgetIntegration
-    executorService = new SentryExecutorService();
+    this(false);
+  }
+
+  /**
+   * Creates SentryOptions instance without initializing any of the internal parts.
+   *
+   * @param empty if options should be empty.
+   */
+  private SentryOptions(final boolean empty) {
+    if (!empty) {
+      // SentryExecutorService should be initialized before any
+      // SendCachedEventFireAndForgetIntegration
+      executorService = new SentryExecutorService();
 
-    // UncaughtExceptionHandlerIntegration should be inited before any other Integration.
-    // if there's an error on the setup, we are able to capture it
-    integrations.add(new UncaughtExceptionHandlerIntegration());
+      // UncaughtExceptionHandlerIntegration should be inited before any other Integration.
+      // if there's an error on the setup, we are able to capture it
+      integrations.add(new UncaughtExceptionHandlerIntegration());
 
-    integrations.add(new ShutdownHookIntegration());
+      integrations.add(new ShutdownHookIntegration());
 
-    eventProcessors.add(new MainEventProcessor(this));
-    eventProcessors.add(new DuplicateEventDetectionEventProcessor(this));
+      eventProcessors.add(new MainEventProcessor(this));
+      eventProcessors.add(new DuplicateEventDetectionEventProcessor(this));
 
-    setSentryClientName(BuildConfig.SENTRY_JAVA_SDK_NAME + "/" + BuildConfig.VERSION_NAME);
-    setSdkVersion(createSdkVersion());
+      setSentryClientName(BuildConfig.SENTRY_JAVA_SDK_NAME + "/" + BuildConfig.VERSION_NAME);
+      setSdkVersion(createSdkVersion());
+    }
   }
 
   /**