diff --git a/packages/vinext/src/server/api-handler.ts b/packages/vinext/src/server/api-handler.ts index 99129d861..01451a816 100644 --- a/packages/vinext/src/server/api-handler.ts +++ b/packages/vinext/src/server/api-handler.ts @@ -35,6 +35,7 @@ import { type PagesReqResRequest, type PagesReqResResponse, } from "./pages-node-compat.js"; +import { tracePagesApiHandler } from "./pages-execution-tracing.js"; /** * Extend the Node.js request with Next.js-style helpers. @@ -416,7 +417,9 @@ export async function handleApiRoute( } : undefined, ); - const handlerResponse = await apiModule.default(nextRequest); + const handlerResponse = await tracePagesApiHandler(route.pattern, () => + apiModule.default(nextRequest), + ); if (!(handlerResponse instanceof Response)) { throw new Error("Edge API route did not return a Response"); } @@ -473,7 +476,7 @@ export async function handleApiRoute( ); // Call the handler - await handler(apiReq, apiRes); + await tracePagesApiHandler(route.pattern, () => handler(apiReq, apiRes)); return true; } catch (e) { if (e instanceof PagesBodyParseError) { diff --git a/packages/vinext/src/server/dev-server.ts b/packages/vinext/src/server/dev-server.ts index 7782d8595..06fa6c7ae 100644 --- a/packages/vinext/src/server/dev-server.ts +++ b/packages/vinext/src/server/dev-server.ts @@ -86,6 +86,7 @@ import { type PagesPreviewState, } from "./pages-preview.js"; import { isBotUserAgent } from "../utils/html-limited-bots.js"; +import { tracePagesData } from "./pages-execution-tracing.js"; /** * Render a React element to a string using renderToReadableStream. @@ -1100,7 +1101,9 @@ export function createSSRHandler( defaultLocale: currentDefaultLocale, ...previewContext, }; - const result = await pageModule.getServerSideProps(context); + const result = await tracePagesData("getServerSideProps", route.pattern, () => + pageModule.getServerSideProps!(context), + ); // If gSSP called res.end() directly (short-circuit pattern), // the response is already sent. Do not continue rendering. // Note: middleware headers are already on `res` (middleware runs @@ -1258,7 +1261,9 @@ export function createSSRHandler( return; } - const result = await pageModule.getStaticProps(context); + const result = await tracePagesData("getStaticProps", route.pattern, () => + pageModule.getStaticProps!(context), + ); const routePattern = patternToNextFormat(route.pattern); assertPages404DoesNotReturnNotFound(routePattern, result); if (result) { @@ -1849,12 +1854,14 @@ async function renderErrorPage( const isOnDemandRevalidate = isOnDemandRevalidateRequest( req.headers[PRERENDER_REVALIDATE_HEADER], ); - const staticResult = await errorModule.getStaticProps({ - locale: context.locale, - locales: context.locales, - defaultLocale: context.defaultLocale, - revalidateReason: isOnDemandRevalidate ? "on-demand" : "stale", - }); + const staticResult = await tracePagesData("getStaticProps", errorPage, () => + errorModule.getStaticProps({ + locale: context.locale, + locales: context.locales, + defaultLocale: context.defaultLocale, + revalidateReason: isOnDemandRevalidate ? "on-demand" : "stale", + }), + ); assertPages404DoesNotReturnNotFound(errorPage, staticResult); if (staticResult?.redirect) { applyDevPagesCacheHeaders( diff --git a/packages/vinext/src/server/pages-api-route.ts b/packages/vinext/src/server/pages-api-route.ts index 6db574179..4bb2a0082 100644 --- a/packages/vinext/src/server/pages-api-route.ts +++ b/packages/vinext/src/server/pages-api-route.ts @@ -29,6 +29,7 @@ import { type ExecutionContextLike, } from "vinext/shims/request-context"; import { NextRequest } from "vinext/shims/server"; +import { tracePagesApiHandler } from "./pages-execution-tracing.js"; type PagesApiRouteConfig = { runtime?: string; @@ -158,6 +159,7 @@ async function _handlePagesApiRoute(options: HandlePagesApiRouteOptions): Promis try { if (isEdgeApiRouteModule(route.module)) { + const handler = route.module.default; // Next.js wraps the incoming Request in a NextRequest before invoking // edge API handlers, so handlers can use `req.nextUrl.searchParams`, // `req.cookies`, etc. (Cf. NextRequestHint in next/src/server/web/adapter.ts.) @@ -173,7 +175,7 @@ async function _handlePagesApiRoute(options: HandlePagesApiRouteOptions): Promis } : undefined, ); - const response = await route.module.default(nextRequest); + const response = await tracePagesApiHandler(route.pattern, () => handler(nextRequest)); if (response instanceof Response) { const finalized = finalizeEdgeApiResponse(response, options.edgeRuntime ?? "worker"); if ( @@ -284,7 +286,7 @@ async function _handlePagesApiRoute(options: HandlePagesApiRouteOptions): Promis // handlers attached. A synchronous throw may destroy the response bridge, // which rejects responsePromise as well as the handler completion. const handlerCompletion = Promise.resolve() - .then(() => handler(req, res)) + .then(() => tracePagesApiHandler(route.pattern, () => handler(req, res))) .then(() => ({ type: "handler" as const }), destroyAfterHandlerError); // A real Node ServerResponse is consumed by the socket while the API diff --git a/packages/vinext/src/server/pages-execution-tracing.ts b/packages/vinext/src/server/pages-execution-tracing.ts new file mode 100644 index 000000000..ba3b5432b --- /dev/null +++ b/packages/vinext/src/server/pages-execution-tracing.ts @@ -0,0 +1,33 @@ +import { patternToNextFormat } from "../routing/route-validation.js"; +import { frameworkTracer } from "./tracer.js"; + +type PagesDataMethod = "getServerSideProps" | "getStaticProps"; + +export function createPagesDataSpanDescriptor(method: PagesDataMethod, routePattern: string) { + const route = patternToNextFormat(routePattern); + return { + attributes: { "next.route": route }, + name: `${method} ${route}`, + type: `Render.${method}`, + } as const; +} + +export function tracePagesData( + method: PagesDataMethod, + routePattern: string, + callback: () => T, +): T { + return frameworkTracer.trace(createPagesDataSpanDescriptor(method, routePattern), callback); +} + +export function createPagesApiHandlerSpanDescriptor(routePattern: string) { + const route = patternToNextFormat(routePattern); + return { + name: `executing api route (pages) ${route}`, + type: "Node.runHandler", + } as const; +} + +export function tracePagesApiHandler(routePattern: string, callback: () => T): T { + return frameworkTracer.trace(createPagesApiHandlerSpanDescriptor(routePattern), callback); +} diff --git a/packages/vinext/src/server/pages-page-data.ts b/packages/vinext/src/server/pages-page-data.ts index 667538ec4..d0f7efde1 100644 --- a/packages/vinext/src/server/pages-page-data.ts +++ b/packages/vinext/src/server/pages-page-data.ts @@ -43,6 +43,7 @@ import { isBotUserAgent } from "../utils/html-limited-bots.js"; import { isUnknownRecord } from "../utils/record.js"; import { isDangerousScheme } from "vinext/shims/url-safety"; import { encodeCacheTag } from "../utils/encode-cache-tag.js"; +import { tracePagesData } from "./pages-execution-tracing.js"; export type PagesRedirectResult = { destination: string; @@ -1332,17 +1333,19 @@ export async function resolvePagesPageData( } renderProps = { ...renderProps, __N_SSP: true }; const { req, res, responsePromise } = getSharedReqRes(); - const result = await options.pageModule.getServerSideProps({ - params: userFacingParams, - req, - res, - query: options.query, - resolvedUrl: options.resolvedUrl ?? options.routeUrl, - locale: options.i18n.locale, - locales: options.i18n.locales, - defaultLocale: options.i18n.defaultLocale, - ...previewContext, - }); + const result = await tracePagesData("getServerSideProps", options.routePattern, () => + options.pageModule.getServerSideProps!({ + params: userFacingParams, + req, + res, + query: options.query, + resolvedUrl: options.resolvedUrl ?? options.routeUrl, + locale: options.i18n.locale, + locales: options.i18n.locales, + defaultLocale: options.i18n.defaultLocale, + ...previewContext, + }), + ); if (isResponseSent(res)) { return { @@ -1430,13 +1433,15 @@ export async function resolvePagesPageData( let freshPageProps = freshAppResult.pageProps; let freshRenderProps = freshAppResult.renderProps; - const freshResult = await options.pageModule.getStaticProps?.({ - params: userFacingParams, - locale: options.i18n.locale, - locales: options.i18n.locales, - defaultLocale: options.i18n.defaultLocale, - revalidateReason: "stale", - }); + const freshResult = await tracePagesData("getStaticProps", options.routePattern, () => + options.pageModule.getStaticProps?.({ + params: userFacingParams, + locale: options.i18n.locale, + locales: options.i18n.locales, + defaultLocale: options.i18n.defaultLocale, + revalidateReason: "stale", + }), + ); if (!freshResult) return; assertPages404DoesNotReturnNotFound(options.routePattern, freshResult); @@ -1696,18 +1701,20 @@ export async function resolvePagesPageData( } const result = generatedPageData ? null - : await options.pageModule.getStaticProps({ - params: userFacingParams, - locale: options.i18n.locale, - locales: options.i18n.locales, - defaultLocale: options.i18n.defaultLocale, - ...previewContext, - revalidateReason: options.isOnDemandRevalidate - ? "on-demand" - : options.isBuildTimePrerendering - ? "build" - : "stale", - }); + : await tracePagesData("getStaticProps", options.routePattern, () => + options.pageModule.getStaticProps!({ + params: userFacingParams, + locale: options.i18n.locale, + locales: options.i18n.locales, + defaultLocale: options.i18n.defaultLocale, + ...previewContext, + revalidateReason: options.isOnDemandRevalidate + ? "on-demand" + : options.isBuildTimePrerendering + ? "build" + : "stale", + }), + ); assertPages404DoesNotReturnNotFound(options.routePattern, result); if (generatedPageData) { diff --git a/tests/e2e/cloudflare-sentry-pages/sentry.spec.ts b/tests/e2e/cloudflare-sentry-pages/sentry.spec.ts index a7256be53..c0ec812dd 100644 --- a/tests/e2e/cloudflare-sentry-pages/sentry.spec.ts +++ b/tests/e2e/cloudflare-sentry-pages/sentry.spec.ts @@ -124,11 +124,23 @@ test.describe("Sentry on Cloudflare Workers Pages Router", () => { expect(transaction.traceId).toMatch(/^[0-9a-f]{32}$/); expect(transaction.spanId).toMatch(/^[0-9a-f]{16}$/); + const handlerSpan = transaction.spans.find( + ({ attributes }) => attributes["next.span_type"] === "Node.runHandler", + ); + expect(handlerSpan).toMatchObject({ + attributes: expect.objectContaining({ + "next.span_name": "executing api route (pages) /api/trace/[slug]", + "next.span_type": "Node.runHandler", + }), + name: "executing api route (pages) /api/trace/[slug]", + parentSpanId: transaction.spanId, + traceId: transaction.traceId, + }); expect(transaction.spans).toContainEqual( expect.objectContaining({ name: "fixture.pages.child", traceId: transaction.traceId, - parentSpanId: transaction.spanId, + parentSpanId: handlerSpan?.spanId, operation: "fixture.child", attributes: expect.objectContaining({ "fixture.router": "pages", @@ -138,6 +150,69 @@ test.describe("Sentry on Cloudflare Workers Pages Router", () => { ); }); + // Ported from Next.js: test/e2e/opentelemetry/instrumentation/opentelemetry.test.ts + // https://github.com/vercel/next.js/blob/canary/test/e2e/opentelemetry/instrumentation/opentelemetry.test.ts + test("parents getServerSideProps application spans beneath the framework span", async ({ + request, + }) => { + const traceRes = await request.get("/trace-gssp/product-42"); + expect(traceRes.status()).toBe(200); + + const transaction = await expectReportedTransaction(request, "GET /trace-gssp/[slug]"); + const dataSpan = transaction.spans.find( + ({ attributes }) => attributes["next.span_type"] === "Render.getServerSideProps", + ); + expect(dataSpan).toMatchObject({ + attributes: expect.objectContaining({ + "next.route": "/trace-gssp/[slug]", + "next.span_name": "getServerSideProps /trace-gssp/[slug]", + "next.span_type": "Render.getServerSideProps", + }), + name: "getServerSideProps /trace-gssp/[slug]", + parentSpanId: transaction.spanId, + traceId: transaction.traceId, + }); + expect(transaction.spans).toContainEqual( + expect.objectContaining({ + attributes: expect.objectContaining({ "fixture.slug": "product-42" }), + name: "fixture.pages.gssp.child", + operation: "fixture.gssp", + parentSpanId: dataSpan?.spanId, + traceId: transaction.traceId, + }), + ); + }); + + test("traces request-time getStaticProps for a blocking fallback", async ({ request }) => { + const slug = `runtime-${Date.now()}`; + const traceRes = await request.get(`/trace-gsp/${slug}`); + expect(traceRes.status()).toBe(200); + + const transaction = await expectReportedTransaction(request, "GET /trace-gsp/[slug]"); + const dataSpan = transaction.spans.find( + ({ attributes }) => attributes["next.span_type"] === "Render.getStaticProps", + ); + expect(dataSpan).toMatchObject({ + attributes: expect.objectContaining({ + "next.route": "/trace-gsp/[slug]", + "next.span_name": "getStaticProps /trace-gsp/[slug]", + "next.span_type": "Render.getStaticProps", + }), + name: "getStaticProps /trace-gsp/[slug]", + parentSpanId: transaction.spanId, + traceId: transaction.traceId, + }); + expect(transaction.spans).toContainEqual( + expect.objectContaining({ + attributes: expect.objectContaining({ "fixture.slug": slug }), + name: "fixture.pages.gsp.child", + operation: "fixture.gsp", + parentSpanId: dataSpan?.spanId, + traceId: transaction.traceId, + }), + ); + }); + test("continues incoming Sentry traces without leaking parallel request context", async ({ request, }) => { diff --git a/tests/fixtures/cf-sentry-pages/pages/trace-gsp/[slug].tsx b/tests/fixtures/cf-sentry-pages/pages/trace-gsp/[slug].tsx new file mode 100644 index 000000000..674db36d8 --- /dev/null +++ b/tests/fixtures/cf-sentry-pages/pages/trace-gsp/[slug].tsx @@ -0,0 +1,20 @@ +import * as Sentry from "@sentry/nextjs"; +import type { GetStaticPaths, GetStaticProps } from "next"; + +export const getStaticPaths: GetStaticPaths = () => ({ fallback: "blocking", paths: [] }); + +export const getStaticProps: GetStaticProps<{ slug: string }> = async ({ params }) => { + const slug = String(params?.slug ?? ""); + return Sentry.startSpan( + { + attributes: { "fixture.slug": slug }, + name: "fixture.pages.gsp.child", + op: "fixture.gsp", + }, + () => ({ props: { slug }, revalidate: 60 }), + ); +}; + +export default function TraceGspPage({ slug }: { slug: string }) { + return
GSP trace: {slug}
; +} diff --git a/tests/fixtures/cf-sentry-pages/pages/trace-gssp/[slug].tsx b/tests/fixtures/cf-sentry-pages/pages/trace-gssp/[slug].tsx new file mode 100644 index 000000000..26a07a188 --- /dev/null +++ b/tests/fixtures/cf-sentry-pages/pages/trace-gssp/[slug].tsx @@ -0,0 +1,18 @@ +import * as Sentry from "@sentry/nextjs"; +import type { GetServerSideProps } from "next"; + +export const getServerSideProps: GetServerSideProps<{ slug: string }> = async ({ params }) => { + const slug = String(params?.slug ?? ""); + return Sentry.startSpan( + { + attributes: { "fixture.slug": slug }, + name: "fixture.pages.gssp.child", + op: "fixture.gssp", + }, + () => ({ props: { slug } }), + ); +}; + +export default function TraceGsspPage({ slug }: { slug: string }) { + return
GSSP trace: {slug}
; +} diff --git a/tests/pages-execution-tracing.test.ts b/tests/pages-execution-tracing.test.ts new file mode 100644 index 000000000..983cba161 --- /dev/null +++ b/tests/pages-execution-tracing.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vite-plus/test"; +import { + createPagesApiHandlerSpanDescriptor, + createPagesDataSpanDescriptor, +} from "../packages/vinext/src/server/pages-execution-tracing.js"; + +describe("Pages execution tracing", () => { + // Ported from Next.js: test/e2e/opentelemetry/instrumentation/opentelemetry.test.ts + // https://github.com/vercel/next.js/blob/canary/test/e2e/opentelemetry/instrumentation/opentelemetry.test.ts + it.each([ + ["getServerSideProps", "Render.getServerSideProps"], + ["getStaticProps", "Render.getStaticProps"], + ] as const)("matches the stable Next.js %s span descriptor", (method, type) => { + expect(createPagesDataSpanDescriptor(method, "/products/:slug")).toEqual({ + attributes: { "next.route": "/products/[slug]" }, + name: `${method} /products/[slug]`, + type, + }); + }); + + it("matches the stable Next.js Pages API handler span descriptor", () => { + expect(createPagesApiHandlerSpanDescriptor("/api/products/:slug")).toEqual({ + name: "executing api route (pages) /api/products/[slug]", + type: "Node.runHandler", + }); + }); +}); diff --git a/tests/pages-router.test.ts b/tests/pages-router.test.ts index 8ea7c6aad..b18c745d5 100644 --- a/tests/pages-router.test.ts +++ b/tests/pages-router.test.ts @@ -16,6 +16,18 @@ import { PHASE_PRODUCTION_BUILD, } from "../packages/vinext/src/shims/constants.js"; import { PAGES_FIXTURE_DIR, buildPagesFixture, startFixtureServer } from "./helpers.js"; +import { registerFrameworkTracingIntegration } from "../packages/vinext/src/server/tracer.js"; +import type { ResolvedFrameworkSpanDescriptor } from "../packages/vinext/src/server/framework-tracer.js"; + +let captureFrameworkSpans = false; +const capturedFrameworkSpans: ResolvedFrameworkSpanDescriptor[] = []; +registerFrameworkTracingIntegration({ + id: "pages-router-test", + enterSpan(descriptor, callback) { + if (captureFrameworkSpans) capturedFrameworkSpans.push(descriptor); + return callback({ setAttribute() {} }); + }, +}); const FIXTURE_DIR = PAGES_FIXTURE_DIR; const PAGES_APP_COMPONENT = `export default function App({ Component, pageProps }) { @@ -1310,7 +1322,10 @@ export async function getStaticPaths() { const started = await startFixtureServer(tmpDir); tempServer = started.server; + capturedFrameworkSpans.length = 0; + captureFrameworkSpans = true; const first = await fetch(`${started.baseUrl}/first`); + captureFrameworkSpans = false; expect(first.status).toBe(404); expect(first.headers.get("x-nextjs-cache")).toBe("HIT"); expect(first.headers.get("x-vinext-cache")).toBeNull(); @@ -1318,6 +1333,11 @@ export async function getStaticPaths() { const firstHtml = await first.text(); expect(firstHtml).toContain('

404 page 1

'); expect(firstHtml).toContain('"paramsAreUndefined":true'); + expect( + capturedFrameworkSpans + .filter(({ type }) => type === "Render.getStaticProps") + .map(({ name }) => name), + ).toEqual(["getStaticProps /[slug]", "getStaticProps /404"]); const second = await fetch(`${started.baseUrl}/first`); expect(second.status).toBe(404); @@ -1328,6 +1348,7 @@ export async function getStaticPaths() { expect(secondHtml).toContain('

404 page 2

'); expect(secondHtml).toContain('"paramsAreUndefined":true'); } finally { + captureFrameworkSpans = false; await tempServer?.close(); fs.rmSync(tmpDir, { recursive: true, force: true }); }