Handle Text and Log marker payloads with their marker schema

src/profile-logic/marker-data.ts

@@ -23,6 +23,7 @@ import {
 } from 'firefox-profiler/app-logic/constants';
 import {
   getSchemaFromMarker,
+  isStringIndexMarkerField,
   markerPayloadMatchesSearch,
   markerSchemaFrontEndOnly,
 } from './marker-schema';
@@ -1468,36 +1469,83 @@ export function removePrefMarkerPreferenceValues(
 }
 
 /**
- * Sanitize Text marker's name property for potential URLs.
+ * Apply a transformation to a Text marker's text. The schema tells us whether the
+ * payload holds the text inline or as a string table index. In the latter case the
+ * result is interned as a new string, as other markers and frames may share that
+ * entry.
+ */
+function _updateTextMarkerText(
+  payload: TextMarkerPayload,
+  stringIndexMarkerFieldsByDataType: Map<string, string[]>,
+  stringTable: StringTable,
+  transform: (text: string) => string
+): TextMarkerPayload {
+  // The casts below follow the storage layout the schema declares, which
+  // TypeScript can't verify from the payload type alone.
+  if (
+    !isStringIndexMarkerField(
+      stringIndexMarkerFieldsByDataType,
+      payload.type,
+      'name'
+    )
+  ) {
+    return { ...payload, name: transform(payload.name as string) };
+  }
+
+  const nameIndex = payload.name as IndexIntoStringTable;
+  if (!stringTable.hasIndex(nameIndex)) {
+    return payload;
+  }
+  const text = stringTable.getString(nameIndex);
+  const newText = transform(text);
+  if (newText === text) {
+    return payload;
+  }
+  return { ...payload, name: stringTable.indexForString(newText) };
+}
+
+/**
+ * Sanitize Text marker's name property for potential URLs. Only for payloads
+ * holding their text inline, as the string table is sanitized as a whole.
  */
 export function sanitizeTextMarker(
-  payload: TextMarkerPayload
+  payload: TextMarkerPayload,
+  stringIndexMarkerFieldsByDataType: Map<string, string[]>,
+  stringTable: StringTable
 ): TextMarkerPayload {
-  return {
-    ...payload,
-    name: removeURLs(payload.name),
-  };
+  return _updateTextMarkerText(
+    payload,
+    stringIndexMarkerFieldsByDataType,
+    stringTable,
+    removeURLs
+  );
 }
 
 /**
  * Sanitize Extension Text marker's name property for potential add-on ids.
  */
 export function sanitizeExtensionTextMarker(
   markerName: string,
-  payload: TextMarkerPayload
+  payload: TextMarkerPayload,
+  stringIndexMarkerFieldsByDataType: Map<string, string[]>,
+  stringTable: StringTable
 ): TextMarkerPayload {
   if (['ExtensionParent', 'ExtensionChild'].includes(markerName)) {
-    return {
-      ...payload,
-      name: payload.name.replace(/^.*, (api_(call|event): )/, '$1'),
-    };
+    return _updateTextMarkerText(
+      payload,
+      stringIndexMarkerFieldsByDataType,
+      stringTable,
+      (text) => text.replace(/^.*, (api_(call|event): )/, '$1')
+    );
   }
 
   if (markerName === 'Extension Suspend') {
-    return {
-      ...payload,
-      name: payload.name.replace(/ by .*$/, ''),
-    };
+    return _updateTextMarkerText(
+      payload,
+      stringIndexMarkerFieldsByDataType,
+      stringTable,
+      (text) => text.replace(/ by .*$/, '')
+    );
   }
 
   return payload;
@@ -1731,6 +1779,31 @@ export function formatLogTimestamp(absoluteMs: number): string {
   );
 }
 
