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: #64212 Closes NEXT-3159
vercel · Apr 23, 2024 · a230be4 · a230be4
1 parent 73c2d63
commit a230be4
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 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
@@ -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
@@ -585,7 +585,8 @@ async function renderToHTMLOrFlightImpl(
   pagePath: string,
   query: NextParsedUrlQuery,
   renderOpts: RenderOpts,
-  baseCtx: AppRenderBaseContext
+  baseCtx: AppRenderBaseContext,
+  requestEndedState: { ended?: boolean }
 ) {
   const isNotFoundPath = pagePath === '/404'
 
@@ -621,6 +622,7 @@ async function renderToHTMLOrFlightImpl(
 
   if (typeof req.on === 'function') {
     req.on('end', () => {
+      requestEndedState.ended = true
       if ('performance' in globalThis) {
         const metrics = getClientComponentLoaderMetrics({ reset: true })
         if (metrics) {
@@ -1438,14 +1440,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
@@ -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<Result>(
     storage: AsyncLocalStorage<StaticGenerationStore>,
-    { 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
@@ -163,7 +163,13 @@ function trackFetchMetric(
   staticGenerationStore: StaticGenerationStore,
   ctx: Omit<FetchMetric, 'end' | 'idx'>
 ) {
-  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
@@ -1127,6 +1127,7 @@ export default class NextNodeServer extends BaseServer {
       if (this.renderOpts.dev) {
         const { blue, green, yellow, red, gray, white } =
           require('../lib/picocolors') as typeof import('../lib/picocolors')
+
         const _res = res as NodeNextResponse | ServerResponse
         const origRes =
           'originalResponse' in _res ? _res.originalResponse : _res
@@ -1247,6 +1248,7 @@ export default class NextNodeServer extends BaseServer {
               }
             }
           }
+          delete normalizedReq.fetchMetrics
           origRes.off('close', reqCallback)
         }
         origRes.on('close', reqCallback)