Make closing scope idempotent and noop when different context is active

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+### Bugfixes
+
+* Make closing scope idempotent and non-operational when corresponding context is not current.
+  [(#5061)](https://github.com/open-telemetry/opentelemetry-java/pull/5061)
+*
 ## Version 1.21.0 (2022-12-09)
 
 ### API

context/src/main/java/io/opentelemetry/context/ThreadLocalContextStorage.java

@@ -7,6 +7,7 @@
 
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 enum ThreadLocalContextStorage implements ContextStorage {
@@ -30,14 +31,30 @@ public Scope attach(Context toAttach) {
 
     THREAD_LOCAL_STORAGE.set(toAttach);
 
-    return () -> {
-      if (current() != toAttach) {
+    return new ScopeImpl(beforeAttach, toAttach);
+  }
+
+  private class ScopeImpl implements Scope {
+    @Nullable private final Context beforeAttach;
+    private final Context toAttach;
+    private boolean closed;
+
+    private ScopeImpl(@Nullable Context beforeAttach, @Nonnull Context toAttach) {
+      this.beforeAttach = beforeAttach;
+      this.toAttach = toAttach;
+    }
+
+    @Override
+    public void close() {
+      if (!closed && current() == toAttach) {
+        closed = true;
+        THREAD_LOCAL_STORAGE.set(beforeAttach);
+      } else {
         logger.log(
             Level.FINE,
-            "Context in storage not the expected context, Scope.close was not called correctly");
+            " Trying to close scope which does not represent current context. Ignoring the call.");
       }
-      THREAD_LOCAL_STORAGE.set(beforeAttach);
-    };
+    }
   }
 
   @Override

context/src/test/java/io/opentelemetry/context/ContextTest.java

@@ -118,21 +118,42 @@ void newThreadStartsWithRoot() throws Exception {
   }
 
   @Test
-  public void closingScopeWhenNotActiveIsLogged() {
+  public void closingScopeWhenNotActiveIsNoopAndLogged() {
     Context initial = Context.current();
     Context context = initial.with(ANIMAL, "cat");
     try (Scope scope = context.makeCurrent()) {
       Context context2 = context.with(ANIMAL, "dog");
       try (Scope ignored = context2.makeCurrent()) {
         assertThat(Context.current().get(ANIMAL)).isEqualTo("dog");
         scope.close();
+        assertThat(Context.current().get(ANIMAL)).isEqualTo("dog");
       }
     }
     assertThat(Context.current()).isEqualTo(initial);
-    LoggingEvent log = logs.assertContains("Context in storage not the expected context");
+    LoggingEvent log =
+        logs.assertContains("Trying to close scope which does not represent current context");
     assertThat(log.getLevel()).isEqualTo(Level.DEBUG);
   }
 
+  @SuppressWarnings("MustBeClosedChecker")
+  @Test
+  public void closeScopeIsIdempotent() {
+    Context initial = Context.current();
+    Context context1 = Context.root().with(ANIMAL, "cat");
+    Scope scope1 = context1.makeCurrent();
+    Context context2 = context1.with(ANIMAL, "dog");
+    Scope scope2 = context2.makeCurrent();
+
+    scope2.close();
+    assertThat(Context.current()).isEqualTo(context1);
+
+    scope1.close();
+    assertThat(Context.current()).isEqualTo(initial);
+
+    scope2.close();
+    assertThat(Context.current()).isEqualTo(initial);
+  }
+
   @Test
   void withValues() {
     Context context1 = Context.current().with(ANIMAL, "cat");