Skip to content

Commit 0fbe39a

Browse files
authored
feat(browser): Do not include metrics in base CDN bundle (#12230)
Metrics are only included when performance is included, reducing the base bundle size. We always expose a shim so there is no API breakage, it just does nothing. I noticed this while working on #12226.
1 parent 8dca102 commit 0fbe39a

File tree

21 files changed

+156
-26
lines changed

21 files changed

+156
-26
lines changed

dev-packages/browser-integration-tests/suites/metrics/test.ts renamed to dev-packages/browser-integration-tests/suites/metrics/metricsEvent/test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
import { expect } from '@playwright/test';
22

3-
import { sentryTest } from '../../utils/fixtures';
4-
import { getFirstSentryEnvelopeRequest, properEnvelopeRequestParser } from '../../utils/helpers';
3+
import { sentryTest } from '../../../utils/fixtures';
4+
import {
5+
getFirstSentryEnvelopeRequest,
6+
properEnvelopeRequestParser,
7+
shouldSkipMetricsTest,
8+
} from '../../../utils/helpers';
59

610
sentryTest('collects metrics', async ({ getLocalTestUrl, page }) => {
11+
if (shouldSkipMetricsTest()) {
12+
sentryTest.skip();
13+
}
14+
715
const url = await getLocalTestUrl({ testDir: __dirname });
816

917
const statsdBuffer = await getFirstSentryEnvelopeRequest<Uint8Array>(page, url, properEnvelopeRequestParser);
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
});
8+
9+
// This should not fail
10+
Sentry.metrics.increment('increment');
11+
Sentry.metrics.distribution('distribution', 42);
12+
Sentry.metrics.gauge('gauge', 5);
13+
Sentry.metrics.set('set', 'nope');
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../utils/fixtures';
4+
import { shouldSkipMetricsTest } from '../../../utils/helpers';
5+
6+
sentryTest('exports shim metrics integration for non-tracing bundles', async ({ getLocalTestPath, page }) => {
7+
// Skip in tracing tests
8+
if (!shouldSkipMetricsTest()) {
9+
sentryTest.skip();
10+
}
11+
12+
const consoleMessages: string[] = [];
13+
page.on('console', msg => consoleMessages.push(msg.text()));
14+
15+
let requestCount = 0;
16+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
17+
requestCount++;
18+
return route.fulfill({
19+
status: 200,
20+
contentType: 'application/json',
21+
body: JSON.stringify({ id: 'test-id' }),
22+
});
23+
});
24+
25+
const url = await getLocalTestPath({ testDir: __dirname });
26+
27+
await page.goto(url);
28+
29+
expect(requestCount).toBe(0);
30+
expect(consoleMessages).toEqual([
31+
'You are using metrics even though this bundle does not include tracing.',
32+
'You are using metrics even though this bundle does not include tracing.',
33+
'You are using metrics even though this bundle does not include tracing.',
34+
'You are using metrics even though this bundle does not include tracing.',
35+
]);
36+
});

dev-packages/browser-integration-tests/utils/helpers.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ export function shouldSkipTracingTest(): boolean {
241241
}
242242

