diff --git a/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.client.config.ts index 1db88970e45e..1d7351a64c24 100644 --- a/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.client.config.ts +++ b/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.client.config.ts @@ -5,7 +5,6 @@ import * as Sentry from '@sentry/nextjs'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', environment: 'qa', // dynamic sampling bias to keep transactions diff --git a/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.edge.config.ts b/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.edge.config.ts index 58972d6c07e8..3686f8dc86bf 100644 --- a/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.edge.config.ts +++ b/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.edge.config.ts @@ -5,7 +5,6 @@ import * as Sentry from '@sentry/nextjs'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', environment: 'qa', // dynamic sampling bias to keep transactions tracesSampleRate: 1, diff --git a/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.server.config.ts b/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.server.config.ts index d5ac22930079..ffd3c36582b2 100644 --- a/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.server.config.ts +++ b/dev-packages/e2e-tests/test-applications/supabase-nextjs/sentry.server.config.ts @@ -5,7 +5,6 @@ import * as Sentry from '@sentry/nextjs'; Sentry.init({ - traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', environment: 'qa', // dynamic sampling bias to keep transactions tracesSampleRate: 1, diff --git a/dev-packages/e2e-tests/test-applications/supabase-nextjs/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/supabase-nextjs/tests/performance.test.ts index c0357a515b02..6fe1a285f77b 100644 --- a/dev-packages/e2e-tests/test-applications/supabase-nextjs/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/supabase-nextjs/tests/performance.test.ts @@ -1,45 +1,78 @@ import { expect, test } from '@playwright/test'; -import { waitForTransaction } from '@sentry-internal/test-utils'; +import { collectStreamedSpans, getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils'; + +type StreamedSpan = Awaited>; // This test should be run in serial mode to ensure that the test user is created before the other tests test.describe.configure({ mode: 'serial' }); +const DB_ATTRIBUTES = { + 'db.system.name': { value: 'postgresql', type: 'string' }, + 'sentry.op': { value: 'db', type: 'string' }, + 'sentry.origin': { value: 'auto.db.supabase', type: 'string' }, +}; + +function collectSpansUntilSegment(segmentName: string): Promise { + return collectStreamedSpans('supabase-nextjs', spans => + spans.some(span => span.name === segmentName && span.is_segment), + ); +} + +function expectDbSpan( + span: StreamedSpan | undefined, + name: string, + attributes: Record, +): asserts span is StreamedSpan { + expect(span).toEqual( + expect.objectContaining({ + name, + status: 'ok', + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + end_timestamp: expect.any(Number), + }), + ); + expect(getSpanOp(span!)).toBe('db'); + expect(span!.attributes).toMatchObject({ ...DB_ATTRIBUTES, ...attributes }); +} + // This should be the first test as it will be needed for the other tests -test('Sends server-side Supabase auth admin `createUser` span', async ({ page, baseURL }) => { - const httpTransactionPromise = waitForTransaction('supabase-nextjs', transactionEvent => { - return ( - transactionEvent?.contexts?.trace?.op === 'http.server' && - transactionEvent?.transaction === 'GET /api/create-test-user' - ); - }); +test('Sends server-side Supabase auth admin `createUser` span', async ({ baseURL }) => { + const spansPromise = collectSpansUntilSegment('GET /api/create-test-user'); await fetch(`${baseURL}/api/create-test-user`); - const transactionEvent = await httpTransactionPromise; - - expect(transactionEvent.spans).toContainEqual({ - data: expect.objectContaining({ - 'db.operation.name': 'auth.admin.createUser', - 'db.system.name': 'postgresql', - 'sentry.op': 'db', - 'sentry.origin': 'auto.db.supabase', - }), - description: 'auth (admin) createUser', - op: 'db', - parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - status: 'ok', - timestamp: expect.any(Number), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.db.supabase', + const spans = await spansPromise; + + const rootSpan = spans.find(span => span.name === 'GET /api/create-test-user' && span.is_segment)!; + const createUserSpan = spans.find(span => span.name === 'auth.admin.createUser'); + + expectDbSpan(createUserSpan, 'auth.admin.createUser', { + 'db.operation.name': { value: 'auth.admin.createUser', type: 'string' }, }); + expect(createUserSpan.is_segment).toBe(false); + expect(createUserSpan.trace_id).toBe(rootSpan.trace_id); + expect(createUserSpan.parent_span_id).toEqual(expect.stringMatching(/[a-f0-9]{16}/)); }); -test('Sends client-side Supabase db-operation spans and breadcrumbs to Sentry', async ({ page, baseURL }) => { - const pageloadTransactionPromise = waitForTransaction('supabase-nextjs', transactionEvent => { - return transactionEvent?.contexts?.trace?.op === 'pageload' && transactionEvent?.transaction === '/'; +test('Sends client-side Supabase db-operation spans to Sentry', async ({ page }) => { + const pageloadSpanPromise = waitForStreamedSpan('supabase-nextjs', span => { + return span.name === '/' && getSpanOp(span) === 'pageload' && span.is_segment; + }); + + // The `order` filter only exists on the client-side select, which keeps it distinguishable from the + // server-side selects now that the span name no longer carries the filters. + const selectSpanPromise = waitForStreamedSpan('supabase-nextjs', span => { + const query = span.attributes['db.query']; + return ( + span.name === 'select todos' && + query?.type === 'array' && + (query.value as unknown[]).includes('filter(order, asc)') + ); }); + const insertSpanPromise = waitForStreamedSpan('supabase-nextjs', span => span.name === 'insert todos'); + await page.goto('/'); // Fill in login credentials @@ -55,149 +88,64 @@ test('Sends client-side Supabase db-operation spans and breadcrumbs to Sentry', await page.locator('input[id=new-task-text]').fill('test'); await page.locator('button[id=add-task]').click(); - const transactionEvent = await pageloadTransactionPromise; + const [pageloadSpan, selectSpan, insertSpan] = await Promise.all([ + pageloadSpanPromise, + selectSpanPromise, + insertSpanPromise, + ]); // Client-side database query data is collected by default. - const selectSpanExpectation = expect.objectContaining({ - description: 'select(*) filter(order, asc) from(todos)', - op: 'db', - data: expect.objectContaining({ - 'db.operation.name': 'select', - 'db.system.name': 'postgresql', - 'sentry.op': 'db', - 'sentry.origin': 'auto.db.supabase', - }), - parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - status: 'ok', - timestamp: expect.any(Number), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.db.supabase', - }); - - expect(transactionEvent.spans).toContainEqual(selectSpanExpectation); - - const selectSpan = transactionEvent.spans?.find( - (s: { description?: string }) => s.description === 'select(*) filter(order, asc) from(todos)', - ); - expect(selectSpan).toBeDefined(); - expect(selectSpan!.data?.['db.query']).toEqual(['select(*)', 'filter(order, asc)']); - - expect(transactionEvent.breadcrumbs).toContainEqual({ - timestamp: expect.any(Number), - type: 'supabase', - category: 'db.select', - message: 'select(*) filter(order, asc) from(todos)', - data: expect.objectContaining({ - query: ['select(*)', 'filter(order, asc)'], - }), + expectDbSpan(selectSpan, 'select todos', { + 'db.operation.name': { value: 'select', type: 'string' }, + 'db.query': { value: ['select(*)', 'filter(order, asc)'], type: 'array' }, }); + expect(selectSpan.trace_id).toBe(pageloadSpan.trace_id); - expect(transactionEvent.breadcrumbs).toContainEqual({ - timestamp: expect.any(Number), - type: 'supabase', - category: 'db.insert', - message: 'insert(...) select(*) from(todos)', - data: expect.objectContaining({ - query: ['select(*)'], - }), + // The insert is triggered long after the pageload span has ended, so it is streamed on its own + // rather than as a child of the pageload span. + expectDbSpan(insertSpan, 'insert todos', { + 'db.operation.name': { value: 'insert', type: 'string' }, + 'db.query': { value: ['select(*)'], type: 'array' }, }); }); -test('Sends server-side Supabase db-operation spans and breadcrumbs to Sentry', async ({ page, baseURL }) => { - const httpTransactionPromise = waitForTransaction('supabase-nextjs', transactionEvent => { - return ( - transactionEvent?.contexts?.trace?.op === 'http.server' && - transactionEvent?.transaction === 'GET /api/add-todo-entry' - ); - }); +test('Sends server-side Supabase db-operation spans to Sentry', async ({ baseURL }) => { + const spansPromise = collectSpansUntilSegment('GET /api/add-todo-entry'); await fetch(`${baseURL}/api/add-todo-entry`); - const transactionEvent = await httpTransactionPromise; - - expect(transactionEvent.spans).toContainEqual( - expect.objectContaining({ - data: expect.objectContaining({ - 'db.operation.name': 'insert', - 'db.query': ['select(*)'], - 'db.system.name': 'postgresql', - 'sentry.op': 'db', - 'sentry.origin': 'auto.db.supabase', - }), - description: 'insert(...) select(*) from(todos)', - op: 'db', - parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - status: 'ok', - timestamp: expect.any(Number), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.db.supabase', - }), - ); + const spans = await spansPromise; - expect(transactionEvent.spans).toContainEqual({ - data: expect.objectContaining({ - 'db.operation.name': 'select', - 'db.query': ['select(*)'], - 'db.system.name': 'postgresql', - 'sentry.op': 'db', - 'sentry.origin': 'auto.db.supabase', - }), - description: 'select(*) from(todos)', - op: 'db', - parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - status: 'ok', - timestamp: expect.any(Number), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.db.supabase', - }); + const rootSpan = spans.find(span => span.name === 'GET /api/add-todo-entry' && span.is_segment)!; + const insertSpan = spans.find(span => span.name === 'insert todos'); + const selectSpan = spans.find(span => span.name === 'select todos'); - expect(transactionEvent.breadcrumbs).toContainEqual({ - timestamp: expect.any(Number), - type: 'supabase', - category: 'db.select', - message: 'select(*) from(todos)', - data: expect.any(Object), + expectDbSpan(insertSpan, 'insert todos', { + 'db.operation.name': { value: 'insert', type: 'string' }, + 'db.query': { value: ['select(*)'], type: 'array' }, }); + expect(insertSpan.is_segment).toBe(false); + expect(insertSpan.trace_id).toBe(rootSpan.trace_id); - expect(transactionEvent.breadcrumbs).toContainEqual({ - timestamp: expect.any(Number), - type: 'supabase', - category: 'db.insert', - message: 'insert(...) select(*) from(todos)', - data: expect.any(Object), + expectDbSpan(selectSpan, 'select todos', { + 'db.operation.name': { value: 'select', type: 'string' }, + 'db.query': { value: ['select(*)'], type: 'array' }, }); + expect(selectSpan.is_segment).toBe(false); + expect(selectSpan.trace_id).toBe(rootSpan.trace_id); }); -test('Sends server-side Supabase auth admin `listUsers` span', async ({ page, baseURL }) => { - const httpTransactionPromise = waitForTransaction('supabase-nextjs', transactionEvent => { - return ( - transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /api/list-users' - ); - }); +test('Sends server-side Supabase auth admin `listUsers` span', async ({ baseURL }) => { + const spansPromise = collectSpansUntilSegment('GET /api/list-users'); await fetch(`${baseURL}/api/list-users`); - const transactionEvent = await httpTransactionPromise; - - expect(transactionEvent.spans).toContainEqual({ - data: expect.objectContaining({ - 'db.operation.name': 'auth.admin.listUsers', - 'db.system.name': 'postgresql', - 'sentry.op': 'db', - 'sentry.origin': 'auto.db.supabase', - }), - description: 'auth (admin) listUsers', - op: 'db', - parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - status: 'ok', - timestamp: expect.any(Number), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.db.supabase', + const spans = await spansPromise; + + const rootSpan = spans.find(span => span.name === 'GET /api/list-users' && span.is_segment)!; + const listUsersSpan = spans.find(span => span.name === 'auth.admin.listUsers'); + + expectDbSpan(listUsersSpan, 'auth.admin.listUsers', { + 'db.operation.name': { value: 'auth.admin.listUsers', type: 'string' }, }); + expect(listUsersSpan.is_segment).toBe(false); + expect(listUsersSpan.trace_id).toBe(rootSpan.trace_id); }); diff --git a/packages/core/src/integrations/supabase.ts b/packages/core/src/integrations/supabase.ts index 1f405ed4b098..c0ef66a2650d 100644 --- a/packages/core/src/integrations/supabase.ts +++ b/packages/core/src/integrations/supabase.ts @@ -11,6 +11,7 @@ import { captureException } from '../exports'; import { defineIntegration } from '../integration'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes'; import { setHttpStatus, SPAN_STATUS_ERROR, SPAN_STATUS_OK } from '../tracing'; +import { hasSpanStreamingEnabled } from '../tracing/spans/hasSpanStreamingEnabled'; import { startSpan } from '../tracing/trace'; import type { IntegrationFn } from '../types/integration'; import type { WebFetchHeaders } from '../types/webfetchapi'; @@ -272,14 +273,31 @@ export function translateFiltersIntoMethods(key: string, query: string): string function instrumentAuthOperation(operation: AuthOperationFn, isAdmin = false): AuthOperationFn { return new Proxy(operation, { apply(target, thisArg, argumentsList) { + const operationName = `auth${isAdmin ? '.admin' : ''}.${operation.name}`; + + const client = getClient(); + const name = + client && hasSpanStreamingEnabled(client) + ? // Usually, the operation name alone is not a valid span name according to conventions. + // However, for this span, we neither have a table, nor a namespace, since this is a Supabase-SDK + // operation that internally makes the respective request to the database. + // So I think we can interpret this as a "db.query.summary"-esque span name. + // Either way, it's definitely low-cardinality. + // see: https://getsentry.github.io/sentry-conventions/names/#db-queries + operationName + : // This name makes little sense semantically but preserving it for now to + // avoid a breaking change in the transaction path. Will be removed once we remove + // transactions. + `auth ${isAdmin ? '(admin) ' : ''}${operation.name}`; + return startSpan( { - name: `auth ${isAdmin ? '(admin) ' : ''}${operation.name}`, + name, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.db.supabase', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'db', [DB_SYSTEM_NAME]: 'postgresql', - [DB_OPERATION_NAME]: `auth.${isAdmin ? 'admin.' : ''}${operation.name}`, + [DB_OPERATION_NAME]: operationName, }, }, span => { @@ -433,6 +451,9 @@ function instrumentPostgRESTFilterBuilder( const descriptionMiddle = [mutationPart.trimEnd(), queryPart].filter(Boolean).join(' '); const description = descriptionMiddle ? `${descriptionMiddle} from(${table})` : `from(${table})`; + const name = + client && hasSpanStreamingEnabled(client) ? `${operation}${table ? ` ${table}` : ''}` : description; + const attributes: Record = { 'db.table': table, 'db.schema': typedThis.schema, @@ -454,7 +475,7 @@ function instrumentPostgRESTFilterBuilder( return startSpan( { - name: description, + name, attributes, }, span => { diff --git a/packages/core/test/lib/integrations/supabase.test.ts b/packages/core/test/lib/integrations/supabase.test.ts index d9a47452e282..175e6caee6d8 100644 --- a/packages/core/test/lib/integrations/supabase.test.ts +++ b/packages/core/test/lib/integrations/supabase.test.ts @@ -51,6 +51,8 @@ type CreateMockSupabaseClientOptions = { headers?: PostgRESTHeaders; /** When set, configures the mocked Sentry client's `dataCollection.databaseQueryData`. Omit to leave `getClient` to the test file `beforeEach`. */ dataCollectionDatabaseQueryData?: boolean; + /** Defaults to `'static'`, so span names keep the full description. */ + traceLifecycle?: 'static' | 'stream'; }; const DEFAULT_MOCK_SUPABASE_REST_URL = 'https://example.supabase.co/rest/v1/todos'; @@ -66,6 +68,7 @@ function createMockSupabaseClient(resolveWith: unknown, options?: CreateMockSupa if (options?.dataCollectionDatabaseQueryData !== undefined) { currentScopesMocks.getClient.mockReturnValue({ getDataCollectionOptions: () => ({ databaseQueryData: options.dataCollectionDatabaseQueryData }), + getOptions: () => ({ traceLifecycle: options.traceLifecycle ?? 'static' }), } as any); } @@ -371,6 +374,7 @@ describe('Supabase Integration', () => { const resolved = resolveDataCollectionOptions({}); currentScopesMocks.getClient.mockReturnValue({ getDataCollectionOptions: () => resolved, + getOptions: () => ({ traceLifecycle: 'static' }), } as any); const client = createMockSupabaseClient({ status: 200 }, { ...MOCK_SUPABASE_PII_SCENARIO }); @@ -391,10 +395,36 @@ describe('Supabase Integration', () => { ); }); + it('names the span from the conventions instead of the description with span streaming enabled', async () => { + const resolved = resolveDataCollectionOptions({ dataCollection: { databaseQueryData: true } }); + currentScopesMocks.getClient.mockReturnValue({ + getDataCollectionOptions: () => resolved, + getOptions: () => ({ traceLifecycle: 'stream' }), + } as any); + + const client = createMockSupabaseClient({ status: 200 }, { ...MOCK_SUPABASE_PII_SCENARIO }); + instrumentSupabaseClient(client); + + await (client as any).from('users').update({}).then(); + + const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as { + name: string; + attributes: Record; + }; + // `{db.operation.name} {db.collection.name}` — the description, which carries the filters, is + // not used as the name. + expect(spanOptions.name).toBe('update users'); + // the filters are still reported, just not as the name + expect(spanOptions.attributes['db.query']).toEqual( + expect.arrayContaining([expect.stringContaining('secret@example.com')]), + ); + }); + it('redacts data when dataCollection.databaseQueryData is false', async () => { const resolved = resolveDataCollectionOptions({ dataCollection: { databaseQueryData: false } }); currentScopesMocks.getClient.mockReturnValue({ getDataCollectionOptions: () => resolved, + getOptions: () => ({ traceLifecycle: 'static' }), } as any); const client = createMockSupabaseClient({ status: 200 }, { ...MOCK_SUPABASE_PII_SCENARIO });