fix(tracing): ensure CompositeTracer scopes are closed safely in LIFO order

Author: blakeli0

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/CompositeTracer.java

@@ -53,13 +53,36 @@ public CompositeTracer(List<ApiTracer> children) {
   public Scope inScope() {
     final List<Scope> childScopes = new ArrayList<>(children.size());
 
-    for (ApiTracer child : children) {
-      childScopes.add(child.inScope());
+    try {
+      for (ApiTracer child : children) {
+        childScopes.add(child.inScope());
+      }
+    } catch (RuntimeException e) {
+      for (int i = childScopes.size() - 1; i >= 0; i--) {
+        try {
+          childScopes.get(i).close();
+        } catch (RuntimeException suppressed) {
+          e.addSuppressed(suppressed);
+        }
+      }
+      throw e;
     }
 
     return () -> {
-      for (Scope childScope : childScopes) {
-        childScope.close();
+      RuntimeException exception = null;
+      for (int i = childScopes.size() - 1; i >= 0; i--) {
+        try {
+          childScopes.get(i).close();
+        } catch (RuntimeException e) {
+          if (exception == null) {
+            exception = e;
+          } else {
+            exception.addSuppressed(e);
+          }
+        }
+      }
+      if (exception != null) {
+        throw exception;
       }
     };
   }

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracer.java

@@ -85,6 +85,15 @@ public SpanTracer(Tracer tracer, ApiTracerContext apiTracerContext) {
     buildAttributes();
   }
 
+  @Override
+  public Scope inScope() {
+    if (attemptSpan == null) {
+      return () -> {};
+    }
+    final io.opentelemetry.context.Scope otelScope = attemptSpan.makeCurrent();
+    return () -> otelScope.close();
+  }
+
   private static String resolveAttemptSpanName(ApiTracerContext apiTracerContext) {
     if (apiTracerContext.transport() == ApiTracerContext.Transport.GRPC) {
       // gRPC Uses the full method name as span name.

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/CompositeTracerTest.java

@@ -29,6 +29,11 @@
  */
 package com.google.api.gax.tracing;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -38,6 +43,7 @@
 import java.util.Map;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.InOrder;
 
 class CompositeTracerTest {
 
@@ -53,7 +59,7 @@ void setUp() {
   }
 
   @Test
-  void testInScope() {
+  void testInScope_lifoOrder() {
     ApiTracer.Scope scope1 = mock(ApiTracer.Scope.class);
     ApiTracer.Scope scope2 = mock(ApiTracer.Scope.class);
 
@@ -63,10 +69,54 @@ void testInScope() {
     ApiTracer.Scope compositeScope = compositeTracer.inScope();
     compositeScope.close();
 
+    verify(child1).inScope();
+    verify(child2).inScope();
+
+    InOrder inOrder = inOrder(scope2, scope1);
+    inOrder.verify(scope2).close();
+    inOrder.verify(scope1).close();
+  }
+
+  @Test
+  void testInScope_childInScopeThrows() {
+    ApiTracer.Scope scope1 = mock(ApiTracer.Scope.class);
+    RuntimeException exception = new RuntimeException("Runtime Error");
+
+    when(child1.inScope()).thenReturn(scope1);
+    when(child2.inScope()).thenThrow(exception);
+
+    RuntimeException thrown = assertThrows(RuntimeException.class, () -> compositeTracer.inScope());
+
+    assertEquals(exception, thrown);
     verify(child1).inScope();
     verify(child2).inScope();
     verify(scope1).close();
-    verify(scope2).close();
+  }
+
+  @Test
+  void testInScope_childScopeCloseThrows() {
+    ApiTracer.Scope scope1 = mock(ApiTracer.Scope.class);
+    ApiTracer.Scope scope2 = mock(ApiTracer.Scope.class);
+
+    RuntimeException exception2 = new RuntimeException("Scope 2 close Error");
+    RuntimeException exception1 = new RuntimeException("Scope 1 close Error");
+
+    when(child1.inScope()).thenReturn(scope1);
+    when(child2.inScope()).thenReturn(scope2);
+
+    doThrow(exception2).when(scope2).close();
+    doThrow(exception1).when(scope1).close();
+
+    ApiTracer.Scope compositeScope = compositeTracer.inScope();
+
+    RuntimeException thrown = assertThrows(RuntimeException.class, () -> compositeScope.close());
+
+    assertEquals(exception2, thrown);
+    assertTrue(Arrays.asList(thrown.getSuppressed()).contains(exception1));
+
+    InOrder inOrder = inOrder(scope2, scope1);
+    inOrder.verify(scope2).close();
+    inOrder.verify(scope1).close();
   }
 
   @Test