243243
/**
244-
* We can only test replay tests in certain bundles/packages:
244+
* We can only test feedback tests in certain bundles/packages:
245245
* - NPM (ESM, CJS)
246246
* - CDN bundles that contain the Replay integration
247247
*
@@ -252,6 +252,18 @@ export function shouldSkipFeedbackTest(): boolean {
252252
return bundle != null && !bundle.includes('feedback') && !bundle.includes('esm') && !bundle.includes('cjs');
253253
}
254254

255+
/**
256+
* We can only test metrics tests in certain bundles/packages:
257+
* - NPM (ESM, CJS)
258+
* - CDN bundles that include tracing
259+
*
260+
* @returns `true` if we should skip the metrics test
261+
*/
262+
export function shouldSkipMetricsTest(): boolean {
263+
const bundle = process.env.PW_BUNDLE as string | undefined;
264+
return bundle != null && !bundle.includes('tracing') && !bundle.includes('esm') && !bundle.includes('cjs');
265+
}
266+
255267
/**
256268
* Waits until a number of requests matching urlRgx at the given URL arrive.
257269
* If the timout option is configured, this function will abort waiting, even if it hasn't reveived the configured

packages/browser/src/exports.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,6 @@ export {
6868
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
6969
} from '@sentry/core';
7070

71-
export * from './metrics';
72-
7371
export { WINDOW } from './helpers';
7472
export { BrowserClient } from './client';
7573
export { makeFetchTransport } from './transports/fetch';

packages/browser/src/index.bundle.feedback.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { browserTracingIntegrationShim, replayIntegrationShim } from '@sentry-internal/integration-shims';
1+
import { browserTracingIntegrationShim, metricsShim, replayIntegrationShim } from '@sentry-internal/integration-shims';
22
import { feedbackAsyncIntegration } from './feedbackAsync';
33

44
export * from './index.bundle.base';
@@ -10,6 +10,7 @@ export {
1010
feedbackAsyncIntegration as feedbackAsyncIntegration,
1111
feedbackAsyncIntegration as feedbackIntegration,
1212
replayIntegrationShim as replayIntegration,
13+
metricsShim as metrics,
1314
};
1415

1516
export { captureFeedback } from '@sentry/core';

packages/browser/src/index.bundle.replay.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { browserTracingIntegrationShim, feedbackIntegrationShim } from '@sentry-internal/integration-shims';
1+
import {
2+
browserTracingIntegrationShim,
3+
feedbackIntegrationShim,
4+
metricsShim,
5+
} from '@sentry-internal/integration-shims';
26

37
export * from './index.bundle.base';
48

@@ -8,4 +12,5 @@ export {
812
browserTracingIntegrationShim as browserTracingIntegration,
913
feedbackIntegrationShim as feedbackAsyncIntegration,
1014
feedbackIntegrationShim as feedbackIntegration,
15+
metricsShim as metrics,
1116
};

packages/browser/src/index.bundle.tracing.replay.feedback.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ registerSpanErrorInstrumentation();
44

55
export * from './index.bundle.base';
66

7+
export * from './metrics';
8+
79
export {
810
getActiveSpan,
911
getRootSpan,

packages/browser/src/index.bundle.tracing.replay.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ registerSpanErrorInstrumentation();
44

55
export * from './index.bundle.base';
66

7+
export * from './metrics';
8+
79
export {
810
getActiveSpan,
911
getRootSpan,

packages/browser/src/index.bundle.tracing.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ registerSpanErrorInstrumentation();
55

66
export * from './index.bundle.base';
77

8+
export * from './metrics';
9+
810
export {
911
getActiveSpan,
1012
getRootSpan,

packages/browser/src/index.bundle.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
browserTracingIntegrationShim,
33
feedbackIntegrationShim,
4+
metricsShim,
45
replayIntegrationShim,
56
} from '@sentry-internal/integration-shims';
67

@@ -11,4 +12,5 @@ export {
1112
feedbackIntegrationShim as feedbackAsyncIntegration,
1213
feedbackIntegrationShim as feedbackIntegration,
1314
replayIntegrationShim as replayIntegration,
15+
metricsShim as metrics,
1416
};

packages/browser/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ export {
3939
sendFeedback,
4040
} from '@sentry-internal/feedback';
4141

42+
export * from './metrics';
43+
4244
export {
4345
defaultRequestInstrumentationOptions,
4446
instrumentOutgoingRequests,

packages/browser/src/metrics.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import type { MetricData } from '@sentry/core';
21
import { BrowserMetricsAggregator, metrics as metricsCore } from '@sentry/core';
2+
import type { MetricData, Metrics } from '@sentry/types';
33

44
/**
55
* Adds a value to a counter metric
@@ -37,7 +37,7 @@ function gauge(name: string, value: number, data?: MetricData): void {
3737
metricsCore.gauge(BrowserMetricsAggregator, name, value, data);
3838
}
3939

40-
export const metrics = {
40+
export const metrics: Metrics = {
4141
increment,
4242
distribution,
4343
set,

packages/core/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export { rewriteFramesIntegration } from './integrations/rewriteframes';
9797
export { sessionTimingIntegration } from './integrations/sessiontiming';
9898
export { zodErrorsIntegration } from './integrations/zoderrors';
9999
export { metrics } from './metrics/exports';
100-
export type { MetricData } from './metrics/exports';
100+
export type { MetricData } from '@sentry/types';
101101
export { metricsDefault } from './metrics/exports-default';
102102
export { BrowserMetricsAggregator } from './metrics/browser-aggregator';
103103
export { getMetricSummaryJsonForSpan } from './metrics/metric-summary';

packages/core/src/metrics/exports-default.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import type { Client, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
1+
import type { Client, MetricData, Metrics, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
22
import { MetricsAggregator } from './aggregator';
3-
import type { MetricData } from './exports';
43
import { metrics as metricsCore } from './exports';
54

65
/**
@@ -46,11 +45,14 @@ function getMetricsAggregatorForClient(client: Client): MetricsAggregatorInterfa
4645
return metricsCore.getMetricsAggregatorForClient(client, MetricsAggregator);
4746
}
4847

49-
export const metricsDefault = {
48+
export const metricsDefault: Metrics & {
49+
getMetricsAggregatorForClient: typeof getMetricsAggregatorForClient;
50+
} = {
5051
increment,
5152
distribution,
5253
set,
5354
gauge,
55+
5456
/**
5557
* @ignore This is for internal use only.
5658
*/

