fix(browser): Emit the outgoingRequestSpanStart hook after the span has started

Signed-off-by: Kaung Zin Hein <[email protected]>

Author: Zen-cronic

dev-packages/browser-integration-tests/suites/integrations/graphqlClient/fetch/test.ts

@@ -4,6 +4,15 @@ import type { Event } from '@sentry/types';
 import { sentryTest } from '../../../../utils/fixtures';
 import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
 
+// Duplicate from subject.js
+const query = `query Test{
+  people {
+    name
+    pet
+  }
+}`;
+const queryPayload = JSON.stringify({ query });
+
 sentryTest('should create spans for GraphQL Fetch requests', async ({ getLocalTestPath, page }) => {
   const url = await getLocalTestPath({ testDir: __dirname });
 
@@ -43,9 +52,7 @@ sentryTest('should create spans for GraphQL Fetch requests', async ({ getLocalTe
       'server.address': 'sentry-test.io',
       'sentry.op': 'http.client',
       'sentry.origin': 'auto.http.browser',
-      body: {
-        query: expect.any(String),
-      },
+      body: queryPayload,
     }),
   });
 });

dev-packages/browser-integration-tests/suites/integrations/graphqlClient/xhr/subject.js

@@ -5,11 +5,10 @@ xhr.setRequestHeader('Accept', 'application/json');
 xhr.setRequestHeader('Content-Type', 'application/json');
 
 const query = `query Test{
-
-    people {
-      name
-      pet
-    }
+  people {
+    name
+    pet
+  }
 }`;
 
 const requestBody = JSON.stringify({ query });

dev-packages/browser-integration-tests/suites/integrations/graphqlClient/xhr/test.ts

@@ -4,6 +4,15 @@ import type { Event } from '@sentry/types';
 import { sentryTest } from '../../../../utils/fixtures';
 import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
 
+// Duplicate from subject.js
+const query = `query Test{
+  people {
+    name
+    pet
+  }
+}`;
+const queryPayload = JSON.stringify({ query });
+
 sentryTest('should create spans for GraphQL XHR requests', async ({ getLocalTestPath, page }) => {
   const url = await getLocalTestPath({ testDir: __dirname });
 
@@ -43,9 +52,7 @@ sentryTest('should create spans for GraphQL XHR requests', async ({ getLocalTest
       'server.address': 'sentry-test.io',
       'sentry.op': 'http.client',
       'sentry.origin': 'auto.http.browser',
-      body: {
-        query: expect.any(String),
-      },
+      body: queryPayload,
     },
   });
 });

packages/browser/src/integrations/graphqlClient.ts

@@ -12,17 +12,19 @@ interface GraphQLClientOptions {
   endpoints: Array<string>;
 }
 