+/**
+ * Resolve a new format Log marker's message. The schema tells us whether the
+ * payload holds the message inline or as an index into the string table, which
+ * is what newer profiles do.
+ */
+export function resolveLogMarkerMessage(
+  message: string | IndexIntoStringTable,
+  stringArray: string[],
+  stringIndexMarkerFieldsByDataType: Map<string, string[]>
+): string {
+  // The casts below follow the storage layout the schema declares, which
+  // TypeScript can't verify from the payload type alone.
+  if (
+    !isStringIndexMarkerField(
+      stringIndexMarkerFieldsByDataType,
+      'Log',
+      'message'
+    )
+  ) {
+    return message as string;
+  }
+
+  return stringArray[message as IndexIntoStringTable] ?? '';
+}
+
 /**
  * Format a Log marker payload into a MOZ_LOG canonical line.
  *
@@ -1751,15 +1824,21 @@ export function formatLogStatement(
   threadName: string,
   data: LogMarkerPayload,
   moduleName: string,
-  stringArray: string[]
+  stringArray: string[],
+  stringIndexMarkerFieldsByDataType: Map<string, string[]>
 ): string | null {
   if ('message' in data) {
-    if (!data.message) {
+    const message = resolveLogMarkerMessage(
+      data.message,
+      stringArray,
+      stringIndexMarkerFieldsByDataType
+    );
+    if (!message) {
       return null;
     }
     const levelStr = stringArray[data.level] ?? '';
     const levelLetter = LOG_LEVEL_STRING_TO_LETTER[levelStr] ?? 'D';
-    return `${timestampStr} - [${processName} ${pid}: ${threadName}]: ${levelLetter}/${moduleName} ${data.message.trim()}`;
+    return `${timestampStr} - [${processName} ${pid}: ${threadName}]: ${levelLetter}/${moduleName} ${message.trim()}`;
   }
   if (!data.name) {
     return null;

src/profile-logic/marker-schema.ts

@@ -642,11 +642,7 @@ export function markerPayloadMatchesSearch(
       continue;
     }
 
-    if (
-      payloadField.format === 'unique-string' ||
-      payloadField.format === 'flow-id' ||
-      payloadField.format === 'terminating-flow-id'
-    ) {
+    if (isStringIndexFormat(payloadField.format)) {
       if (typeof value !== 'number') {
         console.warn(
           `In marker ${marker.name}, the key ${payloadField.key} has an invalid value "${value}" as a unique string, it isn't a number.`
@@ -671,6 +667,19 @@ export function markerPayloadMatchesSearch(
   return false;
 }
 
+/**
+ * Whether this field format means the payload holds a string table index.
+ */
+export function isStringIndexFormat(
+  format: MarkerFormatType | undefined
+): boolean {
+  return (
+    format === 'unique-string' ||
+    format === 'flow-id' ||
+    format === 'terminating-flow-id'
+  );
+}
+
 /**
  * Returns a map of marker schema name -> array of field keys, listing any fields
  * that contain indexes into the string table. If a marker schema has no such
@@ -689,12 +698,7 @@ export function computeStringIndexMarkerFieldsByDataType(
     const { name, fields } = schema;
     const stringIndexFields = [];
     for (const field of fields) {
-      if (
-        (field.format === 'unique-string' ||
-          field.format === 'flow-id' ||
-          field.format === 'terminating-flow-id') &&
-        field.key
-      ) {
+      if (isStringIndexFormat(field.format) && field.key) {
         stringIndexFields.push(field.key);
       }
     }
@@ -704,3 +708,18 @@ export function computeStringIndexMarkerFieldsByDataType(
   }
   return stringIndexMarkerFieldsByDataType;
 }
+
+/**
+ * Whether the schema of this marker data type declares the given payload field
+ * as a string table index. Takes the map computed by
+ * `computeStringIndexMarkerFieldsByDataType`.
+ */
+export function isStringIndexMarkerField(
+  stringIndexMarkerFieldsByDataType: Map<string, string[]>,
+  dataType: string,
+  fieldKey: string
+): boolean {
+  return (
+    stringIndexMarkerFieldsByDataType.get(dataType)?.includes(fieldKey) ?? false
+  );
+}

src/profile-logic/sanitize.ts

@@ -18,7 +18,11 @@ import {
   sanitizeTextMarker,
   sanitizeFromMarkerSchema,
 } from './marker-data';
