SDKs send queue is no longer shutdown immediately on re-init (#4564)

* Let queue drain on a restart

* Format code

* Format code

* Update sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/systemtest/ConsoleApplicationSystemTest.kt

* Let queue drain on a restart

* Format code

* Format code

* Update sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/systemtest/ConsoleApplicationSystemTest.kt

* adapt tests

* changelog

---------

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>

Author: adinauer

CHANGELOG.md

@@ -30,6 +30,8 @@
   - This was causing Sentry SDK to log warnings: "Sentry Log is disabled and this 'logger' call is a no-op."
 - Do not use Sentry logging API in Log4j2 if logs are disabled ([#4573](https://github.com/getsentry/sentry-java/pull/4573))
   - This was causing Sentry SDK to log warnings: "Sentry Log is disabled and this 'logger' call is a no-op."
+- SDKs send queue is no longer shutdown immediately on re-init ([#4564](https://github.com/getsentry/sentry-java/pull/4564))
+  - This means we're no longer losing events that have been enqueued right before SDK re-init.
 - Reduce scope forking when using OpenTelemetry ([#4565](https://github.com/getsentry/sentry-java/pull/4565))
   - `Sentry.withScope` now has the correct current scope passed to the callback. Previously our OpenTelemetry integration forked scopes an additional.
   - Overall the SDK is now forking scopes a bit less often.

sentry/src/main/java/io/sentry/ShutdownHookIntegration.java

@@ -32,7 +32,8 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
     Objects.requireNonNull(options, "SentryOptions is required");
 
     if (options.isEnableShutdownHook()) {
-      thread = new Thread(() -> scopes.flush(options.getFlushTimeoutMillis()));
+      thread =
+          new Thread(() -> scopes.flush(options.getFlushTimeoutMillis()), "sentry-shutdownhook");
       handleShutdownInProgress(
           () -> {
             runtime.addShutdownHook(thread);

sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java

@@ -174,20 +174,23 @@ public void close(final boolean isRestarting) throws IOException {
     executor.shutdown();
     options.getLogger().log(SentryLevel.DEBUG, "Shutting down");
     try {
-      // We need a small timeout to be able to save to disk any rejected envelope
-      long timeout = isRestarting ? 0 : options.getFlushTimeoutMillis();
-      if (!executor.awaitTermination(timeout, TimeUnit.MILLISECONDS)) {
-        options
-            .getLogger()
-            .log(
-                SentryLevel.WARNING,
-                "Failed to shutdown the async connection async sender  within "
-                    + timeout
-                    + " ms. Trying to force it now.");
-        executor.shutdownNow();
-        if (currentRunnable != null) {
-          // We store to disk any envelope that is currently being sent
-          executor.getRejectedExecutionHandler().rejectedExecution(currentRunnable, executor);
+      // only stop sending events on a real shutdown, not on a restart
+      if (!isRestarting) {
+        // We need a small timeout to be able to save to disk any rejected envelope
+        long timeout = options.getFlushTimeoutMillis();
+        if (!executor.awaitTermination(timeout, TimeUnit.MILLISECONDS)) {
+          options
+              .getLogger()
+              .log(
+                  SentryLevel.WARNING,
+                  "Failed to shutdown the async connection async sender  within "
+                      + timeout
+                      + " ms. Trying to force it now.");
+          executor.shutdownNow();
+          if (currentRunnable != null) {
+            // We store to disk any envelope that is currently being sent
+            executor.getRejectedExecutionHandler().rejectedExecution(currentRunnable, executor);
+          }
         }
       }
     } catch (InterruptedException e) {

sentry/src/test/java/io/sentry/transport/AsyncHttpTransportTest.kt

@@ -352,16 +352,22 @@ class AsyncHttpTransportTest {
   }
 
   @Test
-  fun `close with isRestarting true does not await termination`() {
-    fixture.sentryOptions.flushTimeoutMillis = 123
+  fun `close shuts down the executor and runs executing runnable through rejectedExecutionHandler`() {
+    val rejectedExecutionHandler = mock<RejectedExecutionHandler>()
     val sut = fixture.getSUT()
-    sut.close(true)
+    val runnable = mock<Runnable>()
 
-    verify(fixture.executor).awaitTermination(eq(0), eq(TimeUnit.MILLISECONDS))
+    // Emulate a runnable currently being executed
+    sut.injectForField("currentRunnable", runnable)
+    whenever(fixture.executor.rejectedExecutionHandler).thenReturn(rejectedExecutionHandler)
+    sut.close(false)
+
+    verify(fixture.executor).shutdownNow()
+    verify(rejectedExecutionHandler).rejectedExecution(eq(runnable), eq(fixture.executor))
   }
 
   @Test
-  fun `close shuts down the executor and runs executing runnable through rejectedExecutionHandler`() {
+  fun `does not shut down executor immediately on restart`() {
     val rejectedExecutionHandler = mock<RejectedExecutionHandler>()
     val sut = fixture.getSUT()
     val runnable = mock<Runnable>()
@@ -371,8 +377,9 @@ class AsyncHttpTransportTest {
     whenever(fixture.executor.rejectedExecutionHandler).thenReturn(rejectedExecutionHandler)
     sut.close(true)
 
-    verify(fixture.executor).shutdownNow()
-    verify(rejectedExecutionHandler).rejectedExecution(eq(runnable), eq(fixture.executor))
+    verify(fixture.executor).shutdown()
+    verify(fixture.executor, never()).shutdownNow()
+    verify(rejectedExecutionHandler, never()).rejectedExecution(eq(runnable), eq(fixture.executor))
   }
 
   @Test