Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(browser): Add graphqlClientIntegration #13783

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const query = `query Test{
people {
name
pet
}
}`;

const requestBody = JSON.stringify({ query });

fetch('http://sentry-test.io/foo', {
method: 'POST',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
},
body: requestBody,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { expect } from '@playwright/test';
import type { Event } from '@sentry/types';

import { sentryTest } from '../../../../utils/fixtures';
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';

sentryTest.only('should create spans for GraphQL Fetch requests', async ({ getLocalTestPath, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

const url = await getLocalTestPath({ testDir: __dirname });

await page.route('**/foo', route => {
return route.fulfill({
status: 200,
body: JSON.stringify({
people: [
{ name: 'Amy', pet: 'dog' },
{ name: 'Jay', pet: 'cat' },
],
}),
headers: {
'Content-Type': 'application/json',
},
});
});

const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
const requestSpans = eventData.spans?.filter(({ op }) => op === 'http.client');

expect(requestSpans).toHaveLength(1);

expect(requestSpans![0]).toMatchObject({
description: 'POST http://sentry-test.io/foo (query Test)',
parent_span_id: eventData.contexts?.trace?.span_id,
span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
trace_id: eventData.contexts?.trace?.trace_id,
status: 'ok',
data: expect.objectContaining({
type: 'fetch',
'http.method': 'POST',
'http.url': 'http://sentry-test.io/foo',
url: 'http://sentry-test.io/foo',
'server.address': 'sentry-test.io',
'sentry.op': 'http.client',
'sentry.origin': 'auto.http.browser',
body: {
query: expect.any(String),
},
}),
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
integrations: [
Sentry.browserTracingIntegration(),
Sentry.graphqlClientIntegration({
endpoints: ['http://sentry-test.io/foo'],
}),
],
tracesSampleRate: 1,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const xhr = new XMLHttpRequest();

xhr.open('POST', 'http://sentry-test.io/foo');
xhr.setRequestHeader('Accept', 'application/json');
xhr.setRequestHeader('Content-Type', 'application/json');

const query = `query Test{

people {
name
pet
}
}`;

const requestBody = JSON.stringify({ query });
xhr.send(requestBody);
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { expect } from '@playwright/test';
import type { Event } from '@sentry/types';

import { sentryTest } from '../../../../utils/fixtures';
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';

sentryTest.only('should create spans for GraphQL XHR requests', async ({ getLocalTestPath, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

const url = await getLocalTestPath({ testDir: __dirname });

await page.route('**/foo', route => {
return route.fulfill({
status: 200,
body: JSON.stringify({
people: [
{ name: 'Amy', pet: 'dog' },
{ name: 'Jay', pet: 'cat' },
],
}),
headers: {
'Content-Type': 'application/json',
},
});
});

const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
const requestSpans = eventData.spans?.filter(({ op }) => op === 'http.client');

expect(requestSpans).toHaveLength(1);

expect(requestSpans![0]).toMatchObject({
description: 'POST http://sentry-test.io/foo (query Test)',
parent_span_id: eventData.contexts?.trace?.span_id,
span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
trace_id: eventData.contexts?.trace?.trace_id,
status: 'ok',
data: {
type: 'xhr',
'http.method': 'POST',
'http.url': 'http://sentry-test.io/foo',
url: 'http://sentry-test.io/foo',
'server.address': 'sentry-test.io',
'sentry.op': 'http.client',
'sentry.origin': 'auto.http.browser',
body: {
query: expect.any(String),
},
},
});
});
16 changes: 4 additions & 12 deletions packages/browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export * from './exports';
export { reportingObserverIntegration } from './integrations/reportingobserver';
export { httpClientIntegration } from './integrations/httpclient';
export { contextLinesIntegration } from './integrations/contextlines';
export { graphqlClientIntegration } from './integrations/graphqlClient';

export {
captureConsoleIntegration,
Expand All @@ -13,10 +14,7 @@ export {
captureFeedback,
} from '@sentry/core';

export {
replayIntegration,
getReplay,
} from '@sentry-internal/replay';
export { replayIntegration, getReplay } from '@sentry-internal/replay';
export type {
ReplayEventType,
ReplayEventWithTime,
Expand All @@ -34,17 +32,11 @@ export { replayCanvasIntegration } from '@sentry-internal/replay-canvas';
import { feedbackAsyncIntegration } from './feedbackAsync';
import { feedbackSyncIntegration } from './feedbackSync';
export { feedbackAsyncIntegration, feedbackSyncIntegration, feedbackSyncIntegration as feedbackIntegration };
export {
getFeedback,
sendFeedback,
} from '@sentry-internal/feedback';
export { getFeedback, sendFeedback } from '@sentry-internal/feedback';

export * from './metrics';

export {
defaultRequestInstrumentationOptions,
instrumentOutgoingRequests,
} from './tracing/request';
export { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './tracing/request';
export {
browserTracingIntegration,
startBrowserTracingNavigationSpan,
Expand Down
62 changes: 62 additions & 0 deletions packages/browser/src/integrations/graphqlClient.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import {
SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD,
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_URL_FULL,
defineIntegration,
spanToJSON,
} from '@sentry/core';
import type { IntegrationFn } from '@sentry/types';
import { parseGraphQLQuery } from '@sentry/utils';

interface GraphQLClientOptions {
endpoints: Array<string>;
}

const INTEGRATION_NAME = 'GraphQLClient';

const _graphqlClientIntegration = ((options: GraphQLClientOptions) => {
return {
name: INTEGRATION_NAME,
setup(client) {
client.on('spanStart', span => {
client.emit('outgoingRequestSpanStart', span);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was maybe a bit confusingly explained by me - I would not do this, so I'd remove these lines here. I'll leave a comment further below where/how I would suggest to do this!

Suggested change
client.on('spanStart', span => {
client.emit('outgoingRequestSpanStart', span);
});


client.on('outgoingRequestSpanStart', span => {
const spanJSON = spanToJSON(span);

const spanAttributes = spanJSON.data || {};

const spanOp = spanAttributes[SEMANTIC_ATTRIBUTE_SENTRY_OP];
const isHttpClientSpan = spanOp === 'http.client';

if (isHttpClientSpan) {
const httpUrl = spanAttributes[SEMANTIC_ATTRIBUTE_URL_FULL] || spanAttributes['http.url'];

const { endpoints } = options;

const isTracedGraphqlEndpoint = endpoints.includes(httpUrl);

if (isTracedGraphqlEndpoint) {
const httpMethod = spanAttributes[SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD] || spanAttributes['http.method'];
const graphqlBody = spanAttributes['body'];

// Standard graphql request shape: https://graphql.org/learn/serving-over-http/#post-request
const graphqlQuery = graphqlBody && (graphqlBody['query'] as string);
const graphqlOperationName = graphqlBody && (graphqlBody['operationName'] as string);

const { operationName = graphqlOperationName, operationType } = parseGraphQLQuery(graphqlQuery);
const newOperation = operationName ? `${operationType} ${operationName}` : `${operationType}`;

span.updateName(`${httpMethod} ${httpUrl} (${newOperation})`);
}
}
});
},
};
}) satisfies IntegrationFn;

/**
* GraphQL Client integration for the browser.
*/
export const graphqlClientIntegration = defineIntegration(_graphqlClientIntegration);
4 changes: 4 additions & 0 deletions packages/browser/src/tracing/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
browserPerformanceTimeOrigin,
dynamicSamplingContextToSentryBaggageHeader,
generateSentryTraceHeader,
getGraphQLRequestPayload,
parseUrl,
stringMatchesSomePattern,
} from '@sentry/utils';
Expand Down Expand Up @@ -362,6 +363,8 @@ export function xhrCallback(

const hasParent = !!getActiveSpan();

const graphqlRequest = getGraphQLRequestPayload(sentryXhrData.body as string);

const span =
shouldCreateSpanResult && hasParent
? startInactiveSpan({
Expand All @@ -374,6 +377,7 @@ export function xhrCallback(
'server.address': host,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
body: graphqlRequest,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding this here, after the span was started, in this file do:

client.emit('outgoingRequestSpanStart', span, { body });

So the hook receives the body as second argument, and then in the integration we can use the body and add it to the span or use it otherwise. This way, this is not added to all spans, as we do not want to generally capture this for every span.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get what you mean now! Will implement

},
})
: new SentryNonRecordingSpan();
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/** @inheritdoc */
public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): () => void;

/** @inheritdoc */
public on(hook: 'outgoingRequestSpanStart', callback: (span: Span) => void): () => void;

public on(hook: 'flush', callback: () => void): () => void;

public on(hook: 'close', callback: () => void): () => void;
Expand Down Expand Up @@ -540,6 +543,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/** @inheritdoc */
public emit(hook: 'startNavigationSpan', options: StartSpanOptions): void;

/** @inheritdoc */
public emit(hook: 'outgoingRequestSpanStart', span: Span): void;

/** @inheritdoc */
public emit(hook: 'flush'): void;

Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
BAGGAGE_HEADER_NAME,
dynamicSamplingContextToSentryBaggageHeader,
generateSentryTraceHeader,
getGraphQLRequestPayload,
isInstanceOf,
parseUrl,
} from '@sentry/utils';
Expand Down Expand Up @@ -65,13 +66,15 @@ export function instrumentFetchRequest(
const scope = getCurrentScope();
const client = getClient();

const { method, url } = handlerData.fetchData;
const { method, url, body } = handlerData.fetchData;

const fullUrl = getFullURL(url);
const host = fullUrl ? parseUrl(fullUrl).host : undefined;

const hasParent = !!getActiveSpan();

const graphqlRequest = getGraphQLRequestPayload(body as string);

const span =
shouldCreateSpanResult && hasParent
? startInactiveSpan({
Expand All @@ -84,6 +87,7 @@ export function instrumentFetchRequest(
'server.address': host,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client',
body: graphqlRequest,
},
})
: new SentryNonRecordingSpan();
Expand Down
11 changes: 11 additions & 0 deletions packages/types/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,12 @@ export interface Client<O extends ClientOptions = ClientOptions> {
*/
on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): () => void;

/**
* 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;

/**
* A hook that is called when the client is flushing
* @returns A function that, when executed, removes the registered callback.
Expand Down Expand Up @@ -387,6 +393,11 @@ export interface Client<O extends ClientOptions = ClientOptions> {
*/
emit(hook: 'startNavigationSpan', options: StartSpanOptions): void;

/**
* Emit a hook event for GraphQL client integration to enhance a span and breadcrumbs with request data.
*/
emit(hook: 'outgoingRequestSpanStart', span: Span): void;

/**
* Emit a hook event for client flush
*/
Expand Down
3 changes: 3 additions & 0 deletions packages/types/src/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import type { WebFetchHeaders } from './webfetchapi';
// Make sure to cast it where needed!
type XHRSendInput = unknown;

type FetchInput = unknown;

export type ConsoleLevel = 'debug' | 'info' | 'warn' | 'error' | 'log' | 'assert' | 'trace';

export interface SentryWrappedXMLHttpRequest {
Expand Down Expand Up @@ -37,6 +39,7 @@ export interface HandlerDataXhr {
interface SentryFetchData {
method: string;
url: string;
body?: FetchInput;
request_body_size?: number;
response_body_size?: number;
// span_id for the fetch request
Expand Down
Loading
Loading