packages/core/src/metrics/exports.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,11 @@
1-
import type {
2-
Client,
3-
MeasurementUnit,
4-
MetricsAggregator as MetricsAggregatorInterface,
5-
Primitive,
6-
} from '@sentry/types';
1+
import type { Client, MetricData, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
72
import { getGlobalSingleton, logger } from '@sentry/utils';
83
import { getClient } from '../currentScopes';
94
import { DEBUG_BUILD } from '../debug-build';
105
import { getActiveSpan, getRootSpan, spanToJSON } from '../utils/spanUtils';
116
import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants';
127
import type { MetricType } from './types';
138

14-
export interface MetricData {
15-
unit?: MeasurementUnit;
16-
tags?: Record<string, Primitive>;
17-
timestamp?: number;
18-
client?: Client;
19-
}
20-
219
type MetricsAggregatorConstructor = {
2210
new (client: Client): MetricsAggregatorInterface;
2311
};
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
export { feedbackIntegrationShim } from './Feedback';
22
export { replayIntegrationShim } from './Replay';
33
export { browserTracingIntegrationShim } from './BrowserTracing';
4+
export { metricsShim } from './metrics';
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import type { Metrics } from '@sentry/types';
2+
import { consoleSandbox } from '@sentry/utils';
3+
4+
function warn(): void {
5+
consoleSandbox(() => {
6+
// eslint-disable-next-line no-console
7+
console.warn('You are using metrics even though this bundle does not include tracing.');
8+
});
9+
}
10+
11+
export const metricsShim: Metrics = {
12+
increment: warn,
13+
distribution: warn,
14+
set: warn,
15+
gauge: warn,
16+
};

packages/types/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ export type {
164164
MetricsAggregator,
165165
MetricBucketItem,
166166
MetricInstance,
167+
MetricData,
168+
Metrics,
167169
} from './metrics';
168170
export type { ParameterizedString } from './parameterize';
169171
export type { ViewHierarchyData, ViewHierarchyWindow } from './view-hierarchy';

packages/types/src/metrics.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
1+
import type { Client } from './client';
12
import type { MeasurementUnit } from './measurement';
23
import type { Primitive } from './misc';
34

5+
export interface MetricData {
6+
unit?: MeasurementUnit;
7+
tags?: Record<string, Primitive>;
8+
timestamp?: number;
9+
client?: Client;
10+
}
11+
412
/**
513
* An abstract definition of the minimum required API
614
* for a metric instance.
@@ -62,3 +70,33 @@ export interface MetricsAggregator {
6270
*/
6371
toString(): string;
6472
}
73+
74+
export interface Metrics {
75+
/**
76+
* Adds a value to a counter metric
77+
*
78+
* @experimental This API is experimental and might have breaking changes in the future.
79+
*/
80+
increment(name: string, value?: number, data?: MetricData): void;
81+
82+
/**
83+
* Adds a value to a distribution metric
84+
*
85+
* @experimental This API is experimental and might have breaking changes in the future.
86+
*/
87+
distribution(name: string, value: number, data?: MetricData): void;
88+
89+
/**
90+
* Adds a value to a set metric. Value must be a string or integer.
91+
*
92+
* @experimental This API is experimental and might have breaking changes in the future.
93+
*/
94+
set(name: string, value: number | string, data?: MetricData): void;
95+
96+
/**
97+
* Adds a value to a gauge metric
98+
*
99+
* @experimental This API is experimental and might have breaking changes in the future.
100+
*/
101+
gauge(name: string, value: number, data?: MetricData): void;
102+
}

0 commit comments

Comments
 (0)