Skip to content

Commit

Permalink
Add more spans into OTEL instrumentation to wrap all user defined fun…
Browse files Browse the repository at this point in the history
…ctions (vercel#47368)

- Move span wrapping rendering closer to the user code and don't add
span when we have cache-hit
- Add `getStaticProps` span
- Add spans around API handlers (pages and app)
- Add `generateMetadata` span
- Clarify naming that we use `page` for entrypoints like
`/path/[param]/page` or `/path/[param]/layout`. And `route` for
`/path/[param]`

fix NEXT-857 ([link](https://linear.app/vercel/issue/NEXT-857))
  • Loading branch information
jankaifer authored Mar 22, 2023
1 parent 8195e19 commit c04ca8d
Show file tree
Hide file tree
Showing 25 changed files with 9,599 additions and 301 deletions.
9,118 changes: 9,111 additions & 7 deletions .github/actions/issue-labeler/lib/index.js

Large diffs are not rendered by default.

42 changes: 34 additions & 8 deletions packages/next/src/lib/metadata/resolve-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import {
resolveViewport,
} from './resolvers/resolve-basics'
import { resolveIcons } from './resolvers/resolve-icons'
import { getTracer } from '../../server/lib/trace/tracer'
import { ResolveMetadataSpan } from '../../server/lib/trace/constants'

type StaticMetadata = Awaited<ReturnType<typeof resolveStaticMetadata>>

Expand Down Expand Up @@ -181,7 +183,8 @@ function merge(

async function getDefinedMetadata(
mod: any,
props: any
props: any,
route: string
): Promise<Metadata | MetadataResolver | null> {
// Layer is a client component, we just skip it. It can't have metadata exported.
// Return early to avoid accessing properties error for client references.
Expand All @@ -190,7 +193,17 @@ async function getDefinedMetadata(
}
return (
(mod.generateMetadata
? (parent: ResolvingMetadata) => mod.generateMetadata(props, parent)
? (parent: ResolvingMetadata) =>
getTracer().trace(
ResolveMetadataSpan.generateMetadata,
{
spanName: `generateMetadata ${route}`,
attributes: {
'next.route': route,
},
},
() => mod.generateMetadata(props, parent)
)
: mod.metadata) || null
)
}
Expand Down Expand Up @@ -231,14 +244,27 @@ async function resolveStaticMetadata(components: ComponentsType) {
}

// [layout.metadata, static files metadata] -> ... -> [page.metadata, static files metadata]
export async function collectMetadata(
loaderTree: LoaderTree,
props: any,
export async function collectMetadata({
loaderTree,
props,
array,
route,
}: {
loaderTree: LoaderTree
props: any
array: MetadataItems
) {
const mod = await getLayoutOrPageModule(loaderTree)
route: string
}) {
const [mod, modType] = await getLayoutOrPageModule(loaderTree)

if (modType) {
route += `/${modType}`
}

const staticFilesMetadata = await resolveStaticMetadata(loaderTree[2])
const metadataExport = mod ? await getDefinedMetadata(mod, props) : null
const metadataExport = mod
? await getDefinedMetadata(mod, props, route)
: null

array.push([metadataExport, staticFilesMetadata])
}
Expand Down
10 changes: 9 additions & 1 deletion packages/next/src/server/api-utils/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import {
RESPONSE_LIMIT_DEFAULT,
} from './index'
import { mockRequest } from '../lib/mock-request'
import { getTracer } from '../lib/trace/tracer'
import { NodeSpan } from '../lib/trace/constants'

export function tryGetPreviewData(
req: IncomingMessage | BaseNextRequest,
Expand Down Expand Up @@ -528,7 +530,13 @@ export async function apiResolver(
}

// Call API route method
await resolver(req, res)
await getTracer().trace(
NodeSpan.runHandler,
{
spanName: `executing api route (pages) ${page}`,
},
() => resolver(req, res)
)

if (
process.env.NODE_ENV !== 'production' &&
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { LoaderTree } from '../lib/app-dir-module'
import { FlightRouterState, Segment } from './types'
import { GetDynamicParamFromSegment } from './index'

// TODO-APP: Move __PAGE__ to a shared constant
const PAGE_SEGMENT_KEY = '__PAGE__'
import { PAGE_SEGMENT_KEY } from '../../shared/lib/constants'

export function addSearchParamsIfPageSegment(
segment: Segment,
Expand Down
61 changes: 43 additions & 18 deletions packages/next/src/server/app-render/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import {
addSearchParamsIfPageSegment,
createFlightRouterStateFromLoaderTree,
} from './create-flight-router-state-from-loader-tree'
import { PAGE_SEGMENT_KEY } from '../../shared/lib/constants'

export const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge'

Expand Down Expand Up @@ -279,12 +280,20 @@ export async function renderToHTMLOrFlight(
}
}

async function resolveHead(
tree: LoaderTree,
parentParams: { [key: string]: any },
async function resolveHead({
tree,
parentParams,
metadataItems,
treePrefix = [],
}: {
tree: LoaderTree
parentParams: { [key: string]: any }
metadataItems: MetadataItems
): Promise<[React.ReactNode, MetadataItems]> {
/** Provided tree can be nested subtree, this argument says what is the path of such subtree */
treePrefix?: string[]
}): Promise<[React.ReactNode, MetadataItems]> {
const [segment, parallelRoutes, { head, page }] = tree
const currentTreePrefix = [...treePrefix, segment]
const isPage = typeof page !== 'undefined'
// Handle dynamic segment params.
const segmentParam = getDynamicParamFromSegment(segment)
Expand All @@ -306,15 +315,24 @@ export async function renderToHTMLOrFlight(
...(isPage && searchParamsProps),
}

await collectMetadata(tree, layerProps, metadataItems)
await collectMetadata({
loaderTree: tree,
props: layerProps,
array: metadataItems,
route: currentTreePrefix
// __PAGE__ shouldn't be shown in a route
.filter((s) => s !== PAGE_SEGMENT_KEY)
.join('/'),
})

for (const key in parallelRoutes) {
const childTree = parallelRoutes[key]
const [returnedHead] = await resolveHead(
childTree,
currentParams,
metadataItems
)
const [returnedHead] = await resolveHead({
tree: childTree,
parentParams: currentParams,
metadataItems,
treePrefix: currentTreePrefix,
})
if (returnedHead) {
return [returnedHead, metadataItems]
}
Expand Down Expand Up @@ -473,7 +491,7 @@ export async function renderToHTMLOrFlight(

const isLayout = typeof layout !== 'undefined'
const isPage = typeof page !== 'undefined'
const layoutOrPageMod = await getLayoutOrPageModule(tree)
const [layoutOrPageMod] = await getLayoutOrPageModule(tree)

/**
* Checks if the current segment is a root layout.
Expand Down Expand Up @@ -979,11 +997,11 @@ export async function renderToHTMLOrFlight(
return [actualSegment]
}

const [resolvedHead, metadataItems] = await resolveHead(
loaderTree,
{},
[]
)
const [resolvedHead, metadataItems] = await resolveHead({
tree: loaderTree,
parentParams: {},
metadataItems: [],
})
// Flight data that is going to be passed to the browser.
// Currently a single item array but in the future multiple patches might be combined in a single request.
const flightData: FlightData = [
Expand Down Expand Up @@ -1074,7 +1092,11 @@ export async function renderToHTMLOrFlight(
}
: {}

const [initialHead, metadataItems] = await resolveHead(loaderTree, {}, [])
const [initialHead, metadataItems] = await resolveHead({
tree: loaderTree,
parentParams: {},
metadataItems: [],
})

/**
* A new React Component that renders the provided React Component
Expand Down Expand Up @@ -1156,6 +1178,9 @@ export async function renderToHTMLOrFlight(

const bodyResult = getTracer().wrap(
AppRenderSpan.getBodyResult,
{
spanName: `render route (app) ${pathname}`,
},
async ({
asNotFound,
}: {
Expand Down Expand Up @@ -1314,7 +1339,7 @@ export async function renderToHTMLOrFlight(
}
)

// For action requests, we handle them differently with a sepcial render result.
// For action requests, we handle them differently with a special render result.
let actionId = req.headers[ACTION.toLowerCase()] as string
const isFormAction =
req.method === 'POST' &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import { AppConfig } from '../../../build/utils'
import { RequestCookies } from 'next/dist/compiled/@edge-runtime/cookies'
import { NextURL } from '../../web/next-url'
import { NextConfig } from '../../config-shared'
import { getTracer } from '../../lib/trace/tracer'
import { AppRouteRouteHandlersSpan } from '../../lib/trace/constants'
import { AppRouteRouteDefinition } from '../route-definitions/app-route-route-definition'
import { WebNextRequest } from '../../base-http/web'

Expand Down Expand Up @@ -296,6 +298,20 @@ function proxyRequest(
})
}

function getPathnameFromAbsolutePath(absolutePath: string) {
// Remove prefix including app dir
let appDir = '/app/'
if (!absolutePath.includes(appDir)) {
appDir = '\\app\\'
}
const [, ...parts] = absolutePath.split(appDir)
const relativePath = appDir[0] + parts.join(appDir)

// remove extension
const pathname = relativePath.split('.').slice(0, -1).join('.')
return pathname
}

/**
* Validate that the module is exporting methods supported by the handler.
*
Expand Down Expand Up @@ -532,7 +548,16 @@ export class AppRouteRouteHandler implements RouteHandler<AppRouteRouteMatch> {
module
)

return handle(wrappedRequest, { params })
return getTracer().trace(
AppRouteRouteHandlersSpan.runHandler,
{
// TODO: propagate this pathname from route matcher
spanName: `executing api route (app) ${getPathnameFromAbsolutePath(
module.resolvedPagePath
)}`,
},
() => handle(wrappedRequest, { params })
)
}
)
)
Expand Down
14 changes: 13 additions & 1 deletion packages/next/src/server/lib/app-dir-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,17 @@ export async function getLayoutOrPageModule(loaderTree: LoaderTree) {
const { layout, page } = loaderTree[2]
const isLayout = typeof layout !== 'undefined'
const isPage = typeof page !== 'undefined'
return isLayout ? await layout[0]() : isPage ? await page[0]() : undefined

let value = undefined
let modType: 'layout' | 'page' | undefined = undefined
if (isLayout) {
value = await layout[0]()
modType = 'layout'
}
if (isPage) {
value = await page[0]()
modType = 'page'
}

return [value, modType] as const
}
27 changes: 25 additions & 2 deletions packages/next/src/server/lib/trace/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ enum StartServerSpan {

enum RenderSpan {
getServerSideProps = 'Render.getServerSideProps',
getStaticProps = 'Render.getStaticProps',
renderToString = 'Render.renderToString',
renderDocument = 'Render.renderDocument',
createBodyResult = 'Render.createBodyResult',
Expand All @@ -88,6 +89,18 @@ enum RouterSpan {
executeRoute = 'Router.executeRoute',
}

enum NodeSpan {
runHandler = 'Node.runHandler',
}

enum AppRouteRouteHandlersSpan {
runHandler = 'AppRouteRouteHandlers.runHandler',
}

enum ResolveMetadataSpan {
generateMetadata = 'ResolveMetadata.generateMetadata',
}

type SpanTypes =
| `${BaseServerSpan}`
| `${LoadComponentsSpan}`
Expand All @@ -97,14 +110,21 @@ type SpanTypes =
| `${RenderSpan}`
| `${RouterSpan}`
| `${AppRenderSpan}`
| `${NodeSpan}`
| `${AppRouteRouteHandlersSpan}`
| `${ResolveMetadataSpan}`

// This list is used to filter out spans that are not relevant to the user
export const NextVanillaSpanAllowlist = [
BaseServerSpan.handleRequest,
NextNodeServerSpan.findPageComponents,
BaseServerSpan.renderToResponse,
RenderSpan.getServerSideProps,
RenderSpan.getStaticProps,
AppRenderSpan.fetch,
AppRenderSpan.getBodyResult,
RenderSpan.renderDocument,
NodeSpan.runHandler,
AppRouteRouteHandlersSpan.runHandler,
ResolveMetadataSpan.generateMetadata,
]

export {
Expand All @@ -117,4 +137,7 @@ export {
RenderSpan,
RouterSpan,
AppRenderSpan,
NodeSpan,
AppRouteRouteHandlersSpan,
ResolveMetadataSpan,
}
Loading

0 comments on commit c04ca8d

Please sign in to comment.