Fix instrumentation:

- do not finish parent span
- handle exceptions thrown by data fetchers
- set parent span status if any of child spans fails

sentry-graphql-java/src/main/java/io/sentry/graphql/SentryInstrumentation.java

@@ -1,6 +1,7 @@
 package io.sentry.graphql;
 
 import graphql.ExecutionResult;
+import graphql.GraphQLError;
 import graphql.execution.instrumentation.InstrumentationContext;
 import graphql.execution.instrumentation.InstrumentationState;
 import graphql.execution.instrumentation.SimpleInstrumentation;
@@ -13,7 +14,9 @@
 import io.sentry.HubAdapter;
 import io.sentry.IHub;
 import io.sentry.ISpan;
+import io.sentry.SpanStatus;
 import io.sentry.util.Objects;
+import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -57,17 +60,28 @@ public SentryInstrumentation() {
       final ISpan transaction = tracingState.getTransaction();
       if (transaction != null) {
         final ISpan span = transaction.startChild(findDataFetcherTag(parameters));
-        final Object result = dataFetcher.get(environment);
-        if (result instanceof CompletableFuture) {
-          ((CompletableFuture<?>) result)
-              .whenComplete(
-                  (r, ex) -> {
-                    span.finish();
-                  });
-        } else {
-          span.finish();
+        try {
+          final Object result = dataFetcher.get(environment);
+          if (result instanceof CompletableFuture) {
+            ((CompletableFuture<?>) result)
+                .whenComplete(
+                    (r, ex) -> {
+                      if (ex != null) {
+                        span.setThrowable(ex);
+                        span.finish(SpanStatus.INTERNAL_ERROR);
+                      } else {
+                        span.finish();
+                      }
+                    });
+          } else {
+            span.finish();
+          }
+          return result;
+        } catch (Exception e) {
+          span.setThrowable(e);
+          span.finish(SpanStatus.INTERNAL_ERROR);
+          throw e;
         }
-        return result;
       } else {
         return dataFetcher.get(environment);
       }
@@ -81,8 +95,9 @@ public SentryInstrumentation() {
     TracingState tracingState = parameters.getInstrumentationState();
     final ISpan transaction = tracingState.getTransaction();
 
-    if (transaction != null) {
-      transaction.finish();
+    final List<GraphQLError> errors = executionResult.getErrors();
+    if (transaction != null && errors != null && !errors.isEmpty()) {
+      transaction.setStatus(SpanStatus.INTERNAL_ERROR);
     }
 
     return super.instrumentExecutionResult(executionResult, parameters);

sentry-graphql-java/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt

@@ -11,17 +11,19 @@ import graphql.schema.idl.SchemaGenerator
 import graphql.schema.idl.SchemaParser
 import io.sentry.IHub
 import io.sentry.ISpan
+import io.sentry.SpanStatus
+import java.lang.RuntimeException
 import kotlin.random.Random
 import kotlin.test.Test
 import kotlin.test.assertTrue
 
 class SentryInstrumentationTest {
 
     class Fixture {
-        val transaction = mock<ISpan>()
+        val activeSpan = mock<ISpan>()
         val innerSpan = mock<ISpan>()
 
-        fun getSut(isTransactionActive: Boolean = true): GraphQL {
+        fun getSut(isTransactionActive: Boolean = true, dataFetcherThrows: Boolean = false): GraphQL {
             val schema = """
             type Query {
                 shows: [Show]
@@ -33,25 +35,29 @@ class SentryInstrumentationTest {
             """.trimIndent()
             val hub = mock<IHub>()
 
-            val graphQLSchema = SchemaGenerator().makeExecutableSchema(SchemaParser().parse(schema), buildRuntimeWiring())
+            val graphQLSchema = SchemaGenerator().makeExecutableSchema(SchemaParser().parse(schema), buildRuntimeWiring(dataFetcherThrows))
             val graphQL = GraphQL.newGraphQL(graphQLSchema)
                 .instrumentation(SentryInstrumentation(hub))
                 .build()
 
             if (isTransactionActive) {
-                whenever(hub.span).thenReturn(transaction)
+                whenever(hub.span).thenReturn(activeSpan)
             } else {
                 whenever(hub.span).thenReturn(null)
             }
-            whenever(transaction.startChild(any())).thenReturn(innerSpan)
+            whenever(activeSpan.startChild(any())).thenReturn(innerSpan)
 
             return graphQL
         }
 
-        private fun buildRuntimeWiring() = RuntimeWiring.newRuntimeWiring()
+        private fun buildRuntimeWiring(dataFetcherThrows: Boolean) = RuntimeWiring.newRuntimeWiring()
             .type("Query") {
                 it.dataFetcher("shows") {
-                    listOf(Show(Random.nextInt()), Show(Random.nextInt()))
+                    if (dataFetcherThrows) {
+                        throw RuntimeException("error")
+                    } else {
+                        listOf(Show(Random.nextInt()), Show(Random.nextInt()))
+                    }
                 }
             }.build()
     }
@@ -65,19 +71,32 @@ class SentryInstrumentationTest {
         val result = sut.execute("{ shows { id } }")
 
         assertTrue(result.errors.isEmpty())
-        verify(fixture.transaction).startChild("Query.shows")
+        verify(fixture.activeSpan).startChild("Query.shows")
         verify(fixture.innerSpan).finish()
-        verify(fixture.transaction).finish()
+        verifyNoMoreInteractions(fixture.activeSpan)
     }
 
     @Test
-    fun `when transaction is not , does not create spans`() {
+    fun `when transaction is active, and data fetcher throws, creates inner spans`() {
+        val sut = fixture.getSut(dataFetcherThrows = true)
+
+        val result = sut.execute("{ shows { id } }")
+
+        assertTrue(result.errors.isNotEmpty())
+        verify(fixture.activeSpan).startChild("Query.shows")
+        verify(fixture.innerSpan).finish(SpanStatus.INTERNAL_ERROR)
+        verify(fixture.activeSpan).status = SpanStatus.INTERNAL_ERROR
+        verifyNoMoreInteractions(fixture.activeSpan)
+    }
+
+    @Test
+    fun `when transaction is not active, does not create spans`() {
         val sut = fixture.getSut(isTransactionActive = false)
 
         val result = sut.execute("{ shows { id } }")
 
         assertTrue(result.errors.isEmpty())
-        verifyNoMoreInteractions(fixture.transaction)
+        verifyNoMoreInteractions(fixture.activeSpan)
         verifyNoMoreInteractions(fixture.innerSpan)
     }
 

sentry-samples/sentry-samples-netflix-dgs/src/main/java/io/sentry/samples/netflix/dgs/ShowsDatafetcher.java

@@ -4,6 +4,7 @@
 import com.netflix.graphql.dgs.DgsQuery;
 import io.sentry.samples.netflix.dgs.graphql.DgsConstants;
 import io.sentry.samples.netflix.dgs.graphql.types.Show;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Random;
 
@@ -13,11 +14,14 @@ public class ShowsDatafetcher {
   @DgsQuery(field = DgsConstants.QUERY.Shows)
   public List<Show> shows() throws InterruptedException {
     Thread.sleep(new Random().nextInt(500));
+    return Arrays.asList(
+        Show.newBuilder().id(1).title("Stranger Things").releaseYear(2015).build(),
+        Show.newBuilder().id(2).title("Breaking Bad").releaseYear(2013).build());
+  }
 
-    //        return Arrays.asList(
-    //          Show.newBuilder().id(1).title("Stranger Things").releaseYear(2015).build(),
-    //          Show.newBuilder().id(2).title("Breaking Bad").releaseYear(2013).build()
-    //        );
-    throw new RuntimeException("foo");
+  @DgsQuery(field = DgsConstants.QUERY.NewShows)
+  public List<Show> newShows() throws InterruptedException {
+    Thread.sleep(new Random().nextInt(500));
+    throw new RuntimeException("error when loading new shows");
   }
 }

sentry-samples/sentry-samples-netflix-dgs/src/main/java/io/sentry/samples/netflix/dgs/graphql/DgsConstants.java

@@ -7,6 +7,8 @@ public static class QUERY {
     public static final String TYPE_NAME = "Query";
 
     public static final String Shows = "shows";
+
+    public static final String NewShows = "newShows";
   }
 
   public static class SHOW {

sentry-samples/sentry-samples-netflix-dgs/src/main/resources/schema/schema.graphqls

@@ -1,5 +1,6 @@
 type Query {
     shows: [Show]
+    newShows: [Show]
 }
 
 type Show {