diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch-external-disallowed/check/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch-external-disallowed/check/route.ts new file mode 100644 index 000000000000..e5f497a1bed6 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch-external-disallowed/check/route.ts @@ -0,0 +1,5 @@ +import { checkHandler } from '../../utils'; + +export const dynamic = 'force-dynamic'; + +export const GET = checkHandler; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch-external-disallowed/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch-external-disallowed/route.ts new file mode 100644 index 000000000000..b57d873f3ce7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch-external-disallowed/route.ts @@ -0,0 +1,10 @@ +import { NextResponse } from 'next/server'; + +export const dynamic = 'force-dynamic'; + +export async function GET() { + const data = await fetch(`http://localhost:3030/propagation/test-outgoing-fetch-external-disallowed/check`).then( + res => res.json(), + ); + return NextResponse.json(data); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch/check/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch/check/route.ts new file mode 100644 index 000000000000..e5f497a1bed6 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch/check/route.ts @@ -0,0 +1,5 @@ +import { checkHandler } from '../../utils'; + +export const dynamic = 'force-dynamic'; + +export const GET = checkHandler; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch/route.ts new file mode 100644 index 000000000000..df9f2e772931 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-fetch/route.ts @@ -0,0 +1,8 @@ +import { NextResponse } from 'next/server'; + +export const dynamic = 'force-dynamic'; + +export async function GET() { + const data = await fetch(`http://localhost:3030/propagation/test-outgoing-fetch/check`).then(res => res.json()); + return NextResponse.json(data); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http-external-disallowed/check/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http-external-disallowed/check/route.ts new file mode 100644 index 000000000000..e5f497a1bed6 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http-external-disallowed/check/route.ts @@ -0,0 +1,5 @@ +import { checkHandler } from '../../utils'; + +export const dynamic = 'force-dynamic'; + +export const GET = checkHandler; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http-external-disallowed/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http-external-disallowed/route.ts new file mode 100644 index 000000000000..16fa727ce303 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http-external-disallowed/route.ts @@ -0,0 +1,9 @@ +import { NextResponse } from 'next/server'; +import { makeHttpRequest } from '../utils'; + +export const dynamic = 'force-dynamic'; + +export async function GET() { + const data = await makeHttpRequest(`http://localhost:3030/propagation/test-outgoing-http-external-disallowed/check`); + return NextResponse.json(data); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http/check/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http/check/route.ts new file mode 100644 index 000000000000..e5f497a1bed6 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http/check/route.ts @@ -0,0 +1,5 @@ +import { checkHandler } from '../../utils'; + +export const dynamic = 'force-dynamic'; + +export const GET = checkHandler; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http/route.ts new file mode 100644 index 000000000000..fe0bfef09ffa --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/test-outgoing-http/route.ts @@ -0,0 +1,9 @@ +import { NextResponse } from 'next/server'; +import { makeHttpRequest } from '../utils'; + +export const dynamic = 'force-dynamic'; + +export async function GET() { + const data = await makeHttpRequest(`http://localhost:3030/propagation/test-outgoing-http/check`); + return NextResponse.json(data); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/utils.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/utils.ts new file mode 100644 index 000000000000..a065c53ee4c9 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/propagation/utils.ts @@ -0,0 +1,38 @@ +import http from 'http'; +import { headers } from 'next/headers'; +import { NextResponse } from 'next/server'; + +export function makeHttpRequest(url: string) { + return new Promise(resolve => { + const data: any[] = []; + http + .request(url, httpRes => { + httpRes.on('data', chunk => { + data.push(chunk); + }); + httpRes.on('error', error => { + resolve({ error: error.message, url }); + }); + httpRes.on('end', () => { + try { + const json = JSON.parse(Buffer.concat(data).toString()); + resolve(json); + } catch { + resolve({ data: Buffer.concat(data).toString(), url }); + } + }); + }) + .end(); + }); +} + +export function checkHandler() { + const headerList = headers(); + + const headerObj: Record = {}; + headerList.forEach((value, key) => { + headerObj[key] = value; + }); + + return NextResponse.json({ headers: headerObj }); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/app/request-instrumentation/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-14/app/request-instrumentation/page.tsx index 7e59ffbe0a91..c11efda8adc9 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/app/request-instrumentation/page.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/app/request-instrumentation/page.tsx @@ -3,7 +3,7 @@ import http from 'http'; export const dynamic = 'force-dynamic'; export default async function Page() { - await fetch('http://example.com/', { cache: 'no-cache' }); + await fetch('http://example.com/', { cache: 'no-cache' }).then(res => res.text()); await new Promise(resolve => { http.get('http://example.com/', res => { res.on('data', () => { diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/instrumentation.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/instrumentation.ts index cd269ab160e7..0999fdd8e089 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/instrumentation.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/instrumentation.ts @@ -12,6 +12,10 @@ export function register() { // We are doing a lot of events at once in this test bufferSize: 1000, }, + tracePropagationTargets: [ + 'http://localhost:3030/propagation/test-outgoing-fetch/check', + 'http://localhost:3030/propagation/test-outgoing-http/check', + ], }); } } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts index af4d9046b2fa..eae01c2c4287 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts @@ -22,7 +22,7 @@ const eventProxyPort = 3031; const config: PlaywrightTestConfig = { testDir: './tests', /* Maximum time one test can run for. */ - timeout: 150_000, + timeout: 30_000, expect: { /** * Maximum time expect() should wait for the condition to be met. diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/propagation.test.ts new file mode 100644 index 000000000000..2653d57ca179 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/propagation.test.ts @@ -0,0 +1,113 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/event-proxy-server'; + +test('Propagates trace for outgoing http requests', async ({ baseURL, request }) => { + const inboundTransactionPromise = waitForTransaction('nextjs-14', transactionEvent => { + return transactionEvent.transaction === 'GET /propagation/test-outgoing-http/check'; + }); + + const outboundTransactionPromise = waitForTransaction('nextjs-14', transactionEvent => { + return transactionEvent.transaction === 'GET /propagation/test-outgoing-http'; + }); + + const { headers } = await (await request.get(`${baseURL}/propagation/test-outgoing-http`)).json(); + + const inboundTransaction = await inboundTransactionPromise; + const outboundTransaction = await outboundTransactionPromise; + + expect(inboundTransaction.contexts?.trace?.trace_id).toStrictEqual(expect.any(String)); + expect(inboundTransaction.contexts?.trace?.trace_id).toBe(outboundTransaction.contexts?.trace?.trace_id); + + const httpClientSpan = outboundTransaction.spans?.find(span => span.op === 'http.client'); + + expect(httpClientSpan).toBeDefined(); + expect(httpClientSpan?.span_id).toStrictEqual(expect.any(String)); + expect(inboundTransaction.contexts?.trace?.parent_span_id).toBe(httpClientSpan?.span_id); + + expect(headers).toMatchObject({ + baggage: expect.any(String), + 'sentry-trace': `${outboundTransaction.contexts?.trace?.trace_id}-${httpClientSpan?.span_id}-1`, + }); +}); + +test('Propagates trace for outgoing fetch requests', async ({ baseURL, request }) => { + const inboundTransactionPromise = waitForTransaction('nextjs-14', transactionEvent => { + return transactionEvent.transaction === 'GET /propagation/test-outgoing-fetch/check'; + }); + + const outboundTransactionPromise = waitForTransaction('nextjs-14', transactionEvent => { + return transactionEvent.transaction === 'GET /propagation/test-outgoing-fetch'; + }); + + const { headers } = await (await request.get(`${baseURL}/propagation/test-outgoing-fetch`)).json(); + + const inboundTransaction = await inboundTransactionPromise; + const outboundTransaction = await outboundTransactionPromise; + + expect(inboundTransaction.contexts?.trace?.trace_id).toStrictEqual(expect.any(String)); + expect(inboundTransaction.contexts?.trace?.trace_id).toBe(outboundTransaction.contexts?.trace?.trace_id); + + const httpClientSpan = outboundTransaction.spans?.find( + span => span.op === 'http.client' && span.data?.['sentry.origin'] === 'auto.http.otel.node_fetch', + ); + + // Right now we assert that the OTEL span is the last span before propagating + expect(httpClientSpan).toBeDefined(); + expect(httpClientSpan?.span_id).toStrictEqual(expect.any(String)); + expect(inboundTransaction.contexts?.trace?.parent_span_id).toBe(httpClientSpan?.span_id); + + expect(headers).toMatchObject({ + baggage: expect.any(String), + 'sentry-trace': `${outboundTransaction.contexts?.trace?.trace_id}-${httpClientSpan?.span_id}-1`, + }); +}); + +test('Does not propagate outgoing http requests not covered by tracePropagationTargets', async ({ + baseURL, + request, +}) => { + const inboundTransactionPromise = waitForTransaction('nextjs-14', transactionEvent => { + return transactionEvent.transaction === 'GET /propagation/test-outgoing-http-external-disallowed/check'; + }); + + const outboundTransactionPromise = waitForTransaction('nextjs-14', transactionEvent => { + return transactionEvent.transaction === 'GET /propagation/test-outgoing-http-external-disallowed'; + }); + + const { headers } = await (await request.get(`${baseURL}/propagation/test-outgoing-http-external-disallowed`)).json(); + + expect(headers.baggage).toBeUndefined(); + expect(headers['sentry-trace']).toBeUndefined(); + + const inboundTransaction = await inboundTransactionPromise; + const outboundTransaction = await outboundTransactionPromise; + + expect(typeof outboundTransaction.contexts?.trace?.trace_id).toBe('string'); + expect(inboundTransaction.contexts?.trace?.trace_id).not.toBe(outboundTransaction.contexts?.trace?.trace_id); +}); + +test('Does not propagate outgoing fetch requests not covered by tracePropagationTargets', async ({ + baseURL, + request, +}) => { + const inboundTransactionPromise = waitForTransaction('nextjs-14', transactionEvent => { + return transactionEvent.transaction === 'GET /propagation/test-outgoing-fetch-external-disallowed/check'; + }); + + const outboundTransactionPromise = waitForTransaction('nextjs-14', transactionEvent => { + return transactionEvent.transaction === 'GET /propagation/test-outgoing-fetch-external-disallowed'; + }); + + const { headers } = await ( + await request.get(`${baseURL}/propagation/test-outgoing-fetch-external-disallowed`) + ).json(); + + expect(headers.baggage).toBeUndefined(); + expect(headers['sentry-trace']).toBeUndefined(); + + const inboundTransaction = await inboundTransactionPromise; + const outboundTransaction = await outboundTransactionPromise; + + expect(typeof outboundTransaction.contexts?.trace?.trace_id).toBe('string'); + expect(inboundTransaction.contexts?.trace?.trace_id).not.toBe(outboundTransaction.contexts?.trace?.trace_id); +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts index 3e25a99133da..6beb6f531d11 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts @@ -8,24 +8,27 @@ test('Should send a transaction with a fetch span', async ({ page }) => { await page.goto(`/request-instrumentation`); - expect((await transactionPromise).spans).toContainEqual( + await expect(transactionPromise).resolves.toBeDefined(); + + const transactionEvent = await transactionPromise; + + expect(transactionEvent.spans).toContainEqual( expect.objectContaining({ data: expect.objectContaining({ 'http.method': 'GET', 'sentry.op': 'http.client', - 'next.span_type': 'AppRender.fetch', // This span is created by Next.js own fetch instrumentation + 'sentry.origin': 'auto.http.otel.node_fetch', }), description: 'GET http://example.com/', }), ); - expect((await transactionPromise).spans).toContainEqual( + expect(transactionEvent.spans).toContainEqual( expect.objectContaining({ data: expect.objectContaining({ 'http.method': 'GET', 'sentry.op': 'http.client', - // todo: without the HTTP integration in the Next.js SDK, this is set to 'manual' -> we could rename this to be more specific - 'sentry.origin': 'manual', + 'sentry.origin': 'auto.http.otel.http', }), description: 'GET http://example.com/', }), diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 0251f5782bcb..b093d9bd2e97 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -73,14 +73,16 @@ export function init(options: NodeOptions): void { const customDefaultIntegrations = [ ...getDefaultIntegrations(options).filter( integration => - // Next.js comes with its own Node-Fetch instrumentation, so we shouldn't add ours on-top - integration.name !== 'NodeFetch' && // Next.js comes with its own Http instrumentation for OTel which would lead to double spans for route handler requests integration.name !== 'Http', ), httpIntegration(), ]; + // Turn off Next.js' own fetch instrumentation + // https://github.com/lforst/nextjs-fork/blob/1994fd186defda77ad971c36dc3163db263c993f/packages/next/src/server/lib/patch-fetch.ts#L245 + process.env.NEXT_OTEL_FETCH_DISABLED = '1'; + // This value is injected at build time, based on the output directory specified in the build config. Though a default // is set there, we set it here as well, just in case something has gone wrong with the injection. const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__;