Skip to content

Commit d5dc166

Browse files
authored
[Cache Components] Allow span creation while prerendering (vercel#82350)
When creating spans with OTel random values must be created to follow the OTel spec and is expected in practical implementations. While it is a loose concession Next.js will allow spans to be created while prerendering that have ids generated with random values. and by reading the current time from the system clock. This is pragmatic but potentially risky because if you are not careful and for instance wrap a "use cache" function in a span it is possible you will accidentally pass the span into the cache function causing it to miss on every invocation due to the random nature of one of it's arguments. To defend against this we do check whether `startActiveSpan` is being passed a Cache Function. However it is easy to circumvent by wrapping it in an intermediate function or by using startSpan and context.with yourself. In the long run we should consider tainting the Span objects created in these scopes so they cannot be sent through to a cache. I will consider a follow up PR to land this if we determine taint is likely to be stabilized in React. The pratical implementation of this change is to patch the tracer provider so that it always provides a patched tracer which exits the workUnitStorage on span start/creation and re-enters the scope on inner function exectuion.
1 parent bdd41bb commit d5dc166

File tree

17 files changed

+858
-1
lines changed

17 files changed

+858
-1
lines changed

packages/next/errors.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -772,5 +772,6 @@
772772
"771": "\\`%s\\` was called during a runtime prerender. Next.js should be preventing %s from being included in server components statically, but did not in this case.",
773773
"772": "FetchStrategy.PPRRuntime should never be used when `experimental.clientSegmentCache` is disabled",
774774
"773": "Missing workStore in createPrerenderParamsForClientSegment",
775-
"774": "Route %s used %s outside of a Server Component. This is not allowed."
775+
"774": "Route %s used %s outside of a Server Component. This is not allowed.",
776+
"775": "Node.js instrumentation extensions should not be loaded in the Edge runtime."
776777
}

packages/next/src/server/lib/router-utils/instrumentation-globals.external.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type {
66
InstrumentationOnRequestError,
77
} from '../../instrumentation/types'
88
import { interopDefault } from '../../../lib/interop-default'
9+
import { afterRegistration as extendInstrumentationAfterRegistration } from './instrumentation-node-extensions'
910

1011
let cachedInstrumentationModule: InstrumentationModule
1112

