Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add time information to Call and Network tabs in Trace Viewer #33935

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+66 −34
Open

feat: Add time information to Call and Network tabs in Trace Viewer #33935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d13736d
feat: Add startTime to call tab
agg23 Dec 9, 2024
ef78bc6
Cleaned up callTab React code
agg23 Dec 9, 2024
d022d77
Time support in Network details tab
agg23 Dec 10, 2024
2b24d05
Fix tests that check call tab contents
agg23 Dec 10, 2024
d570b89
Merge branch 'main' of https://github.com/microsoft/playwright into s…
agg23 Dec 18, 2024
4bdd625
Address PR comments
agg23 Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 45 additions & 27 deletions packages/trace-viewer/src/ui/callTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,50 +27,68 @@ import type { ActionTraceEventInContext } from './modelUtil';

export const CallTab: React.FunctionComponent<{
action: ActionTraceEventInContext | undefined,
executionStartTime: number,
executionStartWallTime: number,
sdkLanguage: Language | undefined,
}> = ({ action, sdkLanguage }) => {
}> = ({ action, executionStartTime, executionStartWallTime, sdkLanguage }) => {
// We never need the waitForEventInfo (`info`).
const paramKeys = React.useMemo(() => Object.keys(action?.params ?? {}).filter(name => name !== 'info'), [action]);

if (!action)
return <PlaceholderPanel text='No action selected' />;
const params = { ...action.params };
// Strip down the waitForEventInfo data, we never need it.
delete params.info;
const paramKeys = Object.keys(params);
const timeMillis = action.startTime + (action.context.wallTime - action.context.startTime);
const wallTime = new Date(timeMillis).toLocaleString();

// Calculate execution time relative to the test runner's start time
const startTimeMillis = action.startTime - executionStartTime;
const startTime = msToString(startTimeMillis);

const wallTimeMillis = startTimeMillis + executionStartWallTime;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be just a guess at this point as we can't add ticks to millis, do we really want to explicitly mislead? I would omit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Offline discussion with Yury where I am saying that we should not do wall time math: maybe we could formalize model.startTime and then we can remove the wallTime from individual actions and operate in terms or ticks (monotonic time) for actions only.

const wallTime = new Date(wallTimeMillis).toLocaleString(undefined, { timeZoneName: 'short' });

const duration = action.endTime ? msToString(action.endTime - action.startTime) : 'Timed Out';

return <div className='call-tab'>
<div className='call-line'>{action.apiName}</div>
{<>
<div className='call-section'>Time</div>
{wallTime && <div className='call-line'>wall time:<span className='call-value datetime' title={wallTime}>{wallTime}</span></div>}
<div className='call-line'>duration:<span className='call-value datetime' title={duration}>{duration}</span></div>
</>}
{ !!paramKeys.length && <div className='call-section'>Parameters</div> }
{
!!paramKeys.length && paramKeys.map((name, index) => renderProperty(propertyToString(action, name, params[name], sdkLanguage), 'param-' + index))
}
{ !!action.result && <div className='call-section'>Return value</div> }
{
!!action.result && Object.keys(action.result).map((name, index) =>
renderProperty(propertyToString(action, name, action.result[name], sdkLanguage), 'result-' + index)
)
}
</div>;
return (
<div className='call-tab'>
<div className='call-line'>{action.apiName}</div>
{
<>
<div className='call-section'>Time</div>
<DateTimeCallLine name='wall time:' value={wallTime} />
<DateTimeCallLine name='start:' value={startTime} />
<DateTimeCallLine name='duration:' value={duration} />
</>
}
{
!!paramKeys.length && <>
<div className='call-section'>Parameters</div>
{paramKeys.map(name => renderProperty(propertyToString(action, name, action.params[name], sdkLanguage)))}
</>
}
{
!!action.result && <>
<div className='call-section'>Return value</div>
{Object.keys(action.result).map(name =>
renderProperty(propertyToString(action, name, action.result[name], sdkLanguage))
)}
</>
}
</div>
);
};

