Skip to content

Commit

Permalink
fix(tracing): Don't send negative ttfb (#10286)
Browse files Browse the repository at this point in the history
As per
https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStart,
`responseStart` can be 0 if the request is coming straight from the
cache. This might lead us to calculate a negative ttfb.

To account for these scenarios, use `Math.max` to make sure we always
set to 0 in the case of a negative value.
  • Loading branch information
AbhiPrasad authored Jan 23, 2024
1 parent 675309d commit c017181
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 18 deletions.
57 changes: 39 additions & 18 deletions packages/tracing-internal/src/browser/metrics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,24 +240,7 @@ export function addPerformanceEntries(transaction: Transaction): void {

// Measurements are only available for pageload transactions
if (op === 'pageload') {
// Generate TTFB (Time to First Byte), which measured as the time between the beginning of the transaction and the
// start of the response in milliseconds
if (typeof responseStartTimestamp === 'number' && transactionStartTime) {
DEBUG_BUILD && logger.log('[Measurements] Adding TTFB');
_measurements['ttfb'] = {
value: (responseStartTimestamp - transactionStartTime) * 1000,
unit: 'millisecond',
};

if (typeof requestStartTimestamp === 'number' && requestStartTimestamp <= responseStartTimestamp) {
// Capture the time spent making the request and receiving the first byte of the response.
// This is the time between the start of the request and the start of the response in milliseconds.
_measurements['ttfb.requestTime'] = {
value: (responseStartTimestamp - requestStartTimestamp) * 1000,
unit: 'millisecond',
};
}
}
_addTtfbToMeasurements(_measurements, responseStartTimestamp, requestStartTimestamp, transactionStartTime);

['fcp', 'fp', 'lcp'].forEach(name => {
if (!_measurements[name] || !transactionStartTime || timeOrigin >= transactionStartTime) {
Expand Down Expand Up @@ -547,3 +530,41 @@ function setResourceEntrySizeData(
data[dataKey] = entryVal;
}
}

/**
* Add ttfb information to measurements
*
* Exported for tests
*/
export function _addTtfbToMeasurements(
_measurements: Measurements,
responseStartTimestamp: number | undefined,
requestStartTimestamp: number | undefined,
transactionStartTime: number | undefined,
): void {
// Generate TTFB (Time to First Byte), which measured as the time between the beginning of the transaction and the
// start of the response in milliseconds
if (typeof responseStartTimestamp === 'number' && transactionStartTime) {
DEBUG_BUILD && logger.log('[Measurements] Adding TTFB');
_measurements['ttfb'] = {
// As per https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStart,
// responseStart can be 0 if the request is coming straight from the cache.
// This might lead us to calculate a negative ttfb if we don't use Math.max here.
//
// This logic is the same as what is in the web-vitals library to calculate ttfb
// https://github.com/GoogleChrome/web-vitals/blob/2301de5015e82b09925238a228a0893635854587/src/onTTFB.ts#L92
// TODO(abhi): We should use the web-vitals library instead of this custom calculation.
value: Math.max(responseStartTimestamp - transactionStartTime, 0) * 1000,
unit: 'millisecond',
};

if (typeof requestStartTimestamp === 'number' && requestStartTimestamp <= responseStartTimestamp) {
// Capture the time spent making the request and receiving the first byte of the response.
// This is the time between the start of the request and the start of the response in milliseconds.
_measurements['ttfb.requestTime'] = {
value: (responseStartTimestamp - requestStartTimestamp) * 1000,
unit: 'millisecond',
};
}
}
}
29 changes: 29 additions & 0 deletions packages/tracing-internal/test/browser/metrics/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Transaction } from '../../../src';
import type { ResourceEntry } from '../../../src/browser/metrics';
import { _addTtfbToMeasurements } from '../../../src/browser/metrics';
import { _addMeasureSpans, _addResourceSpans } from '../../../src/browser/metrics';
import { WINDOW } from '../../../src/browser/types';

Expand Down Expand Up @@ -261,6 +262,34 @@ describe('_addResourceSpans', () => {
});
});

describe('_addTtfbToMeasurements', () => {
it('adds ttfb to measurements', () => {
const measurements = {};
_addTtfbToMeasurements(measurements, 300, 200, 100);
expect(measurements).toEqual({
ttfb: {
unit: 'millisecond',
value: 200000,
},
'ttfb.requestTime': {
unit: 'millisecond',
value: 100000,
},
});
});

it('does not add negative ttfb', () => {
const measurements = {};
_addTtfbToMeasurements(measurements, 100, 200, 300);
expect(measurements).toEqual({
ttfb: {
unit: 'millisecond',
value: 0,
},
});
});
});

const setGlobalLocation = (location: Location) => {
// @ts-expect-error need to delete this in order to set to new value
delete WINDOW.location;
Expand Down

0 comments on commit c017181

Please sign in to comment.