@@ -55,6 +56,7 @@ async function registerInstrumentation(projectDir: string, distDir: string) {
5556
if (instrumentation?.register) {
5657
try {
5758
await instrumentation.register()
59+
extendInstrumentationAfterRegistration()
5860
} catch (err: any) {
5961
err.message = `An error occurred while loading instrumentation hook: ${err.message}`
6062
throw err
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/**
2+
* This extension augments opentelemetry after registration if applicable.
3+
* This extension must only be loaded in Node environments.
4+
*/
5+
6+
import type { Tracer } from '@opentelemetry/api'
7+
import {
8+
type WorkUnitStore,
9+
workUnitAsyncStorage,
10+
} from '../../app-render/work-unit-async-storage.external'
11+
import { InvariantError } from '../../../shared/lib/invariant-error'
12+
import { isUseCacheFunction } from '../../../lib/client-and-server-references'
13+
14+
export function afterRegistration(): void {
15+
if (process.env.NEXT_RUNTIME === 'edge') {
16+
throw new InvariantError(
17+
'Node.js instrumentation extensions should not be loaded in the Edge runtime.'
18+
)
19+
}
20+
21+
extendTracerProviderForCacheComponents()
22+
}
23+
24+
// In theory we only want to enable this extension when cacheComponents is enabled
25+
// however there are certain servers that might load instrumentation before nextConfig is available
26+
// and so gating it on the config might lead to skipping this extension even when it is necessary.
27+
// When cacheComponents is disabled this extension should be a no-op so we enable it universally.
28+
// Additionally, soon, cacheComponents will be enabled always so this just pulls the extension forward in time
29+
function extendTracerProviderForCacheComponents(): void {
30+
let api: typeof import('next/dist/compiled/@opentelemetry/api')
31+
32+
// we want to allow users to use their own version of @opentelemetry/api if they
33+
// want to, so we try to require it first, and if it fails we fall back to the
34+
// version that is bundled with Next.js
35+
// this is because @opentelemetry/api has to be synced with the version of
36+
// @opentelemetry/tracing that is used, and we don't want to force users to use
37+
// the version that is bundled with Next.js.
38+
// the API is ~stable, so this should be fine
39+
try {
40+
api = require('@opentelemetry/api') as typeof import('@opentelemetry/api')
41+
} catch (err) {
42+
api =
43+
require('next/dist/compiled/@opentelemetry/api') as typeof import('next/dist/compiled/@opentelemetry/api')
44+
}
45+
46+
const provider = api.trace.getTracerProvider()
47+
48+
// When Cache Components is enabled we need to instrument the tracer
49+
// to exit the workUnitAsyncStorage context when generating spans.
50+
const originalGetTracer = provider.getTracer.bind(provider)
51+
provider.getTracer = (...args) => {
52+
const tracer = originalGetTracer.apply(provider, args)
53+
if (WeakTracers.has(tracer)) {
54+
return tracer
55+
}
56+
const originalStartSpan = tracer.startSpan
57+
tracer.startSpan = (...startSpanArgs) => {
58+
return workUnitAsyncStorage.exit(() =>
59+
originalStartSpan.apply(tracer, startSpanArgs)
60+
)
61+
}
62+
63+
const originalStartActiveSpan = tracer.startActiveSpan
64+
// @ts-ignore TS doesn't recognize the overloads correctly
65+
tracer.startActiveSpan = (...startActiveSpanArgs: any[]) => {
66+
const workUnitStore = workUnitAsyncStorage.getStore()
67+
if (!workUnitStore) {
68+
// @ts-ignore TS doesn't recognize the overloads correctly
69+
return originalStartActiveSpan.apply(tracer, startActiveSpanArgs)
70+
}
71+
72+
let fnIdx: number = 0
73+
if (
74+
startActiveSpanArgs.length === 2 &&
75+
typeof startActiveSpanArgs[1] === 'function'
76+
) {
77+
fnIdx = 1
78+
} else if (
79+
startActiveSpanArgs.length === 3 &&
80+
typeof startActiveSpanArgs[2] === 'function'
81+
) {
82+
fnIdx = 2
83+
} else if (
84+
startActiveSpanArgs.length > 3 &&
85+
typeof startActiveSpanArgs[3] === 'function'
86+
) {
87+
fnIdx = 3
88+
}
89+
90+
if (fnIdx) {
91+
const originalFn = startActiveSpanArgs[fnIdx]
92+
if (isUseCacheFunction(originalFn)) {
93+
console.error(
94+
'A Cache Function (`use cache`) was passed to startActiveSpan which means it will receive a Span argument with a possibly random ID on every invocation leading to cache misses. Provide a wrapping function around the Cache Function that does not forward the Span argument to avoid this issue.'
95+
)
96+
}
97+
startActiveSpanArgs[fnIdx] = withWorkUnitContext(
98+
workUnitStore,
99+
originalFn
100+
)
101+
}
102+
103+
return workUnitAsyncStorage.exit(() => {
104+
// @ts-ignore TS doesn't recognize the overloads correctly
105+
return originalStartActiveSpan.apply(tracer, startActiveSpanArgs)
106+
})
107+
}
108+
109+
WeakTracers.add(tracer)
110+
return tracer
111+
}
112+
}
113+
114+
const WeakTracers = new WeakSet<Tracer>()
115+
116+
function withWorkUnitContext(
117+
workUnitStore: WorkUnitStore,
118+
fn: (...args: any[]) => any
119+
) {
120+
return (...args: any[]) =>
121+
workUnitAsyncStorage.run(workUnitStore, fn, ...args)
122+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
node_modules
2+
pnpm-lock.yaml
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import {
2+
CachedInnerTraceManualSpan,
3+
InnerTraceManualSpan,
4+
CachedTracedComponentManualSpan,
5+
TracedComponentManualSpan,
6+
CachedInnerTraceActiveSpan,
7+
InnerTraceActiveSpan,
8+
CachedTracedComponentActiveSpan,
9+
TracedComponentActiveSpan,
10+
} from '../../traced-work'
11+
12+
export function generateStaticParams() {
13+
return [{ slug: 'prerendered' }]
14+
}
15+
16+
export default async function Page({
17+
params,
18+
}: {
19+
params: Promise<{ slug: string }>
20+
}) {
21+
'use cache'
22+
return (
23+
<>
24+
<h1>{(await params).slug}</h1>
25+
<div>We are inside a "use cache" scope</div>
26+
<CachedInnerTraceManualSpan />
27+
<InnerTraceManualSpan />
28+
<CachedTracedComponentManualSpan />
29+
<TracedComponentManualSpan />
30+
<CachedInnerTraceActiveSpan />
31+
<InnerTraceActiveSpan />
32+
<CachedTracedComponentActiveSpan />
33+
<TracedComponentActiveSpan />
34+
</>
35+
)
36+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { Suspense } from 'react'
2+
3+
export default async function Layout({
4+
children,
5+
}: {
6+
children: React.ReactNode
7+
}) {
8+
return <Suspense fallback={<div>Loading...</div>}>{children}</Suspense>
9+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import {
2+
CachedInnerTraceManualSpan,
3+
InnerTraceManualSpan,
4+
CachedTracedComponentManualSpan,
5+
TracedComponentManualSpan,
6+
CachedInnerTraceActiveSpan,
7+
InnerTraceActiveSpan,
8+
CachedTracedComponentActiveSpan,
9+
TracedComponentActiveSpan,
10+
} from '../../traced-work'
11+
12+
export function generateStaticParams() {
13+
return [{ slug: 'prerendered' }]
14+
}
15+
16+
export default async function Page({
17+
params,
18+
}: {
19+
params: Promise<{ slug: string }>
20+
}) {
21+
return (
22+
<>
23+
<h1>{(await params).slug}</h1>
24+
<div>We are inside a "use server" scope</div>
25+
<CachedInnerTraceManualSpan />
26+
<InnerTraceManualSpan />
27+
<CachedTracedComponentManualSpan />
28+
<TracedComponentManualSpan />
29+
<CachedInnerTraceActiveSpan />
30+
<InnerTraceActiveSpan />
31+
<CachedTracedComponentActiveSpan />
32+
<TracedComponentActiveSpan />
33+
</>
34+
)
35+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import {
2+
CachedInnerTraceManualSpan,
3+
InnerTraceManualSpan,
4+
CachedTracedComponentManualSpan,
5+
TracedComponentManualSpan,
6+
CachedInnerTraceActiveSpan,
7+
InnerTraceActiveSpan,
8+
CachedTracedComponentActiveSpan,
9+
TracedComponentActiveSpan,
10+
} from '../../traced-work'
11+
12+
export function generateStaticParams() {
13+
return [{ slug: 'prerendered' }]
14+
}
15+
16+
export default async function Page({
17+
params,
18+
}: {
19+
params: Promise<{ slug: string }>
20+
}) {
21+
return (
22+
<>
23+
<h1>{(await params).slug}</h1>
24+
<div>We are inside a "use server" scope</div>
25+
<CachedInnerTraceManualSpan />
26+
<InnerTraceManualSpan />
27+
<CachedTracedComponentManualSpan />
28+
<TracedComponentManualSpan />
29+
<CachedInnerTraceActiveSpan />
30+
<InnerTraceActiveSpan />
31+
<CachedTracedComponentActiveSpan />
32+
<TracedComponentActiveSpan />
33+
</>
34+
)
35+
}
14.7 KB
Binary file not shown.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function Root({ children }: { children: React.ReactNode }) {
2+
return (
3+
<html>
4+
<body>{children}</body>
5+
</html>
6+
)
7+
}

0 commit comments

Comments
 (0)