fix(node): Ensure graphql errors result in errored spans (#16678)

Closes #16649

Author: mydea

dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-error.js

@@ -0,0 +1,43 @@
+const Sentry = require('@sentry/node');
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+});
+
+// Stop the process from exiting before the transaction is sent
+setInterval(() => {}, 1000);
+
+async function run() {
+  const { gql } = require('apollo-server');
+  const server = require('./apollo-server')();
+
+  await Sentry.startSpan(
+    {
+      name: 'Test Transaction',
+      op: 'transaction',
+    },
+    async span => {
+      // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation
+      await server.executeOperation({
+        query: gql`
+          mutation Mutation($email: String) {
+            login(email: $email)
+          }
+        `,
+        // We want to trigger an error by passing an invalid variable type
+        variables: { email: 123 },
+      });
+
+      setTimeout(() => {
+        span.end();
+        server.stop();
+      }, 500);
+    },
+  );
+}
+
+run();

dev-packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts

@@ -55,4 +55,29 @@ describe('GraphQL/Apollo Tests', () => {
       .start()
       .completed();
   });
+
+  test('should handle GraphQL errors.', async () => {
+    const EXPECTED_TRANSACTION = {
+      transaction: 'Test Transaction (mutation Mutation)',
+      spans: expect.arrayContaining([
+        expect.objectContaining({
+          data: {
+            'graphql.operation.name': 'Mutation',
+            'graphql.operation.type': 'mutation',
+            'graphql.source': 'mutation Mutation($email: String) {\n  login(email: $email)\n}',
+            'sentry.origin': 'auto.graphql.otel.graphql',
+          },
+          description: 'mutation Mutation',
+          status: 'unknown_error',
+          origin: 'auto.graphql.otel.graphql',
+        }),
+      ]),
+    };
+
+    await createRunner(__dirname, 'scenario-error.js')
+      .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION })
+      .expect({ transaction: EXPECTED_TRANSACTION })
+      .start()
+      .completed();
+  });
 });

packages/node/src/integrations/tracing/graphql.ts

@@ -1,4 +1,5 @@
 import type { AttributeValue } from '@opentelemetry/api';
+import { SpanStatusCode } from '@opentelemetry/api';
 import { GraphQLInstrumentation } from '@opentelemetry/instrumentation-graphql';
 import type { IntegrationFn } from '@sentry/core';
 import { defineIntegration, getRootSpan, spanToJSON } from '@sentry/core';
@@ -45,9 +46,16 @@ export const instrumentGraphql = generateInstrumentOnce(
 
     return {
       ...options,
-      responseHook(span) {
+      responseHook(span, result) {
         addOriginToSpan(span, 'auto.graphql.otel.graphql');
 
+        // We want to ensure spans are marked as errored if there are errors in the result
+        // We only do that if the span is not already marked with a status
+        const resultWithMaybeError = result as { errors?: { message: string }[] };
+        if (resultWithMaybeError.errors?.length && !spanToJSON(span).status) {
+          span.setStatus({ code: SpanStatusCode.ERROR });
+        }
+
         const attributes = spanToJSON(span).data;
 
         // If operation.name is not set, we fall back to use operation.type only