Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more spans into OTEL instrumentation to wrap all user defined functions #47368

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f325c16
Add rendering in SSR as a span
jankaifer Mar 21, 2023
c937f59
reorder spans to be sorted by their span type
jankaifer Mar 21, 2023
ea5b1c5
fix ordering of spans in tests
jankaifer Mar 21, 2023
250030b
Switch rendering to show only when user code is ran
jankaifer Mar 21, 2023
8e9395d
add proper rendering span for app directory
jankaifer Mar 21, 2023
316adfa
refactored tests and add test for app api handlers
jankaifer Mar 21, 2023
06e7064
Wrap pages api routes with span
jankaifer Mar 21, 2023
3a546e1
add span around api handlers
jankaifer Mar 21, 2023
d69189d
fix loading example
jankaifer Mar 21, 2023
00457b5
Add generate metadata span
jankaifer Mar 21, 2023
a06326d
Add generate metadata to layout and page
jankaifer Mar 21, 2023
616f0d9
Merge remote-tracking branch 'origin/canary' into jankaifer/next-857-…
jankaifer Mar 21, 2023
41d60bb
remove denpendency on `path`
jankaifer Mar 21, 2023
467600c
remove findPageComponent span
jankaifer Mar 21, 2023
10e35db
Added a proper name for generateMetadata
jankaifer Mar 21, 2023
6d9bb29
rename pathname to route
jankaifer Mar 21, 2023
4004295
regenerate snapshots
jankaifer Mar 21, 2023
377f7e4
remove debug log
jankaifer Mar 21, 2023
e38fa69
move test paths behind param to distinguish pathname from routes
jankaifer Mar 21, 2023
c34d5a1
Added span for getStaticProps
jankaifer Mar 21, 2023
55f50bc
fix naming to adhere to page vs route distinction
jankaifer Mar 21, 2023
e1af7ef
Merge branch 'canary' into jankaifer/next-857-wrap-all-user-code-in-i…
jankaifer Mar 21, 2023
9759f6c
Merge branch 'canary' into jankaifer/next-857-wrap-all-user-code-in-i…
jankaifer Mar 22, 2023
0577ee4
Merge branch 'canary' into jankaifer/next-857-wrap-all-user-code-in-i…
kodiakhq[bot] Mar 22, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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,
}