Skip to content

Commit

Permalink
use array to filer the injected metadata
Browse files Browse the repository at this point in the history
  • Loading branch information
huozhi committed May 14, 2024
1 parent 38536c4 commit e430d57
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 52 deletions.
2 changes: 1 addition & 1 deletion packages/next/src/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,8 @@ export async function exportAppImpl(
deploymentId: nextConfig.deploymentId,
experimental: {
isAppPPREnabled: checkIsAppPPREnabled(nextConfig.experimental.ppr),
clientTraceMetadata: nextConfig.experimental.clientTraceMetadata,
swrDelta: nextConfig.experimental.swrDelta,
clientTraceMetadata: nextConfig.experimental.clientTraceMetadata === true,
},
}

Expand Down
18 changes: 11 additions & 7 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ import { appendMutableCookies } from '../web/spec-extension/adapters/request-coo
import { createServerInsertedHTML } from './server-inserted-html'
import { getRequiredScripts } from './required-scripts'
import { addPathPrefix } from '../../shared/lib/router/utils/add-path-prefix'
import { makeGetServerInsertedHTML } from './make-get-server-inserted-html'
import {
getTracedMetadata,
makeGetServerInsertedHTML,
} from './make-get-server-inserted-html'
import { walkTreeWithFlightRouterState } from './walk-tree-with-flight-router-state'
import { createComponentTree } from './create-component-tree'
import { getAssetQueryString } from './get-asset-query-string'
Expand Down Expand Up @@ -912,6 +915,11 @@ async function renderToHTMLOrFlightImpl(
tree,
formState,
}: RenderToStreamOptions): Promise<RenderToStreamResult> => {
const tracingMetadata = getTracedMetadata(
getTracer().getTracePropagationData(),
renderOpts.experimental.clientTraceMetadata
)

const polyfills: JSX.IntrinsicElements['script'][] =
buildManifest.polyfillFiles
.filter(
Expand Down Expand Up @@ -995,9 +1003,7 @@ async function renderToHTMLOrFlightImpl(
renderServerInsertedHTML,
serverCapturedErrors: allCapturedErrors,
basePath: renderOpts.basePath,
traceData: renderOpts.experimental.clientTraceMetadata
? getTracer().getTracePropagationData()
: [],
tracingMetadata: tracingMetadata,
})

const renderer = createStaticRenderer({
Expand Down Expand Up @@ -1322,9 +1328,7 @@ async function renderToHTMLOrFlightImpl(
renderServerInsertedHTML,
serverCapturedErrors: [],
basePath: renderOpts.basePath,
traceData: renderOpts.experimental.clientTraceMetadata
? getTracer().getTracePropagationData()
: [],
tracingMetadata: tracingMetadata,
}),
serverInsertedHTMLToHead: true,
validateRootLayout,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,24 @@ import { RedirectStatusCode } from '../../client/components/redirect-status-code
import { addPathPrefix } from '../../shared/lib/router/utils/add-path-prefix'
import type { ClientTraceDataEntry } from '../lib/trace/tracer'

export function getTracedMetadata(
traceData: ClientTraceDataEntry[],
clientTraceMetadata: string[] | undefined
): ClientTraceDataEntry[] | undefined {
if (!clientTraceMetadata) return undefined
return traceData.filter(({ key }) => clientTraceMetadata.includes(key))
}

export function makeGetServerInsertedHTML({
polyfills,
renderServerInsertedHTML,
serverCapturedErrors,
traceData,
tracingMetadata,
basePath,
}: {
polyfills: JSX.IntrinsicElements['script'][]
renderServerInsertedHTML: () => React.ReactNode
traceData: ClientTraceDataEntry[]
tracingMetadata: ClientTraceDataEntry[] | undefined
serverCapturedErrors: Error[]
basePath: string
}) {
Expand Down Expand Up @@ -84,14 +92,17 @@ export function makeGetServerInsertedHTML({
return <script key={polyfill.src} {...polyfill} />
})
}
{traceData.map(({ key, value }) => {
// The meta-tag name is prefixed with "_next-trace-data-" for mainly two reasons:
// - Avoid collisions with meta tags created by the user
// - Allow clients that want to propagate _all_ trace data to explicitly query for meta tags prefixed with "_next-trace-data-"
return (
<meta key={key} name={`_next-trace-data-${key}`} content={value} />
)
})}
{tracingMetadata
? tracingMetadata.map(({ key, value }) => {
return (
<meta
key={`next-trace-data-${key}:${value}`}
name={key}
content={value}
/>
)
})
: null}
{errorMetaTags}
</>,
{
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/app-render/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export interface RenderOptsPartial {
*/
isRoutePPREnabled?: boolean
swrDelta: SwrDelta | undefined
clientTraceMetadata: boolean
clientTraceMetadata: string[] | undefined
}
postponed?: string
/**
Expand Down
3 changes: 1 addition & 2 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -556,8 +556,7 @@ export default abstract class Server<
experimental: {
isAppPPREnabled,
swrDelta: this.nextConfig.experimental.swrDelta,
clientTraceMetadata:
this.nextConfig.experimental.clientTraceMetadata === true,
clientTraceMetadata: this.nextConfig.experimental.clientTraceMetadata,
},
}

Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/config-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ export const configSchema: zod.ZodType<NextConfig> = z.lazy(() =>
optimizePackageImports: z.array(z.string()).optional(),
optimizeServerReact: z.boolean().optional(),
instrumentationHook: z.boolean().optional(),
clientTraceMetadata: z.boolean().optional(),
clientTraceMetadata: z.array(z.string()).optional(),
turbotrace: z
.object({
logLevel: z
Expand Down
6 changes: 3 additions & 3 deletions packages/next/src/server/config-shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,9 @@ export interface ExperimentalConfig {
instrumentationHook?: boolean

/**
* Enables the propagation of trace data through meta tags to the client.
* The array of the meta tags to the client injected by tracing propagation data.
*/
clientTraceMetadata?: boolean
clientTraceMetadata?: string[]

/**
* Using this feature will enable the `react@experimental` for the `app` directory.
Expand Down Expand Up @@ -924,7 +924,7 @@ export const defaultConfig: NextConfig = {
turbotrace: undefined,
typedRoutes: false,
instrumentationHook: false,
clientTraceMetadata: false,
clientTraceMetadata: undefined,
bundlePagesExternals: false,
parallelServerCompiles: false,
parallelServerBuildTraces: false,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,41 +1,34 @@
import { nextTestSetup } from 'e2e-utils'

describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
describe('clientTraceMetadata (ppr=%p)', () => {
const { next, isNextDev } = nextTestSetup({
files: __dirname,
dependencies: require('./package.json').dependencies,
nextConfig: {
experimental: {
instrumentationHook: true,
clientTraceMetadata: true,
ppr: pprEnabled,
},
},
})

it('should inject propagation data for a dynamically server-side-rendered page', async () => {
const $ = await next.render$('/dynamic-page')
const headHtml = $.html('head')
expect(headHtml).toContain(
'<meta name="_next-trace-data-my-test-key-1" content="my-test-value-1">'
'<meta name="my-test-key-1" content="my-test-value-1">'
)
expect($.html('head')).toContain(
'<meta name="_next-trace-data-my-test-key-2" content="my-test-value-2">'
'<meta name="my-test-key-2" content="my-test-value-2">'
)
expect($.html('head')).toMatch(
/<meta name="_next-trace-data-my-parent-span-id" content="[a-f0-9]{16}">/
/<meta name="my-parent-span-id" content="[a-f0-9]{16}">/
)
})

it('hard loading a dynamic page twice should yield different dynamic trace data', async () => {
const browser1 = await next.browser('/dynamic-page')
const firstLoadSpanIdContent = await browser1
.elementByCss('meta[name="_next-trace-data-my-parent-span-id"]')
.elementByCss('meta[name="my-parent-span-id"]')
.getAttribute('content')

const browser2 = await next.browser('/dynamic-page')
const secondLoadSpanIdContent = await browser2
.elementByCss('meta[name="_next-trace-data-my-parent-span-id"]')
.elementByCss('meta[name="my-parent-span-id"]')
.getAttribute('content')

expect(firstLoadSpanIdContent).toMatch(/[a-f0-9]{16}/)
Expand All @@ -49,21 +42,21 @@ describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
const $ = await next.render$('/static-page')
const headHtml = $.html('head')
expect(headHtml).toContain(
'<meta name="_next-trace-data-my-test-key-1" content="my-test-value-1">'
'<meta name="my-test-key-1" content="my-test-value-1">'
)
expect($.html('head')).toContain(
'<meta name="_next-trace-data-my-test-key-2" content="my-test-value-2">'
'<meta name="my-test-key-2" content="my-test-value-2">'
)
expect($.html('head')).toMatch(
/<meta name="_next-trace-data-my-parent-span-id" content="[a-f0-9]{16}">/
/<meta name="my-parent-span-id" content="[a-f0-9]{16}">/
)
})

it('soft navigating to a dynamic page should not transform previous propagation data', async () => {
const browser = await next.browser('/static-page')

const initialSpanIdTagContent = await browser
.elementByCss('meta[name="_next-trace-data-my-parent-span-id"]')
.elementByCss('meta[name="my-parent-span-id"]')
.getAttribute('content')

// We are in dev mode so the static page should contain propagation data
Expand All @@ -73,7 +66,7 @@ describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
await browser.elementByCss('#dynamic-page-header')

const updatedSpanIdTagContent = await browser
.elementByCss('meta[name="_next-trace-data-my-parent-span-id"]')
.elementByCss('meta[name="my-parent-span-id"]')
.getAttribute('content')

expect(initialSpanIdTagContent).toBe(updatedSpanIdTagContent)
Expand All @@ -83,7 +76,7 @@ describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
const browser = await next.browser('/static-page')

const initialSpanIdTagContent = await browser
.elementByCss('meta[name="_next-trace-data-my-parent-span-id"]')
.elementByCss('meta[name="my-parent-span-id"]')
.getAttribute('content')

// We are in dev mode so the static page should contain propagation data
Expand All @@ -93,7 +86,7 @@ describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
await browser.elementByCss('#static-page-2-header')

const updatedSpanIdTagContent = await browser
.elementByCss('meta[name="_next-trace-data-my-parent-span-id"]')
.elementByCss('meta[name="my-parent-span-id"]')
.getAttribute('content')

expect(initialSpanIdTagContent).toBe(updatedSpanIdTagContent)
Expand All @@ -105,13 +98,13 @@ describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
const $ = await next.render$('/static-page')
const headHtml = $.html('head')
expect(headHtml).not.toContain(
'<meta name="_next-trace-data-my-test-key-1" content="my-test-value-1">'
'<meta name="my-test-key-1" content="my-test-value-1">'
)
expect($.html('head')).not.toContain(
'<meta name="_next-trace-data-my-test-key-2" content="my-test-value-2">'
'<meta name="my-test-key-2" content="my-test-value-2">'
)
expect($.html('head')).not.toMatch(
/<meta name="_next-trace-data-my-parent-span-id" content="[a-f0-9]{16}">/
/<meta name="my-parent-span-id" content="[a-f0-9]{16}">/
)
})

Expand All @@ -121,7 +114,7 @@ describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
await browser.elementByCss('#static-page-header')

const initialSpanIdTag = await browser.eval(
'document.querySelector(\'meta[name="_next-trace-data-my-parent-span-id"]\')'
'document.querySelector(\'meta[name="my-parent-span-id"]\')'
)

// We are in prod mode so we are not expecting propagation data to be present for a static page
Expand All @@ -131,7 +124,7 @@ describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
await browser.elementByCss('#dynamic-page-header')

const updatedSpanIdTag = await browser.eval(
'document.querySelector(\'meta[name="_next-trace-data-my-parent-span-id"]\')'
'document.querySelector(\'meta[name="my-parent-span-id"]\')'
)

// After the navigation to the dynamic page, there should still be no meta tag with propagation data
Expand All @@ -144,7 +137,7 @@ describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
await browser.elementByCss('#static-page-header')

const initialSpanIdTag = await browser.eval(
'document.querySelector(\'meta[name="_next-trace-data-my-parent-span-id"]\')'
'document.querySelector(\'meta[name="my-parent-span-id"]\')'
)

// We are in prod mode so we are not expecting propagation data to be present for a static page
Expand All @@ -154,7 +147,7 @@ describe.each([false, true])('clientTraceMetadata (ppr=%p)', (pprEnabled) => {
await browser.elementByCss('#static-page-2-header')

const updatedSpanIdTag = await browser.eval(
'document.querySelector(\'meta[name="_next-trace-data-my-parent-span-id"]\')'
'document.querySelector(\'meta[name="my-parent-span-id"]\')'
)

// After the navigation to the dynamic page, there should still be no meta tag with propagation data
Expand Down
10 changes: 10 additions & 0 deletions test/e2e/opentelemetry/client-trace-metadata/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module.exports = {
experimental: {
instrumentationHook: true,
clientTraceMetadata: [
'my-parent-span-id',
'my-test-key-1',
'my-test-key-2',
],
},
}

0 comments on commit e430d57

Please sign in to comment.