diff --git a/CHANGELOG.md b/CHANGELOG.md index c015f1e3c3d..de32df6a2e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,10 +49,10 @@ ### Installing `sentry-opentelemetry-agent` -### Upgrading from a previous agent +#### Upgrading from a previous agent If you've been using the previous version of `sentry-opentelemetry-agent`, simply replace the agent JAR with the [latest release](https://central.sonatype.com/artifact/io.sentry/sentry-opentelemetry-agent?smo=true) and start your application. That should be it. -### New to the agent +#### New to the agent If you've not been using OpenTelemetry before, you can add `sentry-opentelemetry-agent` to your setup by downloading the latest release and using it when starting up your application - `SENTRY_PROPERTIES_FILE=sentry.properties java -javaagent:sentry-opentelemetry-agent-x.x.x.jar -jar your-application.jar` - Please use `sentry.properties` or environment variables to configure the SDK as the agent is now in charge of initializing the SDK and options coming from things like logging integrations or our Spring Boot integration will not take effect. diff --git a/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/build.gradle.kts b/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/build.gradle.kts index 475796d2466..c0e002ee313 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/build.gradle.kts +++ b/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/build.gradle.kts @@ -24,6 +24,7 @@ dependencies { exclude(group = "io.opentelemetry") exclude(group = "io.opentelemetry.javaagent") } +// compileOnly(projects.sentryOpentelemetry.sentryOpentelemetryBootstrap) implementation(projects.sentryOpentelemetry.sentryOpentelemetryBootstrap) compileOnly(Config.Libs.OpenTelemetry.otelSdk) diff --git a/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/src/main/java/io/sentry/opentelemetry/SentryAutoConfigurationCustomizerProvider.java b/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/src/main/java/io/sentry/opentelemetry/SentryAutoConfigurationCustomizerProvider.java index 2b1b79c5217..66b55a4f0be 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/src/main/java/io/sentry/opentelemetry/SentryAutoConfigurationCustomizerProvider.java +++ b/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/src/main/java/io/sentry/opentelemetry/SentryAutoConfigurationCustomizerProvider.java @@ -32,9 +32,19 @@ public final class SentryAutoConfigurationCustomizerProvider public void customize(AutoConfigurationCustomizer autoConfiguration) { final @Nullable VersionInfoHolder versionInfoHolder = createVersionInfo(); - ContextStorage.addWrapper((storage) -> new SentryContextStorage(storage)); final @NotNull OtelSpanFactory spanFactory = new OtelSpanFactory(); SentrySpanFactoryHolder.setSpanFactory(spanFactory); + /** + * We're currently overriding the storage mechanism to allow for cleanup of non closed OTel + * scopes. These happen when using e.g. Sentry static API due to getCurrentScopes() invoking + * Context.makeCurrent and then ignoring the returned lifecycle token (OTel Scope). After fixing + * the classloader problem (sentry bootstrap dependency is currently in agent classloader) we + * can revisit and try again to set the storage instead of overriding it in the wrapper. We + * should try to use OTels StorageProvider mechanism instead. + */ + // ContextStorage.addWrapper((storage) -> new SentryContextStorage(storage)); + ContextStorage.addWrapper( + (storage) -> new SentryContextStorage(new SentryOtelThreadLocalStorage())); if (isSentryAutoInitEnabled()) { Sentry.init( 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 31aaa4ba9a1..0511fed18dc 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api @@ -164,6 +164,12 @@ public final class io/sentry/opentelemetry/SentryOtelKeys { public fun ()V } +public final class io/sentry/opentelemetry/SentryOtelThreadLocalStorage : io/opentelemetry/context/ContextStorage { + public fun ()V + public fun attach (Lio/opentelemetry/context/Context;)Lio/opentelemetry/context/Scope; + public fun current ()Lio/opentelemetry/context/Context; +} + public final class io/sentry/opentelemetry/SentryWeakSpanStorage { public static fun getInstance ()Lio/sentry/opentelemetry/SentryWeakSpanStorage; public fun getSentrySpan (Lio/opentelemetry/api/trace/SpanContext;)Lio/sentry/opentelemetry/OtelSpanWrapper; diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryOtelThreadLocalStorage.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryOtelThreadLocalStorage.java new file mode 100644 index 00000000000..e44afc5a2dc --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryOtelThreadLocalStorage.java @@ -0,0 +1,85 @@ +/* + * Adapted from https://github.com/open-telemetry/opentelemetry-java/blob/0aacc55d1e3f5cc6dbb4f8fa26bcb657b01a7bc9/context/src/main/java/io/opentelemetry/context/ThreadLocalContextStorage.java + * + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.sentry.opentelemetry; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextStorage; +import io.opentelemetry.context.Scope; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; + +/** + * Workaround to make OpenTelemetry context storage work for Sentry since Sentry sometimes forks + * Context without cleaning up. We are not yet sure if this is something we can easliy fix, since + * Sentry static API makes heavy use of getCurrentScopes and there is no easy way of knowing when to + * restore previous Context. + */ +@ApiStatus.Experimental +@ApiStatus.Internal +public final class SentryOtelThreadLocalStorage implements ContextStorage { + private static final Logger logger = + Logger.getLogger(SentryOtelThreadLocalStorage.class.getName()); + + private static final ThreadLocal THREAD_LOCAL_STORAGE = new ThreadLocal<>(); + + @Override + public Scope attach(Context toAttach) { + if (toAttach == null) { + // Null context not allowed so ignore it. + return NoopScope.INSTANCE; + } + + Context beforeAttach = current(); + if (toAttach == beforeAttach) { + return NoopScope.INSTANCE; + } + + THREAD_LOCAL_STORAGE.set(toAttach); + + return new SentryScopeImpl(beforeAttach); + } + + private static class SentryScopeImpl implements Scope { + @Nullable private final Context beforeAttach; + private boolean closed; + + private SentryScopeImpl(@Nullable Context beforeAttach) { + this.beforeAttach = beforeAttach; + } + + @Override + public void close() { + // if (!closed && current() == toAttach) { + // Used to make OTel thread local storage compatible with Sentry where cleanup isn't always + // performed correctly + if (!closed) { + closed = true; + THREAD_LOCAL_STORAGE.set(beforeAttach); + } else { + logger.log( + Level.FINE, + " Trying to close scope which does not represent current context. Ignoring the call."); + } + } + } + + @Override + @Nullable + public Context current() { + return THREAD_LOCAL_STORAGE.get(); + } + + enum NoopScope implements Scope { + INSTANCE; + + @Override + public void close() {} + } +}