Skip to content

ref(browser): Store browser metrics as attributes instead of tags #10823

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 7, 2024
Merged

ref(browser): Store browser metrics as attributes instead of tags #10823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c42a9d8
ref(browser): Store browser metrics as attributes instead of tags
mydea Feb 27, 2024
0952fed
fix tests
mydea Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ sentryTest(
expect(eventData.measurements).toBeDefined();
expect(eventData.measurements?.lcp?.value).toBeDefined();

expect(eventData.tags?.['lcp.element']).toBe('body > img');
expect(eventData.tags?.['lcp.size']).toBe(107400);
expect(eventData.tags?.['lcp.url']).toBe('https://example.com/path/to/image.png');
expect(eventData.contexts?.trace?.data?.['lcp.element']).toBe('body > img');
expect(eventData.contexts?.trace?.data?.['lcp.size']).toBe(107400);
expect(eventData.contexts?.trace?.data?.['lcp.url']).toBe('https://example.com/path/to/image.png');

const lcp = await (await page.waitForFunction('window._LCP')).jsonValue();
const lcp2 = await (await page.waitForFunction('window._LCP2')).jsonValue();
Expand Down
6 changes: 3 additions & 3 deletions dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ sentryTest('should capture a "GOOD" CLS vital with its source(s).', async ({ get
expect(eventData.measurements?.cls?.value).toBeGreaterThan(0.03);
expect(eventData.measurements?.cls?.value).toBeLessThan(0.07);

expect(eventData.tags?.['cls.source.1']).toBe('body > div#content > p#partial');
expect(eventData.contexts?.trace?.data?.['cls.source.1']).toBe('body > div#content > p#partial');
});

sentryTest('should capture a "MEH" CLS vital with its source(s).', async ({ getLocalTestPath, page }) => {
Expand All @@ -37,7 +37,7 @@ sentryTest('should capture a "MEH" CLS vital with its source(s).', async ({ getL
expect(eventData.measurements?.cls?.value).toBeGreaterThan(0.18);
expect(eventData.measurements?.cls?.value).toBeLessThan(0.23);

expect(eventData.tags?.['cls.source.1']).toBe('body > div#content > p');
expect(eventData.contexts?.trace?.data?.['cls.source.1']).toBe('body > div#content > p');
});

sentryTest('should capture a "POOR" CLS vital with its source(s).', async ({ getLocalTestPath, page }) => {
Expand All @@ -50,5 +50,5 @@ sentryTest('should capture a "POOR" CLS vital with its source(s).', async ({ get
// Flakey value dependent on timings -> we check for a range
expect(eventData.measurements?.cls?.value).toBeGreaterThan(0.34);
expect(eventData.measurements?.cls?.value).toBeLessThan(0.36);
expect(eventData.tags?.['cls.source.1']).toBe('body > div#content > p');
expect(eventData.contexts?.trace?.data?.['cls.source.1']).toBe('body > div#content > p');
});
13 changes: 7 additions & 6 deletions dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN
sentryTest.skip();
}

await page.route('**/path/to/image.png', (route: Route) =>
route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png` }),
);
page.route('**', route => route.continue());
page.route('**/path/to/image.png', async (route: Route) => {
return route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png` });
});

const url = await getLocalTestPath({ testDir: __dirname });
const [eventData] = await Promise.all([
Expand All @@ -24,7 +25,7 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN
expect(eventData.measurements).toBeDefined();
expect(eventData.measurements?.lcp?.value).toBeDefined();

expect(eventData.tags?.['lcp.element']).toBe('body > img');
expect(eventData.tags?.['lcp.size']).toBe(107400);
expect(eventData.tags?.['lcp.url']).toBe('https://example.com/path/to/image.png');
expect(eventData.contexts?.trace?.data?.['lcp.element']).toBe('body > img');
expect(eventData.contexts?.trace?.data?.['lcp.size']).toBe(107400);
expect(eventData.contexts?.trace?.data?.['lcp.url']).toBe('https://example.com/path/to/image.png');
});
37 changes: 9 additions & 28 deletions packages/tracing-internal/src/browser/metrics/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable max-lines */
import type { SentrySpan } from '@sentry/core';
import { getActiveSpan, startInactiveSpan } from '@sentry/core';
import { setMeasurement } from '@sentry/core';
import type { Measurements, Span, StartSpanOptions } from '@sentry/types';
Expand Down Expand Up @@ -445,15 +444,11 @@ function _trackNavigator(span: Span): void {
const connection = navigator.connection;
if (connection) {
if (connection.effectiveType) {
// TODO: Can we rewrite this to an attribute?
// eslint-disable-next-line deprecation/deprecation
(span as SentrySpan).setTag('effectiveConnectionType', connection.effectiveType);
span.setAttribute('effectiveConnectionType', connection.effectiveType);
}

if (connection.type) {
// TODO: Can we rewrite this to an attribute?
// eslint-disable-next-line deprecation/deprecation
(span as SentrySpan).setTag('connectionType', connection.type);
span.setAttribute('connectionType', connection.type);
}

if (isMeasurementValue(connection.rtt)) {
Expand All @@ -462,15 +457,11 @@ function _trackNavigator(span: Span): void {
}

if (isMeasurementValue(navigator.deviceMemory)) {
// TODO: Can we rewrite this to an attribute?
// eslint-disable-next-line deprecation/deprecation
(span as SentrySpan).setTag('deviceMemory', `${navigator.deviceMemory} GB`);
span.setAttribute('deviceMemory', `${navigator.deviceMemory} GB`);
}

if (isMeasurementValue(navigator.hardwareConcurrency)) {
// TODO: Can we rewrite this to an attribute?
// eslint-disable-next-line deprecation/deprecation
(span as SentrySpan).setTag('hardwareConcurrency', String(navigator.hardwareConcurrency));
span.setAttribute('hardwareConcurrency', String(navigator.hardwareConcurrency));
}
}

Expand All @@ -482,36 +473,26 @@ function _tagMetricInfo(span: Span): void {
// Capture Properties of the LCP element that contributes to the LCP.

if (_lcpEntry.element) {
// TODO: Can we rewrite this to an attribute?
// eslint-disable-next-line deprecation/deprecation
(span as SentrySpan).setTag('lcp.element', htmlTreeAsString(_lcpEntry.element));
span.setAttribute('lcp.element', htmlTreeAsString(_lcpEntry.element));
}

if (_lcpEntry.id) {
// TODO: Can we rewrite this to an attribute?
// eslint-disable-next-line deprecation/deprecation
(span as SentrySpan).setTag('lcp.id', _lcpEntry.id);
span.setAttribute('lcp.id', _lcpEntry.id);
}

if (_lcpEntry.url) {
// Trim URL to the first 200 characters.
// TODO: Can we rewrite this to an attribute?
// eslint-disable-next-line deprecation/deprecation
(span as SentrySpan).setTag('lcp.url', _lcpEntry.url.trim().slice(0, 200));
span.setAttribute('lcp.url', _lcpEntry.url.trim().slice(0, 200));
}

// TODO: Can we rewrite this to an attribute?
// eslint-disable-next-line deprecation/deprecation
(span as SentrySpan).setTag('lcp.size', _lcpEntry.size);
span.setAttribute('lcp.size', _lcpEntry.size);
}

// See: https://developer.mozilla.org/en-US/docs/Web/API/LayoutShift
if (_clsEntry && _clsEntry.sources) {
DEBUG_BUILD && logger.log('[Measurements] Adding CLS Data');
_clsEntry.sources.forEach((source, index) =>
// TODO: Can we rewrite this to an attribute?
// eslint-disable-next-line deprecation/deprecation
(span as SentrySpan).setTag(`cls.source.${index + 1}`, htmlTreeAsString(source.node)),
span.setAttribute(`cls.source.${index + 1}`, htmlTreeAsString(source.node)),
);
}
}
Expand Down