Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions packages/astro/test/server/middleware.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -64,6 +64,7 @@ describe('sentryMiddleware', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
11 changes: 6 additions & 5 deletions packages/browser/src/integrations/graphqlClient.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import {
} from '@sentry/core/browser';
import type { FetchHint, XhrHint } from '@sentry/browser-utils';
import { getBodyString, getFetchRequestArgBody, SENTRY_XHR_DATA_KEY } from '@sentry/browser-utils';
import { GRAPHQL_DOCUMENT } from '@sentry/conventions/attributes';

interface GraphQLClientOptions {
endpoints: Array<string | RegExp>;
Expand DownExpand Up@@ -88,9 +89,9 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
const operationInfo = _getGraphQLOperation(graphqlBody);
span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);

// Handle standard requests - always capture the query document
if (isStandardRequest(graphqlBody)) {
span.setAttribute('graphql.document', graphqlBody.query);
// Handle standard requests - capture the query document when enabled via dataCollection (default true)
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
span.setAttribute(GRAPHQL_DOCUMENT, graphqlBody.query);
}

// Handle persisted operations - capture hash for debugging
Expand DownExpand Up@@ -126,8 +127,8 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient

data['graphql.operation'] = operationInfo;

if (isStandardRequest(graphqlBody)) {
data['graphql.document'] = graphqlBody.query;
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
data[GRAPHQL_DOCUMENT] = graphqlBody.query;
}

if (isPersistedRequest(graphqlBody)) {
Expand Down
26 changes: 25 additions & 1 deletion packages/browser/test/integrations/graphqlClient.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -313,14 +313,18 @@ describe('GraphqlClient', () => {
});

describe('beforeOutgoingRequestSpan handler', () => {
function setupHandler(endpoints: Array<string | RegExp>): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
function setupHandler(
endpoints: Array<string | RegExp>,
graphQLDocument = true,
): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
let capturedListener: ((span: SentrySpan, hint: FetchHint | XhrHint) => void) | undefined;
const mockClient = {
on: (eventName: string, cb: (span: SentrySpan, hint: FetchHint | XhrHint) => void) => {
if (eventName === 'beforeOutgoingRequestSpan') {
capturedListener = cb;
}
},
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as unknown as Client;

const integration = graphqlClientIntegration({ endpoints });
Expand DownExpand Up@@ -420,5 +424,25 @@ describe('GraphqlClient', () => {
expect(json.description).toBe('custom span');
expect(json.data['graphql.document']).toBeUndefined();
});

test('omits graphql.document when dataCollection.graphQL.document is false', () => {
const handler = setupHandler([/\/graphql$/], false);
const span = new SentrySpan({
name: 'POST http://localhost:4000/graphql',
op: 'http.client',
attributes: {
'http.method': 'POST',
'http.url': 'http://localhost:4000/graphql',
url: 'http://localhost:4000/graphql',
},
});

handler(span, makeFetchHint('http://localhost:4000/graphql', requestBody));

const json = spanToJSON(span);
// The span is still renamed with the operation, but the document is not attached.
expect(json.description).toBe('POST http://localhost:4000/graphql (query GetHello)');
expect(json.data['graphql.document']).toBeUndefined();
});
});
});
22 changes: 21 additions & 1 deletion packages/core/src/types/datacollection.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,7 +51,26 @@ export interface DataCollection {
queryParams?: CollectBehavior;

/**
* Controls generative AI input/output recording.
* Allow collection of GraphQL operation data when using the SDK's GraphQL integrations
*
* These options do not add GraphQL instrumentation on their own. The corresponding integration must still be enabled
* for any data to be captured.
*
* - `document`: attach the GraphQL document (query/mutation source text).
* - `variables`: attach the variables passed to GraphQL operations.
* @default { document: true, variables: true }
*/
graphQL?: {
document?: boolean;
variables?: boolean;
};
Comment on lines +63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The graphQL.variables configuration option is defined but never used, making the feature to control GraphQL variable collection non-functional.
Severity: HIGH

Suggested Fix

Implement the logic to read the getDataCollectionOptions().graphQL.variables setting. Based on its value, conditionally collect or suppress the variables field from GraphQL requests. This logic should be added where GraphQL bodies are processed, such as in packages/browser/src/integrations/graphqlClient.ts and packages/server-utils/src/graphql/utils.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/types/datacollection.ts#L59-L62
Potential issue: The `graphQL.variables` configuration option is defined and documented,
but its value is never read in the production codebase. No code checks
`getDataCollectionOptions().graphQL.variables` before collecting GraphQL variables.
Since no variable collection code exists, the setting has no effect. Users who set
`graphQL: { variables: false }` believing they are suppressing variable collection are
not actually having variables collected (as the feature is missing), and users who set
`variables: true` believing variables will be captured find they are not. The API
contract is unfulfilled as the implementation is missing.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +63 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a blocker but something I was discussing with @ericapisani, this creates the expectation that the SDK is going to apply these options to all GraphQL requests which is not accurate.

It would only apply if the user is using the client or the server GraphQL integrations. GraphQL requests outside of the integrations coverage won't be covered by these options which I find weird. I know the gen AI spans is already a precedent, so I don't have strong opinions here.

An alternative I considered is adding the dataCollection option to the integrations themselves rather than a top-level option.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The core idea of the top-level option is to have only one place where all of this is defined in an on/off switch manner (like: this is allowed to be sent, but you also need the integration to be enabled). This makes it easier to see it all at once. According to the spec, it's also possible to add it on integration level, which would overwrite the option set on the root-level.

Maybe we should make this more clear in the JSDoc?

@logaretmlogaretmJul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine, we already have a precedent for it with gen AI stuff so not feeling too strongly about it but it is something that I wanted to bring up is all.

Maybe adding a line or two in the JS doc to explain what "it won't do" will help here.


/**
* Allow generative AI input/output recording when using the SDK's AI integrations.
*
* These options do not add AI instrumentation on their own. The corresponding integration must still be enabled for
* any data to be captured. Integration-level options, when set, take precedence over these
* values.
* @default { inputs: true, outputs: true }
*/
genAI?: {
Expand All@@ -77,5 +96,6 @@ export interface DataCollection {
*/
export type ResolvedDataCollection = Required<DataCollection> & {
httpHeaders: Required<NonNullable<DataCollection['httpHeaders']>>;
graphQL: Required<NonNullable<DataCollection['graphQL']>>;
genAI: Required<NonNullable<DataCollection['genAI']>>;
};
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand All@@ -26,6 +27,9 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: { deny: PII_HEADER_SNIPPETS }, response: { deny: PII_HEADER_SNIPPETS } },
httpBodies: [],
queryParams: { deny: PII_HEADER_SNIPPETS },
// The GraphQL document has literal values redacted at collection time, so it was historically
// always attached regardless of `sendDefaultPii`; keep it on to preserve that behavior.
graphQL: { document: true, variables: true },
Comment on lines +30 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: When sendDefaultPii is false, graphQL.variables is incorrectly set to true. This contradicts the goal of minimizing PII collection and sets up a potential future PII leak.
Severity: HIGH

Suggested Fix

Change the default setting for graphQL.variables to false when sendDefaultPii is false in defaultPiiToCollectionOptions.ts. This aligns the behavior with other PII-sensitive settings and prevents a future potential PII leak. The line should be changed to graphQL: { document: true, variables: false }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/core/src/utils/data-collection/defaultPiiToCollectionOptions.ts#L30-L32
Potential issue: The default data collection options set `graphQL.variables` to `true`
even when `sendDefaultPii` is `false`. The purpose of `sendDefaultPii: false` is to
prevent the collection of Personally Identifiable Information (PII). GraphQL variables
can contain sensitive user data, and unlike `graphQL.document`, they are not redacted.
While no code currently consumes the `graphQL.variables` setting, this configuration is
a design error. It creates a high risk that future code implementing GraphQL variable
collection will inadvertently capture and send PII for users who have opted out, as the
default setting will be assumed to be safe.

genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ const DEFAULTS: ResolvedDataCollection = {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand DownExpand Up@@ -43,6 +44,10 @@ export function resolveDataCollectionOptions(options: {
},
httpBodies: dc.httpBodies ?? base.httpBodies,
queryParams: dc.queryParams ?? base.queryParams,
graphQL: {
document: dc.graphQL?.document ?? base.graphQL.document,
variables: dc.graphQL?.variables ?? base.graphQL.variables,
},
genAI: {
inputs: dc.genAI?.inputs ?? base.genAI.inputs,
outputs: dc.genAI?.outputs ?? base.genAI.outputs,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ export function createMockClient(userInfo = true, genAI?: { inputs: boolean; out
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: genAIOptions,
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@ describe('defaultPiiToCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -25,6 +26,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -41,6 +43,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -57,6 +60,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ describe('resolveDataCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand All@@ -21,6 +22,8 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
// GraphQL documents are redacted at collection time, so they stay on to preserve legacy behavior.
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(7);
});
Expand All@@ -45,6 +48,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
});

Expand All@@ -54,6 +58,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
expect(result.graphQL).toEqual({ document: true, variables: true });
});
});

Expand DownExpand Up@@ -87,6 +92,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.cookies).toBe(true);
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(5);
Expand DownExpand Up@@ -114,6 +120,17 @@ describe('resolveDataCollectionOptions', () => {
expect(result.genAI.outputs).toBe(true);
});

it('merges nested graphQL partially', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
graphQL: { document: false },
},
});

expect(result.graphQL.document).toBe(false);
expect(result.graphQL.variables).toBe(true);
});

