Skip to content

Commit

Permalink
Trace upload: only send traces for current session (#71838)
Browse files Browse the repository at this point in the history
The `.next/trace` file contains trace events spanning multiple traces
from different sessions. This changes our upload process to only send
trace events for the current session that’s ending.

Before this change, we would send a single set of metadata for many
traces, but these older session traces could have been invoked with
different parameters like `mode`, `isTurboSession`, etc. Because of
this, we started including `isTurboSession` as a span attribute on
spans, but this value was dependent on an environment variable that
wasn’t always set. In this case, the top-level `next-dev` span never had
the correct value.

This makes it so the metadata, now always determined at upload time,
always reflects the session sent, and never any others.

Other bugs fixed:

- Trace events within a single logical session had different trace ids
because of worker threads. This aligns them.
- Because we upload traces out-of-process, the Telemetry `sessionId` did
not reflect the real session id of other analytics sent. This passes it
along to the external process.

Test Plan:

In a test app, using `NEXT_TRACE_UPLOAD_DEBUG=1` and a running Otel
server:
- Start a non-turbopack dev session and end it. Verify trace events are
sent with `isTurboSession: false` and note the trace id.
- Start a turbopack dev session and end it. Verify trace events are sent
with `isTurboSession: true` and note events are sent for this trace, and
not the previous one.
- Start a build and verify trace events are sent with `mode: build` and
`isTurboSession: false` and that no older trace events are sent.


Closes PACK-3338
  • Loading branch information
wbinnssmith authored Oct 25, 2024
1 parent f9b281a commit 6ee3d4b
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 29 deletions.
1 change: 1 addition & 0 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3865,6 +3865,7 @@ export default async function build(
mode: 'build',
projectDir: dir,
distDir: loadedConfig.distDir,
isTurboSession: turboNextBuild,
sync: true,
})
}
Expand Down
4 changes: 4 additions & 0 deletions packages/next/src/cli/next-dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import os from 'os'
import { once } from 'node:events'
import { clearTimeout } from 'timers'
import { flushAllTraces, trace } from '../trace'
import { traceId } from '../trace/shared'

export type NextDevOptions = {
turbo?: boolean
Expand Down Expand Up @@ -137,6 +138,7 @@ const handleSessionStop = async (signal: NodeJS.Signals | number | null) => {
mode: 'dev',
projectDir: dir,
distDir: config.distDir,
isTurboSession,
})
}

Expand Down Expand Up @@ -277,6 +279,7 @@ const nextDev = async (
...defaultEnv,
TURBOPACK: process.env.TURBOPACK,
NEXT_PRIVATE_WORKER: '1',
NEXT_PRIVATE_TRACE_ID: traceId,
NODE_EXTRA_CA_CERTS: startServerOptions.selfSignedCertificate
? startServerOptions.selfSignedCertificate.rootCA
: defaultEnv.NODE_EXTRA_CA_CERTS,
Expand Down Expand Up @@ -309,6 +312,7 @@ const nextDev = async (
mode: 'dev',
projectDir: dir,
distDir: config.distDir,
isTurboSession,
sync: true,
})
}
Expand Down
8 changes: 1 addition & 7 deletions packages/next/src/trace/report/to-json.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { randomBytes } from 'crypto'
import { traceGlobals } from '../shared'
import { traceGlobals, traceId } from '../shared'
import fs from 'fs'
import path from 'path'
import { PHASE_DEVELOPMENT_SERVER } from '../../shared/lib/constants'
Expand Down Expand Up @@ -44,7 +43,6 @@ export function batcher(reportEvents: (evts: Event[]) => Promise<void>) {
}

let writeStream: RotatingWriteStream
let traceId: string
let batch: ReturnType<typeof batcher> | undefined