const DateTimeCallLine: React.FC<{ name: string, value: string }> = ({ name, value }) => <div className='call-line'>{name}<span className='call-value datetime' title={value}>{value}</span></div>;

type Property = {
name: string;
type: 'string' | 'number' | 'object' | 'locator' | 'handle' | 'bigint' | 'boolean' | 'symbol' | 'undefined' | 'function';
text: string;
};

function renderProperty(property: Property, key: string) {
function renderProperty(property: Property) {
let text = property.text.replace(/\n/g, '↵');
if (property.type === 'string')
text = `"${text}"`;
return (
<div key={key} className='call-line'>
<div key={property.name} className='call-line'>
{property.name}:<span className={clsx('call-value', property.type)} title={property.text}>{text}</span>
{ ['string', 'number', 'object', 'locator'].includes(property.type) &&
<CopyToClipboard value={property.text} />
Expand Down
4 changes: 3 additions & 1 deletion packages/trace-viewer/src/ui/metadataView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ export const MetadataView: React.FunctionComponent<{
if (!model)
return <></>;

const wallTime = model.wallTime !== undefined ? new Date(model.wallTime).toLocaleString(undefined, { timeZoneName: 'short' }) : undefined;

return <div data-testid='metadata-view' className='vbox' style={{ flexShrink: 0 }}>
<div className='call-section' style={{ paddingTop: 2 }}>Time</div>
{!!model.wallTime && <div className='call-line'>start time:<span className='call-value datetime' title={new Date(model.wallTime).toLocaleString()}>{new Date(model.wallTime).toLocaleString()}</span></div>}
{!!wallTime && <div className='call-line'>start time:<span className='call-value datetime' title={wallTime}>{wallTime}</span></div>}
<div className='call-line'>duration:<span className='call-value number' title={msToString(model.endTime - model.startTime)}>{msToString(model.endTime - model.startTime)}</span></div>
<div className='call-section'>Browser</div>
<div className='call-line'>engine:<span className='call-value string' title={model.browserName}>{model.browserName}</span></div>
Expand Down
17 changes: 13 additions & 4 deletions packages/trace-viewer/src/ui/networkResourceDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ import { generateCurlCommand, generateFetchCall } from '../third_party/devtools'
import { CopyToClipboardTextButton } from './copyToClipboard';
import { getAPIRequestCodeGen } from './codegen';
import type { Language } from '@isomorphic/locatorGenerators';
import { msToString } from '@web/uiUtils';

export const NetworkResourceDetails: React.FunctionComponent<{
resource: ResourceSnapshot;
onClose: () => void;
sdkLanguage: Language;
}> = ({ resource, onClose, sdkLanguage }) => {
startTimeOffset: number;
onClose: () => void;
}> = ({ resource, sdkLanguage, startTimeOffset, onClose }) => {
const [selectedTab, setSelectedTab] = React.useState('request');

return <TabbedPane
Expand All @@ -39,7 +41,7 @@ export const NetworkResourceDetails: React.FunctionComponent<{
{
id: 'request',
title: 'Request',
render: () => <RequestTab resource={resource} sdkLanguage={sdkLanguage} />,
render: () => <RequestTab resource={resource} sdkLanguage={sdkLanguage} startTimeOffset={startTimeOffset} />,
},
{
id: 'response',
Expand All @@ -59,9 +61,12 @@ export const NetworkResourceDetails: React.FunctionComponent<{
const RequestTab: React.FunctionComponent<{
resource: ResourceSnapshot;
sdkLanguage: Language;
}> = ({ resource, sdkLanguage }) => {
startTimeOffset: number;
}> = ({ resource, sdkLanguage, startTimeOffset }) => {
const [requestBody, setRequestBody] = React.useState<{ text: string, mimeType?: string } | null>(null);

const wallTimeString = React.useMemo(() => resource.startedDateTime.length > 0 ? new Date(resource.startedDateTime).toLocaleString(undefined, { timeZoneName: 'short' }) : '-', [resource.startedDateTime]);

React.useEffect(() => {
const readResources = async () => {
if (resource.request.postData) {
Expand Down Expand Up @@ -96,6 +101,10 @@ const RequestTab: React.FunctionComponent<{
</> : null}
<div className='network-request-details-header'>Request Headers</div>
<div className='network-request-details-headers'>{resource.request.headers.map(pair => `${pair.name}: ${pair.value}`).join('\n')}</div>
<div className='network-request-details-header'>Time</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point in showing these values in the details, given that they are already present in the network table where one can see other network requests and compare their start time/duration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a strange omission for it to not be available. Why would expanding the details of something show fewer details?

While you're right that they can only compare when the details are not open, I think it's still valuable to be able to view all the metrics on this screen.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree with you that it's nice to see this information in the details view, I still cannot come up with a good explanation why it should be shown above request headers. Maybe push it below headers at least? We should probably put timing information into its own tab and populate it with other details that we have, but I don't remember anyone asking for this.

<div className='network-request-details-general'>{`Wall Time: ${wallTimeString}`}</div>
<div className='network-request-details-general'>{`Start: ${msToString(startTimeOffset)}`}</div>
<div className='network-request-details-general'>{`Duration: ${msToString(resource.time)}`}</div>

<div className='network-request-details-copy'>
<CopyToClipboardTextButton description='Copy as cURL' value={() => generateCurlCommand(resource)} />
Expand Down
2 changes: 1 addition & 1 deletion packages/trace-viewer/src/ui/networkTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export const NetworkTab: React.FunctionComponent<{
sidebarIsFirst={true}
orientation='horizontal'
settingName='networkResourceDetails'
main={<NetworkResourceDetails resource={selectedEntry.resource} onClose={() => setSelectedEntry(undefined)} sdkLanguage={sdkLanguage} />}
main={<NetworkResourceDetails resource={selectedEntry.resource} sdkLanguage={sdkLanguage} startTimeOffset={selectedEntry.start} onClose={() => setSelectedEntry(undefined)} />}
sidebar={grid}
/>}
</>;
Expand Down
2 changes: 1 addition & 1 deletion packages/trace-viewer/src/ui/workbench.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ export const Workbench: React.FunctionComponent<{
const callTab: TabbedPaneTabModel = {
id: 'call',
title: 'Call',
render: () => <CallTab action={activeAction} sdkLanguage={sdkLanguage} />
render: () => <CallTab action={activeAction} executionStartTime={model?.startTime ?? 0} executionStartWallTime={model?.wallTime ?? 0} sdkLanguage={sdkLanguage} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "execution" helps here, start time usually applies to execution, maybe name it timeOrigin?

};
const logTab: TabbedPaneTabModel = {
id: 'log',
Expand Down
3 changes: 3 additions & 0 deletions tests/library/trace-viewer.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ test('should show params and return value', async ({ showTraceViewer }) => {
await expect(traceViewer.callLines).toHaveText([
/page.evaluate/,
/wall time:[0-9/:,APM ]+/,
/start:[\d\.]+m?s/,
/duration:[\d]+ms/,
/expression:"\({↵ a↵ }\) => {↵ console\.log\(\'Info\'\);↵ console\.warn\(\'Warning\'\);↵ console/,
'isFunction:true',
Expand All @@ -252,6 +253,7 @@ test('should show params and return value', async ({ showTraceViewer }) => {
await expect(traceViewer.callLines).toContainText([
/expect.toHaveText/,
/wall time:[0-9/:,APM ]+/,
/start:[\d\.]+m?s/,
/duration:[\d]+ms/,
/locator:locator\('button'\)/,
/expression:"to.have.text"/,
Expand All @@ -267,6 +269,7 @@ test('should show null as a param', async ({ showTraceViewer, browserName }) =>
await expect(traceViewer.callLines).toHaveText([
/page.evaluate/,
/wall time:[0-9/:,APM ]+/,
/start:[\d\.]+m?s/,
/duration:[\d]+ms/,
'expression:"() => 1 + 1"',
'isFunction:true',
Expand Down
Loading