Skip to content

Commit acc6fe1

Browse files
authored
Revert "feat(browser): Track measure detail as span attributes" (#16348)
It seems like this change introduced a problem where accessing `event.detail` causes a `DOMException` to be thrown (only on firefox?). This is causing the SDK to crash if someone has tracing enabled. Until we figure out a reproduction and a more formal fix (with some e2e tests) let's revert the problematic PR and move forward. Reverts #16240 ref #16347
1 parent 9c36322 commit acc6fe1

File tree

3 files changed

+8
-197
lines changed

3 files changed

+8
-197
lines changed

.size-limit.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ module.exports = [
5252
path: 'packages/browser/build/npm/esm/index.js',
5353
import: createImport('init', 'browserTracingIntegration', 'replayIntegration'),
5454
gzip: true,
55-
limit: '71 KB',
55+
limit: '70.1 KB',
5656
modifyWebpackConfig: function (config) {
5757
const webpack = require('webpack');
5858

@@ -206,7 +206,7 @@ module.exports = [
206206
import: createImport('init'),
207207
ignore: ['next/router', 'next/constants'],
208208
gzip: true,
209-
limit: '42.5 KB',
209+
limit: '42 KB',
210210
},
211211
// SvelteKit SDK (ESM)
212212
{

packages/browser-utils/src/metrics/browserMetrics.ts

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
/* eslint-disable max-lines */
2-
import type { Measurements, Span, SpanAttributes, SpanAttributeValue, StartSpanOptions } from '@sentry/core';
2+
import type { Measurements, Span, SpanAttributes, StartSpanOptions } from '@sentry/core';
33
import {
44
browserPerformanceTimeOrigin,
55
getActiveSpan,
66
getComponentName,
77
htmlTreeAsString,
8-
isPrimitive,
98
parseUrl,
109
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
1110
setMeasurement,
@@ -340,7 +339,7 @@ export function addPerformanceEntries(span: Span, options: AddPerformanceEntries
340339
case 'mark':
341340
case 'paint':
342341
case 'measure': {
343-
_addMeasureSpans(span, entry as PerformanceMeasure, startTime, duration, timeOrigin);
342+
_addMeasureSpans(span, entry, startTime, duration, timeOrigin);
344343

345344
// capture web vitals
346345
const firstHidden = getVisibilityWatcher();
@@ -422,7 +421,7 @@ export function addPerformanceEntries(span: Span, options: AddPerformanceEntries
422421
*/
423422
export function _addMeasureSpans(
424423
span: Span,
425-
entry: PerformanceMeasure,
424+
entry: PerformanceEntry,
426425
startTime: number,
427426
duration: number,
428427
timeOrigin: number,
@@ -451,34 +450,6 @@ export function _addMeasureSpans(
451450
attributes['sentry.browser.measure_start_time'] = measureStartTimestamp;
452451
}
453452

454-
// https://developer.mozilla.org/en-US/docs/Web/API/Performance/measure#detail
455-
if (entry.detail) {
456-
// Handle detail as an object
457-
if (typeof entry.detail === 'object') {
458-
for (const [key, value] of Object.entries(entry.detail)) {
459-
if (value && isPrimitive(value)) {
460-
attributes[`sentry.browser.measure.detail.${key}`] = value as SpanAttributeValue;
461-
} else {
462-
try {
463-
// This is user defined so we can't guarantee it's serializable
464-
attributes[`sentry.browser.measure.detail.${key}`] = JSON.stringify(value);
465-
} catch {
466-
// skip
467-
}
468-
}
469-
}
470-
} else if (isPrimitive(entry.detail)) {
471-
attributes['sentry.browser.measure.detail'] = entry.detail as SpanAttributeValue;
472-
} else {
473-
// This is user defined so we can't guarantee it's serializable
474-
try {
475-
attributes['sentry.browser.measure.detail'] = JSON.stringify(entry.detail);
476-
} catch {
477-
// skip
478-
}
479-
}
480-
}
481-
482453
// Measurements from third parties can be off, which would create invalid spans, dropping transactions in the process.
483454
if (measureStartTimestamp <= measureEndTimestamp) {
484455
startAndEndSpan(span, measureStartTimestamp, measureEndTimestamp, {

packages/browser-utils/test/browser/browserMetrics.test.ts

Lines changed: 3 additions & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ describe('_addMeasureSpans', () => {
7070
name: 'measure-1',
7171
duration: 10,
7272
startTime: 12,
73-
detail: null,
74-
} as PerformanceMeasure;
73+
} as PerformanceEntry;
7574

7675
const timeOrigin = 100;
7776
const startTime = 23;
@@ -107,8 +106,7 @@ describe('_addMeasureSpans', () => {
107106
name: 'measure-1',
108107
duration: 10,
109108
startTime: 12,
110-
detail: null,
111-
} as PerformanceMeasure;
109+
} as PerformanceEntry;
112110

113111
const timeOrigin = 100;
114112
const startTime = 23;
@@ -118,165 +116,6 @@ describe('_addMeasureSpans', () => {
118116

119117
expect(spans).toHaveLength(0);
120118
});
121-
122-
it('adds measure spans with primitive detail', () => {
123-
const spans: Span[] = [];
124-
125-
getClient()?.on('spanEnd', span => {
126-
spans.push(span);
127-
});
128-
129-
const entry = {
130-
entryType: 'measure',
131-
name: 'measure-1',
132-
duration: 10,
133-
startTime: 12,
134-
detail: 'test-detail',
135-
} as PerformanceMeasure;
136-
137-
const timeOrigin = 100;
138-
const startTime = 23;
139-
const duration = 356;
140-
141-
_addMeasureSpans(span, entry, startTime, duration, timeOrigin);
142-
143-
expect(spans).toHaveLength(1);
144-
expect(spanToJSON(spans[0]!)).toEqual(
145-
expect.objectContaining({
146-
description: 'measure-1',
147-
start_timestamp: timeOrigin + startTime,
148-
timestamp: timeOrigin + startTime + duration,
149-
op: 'measure',
150-
origin: 'auto.resource.browser.metrics',
151-
data: {
152-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'measure',
153-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics',
154-
'sentry.browser.measure.detail': 'test-detail',
155-
},
156-
}),
157-
);
158-
});
159-
160-
it('adds measure spans with object detail', () => {
161-
const spans: Span[] = [];
162-
163-
getClient()?.on('spanEnd', span => {
164-
spans.push(span);
165-
});
166-
167-
const detail = {
168-
component: 'Button',
169-
action: 'click',
170-
metadata: { id: 123 },
171-
};
172-
173-
const entry = {
174-
entryType: 'measure',
175-
name: 'measure-1',
176-
duration: 10,
177-
startTime: 12,
178-
detail,
179-
} as PerformanceMeasure;
180-
181-
const timeOrigin = 100;
182-
const startTime = 23;
183-
const duration = 356;
184-
185-
_addMeasureSpans(span, entry, startTime, duration, timeOrigin);
186-
187-
expect(spans).toHaveLength(1);
188-
expect(spanToJSON(spans[0]!)).toEqual(
189-
expect.objectContaining({
190-
description: 'measure-1',
191-
start_timestamp: timeOrigin + startTime,
192-
timestamp: timeOrigin + startTime + duration,
193-
op: 'measure',
194-
origin: 'auto.resource.browser.metrics',
195-
data: {
196-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'measure',
197-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics',
198-
'sentry.browser.measure.detail.component': 'Button',
199-
'sentry.browser.measure.detail.action': 'click',
200-
'sentry.browser.measure.detail.metadata': JSON.stringify({ id: 123 }),
201-
},
202-
}),
203-
);
204-
});
205-
206-
it('handles non-primitive detail values by stringifying them', () => {
207-
const spans: Span[] = [];
208-
209-
getClient()?.on('spanEnd', span => {
210-
spans.push(span);
211-
});
212-
213-
const detail = {
214-
component: 'Button',
215-
action: 'click',
216-
metadata: { id: 123 },
217-
callback: () => {},
218-
};
219-
220-
const entry = {
221-
entryType: 'measure',
222-
name: 'measure-1',
223-
duration: 10,
224-
startTime: 12,
225-
detail,
226-
} as PerformanceMeasure;
227-
228-
const timeOrigin = 100;
229-
const startTime = 23;
230-
const duration = 356;
231-
232-
_addMeasureSpans(span, entry, startTime, duration, timeOrigin);
233-
234-
expect(spans).toHaveLength(1);
235-
const spanData = spanToJSON(spans[0]!).data;
236-
expect(spanData['sentry.browser.measure.detail.component']).toBe('Button');
237-
expect(spanData['sentry.browser.measure.detail.action']).toBe('click');
238-
expect(spanData['sentry.browser.measure.detail.metadata']).toBe(JSON.stringify({ id: 123 }));
239-
expect(spanData['sentry.browser.measure.detail.callback']).toBe(JSON.stringify(detail.callback));
240-
});
241-
242-
it('handles errors in object detail value stringification', () => {
243-
const spans: Span[] = [];
244-
245-
getClient()?.on('spanEnd', span => {
246-
spans.push(span);
247-
});
248-
249-
const circular: any = {};
250-
circular.self = circular;
251-
252-
const detail = {
253-
component: 'Button',
254-
action: 'click',
255-
circular,
256-
};
257-
258-
const entry = {
259-
entryType: 'measure',
260-
name: 'measure-1',
261-
duration: 10,
262-
startTime: 12,
263-
detail,
264-
} as PerformanceMeasure;
265-
266-
const timeOrigin = 100;
267-
const startTime = 23;
268-
const duration = 356;
269-
270-
// Should not throw
271-
_addMeasureSpans(span, entry, startTime, duration, timeOrigin);
272-
273-
expect(spans).toHaveLength(1);
274-
const spanData = spanToJSON(spans[0]!).data;
275-
expect(spanData['sentry.browser.measure.detail.component']).toBe('Button');
276-
expect(spanData['sentry.browser.measure.detail.action']).toBe('click');
277-
// The circular reference should be skipped
278-
expect(spanData['sentry.browser.measure.detail.circular']).toBeUndefined();
279-
});
280119
});
281120

282121
describe('_addResourceSpans', () => {
@@ -625,6 +464,7 @@ describe('_addNavigationSpans', () => {
625464
transferSize: 14726,
626465
encodedBodySize: 14426,
627466
decodedBodySize: 67232,
467+
responseStatus: 200,
628468
serverTiming: [],
629469
unloadEventStart: 0,
630470
unloadEventEnd: 0,

0 commit comments

Comments
 (0)