Skip to content

Commit

Permalink
ref(measurements): Update setMeasurements to only set a single meas…
Browse files Browse the repository at this point in the history
…urement (#4920)
  • Loading branch information
lforst committed Apr 13, 2022
1 parent 6445db2 commit 20b8c52
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const transaction = Sentry.startTransaction({ name: 'some_transaction' });

transaction.setMeasurement('metric.foo', 42, 'ms');
transaction.setMeasurement('metric.bar', 1337, 'nanoseconds');
transaction.setMeasurement('metric.baz', 99, 's');
transaction.setMeasurement('metric.baz', 1);

transaction.finish();
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { expect } from '@playwright/test';
import { Event } from '@sentry/types';

import { sentryTest } from '../../../../utils/fixtures';
import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';

sentryTest('should attach measurement to transaction', async ({ getLocalTestPath, page }) => {
const url = await getLocalTestPath({ testDir: __dirname });
const event = await getFirstSentryEnvelopeRequest<Event>(page, url);

expect(event.measurements?.['metric.foo'].value).toBe(42);
expect(event.measurements?.['metric.bar'].value).toBe(1337);
expect(event.measurements?.['metric.baz'].value).toBe(1);

expect(event.measurements?.['metric.foo'].unit).toBe('ms');
expect(event.measurements?.['metric.bar'].unit).toBe('nanoseconds');
expect(event.measurements?.['metric.baz'].unit).toBe('');
});
43 changes: 28 additions & 15 deletions packages/tracing/src/browser/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,14 @@ export class MetricsInstrumentation {

if (entry.name === 'first-paint' && shouldRecord) {
IS_DEBUG_BUILD && logger.log('[Measurements] Adding FP');
this._measurements['fp'] = { value: entry.startTime };
this._measurements['mark.fp'] = { value: startTimestamp };
this._measurements['fp'] = { value: entry.startTime, unit: 'millisecond' };
this._measurements['mark.fp'] = { value: startTimestamp, unit: 'second' };
}

if (entry.name === 'first-contentful-paint' && shouldRecord) {
IS_DEBUG_BUILD && logger.log('[Measurements] Adding FCP');
this._measurements['fcp'] = { value: entry.startTime };
this._measurements['mark.fcp'] = { value: startTimestamp };
this._measurements['fcp'] = { value: entry.startTime, unit: 'millisecond' };
this._measurements['mark.fcp'] = { value: startTimestamp, unit: 'second' };
}

break;
Expand Down Expand Up @@ -115,12 +115,18 @@ export class MetricsInstrumentation {
// start of the response in milliseconds
if (typeof responseStartTimestamp === 'number') {
IS_DEBUG_BUILD && logger.log('[Measurements] Adding TTFB');
this._measurements['ttfb'] = { value: (responseStartTimestamp - transaction.startTimestamp) * 1000 };
this._measurements['ttfb'] = {
value: (responseStartTimestamp - transaction.startTimestamp) * 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.
this._measurements['ttfb.requestTime'] = { value: (responseStartTimestamp - requestStartTimestamp) * 1000 };
this._measurements['ttfb.requestTime'] = {
value: (responseStartTimestamp - requestStartTimestamp) * 1000,
unit: 'second',
};
}
}

Expand Down Expand Up @@ -162,7 +168,14 @@ export class MetricsInstrumentation {
delete this._measurements.cls;
}

transaction.setMeasurements(this._measurements);
Object.keys(this._measurements).forEach(measurementName => {
transaction.setMeasurement(
measurementName,
this._measurements[measurementName].value,
this._measurements[measurementName].unit,
);
});

tagMetricInfo(transaction, this._lcpEntry, this._clsEntry);
transaction.setTag('sentry_reportAllChanges', this._reportAllChanges);
}
Expand All @@ -189,11 +202,11 @@ export class MetricsInstrumentation {
}

if (isMeasurementValue(connection.rtt)) {
this._measurements['connection.rtt'] = { value: connection.rtt as number };
this._measurements['connection.rtt'] = { value: connection.rtt, unit: 'millisecond' };
}

if (isMeasurementValue(connection.downlink)) {
this._measurements['connection.downlink'] = { value: connection.downlink as number };
this._measurements['connection.downlink'] = { value: connection.downlink, unit: '' }; // unit is empty string for now, while relay doesn't support download speed units
}
}

Expand All @@ -218,7 +231,7 @@ export class MetricsInstrumentation {
}

IS_DEBUG_BUILD && logger.log('[Measurements] Adding CLS');
this._measurements['cls'] = { value: metric.value };
this._measurements['cls'] = { value: metric.value, unit: 'millisecond' };
this._clsEntry = entry as LayoutShift;
});
}
Expand All @@ -234,8 +247,8 @@ export class MetricsInstrumentation {
const timeOrigin = msToSec(browserPerformanceTimeOrigin as number);
const startTime = msToSec(entry.startTime);
IS_DEBUG_BUILD && logger.log('[Measurements] Adding LCP');
this._measurements['lcp'] = { value: metric.value };
this._measurements['mark.lcp'] = { value: timeOrigin + startTime };
this._measurements['lcp'] = { value: metric.value, unit: 'millisecond' };
this._measurements['mark.lcp'] = { value: timeOrigin + startTime, unit: 'second' };
this._lcpEntry = entry as LargestContentfulPaint;
}, this._reportAllChanges);
}
Expand All @@ -251,8 +264,8 @@ export class MetricsInstrumentation {
const timeOrigin = msToSec(browserPerformanceTimeOrigin as number);
const startTime = msToSec(entry.startTime);
IS_DEBUG_BUILD && logger.log('[Measurements] Adding FID');
this._measurements['fid'] = { value: metric.value };
this._measurements['mark.fid'] = { value: timeOrigin + startTime };
this._measurements['fid'] = { value: metric.value, unit: 'millisecond' };
this._measurements['mark.fid'] = { value: timeOrigin + startTime, unit: 'second' };
});
}
}
Expand Down Expand Up @@ -392,7 +405,7 @@ export function _startChild(transaction: Transaction, { startTimestamp, ...ctx }
/**
* Checks if a given value is a valid measurement value.
*/
function isMeasurementValue(value: any): boolean {
function isMeasurementValue(value: unknown): value is number {
return typeof value === 'number' && isFinite(value);
}

Expand Down
10 changes: 7 additions & 3 deletions packages/tracing/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,15 @@ export class Transaction extends SpanClass implements TransactionInterface {
}

/**
* Set observed measurements for this transaction.
* Set observed measurement for this transaction.
*
* @param name Name of the measurement
* @param value Value of the measurement
* @param unit Unit of the measurement. (Defaults to an empty string)
* @hidden
*/
public setMeasurements(measurements: Measurements): void {
this._measurements = { ...measurements };
public setMeasurement(name: string, value: number, unit: string = ''): void {
this._measurements[name] = { value, unit };
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export interface SamplingContext extends CustomSamplingContext {
request?: ExtractedNodeRequestData;
}

export type Measurements = Record<string, { value: number }>;
export type Measurements = Record<string, { value: number; unit: string }>;

export type TransactionSamplingMethod = 'explicitly_set' | 'client_sampler' | 'client_rate' | 'inheritance';

Expand Down

0 comments on commit 20b8c52

Please sign in to comment.