From 55c95a4463be14a6bd39ce3b447ac93c863f5f3e Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 22 Feb 2023 21:08:47 -0800 Subject: [PATCH] chore: do not send stacks as a part of the call metainfo (#21089) --- .../playwright-core/src/client/connection.ts | 14 ++++- .../playwright-core/src/client/tracing.ts | 36 ++++++++----- .../playwright-core/src/protocol/validator.ts | 17 +++++-- .../src/server/dispatchers/dispatcher.ts | 2 +- .../dispatchers/localUtilsDispatcher.ts | 32 +++++++++--- .../server/dispatchers/tracingDispatcher.ts | 4 +- .../playwright-core/src/server/recorder.ts | 4 +- .../src/server/trace/recorder/tracing.ts | 51 ++++++------------- packages/playwright-core/src/utils/index.ts | 1 + .../playwright-core/src/utils/traceUtils.ts | 41 +++++++++++++++ packages/protocol/src/callMetadata.ts | 4 +- packages/protocol/src/channels.ts | 18 +++++-- packages/protocol/src/protocol.yml | 37 +++++++++++--- packages/trace-viewer/src/traceModel.ts | 13 +++++ packages/trace-viewer/vite.sw.config.ts | 1 + packages/trace/src/trace.ts | 3 +- packages/trace/src/traceUtils.ts | 32 ++++++++++++ tests/config/utils.ts | 10 +++- tests/library/tracing.spec.ts | 14 ++--- 19 files changed, 249 insertions(+), 85 deletions(-) create mode 100644 packages/playwright-core/src/utils/traceUtils.ts create mode 100644 packages/trace/src/traceUtils.ts diff --git a/packages/playwright-core/src/client/connection.ts b/packages/playwright-core/src/client/connection.ts index be8533fdf0..2db70c9b24 100644 --- a/packages/playwright-core/src/client/connection.ts +++ b/packages/playwright-core/src/client/connection.ts @@ -71,6 +71,7 @@ export class Connection extends EventEmitter { private _localUtils?: LocalUtils; // Some connections allow resolving in-process dispatchers. toImpl: ((client: ChannelOwner) => any) | undefined; + private _stackCollectors = new Set(); constructor(localUtils?: LocalUtils) { super(); @@ -102,6 +103,14 @@ export class Connection extends EventEmitter { return this._objects.get(guid)!; } + startCollectingCallMetadata(collector: channels.ClientSideCallMetadata[]) { + this._stackCollectors.add(collector); + } + + stopCollectingCallMetadata(collector: channels.ClientSideCallMetadata[]) { + this._stackCollectors.delete(collector); + } + async sendMessageToServer(object: ChannelOwner, type: string, method: string, params: any, stackTrace: ParsedStackTrace | null): Promise { if (this._closedErrorMessage) throw new Error(this._closedErrorMessage); @@ -112,7 +121,10 @@ export class Connection extends EventEmitter { const converted = { id, guid, method, params }; // Do not include metadata in debug logs to avoid noise. debugLogger.log('channel:command', converted); - const metadata: channels.Metadata = { stack: frames, apiName, internal: !apiName }; + for (const collector of this._stackCollectors) + collector.push({ stack: frames, id: id }); + const location = frames[0] ? { file: frames[0].file, line: frames[0].line, column: frames[0].column } : undefined; + const metadata: channels.Metadata = { apiName, location, internal: !apiName }; this.onmessage({ ...converted, metadata }); return await new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject, stackTrace, type, method })); diff --git a/packages/playwright-core/src/client/tracing.ts b/packages/playwright-core/src/client/tracing.ts index 9d26fe794e..2a5507987a 100644 --- a/packages/playwright-core/src/client/tracing.ts +++ b/packages/playwright-core/src/client/tracing.ts @@ -20,6 +20,8 @@ import { Artifact } from './artifact'; import { ChannelOwner } from './channelOwner'; export class Tracing extends ChannelOwner implements api.Tracing { + private _includeSources = false; + private _metadataCollector: channels.ClientSideCallMetadata[] = []; static from(channel: channels.TracingChannel): Tracing { return (channel as any)._object; } @@ -29,14 +31,19 @@ export class Tracing extends ChannelOwner implements ap } async start(options: { name?: string, title?: string, snapshots?: boolean, screenshots?: boolean, sources?: boolean } = {}) { + this._includeSources = !!options.sources; await this._wrapApiCall(async () => { await this._channel.tracingStart(options); await this._channel.tracingStartChunk({ title: options.title }); }); + this._metadataCollector = []; + this._connection.startCollectingCallMetadata(this._metadataCollector); } async startChunk(options: { title?: string } = {}) { await this._channel.tracingStartChunk(options); + this._metadataCollector = []; + this._connection.startCollectingCallMetadata(this._metadataCollector); } async stopChunk(options: { path?: string } = {}) { @@ -51,22 +58,25 @@ export class Tracing extends ChannelOwner implements ap } private async _doStopChunk(filePath: string | undefined) { - const isLocal = !this._connection.isRemote(); - - let mode: channels.TracingTracingStopChunkParams['mode'] = 'doNotSave'; - if (filePath) { - if (isLocal) - mode = 'compressTraceAndSources'; - else - mode = 'compressTrace'; - } - - const result = await this._channel.tracingStopChunk({ mode }); + this._connection.stopCollectingCallMetadata(this._metadataCollector); + const metadata = this._metadataCollector; + this._metadataCollector = []; if (!filePath) { + await this._channel.tracingStopChunk({ mode: 'discard' }); // Not interested in artifacts. return; } + const isLocal = !this._connection.isRemote(); + + if (isLocal) { + const result = await this._channel.tracingStopChunk({ mode: 'entries' }); + await this._connection.localUtils()._channel.zip({ zipFile: filePath, entries: result.entries!, metadata, mode: 'write', includeSources: this._includeSources }); + return; + } + + const result = await this._channel.tracingStopChunk({ mode: 'archive' }); + // The artifact may be missing if the browser closed while stopping tracing. if (!result.artifact) return; @@ -77,7 +87,7 @@ export class Tracing extends ChannelOwner implements ap await artifact.delete(); // Add local sources to the remote trace if necessary. - if (result.sourceEntries?.length) - await this._connection.localUtils()._channel.zip({ zipFile: filePath, entries: result.sourceEntries }); + if (result.entries?.length) + await this._connection.localUtils()._channel.zip({ zipFile: filePath, entries: result.entries!, metadata, mode: 'append', includeSources: this._includeSources }); } } diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 85f349c5fe..f12d6623cd 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -27,10 +27,18 @@ scheme.StackFrame = tObject({ function: tOptional(tString), }); scheme.Metadata = tObject({ - stack: tOptional(tArray(tType('StackFrame'))), + location: tOptional(tObject({ + file: tString, + line: tOptional(tNumber), + column: tOptional(tNumber), + })), apiName: tOptional(tString), internal: tOptional(tBoolean), }); +scheme.ClientSideCallMetadata = tObject({ + id: tNumber, + stack: tOptional(tArray(tType('StackFrame'))), +}); scheme.Point = tObject({ x: tNumber, y: tNumber, @@ -211,6 +219,9 @@ scheme.LocalUtilsInitializer = tOptional(tObject({})); scheme.LocalUtilsZipParams = tObject({ zipFile: tString, entries: tArray(tType('NameValue')), + mode: tEnum(['write', 'append']), + metadata: tArray(tType('ClientSideCallMetadata')), + includeSources: tBoolean, }); scheme.LocalUtilsZipResult = tOptional(tObject({})); scheme.LocalUtilsHarOpenParams = tObject({ @@ -2084,11 +2095,11 @@ scheme.TracingTracingStartChunkParams = tObject({ }); scheme.TracingTracingStartChunkResult = tOptional(tObject({})); scheme.TracingTracingStopChunkParams = tObject({ - mode: tEnum(['doNotSave', 'compressTrace', 'compressTraceAndSources']), + mode: tEnum(['archive', 'discard', 'entries']), }); scheme.TracingTracingStopChunkResult = tObject({ artifact: tOptional(tChannel(['Artifact'])), - sourceEntries: tOptional(tArray(tType('NameValue'))), + entries: tOptional(tArray(tType('NameValue'))), }); scheme.TracingTracingStopParams = tOptional(tObject({})); scheme.TracingTracingStopResult = tOptional(tObject({})); diff --git a/packages/playwright-core/src/server/dispatchers/dispatcher.ts b/packages/playwright-core/src/server/dispatchers/dispatcher.ts index d82fccd914..73484b109b 100644 --- a/packages/playwright-core/src/server/dispatchers/dispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/dispatcher.ts @@ -251,7 +251,7 @@ export class DispatcherConnection { const sdkObject = dispatcher._object instanceof SdkObject ? dispatcher._object : undefined; const callMetadata: CallMetadata = { id: `call@${id}`, - stack: validMetadata.stack, + location: validMetadata.location, apiName: validMetadata.apiName, internal: validMetadata.internal, objectId: sdkObject?.guid, diff --git a/packages/playwright-core/src/server/dispatchers/localUtilsDispatcher.ts b/packages/playwright-core/src/server/dispatchers/localUtilsDispatcher.ts index c61f7c3837..ea04d54b1d 100644 --- a/packages/playwright-core/src/server/dispatchers/localUtilsDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/localUtilsDispatcher.ts @@ -19,7 +19,7 @@ import fs from 'fs'; import path from 'path'; import type * as channels from '@protocol/channels'; import { ManualPromise } from '../../utils/manualPromise'; -import { assert, createGuid } from '../../utils'; +import { assert, calculateSha1, createGuid } from '../../utils'; import type { RootDispatcher } from './dispatcher'; import { Dispatcher } from './dispatcher'; import { yazl, yauzl } from '../../zipBundle'; @@ -38,6 +38,7 @@ import type { HTTPRequestParams } from '../../utils/network'; import type http from 'http'; import type { Playwright } from '../playwright'; import { SdkObject } from '../../server/instrumentation'; +import { serializeClientSideCallMetadata } from '../../utils'; export class LocalUtilsDispatcher extends Dispatcher<{ guid: string }, channels.LocalUtilsChannel, RootDispatcher> implements channels.LocalUtilsChannel { _type_LocalUtils: boolean; @@ -49,20 +50,39 @@ export class LocalUtilsDispatcher extends Dispatcher<{ guid: string }, channels. this._type_LocalUtils = true; } - async zip(params: channels.LocalUtilsZipParams, metadata: CallMetadata): Promise { + async zip(params: channels.LocalUtilsZipParams): Promise { const promise = new ManualPromise(); const zipFile = new yazl.ZipFile(); (zipFile as any as EventEmitter).on('error', error => promise.reject(error)); - for (const entry of params.entries) { + const addFile = (file: string, name: string) => { try { - if (fs.statSync(entry.value).isFile()) - zipFile.addFile(entry.value, entry.name); + if (fs.statSync(file).isFile()) + zipFile.addFile(file, name); } catch (e) { } + }; + + for (const entry of params.entries) + addFile(entry.value, entry.name); + + // Add stacks and the sources. + zipFile.addBuffer(Buffer.from(JSON.stringify(serializeClientSideCallMetadata(params.metadata))), 'trace.stacks'); + + // Collect sources from stacks. + if (params.includeSources) { + const sourceFiles = new Set(); + for (const { stack } of params.metadata) { + if (!stack) + continue; + for (const { file } of stack) + sourceFiles.add(file); + } + for (const sourceFile of sourceFiles) + addFile(sourceFile, 'resources/src@' + calculateSha1(sourceFile) + '.txt'); } - if (!fs.existsSync(params.zipFile)) { + if (params.mode === 'write') { // New file, just compress the entries. await fs.promises.mkdir(path.dirname(params.zipFile), { recursive: true }); zipFile.end(undefined, () => { diff --git a/packages/playwright-core/src/server/dispatchers/tracingDispatcher.ts b/packages/playwright-core/src/server/dispatchers/tracingDispatcher.ts index d0b6d7654f..f6c52cefb7 100644 --- a/packages/playwright-core/src/server/dispatchers/tracingDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/tracingDispatcher.ts @@ -42,8 +42,8 @@ export class TracingDispatcher extends Dispatcher { - const { artifact, sourceEntries } = await this._object.stopChunk(params); - return { artifact: artifact ? ArtifactDispatcher.from(this, artifact) : undefined, sourceEntries }; + const { artifact, entries } = await this._object.stopChunk(params); + return { artifact: artifact ? ArtifactDispatcher.from(this, artifact) : undefined, entries }; } async tracingStop(params: channels.TracingTracingStopParams): Promise { diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 3d8d3b86df..9f42a5abaa 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -275,9 +275,9 @@ export class Recorder implements InstrumentationListener { // Apply new decorations. let fileToSelect = undefined; for (const metadata of this._currentCallsMetadata.keys()) { - if (!metadata.stack || !metadata.stack[0]) + if (!metadata.location) continue; - const { file, line } = metadata.stack[0]; + const { file, line } = metadata.location; let source = this._userSources.get(file); if (!source) { source = { isRecorded: false, label: file, id: file, text: this._readSource(file), highlight: [], language: languageForFile(file) }; diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts index 85a119f751..730e45ef0f 100644 --- a/packages/playwright-core/src/server/trace/recorder/tracing.ts +++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts @@ -24,7 +24,7 @@ import { commandsWithTracingSnapshots } from '../../../protocol/debug'; import { ManualPromise } from '../../../utils/manualPromise'; import type { RegisteredListener } from '../../../utils/eventsHelper'; import { eventsHelper } from '../../../utils/eventsHelper'; -import { assert, calculateSha1, createGuid, monotonicTime } from '../../../utils'; +import { assert, createGuid, monotonicTime } from '../../../utils'; import { mkdirIfNeeded, removeFolders } from '../../../utils/fileUtils'; import { Artifact } from '../../artifact'; import { BrowserContext } from '../../browserContext'; @@ -49,7 +49,6 @@ export type TracerOptions = { name?: string; snapshots?: boolean; screenshots?: boolean; - sources?: boolean; }; type RecordingState = { @@ -62,7 +61,6 @@ type RecordingState = { filesCount: number, networkSha1s: Set, traceSha1s: Set, - sources: Set, recording: boolean; }; @@ -133,7 +131,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps // and conflict. const traceName = options.name || createGuid(); // Init the state synchrounously. - this._state = { options, traceName, traceFile: '', networkFile: '', tracesDir: '', resourcesDir: '', filesCount: 0, traceSha1s: new Set(), networkSha1s: new Set(), sources: new Set(), recording: false }; + this._state = { options, traceName, traceFile: '', networkFile: '', tracesDir: '', resourcesDir: '', filesCount: 0, traceSha1s: new Set(), networkSha1s: new Set(), recording: false }; const state = this._state; state.tracesDir = await this._createTracesDirIfNeeded(); @@ -147,7 +145,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps async startChunk(options: { title?: string } = {}) { if (this._state && this._state.recording) - await this.stopChunk({ mode: 'doNotSave' }); + await this.stopChunk({ mode: 'discard' }); if (!this._state) throw new Error('Must start tracing before starting a new chunk'); @@ -220,16 +218,16 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps await this._writeChain; } - async stopChunk(params: TracingTracingStopChunkParams): Promise<{ artifact: Artifact | null, sourceEntries: NameValue[] | undefined }> { + async stopChunk(params: TracingTracingStopChunkParams): Promise<{ artifact?: Artifact, entries?: NameValue[], sourceEntries?: NameValue[] }> { if (this._isStopping) throw new Error(`Tracing is already stopping`); this._isStopping = true; if (!this._state || !this._state.recording) { this._isStopping = false; - if (params.mode !== 'doNotSave') + if (params.mode !== 'discard') throw new Error(`Must start tracing before stopping`); - return { artifact: null, sourceEntries: [] }; + return { sourceEntries: [] }; } const state = this._state!; @@ -256,10 +254,10 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps // Chain the export operation against write operations, // so that neither trace files nor sha1s change during the export. return await this._appendTraceOperation(async () => { - if (params.mode === 'doNotSave') - return { artifact: null, sourceEntries: undefined }; + if (params.mode === 'discard') + return {}; - // Har files a live, make a snapshot before returning the resulting entries. + // Har files are live, make a snapshot before returning the resulting entries. const networkFile = path.join(state.networkFile, '..', createGuid()); await fs.promises.copyFile(state.networkFile, networkFile); @@ -269,34 +267,21 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps for (const sha1 of new Set([...state.traceSha1s, ...state.networkSha1s])) entries.push({ name: path.join('resources', sha1), value: path.join(state.resourcesDir, sha1) }); - let sourceEntries: NameValue[] | undefined; - if (state.sources.size) { - sourceEntries = []; - for (const value of state.sources) { - const entry = { name: 'resources/src@' + calculateSha1(value) + '.txt', value }; - if (params.mode === 'compressTraceAndSources') { - if (fs.existsSync(entry.value)) - entries.push(entry); - } else { - sourceEntries.push(entry); - } - } - } - - const artifact = await this._exportZip(entries, state).catch(() => null); - return { artifact, sourceEntries }; + if (params.mode === 'entries') + return { entries }; + const artifact = await this._exportZip(entries, state).catch(() => undefined); + return { artifact, entries }; }).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, sourceEntries: undefined }; + }) || { }; } - private async _exportZip(entries: NameValue[], state: RecordingState): Promise { + private _exportZip(entries: NameValue[], state: RecordingState): Promise { const zipFile = new yazl.ZipFile(); - const result = new ManualPromise(); + const result = new ManualPromise(); (zipFile as any as EventEmitter).on('error', error => result.reject(error)); for (const entry of entries) zipFile.addFile(entry.value, entry.name); @@ -335,10 +320,6 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps 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; } diff --git a/packages/playwright-core/src/utils/index.ts b/packages/playwright-core/src/utils/index.ts index d9b79a3e6a..bab21bc674 100644 --- a/packages/playwright-core/src/utils/index.ts +++ b/packages/playwright-core/src/utils/index.ts @@ -35,5 +35,6 @@ export * from './stackTrace'; export * from './task'; export * from './time'; export * from './timeoutRunner'; +export * from './traceUtils'; export * from './userAgent'; export * from './zipFile'; diff --git a/packages/playwright-core/src/utils/traceUtils.ts b/packages/playwright-core/src/utils/traceUtils.ts new file mode 100644 index 0000000000..8aea01fe8f --- /dev/null +++ b/packages/playwright-core/src/utils/traceUtils.ts @@ -0,0 +1,41 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { ClientSideCallMetadata, StackFrame } from '@protocol/channels'; +import type { SerializedClientSideCallMetadata } from '@trace/traceUtils'; + +export function serializeClientSideCallMetadata(metadatas: ClientSideCallMetadata[]): SerializedClientSideCallMetadata { + const stackFrames = new Map(); + const frames: StackFrame[] = []; + const stacks: [number, number[]][] = []; + for (const m of metadatas) { + if (!m.stack || !m.stack.length) + continue; + const stack: number[] = []; + for (const frame of m.stack) { + const key = `${frame.file}:${frame.line || 0}:${frame.column || 0}`; + let ordinal = stackFrames.get(key); + if (typeof ordinal !== 'number') { + ordinal = stackFrames.size; + stackFrames.set(key, ordinal); + frames.push(frame); + } + stack.push(ordinal); + } + stacks.push([m.id, stack]); + } + return { frames, stacks }; +} diff --git a/packages/protocol/src/callMetadata.ts b/packages/protocol/src/callMetadata.ts index 370bd245af..6444623820 100644 --- a/packages/protocol/src/callMetadata.ts +++ b/packages/protocol/src/callMetadata.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import type { Point, StackFrame, SerializedError } from './channels'; +import type { Point, SerializedError } from './channels'; export type CallMetadata = { id: string; @@ -33,7 +33,7 @@ export type CallMetadata = { // Service-side is making a call to itself, this metadata does not go // through the dispatcher, so is always excluded from inspector / tracing. isServerSide?: boolean; - stack?: StackFrame[]; + location?: { file: string, line?: number, column?: number }; log: string[]; afterSnapshot?: string; snapshots: { title: string, snapshotName: string }[]; diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts index 3e75809e84..2dada44a55 100644 --- a/packages/protocol/src/channels.ts +++ b/packages/protocol/src/channels.ts @@ -143,11 +143,20 @@ export type StackFrame = { }; export type Metadata = { - stack?: StackFrame[], + location?: { + file: string, + line?: number, + column?: number, + }, apiName?: string, internal?: boolean, }; +export type ClientSideCallMetadata = { + id: number, + stack?: StackFrame[], +}; + export type Point = { x: number, y: number, @@ -394,6 +403,9 @@ export interface LocalUtilsChannel extends LocalUtilsEventTarget, Channel { export type LocalUtilsZipParams = { zipFile: string, entries: NameValue[], + mode: 'write' | 'append', + metadata: ClientSideCallMetadata[], + includeSources: boolean, }; export type LocalUtilsZipOptions = { @@ -3741,14 +3753,14 @@ export type TracingTracingStartChunkOptions = { }; export type TracingTracingStartChunkResult = void; export type TracingTracingStopChunkParams = { - mode: 'doNotSave' | 'compressTrace' | 'compressTraceAndSources', + mode: 'archive' | 'discard' | 'entries', }; export type TracingTracingStopChunkOptions = { }; export type TracingTracingStopChunkResult = { artifact?: ArtifactChannel, - sourceEntries?: NameValue[], + entries?: NameValue[], }; export type TracingTracingStopParams = {}; export type TracingTracingStopOptions = {}; diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index 0551af5876..bbe7c001d6 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -25,12 +25,22 @@ StackFrame: Metadata: type: object properties: - stack: - type: array? - items: StackFrame + location: + type: object? + properties: + file: string + line: number? + column: number? apiName: string? internal: boolean? +ClientSideCallMetadata: + type: object + properties: + id: number + stack: + type: array? + items: StackFrame Point: type: object @@ -488,6 +498,15 @@ LocalUtils: entries: type: array items: NameValue + mode: + type: enum + literals: + - write + - append + metadata: + type: array + items: ClientSideCallMetadata + includeSources: boolean harOpen: parameters: @@ -2911,13 +2930,15 @@ Tracing: mode: type: enum literals: - - doNotSave - - compressTrace - - compressTraceAndSources + - archive + - discard + - entries returns: - # The artifact may be missing if the browser closes while tracing is beeing stopped. + # The artifact may be missing if the browser closes while tracing is being stopped. + # Or it can be missing if client-side compression is taking place. artifact: Artifact? - sourceEntries: + # For local mode, these are all entries. + entries: type: array? items: NameValue diff --git a/packages/trace-viewer/src/traceModel.ts b/packages/trace-viewer/src/traceModel.ts index 30f9cb35ed..de7ad6f581 100644 --- a/packages/trace-viewer/src/traceModel.ts +++ b/packages/trace-viewer/src/traceModel.ts @@ -16,6 +16,7 @@ import type { CallMetadata } from '@protocol/callMetadata'; import type * as trace from '@trace/trace'; +import { parseClientSideCallMetadata } from '@trace/traceUtils'; import type zip from '@zip.js/zip.js'; // @ts-ignore import zipImport from '@zip.js/zip.js/dist/zip-no-worker-inflate.min.js'; @@ -52,11 +53,14 @@ export class TraceModel { { useWebWorkers: false }) as zip.ZipReader; let traceEntry: zip.Entry | undefined; let networkEntry: zip.Entry | undefined; + let stacksEntry: zip.Entry | undefined; for (const entry of await this._zipReader.getEntries({ onprogress: progress })) { if (entry.filename.endsWith('.trace')) traceEntry = entry; if (entry.filename.endsWith('.network')) networkEntry = entry; + if (entry.filename.endsWith('.stacks')) + stacksEntry = entry; if (entry.filename.includes('src@')) this.contextEntry.hasSource = true; this._entries.set(entry.filename, entry); @@ -77,6 +81,15 @@ export class TraceModel { for (const line of (await networkWriter.getData()).split('\n')) this.appendEvent(line); } + + if (stacksEntry) { + const writer = new zipjs.TextWriter(); + await stacksEntry.getData!(writer); + const metadataMap = parseClientSideCallMetadata(JSON.parse(await writer.getData())); + for (const action of this.contextEntry.actions) + action.metadata.stack = metadataMap.get(action.metadata.id); + } + this._build(); } diff --git a/packages/trace-viewer/vite.sw.config.ts b/packages/trace-viewer/vite.sw.config.ts index 325fb0681c..f4da45137d 100644 --- a/packages/trace-viewer/vite.sw.config.ts +++ b/packages/trace-viewer/vite.sw.config.ts @@ -31,6 +31,7 @@ export default defineConfig({ alias: { '@isomorphic': path.resolve(__dirname, '../playwright-core/src/server/isomorphic'), '@protocol': path.resolve(__dirname, '../protocol/src'), + '@trace': path.resolve(__dirname, '../trace/src'), '@web': path.resolve(__dirname, '../web/src'), }, }, diff --git a/packages/trace/src/trace.ts b/packages/trace/src/trace.ts index f1fac195bb..e1e5dcb253 100644 --- a/packages/trace/src/trace.ts +++ b/packages/trace/src/trace.ts @@ -15,6 +15,7 @@ */ import type { CallMetadata } from '@protocol/callMetadata'; +import type { StackFrame } from '@protocol/channels'; import type { Language } from '../../playwright-core/src/server/isomorphic/locatorGenerators'; import type { FrameSnapshot, ResourceSnapshot } from './snapshot'; @@ -53,7 +54,7 @@ export type ScreencastFrameTraceEvent = { export type ActionTraceEvent = { type: 'action' | 'event', - metadata: CallMetadata, + metadata: CallMetadata & { stack?: StackFrame[] }, }; export type ResourceSnapshotTraceEvent = { diff --git a/packages/trace/src/traceUtils.ts b/packages/trace/src/traceUtils.ts new file mode 100644 index 0000000000..f72fb6fe73 --- /dev/null +++ b/packages/trace/src/traceUtils.ts @@ -0,0 +1,32 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { StackFrame } from '@protocol/channels'; + +export type SerializedClientSideCallMetadata = { + frames: StackFrame[]; + stacks: [number, number[]][]; +}; + +export function parseClientSideCallMetadata(data: SerializedClientSideCallMetadata): Map { + const result = new Map(); + const { frames, stacks } = data; + for (const s of stacks) { + const [id, ff] = s; + result.set(`call@${id}`, ff.map((f: number) => frames[f])); + } + return result; +} diff --git a/tests/config/utils.ts b/tests/config/utils.ts index a946a6807e..1526dd2dbe 100644 --- a/tests/config/utils.ts +++ b/tests/config/utils.ts @@ -16,6 +16,8 @@ import type { Frame, Page } from 'playwright-core'; import { ZipFile } from '../../packages/playwright-core/lib/utils/zipFile'; +import type { StackFrame } from '@protocol/channels'; +import { parseClientSideCallMetadata } from '../../packages/trace/src/traceUtils'; export async function attachFrame(page: Page, frameId: string, url: string): Promise { const handle = await page.evaluateHandle(async ({ frameId, url }) => { @@ -91,7 +93,7 @@ export function suppressCertificateWarning() { }; } -export async function parseTrace(file: string): Promise<{ events: any[], resources: Map, actions: string[] }> { +export async function parseTrace(file: string): Promise<{ events: any[], resources: Map, actions: string[], stacks: Map }> { const zipFS = new ZipFile(file); const resources = new Map(); for (const entry of await zipFS.entries()) @@ -103,14 +105,18 @@ export async function parseTrace(file: string): Promise<{ events: any[], resourc if (line) events.push(JSON.parse(line)); } + for (const line of resources.get('trace.network').toString().split('\n')) { if (line) events.push(JSON.parse(line)); } + + const stacks = parseClientSideCallMetadata(JSON.parse(resources.get('trace.stacks').toString())); return { events, resources, - actions: eventsToActions(events) + actions: eventsToActions(events), + stacks, }; } diff --git a/tests/library/tracing.spec.ts b/tests/library/tracing.spec.ts index c9792d0b17..2658f8762b 100644 --- a/tests/library/tracing.spec.ts +++ b/tests/library/tracing.spec.ts @@ -19,6 +19,7 @@ import { jpegjs } from 'playwright-core/lib/utilsBundle'; import path from 'path'; import { browserTest, contextTest as test, expect } from '../config/browserTest'; import { parseTrace } from '../config/utils'; +import type { StackFrame } from '@protocol/channels'; test.skip(({ trace }) => trace === 'on'); @@ -208,8 +209,8 @@ test('should not include trace resources from the provious chunks', async ({ con // 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); + // 0 source file for the second test. + expect(names.filter(n => n.endsWith('.txt')).length).toBe(0); } }); @@ -474,7 +475,7 @@ test('should hide internal stack frames', async ({ context, page }, testInfo) => const actions = trace.events.filter(e => e.type === 'action' && !e.metadata.apiName.startsWith('tracing.')); expect(actions).toHaveLength(4); for (const action of actions) - expect(relativeStack(action)).toEqual(['tracing.spec.ts']); + expect(relativeStack(action, trace.stacks)).toEqual(['tracing.spec.ts']); }); test('should hide internal stack frames in expect', async ({ context, page }, testInfo) => { @@ -495,7 +496,7 @@ test('should hide internal stack frames in expect', async ({ context, page }, te const actions = trace.events.filter(e => e.type === 'action' && !e.metadata.apiName.startsWith('tracing.')); expect(actions).toHaveLength(5); for (const action of actions) - expect(relativeStack(action)).toEqual(['tracing.spec.ts']); + expect(relativeStack(action, trace.stacks)).toEqual(['tracing.spec.ts']); }); test('should record global request trace', async ({ request, context, server }, testInfo) => { @@ -605,6 +606,7 @@ function expectBlue(pixels: Buffer, offset: number) { expect(a).toBe(255); } -function relativeStack(action: any): string[] { - return action.metadata.stack.map(f => f.file.replace(__dirname + path.sep, '')); +function relativeStack(action: any, stacks: Map): string[] { + const stack = stacks.get(action.metadata.id) || []; + return stack.map(f => f.file.replace(__dirname + path.sep, '')); }