diff --git a/CHANGELOG.md b/CHANGELOG.md index dd255f9fb2e..85dc62b5559 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,11 @@ - 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. ## 8.17.0 diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api index 8e6b59b4be2..3a63bf04d98 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api @@ -151,6 +151,7 @@ public final class io/sentry/opentelemetry/SentryContextStorage : io/opentelemet public fun (Lio/opentelemetry/context/ContextStorage;)V public fun attach (Lio/opentelemetry/context/Context;)Lio/opentelemetry/context/Scope; public fun current ()Lio/opentelemetry/context/Context; + public fun root ()Lio/opentelemetry/context/Context; } public final class io/sentry/opentelemetry/SentryContextStorageProvider : io/opentelemetry/context/ContextStorageProvider { diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextStorage.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextStorage.java index 4f3efa40c29..5a916a9ecab 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextStorage.java +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextStorage.java @@ -38,4 +38,9 @@ public Scope attach(Context toAttach) { public Context current() { return contextStorage.current(); } + + @Override + public Context root() { + return SentryContextWrapper.wrap(ContextStorage.super.root()); + } } diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextWrapper.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextWrapper.java index a0213bafe65..1b0581fc9e2 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextWrapper.java +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextWrapper.java @@ -32,7 +32,7 @@ public Context with(final @NotNull ContextKey contextKey, V v) { if (isOpentelemetrySpan(contextKey)) { return forkCurrentScope(modifiedContext); } else { - return modifiedContext; + return new SentryContextWrapper(modifiedContext); } } diff --git a/sentry-samples/sentry-samples-console-opentelemetry-noagent/build.gradle.kts b/sentry-samples/sentry-samples-console-opentelemetry-noagent/build.gradle.kts index 945d906df8a..8821c25626b 100644 --- a/sentry-samples/sentry-samples-console-opentelemetry-noagent/build.gradle.kts +++ b/sentry-samples/sentry-samples-console-opentelemetry-noagent/build.gradle.kts @@ -1,14 +1,83 @@ +import org.jetbrains.kotlin.gradle.tasks.KotlinCompile + plugins { java application + kotlin("jvm") alias(libs.plugins.gradle.versions) + id("com.github.johnrengelman.shadow") version "8.1.1" } application { mainClass.set("io.sentry.samples.console.Main") } +java.sourceCompatibility = JavaVersion.VERSION_17 + +java.targetCompatibility = JavaVersion.VERSION_17 + +repositories { mavenCentral() } + configure { - sourceCompatibility = JavaVersion.VERSION_1_8 - targetCompatibility = JavaVersion.VERSION_1_8 + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 +} + +tasks.withType().configureEach { + kotlinOptions.jvmTarget = JavaVersion.VERSION_17.toString() +} + +tasks.withType().configureEach { + kotlinOptions { + freeCompilerArgs = listOf("-Xjsr305=strict") + jvmTarget = JavaVersion.VERSION_17.toString() + } +} + +dependencies { + implementation(projects.sentryOpentelemetry.sentryOpentelemetryAgentless) + + testImplementation(kotlin(Config.kotlinStdLib)) + testImplementation(projects.sentry) + testImplementation(projects.sentrySystemTestSupport) + testImplementation(libs.kotlin.test.junit) + testImplementation(libs.slf4j.api) + testImplementation(libs.slf4j.jdk14) +} + +// Configure the Shadow JAR (executable JAR with all dependencies) +tasks.shadowJar { + manifest { attributes["Main-Class"] = "io.sentry.samples.console.Main" } + archiveClassifier.set("") // Remove the classifier so it replaces the regular JAR + mergeServiceFiles() } -dependencies { implementation(projects.sentryOpentelemetry.sentryOpentelemetryAgentless) } +// Make the regular jar task depend on shadowJar +tasks.jar { + enabled = false + dependsOn(tasks.shadowJar) +} + +// Fix the startScripts task dependency +tasks.startScripts { dependsOn(tasks.shadowJar) } + +configure { test { java.srcDir("src/test/java") } } + +tasks.register("systemTest").configure { + group = "verification" + description = "Runs the System tests" + + outputs.upToDateWhen { false } + + maxParallelForks = 1 + + // Cap JVM args per test + minHeapSize = "128m" + maxHeapSize = "1g" + + filter { includeTestsMatching("io.sentry.systemtest*") } +} + +tasks.named("test").configure { + require(this is Test) + + filter { excludeTestsMatching("io.sentry.systemtest.*") } +} diff --git a/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/main/java/io/sentry/samples/console/Main.java b/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/main/java/io/sentry/samples/console/Main.java index c27aad737b1..8af939c32bf 100644 --- a/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/main/java/io/sentry/samples/console/Main.java +++ b/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/main/java/io/sentry/samples/console/Main.java @@ -4,6 +4,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.context.Scope; +import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; import io.sentry.Breadcrumb; import io.sentry.EventProcessor; import io.sentry.Hint; @@ -17,90 +18,23 @@ import io.sentry.protocol.Message; import io.sentry.protocol.User; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; public class Main { public static void main(String[] args) throws InterruptedException { - Sentry.init( - options -> { - // NOTE: Replace the test DSN below with YOUR OWN DSN to see the events from this app in - // your Sentry project/dashboard - options.setDsn( - "https://502f25099c204a2fbf4cb16edc5975d1@o447951.ingest.sentry.io/5428563"); - - // All events get assigned to the release. See more at - // https://docs.sentry.io/workflow/releases/ - options.setRelease("io.sentry.samples.console@3.0.0+1"); - - // Modifications to event before it goes out. Could replace the event altogether - options.setBeforeSend( - (event, hint) -> { - // Drop an event altogether: - if (event.getTag("SomeTag") != null) { - return null; - } - return event; - }); - - options.setBeforeSendTransaction( - (transaction, hint) -> { - // Drop a transaction: - if (transaction.getTag("SomeTransactionTag") != null) { - return null; - } - - return transaction; - }); - - // Allows inspecting and modifying, returning a new or simply rejecting (returning null) - options.setBeforeBreadcrumb( - (breadcrumb, hint) -> { - // Don't add breadcrumbs with message containing: - if (breadcrumb.getMessage() != null - && breadcrumb.getMessage().contains("bad breadcrumb")) { - return null; - } - return breadcrumb; - }); - - // Configure the background worker which sends events to sentry: - // Wait up to 5 seconds before shutdown while there are events to send. - options.setShutdownTimeoutMillis(5000); - - // Enable SDK logging with Debug level - options.setDebug(true); - // To change the verbosity, use: - // By default it's DEBUG. - // options.setDiagnosticLevel( - // SentryLevel - // .ERROR); // A good option to have SDK debug log in prod is to use - // only level - // ERROR here. - options.setEnablePrettySerializationOutput(false); - - // Exclude frames from some packages from being "inApp" so are hidden by default in Sentry - // UI: - options.addInAppExclude("org.jboss"); - - // Include frames from our package - options.addInAppInclude("io.sentry.samples"); - - // Performance configuration options - // Set what percentage of traces should be collected - options.setTracesSampleRate(1.0); // set 0.5 to send 50% of traces - - // Determine traces sample rate based on the sampling context - // options.setTracesSampler( - // context -> { - // // only 10% of transactions with "/product" prefix will be collected - // if (!context.getTransactionContext().getName().startsWith("/products")) - // { - // return 0.1; - // } else { - // return 0.5; - // } - // }); - }); + AutoConfiguredOpenTelemetrySdk.builder() + .setResultAsGlobal() + .addPropertiesSupplier( + () -> { + final Map properties = new HashMap<>(); + properties.put("otel.logs.exporter", "none"); + properties.put("otel.metrics.exporter", "none"); + properties.put("otel.traces.exporter", "none"); + return properties; + }) + .build(); Sentry.addBreadcrumb( "A 'bad breadcrumb' that will be rejected because of 'BeforeBreadcrumb callback above.'"); diff --git a/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/DummyTest.kt b/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/DummyTest.kt new file mode 100644 index 00000000000..6f762b7e453 --- /dev/null +++ b/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/DummyTest.kt @@ -0,0 +1,12 @@ +package io.sentry + +import kotlin.test.Test +import kotlin.test.assertTrue + +class DummyTest { + @Test + fun `the only test`() { + // only needed to have more than 0 tests and not fail the build + assertTrue(true) + } +} diff --git a/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/systemtest/ConsoleApplicationSystemTest.kt b/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/systemtest/ConsoleApplicationSystemTest.kt new file mode 100644 index 00000000000..427c930653b --- /dev/null +++ b/sentry-samples/sentry-samples-console-opentelemetry-noagent/src/test/kotlin/sentry/systemtest/ConsoleApplicationSystemTest.kt @@ -0,0 +1,101 @@ +package io.sentry.systemtest + +import io.sentry.systemtest.util.TestHelper +import java.util.concurrent.TimeUnit +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +class ConsoleApplicationSystemTest { + lateinit var testHelper: TestHelper + + @Before + fun setup() { + testHelper = TestHelper("http://localhost:8000") + testHelper.reset() + } + + @Test + fun `console application sends expected events when run as JAR`() { + val jarFile = testHelper.findJar("sentry-samples-console-opentelemetry-noagent") + val process = + testHelper.launch( + jarFile, + mapOf( + "SENTRY_DSN" to testHelper.dsn, + // "SENTRY_AUTO_INIT" to "false", + "SENTRY_TRACES_SAMPLE_RATE" to "1.0", + "SENTRY_ENABLE_PRETTY_SERIALIZATION_OUTPUT" to "false", + "SENTRY_DEBUG" to "true", + "OTEL_METRICS_EXPORTER" to "none", + "OTEL_LOGS_EXPORTER" to "none", + "OTEL_TRACES_EXPORTER" to "none", + ), + ) + + process.waitFor(30, TimeUnit.SECONDS) + assertEquals(0, process.exitValue()) + + // Verify that we received the expected events + verifyExpectedEvents() + } + + private fun verifyExpectedEvents() { + // Verify we received a "Fatal message!" event + testHelper.ensureErrorReceived { event -> + event.message?.formatted == "Fatal message!" && event.level?.name == "FATAL" + } + + // Verify we received a "Some warning!" event + testHelper.ensureErrorReceived { event -> + event.message?.formatted == "Some warning!" && event.level?.name == "WARNING" + } + + // Verify we received the RuntimeException + testHelper.ensureErrorReceived { event -> + event.exceptions?.any { ex -> ex.type == "RuntimeException" && ex.value == "Some error!" } == + true + } + + // Verify we received the detailed event with fingerprint + testHelper.ensureErrorReceived { event -> + event.message?.message == "Detailed event" && + event.fingerprints?.contains("NewClientDebug") == true && + event.level?.name == "DEBUG" + } + + // Verify we received transaction events + testHelper.ensureTransactionReceived { transaction, _ -> + transaction.transaction == "transaction name" && + transaction.spans?.any { span -> span.op == "child" } == true + } + + // Verify we received the loop messages (should be 10 of them) + var loopMessageCount = 0 + try { + for (i in 0..9) { + testHelper.ensureErrorReceived { event -> + val matches = + event.message?.message?.contains("items we'll wait to flush to Sentry!") == true + if (matches) loopMessageCount++ + matches + } + } + } catch (e: Exception) { + // Some loop messages might be missing, but we should have at least some + } + + assertTrue( + "Should receive at least 5 loop messages, got $loopMessageCount", + loopMessageCount >= 5, + ) + + // Verify we have breadcrumbs + testHelper.ensureErrorReceived { event -> + event.breadcrumbs?.any { breadcrumb -> + breadcrumb.message?.contains("Processed by") == true + } == true + } + } +} diff --git a/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java b/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java index 16f5a09d1b4..7ee46649e97 100644 --- a/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java +++ b/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java @@ -66,8 +66,8 @@ public static void main(String[] args) throws InterruptedException { // Enable SDK logging with Debug level options.setDebug(true); // To change the verbosity, use: - // options.setDiagnosticLevel(SentryLevel.ERROR); // By default it's DEBUG. + // options.setDiagnosticLevel(SentryLevel.ERROR); // A good option to have SDK debug log in prod is to use only level ERROR here. // Exclude frames from some packages from being "inApp" so are hidden by default in Sentry @@ -82,15 +82,16 @@ public static void main(String[] args) throws InterruptedException { options.setTracesSampleRate(1.0); // set 0.5 to send 50% of traces // Determine traces sample rate based on the sampling context - // options.setTracesSampler( - // context -> { - // // only 10% of transactions with "/product" prefix will be collected - // if (!context.getTransactionContext().getName().startsWith("/products")) { - // return 0.1; - // } else { - // return 0.5; - // } - // }); + // options.setTracesSampler( + // context -> { + // // only 10% of transactions with "/product" prefix will be collected + // if (!context.getTransactionContext().getName().startsWith("/products")) + // { + // return 0.1; + // } else { + // return 0.5; + // } + // }); }); Sentry.addBreadcrumb( diff --git a/sentry-system-test-support/api/sentry-system-test-support.api b/sentry-system-test-support/api/sentry-system-test-support.api index 3e2653f717a..b32583074d4 100644 --- a/sentry-system-test-support/api/sentry-system-test-support.api +++ b/sentry-system-test-support/api/sentry-system-test-support.api @@ -576,7 +576,8 @@ public final class io/sentry/systemtest/util/TestHelper { public final fun getJsonSerializer ()Lio/sentry/JsonSerializer; public final fun getRestClient ()Lio/sentry/systemtest/util/RestTestClient; public final fun getSentryClient ()Lio/sentry/systemtest/util/SentryMockServerClient; - public final fun launch (Ljava/io/File;Ljava/util/Map;)Ljava/lang/Process; + public final fun launch (Ljava/io/File;Ljava/util/Map;Z)Ljava/lang/Process; + public static synthetic fun launch$default (Lio/sentry/systemtest/util/TestHelper;Ljava/io/File;Ljava/util/Map;ZILjava/lang/Object;)Ljava/lang/Process; public final fun logObject (Ljava/lang/Object;)V public final fun reset ()V public final fun setEnvelopeCounts (Lio/sentry/systemtest/util/EnvelopeCounts;)V diff --git a/sentry-system-test-support/src/main/kotlin/io/sentry/systemtest/util/TestHelper.kt b/sentry-system-test-support/src/main/kotlin/io/sentry/systemtest/util/TestHelper.kt index 56c699e995e..73b2acd2ae1 100644 --- a/sentry-system-test-support/src/main/kotlin/io/sentry/systemtest/util/TestHelper.kt +++ b/sentry-system-test-support/src/main/kotlin/io/sentry/systemtest/util/TestHelper.kt @@ -288,9 +288,18 @@ class TestHelper(backendUrl: String) { return jarFiles.maxOf { it } } - fun launch(jar: File, env: Map): Process { + fun launch(jar: File, env: Map, enableOtelAutoConfig: Boolean = false): Process { + val processBuilderList = mutableListOf("java", "--add-opens", "java.base/java.lang=ALL-UNNAMED") + + if (enableOtelAutoConfig) { + processBuilderList.add("-Dotel.java.global-autoconfigure.enabled=true") + } + + processBuilderList.add("-jar") + processBuilderList.add(jar.absolutePath) + val processBuilder = - ProcessBuilder("java", "-jar", jar.absolutePath).inheritIO() // forward i/o to current process + ProcessBuilder(processBuilderList).inheritIO() // forward i/o to current process processBuilder.environment().putAll(env) diff --git a/sentry/src/main/java/io/sentry/ShutdownHookIntegration.java b/sentry/src/main/java/io/sentry/ShutdownHookIntegration.java index 3d5bccc3d7b..9a70e31cee6 100644 --- a/sentry/src/main/java/io/sentry/ShutdownHookIntegration.java +++ b/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); diff --git a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java b/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java index 24f954c0c10..149c04f2f81 100644 --- a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java +++ b/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) { diff --git a/sentry/src/test/java/io/sentry/transport/AsyncHttpTransportTest.kt b/sentry/src/test/java/io/sentry/transport/AsyncHttpTransportTest.kt index c7ed2b46aa8..c3a7f2ce04f 100644 --- a/sentry/src/test/java/io/sentry/transport/AsyncHttpTransportTest.kt +++ b/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() val sut = fixture.getSUT() - sut.close(true) + val runnable = mock() - 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() val sut = fixture.getSUT() val runnable = mock() @@ -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 diff --git a/test/system-test-runner.py b/test/system-test-runner.py index 619e6bda104..fda5042c819 100644 --- a/test/system-test-runner.py +++ b/test/system-test-runner.py @@ -587,6 +587,7 @@ def get_available_modules(self) -> List[ModuleConfig]: ModuleConfig("sentry-samples-spring-boot-jakarta-opentelemetry", "true", "true", "false"), ModuleConfig("sentry-samples-spring-boot-jakarta-opentelemetry", "true", "false", "false"), ModuleConfig("sentry-samples-console", "false", "true", "false"), + ModuleConfig("sentry-samples-console-opentelemetry-noagent", "false", "true", "false"), ] def _find_module_number(self, module_name: str, agent: str, auto_init: str) -> int: