chore(tracing): collect source names on server (#10862)

packages/playwright-core/src/client/tracing.ts

@@ -14,32 +14,20 @@
  * limitations under the License.
  */
 
+import fs from 'fs';
 import * as api from '../../types/types';
 import * as channels from '../protocol/channels';
-import { ParsedStackTrace } from '../utils/stackTrace';
-import { calculateSha1 } from '../utils/utils';
 import { Artifact } from './artifact';
 import { BrowserContext } from './browserContext';
-import { ClientInstrumentationListener } from './clientInstrumentation';
 
 export class Tracing implements api.Tracing {
   private _context: BrowserContext;
-  private _sources = new Set<string>();
-  private _instrumentationListener: ClientInstrumentationListener;
 
   constructor(channel: BrowserContext) {
     this._context = channel;
-    this._instrumentationListener = {
-      onApiCallBegin: (apiCall: string, stackTrace: ParsedStackTrace | null) => {
-        for (const frame of stackTrace?.frames || [])
-          this._sources.add(frame.file);
-      }
-    };
   }
 
   async start(options: { name?: string, title?: string, snapshots?: boolean, screenshots?: boolean, sources?: boolean } = {}) {
-    if (options.sources)
-      this._context._instrumentation!.addListener(this._instrumentationListener);
     await this._context._wrapApiCall(async () => {
       await this._context._channel.tracingStart(options);
       await this._context._channel.tracingStartChunk({ title: options.title });
@@ -47,7 +35,6 @@ export class Tracing implements api.Tracing {
   }
 
   async startChunk(options: { title?: string } = {}) {
-    this._sources = new Set();
     await this._context._channel.tracingStartChunk(options);
   }
 
@@ -63,9 +50,6 @@ export class Tracing implements api.Tracing {
   }
 
   private async _doStopChunk(channel: channels.BrowserContextChannel, filePath: string | undefined) {
-    const sources = this._sources;
-    this._sources = new Set();
-    this._context._instrumentation!.removeListener(this._instrumentationListener);
     const isLocal = !this._context._connection.isRemote();
 
     const result = await channel.tracingStopChunk({ save: !!filePath, skipCompress: isLocal });
@@ -74,9 +58,8 @@ export class Tracing implements api.Tracing {
       return;
     }
 
-    const sourceEntries: channels.NameValue[] = [];
+    if (filePath && fs.existsSync(filePath))
-    for (const value of sources)
+      await fs.promises.unlink(filePath);
-      sourceEntries.push({ name: 'resources/src@' + calculateSha1(value) + '.txt', value });
 
     if (!isLocal) {
       // We run against remote Playwright, compress on remote side.
@@ -85,7 +68,7 @@ export class Tracing implements api.Tracing {
       await artifact.delete();
     }
 
-    if (isLocal || sourceEntries)
+    if (isLocal || result.sourceEntries.length)
-      await this._context._localUtils.zip(filePath, sourceEntries.concat(result.entries));
+      await this._context._localUtils.zip(filePath, result.sourceEntries.concat(result.entries));
   }
 }

packages/playwright-core/src/dispatchers/browserContextDispatcher.ts

@@ -198,8 +198,8 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
   }
 
   async tracingStopChunk(params: channels.BrowserContextTracingStopChunkParams): Promise<channels.BrowserContextTracingStopChunkResult> {
-    const { artifact, entries } = await this._context.tracing.stopChunk(params.save, params.skipCompress);
+    const { artifact, entries, sourceEntries } = await this._context.tracing.stopChunk(params.save, params.skipCompress);
-    return { artifact: artifact ? new ArtifactDispatcher(this._scope, artifact) : undefined, entries };
+    return { artifact: artifact ? new ArtifactDispatcher(this._scope, artifact) : undefined, entries, sourceEntries };
   }
 
   async tracingStop(params: channels.BrowserContextTracingStopParams): Promise<channels.BrowserContextTracingStopResult> {

packages/playwright-core/src/protocol/channels.ts

@@ -1249,11 +1249,13 @@ export type BrowserContextTracingStartParams = {
   name?: string,
   snapshots?: boolean,
   screenshots?: boolean,
+  sources?: boolean,
 };
 export type BrowserContextTracingStartOptions = {
   name?: string,
   snapshots?: boolean,
   screenshots?: boolean,
+  sources?: boolean,
 };
 export type BrowserContextTracingStartResult = void;
 export type BrowserContextTracingStartChunkParams = {
@@ -1273,6 +1275,7 @@ export type BrowserContextTracingStopChunkOptions = {
 export type BrowserContextTracingStopChunkResult = {
   artifact?: ArtifactChannel,
   entries: NameValue[],
+  sourceEntries: NameValue[],
 };
 export type BrowserContextTracingStopParams = {};
 export type BrowserContextTracingStopOptions = {};

packages/playwright-core/src/protocol/protocol.yml

@@ -825,6 +825,7 @@ BrowserContext:
         name: string?
         snapshots: boolean?
         screenshots: boolean?
+        sources: boolean?
 
     tracingStartChunk:
       parameters:
@@ -839,6 +840,9 @@ BrowserContext:
         entries:
           type: array
           items: NameValue
+        sourceEntries:
+          type: array
+          items: NameValue
 
     tracingStop:
 

packages/playwright-core/src/protocol/validator.ts

@@ -503,6 +503,7 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
     name: tOptional(tString),
     snapshots: tOptional(tBoolean),
     screenshots: tOptional(tBoolean),
+    sources: tOptional(tBoolean),
   });
   scheme.BrowserContextTracingStartChunkParams = tObject({
     title: tOptional(tString),

packages/playwright-core/src/server/trace/recorder/tracing.ts

@@ -14,31 +14,32 @@
  * limitations under the License.
  */
 
+import { EventEmitter } from 'events';
 import fs from 'fs';
 import path from 'path';
 import yazl from 'yazl';
-import { EventEmitter } from 'events';
+import { NameValue } from '../../../common/types';
-import { createGuid, mkdirIfNeeded, monotonicTime } from '../../../utils/utils';
+import { commandsWithTracingSnapshots } from '../../../protocol/channels';
+import { ManualPromise } from '../../../utils/async';
+import { eventsHelper, RegisteredListener } from '../../../utils/eventsHelper';
+import { calculateSha1, createGuid, mkdirIfNeeded, monotonicTime } from '../../../utils/utils';
 import { Artifact } from '../../artifact';
 import { BrowserContext } from '../../browserContext';
 import { ElementHandle } from '../../dom';
-import { eventsHelper, RegisteredListener } from '../../../utils/eventsHelper';
 import { CallMetadata, InstrumentationListener, SdkObject } from '../../instrumentation';
 import { Page } from '../../page';
-import * as trace from '../common/traceEvents';
-import { commandsWithTracingSnapshots } from '../../../protocol/channels';
-import { Snapshotter, SnapshotterBlob, SnapshotterDelegate } from './snapshotter';
-import { FrameSnapshot } from '../common/snapshotTypes';
-import { HarTracer, HarTracerDelegate } from '../../supplements/har/harTracer';
 import * as har from '../../supplements/har/har';
+import { HarTracer, HarTracerDelegate } from '../../supplements/har/harTracer';
+import { FrameSnapshot } from '../common/snapshotTypes';
+import * as trace from '../common/traceEvents';
 import { VERSION } from '../common/traceEvents';
-import { NameValue } from '../../../common/types';
+import { Snapshotter, SnapshotterBlob, SnapshotterDelegate } from './snapshotter';
-import { ManualPromise } from '../../../utils/async';
 
 export type TracerOptions = {
   name?: string;
   snapshots?: boolean;
   screenshots?: boolean;
+  sources?: boolean;
 };
 
 type RecordingState = {
@@ -47,7 +48,9 @@ type RecordingState = {
   networkFile: string,
   traceFile: string,
   filesCount: number,
-  sha1s: Set<string>,
+  networkSha1s: Set<string>,
+  traceSha1s: Set<string>,
+  sources: Set<string>,
   recording: boolean;
 };
 
@@ -102,8 +105,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
     const traceName = options.name || createGuid();
     const traceFile = path.join(this._tracesDir, traceName + '.trace');
     const networkFile = path.join(this._tracesDir, traceName + '.network');
-    this._state = { options, traceName, traceFile, networkFile, filesCount: 0, sha1s: new Set(), recording: false };
+    this._state = { options, traceName, traceFile, networkFile, filesCount: 0, traceSha1s: new Set(), networkSha1s: new Set(), sources: new Set(), recording: false };
-
     this._writeChain = fs.promises.mkdir(this._resourcesDir, { recursive: true }).then(() => fs.promises.writeFile(networkFile, ''));
     if (options.snapshots)
       this._harTracer.start();
@@ -167,7 +169,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
     await this._writeChain;
   }
 
-  async stopChunk(save: boolean, skipCompress: boolean): Promise<{ artifact: Artifact | null, entries: NameValue[] }> {
+  async stopChunk(save: boolean, skipCompress: boolean): Promise<{ artifact: Artifact | null, entries: NameValue[], sourceEntries: NameValue[] }> {
     if (this._isStopping)
       throw new Error(`Tracing is already stopping`);
     this._isStopping = true;
@@ -176,7 +178,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
       this._isStopping = false;
       if (save)
         throw new Error(`Must start tracing before stopping`);
-      return { artifact: null, entries: [] };
+      return { artifact: null, entries: [], sourceEntries: [] };
     }
 
     const state = this._state!;
@@ -203,11 +205,8 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
     // Chain the export operation against write operations,
     // so that neither trace files nor sha1s change during the export.
     return await this._appendTraceOperation(async () => {
-      this._isStopping = false;
-      state.recording = false;
-
       if (!save)
-        return { artifact: null, entries: [] };
+        return { artifact: null, entries: [], sourceEntries: [] };
 
       // Har files a live, make a snapshot before returning the resulting entries.
       const networkFile = path.join(state.networkFile, '..', createGuid());
@@ -216,12 +215,22 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
       const entries: NameValue[] = [];
       entries.push({ name: 'trace.trace', value: state.traceFile });
       entries.push({ name: 'trace.network', value: networkFile });
-      for (const sha1 of state.sha1s)
+      for (const sha1 of new Set([...state.traceSha1s, ...state.networkSha1s]))
         entries.push({ name: path.join('resources', sha1), value: path.join(this._resourcesDir, sha1) });
 
-      const zipArtifact = skipCompress ? null : await this._exportZip(entries, state).catch(() => null);
+      const sourceEntries: NameValue[] = [];
-      return { artifact: zipArtifact, entries };
+      for (const value of state.sources)
-    }) || { artifact: null, entries: [] };
+        sourceEntries.push({ name: 'resources/src@' + calculateSha1(value) + '.txt', value });
+
+      const artifact = skipCompress ? null : await this._exportZip(entries, state).catch(() => null);
+      return { artifact, entries, sourceEntries };
+    }).finally(() => {
+      // Only reset trace sha1s, network resources are preserved between chunks.
+      state.traceSha1s = new Set();
+      state.sources = new Set();
+      this._isStopping = false;
+      state.recording = false;
+    }) || { artifact: null, entries: [], sourceEntries: [] };
   }
 
   private async _exportZip(entries: NameValue[], state: RecordingState): Promise<Artifact | null> {
@@ -263,6 +272,10 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
     metadata.afterSnapshot = `after@${metadata.id}`;
     const beforeSnapshot = this._captureSnapshot('before', sdkObject, metadata);
     this._pendingCalls.set(metadata.id, { sdkObject, metadata, beforeSnapshot });
+    if (this._state?.options.sources) {
+      for (const frame of metadata.stack || [])
+        this._state.sources.add(frame.file);
+    }
     await beforeSnapshot;
   }
 
@@ -302,7 +315,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
   onEntryFinished(entry: har.Entry) {
     const event: trace.ResourceSnapshotTraceEvent = { type: 'resource-snapshot', snapshot: entry };
     this._appendTraceOperation(async () => {
-      visitSha1s(event, this._state!.sha1s);
+      visitSha1s(event, this._state!.networkSha1s);
       await fs.promises.appendFile(this._state!.networkFile, JSON.stringify(event) + '\n');
     });
   }
@@ -344,7 +357,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
 
   private _appendTraceEvent(event: trace.TraceEvent) {
     this._appendTraceOperation(async () => {
-      visitSha1s(event, this._state!.sha1s);
+      visitSha1s(event, this._state!.traceSha1s);
       await fs.promises.appendFile(this._state!.traceFile, JSON.stringify(event) + '\n');
     });
   }

tests/tracing.spec.ts

@@ -132,6 +132,63 @@ test('should collect two traces', async ({ context, page, server }, testInfo) =>
   }
 });
 
+test('should not include trace resources from the provious chunks', async ({ context, page, server }, testInfo) => {
+  await context.tracing.start({ screenshots: true, snapshots: true, sources: true });
+
+  await context.tracing.startChunk();
+  await page.goto(server.EMPTY_PAGE);
+  await page.setContent('<button>Click</button>');
+  await page.click('"Click"');
+  await context.tracing.stopChunk({ path: testInfo.outputPath('trace1.zip') });
+
+  await context.tracing.startChunk();
+  await context.tracing.stopChunk({ path: testInfo.outputPath('trace2.zip') });
+
+  {
+    const { resources } = await parseTrace(testInfo.outputPath('trace1.zip'));
+    const names = Array.from(resources.keys());
+    expect(names.filter(n => n.endsWith('.html')).length).toBe(1);
+    expect(names.filter(n => n.endsWith('.jpeg')).length).toBeGreaterThan(1);
+    // 1 source file for the test.
+    expect(names.filter(n => n.endsWith('.txt')).length).toBe(1);
+  }
+
+  {
+    const { resources } = await parseTrace(testInfo.outputPath('trace2.zip'));
+    const names = Array.from(resources.keys());
+    // 1 network resource should be preserved.
+    expect(names.filter(n => n.endsWith('.html')).length).toBe(1);
+    expect(names.filter(n => n.endsWith('.jpeg')).length).toBe(0);
+    // 1 source file for the test.
+    expect(names.filter(n => n.endsWith('.txt')).length).toBe(1);
+  }
+});
+
+test('should overwrite existing file', async ({ context, page, server }, testInfo) => {
+  await context.tracing.start({ screenshots: true, snapshots: true, sources: true });
+  await page.goto(server.EMPTY_PAGE);
+  await page.setContent('<button>Click</button>');
+  await page.click('"Click"');
+  const path = testInfo.outputPath('trace1.zip');
+  await context.tracing.stop({ path });
+  {
+    const { resources } = await parseTrace(path);
+    const names = Array.from(resources.keys());
+    expect(names.filter(n => n.endsWith('.html')).length).toBe(1);
+    expect(names.filter(n => n.endsWith('.jpeg')).length).toBeGreaterThan(1);
+  }
+
+  await context.tracing.start({ screenshots: true, snapshots: true, sources: true });
+  await context.tracing.stop({ path });
+
+  {
+    const { resources } = await parseTrace(path);
+    const names = Array.from(resources.keys());
+    expect(names.filter(n => n.endsWith('.html')).length).toBe(0);
+    expect(names.filter(n => n.endsWith('.jpeg')).length).toBe(0);
+  }
+});
+
 test('should collect sources', async ({ context, page, server }, testInfo) => {
   await context.tracing.start({ sources: true });
   await page.goto(server.EMPTY_PAGE);