diff --git a/dev-packages/e2e-tests/package.json b/dev-packages/e2e-tests/package.json index 16fd64cdb58c..965637b71600 100644 --- a/dev-packages/e2e-tests/package.json +++ b/dev-packages/e2e-tests/package.json @@ -14,7 +14,7 @@ "test:prepare": "ts-node prepare.ts", "test:validate": "run-s test:validate-configuration test:validate-test-app-setups", "clean": "rimraf tmp node_modules pnpm-lock.yaml && yarn clean:test-applications", - "clean:test-applications": "rimraf test-applications/**/{node_modules,dist,build,.next,.sveltekit,pnpm-lock.yaml} .last-run.json" + "clean:test-applications": "rimraf test-applications/**/{node_modules,dist,build,.next,.sveltekit,pnpm-lock.yaml} .last-run.json && pnpm store prune" }, "devDependencies": { "@types/glob": "8.0.0", diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/globals.d.ts b/dev-packages/e2e-tests/test-applications/create-next-app/globals.d.ts index 109dbcd55648..e69de29bb2d1 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/globals.d.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/globals.d.ts @@ -1,4 +0,0 @@ -interface Window { - recordedTransactions?: string[]; - capturedExceptionId?: string; -} diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/instrumentation.ts b/dev-packages/e2e-tests/test-applications/create-next-app/instrumentation.ts index 5ddf6e7b823a..dd214cc7a7bc 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/instrumentation.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/instrumentation.ts @@ -1,11 +1,5 @@ import * as Sentry from '@sentry/nextjs'; -declare global { - namespace globalThis { - var transactionIds: string[]; - } -} - export function register() { if (process.env.NEXT_RUNTIME === 'nodejs') { Sentry.init({ @@ -13,21 +7,7 @@ export function register() { dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, // Adjust this value in production, or use tracesSampler for greater control tracesSampleRate: 1.0, - integrations: [Sentry.localVariablesIntegration()], - }); - - Sentry.addEventProcessor(event => { - global.transactionIds = global.transactionIds || []; - - if (event.type === 'transaction') { - const eventId = event.event_id; - - if (eventId) { - global.transactionIds.push(eventId); - } - } - - return event; + tunnel: 'http://localhost:3031', }); } } diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/package.json b/dev-packages/e2e-tests/test-applications/create-next-app/package.json index 330e8b4097ee..2c0945051ee5 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/package.json +++ b/dev-packages/e2e-tests/test-applications/create-next-app/package.json @@ -4,7 +4,7 @@ "private": true, "scripts": { "build": "next build", - "clean": "npx rimraf node_modules pnpm-lock.yaml", + "clean": "npx rimraf node_modules pnpm-lock.yaml .next", "test:prod": "TEST_ENV=prod playwright test", "test:dev": "TEST_ENV=dev playwright test", "test:build": "pnpm install && npx playwright install && pnpm build", @@ -23,7 +23,8 @@ "typescript": "4.9.5" }, "devDependencies": { - "@playwright/test": "^1.44.1" + "@playwright/test": "^1.44.1", + "@sentry-internal/test-utils": "link:../../../test-utils" }, "volta": { "extends": "../../package.json" diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/error.ts b/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/error.ts index 5440074c39aa..6debfd151870 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/error.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/error.ts @@ -1,11 +1,6 @@ -import * as Sentry from '@sentry/nextjs'; // Next.js API route support: https://nextjs.org/docs/api-routes/introduction import type { NextApiRequest, NextApiResponse } from 'next'; export default async function handler(req: NextApiRequest, res: NextApiResponse) { - const exceptionId = Sentry.captureException(new Error('This is an error')); - - await Sentry.flush(2000); - - res.status(200).json({ exceptionId }); + throw new Error('I am a server error!'); } diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts b/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts index 94f7b003ffcb..d3504dc73d98 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts @@ -3,11 +3,9 @@ import * as Sentry from '@sentry/nextjs'; import type { NextApiRequest, NextApiResponse } from 'next'; export default function handler(req: NextApiRequest, res: NextApiResponse) { - Sentry.startSpan({ name: 'test-span' }, span => undefined); + Sentry.startSpan({ name: 'test-span' }, () => undefined); Sentry.flush().then(() => { - res.status(200).json({ - transactionIds: global.transactionIds, - }); + res.status(200).json({}); }); } diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/pages/index.tsx b/dev-packages/e2e-tests/test-applications/create-next-app/pages/index.tsx index 059230d236a5..6bb62f18deb4 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/pages/index.tsx +++ b/dev-packages/e2e-tests/test-applications/create-next-app/pages/index.tsx @@ -1,4 +1,3 @@ -import * as Sentry from '@sentry/nextjs'; import Head from 'next/head'; import Link from 'next/link'; @@ -17,8 +16,7 @@ export default function Home() { value="Capture Exception" id="exception-button" onClick={() => { - const eventId = Sentry.captureException(new Error('I am an error!')); - window.capturedExceptionId = eventId; + throw new Error('I am an error!'); }} /> diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/create-next-app/playwright.config.mjs new file mode 100644 index 000000000000..f97facbf0cc5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-next-app/playwright.config.mjs @@ -0,0 +1,13 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const testEnv = process.env.TEST_ENV; + +if (!testEnv) { + throw new Error('No test env defined'); +} + +const config = getPlaywrightConfig({ + startCommand: testEnv === 'development' ? `pnpm next dev -p 3030` : `pnpm next start -p 3030`, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/playwright.config.ts b/dev-packages/e2e-tests/test-applications/create-next-app/playwright.config.ts deleted file mode 100644 index b29068c3141c..000000000000 --- a/dev-packages/e2e-tests/test-applications/create-next-app/playwright.config.ts +++ /dev/null @@ -1,75 +0,0 @@ -import type { PlaywrightTestConfig } from '@playwright/test'; -import { devices } from '@playwright/test'; - -const testEnv = process.env.TEST_ENV; - -if (!testEnv) { - throw new Error('No test env defined'); -} - -const port = 3030; - -/** - * See https://playwright.dev/docs/test-configuration. - */ -const config: PlaywrightTestConfig = { - testDir: './tests', - /* Maximum time one test can run for. */ - timeout: 150_000, - expect: { - /** - * Maximum time expect() should wait for the condition to be met. - * For example in `await expect(locator).toHaveText();` - */ - timeout: 5000, - }, - /* Run tests in files in parallel */ - fullyParallel: true, - /* Fail the build on CI if you accidentally left test.only in the source code. */ - forbidOnly: !!process.env.CI, - /* Retry on CI only */ - retries: 0, - /* Opt out of parallel tests on CI. */ - workers: 1, - /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'list', - /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ - use: { - /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ - actionTimeout: 0, - - /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ - trace: 'on-first-retry', - }, - - /* Configure projects for major browsers */ - projects: [ - { - name: 'chromium', - use: { - ...devices['Desktop Chrome'], - }, - }, - // For now we only test Chrome! - // { - // name: 'firefox', - // use: { - // ...devices['Desktop Firefox'], - // }, - // }, - // { - // name: 'webkit', - // use: { - // ...devices['Desktop Safari'], - // }, - // }, - ], - - /* Run your local dev server before starting the tests */ - webServer: { - command: testEnv === 'development' ? `pnpm next dev -p ${port}` : `pnpm next start -p ${port}`, - port, - }, -}; - -export default config; diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/create-next-app/sentry.client.config.ts index 1318bf32e5ac..e24170711a83 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/sentry.client.config.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/sentry.client.config.ts @@ -7,26 +7,6 @@ import * as Sentry from '@sentry/nextjs'; Sentry.init({ environment: 'qa', // dynamic sampling bias to keep transactions dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, - // Adjust this value in production, or use tracesSampler for greater control tracesSampleRate: 1.0, - - // ... - // Note: if you want to override the automatic release value, do not set a - // `release` value here - use the environment variable `SENTRY_RELEASE`, so - // that it will also get attached to your source maps -}); - -Sentry.addEventProcessor(event => { - if ( - event.type === 'transaction' && - (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') - ) { - const eventId = event.event_id; - if (eventId) { - window.recordedTransactions = window.recordedTransactions || []; - window.recordedTransactions.push(eventId); - } - } - - return event; + tunnel: 'http://localhost:3031', }); diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/create-next-app/start-event-proxy.mjs new file mode 100644 index 000000000000..db6c74e4afe3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-next-app/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'create-next-app', +}); diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-client.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-client.test.ts deleted file mode 100644 index 23d1d7126e16..000000000000 --- a/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-client.test.ts +++ /dev/null @@ -1,140 +0,0 @@ -import { expect, test } from '@playwright/test'; - -const authToken = process.env.E2E_TEST_AUTH_TOKEN; -const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; -const sentryTestProject = process.env.E2E_TEST_SENTRY_PROJECT; -const EVENT_POLLING_TIMEOUT = 90_000; - -test('Sends a client-side exception to Sentry', async ({ page }) => { - await page.goto('/'); - - const exceptionButton = page.locator('id=exception-button'); - await exceptionButton.click(); - - const exceptionIdHandle = await page.waitForFunction(() => window.capturedExceptionId); - const exceptionEventId = await exceptionIdHandle.jsonValue(); - - console.log(`Polling for error eventId: ${exceptionEventId}`); - - await expect - .poll( - async () => { - const response = await fetch( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${exceptionEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); - - return response.status; - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); -}); - -test('Sends a pageload transaction to Sentry', async ({ page }) => { - await page.goto('/'); - - const recordedTransactionsHandle = await page.waitForFunction(() => { - if (window.recordedTransactions && window.recordedTransactions?.length >= 1) { - return window.recordedTransactions; - } else { - return undefined; - } - }); - const recordedTransactionEventIds = await recordedTransactionsHandle.jsonValue(); - - if (recordedTransactionEventIds === undefined) { - throw new Error("Application didn't record any transaction event IDs."); - } - - let hadPageLoadTransaction = false; - - console.log(`Polling for transaction eventIds: ${JSON.stringify(recordedTransactionEventIds)}`); - - await Promise.all( - recordedTransactionEventIds.map(async transactionEventId => { - await expect - .poll( - async () => { - const response = await fetch( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${transactionEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); - - if (response.ok) { - const data = await response.json(); - if (data.contexts.trace.op === 'pageload') { - hadPageLoadTransaction = true; - } - } - - return response.status; - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); - }), - ); - - expect(hadPageLoadTransaction).toBe(true); -}); - -test('Sends a navigation transaction to Sentry', async ({ page }) => { - await page.goto('/'); - - // Give pageload transaction time to finish - await page.waitForTimeout(4000); - - const linkElement = page.locator('id=navigation'); - await linkElement.click(); - - const recordedTransactionsHandle = await page.waitForFunction(() => { - if (window.recordedTransactions && window.recordedTransactions?.length >= 2) { - return window.recordedTransactions; - } else { - return undefined; - } - }); - const recordedTransactionEventIds = await recordedTransactionsHandle.jsonValue(); - - if (recordedTransactionEventIds === undefined) { - throw new Error("Application didn't record any transaction event IDs."); - } - - let hadPageNavigationTransaction = false; - - console.log(`Polling for transaction eventIds: ${JSON.stringify(recordedTransactionEventIds)}`); - - await Promise.all( - recordedTransactionEventIds.map(async transactionEventId => { - await expect - .poll( - async () => { - const response = await fetch( - `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${transactionEventId}/`, - { headers: { Authorization: `Bearer ${authToken}` } }, - ); - - if (response.ok) { - const data = await response.json(); - if (data.contexts.trace.op === 'navigation') { - hadPageNavigationTransaction = true; - } - } - - return response.status; - }, - { - timeout: EVENT_POLLING_TIMEOUT, - }, - ) - .toBe(200); - }), - ); - - expect(hadPageNavigationTransaction).toBe(true); -}); diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts deleted file mode 100644 index ef30d70c0a52..000000000000 --- a/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { expect, test } from '@playwright/test'; - -const authToken = process.env.E2E_TEST_AUTH_TOKEN; -const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; -const sentryTestProject = process.env.E2E_TEST_SENTRY_PROJECT; -const EVENT_POLLING_TIMEOUT = 90_000; - -test('Sends a server-side exception to Sentry', async ({ baseURL }) => { - const response = await fetch(`${baseURL}/api/error`); - const data = await response.json(); - const { exceptionId } = data; - - const url = `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${exceptionId}/`; - - console.log(`Polling for error eventId: ${exceptionId}`); - - await expect - .poll( - async () => { - const response = await fetch(url, { headers: { Authorization: `Bearer ${authToken}` } }); - return response.status; - }, - { timeout: EVENT_POLLING_TIMEOUT }, - ) - .toBe(200); -}); - -test('Sends server-side transactions to Sentry', async ({ baseURL }) => { - const response = await fetch(`${baseURL}/api/success`); - const data = await response.json(); - const { transactionIds } = data; - - console.log(`Polling for transaction eventIds: ${JSON.stringify(transactionIds)}`); - - await Promise.all( - transactionIds.map(async (transactionId: string) => { - const url = `https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${transactionId}/`; - - await expect - .poll( - async () => { - const response = await fetch(url, { headers: { Authorization: `Bearer ${authToken}` } }); - return response.status; - }, - { timeout: EVENT_POLLING_TIMEOUT }, - ) - .toBe(200); - }), - ); -}); diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-errors.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-errors.test.ts new file mode 100644 index 000000000000..5e06086f1a29 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-errors.test.ts @@ -0,0 +1,30 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test('Sends a client-side exception to Sentry', async ({ page }) => { + const errorEventPromise = waitForError('create-next-app', event => { + return event.exception?.values?.[0]?.value === 'I am an error!'; + }); + + await page.goto('/'); + + const exceptionButton = page.locator('id=exception-button'); + await exceptionButton.click(); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('I am an error!'); + + expect(errorEvent.request).toEqual({ + headers: expect.any(Object), + url: 'http://localhost:3030/', + }); + + expect(errorEvent.transaction).toEqual('/'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts new file mode 100644 index 000000000000..7af02f35fa47 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/client-transactions.test.ts @@ -0,0 +1,85 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test('Sends a pageload transaction to Sentry', async ({ page }) => { + const pageloadTransactionEventPromise = waitForTransaction('create-next-app', transactionEvent => { + return transactionEvent.contexts?.trace?.op === 'pageload' && transactionEvent.transaction === '/'; + }); + + await page.goto('/'); + + const transactionEvent = await pageloadTransactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + transaction: '/', + tags: { runtime: 'browser' }, + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + op: 'pageload', + origin: 'auto.pageload.nextjs.pages_router_instrumentation', + data: expect.objectContaining({ + 'sentry.idle_span_finish_reason': 'idleTimeout', + 'sentry.op': 'pageload', + 'sentry.origin': 'auto.pageload.nextjs.pages_router_instrumentation', + 'sentry.sample_rate': 1, + 'sentry.source': 'route', + }), + }, + }, + request: { + headers: { + 'User-Agent': expect.any(String), + }, + url: 'http://localhost:3030/', + }, + }), + ); +}); + +test('captures a navigation transcation to Sentry', async ({ page }) => { + const clientNavigationTxnEventPromise = waitForTransaction('create-next-app', txnEvent => { + return txnEvent?.transaction === '/user/[id]'; + }); + + await page.goto('/'); + + // navigation to page + const clickPromise = page.getByText('navigate').click(); + + const [clientTxnEvent, serverTxnEvent, _1] = await Promise.all([clientNavigationTxnEventPromise, clickPromise]); + + expect(clientTxnEvent).toEqual( + expect.objectContaining({ + transaction: '/user/[id]', + tags: { runtime: 'browser' }, + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + op: 'navigation', + origin: 'auto.navigation.nextjs.pages_router_instrumentation', + data: expect.objectContaining({ + 'sentry.idle_span_finish_reason': 'idleTimeout', + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.nextjs.pages_router_instrumentation', + 'sentry.sample_rate': 1, + 'sentry.source': 'route', + }), + }, + }, + request: { + headers: { + 'User-Agent': expect.any(String), + }, + url: 'http://localhost:3030/user/5', + }, + }), + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-errors.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-errors.test.ts new file mode 100644 index 000000000000..ed3263f5eb8b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-errors.test.ts @@ -0,0 +1,31 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test('Sends a server-side exception to Sentry', async ({ baseURL }) => { + const errorEventPromise = waitForError('create-next-app', event => { + return event.exception?.values?.[0]?.value === 'I am a server error!'; + }); + + const response = await fetch(`${baseURL}/api/error`); + + expect(response.status).toBe(500); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('I am a server error!'); + + expect(errorEvent.request).toEqual({ + headers: expect.any(Object), + cookies: {}, + method: 'GET', + url: expect.stringContaining('/api/error'), + }); + + expect(errorEvent.transaction).toEqual('GET /api/error'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-transactions.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-transactions.test.ts new file mode 100644 index 000000000000..01bd22f3cae0 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/server-transactions.test.ts @@ -0,0 +1,69 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +const authToken = process.env.E2E_TEST_AUTH_TOKEN; +const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; +const sentryTestProject = process.env.E2E_TEST_SENTRY_PROJECT; +const EVENT_POLLING_TIMEOUT = 90_000; + +test('Sends server-side transactions to Sentry', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('create-next-app', transactionEvent => { + return ( + transactionEvent.contexts?.trace?.op === 'http.server' && transactionEvent.transaction === 'GET /api/success' + ); + }); + + await fetch(`${baseURL}/api/success`); + + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + transaction: 'GET /api/success', + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: expect.objectContaining({ + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + op: 'http.server', + origin: 'auto.http.nextjs', + data: expect.objectContaining({ + 'http.response.status_code': 200, + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.nextjs', + 'sentry.sample_rate': 1, + 'sentry.source': 'route', + }), + status: 'ok', + }, + runtime: { + name: 'node', + version: expect.any(String), + }, + }), + spans: [ + { + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, + description: 'test-span', + origin: 'manual', + parent_span_id: transactionEvent.contexts?.trace?.span_id, + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: transactionEvent.contexts?.trace?.trace_id, + }, + ], + request: { + headers: expect.any(Object), + method: 'GET', + cookies: {}, + url: expect.stringContaining('/api/success'), + }, + }), + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify/tests/errors.test.ts index eb1a9eb869e0..3ef2bba479db 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify/tests/errors.test.ts @@ -25,6 +25,5 @@ test('Sends correct error event', async ({ baseURL }) => { expect(errorEvent.contexts?.trace).toEqual({ trace_id: expect.any(String), span_id: expect.any(String), - parent_span_id: expect.any(String), }); }); diff --git a/dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts index aca04522f8fc..1fbb1fd42613 100644 --- a/dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts @@ -25,6 +25,5 @@ test('Sends correct error event', async ({ baseURL }) => { expect(errorEvent.contexts?.trace).toEqual({ trace_id: expect.any(String), span_id: expect.any(String), - parent_span_id: expect.any(String), }); }); diff --git a/packages/nextjs/src/common/utils/tracingUtils.ts b/packages/nextjs/src/common/utils/tracingUtils.ts index 0c03bc8f0ec9..b996b6af1877 100644 --- a/packages/nextjs/src/common/utils/tracingUtils.ts +++ b/packages/nextjs/src/common/utils/tracingUtils.ts @@ -1,6 +1,6 @@ -import { Scope, getCurrentScope, withActiveSpan } from '@sentry/core'; +import { Scope, startNewTrace } from '@sentry/core'; import type { PropagationContext } from '@sentry/types'; -import { GLOBAL_OBJ, logger, uuid4 } from '@sentry/utils'; +import { GLOBAL_OBJ, logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; const commonPropagationContextMap = new WeakMap(); @@ -85,11 +85,7 @@ export function escapeNextjsTracing(cb: () => T): T { if (nextjsEscapedAsyncStorage.getStore()) { return cb(); } else { - return withActiveSpan(null, () => { - getCurrentScope().setPropagationContext({ - traceId: uuid4(), - spanId: uuid4().substring(16), - }); + return startNewTrace(() => { return nextjsEscapedAsyncStorage.run(true, () => { return cb(); }); diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts index 5a3417849690..09bca8d23d78 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts @@ -81,8 +81,10 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz // eslint-disable-next-line @typescript-eslint/unbound-method res.end = new Proxy(res.end, { apply(target, thisArg, argArray) { - setHttpStatus(span, res.statusCode); - span.end(); + if (span.isRecording()) { + setHttpStatus(span, res.statusCode); + span.end(); + } vercelWaitUntil(flushSafelyWithTimeout()); target.apply(thisArg, argArray); }, @@ -128,8 +130,10 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz res.statusCode = 500; res.statusMessage = 'Internal Server Error'; - setHttpStatus(span, res.statusCode); - span.end(); + if (span.isRecording()) { + setHttpStatus(span, res.statusCode); + span.end(); + } vercelWaitUntil(flushSafelyWithTimeout()); diff --git a/packages/node/test/integration/scope.test.ts b/packages/node/test/integration/scope.test.ts index 6c08ea902c18..39c2b85e26b5 100644 --- a/packages/node/test/integration/scope.test.ts +++ b/packages/node/test/integration/scope.test.ts @@ -65,8 +65,6 @@ describe('Integration | Scope', () => { trace: { span_id: spanId, trace_id: traceId, - // local span ID from propagation context - ...(enableTracing ? { parent_span_id: expect.any(String) } : undefined), }, }), }), @@ -111,8 +109,6 @@ describe('Integration | Scope', () => { status: 'ok', trace_id: traceId, origin: 'manual', - // local span ID from propagation context - parent_span_id: expect.any(String), }, }), spans: [], @@ -196,8 +192,6 @@ describe('Integration | Scope', () => { ? { span_id: spanId1, trace_id: traceId1, - // local span ID from propagation context - ...(enableTracing ? { parent_span_id: expect.any(String) } : undefined), } : expect.any(Object), }), @@ -222,8 +216,6 @@ describe('Integration | Scope', () => { ? { span_id: spanId2, trace_id: traceId2, - // local span ID from propagation context - ...(enableTracing ? { parent_span_id: expect.any(String) } : undefined), } : expect.any(Object), }), diff --git a/packages/node/test/integration/transactions.test.ts b/packages/node/test/integration/transactions.test.ts index d2a55e129cb9..a690f1187669 100644 --- a/packages/node/test/integration/transactions.test.ts +++ b/packages/node/test/integration/transactions.test.ts @@ -253,8 +253,6 @@ describe('Integration | Transactions', () => { status: 'ok', trace_id: expect.any(String), origin: 'auto.test', - // local span ID from propagation context - parent_span_id: expect.any(String), }, }), spans: [expect.any(Object), expect.any(Object)], @@ -294,8 +292,6 @@ describe('Integration | Transactions', () => { status: 'ok', trace_id: expect.any(String), origin: 'manual', - // local span ID from propagation context - parent_span_id: expect.any(String), }, }), spans: [expect.any(Object), expect.any(Object)], diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index 45dcf811eaa8..05b108583f98 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -1,4 +1,5 @@ import type { Baggage, Context, Span, SpanContext, TextMapGetter, TextMapSetter } from '@opentelemetry/api'; +import { INVALID_TRACEID } from '@opentelemetry/api'; import { context } from '@opentelemetry/api'; import { TraceFlags, propagation, trace } from '@opentelemetry/api'; import { TraceState, W3CBaggagePropagator, isTracingSuppressed } from '@opentelemetry/core'; @@ -45,7 +46,7 @@ export function getPropagationContextFromSpan(span: Span): PropagationContext { const dscString = traceState ? traceState.get(SENTRY_TRACE_STATE_DSC) : undefined; const traceStateDsc = dscString ? baggageHeaderToDynamicSamplingContext(dscString) : undefined; - const parentSpanId = traceState ? traceState.get(SENTRY_TRACE_STATE_PARENT_SPAN_ID) : undefined; + const parentSpanId = traceState ? traceState.get(SENTRY_TRACE_STATE_PARENT_SPAN_ID) || undefined : undefined; const sampled = getSamplingDecision(spanContext); @@ -127,7 +128,7 @@ export class SentryPropagator extends W3CBaggagePropagator { } // We also want to avoid setting the default OTEL trace ID, if we get that for whatever reason - if (traceId && traceId !== '00000000000000000000000000000000') { + if (traceId && traceId !== INVALID_TRACEID) { setter.set(carrier, SENTRY_TRACE_HEADER, generateSentryTraceHeader(traceId, spanId, sampled)); } @@ -196,17 +197,15 @@ export function makeTraceState({ parentSpanId?: string; dsc?: Partial; sampled?: boolean; -}): TraceState | undefined { - if (!parentSpanId && !dsc && sampled !== false) { - return undefined; - } - +}): TraceState { // We store the DSC as OTEL trace state on the span context const dscString = dsc ? dynamicSamplingContextToSentryBaggageHeader(dsc) : undefined; - const traceStateBase = parentSpanId - ? new TraceState().set(SENTRY_TRACE_STATE_PARENT_SPAN_ID, parentSpanId) - : new TraceState(); + // We _always_ set the parent span ID, even if it is empty + // If we'd set this to 'undefined' we could not know if the trace state was set, but there was no parentSpanId, + // vs the trace state was not set at all (in which case we want to do fallback handling) + // If `''`, it should be considered "no parent" + const traceStateBase = new TraceState().set(SENTRY_TRACE_STATE_PARENT_SPAN_ID, parentSpanId || ''); const traceStateWithDsc = dscString ? traceStateBase.set(SENTRY_TRACE_STATE_DSC, dscString) : traceStateBase; diff --git a/packages/opentelemetry/src/setupEventContextTrace.ts b/packages/opentelemetry/src/setupEventContextTrace.ts index ad51d340b7cd..fc3441b06792 100644 --- a/packages/opentelemetry/src/setupEventContextTrace.ts +++ b/packages/opentelemetry/src/setupEventContextTrace.ts @@ -3,6 +3,7 @@ import { dropUndefinedKeys } from '@sentry/utils'; import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; import { getRootSpan } from '@sentry/core'; +import { SENTRY_TRACE_STATE_PARENT_SPAN_ID } from './constants'; import { getActiveSpan } from './utils/getActiveSpan'; import { spanHasParentId } from './utils/spanTypes'; @@ -18,12 +19,22 @@ export function setupEventContextTrace(client: Client): void { const spanContext = span.spanContext(); + // If we have a parent span id from trace state, use that ('' means no parent should be used) + // Else, pick the one from the span + const parentSpanIdFromTraceState = spanContext.traceState?.get(SENTRY_TRACE_STATE_PARENT_SPAN_ID); + const parent_span_id = + typeof parentSpanIdFromTraceState === 'string' + ? parentSpanIdFromTraceState || undefined + : spanHasParentId(span) + ? span.parentSpanId + : undefined; + // If event has already set `trace` context, use that one. event.contexts = { trace: dropUndefinedKeys({ trace_id: spanContext.traceId, span_id: spanContext.spanId, - parent_span_id: spanHasParentId(span) ? span.parentSpanId : undefined, + parent_span_id, }), ...event.contexts, }; diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index aa5f37f550e9..ebd1536ff9b4 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -17,6 +17,7 @@ import { } from '@sentry/core'; import type { SpanJSON, SpanOrigin, TraceContext, TransactionEvent, TransactionSource } from '@sentry/types'; import { dropUndefinedKeys, logger } from '@sentry/utils'; +import { SENTRY_TRACE_STATE_PARENT_SPAN_ID } from './constants'; import { DEBUG_BUILD } from './debug-build'; import { SEMANTIC_ATTRIBUTE_SENTRY_PARENT_IS_REMOTE } from './semanticAttributes'; @@ -201,7 +202,16 @@ function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent { }); const { traceId: trace_id, spanId: span_id } = span.spanContext(); - const parent_span_id = span.parentSpanId; + + const parentSpanIdFromTraceState = span.spanContext().traceState?.get(SENTRY_TRACE_STATE_PARENT_SPAN_ID); + + // If parentSpanIdFromTraceState is defined at all, we want it to take presedence + // In that case, an empty string should be interpreted as "no parent span id", + // even if `span.parentSpanId` is set + // this is the case when we are starting a new trace, where we have a virtual span based on the propagationContext + // We only want to continue the traceId in this case, but ignore the parent span + const parent_span_id = + typeof parentSpanIdFromTraceState === 'string' ? parentSpanIdFromTraceState || undefined : span.parentSpanId; const status = mapStatus(span); diff --git a/packages/opentelemetry/src/trace.ts b/packages/opentelemetry/src/trace.ts index 91fd5832b1fe..d55a7ba04a9f 100644 --- a/packages/opentelemetry/src/trace.ts +++ b/packages/opentelemetry/src/trace.ts @@ -1,7 +1,5 @@ import type { Context, Span, SpanContext, SpanOptions, Tracer } from '@opentelemetry/api'; -import { TraceFlags } from '@opentelemetry/api'; -import { context } from '@opentelemetry/api'; -import { SpanStatusCode, trace } from '@opentelemetry/api'; +import { INVALID_SPANID, SpanStatusCode, TraceFlags, context, trace } from '@opentelemetry/api'; import { suppressTracing } from '@opentelemetry/core'; import { SDK_VERSION, @@ -228,7 +226,7 @@ function getContext(scope: Scope | undefined, forceTransaction: boolean | undefi const traceState = makeTraceState({ dsc, - parentSpanId: spanId, + parentSpanId: spanId !== INVALID_SPANID ? spanId : undefined, sampled, }); diff --git a/packages/opentelemetry/test/integration/scope.test.ts b/packages/opentelemetry/test/integration/scope.test.ts index e6c70fd8c7d4..528c55f2bc33 100644 --- a/packages/opentelemetry/test/integration/scope.test.ts +++ b/packages/opentelemetry/test/integration/scope.test.ts @@ -72,8 +72,6 @@ describe('Integration | Scope', () => { trace: { span_id: spanId, trace_id: traceId, - // local span ID from propagation context - ...(enableTracing ? { parent_span_id: expect.any(String) } : undefined), }, }, }), @@ -118,8 +116,6 @@ describe('Integration | Scope', () => { status: 'ok', trace_id: traceId, origin: 'manual', - // local span ID from propagation context - parent_span_id: expect.any(String), }, }), spans: [], @@ -213,8 +209,6 @@ describe('Integration | Scope', () => { ? { span_id: spanId1, trace_id: traceId1, - // local span ID from propagation context - ...(enableTracing ? { parent_span_id: expect.any(String) } : undefined), } : expect.any(Object), }), @@ -239,8 +233,6 @@ describe('Integration | Scope', () => { ? { span_id: spanId2, trace_id: traceId2, - // local span ID from propagation context - ...(enableTracing ? { parent_span_id: expect.any(String) } : undefined), } : expect.any(Object), }), @@ -342,8 +334,6 @@ describe('Integration | Scope', () => { trace: { span_id: spanId1, trace_id: traceId1, - // local span ID from propagation context - ...(enableTracing ? { parent_span_id: expect.any(String) } : undefined), }, }), tags: { @@ -368,8 +358,6 @@ describe('Integration | Scope', () => { trace: { span_id: spanId2, trace_id: traceId2, - // local span ID from propagation context - ...(enableTracing ? { parent_span_id: expect.any(String) } : undefined), }, }), tags: { diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index 63020fb2edd4..cf4a145775ec 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -16,6 +16,7 @@ import { logger } from '@sentry/utils'; import { TraceState } from '@opentelemetry/core'; import { SENTRY_TRACE_STATE_DSC } from '../../src/constants'; +import { makeTraceState } from '../../src/propagator'; import { SentrySpanProcessor } from '../../src/spanProcessor'; import { startInactiveSpan, startSpan } from '../../src/trace'; import type { TestClientInterface } from '../helpers/TestClient'; @@ -267,8 +268,6 @@ describe('Integration | Transactions', () => { status: 'ok', trace_id: expect.any(String), origin: 'auto.test', - // local span ID from propagation context - parent_span_id: expect.any(String), }, }), spans: [expect.any(Object), expect.any(Object)], @@ -308,8 +307,6 @@ describe('Integration | Transactions', () => { status: 'ok', trace_id: expect.any(String), origin: 'manual', - // local span ID from propagation context - parent_span_id: expect.any(String), }, }), spans: [expect.any(Object), expect.any(Object)], @@ -334,12 +331,18 @@ describe('Integration | Transactions', () => { const traceId = 'd4cda95b652f4a1592b449d5929fda1b'; const parentSpanId = '6e0c63257de34c92'; - const spanContext = { + const traceState = makeTraceState({ + parentSpanId, + dsc: undefined, + sampled: true, + }); + + const spanContext: SpanContext = { traceId, spanId: parentSpanId, - sampled: true, isRemote: true, traceFlags: TraceFlags.SAMPLED, + traceState, }; mockSdkInit({ enableTracing: true, beforeSendTransaction }); diff --git a/packages/opentelemetry/test/propagator.test.ts b/packages/opentelemetry/test/propagator.test.ts index 0d84da98e4f6..d3ee43f4d199 100644 --- a/packages/opentelemetry/test/propagator.test.ts +++ b/packages/opentelemetry/test/propagator.test.ts @@ -616,6 +616,7 @@ describe('SentryPropagator', () => { spanId: expect.any(String), traceFlags: TraceFlags.NONE, traceId: expect.any(String), + traceState: makeTraceState({}), }); expect(getSamplingDecision(trace.getSpanContext(context)!)).toBe(undefined); }); @@ -681,6 +682,7 @@ describe('SentryPropagator', () => { spanId: expect.any(String), traceFlags: TraceFlags.NONE, traceId: expect.any(String), + traceState: makeTraceState({}), }); expect(getSamplingDecision(trace.getSpanContext(context)!)).toBe(undefined); }); @@ -693,6 +695,7 @@ describe('SentryPropagator', () => { spanId: expect.any(String), traceFlags: TraceFlags.NONE, traceId: expect.any(String), + traceState: makeTraceState({}), }); }); }); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index ab662b19db5b..e46ef43f385b 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -960,6 +960,25 @@ describe('trace', () => { }); }); + it('picks up the trace context from the scope, including parentSpanId, if there is no parent', () => { + withScope(scope => { + const propagationContext = scope.getPropagationContext(); + propagationContext.parentSpanId = '1121201211212012'; + const span = startInactiveSpan({ name: 'test span' }); + + expect(span).toBeDefined(); + expect(spanToJSON(span).trace_id).toEqual(propagationContext.traceId); + expect(spanToJSON(span).parent_span_id).toEqual('1121201211212012'); + + expect(getDynamicSamplingContextFromSpan(span)).toEqual({ + ...getDynamicSamplingContextFromClient(propagationContext.traceId, getClient()!), + sample_rate: '1', + sampled: 'true', + transaction: 'test span', + }); + }); + }); + it('picks up the trace context from the parent without DSC', () => { withScope(scope => { const propagationContext = scope.getPropagationContext();