From 7e691ec38c157fa4af02dab9a0ee776f4dc73102 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Wed, 22 Jul 2026 14:40:08 +0200 Subject: [PATCH 1/2] fix: scope column packing to frames with a missing source map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit columnNumbers: 'pack' previously packed the column into the line for every frame. That leaked the packed value into profiles that skip server-side unminification (e.g. Node.js profiles whose maps resolved locally, or that never had missing maps), which in turn forced the backend to strip it back out of every parse-time structure. Pack only frames whose source map was declared but missing (loc.missingMapFile) — exactly the frames that set dd:has-missing-map-files and are sent for server-side unminification. Locally-resolved and no-map frames keep their plain line, so profiles that skip unminification never carry packed line numbers and the backend needs no stripping. This mirrors how the Chrome path already works. Co-Authored-By: Claude Opus 4.8 (1M context) --- ts/src/profile-serializer.ts | 11 ++- ts/test/test-profile-serializer.ts | 120 +++++++++++++++-------------- 2 files changed, 69 insertions(+), 62 deletions(-) diff --git a/ts/src/profile-serializer.ts b/ts/src/profile-serializer.ts index 3435dd1b..a79170c9 100644 --- a/ts/src/profile-serializer.ts +++ b/ts/src/profile-serializer.ts @@ -229,12 +229,15 @@ function serialize( } function getLine(loc: SourceLocation, scriptId?: number): Line { + // Only pack the column for frames whose source map was declared but missing + // locally — i.e. exactly the frames that will be sent for server-side + // unminification (the same condition that sets dd:has-missing-map-files). + // Locally-resolved frames keep their plain line, so packed values never + // reach profiles that skip server-side unminification. + const packColumn = columnNumbers === 'pack' && loc.missingMapFile === true; return new Line({ functionId: getFunction(loc, scriptId).id, - line: - columnNumbers === 'pack' - ? packLineAndColumn(loc.line, loc.column) - : loc.line, + line: packColumn ? packLineAndColumn(loc.line, loc.column) : loc.line, }); } diff --git a/ts/test/test-profile-serializer.ts b/ts/test/test-profile-serializer.ts index d4c7b9ea..c2c0b085 100644 --- a/ts/test/test-profile-serializer.ts +++ b/ts/test/test-profile-serializer.ts @@ -82,15 +82,13 @@ describe('profile-serializer', () => { assert.deepEqual(timeProfileOut, timeProfile); }); - it('should drop the column by default and pack it when columnNumbers is "pack"', () => { - // Regression test for SCP-1293: a bundled/minified module (e.g. a - // Next.js server chunk) emits every function onto generated line 1, so - // the column is the only discriminator between functions. The pprof Line - // message carries no dedicated column, so with columnNumbers='pack' the - // column is packed into the high 32 bits (column<<32 | line) — the - // encoding the Datadog deobfuscation backend decodes. The default 'drop' - // must leave the plain line untouched for backwards compatibility. - const alphaLeaf = { + it('does not pack the column for frames without a missing source map, even when columnNumbers is "pack"', () => { + // Packing is scoped to frames whose source map was declared but missing + // (dd:has-missing-map-files) — the ones bound for server-side + // unminification. A frame that never went through a missing map (here: + // no source mapper at all) keeps its plain line even under 'pack', so + // profiles that skip server-side unminification never carry packed lines. + const leaf = { name: 'alpha', scriptName: '/opt/app/.next/server/chunks/58844.js', scriptId: 1, @@ -99,15 +97,6 @@ describe('profile-serializer', () => { hitCount: 3, children: [], }; - const betaLeaf = { - name: 'beta', - scriptName: '/opt/app/.next/server/chunks/58844.js', - scriptId: 1, - lineNumber: 1, - columnNumber: 14349, - hitCount: 2, - children: [], - }; const bundleProfile: TimeProfile = { startTime: 0, endTime: 10 * 1000 * 1000, @@ -120,45 +109,32 @@ describe('profile-serializer', () => { lineNumber: 0, columnNumber: 0, hitCount: 0, - children: [alphaLeaf, betaLeaf], + children: [leaf], }, }; - const lineByName = (profile: Profile) => { - const m = new Map(); - for (const location of profile.location!) { - const line = location.line![0]; - const fn = getAndVerifyPresence( - profile.function!, - line.functionId as number, - ); - m.set( - profile.stringTable.strings[fn.name as number] as string, - line.line, - ); - } - return m; + const lineOf = (profile: Profile) => { + const location = profile.location![0]; + return BigInt(location.line![0].line); }; - // Default: column dropped, plain generated line 1 retained. - const dropped = lineByName(serializeTimeProfile(bundleProfile, 1000)); - assert.strictEqual(dropped.get('alpha'), 1); - assert.strictEqual(dropped.get('beta'), 1); - - // 'pack': column in the high 32 bits, generated line 1 in the low bits. - const packed = lineByName( - serializeTimeProfile( - bundleProfile, - 1000, - undefined, - false, - undefined, - [], - 'pack', + // Neither 'drop' (default) nor 'pack' packs the column when there is no + // missing source map — there is no server-side unminification to feed. + assert.strictEqual(lineOf(serializeTimeProfile(bundleProfile, 1000)), 1n); + assert.strictEqual( + lineOf( + serializeTimeProfile( + bundleProfile, + 1000, + undefined, + false, + undefined, + [], + 'pack', + ), ), + 1n, ); - assert.strictEqual(BigInt(packed.get('alpha')!), (15665n << 32n) | 1n); - assert.strictEqual(BigInt(packed.get('beta')!), (14349n << 32n) | 1n); }); it('should omit non-jS threads CPU time when profile has no CPU time', () => { @@ -277,9 +253,10 @@ describe('profile-serializer', () => { const heapProfileOut = serializeHeapProfile(v8HeapProfile, 0, 512 * 1024); assert.deepEqual(heapProfileOut, heapProfile); }); - it('should pack the column into the emitted line when columnNumbers is "pack"', () => { - // v8HeapProfile frames all use columnNumber 5; with 'pack' each emitted - // line must carry that column in its high 32 bits. + it('does not pack the column without a missing source map, even when columnNumbers is "pack"', () => { + // Packing is scoped to frames with a missing source map (bound for + // server-side unminification). v8HeapProfile has no source mapper, so no + // frame is missing a map and lines stay plain under both 'drop' and 'pack'. const packed = serializeHeapProfile( v8HeapProfile, 0, @@ -291,14 +268,12 @@ describe('profile-serializer', () => { 'pack', ); for (const location of packed.location!) { - const line = location.line![0]; assert.strictEqual( - BigInt(line.line) >> 32n, - 5n, - 'column packed into high bits', + BigInt(location.line![0].line) >> 32n, + 0n, + 'column must not be packed without a missing source map', ); } - // Default still drops the column (plain line, no high bits). const dropped = serializeHeapProfile(v8HeapProfile, 0, 512 * 1024); for (const location of dropped.location!) { assert.strictEqual(BigInt(location.line![0].line) >> 32n, 0n); @@ -502,6 +477,35 @@ describe('profile-serializer', () => { assertHasMissingMapToken(profile); }); + it('packs the column for a missing-map frame when columnNumbers is "pack"', () => { + // The frame's map is declared but missing, so it is bound for server-side + // unminification (same condition as the missing-map token). Its generated + // column (1) is packed into the high 32 bits of the emitted line. + const profile = serializeTimeProfile( + makeSingleNodeTimeProfile(missingJsPath), + 1000, + sourceMapper, + false, + undefined, + [], + 'pack', + ); + assertHasMissingMapToken(profile); + assert.strictEqual( + BigInt(profile.location![0].line![0].line), + (1n << 32n) | 1n, + ); + }); + + it('leaves a missing-map frame line plain under the default "drop"', () => { + const profile = serializeTimeProfile( + makeSingleNodeTimeProfile(missingJsPath), + 1000, + sourceMapper, + ); + assert.strictEqual(BigInt(profile.location![0].line![0].line), 1n); + }); + it('serializeHeapProfile emits missing-map token when a map is declared but absent', () => { // serialize() iterates root.children, so the node with allocations must // be a child of the root passed to serializeHeapProfile. From fae2430a0aadbda9acb5a7fee3739658d20278cd Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Wed, 22 Jul 2026 15:05:05 +0200 Subject: [PATCH 2/2] small optimization to eliminate repeated string comparison --- ts/src/profile-serializer.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ts/src/profile-serializer.ts b/ts/src/profile-serializer.ts index a79170c9..5f74d7a3 100644 --- a/ts/src/profile-serializer.ts +++ b/ts/src/profile-serializer.ts @@ -157,6 +157,7 @@ function serialize( const functions: Function[] = []; const functionIdMap = new Map(); const locationIdMap = new Map(); + const packColumns = columnNumbers === 'pack'; let hasMissingMapFiles = false; @@ -234,7 +235,7 @@ function serialize( // unminification (the same condition that sets dd:has-missing-map-files). // Locally-resolved frames keep their plain line, so packed values never // reach profiles that skip server-side unminification. - const packColumn = columnNumbers === 'pack' && loc.missingMapFile === true; + const packColumn = packColumns && loc.missingMapFile === true; return new Line({ functionId: getFunction(loc, scriptId).id, line: packColumn ? packLineAndColumn(loc.line, loc.column) : loc.line,