Skip to content

Commit

Permalink
ref(tracing): Remove script evaluation span (#4433)
Browse files Browse the repository at this point in the history
Tracking the script evaluation span is inconsistent, and often doesn't
show up in most pageload transactions. In addition, the value of this
span is not clear.

To reduce bundle size and complexity, this patch removes the script
evaluation span.
  • Loading branch information
AbhiPrasad committed Jan 21, 2022
1 parent 2252c5c commit fcd97c4
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 41 deletions.
41 changes: 3 additions & 38 deletions packages/tracing/src/browser/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,7 @@ export class MetricsInstrumentation {
logger.log('[Tracing] Adding & adjusting spans using Performance API');

const timeOrigin = msToSec(browserPerformanceTimeOrigin);
let entryScriptSrc = '';

if (global.document && global.document.scripts) {
// eslint-disable-next-line @typescript-eslint/prefer-for-of
for (let i = 0; i < global.document.scripts.length; i++) {
// We go through all scripts on the page and look for 'data-entry'
// We remember the name and measure the time between this script finished loading and
// our mark 'sentry-tracing-init'
if (global.document.scripts[i].dataset.entry === 'true') {
entryScriptSrc = global.document.scripts[i].src;
break;
}
}
}

let entryScriptStartTimestamp: number | undefined;
let tracingInitMarkStartTime: number | undefined;
let responseStartTimestamp: number | undefined;
let requestStartTimestamp: number | undefined;

Expand All @@ -86,10 +70,6 @@ export class MetricsInstrumentation {
case 'paint':
case 'measure': {
const startTimestamp = addMeasureSpans(transaction, entry, startTime, duration, timeOrigin);
if (tracingInitMarkStartTime === undefined && entry.name === 'sentry-tracing-init') {
tracingInitMarkStartTime = startTimestamp;
}

// capture web vitals

const firstHidden = getVisibilityWatcher();
Expand All @@ -112,27 +92,14 @@ export class MetricsInstrumentation {
}
case 'resource': {
const resourceName = (entry.name as string).replace(global.location.origin, '');
const endTimestamp = addResourceSpans(transaction, entry, resourceName, startTime, duration, timeOrigin);
// We remember the entry script end time to calculate the difference to the first init mark
if (entryScriptStartTimestamp === undefined && entryScriptSrc.indexOf(resourceName) > -1) {
entryScriptStartTimestamp = endTimestamp;
}
addResourceSpans(transaction, entry, resourceName, startTime, duration, timeOrigin);
break;
}
default:
// Ignore other entry types.
}
});

if (entryScriptStartTimestamp !== undefined && tracingInitMarkStartTime !== undefined) {
_startChild(transaction, {
description: 'evaluation',
endTimestamp: tracingInitMarkStartTime,
op: 'script',
startTimestamp: entryScriptStartTimestamp,
});
}

this._performanceCursor = Math.max(performance.getEntries().length - 1, 0);

this._trackNavigator(transaction);
Expand Down Expand Up @@ -335,11 +302,11 @@ export function addResourceSpans(
startTime: number,
duration: number,
timeOrigin: number,
): number | undefined {
): void {
// we already instrument based on fetch and xhr, so we don't need to
// duplicate spans here.
if (entry.initiatorType === 'xmlhttprequest' || entry.initiatorType === 'fetch') {
return undefined;
return;
}

const data: Record<string, any> = {};
Expand All @@ -363,8 +330,6 @@ export function addResourceSpans(
startTimestamp,
data,
});

return endTimestamp;
}

/** Create performance navigation related spans */
Expand Down
4 changes: 1 addition & 3 deletions packages/tracing/test/browser/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('addResourceSpans', () => {
const startTime = 23;
const duration = 356;

const endTimestamp = addResourceSpans(transaction, entry, '/assets/to/css', startTime, duration, timeOrigin);
addResourceSpans(transaction, entry, '/assets/to/css', startTime, duration, timeOrigin);

// eslint-disable-next-line @typescript-eslint/unbound-method
expect(transaction.startChild).toHaveBeenCalledTimes(1);
Expand All @@ -110,8 +110,6 @@ describe('addResourceSpans', () => {
op: 'resource.css',
startTimestamp: timeOrigin + startTime,
});

expect(endTimestamp).toBe(timeOrigin + startTime + duration);
});

it('creates a variety of resource spans', () => {
Expand Down

0 comments on commit fcd97c4

Please sign in to comment.