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 http.route into root otel span #47392

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 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
6c6cc4e
Add route into root component
jankaifer Mar 22, 2023
b807d0e
Merge remote-tracking branch 'origin/canary' into jankaifer/next-837-…
jankaifer Mar 22, 2023
0200ac2
fix wrong span attribute in renderToResponse
jankaifer Mar 22, 2023
7b40636
Update packages/next/src/server/base-server.ts
jankaifer Mar 22, 2023
ab6b7cb
add missing otel attribute names net.
jankaifer Mar 22, 2023
dea9a06
fix next attribute name
jankaifer Mar 22, 2023
36fbc19
add check when tracing is not enabled
jankaifer Mar 22, 2023
3198279
don't throw when we haven't found root span attributes
jankaifer Mar 22, 2023
4d0c596
Cleanup rootSpanAttributesStore entries when span finishes
jankaifer Mar 23, 2023
11aa1a5
Merge branch 'canary' into jankaifer/next-837-propagate-route-into-ro…
jankaifer Mar 23, 2023
96b8676
Merge branch 'canary' into jankaifer/next-837-propagate-route-into-ro…
jankaifer Mar 23, 2023
8ca6b90
fix cleanup to handle async and sync separately
jankaifer Mar 23, 2023
63cf728
Merge branch 'canary' into jankaifer/next-837-propagate-route-into-ro…
jankaifer Mar 23, 2023
51ec2f6
Merge branch 'canary' into jankaifer/next-837-propagate-route-into-ro…
jankaifer Mar 24, 2023
9577f44
Merge branch 'canary' into jankaifer/next-837-propagate-route-into-ro…
jankaifer Mar 24, 2023
8fc0eb1
Merge branch 'canary' into jankaifer/next-837-propagate-route-into-ro…
jankaifer Mar 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -520,22 +520,48 @@ export default abstract class Server<ServerOptions extends Options = Options> {
res: BaseNextResponse,
parsedUrl?: NextUrlWithParsedQuery
): Promise<void> {
const method = req.method.toUpperCase()
return getTracer().trace(
BaseServerSpan.handleRequest,
{
spanName: [req.method, req.url].join(' '),
spanName: `${method} ${req.url}`,
kind: SpanKind.SERVER,
attributes: {
'http.method': req.method,
'http.method': method,
'http.target': req.url,
},
},
async (span) =>
this.handleRequestImpl(req, res, parsedUrl).finally(() =>
span?.setAttributes({
this.handleRequestImpl(req, res, parsedUrl).finally(() => {
if (!span) return
span.setAttributes({
'http.status_code': res.statusCode,
})
)
const rootSpanAttributes = getTracer().getRootSpanAttributes()
// We were unable to get attributes, probably OTEL is not enabled
if (!rootSpanAttributes) return

if (
rootSpanAttributes.get('next.span_type') !==
BaseServerSpan.handleRequest
) {
console.warn(
`Unexpected root span type '${rootSpanAttributes.get(
'next.span_type'
)}'. Please report this Next.js issue https://github.com/vercel/next.js`
)
return
}

const route = rootSpanAttributes.get('next.route')
if (route) {
span.setAttributes({
'next.route': route,
'http.route': route,
})
span.updateName(`${method} ${route}`)
}
})
)
}

Expand Down Expand Up @@ -1998,7 +2024,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
{
spanName: `rendering page`,
attributes: {
'next.pathname': ctx.pathname,
'next.route': ctx.pathname,
},
},
async () => {
Expand Down
121 changes: 86 additions & 35 deletions packages/next/src/server/lib/trace/tracer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { NextVanillaSpanAllowlist, SpanTypes } from './constants'

import type { ContextAPI, Span, SpanOptions, Tracer } from '@opentelemetry/api'
import type {
ContextAPI,
Span,
SpanOptions,
Tracer,
AttributeValue,
} from '@opentelemetry/api'

let api: typeof import('@opentelemetry/api')

Expand Down Expand Up @@ -31,9 +37,10 @@ const closeSpanWithError = (span: Span, error?: Error) => {
span.end()
}

type TracerSpanOptions = SpanOptions & {
type TracerSpanOptions = Omit<SpanOptions, 'attributes'> & {
parentSpan?: Span
spanName?: string
attributes?: Partial<Record<AttributeNames, AttributeValue | undefined>>
}

interface NextTracer {
Expand Down Expand Up @@ -117,6 +124,23 @@ interface NextTracer {
getActiveScopeSpan(): Span | undefined
}

type NextAttributeNames =
| 'next.route'
| 'next.page'
| 'next.span_name'
| 'next.span_type'
type OTELAttributeNames = `http.${string}` | `net.${string}`
type AttributeNames = NextAttributeNames | OTELAttributeNames

/** we use this map to propagate attributes from nested spans to the top span */
const rootSpanAttributesStore = new Map<
jankaifer marked this conversation as resolved.
Show resolved Hide resolved
number,
Map<AttributeNames, AttributeValue | undefined>
>()
const rootSpanIdKey = api.createContextKey('next.rootSpanId')
let lastSpanId = 0
const getSpanId = () => lastSpanId++

class NextTracerImpl implements NextTracer {
/**
* Returns an instance to the trace with configured name.
Expand Down Expand Up @@ -186,49 +210,71 @@ class NextTracerImpl implements NextTracer {
const spanName = options.spanName ?? type

// Trying to get active scoped span to assign parent. If option specifies parent span manually, will try to use it.
const spanContext = this.getSpanContext(
let spanContext = this.getSpanContext(
options?.parentSpan ?? this.getActiveScopeSpan()
)
let isRootSpan = false

if (!spanContext) {
spanContext = api.ROOT_CONTEXT
isRootSpan = true
}

const spanId = getSpanId()

options.attributes = {
'next.span_name': spanName,
'next.span_type': type,
...options.attributes,
}

const runWithContext = (actualFn: (span: Span) => T | Promise<T>) =>
spanContext
? this.getTracerInstance().startActiveSpan(
spanName,
options,
spanContext,
actualFn
)
: this.getTracerInstance().startActiveSpan(spanName, options, actualFn)

return runWithContext((span: Span) => {
try {
if (fn.length > 1) {
return fn(span, (err?: Error) => closeSpanWithError(span, err))
}

const result = fn(span)

if (isPromise(result)) {
result.then(
() => span.end(),
(err) => closeSpanWithError(span, err)
)
} else {
span.end()
return api.context.with(spanContext.setValue(rootSpanIdKey, spanId), () =>
this.getTracerInstance().startActiveSpan(
spanName,
options,
(span: Span) => {
const onCleanup = () => {
rootSpanAttributesStore.delete(spanId)
}
if (isRootSpan) {
rootSpanAttributesStore.set(
spanId,
new Map(
Object.entries(options.attributes ?? {}) as [
AttributeNames,
AttributeValue | undefined
][]
)
)
}
try {
if (fn.length > 1) {
return fn(span, (err?: Error) => closeSpanWithError(span, err))
}

const result = fn(span)

if (isPromise(result)) {
result
.then(
() => span.end(),
(err) => closeSpanWithError(span, err)
)
.finally(onCleanup)
} else {
span.end()
onCleanup()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can put that in a finally {} clause

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't that will trigger for async branch too.
That was the issue before.

}

return result
} catch (err: any) {
closeSpanWithError(span, err)
onCleanup()
throw err
}
}

return result
} catch (err: any) {
closeSpanWithError(span, err)
throw err
}
})
)
)
}

public wrap<T = (...args: Array<any>) => any>(type: SpanTypes, fn: T): T
Expand Down Expand Up @@ -297,6 +343,11 @@ class NextTracerImpl implements NextTracer {

return spanContext
}

public getRootSpanAttributes() {
const spanId = context.active().getValue(rootSpanIdKey) as number
return rootSpanAttributesStore.get(spanId)
}
}

const getTracer = (() => {
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,7 @@ export default class NextNodeServer extends BaseServer {
params: Params | null
isAppPath: boolean
}): Promise<FindComponentsResult | null> {
getTracer().getRootSpanAttributes()?.set('next.route', pathname)
return getTracer().trace(
NextNodeServerSpan.findPageComponents,
{
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ export default class Router {
RouterSpan.executeRoute,
{
attributes: {
route: route.name,
'next.route': route.name,
},
},
route.fn
Expand Down
16 changes: 12 additions & 4 deletions test/e2e/opentelemetry/opentelemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,15 @@ createNextDescribe(
Object {
"attributes": Object {
"http.method": "GET",
"http.route": "/app/[param]/rsc-fetch/page",
"http.status_code": 200,
"http.target": "/app/param/rsc-fetch",
"next.route": "/app/[param]/rsc-fetch/page",
"next.span_name": "GET /app/param/rsc-fetch",
"next.span_type": "BaseServer.handleRequest",
},
"kind": 1,
"name": "GET /app/param/rsc-fetch",
"name": "GET /app/[param]/rsc-fetch/page",
"parentId": undefined,
"status": Object {
"code": 0,
Expand Down Expand Up @@ -178,13 +180,15 @@ createNextDescribe(
Object {
"attributes": Object {
"http.method": "GET",
"http.route": "/api/app/[param]/data/route",
"http.status_code": 200,
"http.target": "/api/app/param/data",
"next.route": "/api/app/[param]/data/route",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are the differences between Next and HTTP route?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be the same value.

But specs don't allow using http.route outside SERVER span.
So I use next.route there and it felt weird to not include it in the top span too.

"next.span_name": "GET /api/app/param/data",
"next.span_type": "BaseServer.handleRequest",
},
"kind": 1,
"name": "GET /api/app/param/data",
"name": "GET /api/app/[param]/data/route",
"parentId": undefined,
"status": Object {
"code": 0,
Expand All @@ -204,13 +208,15 @@ createNextDescribe(
Object {
"attributes": Object {
"http.method": "GET",
"http.route": "/pages/[param]/getServerSideProps",
"http.status_code": 200,
"http.target": "/pages/param/getServerSideProps",
"next.route": "/pages/[param]/getServerSideProps",
"next.span_name": "GET /pages/param/getServerSideProps",
"next.span_type": "BaseServer.handleRequest",
},
"kind": 1,
"name": "GET /pages/param/getServerSideProps",
"name": "GET /pages/[param]/getServerSideProps",
"parentId": undefined,
"status": Object {
"code": 0,
Expand Down Expand Up @@ -252,13 +258,15 @@ createNextDescribe(
Object {
"attributes": Object {
"http.method": "GET",
"http.route": "/pages/[param]/getStaticProps",
"http.status_code": 200,
"http.target": "/pages/param/getStaticProps",
"next.route": "/pages/[param]/getStaticProps",
"next.span_name": "GET /pages/param/getStaticProps",
"next.span_type": "BaseServer.handleRequest",
},
"kind": 1,
"name": "GET /pages/param/getStaticProps",
"name": "GET /pages/[param]/getStaticProps",
"parentId": undefined,
"status": Object {
"code": 0,
Expand Down