From 36e3f4e7abcad8077a687e307ed26805c2eab61c Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 18 Apr 2024 12:51:34 -0700 Subject: [PATCH] Clean-up fetch metrics tracking (#64746) This ensures we only track fetch metrics in development mode as that's the only time we report them currently, this also adds an upper limit on how many metrics we store per-request as previously this was unbounded, on top of that this ensures we don't keep tracking fetch metrics after the request has ended as we only report on request end, and finally this adds a clean-up to the reference we attach to the request object containing the fetch metrics once we have used them. Closes: https://github.com/vercel/next.js/issues/64212 Closes NEXT-3159 --- ...tatic-generation-async-storage.external.ts | 2 ++ .../next/src/server/app-render/app-render.tsx | 24 ++++++++++++----- ...static-generation-async-storage-wrapper.ts | 4 ++- packages/next/src/server/lib/patch-fetch.ts | 27 ++++++++++++++++++- packages/next/src/server/next-server.ts | 1 + 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/packages/next/src/client/components/static-generation-async-storage.external.ts b/packages/next/src/client/components/static-generation-async-storage.external.ts index 2a21c77357dfe..62a7b2c04490e 100644 --- a/packages/next/src/client/components/static-generation-async-storage.external.ts +++ b/packages/next/src/client/components/static-generation-async-storage.external.ts @@ -51,6 +51,8 @@ export interface StaticGenerationStore { isDraftMode?: boolean isUnstableNoStore?: boolean + + requestEndedState?: { ended?: boolean } } export type StaticGenerationAsyncStorage = diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index ccd353941155f..95560e8aa8fc3 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -590,7 +590,8 @@ async function renderToHTMLOrFlightImpl( pagePath: string, query: NextParsedUrlQuery, renderOpts: RenderOpts, - baseCtx: AppRenderBaseContext + baseCtx: AppRenderBaseContext, + requestEndedState: { ended?: boolean } ) { const isNotFoundPath = pagePath === '/404' @@ -631,6 +632,8 @@ async function renderToHTMLOrFlightImpl( isNodeNextRequest(req) ) { req.originalRequest.on('end', () => { + requestEndedState.ended = true + if ('performance' in globalThis) { const metrics = getClientComponentLoaderMetrics({ reset: true }) if (metrics) { @@ -1448,14 +1451,23 @@ export const renderToHTMLOrFlight: AppPageRender = ( { urlPathname: pathname, renderOpts, + requestEndedState: { ended: false }, }, (staticGenerationStore) => - renderToHTMLOrFlightImpl(req, res, pagePath, query, renderOpts, { - requestStore, - staticGenerationStore, - componentMod: renderOpts.ComponentMod, + renderToHTMLOrFlightImpl( + req, + res, + pagePath, + query, renderOpts, - }) + { + requestStore, + staticGenerationStore, + componentMod: renderOpts.ComponentMod, + renderOpts, + }, + staticGenerationStore.requestEndedState || {} + ) ) ) } diff --git a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts index 2f11e7cf85097..bbe9d42e1bb86 100644 --- a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts +++ b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts @@ -9,6 +9,7 @@ import type { FetchMetric } from '../base-http' export type StaticGenerationContext = { urlPathname: string + requestEndedState?: { ended?: boolean } renderOpts: { incrementalCache?: IncrementalCache isOnDemandRevalidate?: boolean @@ -50,7 +51,7 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper< > = { wrap( storage: AsyncLocalStorage, - { urlPathname, renderOpts }: StaticGenerationContext, + { urlPathname, renderOpts, requestEndedState }: StaticGenerationContext, callback: (store: StaticGenerationStore) => Result ): Result { /** @@ -96,6 +97,7 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper< isDraftMode: renderOpts.isDraftMode, prerenderState, + requestEndedState, } // TODO: remove this when we resolve accessing the store outside the execution context diff --git a/packages/next/src/server/lib/patch-fetch.ts b/packages/next/src/server/lib/patch-fetch.ts index 5bc3e58740d61..2e7687d728714 100644 --- a/packages/next/src/server/lib/patch-fetch.ts +++ b/packages/next/src/server/lib/patch-fetch.ts @@ -163,7 +163,13 @@ function trackFetchMetric( staticGenerationStore: StaticGenerationStore, ctx: Omit ) { - if (!staticGenerationStore) return + if ( + !staticGenerationStore || + staticGenerationStore.requestEndedState?.ended || + process.env.NODE_ENV !== 'development' + ) { + return + } staticGenerationStore.fetchMetrics ??= [] const dedupeFields = ['url', 'status', 'method'] as const @@ -181,6 +187,25 @@ function trackFetchMetric( end: Date.now(), idx: staticGenerationStore.nextFetchId || 0, }) + + // only store top 10 metrics to avoid storing too many + if (staticGenerationStore.fetchMetrics.length > 10) { + // sort slowest first as these should be highlighted + staticGenerationStore.fetchMetrics.sort((a, b) => { + const aDur = a.end - a.start + const bDur = b.end - b.start + + if (aDur < bDur) { + return 1 + } else if (aDur > bDur) { + return -1 + } + return 0 + }) + // now grab top 10 + staticGenerationStore.fetchMetrics = + staticGenerationStore.fetchMetrics.slice(0, 10) + } } interface PatchableModule { diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index 5242a5b028216..3e93a22431519 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -1232,6 +1232,7 @@ export default class NextNodeServer extends BaseServer< } } } + delete normalizedReq.fetchMetrics originalResponse.off('close', reqCallback) } originalResponse.on('close', reqCallback)