Fix regression when decoding stack trace (#73)

* Import necessary types

* Reverse decoding of stack frames and file IDs

* Test decoding of stack traces

* Add reverse run length encoder

This was added to make testing easier.

src/plugins/profiling/common/callercallee.ts

@@ -12,11 +12,16 @@ import {
   compareFrameGroup,
   createStackFrameMetadata,
   defaultGroupBy,
+  Executable,
+  FileID,
   FrameGroup,
   FrameGroupID,
   groupStackFrameMetadataByStackTrace,
   hashFrameGroup,
+  StackFrame,
+  StackFrameID,
   StackFrameMetadata,
+  StackTrace,
   StackTraceID,
 } from './profiling';
 

src/plugins/profiling/server/routes/stacktrace.test.ts

@@ -7,7 +7,34 @@
  */
 
 import { StackTrace } from '../../common/profiling';
-import { decodeStackTrace, EncodedStackTrace, runLengthDecodeReverse } from './stacktrace';
+import {
+  decodeStackTrace,
+  EncodedStackTrace,
+  runLengthDecodeReverse,
+  runLengthEncodeReverse,
+} from './stacktrace';
+
+enum fileID {
+  A = 'aQpJmTLWydNvOapSFZOwKg==',
+  B = 'hz_u-HGyrN6qeIk6UIJeCA==',
+  C = 'AJ8qrcXSoJbl_haPhlc4og==',
+  D = 'lHZiv7a58px6Gumcpo-6yA==',
+  E = 'fkbxUTZgljnk71ZMnqJnyA==',
+  F = 'gnEsgxvvEODj6iFYMQWYlA==',
+}
+
+enum frameID {
+  A = 'aQpJmTLWydNvOapSFZOwKgAAAAAAB924',
+  B = 'hz_u-HGyrN6qeIk6UIJeCAAAAAAAAAZZ',
+  C = 'AJ8qrcXSoJbl_haPhlc4ogAAAAAAAAAH',
+  D = 'lHZiv7a58px6Gumcpo-6yAAAAAAAAAAf',
+  E = 'fkbxUTZgljnk71ZMnqJnyAAAAAAAAABv',
+  F = 'gnEsgxvvEODj6iFYMQWYlAAAAAAGVDgH',
+  G = 'gnEsgxvvEODj6iFYMQWYlAAAAAAGBJv6',
+}
+
+const frameTypeA = [0, 0, 0];
+const frameTypeB = [8, 8, 8, 8];
 
 describe('Stack trace operations', () => {
   test('decodeStackTrace', () => {
@@ -17,24 +44,24 @@ describe('Stack trace operations', () => {
     }> = [
       {
         original: {
-          FrameID: 'aQpJmTLWydNvOapSFZOwKgAAAAAAB924',
-          Type: Buffer.from([0x1, 0x0]).toString('base64url'),
+          FrameID: frameID.A + frameID.B + frameID.C,
+          Type: runLengthEncodeReverse(frameTypeA).toString('base64url'),
         } as EncodedStackTrace,
         expected: {
-          FileID: ['aQpJmTLWydNvOapSFZOwKg=='],
-          FrameID: ['aQpJmTLWydNvOapSFZOwKgAAAAAAB924'],
-          Type: [0],
+          FileID: [fileID.C, fileID.B, fileID.A],
+          FrameID: [frameID.C, frameID.B, frameID.A],
+          Type: frameTypeA,
         } as StackTrace,
       },
       {
         original: {
-          FrameID: 'hz_u-HGyrN6qeIk6UIJeCAAAAAAAAAZZ',
-          Type: Buffer.from([0x1, 0x8]).toString('base64url'),
+          FrameID: frameID.D + frameID.E + frameID.F + frameID.G,
+          Type: runLengthEncodeReverse(frameTypeB).toString('base64url'),
         } as EncodedStackTrace,
         expected: {
-          FileID: ['hz_u-HGyrN6qeIk6UIJeCA=='],
-          FrameID: ['hz_u-HGyrN6qeIk6UIJeCAAAAAAAAAZZ'],
-          Type: [8],
+          FileID: [fileID.F, fileID.F, fileID.E, fileID.D],
+          FrameID: [frameID.G, frameID.F, frameID.E, frameID.D],
+          Type: frameTypeB,
         } as StackTrace,
       },
     ];
@@ -44,6 +71,14 @@ describe('Stack trace operations', () => {
     }
   });
 
+  test('run length is fully reversible', () => {
+    const tests: number[][] = [[], [0], [0, 1, 2, 3], [0, 1, 1, 2, 2, 2, 3, 3, 3, 3]];
+
+    for (const t of tests) {
+      expect(runLengthDecodeReverse(runLengthEncodeReverse(t))).toEqual(t);
+    }
+  });
+
   test('runLengthDecodeReverse with optional parameter', () => {
     const tests: Array<{
       bytes: Buffer;

src/plugins/profiling/server/routes/stacktrace.ts

@@ -28,9 +28,11 @@ const BASE64_FRAME_ID_LENGTH = 32;
 
 export interface EncodedStackTrace {
   // This field is a base64-encoded byte string. The string represents a
-  // serialized list of frame IDs. Each frame ID is composed of two
-  // concatenated values: a 16-byte file ID and an 8-byte address or line
-  // number (depending on the context of the downstream reader).
+  // serialized list of frame IDs in which the order of frames are
+  // reversed to allow for prefix compression (leaf frame last). Each
+  // frame ID is composed of two concatenated values: a 16-byte file ID
+  // and an 8-byte address or line number (depending on the context of
+  // the downstream reader).
   //
   //         Frame ID #1               Frame ID #2
   // +----------------+--------+----------------+--------+----
@@ -43,6 +45,44 @@ export interface EncodedStackTrace {
   Type: string;
 }
 
+// runLengthEncodeReverse encodes the reversed input array using a
+// run-length encoding.
+//
+// The input is a list of uint8s. The output is a binary stream of
+// 2-byte pairs (first byte is the length and the second byte is the
+// binary representation of the object) in reverse order.
+//
+// E.g. uint8 array [2, 2, 0, 0, 0, 0, 0] is converted into the byte
+// array [5, 0, 2, 2].
+export function runLengthEncodeReverse(input: number[]): Buffer {
+  const output: number[] = [];
+
+  if (input.length === 0) {
+    return Buffer.from(output);
+  }
+
+  let count = 0;
+  let current = input[input.length - 1];
+
+  for (let i = input.length - 2; i >= 0; i--) {
+    const next = input[i];
+
+    if (next === current && count < 255) {
+      count++;
+      continue;
+    }
+
+    output.push(count + 1, current);
+
+    count = 0;
+    current = next;
+  }
+
+  output.push(count + 1, current);
+
+  return Buffer.from(output);
+}
+
 // runLengthDecodeReverse decodes a run-length encoding for the reversed input array.
 //
 // The input is a binary stream of 2-byte pairs (first byte is the length and the
@@ -95,7 +135,7 @@ export function decodeStackTrace(input: EncodedStackTrace): StackTrace {
     const frameID = input.FrameID.slice(i, i + BASE64_FRAME_ID_LENGTH);
     const fileIDChunk = frameID.slice(0, BASE64_FILE_ID_LENGTH);
     const fileID = fileIDChunkToFileIDCache.get(fileIDChunk) as string;
-    const j = Math.floor(i / BASE64_FRAME_ID_LENGTH);
+    const j = countsFrameIDs - Math.floor(i / BASE64_FRAME_ID_LENGTH) - 1;
 
     frameIDs[j] = frameID;