Skip to content

Commit 052263b

Browse files
lforstlobsterkatie
authored andcommitted
ref(tracing): Update setMeasurements to only set a single measurement (#4920)
1 parent 08f50c8 commit 052263b

File tree

5 files changed

+62
-19
lines changed

5 files changed

+62
-19
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
const transaction = Sentry.startTransaction({ name: 'some_transaction' });
2+
3+
transaction.setMeasurement('metric.foo', 42, 'ms');
4+
transaction.setMeasurement('metric.bar', 1337, 'nanoseconds');
5+
transaction.setMeasurement('metric.baz', 99, 's');
6+
transaction.setMeasurement('metric.baz', 1);
7+
8+
transaction.finish();
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { expect } from '@playwright/test';
2+
import { Event } from '@sentry/types';
3+
4+
import { sentryTest } from '../../../../utils/fixtures';
5+
import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
6+
7+
sentryTest('should attach measurement to transaction', async ({ getLocalTestPath, page }) => {
8+
const url = await getLocalTestPath({ testDir: __dirname });
9+
const event = await getFirstSentryEnvelopeRequest<Event>(page, url);
10+
11+
expect(event.measurements?.['metric.foo'].value).toBe(42);
12+
expect(event.measurements?.['metric.bar'].value).toBe(1337);
13+
expect(event.measurements?.['metric.baz'].value).toBe(1);
14+
15+
expect(event.measurements?.['metric.foo'].unit).toBe('ms');
16+
expect(event.measurements?.['metric.bar'].unit).toBe('nanoseconds');
17+
expect(event.measurements?.['metric.baz'].unit).toBe('');
18+
});

packages/tracing/src/browser/metrics.ts

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,14 @@ export class MetricsInstrumentation {
7979

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

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

9292
break;
@@ -115,12 +115,18 @@ export class MetricsInstrumentation {
115115
// start of the response in milliseconds
116116
if (typeof responseStartTimestamp === 'number') {
117117
IS_DEBUG_BUILD && logger.log('[Measurements] Adding TTFB');
118-
this._measurements['ttfb'] = { value: (responseStartTimestamp - transaction.startTimestamp) * 1000 };
118+
this._measurements['ttfb'] = {
119+
value: (responseStartTimestamp - transaction.startTimestamp) * 1000,
120+
unit: 'millisecond',
121+
};
119122

120123
if (typeof requestStartTimestamp === 'number' && requestStartTimestamp <= responseStartTimestamp) {
121124
// Capture the time spent making the request and receiving the first byte of the response.
122125
// This is the time between the start of the request and the start of the response in milliseconds.
123-
this._measurements['ttfb.requestTime'] = { value: (responseStartTimestamp - requestStartTimestamp) * 1000 };
126+
this._measurements['ttfb.requestTime'] = {
127+
value: (responseStartTimestamp - requestStartTimestamp) * 1000,
128+
unit: 'second',
129+
};
124130
}
125131
}
126132

@@ -162,7 +168,14 @@ export class MetricsInstrumentation {
162168
delete this._measurements.cls;
163169
}
164170

165-
transaction.setMeasurements(this._measurements);
171+
Object.keys(this._measurements).forEach(measurementName => {
172+
transaction.setMeasurement(
173+
measurementName,
174+
this._measurements[measurementName].value,
175+
this._measurements[measurementName].unit,
176+
);
177+
});
178+
166179
tagMetricInfo(transaction, this._lcpEntry, this._clsEntry);
167180
transaction.setTag('sentry_reportAllChanges', this._reportAllChanges);
168181
}
@@ -189,11 +202,11 @@ export class MetricsInstrumentation {
189202
}
190203

191204
if (isMeasurementValue(connection.rtt)) {
192-
this._measurements['connection.rtt'] = { value: connection.rtt as number };
205+
this._measurements['connection.rtt'] = { value: connection.rtt, unit: 'millisecond' };
193206
}
194207

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

@@ -218,7 +231,7 @@ export class MetricsInstrumentation {
218231
}
219232

220233
IS_DEBUG_BUILD && logger.log('[Measurements] Adding CLS');
221-
this._measurements['cls'] = { value: metric.value };
234+
this._measurements['cls'] = { value: metric.value, unit: 'millisecond' };
222235
this._clsEntry = entry as LayoutShift;
223236
});
224237
}
@@ -234,8 +247,8 @@ export class MetricsInstrumentation {
234247
const timeOrigin = msToSec(browserPerformanceTimeOrigin as number);
235248
const startTime = msToSec(entry.startTime);
236249
IS_DEBUG_BUILD && logger.log('[Measurements] Adding LCP');
237-
this._measurements['lcp'] = { value: metric.value };
238-
this._measurements['mark.lcp'] = { value: timeOrigin + startTime };
250+
this._measurements['lcp'] = { value: metric.value, unit: 'millisecond' };
251+
this._measurements['mark.lcp'] = { value: timeOrigin + startTime, unit: 'second' };
239252
this._lcpEntry = entry as LargestContentfulPaint;
240253
}, this._reportAllChanges);
241254
}
@@ -251,8 +264,8 @@ export class MetricsInstrumentation {
251264
const timeOrigin = msToSec(browserPerformanceTimeOrigin as number);
252265
const startTime = msToSec(entry.startTime);
253266
IS_DEBUG_BUILD && logger.log('[Measurements] Adding FID');
254-
this._measurements['fid'] = { value: metric.value };
255-
this._measurements['mark.fid'] = { value: timeOrigin + startTime };
267+
this._measurements['fid'] = { value: metric.value, unit: 'millisecond' };
268+
this._measurements['mark.fid'] = { value: timeOrigin + startTime, unit: 'second' };
256269
});
257270
}
258271
}
@@ -392,7 +405,7 @@ export function _startChild(transaction: Transaction, { startTimestamp, ...ctx }
392405
/**
393406
* Checks if a given value is a valid measurement value.
394407
*/
395-
function isMeasurementValue(value: any): boolean {
408+
function isMeasurementValue(value: unknown): value is number {
396409
return typeof value === 'number' && isFinite(value);
397410
}
398411

packages/tracing/src/transaction.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,15 @@ export class Transaction extends SpanClass implements TransactionInterface {
6868
}
6969

7070
/**
71-
* Set observed measurements for this transaction.
71+
* Set observed measurement for this transaction.
72+
*
73+
* @param name Name of the measurement
74+
* @param value Value of the measurement
75+
* @param unit Unit of the measurement. (Defaults to an empty string)
7276
* @hidden
7377
*/
74-
public setMeasurements(measurements: Measurements): void {
75-
this._measurements = { ...measurements };
78+
public setMeasurement(name: string, value: number, unit: string = ''): void {
79+
this._measurements[name] = { value, unit };
7680
}
7781

7882
/**

packages/types/src/transaction.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ export interface SamplingContext extends CustomSamplingContext {
115115
request?: ExtractedNodeRequestData;
116116
}
117117

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

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

0 commit comments

Comments
 (0)