const writeStreamOptions = {
Expand Down Expand Up @@ -117,10 +115,6 @@ const reportToLocalHost = (event: TraceEvent) => {
return
}

if (!traceId) {
traceId = process.env.TRACE_ID || randomBytes(8).toString('hex')
}

if (!batch) {
batch = batcher(async (events: Event[]) => {
if (!writeStream) {
Expand Down
7 changes: 7 additions & 0 deletions packages/next/src/trace/shared.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { randomBytes } from 'node:crypto'

let _traceGlobals: Map<any, any> = (global as any)._traceGlobals

if (!_traceGlobals) {
Expand All @@ -9,3 +11,8 @@ export const traceGlobals: Map<any, any> = _traceGlobals
export const setGlobal = (key: any, val: any) => {
traceGlobals.set(key, val)
}

export const traceId =
process.env.TRACE_ID ||
process.env.NEXT_PRIVATE_TRACE_ID ||
randomBytes(8).toString('hex')
44 changes: 26 additions & 18 deletions packages/next/src/trace/trace-uploader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import os from 'os'
import { createInterface } from 'readline'
import { createReadStream } from 'fs'
import path from 'path'
import { Telemetry } from '../telemetry/storage'

const COMMON_ALLOWED_EVENTS = ['memory-usage']

Expand Down Expand Up @@ -66,7 +65,19 @@ const {
const isDebugEnabled = !!NEXT_TRACE_UPLOAD_DEBUG || !!NEXT_TRACE_UPLOAD_FULL
const shouldUploadFullTrace = !!NEXT_TRACE_UPLOAD_FULL

const [, , traceUploadUrl, mode, projectDir, distDir] = process.argv
const [
,
,
traceUploadUrl,
mode,
projectDir,
distDir,
_isTurboSession,
traceId,
anonymousId,
sessionId,
] = process.argv
const isTurboSession = _isTurboSession === 'true'

type TraceRequestBody = {
metadata: TraceMetadata
Expand Down Expand Up @@ -105,8 +116,6 @@ interface TraceMetadata {
)
).version

const telemetry = new Telemetry({ distDir })

const projectPkgJsonPath = await findUp('package.json')
assert(projectPkgJsonPath)

Expand All @@ -129,11 +138,15 @@ interface TraceMetadata {
crlfDelay: Infinity,
})

let isTurboSession = false
const traces = new Map<string, TraceEvent[]>()
const sessionTrace = []
for await (const line of readLineInterface) {
const lineEvents: TraceEvent[] = JSON.parse(line)
for (const event of lineEvents) {
if (event.traceId !== traceId) {
// Only consider events for the current session
continue
}

if (
// Always include root spans
event.parentId === undefined ||
Expand All @@ -142,22 +155,14 @@ interface TraceMetadata {
? DEV_ALLOWED_EVENTS.has(event.name)
: BUILD_ALLOWED_EVENTS.has(event.name))
) {
let trace = traces.get(event.traceId)
if (trace === undefined) {
trace = []
traces.set(event.traceId, trace)
}
if (typeof event.tags.isTurbopack === 'boolean') {
isTurboSession = event.tags.isTurbopack
}
trace.push(event)
sessionTrace.push(event)
}
}
}

const body: TraceRequestBody = {
metadata: {
anonymousId: telemetry.anonymousId,
anonymousId,
arch: os.arch(),
commit,
cpus: os.cpus().length,
Expand All @@ -166,9 +171,12 @@ interface TraceMetadata {
nextVersion,
pkgName,
platform: os.platform(),
sessionId: telemetry.sessionId,
sessionId,
},
traces: [...traces.values()],
// The trace file can contain events spanning multiple sessions.
// Only submit traces for the current session, as the metadata we send is
// intended for this session only.
traces: [sessionTrace],
}

if (isDebugEnabled) {
Expand Down
4 changes: 0 additions & 4 deletions packages/next/src/trace/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ export class Span {
this.name = name
this.parentId = parentId ?? defaultParentSpanId
this.attrs = attrs ? { ...attrs } : {}
if (this.parentId === undefined) {
// Attach additional information to root spans
this.attrs.isTurbopack = Boolean(process.env.TURBOPACK)
}

this.status = SpanStatus.Started
this.id = getId()
Expand Down
10 changes: 10 additions & 0 deletions packages/next/src/trace/upload-trace.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
import { traceId } from './shared'
import { Telemetry } from '../telemetry/storage'

export default function uploadTrace({
traceUploadUrl,
mode,
projectDir,
distDir,
isTurboSession,
sync,
}: {
traceUploadUrl: string
mode: 'dev' | 'build'
projectDir: string
distDir: string
isTurboSession: boolean
sync?: boolean
}) {
const { NEXT_TRACE_UPLOAD_DEBUG } = process.env
const telemetry = new Telemetry({ distDir })

// Note: cross-spawn is not used here as it causes
// a new command window to appear when we don't want it to
Expand All @@ -33,6 +39,10 @@ export default function uploadTrace({
mode,
projectDir,
distDir,
String(isTurboSession),
traceId,
telemetry.anonymousId,
telemetry.sessionId,
],
{
detached: !NEXT_TRACE_UPLOAD_DEBUG,
Expand Down

0 comments on commit 6ee3d4b

Please sign in to comment.