+interface GraphQLRequestPayload {
+  query: string;
+  operationName?: string;
+  variables?: Record<string, any>;
+}
+
 const INTEGRATION_NAME = 'GraphQLClient';
 
 const _graphqlClientIntegration = ((options: GraphQLClientOptions) => {
   return {
     name: INTEGRATION_NAME,
     setup(client) {
-      client.on('spanStart', span => {
-        client.emit('outgoingRequestSpanStart', span);
-      });
-
-      client.on('outgoingRequestSpanStart', span => {
+      client.on('outgoingRequestSpanStart', (span, { body }) => {
         const spanJSON = spanToJSON(span);
 
         const spanAttributes = spanJSON.data || {};
@@ -38,19 +40,17 @@ const _graphqlClientIntegration = ((options: GraphQLClientOptions) => {
 
           if (isTracedGraphqlEndpoint) {
             const httpMethod = spanAttributes[SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD] || spanAttributes['http.method'];
-            const graphqlBody = spanAttributes['body'];
+            const graphqlBody = body as GraphQLRequestPayload;
 
             // Standard graphql request shape: https://graphql.org/learn/serving-over-http/#post-request
-            // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-            const graphqlQuery = graphqlBody && (graphqlBody['query'] as string);
-
-            // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-            const graphqlOperationName = graphqlBody && (graphqlBody['operationName'] as string);
+            const graphqlQuery = graphqlBody.query;
+            const graphqlOperationName = graphqlBody.operationName;
 
             const { operationName = graphqlOperationName, operationType } = parseGraphQLQuery(graphqlQuery);
             const newOperation = operationName ? `${operationType} ${operationName}` : `${operationType}`;
 
             span.updateName(`${httpMethod} ${httpUrl} (${newOperation})`);
+            span.setAttribute('body', JSON.stringify(graphqlBody));
           }
         }
       });

packages/browser/src/tracing/request.ts

@@ -363,8 +363,6 @@ export function xhrCallback(
 
   const hasParent = !!getActiveSpan();
 
-  const graphqlRequest = getGraphQLRequestPayload(sentryXhrData.body as string);
-
   const span =
     shouldCreateSpanResult && hasParent
       ? startInactiveSpan({
@@ -377,7 +375,6 @@ export function xhrCallback(
             'server.address': host,
             [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser',
             [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
-            body: graphqlRequest,
           },
         })
       : new SentryNonRecordingSpan();
@@ -398,6 +395,10 @@ export function xhrCallback(
     );
   }
 
+  if (client) {
+    client.emit('outgoingRequestSpanStart', span, { body: getGraphQLRequestPayload(sentryXhrData.body as string) });
+  }
+
   return span;
 }
 

packages/core/src/baseclient.ts

@@ -463,7 +463,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): () => void;
 
   /** @inheritdoc */
-  public on(hook: 'outgoingRequestSpanStart', callback: (span: Span) => void): () => void;
+  public on(hook: 'outgoingRequestSpanStart', callback: (span: Span, { body }: { body: unknown }) => void): () => void;
 
   public on(hook: 'flush', callback: () => void): () => void;
 
@@ -544,7 +544,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   public emit(hook: 'startNavigationSpan', options: StartSpanOptions): void;
 
   /** @inheritdoc */
-  public emit(hook: 'outgoingRequestSpanStart', span: Span): void;
+  public emit(hook: 'outgoingRequestSpanStart', span: Span, { body }: { body: unknown }): void;
 
   /** @inheritdoc */
   public emit(hook: 'flush'): void;

packages/core/src/fetch.ts

@@ -73,8 +73,6 @@ export function instrumentFetchRequest(
 
   const hasParent = !!getActiveSpan();
 
-  const graphqlRequest = getGraphQLRequestPayload(body as string);
-
   const span =
     shouldCreateSpanResult && hasParent
       ? startInactiveSpan({
@@ -87,7 +85,6 @@ export function instrumentFetchRequest(
             'server.address': host,
             [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin,
             [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
-            body: graphqlRequest,
           },
         })
       : new SentryNonRecordingSpan();
@@ -116,6 +113,10 @@ export function instrumentFetchRequest(
     );
   }
 
+  if (client) {
+    client.emit('outgoingRequestSpanStart', span, { body: getGraphQLRequestPayload(body as string) });
+  }
+
   return span;
 }
 

packages/types/src/client.ts

@@ -295,7 +295,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
    * A hook for GraphQL client integration to enhance a span and breadcrumbs with request data.
    * @returns A function that, when executed, removes the registered callback.
    */
-  on(hook: 'outgoingRequestSpanStart', callback: (span: Span) => void): () => void;
+  on(hook: 'outgoingRequestSpanStart', callback: (span: Span, { body }: { body: unknown }) => void): () => void;
 
   /**
    * A hook that is called when the client is flushing
@@ -396,7 +396,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
   /**
    * Emit a hook event for GraphQL client integration to enhance a span and breadcrumbs with request data.
    */
-  emit(hook: 'outgoingRequestSpanStart', span: Span): void;
+  emit(hook: 'outgoingRequestSpanStart', span: Span, { body }: { body: unknown }): void;
 
   /**
    * Emit a hook event for client flush