POTEL 26 - Customize OpenTelemetry Scope.close behaviour

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.

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)

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(

sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api

@@ -164,6 +164,12 @@ public final class io/sentry/opentelemetry/SentryOtelKeys {
 	public fun <init> ()V
 }
 
+public final class io/sentry/opentelemetry/SentryOtelThreadLocalStorage : io/opentelemetry/context/ContextStorage {
+	public fun <init> ()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;

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<Context> 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() {}
+  }
+}