-import { getSchemaFromMarker } from './marker-schema';
+import {
+  computeStringIndexMarkerFieldsByDataType,
+  getSchemaFromMarker,
+  isStringIndexMarkerField,
+} from './marker-schema';
 import {
   filterRawThreadSamplesToRange,
   filterCounterSamplesToRange,
@@ -141,6 +145,11 @@ export function sanitizePII(
     stringArray,
   };
 
+  // Precompute the payload fields that hold string table indexes, so that the
+  // marker loop below doesn't have to walk the schema fields for every marker.
+  const stringIndexMarkerFieldsByDataType =
+    computeStringIndexMarkerFieldsByDataType(Object.values(markerSchemaByName));
+
   let stackFlags: Uint8Array | null = null;
 
   if (windowIdFromPrivateBrowsing.size > 0) {
@@ -326,6 +335,7 @@ export function sanitizePII(
         PIIToBeRemoved,
         windowIdFromPrivateBrowsing,
         markerSchemaByName,
+        stringIndexMarkerFieldsByDataType,
         stackFlags
       );
 
@@ -441,6 +451,7 @@ function sanitizeThreadPII(
   PIIToBeRemoved: RemoveProfileInformation,
   windowIdFromPrivateBrowsing: Set<InnerWindowID>,
   markerSchemaByName: MarkerSchemaByName,
+  stringIndexMarkerFieldsByDataType: Map<string, string[]>,
   stackFlags: Uint8Array | null
 ): RawThread | null {
   if (PIIToBeRemoved.shouldRemoveThreads.has(threadIndex)) {
@@ -509,9 +520,21 @@ function sanitizeThreadPII(
           markerTable.name[i] = stringTable.indexForString(sanitizedRequestStr);
         }
 
-        if (currentMarker.type === 'Text') {
+        if (
+          currentMarker.type === 'Text' &&
+          !isStringIndexMarkerField(
+            stringIndexMarkerFieldsByDataType,
+            'Text',
+            'name'
+          )
+        ) {
           // Sanitize all the name fields of text markers in case they contain URLs.
-          markerTable.data[i] = sanitizeTextMarker(currentMarker);
+          // Newer profiles hold the text in the string table, sanitized above.
+          markerTable.data[i] = sanitizeTextMarker(
+            currentMarker,
+            stringIndexMarkerFieldsByDataType,
+            stringTable
+          );
           // Re-assign the value of currentMarker as the marker may be
           // sanitized again to remove extension ids.
           currentMarker = markerTable.data[i];
@@ -524,10 +547,13 @@ function sanitizeThreadPII(
         currentMarker.type === 'Text'
       ) {
         const markerName = stringTable.getString(markerTable.name[i]);
-        // Sanitize extension ids out of known extension markers.
+        // Sanitize extension ids out of known extension markers. Unlike URLs,
+        // these aren't removed from the string table as a whole.
         markerTable.data[i] = sanitizeExtensionTextMarker(
           markerName,
-          currentMarker
+          currentMarker,
+          stringIndexMarkerFieldsByDataType,
+          stringTable
         );
       }
 

src/profile-query/formatters/marker-info.ts

@@ -9,6 +9,7 @@ import {
 import {
   getProfile,
   getCategories,
+  getMarkerSchema,
   getMarkerSchemaByName,
   getStringTable,
   getCommittedRange,
@@ -23,6 +24,7 @@ import {
 } from '../network-summary';
 import { getThreadSelectors } from 'firefox-profiler/selectors/per-thread';
 import {
+  computeStringIndexMarkerFieldsByDataType,
   formatFromMarkerSchema,
   getLabelGetter,
 } from 'firefox-profiler/profile-logic/marker-schema';
@@ -61,6 +63,7 @@ import {
   LOG_LETTER_TO_LEVEL,
   formatLogTimestamp,
   formatLogStatement,
+  resolveLogMarkerMessage,
 } from 'firefox-profiler/profile-logic/marker-data';
 import { formatFunctionNameWithLibrary } from '../function-list';
 import type {
@@ -1369,6 +1372,9 @@ export function collectProfileLogs(
   const profile = getProfile(state);
   const profileStartTime = profile.meta.startTime;
   const stringArray = profile.shared.stringArray;
+  // The schema tells us which payload fields hold string table indexes.
+  const stringIndexMarkerFieldsByDataType =
+    computeStringIndexMarkerFieldsByDataType(getMarkerSchema(state));
 
   // Resolve which thread indexes to include.
   const threadIndexes: Set<number> | null =
@@ -1423,13 +1429,18 @@ export function collectProfileLogs(
       let levelLetter: string;
 
       if ('message' in logData) {
-        if (!logData.message) {
+        const rawMessage = resolveLogMarkerMessage(
+          logData.message,
+          stringArray,
+          stringIndexMarkerFieldsByDataType
+        );
+        if (!rawMessage) {
           continue;
         }
         moduleName = stringArray[markers.name[i]] ?? '';
         const levelStr = stringArray[logData.level] ?? '';
         levelLetter = LOG_LEVEL_STRING_TO_LETTER[levelStr] ?? 'D';
-        message = logData.message.trim();
+        message = rawMessage.trim();
       } else {
         if (!logData.name) {
           continue;
@@ -1473,7 +1484,8 @@ export function collectProfileLogs(
         threadName,
         logData,
         moduleName,
-        stringArray
+        stringArray,
+        stringIndexMarkerFieldsByDataType
       );
       if (formatted !== null) {
         entries.push(formatted);

src/test/fixtures/profiles/marker-schema.ts

@@ -114,8 +114,10 @@ export const markerSchemaForTests: MarkerSchema[] = [
     fields: [
       { key: 'module', label: 'Module', format: 'string' },
       { key: 'name', label: 'Name', format: 'string' },
-      // New format: level is a string table index ("Error"/"Warning"/"Info"/"Debug"/"Verbose").
+      // New format: level ("Error"/"Warning"/"Info"/"Debug"/"Verbose") and
+      // message are string table indexes.
       { key: 'level', label: 'Level', format: 'unique-string' },
+      { key: 'message', label: 'Message', format: 'unique-string' },
     ],
   },
   {

src/test/store/__snapshots__/profile-view.test.ts.snap

@@ -283,6 +283,11 @@ Object {
             "key": "level",
             "label": "Level",
           },
+          Object {
+            "format": "unique-string",
+            "key": "message",
+            "label": "Message",
+          },
         ],
         "name": "Log",
         "tableLabel": "({marker.data.module}) {marker.data.name}",