Skip to content

Commit a230be4

Browse files
ijjkhuozhi
authored andcommitted
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
1 parent 73c2d63 commit a230be4

File tree

5 files changed

+50
-8
lines changed

5 files changed

+50
-8
lines changed

packages/next/src/client/components/static-generation-async-storage.external.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ export interface StaticGenerationStore {
5151

5252
isDraftMode?: boolean
5353
isUnstableNoStore?: boolean
54+
55+
requestEndedState?: { ended?: boolean }
5456
}
5557

5658
export type StaticGenerationAsyncStorage =

packages/next/src/server/app-render/app-render.tsx

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,8 @@ async function renderToHTMLOrFlightImpl(
585585
pagePath: string,
586586
query: NextParsedUrlQuery,
587587
renderOpts: RenderOpts,
588-
baseCtx: AppRenderBaseContext
588+
baseCtx: AppRenderBaseContext,
589+
requestEndedState: { ended?: boolean }
589590
) {
590591
const isNotFoundPath = pagePath === '/404'
591592

@@ -621,6 +622,7 @@ async function renderToHTMLOrFlightImpl(
621622

622623
if (typeof req.on === 'function') {
623624
req.on('end', () => {
625+
requestEndedState.ended = true
624626
if ('performance' in globalThis) {
625627
const metrics = getClientComponentLoaderMetrics({ reset: true })
626628
if (metrics) {
@@ -1438,14 +1440,23 @@ export const renderToHTMLOrFlight: AppPageRender = (
14381440
{
14391441
urlPathname: pathname,
14401442
renderOpts,
1443+
requestEndedState: { ended: false },
14411444
},
14421445
(staticGenerationStore) =>
1443-
renderToHTMLOrFlightImpl(req, res, pagePath, query, renderOpts, {
1444-
requestStore,
1445-
staticGenerationStore,
1446-
componentMod: renderOpts.ComponentMod,
1446+
renderToHTMLOrFlightImpl(
1447+
req,
1448+
res,
1449+
pagePath,
1450+
query,
14471451
renderOpts,
1448-
})
1452+
{
1453+
requestStore,
1454+
staticGenerationStore,
1455+
componentMod: renderOpts.ComponentMod,
1456+
renderOpts,
1457+
},
1458+
staticGenerationStore.requestEndedState || {}
1459+
)
14491460
)
14501461
)
14511462
}

packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type { FetchMetric } from '../base-http'
99

1010
export type StaticGenerationContext = {
1111
urlPathname: string
12+
requestEndedState?: { ended?: boolean }
1213
renderOpts: {
1314
incrementalCache?: IncrementalCache
1415
isOnDemandRevalidate?: boolean
@@ -50,7 +51,7 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper<
5051
> = {
5152
wrap<Result>(
5253
storage: AsyncLocalStorage<StaticGenerationStore>,
53-
{ urlPathname, renderOpts }: StaticGenerationContext,
54+
{ urlPathname, renderOpts, requestEndedState }: StaticGenerationContext,
5455
callback: (store: StaticGenerationStore) => Result
5556
): Result {
5657
/**
@@ -96,6 +97,7 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper<
9697
isDraftMode: renderOpts.isDraftMode,
9798

9899
prerenderState,
100+
requestEndedState,
99101
}
100102

101103
// TODO: remove this when we resolve accessing the store outside the execution context

packages/next/src/server/lib/patch-fetch.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,13 @@ function trackFetchMetric(
163163
staticGenerationStore: StaticGenerationStore,
164164
ctx: Omit<FetchMetric, 'end' | 'idx'>
165165
) {
166-
if (!staticGenerationStore) return
166+
if (
167+
!staticGenerationStore ||
168+
staticGenerationStore.requestEndedState?.ended ||
169+
process.env.NODE_ENV !== 'development'
170+
) {
171+
return
172+
}
167173
staticGenerationStore.fetchMetrics ??= []
168174

169175
const dedupeFields = ['url', 'status', 'method'] as const
@@ -181,6 +187,25 @@ function trackFetchMetric(
181187
end: Date.now(),
182188
idx: staticGenerationStore.nextFetchId || 0,
183189
})
190+
191+
// only store top 10 metrics to avoid storing too many
192+
if (staticGenerationStore.fetchMetrics.length > 10) {
193+
// sort slowest first as these should be highlighted
194+
staticGenerationStore.fetchMetrics.sort((a, b) => {
195+
const aDur = a.end - a.start
196+
const bDur = b.end - b.start
197+
198+
if (aDur < bDur) {
199+
return 1
200+
} else if (aDur > bDur) {
201+
return -1
202+
}
203+
return 0
204+
})
205+
// now grab top 10
206+
staticGenerationStore.fetchMetrics =
207+
staticGenerationStore.fetchMetrics.slice(0, 10)
208+
}
184209
}
185210

186211
interface PatchableModule {

packages/next/src/server/next-server.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,7 @@ export default class NextNodeServer extends BaseServer {
11271127
if (this.renderOpts.dev) {
11281128
const { blue, green, yellow, red, gray, white } =
11291129
require('../lib/picocolors') as typeof import('../lib/picocolors')
1130+
11301131
const _res = res as NodeNextResponse | ServerResponse
11311132
const origRes =
11321133
'originalResponse' in _res ? _res.originalResponse : _res
@@ -1247,6 +1248,7 @@ export default class NextNodeServer extends BaseServer {
12471248
}
12481249
}
12491250
}
1251+
delete normalizedReq.fetchMetrics
12501252
origRes.off('close', reqCallback)
12511253
}
12521254
origRes.on('close', reqCallback)

0 commit comments

Comments
 (0)