it('supports allow/deny list for cookies', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
Expand All@@ -139,14 +156,17 @@ describe('resolveDataCollectionOptions', () => {
it('always returns all fields', () => {
const result = resolveDataCollectionOptions({});

expect(Object.keys(result)).toHaveLength(8);
expect(Object.keys(result)).toHaveLength(9);
expect(result).toHaveProperty('userInfo');
expect(result).toHaveProperty('cookies');
expect(result).toHaveProperty('httpHeaders');
expect(result).toHaveProperty('httpHeaders.request');
expect(result).toHaveProperty('httpHeaders.response');
expect(result).toHaveProperty('httpBodies');
expect(result).toHaveProperty('queryParams');
expect(result).toHaveProperty('graphQL');
expect(result).toHaveProperty('graphQL.document');
expect(result).toHaveProperty('graphQL.variables');
expect(result).toHaveProperty('genAI');
expect(result).toHaveProperty('genAI.inputs');
expect(result).toHaveProperty('genAI.outputs');
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/lib/utils/request.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -874,6 +874,7 @@ describe('request utils', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type {
Token,
} from './graphql-types';
import type { Span, SpanAttributes } from '@sentry/core';
import { isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { getClient, isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { AllowedOperationTypes, SpanNames, TokenKind } from './enum';
import { AttributeNames } from './enums/AttributeNames';
import { OTEL_GRAPHQL_DATA_SYMBOL, OTEL_PATCHED_SYMBOL } from './symbols';
Expand All@@ -38,8 +38,10 @@ export const isPromise = (value: any): value is Promise<unknown> => {
};

export function addSpanSource(span: Span, loc?: Location, start?: number, end?: number): void {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
if (getClient()?.getDataCollectionOptions().graphQL.document === true) {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
}
}

function createFieldIfNotExists(
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as core from '@sentry/core';
import { addSpanSource } from '../../../../src/integrations/tracing/graphql/vendored/utils';
import type { Location, Token } from '@sentry/server-utils/orchestrion';

vi.spyOn(core, 'getClient');

function mockClient(graphQLDocument: boolean): void {
vi.mocked(core.getClient).mockReturnValue({
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as any);
}

function makeLocation(): Location {
const token: Token = {
kind: 'Name',
start: 0,
end: 5,
line: 1,
column: 1,
value: 'hello',
prev: null,
next: null,
};
return { start: 0, end: 5, startToken: token };
}

describe('addSpanSource dataCollection gate', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('sets graphql.source attribute when graphQL.document is true', () => {
mockClient(true);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).toHaveBeenCalledWith('graphql.source', expect.any(String));
});

it('does not set attribute when graphQL.document is false', () => {
mockClient(false);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});

it('does not set attribute when getClient() is undefined', () => {
vi.mocked(core.getClient).mockReturnValue(undefined);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,7 @@ describe('wrapMiddlewareHandlerWithSentry', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/test/server/errors.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ function createMockClient(httpBodies: string[] = []): Client {
httpHeaders: { request: true, response: true },
httpBodies,
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions packages/astro/test/server/middleware.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -64,6 +64,7 @@ describe('sentryMiddleware', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
11 changes: 6 additions & 5 deletions packages/browser/src/integrations/graphqlClient.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import {
} from '@sentry/core/browser';
import type { FetchHint, XhrHint } from '@sentry/browser-utils';
import { getBodyString, getFetchRequestArgBody, SENTRY_XHR_DATA_KEY } from '@sentry/browser-utils';
import { GRAPHQL_DOCUMENT } from '@sentry/conventions/attributes';

interface GraphQLClientOptions {
endpoints: Array<string | RegExp>;
Expand DownExpand Up@@ -88,9 +89,9 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
const operationInfo = _getGraphQLOperation(graphqlBody);
span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);

// Handle standard requests - always capture the query document
if (isStandardRequest(graphqlBody)) {
span.setAttribute('graphql.document', graphqlBody.query);
// Handle standard requests - capture the query document when enabled via dataCollection (default true)
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
span.setAttribute(GRAPHQL_DOCUMENT, graphqlBody.query);
}

// Handle persisted operations - capture hash for debugging
Expand DownExpand Up@@ -126,8 +127,8 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient

data['graphql.operation'] = operationInfo;

if (isStandardRequest(graphqlBody)) {
data['graphql.document'] = graphqlBody.query;
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
data[GRAPHQL_DOCUMENT] = graphqlBody.query;
}

if (isPersistedRequest(graphqlBody)) {
Expand Down
26 changes: 25 additions & 1 deletion packages/browser/test/integrations/graphqlClient.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -313,14 +313,18 @@ describe('GraphqlClient', () => {
});

describe('beforeOutgoingRequestSpan handler', () => {
function setupHandler(endpoints: Array<string | RegExp>): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
function setupHandler(
endpoints: Array<string | RegExp>,
graphQLDocument = true,
): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
let capturedListener: ((span: SentrySpan, hint: FetchHint | XhrHint) => void) | undefined;
const mockClient = {
on: (eventName: string, cb: (span: SentrySpan, hint: FetchHint | XhrHint) => void) => {
if (eventName === 'beforeOutgoingRequestSpan') {
capturedListener = cb;
}
},
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as unknown as Client;

const integration = graphqlClientIntegration({ endpoints });
Expand DownExpand Up@@ -420,5 +424,25 @@ describe('GraphqlClient', () => {
expect(json.description).toBe('custom span');
expect(json.data['graphql.document']).toBeUndefined();
});

test('omits graphql.document when dataCollection.graphQL.document is false', () => {
const handler = setupHandler([/\/graphql$/], false);
const span = new SentrySpan({
name: 'POST http://localhost:4000/graphql',
op: 'http.client',
attributes: {
'http.method': 'POST',
'http.url': 'http://localhost:4000/graphql',
url: 'http://localhost:4000/graphql',
},
});

handler(span, makeFetchHint('http://localhost:4000/graphql', requestBody));

const json = spanToJSON(span);
// The span is still renamed with the operation, but the document is not attached.
expect(json.description).toBe('POST http://localhost:4000/graphql (query GetHello)');
expect(json.data['graphql.document']).toBeUndefined();
});
});
});
22 changes: 21 additions & 1 deletion packages/core/src/types/datacollection.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,7 +51,26 @@ export interface DataCollection {
queryParams?: CollectBehavior;

/**
* Controls generative AI input/output recording.
* Allow collection of GraphQL operation data when using the SDK's GraphQL integrations
*
* These options do not add GraphQL instrumentation on their own. The corresponding integration must still be enabled
* for any data to be captured.
*
* - `document`: attach the GraphQL document (query/mutation source text).
* - `variables`: attach the variables passed to GraphQL operations.
* @default { document: true, variables: true }
*/
graphQL?: {
document?: boolean;
variables?: boolean;
};
Comment on lines +63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The graphQL.variables configuration option is defined but never used, making the feature to control GraphQL variable collection non-functional.
Severity: HIGH

Suggested Fix

Implement the logic to read the getDataCollectionOptions().graphQL.variables setting. Based on its value, conditionally collect or suppress the variables field from GraphQL requests. This logic should be added where GraphQL bodies are processed, such as in packages/browser/src/integrations/graphqlClient.ts and packages/server-utils/src/graphql/utils.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/types/datacollection.ts#L59-L62
Potential issue: The `graphQL.variables` configuration option is defined and documented,
but its value is never read in the production codebase. No code checks
`getDataCollectionOptions().graphQL.variables` before collecting GraphQL variables.
Since no variable collection code exists, the setting has no effect. Users who set
`graphQL: { variables: false }` believing they are suppressing variable collection are
not actually having variables collected (as the feature is missing), and users who set
`variables: true` believing variables will be captured find they are not. The API
contract is unfulfilled as the implementation is missing.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +63 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a blocker but something I was discussing with @ericapisani, this creates the expectation that the SDK is going to apply these options to all GraphQL requests which is not accurate.

It would only apply if the user is using the client or the server GraphQL integrations. GraphQL requests outside of the integrations coverage won't be covered by these options which I find weird. I know the gen AI spans is already a precedent, so I don't have strong opinions here.

An alternative I considered is adding the dataCollection option to the integrations themselves rather than a top-level option.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The core idea of the top-level option is to have only one place where all of this is defined in an on/off switch manner (like: this is allowed to be sent, but you also need the integration to be enabled). This makes it easier to see it all at once. According to the spec, it's also possible to add it on integration level, which would overwrite the option set on the root-level.

Maybe we should make this more clear in the JSDoc?

@logaretmlogaretmJul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine, we already have a precedent for it with gen AI stuff so not feeling too strongly about it but it is something that I wanted to bring up is all.

Maybe adding a line or two in the JS doc to explain what "it won't do" will help here.


/**
* Allow generative AI input/output recording when using the SDK's AI integrations.
*
* These options do not add AI instrumentation on their own. The corresponding integration must still be enabled for
* any data to be captured. Integration-level options, when set, take precedence over these
* values.
* @default { inputs: true, outputs: true }
*/
genAI?: {
Expand All@@ -77,5 +96,6 @@ export interface DataCollection {
*/
export type ResolvedDataCollection = Required<DataCollection> & {
httpHeaders: Required<NonNullable<DataCollection['httpHeaders']>>;
graphQL: Required<NonNullable<DataCollection['graphQL']>>;
genAI: Required<NonNullable<DataCollection['genAI']>>;
};
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand All@@ -26,6 +27,9 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: { deny: PII_HEADER_SNIPPETS }, response: { deny: PII_HEADER_SNIPPETS } },
httpBodies: [],
queryParams: { deny: PII_HEADER_SNIPPETS },
// The GraphQL document has literal values redacted at collection time, so it was historically
// always attached regardless of `sendDefaultPii`; keep it on to preserve that behavior.
graphQL: { document: true, variables: true },
Comment on lines +30 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: When sendDefaultPii is false, graphQL.variables is incorrectly set to true. This contradicts the goal of minimizing PII collection and sets up a potential future PII leak.
Severity: HIGH

Suggested Fix

Change the default setting for graphQL.variables to false when sendDefaultPii is false in defaultPiiToCollectionOptions.ts. This aligns the behavior with other PII-sensitive settings and prevents a future potential PII leak. The line should be changed to graphQL: { document: true, variables: false }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/core/src/utils/data-collection/defaultPiiToCollectionOptions.ts#L30-L32
Potential issue: The default data collection options set `graphQL.variables` to `true`
even when `sendDefaultPii` is `false`. The purpose of `sendDefaultPii: false` is to
prevent the collection of Personally Identifiable Information (PII). GraphQL variables
can contain sensitive user data, and unlike `graphQL.document`, they are not redacted.
While no code currently consumes the `graphQL.variables` setting, this configuration is
a design error. It creates a high risk that future code implementing GraphQL variable
collection will inadvertently capture and send PII for users who have opted out, as the
default setting will be assumed to be safe.

genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ const DEFAULTS: ResolvedDataCollection = {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand DownExpand Up@@ -43,6 +44,10 @@ export function resolveDataCollectionOptions(options: {
},
httpBodies: dc.httpBodies ?? base.httpBodies,
queryParams: dc.queryParams ?? base.queryParams,
graphQL: {
document: dc.graphQL?.document ?? base.graphQL.document,
variables: dc.graphQL?.variables ?? base.graphQL.variables,
},
genAI: {
inputs: dc.genAI?.inputs ?? base.genAI.inputs,
outputs: dc.genAI?.outputs ?? base.genAI.outputs,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ export function createMockClient(userInfo = true, genAI?: { inputs: boolean; out
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: genAIOptions,
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@ describe('defaultPiiToCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -25,6 +26,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -41,6 +43,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -57,6 +60,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ describe('resolveDataCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand All@@ -21,6 +22,8 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
// GraphQL documents are redacted at collection time, so they stay on to preserve legacy behavior.
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(7);
});
Expand All@@ -45,6 +48,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
});

Expand All@@ -54,6 +58,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
expect(result.graphQL).toEqual({ document: true, variables: true });
});
});

Expand DownExpand Up@@ -87,6 +92,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.cookies).toBe(true);
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(5);
Expand DownExpand Up@@ -114,6 +120,17 @@ describe('resolveDataCollectionOptions', () => {
expect(result.genAI.outputs).toBe(true);
});

it('merges nested graphQL partially', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
graphQL: { document: false },
},
});

expect(result.graphQL.document).toBe(false);
expect(result.graphQL.variables).toBe(true);
});

it('supports allow/deny list for cookies', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
Expand All@@ -139,14 +156,17 @@ describe('resolveDataCollectionOptions', () => {
it('always returns all fields', () => {
const result = resolveDataCollectionOptions({});

expect(Object.keys(result)).toHaveLength(8);
expect(Object.keys(result)).toHaveLength(9);
expect(result).toHaveProperty('userInfo');
expect(result).toHaveProperty('cookies');
expect(result).toHaveProperty('httpHeaders');
expect(result).toHaveProperty('httpHeaders.request');
expect(result).toHaveProperty('httpHeaders.response');
expect(result).toHaveProperty('httpBodies');
expect(result).toHaveProperty('queryParams');
expect(result).toHaveProperty('graphQL');
expect(result).toHaveProperty('graphQL.document');
expect(result).toHaveProperty('graphQL.variables');
expect(result).toHaveProperty('genAI');
expect(result).toHaveProperty('genAI.inputs');
expect(result).toHaveProperty('genAI.outputs');
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/lib/utils/request.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -874,6 +874,7 @@ describe('request utils', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type {
Token,
} from './graphql-types';
import type { Span, SpanAttributes } from '@sentry/core';
import { isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { getClient, isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { AllowedOperationTypes, SpanNames, TokenKind } from './enum';
import { AttributeNames } from './enums/AttributeNames';
import { OTEL_GRAPHQL_DATA_SYMBOL, OTEL_PATCHED_SYMBOL } from './symbols';
Expand All@@ -38,8 +38,10 @@ export const isPromise = (value: any): value is Promise<unknown> => {
};

export function addSpanSource(span: Span, loc?: Location, start?: number, end?: number): void {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
if (getClient()?.getDataCollectionOptions().graphQL.document === true) {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
}
}

function createFieldIfNotExists(
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as core from '@sentry/core';
import { addSpanSource } from '../../../../src/integrations/tracing/graphql/vendored/utils';
import type { Location, Token } from '@sentry/server-utils/orchestrion';

vi.spyOn(core, 'getClient');

function mockClient(graphQLDocument: boolean): void {
vi.mocked(core.getClient).mockReturnValue({
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as any);
}

function makeLocation(): Location {
const token: Token = {
kind: 'Name',
start: 0,
end: 5,
line: 1,
column: 1,
value: 'hello',
prev: null,
next: null,
};
return { start: 0, end: 5, startToken: token };
}

describe('addSpanSource dataCollection gate', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('sets graphql.source attribute when graphQL.document is true', () => {
mockClient(true);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).toHaveBeenCalledWith('graphql.source', expect.any(String));
});

it('does not set attribute when graphQL.document is false', () => {
mockClient(false);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});

it('does not set attribute when getClient() is undefined', () => {
vi.mocked(core.getClient).mockReturnValue(undefined);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,7 @@ describe('wrapMiddlewareHandlerWithSentry', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/test/server/errors.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ function createMockClient(httpBodies: string[] = []): Client {
httpHeaders: { request: true, response: true },
httpBodies,
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions packages/astro/test/server/middleware.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -64,6 +64,7 @@ describe('sentryMiddleware', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
11 changes: 6 additions & 5 deletions packages/browser/src/integrations/graphqlClient.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import {
} from '@sentry/core/browser';
import type { FetchHint, XhrHint } from '@sentry/browser-utils';
import { getBodyString, getFetchRequestArgBody, SENTRY_XHR_DATA_KEY } from '@sentry/browser-utils';
import { GRAPHQL_DOCUMENT } from '@sentry/conventions/attributes';

interface GraphQLClientOptions {
endpoints: Array<string | RegExp>;
Expand DownExpand Up@@ -88,9 +89,9 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
const operationInfo = _getGraphQLOperation(graphqlBody);
span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);

// Handle standard requests - always capture the query document
if (isStandardRequest(graphqlBody)) {
span.setAttribute('graphql.document', graphqlBody.query);
// Handle standard requests - capture the query document when enabled via dataCollection (default true)
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
span.setAttribute(GRAPHQL_DOCUMENT, graphqlBody.query);
}

// Handle persisted operations - capture hash for debugging
Expand DownExpand Up@@ -126,8 +127,8 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient

data['graphql.operation'] = operationInfo;

if (isStandardRequest(graphqlBody)) {
data['graphql.document'] = graphqlBody.query;
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
data[GRAPHQL_DOCUMENT] = graphqlBody.query;
}

if (isPersistedRequest(graphqlBody)) {
Expand Down
26 changes: 25 additions & 1 deletion packages/browser/test/integrations/graphqlClient.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -313,14 +313,18 @@ describe('GraphqlClient', () => {
});

describe('beforeOutgoingRequestSpan handler', () => {
function setupHandler(endpoints: Array<string | RegExp>): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
function setupHandler(
endpoints: Array<string | RegExp>,
graphQLDocument = true,
): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
let capturedListener: ((span: SentrySpan, hint: FetchHint | XhrHint) => void) | undefined;
const mockClient = {
on: (eventName: string, cb: (span: SentrySpan, hint: FetchHint | XhrHint) => void) => {
if (eventName === 'beforeOutgoingRequestSpan') {
capturedListener = cb;
}
},
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as unknown as Client;

const integration = graphqlClientIntegration({ endpoints });
Expand DownExpand Up@@ -420,5 +424,25 @@ describe('GraphqlClient', () => {
expect(json.description).toBe('custom span');
expect(json.data['graphql.document']).toBeUndefined();
});

test('omits graphql.document when dataCollection.graphQL.document is false', () => {
const handler = setupHandler([/\/graphql$/], false);
const span = new SentrySpan({
name: 'POST http://localhost:4000/graphql',
op: 'http.client',
attributes: {
'http.method': 'POST',
'http.url': 'http://localhost:4000/graphql',
url: 'http://localhost:4000/graphql',
},
});

handler(span, makeFetchHint('http://localhost:4000/graphql', requestBody));

const json = spanToJSON(span);
// The span is still renamed with the operation, but the document is not attached.
expect(json.description).toBe('POST http://localhost:4000/graphql (query GetHello)');
expect(json.data['graphql.document']).toBeUndefined();
});
});
});
22 changes: 21 additions & 1 deletion packages/core/src/types/datacollection.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,7 +51,26 @@ export interface DataCollection {
queryParams?: CollectBehavior;

/**
* Controls generative AI input/output recording.
* Allow collection of GraphQL operation data when using the SDK's GraphQL integrations
*
* These options do not add GraphQL instrumentation on their own. The corresponding integration must still be enabled
* for any data to be captured.
*
* - `document`: attach the GraphQL document (query/mutation source text).
* - `variables`: attach the variables passed to GraphQL operations.
* @default { document: true, variables: true }
*/
graphQL?: {
document?: boolean;
variables?: boolean;
};
Comment on lines +63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The graphQL.variables configuration option is defined but never used, making the feature to control GraphQL variable collection non-functional.
Severity: HIGH

Suggested Fix

Implement the logic to read the getDataCollectionOptions().graphQL.variables setting. Based on its value, conditionally collect or suppress the variables field from GraphQL requests. This logic should be added where GraphQL bodies are processed, such as in packages/browser/src/integrations/graphqlClient.ts and packages/server-utils/src/graphql/utils.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/types/datacollection.ts#L59-L62
Potential issue: The `graphQL.variables` configuration option is defined and documented,
but its value is never read in the production codebase. No code checks
`getDataCollectionOptions().graphQL.variables` before collecting GraphQL variables.
Since no variable collection code exists, the setting has no effect. Users who set
`graphQL: { variables: false }` believing they are suppressing variable collection are
not actually having variables collected (as the feature is missing), and users who set
`variables: true` believing variables will be captured find they are not. The API
contract is unfulfilled as the implementation is missing.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +63 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a blocker but something I was discussing with @ericapisani, this creates the expectation that the SDK is going to apply these options to all GraphQL requests which is not accurate.

It would only apply if the user is using the client or the server GraphQL integrations. GraphQL requests outside of the integrations coverage won't be covered by these options which I find weird. I know the gen AI spans is already a precedent, so I don't have strong opinions here.

An alternative I considered is adding the dataCollection option to the integrations themselves rather than a top-level option.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The core idea of the top-level option is to have only one place where all of this is defined in an on/off switch manner (like: this is allowed to be sent, but you also need the integration to be enabled). This makes it easier to see it all at once. According to the spec, it's also possible to add it on integration level, which would overwrite the option set on the root-level.

Maybe we should make this more clear in the JSDoc?

@logaretmlogaretmJul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine, we already have a precedent for it with gen AI stuff so not feeling too strongly about it but it is something that I wanted to bring up is all.

Maybe adding a line or two in the JS doc to explain what "it won't do" will help here.


/**
* Allow generative AI input/output recording when using the SDK's AI integrations.
*
* These options do not add AI instrumentation on their own. The corresponding integration must still be enabled for
* any data to be captured. Integration-level options, when set, take precedence over these
* values.
* @default { inputs: true, outputs: true }
*/
genAI?: {
Expand All@@ -77,5 +96,6 @@ export interface DataCollection {
*/
export type ResolvedDataCollection = Required<DataCollection> & {
httpHeaders: Required<NonNullable<DataCollection['httpHeaders']>>;
graphQL: Required<NonNullable<DataCollection['graphQL']>>;
genAI: Required<NonNullable<DataCollection['genAI']>>;
};
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand All@@ -26,6 +27,9 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: { deny: PII_HEADER_SNIPPETS }, response: { deny: PII_HEADER_SNIPPETS } },
httpBodies: [],
queryParams: { deny: PII_HEADER_SNIPPETS },
// The GraphQL document has literal values redacted at collection time, so it was historically
// always attached regardless of `sendDefaultPii`; keep it on to preserve that behavior.
graphQL: { document: true, variables: true },
Comment on lines +30 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: When sendDefaultPii is false, graphQL.variables is incorrectly set to true. This contradicts the goal of minimizing PII collection and sets up a potential future PII leak.
Severity: HIGH

Suggested Fix

Change the default setting for graphQL.variables to false when sendDefaultPii is false in defaultPiiToCollectionOptions.ts. This aligns the behavior with other PII-sensitive settings and prevents a future potential PII leak. The line should be changed to graphQL: { document: true, variables: false }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/core/src/utils/data-collection/defaultPiiToCollectionOptions.ts#L30-L32
Potential issue: The default data collection options set `graphQL.variables` to `true`
even when `sendDefaultPii` is `false`. The purpose of `sendDefaultPii: false` is to
prevent the collection of Personally Identifiable Information (PII). GraphQL variables
can contain sensitive user data, and unlike `graphQL.document`, they are not redacted.
While no code currently consumes the `graphQL.variables` setting, this configuration is
a design error. It creates a high risk that future code implementing GraphQL variable
collection will inadvertently capture and send PII for users who have opted out, as the
default setting will be assumed to be safe.

genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ const DEFAULTS: ResolvedDataCollection = {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand DownExpand Up@@ -43,6 +44,10 @@ export function resolveDataCollectionOptions(options: {
},
httpBodies: dc.httpBodies ?? base.httpBodies,
queryParams: dc.queryParams ?? base.queryParams,
graphQL: {
document: dc.graphQL?.document ?? base.graphQL.document,
variables: dc.graphQL?.variables ?? base.graphQL.variables,
},
genAI: {
inputs: dc.genAI?.inputs ?? base.genAI.inputs,
outputs: dc.genAI?.outputs ?? base.genAI.outputs,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ export function createMockClient(userInfo = true, genAI?: { inputs: boolean; out
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: genAIOptions,
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@ describe('defaultPiiToCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -25,6 +26,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -41,6 +43,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -57,6 +60,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ describe('resolveDataCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand All@@ -21,6 +22,8 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
// GraphQL documents are redacted at collection time, so they stay on to preserve legacy behavior.
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(7);
});
Expand All@@ -45,6 +48,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
});

Expand All@@ -54,6 +58,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
expect(result.graphQL).toEqual({ document: true, variables: true });
});
});

Expand DownExpand Up@@ -87,6 +92,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.cookies).toBe(true);
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(5);
Expand DownExpand Up@@ -114,6 +120,17 @@ describe('resolveDataCollectionOptions', () => {
expect(result.genAI.outputs).toBe(true);
});

it('merges nested graphQL partially', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
graphQL: { document: false },
},
});

expect(result.graphQL.document).toBe(false);
expect(result.graphQL.variables).toBe(true);
});

it('supports allow/deny list for cookies', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
Expand All@@ -139,14 +156,17 @@ describe('resolveDataCollectionOptions', () => {
it('always returns all fields', () => {
const result = resolveDataCollectionOptions({});

expect(Object.keys(result)).toHaveLength(8);
expect(Object.keys(result)).toHaveLength(9);
expect(result).toHaveProperty('userInfo');
expect(result).toHaveProperty('cookies');
expect(result).toHaveProperty('httpHeaders');
expect(result).toHaveProperty('httpHeaders.request');
expect(result).toHaveProperty('httpHeaders.response');
expect(result).toHaveProperty('httpBodies');
expect(result).toHaveProperty('queryParams');
expect(result).toHaveProperty('graphQL');
expect(result).toHaveProperty('graphQL.document');
expect(result).toHaveProperty('graphQL.variables');
expect(result).toHaveProperty('genAI');
expect(result).toHaveProperty('genAI.inputs');
expect(result).toHaveProperty('genAI.outputs');
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/lib/utils/request.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -874,6 +874,7 @@ describe('request utils', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type {
Token,
} from './graphql-types';
import type { Span, SpanAttributes } from '@sentry/core';
import { isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { getClient, isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { AllowedOperationTypes, SpanNames, TokenKind } from './enum';
import { AttributeNames } from './enums/AttributeNames';
import { OTEL_GRAPHQL_DATA_SYMBOL, OTEL_PATCHED_SYMBOL } from './symbols';
Expand All@@ -38,8 +38,10 @@ export const isPromise = (value: any): value is Promise<unknown> => {
};

export function addSpanSource(span: Span, loc?: Location, start?: number, end?: number): void {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
if (getClient()?.getDataCollectionOptions().graphQL.document === true) {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
}
}

function createFieldIfNotExists(
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as core from '@sentry/core';
import { addSpanSource } from '../../../../src/integrations/tracing/graphql/vendored/utils';
import type { Location, Token } from '@sentry/server-utils/orchestrion';

vi.spyOn(core, 'getClient');

function mockClient(graphQLDocument: boolean): void {
vi.mocked(core.getClient).mockReturnValue({
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as any);
}

function makeLocation(): Location {
const token: Token = {
kind: 'Name',
start: 0,
end: 5,
line: 1,
column: 1,
value: 'hello',
prev: null,
next: null,
};
return { start: 0, end: 5, startToken: token };
}

describe('addSpanSource dataCollection gate', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('sets graphql.source attribute when graphQL.document is true', () => {
mockClient(true);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).toHaveBeenCalledWith('graphql.source', expect.any(String));
});

it('does not set attribute when graphQL.document is false', () => {
mockClient(false);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});

it('does not set attribute when getClient() is undefined', () => {
vi.mocked(core.getClient).mockReturnValue(undefined);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,7 @@ describe('wrapMiddlewareHandlerWithSentry', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/test/server/errors.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ function createMockClient(httpBodies: string[] = []): Client {
httpHeaders: { request: true, response: true },
httpBodies,
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions packages/astro/test/server/middleware.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -64,6 +64,7 @@ describe('sentryMiddleware', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
11 changes: 6 additions & 5 deletions packages/browser/src/integrations/graphqlClient.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import {
} from '@sentry/core/browser';
import type { FetchHint, XhrHint } from '@sentry/browser-utils';
import { getBodyString, getFetchRequestArgBody, SENTRY_XHR_DATA_KEY } from '@sentry/browser-utils';
import { GRAPHQL_DOCUMENT } from '@sentry/conventions/attributes';

interface GraphQLClientOptions {
endpoints: Array<string | RegExp>;
Expand DownExpand Up@@ -88,9 +89,9 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
const operationInfo = _getGraphQLOperation(graphqlBody);
span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);

// Handle standard requests - always capture the query document
if (isStandardRequest(graphqlBody)) {
span.setAttribute('graphql.document', graphqlBody.query);
// Handle standard requests - capture the query document when enabled via dataCollection (default true)
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
span.setAttribute(GRAPHQL_DOCUMENT, graphqlBody.query);
}

// Handle persisted operations - capture hash for debugging
Expand DownExpand Up@@ -126,8 +127,8 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient

data['graphql.operation'] = operationInfo;

if (isStandardRequest(graphqlBody)) {
data['graphql.document'] = graphqlBody.query;
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
data[GRAPHQL_DOCUMENT] = graphqlBody.query;
}

if (isPersistedRequest(graphqlBody)) {
Expand Down
26 changes: 25 additions & 1 deletion packages/browser/test/integrations/graphqlClient.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -313,14 +313,18 @@ describe('GraphqlClient', () => {
});

describe('beforeOutgoingRequestSpan handler', () => {
function setupHandler(endpoints: Array<string | RegExp>): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
function setupHandler(
endpoints: Array<string | RegExp>,
graphQLDocument = true,
): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
let capturedListener: ((span: SentrySpan, hint: FetchHint | XhrHint) => void) | undefined;
const mockClient = {
on: (eventName: string, cb: (span: SentrySpan, hint: FetchHint | XhrHint) => void) => {
if (eventName === 'beforeOutgoingRequestSpan') {
capturedListener = cb;
}
},
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as unknown as Client;

const integration = graphqlClientIntegration({ endpoints });
Expand DownExpand Up@@ -420,5 +424,25 @@ describe('GraphqlClient', () => {
expect(json.description).toBe('custom span');
expect(json.data['graphql.document']).toBeUndefined();
});

test('omits graphql.document when dataCollection.graphQL.document is false', () => {
const handler = setupHandler([/\/graphql$/], false);
const span = new SentrySpan({
name: 'POST http://localhost:4000/graphql',
op: 'http.client',
attributes: {
'http.method': 'POST',
'http.url': 'http://localhost:4000/graphql',
url: 'http://localhost:4000/graphql',
},
});

handler(span, makeFetchHint('http://localhost:4000/graphql', requestBody));

const json = spanToJSON(span);
// The span is still renamed with the operation, but the document is not attached.
expect(json.description).toBe('POST http://localhost:4000/graphql (query GetHello)');
expect(json.data['graphql.document']).toBeUndefined();
});
});
});
22 changes: 21 additions & 1 deletion packages/core/src/types/datacollection.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,7 +51,26 @@ export interface DataCollection {
queryParams?: CollectBehavior;

/**
* Controls generative AI input/output recording.
* Allow collection of GraphQL operation data when using the SDK's GraphQL integrations
*
* These options do not add GraphQL instrumentation on their own. The corresponding integration must still be enabled
* for any data to be captured.
*
* - `document`: attach the GraphQL document (query/mutation source text).
* - `variables`: attach the variables passed to GraphQL operations.
* @default { document: true, variables: true }
*/
graphQL?: {
document?: boolean;
variables?: boolean;
};
Comment on lines +63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The graphQL.variables configuration option is defined but never used, making the feature to control GraphQL variable collection non-functional.
Severity: HIGH

Suggested Fix

Implement the logic to read the getDataCollectionOptions().graphQL.variables setting. Based on its value, conditionally collect or suppress the variables field from GraphQL requests. This logic should be added where GraphQL bodies are processed, such as in packages/browser/src/integrations/graphqlClient.ts and packages/server-utils/src/graphql/utils.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/types/datacollection.ts#L59-L62
Potential issue: The `graphQL.variables` configuration option is defined and documented,
but its value is never read in the production codebase. No code checks
`getDataCollectionOptions().graphQL.variables` before collecting GraphQL variables.
Since no variable collection code exists, the setting has no effect. Users who set
`graphQL: { variables: false }` believing they are suppressing variable collection are
not actually having variables collected (as the feature is missing), and users who set
`variables: true` believing variables will be captured find they are not. The API
contract is unfulfilled as the implementation is missing.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +63 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a blocker but something I was discussing with @ericapisani, this creates the expectation that the SDK is going to apply these options to all GraphQL requests which is not accurate.

It would only apply if the user is using the client or the server GraphQL integrations. GraphQL requests outside of the integrations coverage won't be covered by these options which I find weird. I know the gen AI spans is already a precedent, so I don't have strong opinions here.

An alternative I considered is adding the dataCollection option to the integrations themselves rather than a top-level option.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The core idea of the top-level option is to have only one place where all of this is defined in an on/off switch manner (like: this is allowed to be sent, but you also need the integration to be enabled). This makes it easier to see it all at once. According to the spec, it's also possible to add it on integration level, which would overwrite the option set on the root-level.

Maybe we should make this more clear in the JSDoc?

@logaretmlogaretmJul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine, we already have a precedent for it with gen AI stuff so not feeling too strongly about it but it is something that I wanted to bring up is all.

Maybe adding a line or two in the JS doc to explain what "it won't do" will help here.


/**
* Allow generative AI input/output recording when using the SDK's AI integrations.
*
* These options do not add AI instrumentation on their own. The corresponding integration must still be enabled for
* any data to be captured. Integration-level options, when set, take precedence over these
* values.
* @default { inputs: true, outputs: true }
*/
genAI?: {
Expand All@@ -77,5 +96,6 @@ export interface DataCollection {
*/
export type ResolvedDataCollection = Required<DataCollection> & {
httpHeaders: Required<NonNullable<DataCollection['httpHeaders']>>;
graphQL: Required<NonNullable<DataCollection['graphQL']>>;
genAI: Required<NonNullable<DataCollection['genAI']>>;
};
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand All@@ -26,6 +27,9 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: { deny: PII_HEADER_SNIPPETS }, response: { deny: PII_HEADER_SNIPPETS } },
httpBodies: [],
queryParams: { deny: PII_HEADER_SNIPPETS },
// The GraphQL document has literal values redacted at collection time, so it was historically
// always attached regardless of `sendDefaultPii`; keep it on to preserve that behavior.
graphQL: { document: true, variables: true },
Comment on lines +30 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: When sendDefaultPii is false, graphQL.variables is incorrectly set to true. This contradicts the goal of minimizing PII collection and sets up a potential future PII leak.
Severity: HIGH

Suggested Fix

Change the default setting for graphQL.variables to false when sendDefaultPii is false in defaultPiiToCollectionOptions.ts. This aligns the behavior with other PII-sensitive settings and prevents a future potential PII leak. The line should be changed to graphQL: { document: true, variables: false }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/core/src/utils/data-collection/defaultPiiToCollectionOptions.ts#L30-L32
Potential issue: The default data collection options set `graphQL.variables` to `true`
even when `sendDefaultPii` is `false`. The purpose of `sendDefaultPii: false` is to
prevent the collection of Personally Identifiable Information (PII). GraphQL variables
can contain sensitive user data, and unlike `graphQL.document`, they are not redacted.
While no code currently consumes the `graphQL.variables` setting, this configuration is
a design error. It creates a high risk that future code implementing GraphQL variable
collection will inadvertently capture and send PII for users who have opted out, as the
default setting will be assumed to be safe.

genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ const DEFAULTS: ResolvedDataCollection = {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand DownExpand Up@@ -43,6 +44,10 @@ export function resolveDataCollectionOptions(options: {
},
httpBodies: dc.httpBodies ?? base.httpBodies,
queryParams: dc.queryParams ?? base.queryParams,
graphQL: {
document: dc.graphQL?.document ?? base.graphQL.document,
variables: dc.graphQL?.variables ?? base.graphQL.variables,
},
genAI: {
inputs: dc.genAI?.inputs ?? base.genAI.inputs,
outputs: dc.genAI?.outputs ?? base.genAI.outputs,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ export function createMockClient(userInfo = true, genAI?: { inputs: boolean; out
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: genAIOptions,
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@ describe('defaultPiiToCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -25,6 +26,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -41,6 +43,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -57,6 +60,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ describe('resolveDataCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand All@@ -21,6 +22,8 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
// GraphQL documents are redacted at collection time, so they stay on to preserve legacy behavior.
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(7);
});
Expand All@@ -45,6 +48,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
});

Expand All@@ -54,6 +58,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
expect(result.graphQL).toEqual({ document: true, variables: true });
});
});

Expand DownExpand Up@@ -87,6 +92,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.cookies).toBe(true);
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(5);
Expand DownExpand Up@@ -114,6 +120,17 @@ describe('resolveDataCollectionOptions', () => {
expect(result.genAI.outputs).toBe(true);
});

it('merges nested graphQL partially', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
graphQL: { document: false },
},
});

expect(result.graphQL.document).toBe(false);
expect(result.graphQL.variables).toBe(true);
});

it('supports allow/deny list for cookies', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
Expand All@@ -139,14 +156,17 @@ describe('resolveDataCollectionOptions', () => {
it('always returns all fields', () => {
const result = resolveDataCollectionOptions({});

expect(Object.keys(result)).toHaveLength(8);
expect(Object.keys(result)).toHaveLength(9);
expect(result).toHaveProperty('userInfo');
expect(result).toHaveProperty('cookies');
expect(result).toHaveProperty('httpHeaders');
expect(result).toHaveProperty('httpHeaders.request');
expect(result).toHaveProperty('httpHeaders.response');
expect(result).toHaveProperty('httpBodies');
expect(result).toHaveProperty('queryParams');
expect(result).toHaveProperty('graphQL');
expect(result).toHaveProperty('graphQL.document');
expect(result).toHaveProperty('graphQL.variables');
expect(result).toHaveProperty('genAI');
expect(result).toHaveProperty('genAI.inputs');
expect(result).toHaveProperty('genAI.outputs');
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/lib/utils/request.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -874,6 +874,7 @@ describe('request utils', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type {
Token,
} from './graphql-types';
import type { Span, SpanAttributes } from '@sentry/core';
import { isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { getClient, isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { AllowedOperationTypes, SpanNames, TokenKind } from './enum';
import { AttributeNames } from './enums/AttributeNames';
import { OTEL_GRAPHQL_DATA_SYMBOL, OTEL_PATCHED_SYMBOL } from './symbols';
Expand All@@ -38,8 +38,10 @@ export const isPromise = (value: any): value is Promise<unknown> => {
};

export function addSpanSource(span: Span, loc?: Location, start?: number, end?: number): void {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
if (getClient()?.getDataCollectionOptions().graphQL.document === true) {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
}
}

function createFieldIfNotExists(
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as core from '@sentry/core';
import { addSpanSource } from '../../../../src/integrations/tracing/graphql/vendored/utils';
import type { Location, Token } from '@sentry/server-utils/orchestrion';

vi.spyOn(core, 'getClient');

function mockClient(graphQLDocument: boolean): void {
vi.mocked(core.getClient).mockReturnValue({
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as any);
}

function makeLocation(): Location {
const token: Token = {
kind: 'Name',
start: 0,
end: 5,
line: 1,
column: 1,
value: 'hello',
prev: null,
next: null,
};
return { start: 0, end: 5, startToken: token };
}

describe('addSpanSource dataCollection gate', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('sets graphql.source attribute when graphQL.document is true', () => {
mockClient(true);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).toHaveBeenCalledWith('graphql.source', expect.any(String));
});

it('does not set attribute when graphQL.document is false', () => {
mockClient(false);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});

it('does not set attribute when getClient() is undefined', () => {
vi.mocked(core.getClient).mockReturnValue(undefined);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,7 @@ describe('wrapMiddlewareHandlerWithSentry', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/test/server/errors.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ function createMockClient(httpBodies: string[] = []): Client {
httpHeaders: { request: true, response: true },
httpBodies,
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions packages/astro/test/server/middleware.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -64,6 +64,7 @@ describe('sentryMiddleware', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
11 changes: 6 additions & 5 deletions packages/browser/src/integrations/graphqlClient.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import {
} from '@sentry/core/browser';
import type { FetchHint, XhrHint } from '@sentry/browser-utils';
import { getBodyString, getFetchRequestArgBody, SENTRY_XHR_DATA_KEY } from '@sentry/browser-utils';
import { GRAPHQL_DOCUMENT } from '@sentry/conventions/attributes';

interface GraphQLClientOptions {
endpoints: Array<string | RegExp>;
Expand DownExpand Up@@ -88,9 +89,9 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
const operationInfo = _getGraphQLOperation(graphqlBody);
span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);

// Handle standard requests - always capture the query document
if (isStandardRequest(graphqlBody)) {
span.setAttribute('graphql.document', graphqlBody.query);
// Handle standard requests - capture the query document when enabled via dataCollection (default true)
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
span.setAttribute(GRAPHQL_DOCUMENT, graphqlBody.query);
}

// Handle persisted operations - capture hash for debugging
Expand DownExpand Up@@ -126,8 +127,8 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient

data['graphql.operation'] = operationInfo;

if (isStandardRequest(graphqlBody)) {
data['graphql.document'] = graphqlBody.query;
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
data[GRAPHQL_DOCUMENT] = graphqlBody.query;
}

if (isPersistedRequest(graphqlBody)) {
Expand Down
26 changes: 25 additions & 1 deletion packages/browser/test/integrations/graphqlClient.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -313,14 +313,18 @@ describe('GraphqlClient', () => {
});

describe('beforeOutgoingRequestSpan handler', () => {
function setupHandler(endpoints: Array<string | RegExp>): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
function setupHandler(
endpoints: Array<string | RegExp>,
graphQLDocument = true,
): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
let capturedListener: ((span: SentrySpan, hint: FetchHint | XhrHint) => void) | undefined;
const mockClient = {
on: (eventName: string, cb: (span: SentrySpan, hint: FetchHint | XhrHint) => void) => {
if (eventName === 'beforeOutgoingRequestSpan') {
capturedListener = cb;
}
},
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as unknown as Client;

const integration = graphqlClientIntegration({ endpoints });
Expand DownExpand Up@@ -420,5 +424,25 @@ describe('GraphqlClient', () => {
expect(json.description).toBe('custom span');
expect(json.data['graphql.document']).toBeUndefined();
});

test('omits graphql.document when dataCollection.graphQL.document is false', () => {
const handler = setupHandler([/\/graphql$/], false);
const span = new SentrySpan({
name: 'POST http://localhost:4000/graphql',
op: 'http.client',
attributes: {
'http.method': 'POST',
'http.url': 'http://localhost:4000/graphql',
url: 'http://localhost:4000/graphql',
},
});

handler(span, makeFetchHint('http://localhost:4000/graphql', requestBody));

const json = spanToJSON(span);
// The span is still renamed with the operation, but the document is not attached.
expect(json.description).toBe('POST http://localhost:4000/graphql (query GetHello)');
expect(json.data['graphql.document']).toBeUndefined();
});
});
});
22 changes: 21 additions & 1 deletion packages/core/src/types/datacollection.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,7 +51,26 @@ export interface DataCollection {
queryParams?: CollectBehavior;

/**
* Controls generative AI input/output recording.
* Allow collection of GraphQL operation data when using the SDK's GraphQL integrations
*
* These options do not add GraphQL instrumentation on their own. The corresponding integration must still be enabled
* for any data to be captured.
*
* - `document`: attach the GraphQL document (query/mutation source text).
* - `variables`: attach the variables passed to GraphQL operations.
* @default { document: true, variables: true }
*/
graphQL?: {
document?: boolean;
variables?: boolean;
};
Comment on lines +63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The graphQL.variables configuration option is defined but never used, making the feature to control GraphQL variable collection non-functional.
Severity: HIGH

Suggested Fix

Implement the logic to read the getDataCollectionOptions().graphQL.variables setting. Based on its value, conditionally collect or suppress the variables field from GraphQL requests. This logic should be added where GraphQL bodies are processed, such as in packages/browser/src/integrations/graphqlClient.ts and packages/server-utils/src/graphql/utils.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/types/datacollection.ts#L59-L62
Potential issue: The `graphQL.variables` configuration option is defined and documented,
but its value is never read in the production codebase. No code checks
`getDataCollectionOptions().graphQL.variables` before collecting GraphQL variables.
Since no variable collection code exists, the setting has no effect. Users who set
`graphQL: { variables: false }` believing they are suppressing variable collection are
not actually having variables collected (as the feature is missing), and users who set
`variables: true` believing variables will be captured find they are not. The API
contract is unfulfilled as the implementation is missing.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +63 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a blocker but something I was discussing with @ericapisani, this creates the expectation that the SDK is going to apply these options to all GraphQL requests which is not accurate.

It would only apply if the user is using the client or the server GraphQL integrations. GraphQL requests outside of the integrations coverage won't be covered by these options which I find weird. I know the gen AI spans is already a precedent, so I don't have strong opinions here.

An alternative I considered is adding the dataCollection option to the integrations themselves rather than a top-level option.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The core idea of the top-level option is to have only one place where all of this is defined in an on/off switch manner (like: this is allowed to be sent, but you also need the integration to be enabled). This makes it easier to see it all at once. According to the spec, it's also possible to add it on integration level, which would overwrite the option set on the root-level.

Maybe we should make this more clear in the JSDoc?

@logaretmlogaretmJul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine, we already have a precedent for it with gen AI stuff so not feeling too strongly about it but it is something that I wanted to bring up is all.

Maybe adding a line or two in the JS doc to explain what "it won't do" will help here.


/**
* Allow generative AI input/output recording when using the SDK's AI integrations.
*
* These options do not add AI instrumentation on their own. The corresponding integration must still be enabled for
* any data to be captured. Integration-level options, when set, take precedence over these
* values.
* @default { inputs: true, outputs: true }
*/
genAI?: {
Expand All@@ -77,5 +96,6 @@ export interface DataCollection {
*/
export type ResolvedDataCollection = Required<DataCollection> & {
httpHeaders: Required<NonNullable<DataCollection['httpHeaders']>>;
graphQL: Required<NonNullable<DataCollection['graphQL']>>;
genAI: Required<NonNullable<DataCollection['genAI']>>;
};
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand All@@ -26,6 +27,9 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: { deny: PII_HEADER_SNIPPETS }, response: { deny: PII_HEADER_SNIPPETS } },
httpBodies: [],
queryParams: { deny: PII_HEADER_SNIPPETS },
// The GraphQL document has literal values redacted at collection time, so it was historically
// always attached regardless of `sendDefaultPii`; keep it on to preserve that behavior.
graphQL: { document: true, variables: true },
Comment on lines +30 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: When sendDefaultPii is false, graphQL.variables is incorrectly set to true. This contradicts the goal of minimizing PII collection and sets up a potential future PII leak.
Severity: HIGH

Suggested Fix

Change the default setting for graphQL.variables to false when sendDefaultPii is false in defaultPiiToCollectionOptions.ts. This aligns the behavior with other PII-sensitive settings and prevents a future potential PII leak. The line should be changed to graphQL: { document: true, variables: false }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/core/src/utils/data-collection/defaultPiiToCollectionOptions.ts#L30-L32
Potential issue: The default data collection options set `graphQL.variables` to `true`
even when `sendDefaultPii` is `false`. The purpose of `sendDefaultPii: false` is to
prevent the collection of Personally Identifiable Information (PII). GraphQL variables
can contain sensitive user data, and unlike `graphQL.document`, they are not redacted.
While no code currently consumes the `graphQL.variables` setting, this configuration is
a design error. It creates a high risk that future code implementing GraphQL variable
collection will inadvertently capture and send PII for users who have opted out, as the
default setting will be assumed to be safe.

genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ const DEFAULTS: ResolvedDataCollection = {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand DownExpand Up@@ -43,6 +44,10 @@ export function resolveDataCollectionOptions(options: {
},
httpBodies: dc.httpBodies ?? base.httpBodies,
queryParams: dc.queryParams ?? base.queryParams,
graphQL: {
document: dc.graphQL?.document ?? base.graphQL.document,
variables: dc.graphQL?.variables ?? base.graphQL.variables,
},
genAI: {
inputs: dc.genAI?.inputs ?? base.genAI.inputs,
outputs: dc.genAI?.outputs ?? base.genAI.outputs,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ export function createMockClient(userInfo = true, genAI?: { inputs: boolean; out
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: genAIOptions,
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@ describe('defaultPiiToCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -25,6 +26,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -41,6 +43,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -57,6 +60,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ describe('resolveDataCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand All@@ -21,6 +22,8 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
// GraphQL documents are redacted at collection time, so they stay on to preserve legacy behavior.
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(7);
});
Expand All@@ -45,6 +48,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
});

Expand All@@ -54,6 +58,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
expect(result.graphQL).toEqual({ document: true, variables: true });
});
});

Expand DownExpand Up@@ -87,6 +92,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.cookies).toBe(true);
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(5);
Expand DownExpand Up@@ -114,6 +120,17 @@ describe('resolveDataCollectionOptions', () => {
expect(result.genAI.outputs).toBe(true);
});

it('merges nested graphQL partially', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
graphQL: { document: false },
},
});

expect(result.graphQL.document).toBe(false);
expect(result.graphQL.variables).toBe(true);
});

it('supports allow/deny list for cookies', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
Expand All@@ -139,14 +156,17 @@ describe('resolveDataCollectionOptions', () => {
it('always returns all fields', () => {
const result = resolveDataCollectionOptions({});

expect(Object.keys(result)).toHaveLength(8);
expect(Object.keys(result)).toHaveLength(9);
expect(result).toHaveProperty('userInfo');
expect(result).toHaveProperty('cookies');
expect(result).toHaveProperty('httpHeaders');
expect(result).toHaveProperty('httpHeaders.request');
expect(result).toHaveProperty('httpHeaders.response');
expect(result).toHaveProperty('httpBodies');
expect(result).toHaveProperty('queryParams');
expect(result).toHaveProperty('graphQL');
expect(result).toHaveProperty('graphQL.document');
expect(result).toHaveProperty('graphQL.variables');
expect(result).toHaveProperty('genAI');
expect(result).toHaveProperty('genAI.inputs');
expect(result).toHaveProperty('genAI.outputs');
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/lib/utils/request.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -874,6 +874,7 @@ describe('request utils', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type {
Token,
} from './graphql-types';
import type { Span, SpanAttributes } from '@sentry/core';
import { isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { getClient, isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { AllowedOperationTypes, SpanNames, TokenKind } from './enum';
import { AttributeNames } from './enums/AttributeNames';
import { OTEL_GRAPHQL_DATA_SYMBOL, OTEL_PATCHED_SYMBOL } from './symbols';
Expand All@@ -38,8 +38,10 @@ export const isPromise = (value: any): value is Promise<unknown> => {
};

export function addSpanSource(span: Span, loc?: Location, start?: number, end?: number): void {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
if (getClient()?.getDataCollectionOptions().graphQL.document === true) {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
}
}

function createFieldIfNotExists(
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as core from '@sentry/core';
import { addSpanSource } from '../../../../src/integrations/tracing/graphql/vendored/utils';
import type { Location, Token } from '@sentry/server-utils/orchestrion';

vi.spyOn(core, 'getClient');

function mockClient(graphQLDocument: boolean): void {
vi.mocked(core.getClient).mockReturnValue({
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as any);
}

function makeLocation(): Location {
const token: Token = {
kind: 'Name',
start: 0,
end: 5,
line: 1,
column: 1,
value: 'hello',
prev: null,
next: null,
};
return { start: 0, end: 5, startToken: token };
}

describe('addSpanSource dataCollection gate', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('sets graphql.source attribute when graphQL.document is true', () => {
mockClient(true);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).toHaveBeenCalledWith('graphql.source', expect.any(String));
});

it('does not set attribute when graphQL.document is false', () => {
mockClient(false);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});

it('does not set attribute when getClient() is undefined', () => {
vi.mocked(core.getClient).mockReturnValue(undefined);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,7 @@ describe('wrapMiddlewareHandlerWithSentry', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/test/server/errors.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ function createMockClient(httpBodies: string[] = []): Client {
httpHeaders: { request: true, response: true },
httpBodies,
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions packages/astro/test/server/middleware.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -64,6 +64,7 @@ describe('sentryMiddleware', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
11 changes: 6 additions & 5 deletions packages/browser/src/integrations/graphqlClient.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import {
} from '@sentry/core/browser';
import type { FetchHint, XhrHint } from '@sentry/browser-utils';
import { getBodyString, getFetchRequestArgBody, SENTRY_XHR_DATA_KEY } from '@sentry/browser-utils';
import { GRAPHQL_DOCUMENT } from '@sentry/conventions/attributes';

interface GraphQLClientOptions {
endpoints: Array<string | RegExp>;
Expand DownExpand Up@@ -88,9 +89,9 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
const operationInfo = _getGraphQLOperation(graphqlBody);
span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);

// Handle standard requests - always capture the query document
if (isStandardRequest(graphqlBody)) {
span.setAttribute('graphql.document', graphqlBody.query);
// Handle standard requests - capture the query document when enabled via dataCollection (default true)
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
span.setAttribute(GRAPHQL_DOCUMENT, graphqlBody.query);
}

// Handle persisted operations - capture hash for debugging
Expand DownExpand Up@@ -126,8 +127,8 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient

data['graphql.operation'] = operationInfo;

if (isStandardRequest(graphqlBody)) {
data['graphql.document'] = graphqlBody.query;
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
data[GRAPHQL_DOCUMENT] = graphqlBody.query;
}

if (isPersistedRequest(graphqlBody)) {
Expand Down
26 changes: 25 additions & 1 deletion packages/browser/test/integrations/graphqlClient.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -313,14 +313,18 @@ describe('GraphqlClient', () => {
});

describe('beforeOutgoingRequestSpan handler', () => {
function setupHandler(endpoints: Array<string | RegExp>): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
function setupHandler(
endpoints: Array<string | RegExp>,
graphQLDocument = true,
): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
let capturedListener: ((span: SentrySpan, hint: FetchHint | XhrHint) => void) | undefined;
const mockClient = {
on: (eventName: string, cb: (span: SentrySpan, hint: FetchHint | XhrHint) => void) => {
if (eventName === 'beforeOutgoingRequestSpan') {
capturedListener = cb;
}
},
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as unknown as Client;

const integration = graphqlClientIntegration({ endpoints });
Expand DownExpand Up@@ -420,5 +424,25 @@ describe('GraphqlClient', () => {
expect(json.description).toBe('custom span');
expect(json.data['graphql.document']).toBeUndefined();
});

test('omits graphql.document when dataCollection.graphQL.document is false', () => {
const handler = setupHandler([/\/graphql$/], false);
const span = new SentrySpan({
name: 'POST http://localhost:4000/graphql',
op: 'http.client',
attributes: {
'http.method': 'POST',
'http.url': 'http://localhost:4000/graphql',
url: 'http://localhost:4000/graphql',
},
});

handler(span, makeFetchHint('http://localhost:4000/graphql', requestBody));

const json = spanToJSON(span);
// The span is still renamed with the operation, but the document is not attached.
expect(json.description).toBe('POST http://localhost:4000/graphql (query GetHello)');
expect(json.data['graphql.document']).toBeUndefined();
});
});
});
22 changes: 21 additions & 1 deletion packages/core/src/types/datacollection.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,7 +51,26 @@ export interface DataCollection {
queryParams?: CollectBehavior;

/**
* Controls generative AI input/output recording.
* Allow collection of GraphQL operation data when using the SDK's GraphQL integrations
*
* These options do not add GraphQL instrumentation on their own. The corresponding integration must still be enabled
* for any data to be captured.
*
* - `document`: attach the GraphQL document (query/mutation source text).
* - `variables`: attach the variables passed to GraphQL operations.
* @default { document: true, variables: true }
*/
graphQL?: {
document?: boolean;
variables?: boolean;
};
Comment on lines +63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The graphQL.variables configuration option is defined but never used, making the feature to control GraphQL variable collection non-functional.
Severity: HIGH

Suggested Fix

Implement the logic to read the getDataCollectionOptions().graphQL.variables setting. Based on its value, conditionally collect or suppress the variables field from GraphQL requests. This logic should be added where GraphQL bodies are processed, such as in packages/browser/src/integrations/graphqlClient.ts and packages/server-utils/src/graphql/utils.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/types/datacollection.ts#L59-L62
Potential issue: The `graphQL.variables` configuration option is defined and documented,
but its value is never read in the production codebase. No code checks
`getDataCollectionOptions().graphQL.variables` before collecting GraphQL variables.
Since no variable collection code exists, the setting has no effect. Users who set
`graphQL: { variables: false }` believing they are suppressing variable collection are
not actually having variables collected (as the feature is missing), and users who set
`variables: true` believing variables will be captured find they are not. The API
contract is unfulfilled as the implementation is missing.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +63 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a blocker but something I was discussing with @ericapisani, this creates the expectation that the SDK is going to apply these options to all GraphQL requests which is not accurate.

It would only apply if the user is using the client or the server GraphQL integrations. GraphQL requests outside of the integrations coverage won't be covered by these options which I find weird. I know the gen AI spans is already a precedent, so I don't have strong opinions here.

An alternative I considered is adding the dataCollection option to the integrations themselves rather than a top-level option.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The core idea of the top-level option is to have only one place where all of this is defined in an on/off switch manner (like: this is allowed to be sent, but you also need the integration to be enabled). This makes it easier to see it all at once. According to the spec, it's also possible to add it on integration level, which would overwrite the option set on the root-level.

Maybe we should make this more clear in the JSDoc?

@logaretmlogaretmJul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine, we already have a precedent for it with gen AI stuff so not feeling too strongly about it but it is something that I wanted to bring up is all.

Maybe adding a line or two in the JS doc to explain what "it won't do" will help here.


/**
* Allow generative AI input/output recording when using the SDK's AI integrations.
*
* These options do not add AI instrumentation on their own. The corresponding integration must still be enabled for
* any data to be captured. Integration-level options, when set, take precedence over these
* values.
* @default { inputs: true, outputs: true }
*/
genAI?: {
Expand All@@ -77,5 +96,6 @@ export interface DataCollection {
*/
export type ResolvedDataCollection = Required<DataCollection> & {
httpHeaders: Required<NonNullable<DataCollection['httpHeaders']>>;
graphQL: Required<NonNullable<DataCollection['graphQL']>>;
genAI: Required<NonNullable<DataCollection['genAI']>>;
};
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand All@@ -26,6 +27,9 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: { deny: PII_HEADER_SNIPPETS }, response: { deny: PII_HEADER_SNIPPETS } },
httpBodies: [],
queryParams: { deny: PII_HEADER_SNIPPETS },
// The GraphQL document has literal values redacted at collection time, so it was historically
// always attached regardless of `sendDefaultPii`; keep it on to preserve that behavior.
graphQL: { document: true, variables: true },
Comment on lines +30 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: When sendDefaultPii is false, graphQL.variables is incorrectly set to true. This contradicts the goal of minimizing PII collection and sets up a potential future PII leak.
Severity: HIGH

Suggested Fix

Change the default setting for graphQL.variables to false when sendDefaultPii is false in defaultPiiToCollectionOptions.ts. This aligns the behavior with other PII-sensitive settings and prevents a future potential PII leak. The line should be changed to graphQL: { document: true, variables: false }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/core/src/utils/data-collection/defaultPiiToCollectionOptions.ts#L30-L32
Potential issue: The default data collection options set `graphQL.variables` to `true`
even when `sendDefaultPii` is `false`. The purpose of `sendDefaultPii: false` is to
prevent the collection of Personally Identifiable Information (PII). GraphQL variables
can contain sensitive user data, and unlike `graphQL.document`, they are not redacted.
While no code currently consumes the `graphQL.variables` setting, this configuration is
a design error. It creates a high risk that future code implementing GraphQL variable
collection will inadvertently capture and send PII for users who have opted out, as the
default setting will be assumed to be safe.

genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ const DEFAULTS: ResolvedDataCollection = {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand DownExpand Up@@ -43,6 +44,10 @@ export function resolveDataCollectionOptions(options: {
},
httpBodies: dc.httpBodies ?? base.httpBodies,
queryParams: dc.queryParams ?? base.queryParams,
graphQL: {
document: dc.graphQL?.document ?? base.graphQL.document,
variables: dc.graphQL?.variables ?? base.graphQL.variables,
},
genAI: {
inputs: dc.genAI?.inputs ?? base.genAI.inputs,
outputs: dc.genAI?.outputs ?? base.genAI.outputs,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ export function createMockClient(userInfo = true, genAI?: { inputs: boolean; out
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: genAIOptions,
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@ describe('defaultPiiToCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -25,6 +26,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -41,6 +43,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -57,6 +60,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ describe('resolveDataCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand All@@ -21,6 +22,8 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
// GraphQL documents are redacted at collection time, so they stay on to preserve legacy behavior.
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(7);
});
Expand All@@ -45,6 +48,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
});

Expand All@@ -54,6 +58,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
expect(result.graphQL).toEqual({ document: true, variables: true });
});
});

Expand DownExpand Up@@ -87,6 +92,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.cookies).toBe(true);
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(5);
Expand DownExpand Up@@ -114,6 +120,17 @@ describe('resolveDataCollectionOptions', () => {
expect(result.genAI.outputs).toBe(true);
});

it('merges nested graphQL partially', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
graphQL: { document: false },
},
});

expect(result.graphQL.document).toBe(false);
expect(result.graphQL.variables).toBe(true);
});

it('supports allow/deny list for cookies', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
Expand All@@ -139,14 +156,17 @@ describe('resolveDataCollectionOptions', () => {
it('always returns all fields', () => {
const result = resolveDataCollectionOptions({});

expect(Object.keys(result)).toHaveLength(8);
expect(Object.keys(result)).toHaveLength(9);
expect(result).toHaveProperty('userInfo');
expect(result).toHaveProperty('cookies');
expect(result).toHaveProperty('httpHeaders');
expect(result).toHaveProperty('httpHeaders.request');
expect(result).toHaveProperty('httpHeaders.response');
expect(result).toHaveProperty('httpBodies');
expect(result).toHaveProperty('queryParams');
expect(result).toHaveProperty('graphQL');
expect(result).toHaveProperty('graphQL.document');
expect(result).toHaveProperty('graphQL.variables');
expect(result).toHaveProperty('genAI');
expect(result).toHaveProperty('genAI.inputs');
expect(result).toHaveProperty('genAI.outputs');
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/lib/utils/request.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -874,6 +874,7 @@ describe('request utils', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type {
Token,
} from './graphql-types';
import type { Span, SpanAttributes } from '@sentry/core';
import { isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { getClient, isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { AllowedOperationTypes, SpanNames, TokenKind } from './enum';
import { AttributeNames } from './enums/AttributeNames';
import { OTEL_GRAPHQL_DATA_SYMBOL, OTEL_PATCHED_SYMBOL } from './symbols';
Expand All@@ -38,8 +38,10 @@ export const isPromise = (value: any): value is Promise<unknown> => {
};

export function addSpanSource(span: Span, loc?: Location, start?: number, end?: number): void {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
if (getClient()?.getDataCollectionOptions().graphQL.document === true) {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
}
}

function createFieldIfNotExists(
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as core from '@sentry/core';
import { addSpanSource } from '../../../../src/integrations/tracing/graphql/vendored/utils';
import type { Location, Token } from '@sentry/server-utils/orchestrion';

vi.spyOn(core, 'getClient');

function mockClient(graphQLDocument: boolean): void {
vi.mocked(core.getClient).mockReturnValue({
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as any);
}

function makeLocation(): Location {
const token: Token = {
kind: 'Name',
start: 0,
end: 5,
line: 1,
column: 1,
value: 'hello',
prev: null,
next: null,
};
return { start: 0, end: 5, startToken: token };
}

describe('addSpanSource dataCollection gate', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('sets graphql.source attribute when graphQL.document is true', () => {
mockClient(true);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).toHaveBeenCalledWith('graphql.source', expect.any(String));
});

it('does not set attribute when graphQL.document is false', () => {
mockClient(false);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});

it('does not set attribute when getClient() is undefined', () => {
vi.mocked(core.getClient).mockReturnValue(undefined);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,7 @@ describe('wrapMiddlewareHandlerWithSentry', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/test/server/errors.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ function createMockClient(httpBodies: string[] = []): Client {
httpHeaders: { request: true, response: true },
httpBodies,
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions packages/astro/test/server/middleware.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -64,6 +64,7 @@ describe('sentryMiddleware', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
11 changes: 6 additions & 5 deletions packages/browser/src/integrations/graphqlClient.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import {
} from '@sentry/core/browser';
import type { FetchHint, XhrHint } from '@sentry/browser-utils';
import { getBodyString, getFetchRequestArgBody, SENTRY_XHR_DATA_KEY } from '@sentry/browser-utils';
import { GRAPHQL_DOCUMENT } from '@sentry/conventions/attributes';

interface GraphQLClientOptions {
endpoints: Array<string | RegExp>;
Expand DownExpand Up@@ -88,9 +89,9 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
const operationInfo = _getGraphQLOperation(graphqlBody);
span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);

// Handle standard requests - always capture the query document
if (isStandardRequest(graphqlBody)) {
span.setAttribute('graphql.document', graphqlBody.query);
// Handle standard requests - capture the query document when enabled via dataCollection (default true)
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
span.setAttribute(GRAPHQL_DOCUMENT, graphqlBody.query);
}

// Handle persisted operations - capture hash for debugging
Expand DownExpand Up@@ -126,8 +127,8 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient

data['graphql.operation'] = operationInfo;

if (isStandardRequest(graphqlBody)) {
data['graphql.document'] = graphqlBody.query;
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
data[GRAPHQL_DOCUMENT] = graphqlBody.query;
}

if (isPersistedRequest(graphqlBody)) {
Expand Down
26 changes: 25 additions & 1 deletion packages/browser/test/integrations/graphqlClient.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -313,14 +313,18 @@ describe('GraphqlClient', () => {
});

describe('beforeOutgoingRequestSpan handler', () => {
function setupHandler(endpoints: Array<string | RegExp>): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
function setupHandler(
endpoints: Array<string | RegExp>,
graphQLDocument = true,
): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
let capturedListener: ((span: SentrySpan, hint: FetchHint | XhrHint) => void) | undefined;
const mockClient = {
on: (eventName: string, cb: (span: SentrySpan, hint: FetchHint | XhrHint) => void) => {
if (eventName === 'beforeOutgoingRequestSpan') {
capturedListener = cb;
}
},
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as unknown as Client;

const integration = graphqlClientIntegration({ endpoints });
Expand DownExpand Up@@ -420,5 +424,25 @@ describe('GraphqlClient', () => {
expect(json.description).toBe('custom span');
expect(json.data['graphql.document']).toBeUndefined();
});

test('omits graphql.document when dataCollection.graphQL.document is false', () => {
const handler = setupHandler([/\/graphql$/], false);
const span = new SentrySpan({
name: 'POST http://localhost:4000/graphql',
op: 'http.client',
attributes: {
'http.method': 'POST',
'http.url': 'http://localhost:4000/graphql',
url: 'http://localhost:4000/graphql',
},
});

handler(span, makeFetchHint('http://localhost:4000/graphql', requestBody));

const json = spanToJSON(span);
// The span is still renamed with the operation, but the document is not attached.
expect(json.description).toBe('POST http://localhost:4000/graphql (query GetHello)');
expect(json.data['graphql.document']).toBeUndefined();
});
});
});
22 changes: 21 additions & 1 deletion packages/core/src/types/datacollection.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,7 +51,26 @@ export interface DataCollection {
queryParams?: CollectBehavior;

/**
* Controls generative AI input/output recording.
* Allow collection of GraphQL operation data when using the SDK's GraphQL integrations
*
* These options do not add GraphQL instrumentation on their own. The corresponding integration must still be enabled
* for any data to be captured.
*
* - `document`: attach the GraphQL document (query/mutation source text).
* - `variables`: attach the variables passed to GraphQL operations.
* @default { document: true, variables: true }
*/
graphQL?: {
document?: boolean;
variables?: boolean;
};
Comment on lines +63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The graphQL.variables configuration option is defined but never used, making the feature to control GraphQL variable collection non-functional.
Severity: HIGH

Suggested Fix

Implement the logic to read the getDataCollectionOptions().graphQL.variables setting. Based on its value, conditionally collect or suppress the variables field from GraphQL requests. This logic should be added where GraphQL bodies are processed, such as in packages/browser/src/integrations/graphqlClient.ts and packages/server-utils/src/graphql/utils.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/types/datacollection.ts#L59-L62
Potential issue: The `graphQL.variables` configuration option is defined and documented,
but its value is never read in the production codebase. No code checks
`getDataCollectionOptions().graphQL.variables` before collecting GraphQL variables.
Since no variable collection code exists, the setting has no effect. Users who set
`graphQL: { variables: false }` believing they are suppressing variable collection are
not actually having variables collected (as the feature is missing), and users who set
`variables: true` believing variables will be captured find they are not. The API
contract is unfulfilled as the implementation is missing.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +63 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a blocker but something I was discussing with @ericapisani, this creates the expectation that the SDK is going to apply these options to all GraphQL requests which is not accurate.

It would only apply if the user is using the client or the server GraphQL integrations. GraphQL requests outside of the integrations coverage won't be covered by these options which I find weird. I know the gen AI spans is already a precedent, so I don't have strong opinions here.

An alternative I considered is adding the dataCollection option to the integrations themselves rather than a top-level option.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The core idea of the top-level option is to have only one place where all of this is defined in an on/off switch manner (like: this is allowed to be sent, but you also need the integration to be enabled). This makes it easier to see it all at once. According to the spec, it's also possible to add it on integration level, which would overwrite the option set on the root-level.

Maybe we should make this more clear in the JSDoc?

@logaretmlogaretmJul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine, we already have a precedent for it with gen AI stuff so not feeling too strongly about it but it is something that I wanted to bring up is all.

Maybe adding a line or two in the JS doc to explain what "it won't do" will help here.


/**
* Allow generative AI input/output recording when using the SDK's AI integrations.
*
* These options do not add AI instrumentation on their own. The corresponding integration must still be enabled for
* any data to be captured. Integration-level options, when set, take precedence over these
* values.
* @default { inputs: true, outputs: true }
*/
genAI?: {
Expand All@@ -77,5 +96,6 @@ export interface DataCollection {
*/
export type ResolvedDataCollection = Required<DataCollection> & {
httpHeaders: Required<NonNullable<DataCollection['httpHeaders']>>;
graphQL: Required<NonNullable<DataCollection['graphQL']>>;
genAI: Required<NonNullable<DataCollection['genAI']>>;
};
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand All@@ -26,6 +27,9 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: { deny: PII_HEADER_SNIPPETS }, response: { deny: PII_HEADER_SNIPPETS } },
httpBodies: [],
queryParams: { deny: PII_HEADER_SNIPPETS },
// The GraphQL document has literal values redacted at collection time, so it was historically
// always attached regardless of `sendDefaultPii`; keep it on to preserve that behavior.
graphQL: { document: true, variables: true },
Comment on lines +30 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: When sendDefaultPii is false, graphQL.variables is incorrectly set to true. This contradicts the goal of minimizing PII collection and sets up a potential future PII leak.
Severity: HIGH

Suggested Fix

Change the default setting for graphQL.variables to false when sendDefaultPii is false in defaultPiiToCollectionOptions.ts. This aligns the behavior with other PII-sensitive settings and prevents a future potential PII leak. The line should be changed to graphQL: { document: true, variables: false }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/core/src/utils/data-collection/defaultPiiToCollectionOptions.ts#L30-L32
Potential issue: The default data collection options set `graphQL.variables` to `true`
even when `sendDefaultPii` is `false`. The purpose of `sendDefaultPii: false` is to
prevent the collection of Personally Identifiable Information (PII). GraphQL variables
can contain sensitive user data, and unlike `graphQL.document`, they are not redacted.
While no code currently consumes the `graphQL.variables` setting, this configuration is
a design error. It creates a high risk that future code implementing GraphQL variable
collection will inadvertently capture and send PII for users who have opted out, as the
default setting will be assumed to be safe.

genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ const DEFAULTS: ResolvedDataCollection = {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand DownExpand Up@@ -43,6 +44,10 @@ export function resolveDataCollectionOptions(options: {
},
httpBodies: dc.httpBodies ?? base.httpBodies,
queryParams: dc.queryParams ?? base.queryParams,
graphQL: {
document: dc.graphQL?.document ?? base.graphQL.document,
variables: dc.graphQL?.variables ?? base.graphQL.variables,
},
genAI: {
inputs: dc.genAI?.inputs ?? base.genAI.inputs,
outputs: dc.genAI?.outputs ?? base.genAI.outputs,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ export function createMockClient(userInfo = true, genAI?: { inputs: boolean; out
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: genAIOptions,
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@ describe('defaultPiiToCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -25,6 +26,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -41,6 +43,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -57,6 +60,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ describe('resolveDataCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand All@@ -21,6 +22,8 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
// GraphQL documents are redacted at collection time, so they stay on to preserve legacy behavior.
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(7);
});
Expand All@@ -45,6 +48,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
});

Expand All@@ -54,6 +58,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
expect(result.graphQL).toEqual({ document: true, variables: true });
});
});

Expand DownExpand Up@@ -87,6 +92,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.cookies).toBe(true);
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(5);
Expand DownExpand Up@@ -114,6 +120,17 @@ describe('resolveDataCollectionOptions', () => {
expect(result.genAI.outputs).toBe(true);
});

it('merges nested graphQL partially', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
graphQL: { document: false },
},
});

expect(result.graphQL.document).toBe(false);
expect(result.graphQL.variables).toBe(true);
});

it('supports allow/deny list for cookies', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
Expand All@@ -139,14 +156,17 @@ describe('resolveDataCollectionOptions', () => {
it('always returns all fields', () => {
const result = resolveDataCollectionOptions({});

expect(Object.keys(result)).toHaveLength(8);
expect(Object.keys(result)).toHaveLength(9);
expect(result).toHaveProperty('userInfo');
expect(result).toHaveProperty('cookies');
expect(result).toHaveProperty('httpHeaders');
expect(result).toHaveProperty('httpHeaders.request');
expect(result).toHaveProperty('httpHeaders.response');
expect(result).toHaveProperty('httpBodies');
expect(result).toHaveProperty('queryParams');
expect(result).toHaveProperty('graphQL');
expect(result).toHaveProperty('graphQL.document');
expect(result).toHaveProperty('graphQL.variables');
expect(result).toHaveProperty('genAI');
expect(result).toHaveProperty('genAI.inputs');
expect(result).toHaveProperty('genAI.outputs');
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/lib/utils/request.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -874,6 +874,7 @@ describe('request utils', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type {
Token,
} from './graphql-types';
import type { Span, SpanAttributes } from '@sentry/core';
import { isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { getClient, isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { AllowedOperationTypes, SpanNames, TokenKind } from './enum';
import { AttributeNames } from './enums/AttributeNames';
import { OTEL_GRAPHQL_DATA_SYMBOL, OTEL_PATCHED_SYMBOL } from './symbols';
Expand All@@ -38,8 +38,10 @@ export const isPromise = (value: any): value is Promise<unknown> => {
};

export function addSpanSource(span: Span, loc?: Location, start?: number, end?: number): void {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
if (getClient()?.getDataCollectionOptions().graphQL.document === true) {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
}
}

function createFieldIfNotExists(
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as core from '@sentry/core';
import { addSpanSource } from '../../../../src/integrations/tracing/graphql/vendored/utils';
import type { Location, Token } from '@sentry/server-utils/orchestrion';

vi.spyOn(core, 'getClient');

function mockClient(graphQLDocument: boolean): void {
vi.mocked(core.getClient).mockReturnValue({
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as any);
}

function makeLocation(): Location {
const token: Token = {
kind: 'Name',
start: 0,
end: 5,
line: 1,
column: 1,
value: 'hello',
prev: null,
next: null,
};
return { start: 0, end: 5, startToken: token };
}

describe('addSpanSource dataCollection gate', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('sets graphql.source attribute when graphQL.document is true', () => {
mockClient(true);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).toHaveBeenCalledWith('graphql.source', expect.any(String));
});

it('does not set attribute when graphQL.document is false', () => {
mockClient(false);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});

it('does not set attribute when getClient() is undefined', () => {
vi.mocked(core.getClient).mockReturnValue(undefined);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,7 @@ describe('wrapMiddlewareHandlerWithSentry', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/test/server/errors.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ function createMockClient(httpBodies: string[] = []): Client {
httpHeaders: { request: true, response: true },
httpBodies,
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions packages/astro/test/server/middleware.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -64,6 +64,7 @@ describe('sentryMiddleware', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
11 changes: 6 additions & 5 deletions packages/browser/src/integrations/graphqlClient.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import {
} from '@sentry/core/browser';
import type { FetchHint, XhrHint } from '@sentry/browser-utils';
import { getBodyString, getFetchRequestArgBody, SENTRY_XHR_DATA_KEY } from '@sentry/browser-utils';
import { GRAPHQL_DOCUMENT } from '@sentry/conventions/attributes';

interface GraphQLClientOptions {
endpoints: Array<string | RegExp>;
Expand DownExpand Up@@ -88,9 +89,9 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
const operationInfo = _getGraphQLOperation(graphqlBody);
span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);

// Handle standard requests - always capture the query document
if (isStandardRequest(graphqlBody)) {
span.setAttribute('graphql.document', graphqlBody.query);
// Handle standard requests - capture the query document when enabled via dataCollection (default true)
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
span.setAttribute(GRAPHQL_DOCUMENT, graphqlBody.query);
}

// Handle persisted operations - capture hash for debugging
Expand DownExpand Up@@ -126,8 +127,8 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient

data['graphql.operation'] = operationInfo;

if (isStandardRequest(graphqlBody)) {
data['graphql.document'] = graphqlBody.query;
if (isStandardRequest(graphqlBody) && client.getDataCollectionOptions().graphQL.document === true) {
data[GRAPHQL_DOCUMENT] = graphqlBody.query;
}

if (isPersistedRequest(graphqlBody)) {
Expand Down
26 changes: 25 additions & 1 deletion packages/browser/test/integrations/graphqlClient.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -313,14 +313,18 @@ describe('GraphqlClient', () => {
});

describe('beforeOutgoingRequestSpan handler', () => {
function setupHandler(endpoints: Array<string | RegExp>): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
function setupHandler(
endpoints: Array<string | RegExp>,
graphQLDocument = true,
): (span: SentrySpan, hint: FetchHint | XhrHint) => void {
let capturedListener: ((span: SentrySpan, hint: FetchHint | XhrHint) => void) | undefined;
const mockClient = {
on: (eventName: string, cb: (span: SentrySpan, hint: FetchHint | XhrHint) => void) => {
if (eventName === 'beforeOutgoingRequestSpan') {
capturedListener = cb;
}
},
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as unknown as Client;

const integration = graphqlClientIntegration({ endpoints });
Expand DownExpand Up@@ -420,5 +424,25 @@ describe('GraphqlClient', () => {
expect(json.description).toBe('custom span');
expect(json.data['graphql.document']).toBeUndefined();
});

test('omits graphql.document when dataCollection.graphQL.document is false', () => {
const handler = setupHandler([/\/graphql$/], false);
const span = new SentrySpan({
name: 'POST http://localhost:4000/graphql',
op: 'http.client',
attributes: {
'http.method': 'POST',
'http.url': 'http://localhost:4000/graphql',
url: 'http://localhost:4000/graphql',
},
});

handler(span, makeFetchHint('http://localhost:4000/graphql', requestBody));

const json = spanToJSON(span);
// The span is still renamed with the operation, but the document is not attached.
expect(json.description).toBe('POST http://localhost:4000/graphql (query GetHello)');
expect(json.data['graphql.document']).toBeUndefined();
});
});
});
22 changes: 21 additions & 1 deletion packages/core/src/types/datacollection.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,7 +51,26 @@ export interface DataCollection {
queryParams?: CollectBehavior;

/**
* Controls generative AI input/output recording.
* Allow collection of GraphQL operation data when using the SDK's GraphQL integrations
*
* These options do not add GraphQL instrumentation on their own. The corresponding integration must still be enabled
* for any data to be captured.
*
* - `document`: attach the GraphQL document (query/mutation source text).
* - `variables`: attach the variables passed to GraphQL operations.
* @default { document: true, variables: true }
*/
graphQL?: {
document?: boolean;
variables?: boolean;
};
Comment on lines +63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The graphQL.variables configuration option is defined but never used, making the feature to control GraphQL variable collection non-functional.
Severity: HIGH

Suggested Fix

Implement the logic to read the getDataCollectionOptions().graphQL.variables setting. Based on its value, conditionally collect or suppress the variables field from GraphQL requests. This logic should be added where GraphQL bodies are processed, such as in packages/browser/src/integrations/graphqlClient.ts and packages/server-utils/src/graphql/utils.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/types/datacollection.ts#L59-L62
Potential issue: The `graphQL.variables` configuration option is defined and documented,
but its value is never read in the production codebase. No code checks
`getDataCollectionOptions().graphQL.variables` before collecting GraphQL variables.
Since no variable collection code exists, the setting has no effect. Users who set
`graphQL: { variables: false }` believing they are suppressing variable collection are
not actually having variables collected (as the feature is missing), and users who set
`variables: true` believing variables will be captured find they are not. The API
contract is unfulfilled as the implementation is missing.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +63 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a blocker but something I was discussing with @ericapisani, this creates the expectation that the SDK is going to apply these options to all GraphQL requests which is not accurate.

It would only apply if the user is using the client or the server GraphQL integrations. GraphQL requests outside of the integrations coverage won't be covered by these options which I find weird. I know the gen AI spans is already a precedent, so I don't have strong opinions here.

An alternative I considered is adding the dataCollection option to the integrations themselves rather than a top-level option.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The core idea of the top-level option is to have only one place where all of this is defined in an on/off switch manner (like: this is allowed to be sent, but you also need the integration to be enabled). This makes it easier to see it all at once. According to the spec, it's also possible to add it on integration level, which would overwrite the option set on the root-level.

Maybe we should make this more clear in the JSDoc?

@logaretmlogaretmJul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine, we already have a precedent for it with gen AI stuff so not feeling too strongly about it but it is something that I wanted to bring up is all.

Maybe adding a line or two in the JS doc to explain what "it won't do" will help here.


/**
* Allow generative AI input/output recording when using the SDK's AI integrations.
*
* These options do not add AI instrumentation on their own. The corresponding integration must still be enabled for
* any data to be captured. Integration-level options, when set, take precedence over these
* values.
* @default { inputs: true, outputs: true }
*/
genAI?: {
Expand All@@ -77,5 +96,6 @@ export interface DataCollection {
*/
export type ResolvedDataCollection = Required<DataCollection> & {
httpHeaders: Required<NonNullable<DataCollection['httpHeaders']>>;
graphQL: Required<NonNullable<DataCollection['graphQL']>>;
genAI: Required<NonNullable<DataCollection['genAI']>>;
};
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand All@@ -26,6 +27,9 @@ export function defaultPiiToCollectionOptions(sendDefaultPii?: boolean): Resolve
httpHeaders: { request: { deny: PII_HEADER_SNIPPETS }, response: { deny: PII_HEADER_SNIPPETS } },
httpBodies: [],
queryParams: { deny: PII_HEADER_SNIPPETS },
// The GraphQL document has literal values redacted at collection time, so it was historically
// always attached regardless of `sendDefaultPii`; keep it on to preserve that behavior.
graphQL: { document: true, variables: true },
Comment on lines +30 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: When sendDefaultPii is false, graphQL.variables is incorrectly set to true. This contradicts the goal of minimizing PII collection and sets up a potential future PII leak.
Severity: HIGH

Suggested Fix

Change the default setting for graphQL.variables to false when sendDefaultPii is false in defaultPiiToCollectionOptions.ts. This aligns the behavior with other PII-sensitive settings and prevents a future potential PII leak. The line should be changed to graphQL: { document: true, variables: false }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/core/src/utils/data-collection/defaultPiiToCollectionOptions.ts#L30-L32
Potential issue: The default data collection options set `graphQL.variables` to `true`
even when `sendDefaultPii` is `false`. The purpose of `sendDefaultPii: false` is to
prevent the collection of Personally Identifiable Information (PII). GraphQL variables
can contain sensitive user data, and unlike `graphQL.document`, they are not redacted.
While no code currently consumes the `graphQL.variables` setting, this configuration is
a design error. It creates a high risk that future code implementing GraphQL variable
collection will inadvertently capture and send PII for users who have opted out, as the
default setting will be assumed to be safe.

genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7, // default should be 5, but ContextLines integration uses 7
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ const DEFAULTS: ResolvedDataCollection = {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand DownExpand Up@@ -43,6 +44,10 @@ export function resolveDataCollectionOptions(options: {
},
httpBodies: dc.httpBodies ?? base.httpBodies,
queryParams: dc.queryParams ?? base.queryParams,
graphQL: {
document: dc.graphQL?.document ?? base.graphQL.document,
variables: dc.graphQL?.variables ?? base.graphQL.variables,
},
genAI: {
inputs: dc.genAI?.inputs ?? base.genAI.inputs,
outputs: dc.genAI?.outputs ?? base.genAI.outputs,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ export function createMockClient(userInfo = true, genAI?: { inputs: boolean; out
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: genAIOptions,
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@ describe('defaultPiiToCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -25,6 +26,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -41,6 +43,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand All@@ -57,6 +60,7 @@ describe('defaultPiiToCollectionOptions', () => {
},
httpBodies: [],
queryParams: { deny: ['forwarded', '-ip', 'remote-', 'via', '-user'] },
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 7,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ describe('resolveDataCollectionOptions', () => {
httpHeaders: { request: true, response: true },
httpBodies: ['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse'],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand All@@ -21,6 +22,8 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
// GraphQL documents are redacted at collection time, so they stay on to preserve legacy behavior.
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(7);
});
Expand All@@ -45,6 +48,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.httpBodies).toEqual(['incomingRequest', 'outgoingRequest', 'incomingResponse', 'outgoingResponse']);
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
});

Expand All@@ -54,6 +58,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.userInfo).toBe(false);
expect(result.httpBodies).toEqual([]);
expect(result.genAI).toEqual({ inputs: false, outputs: false });
expect(result.graphQL).toEqual({ document: true, variables: true });
});
});

Expand DownExpand Up@@ -87,6 +92,7 @@ describe('resolveDataCollectionOptions', () => {
expect(result.cookies).toBe(true);
expect(result.httpHeaders).toEqual({ request: true, response: true });
expect(result.queryParams).toBe(true);
expect(result.graphQL).toEqual({ document: true, variables: true });
expect(result.genAI).toEqual({ inputs: true, outputs: true });
expect(result.stackFrameVariables).toBe(true);
expect(result.frameContextLines).toBe(5);
Expand DownExpand Up@@ -114,6 +120,17 @@ describe('resolveDataCollectionOptions', () => {
expect(result.genAI.outputs).toBe(true);
});

it('merges nested graphQL partially', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
graphQL: { document: false },
},
});

expect(result.graphQL.document).toBe(false);
expect(result.graphQL.variables).toBe(true);
});

it('supports allow/deny list for cookies', () => {
const result = resolveDataCollectionOptions({
dataCollection: {
Expand All@@ -139,14 +156,17 @@ describe('resolveDataCollectionOptions', () => {
it('always returns all fields', () => {
const result = resolveDataCollectionOptions({});

expect(Object.keys(result)).toHaveLength(8);
expect(Object.keys(result)).toHaveLength(9);
expect(result).toHaveProperty('userInfo');
expect(result).toHaveProperty('cookies');
expect(result).toHaveProperty('httpHeaders');
expect(result).toHaveProperty('httpHeaders.request');
expect(result).toHaveProperty('httpHeaders.response');
expect(result).toHaveProperty('httpBodies');
expect(result).toHaveProperty('queryParams');
expect(result).toHaveProperty('graphQL');
expect(result).toHaveProperty('graphQL.document');
expect(result).toHaveProperty('graphQL.variables');
expect(result).toHaveProperty('genAI');
expect(result).toHaveProperty('genAI.inputs');
expect(result).toHaveProperty('genAI.outputs');
Expand Down
1 change: 1 addition & 0 deletions packages/core/test/lib/utils/request.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -874,6 +874,7 @@ describe('request utils', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type {
Token,
} from './graphql-types';
import type { Span, SpanAttributes } from '@sentry/core';
import { isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { getClient, isObjectLike, SPAN_STATUS_ERROR, startInactiveSpan, withActiveSpan } from '@sentry/core';
import { AllowedOperationTypes, SpanNames, TokenKind } from './enum';
import { AttributeNames } from './enums/AttributeNames';
import { OTEL_GRAPHQL_DATA_SYMBOL, OTEL_PATCHED_SYMBOL } from './symbols';
Expand All@@ -38,8 +38,10 @@ export const isPromise = (value: any): value is Promise<unknown> => {
};

export function addSpanSource(span: Span, loc?: Location, start?: number, end?: number): void {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
if (getClient()?.getDataCollectionOptions().graphQL.document === true) {
const source = getSourceFromLocation(loc, start, end);
span.setAttribute(AttributeNames.SOURCE, source);
}
}

function createFieldIfNotExists(
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as core from '@sentry/core';
import { addSpanSource } from '../../../../src/integrations/tracing/graphql/vendored/utils';
import type { Location, Token } from '@sentry/server-utils/orchestrion';

vi.spyOn(core, 'getClient');

function mockClient(graphQLDocument: boolean): void {
vi.mocked(core.getClient).mockReturnValue({
getDataCollectionOptions: () => ({ graphQL: { document: graphQLDocument, variables: true } }),
} as any);
}

function makeLocation(): Location {
const token: Token = {
kind: 'Name',
start: 0,
end: 5,
line: 1,
column: 1,
value: 'hello',
prev: null,
next: null,
};
return { start: 0, end: 5, startToken: token };
}

describe('addSpanSource dataCollection gate', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('sets graphql.source attribute when graphQL.document is true', () => {
mockClient(true);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).toHaveBeenCalledWith('graphql.source', expect.any(String));
});

it('does not set attribute when graphQL.document is false', () => {
mockClient(false);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});

it('does not set attribute when getClient() is undefined', () => {
vi.mocked(core.getClient).mockReturnValue(undefined);
const span = { setAttribute: vi.fn() } as any;
addSpanSource(span, makeLocation());
expect(span.setAttribute).not.toHaveBeenCalled();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,7 @@ describe('wrapMiddlewareHandlerWithSentry', () => {
httpHeaders: { request: true, response: true },
httpBodies: [],
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: false, outputs: false },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
1 change: 1 addition & 0 deletions packages/remix/test/server/errors.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ function createMockClient(httpBodies: string[] = []): Client {
httpHeaders: { request: true, response: true },
httpBodies,
queryParams: true,
graphQL: { document: true, variables: true },
genAI: { inputs: true, outputs: true },
stackFrameVariables: true,
frameContextLines: 5,
Expand Down
Loading
Loading