Skip to content

feat(browser): Do not include metrics in base CDN bundle #12230

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.

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 4 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
import { expect } from '@playwright/test';

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

sentryTest('collects metrics', async ({ getLocalTestUrl, page }) => {
if (shouldSkipMetricsTest()) {
sentryTest.skip();
}

const url = await getLocalTestUrl({ testDir: __dirname });

const statsdBuffer = await getFirstSentryEnvelopeRequest<Uint8Array>(page, url, properEnvelopeRequestParser);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://[email protected]/1337',
});

// This should not fail
Sentry.metrics.increment('increment');
Sentry.metrics.distribution('distribution', 42);
Sentry.metrics.gauge('gauge', 5);
Sentry.metrics.set('set', 'nope');
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { expect } from '@playwright/test';

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

sentryTest('exports shim metrics integration for non-tracing bundles', async ({ getLocalTestPath, page }) => {
// Skip in tracing tests
if (!shouldSkipMetricsTest()) {
sentryTest.skip();
}

const consoleMessages: string[] = [];
page.on('console', msg => consoleMessages.push(msg.text()));

let requestCount = 0;
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
requestCount++;
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

const url = await getLocalTestPath({ testDir: __dirname });

await page.goto(url);

expect(requestCount).toBe(0);
expect(consoleMessages).toEqual([
'You are using metrics even though this bundle does not include tracing.',
'You are using metrics even though this bundle does not include tracing.',
'You are using metrics even though this bundle does not include tracing.',
'You are using metrics even though this bundle does not include tracing.',
]);
});
14 changes: 13 additions & 1 deletion dev-packages/browser-integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export function shouldSkipTracingTest(): boolean {
}

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

/**
* We can only test metrics tests in certain bundles/packages:
* - NPM (ESM, CJS)
* - CDN bundles that include tracing
*
* @returns `true` if we should skip the metrics test
*/
export function shouldSkipMetricsTest(): boolean {
const bundle = process.env.PW_BUNDLE as string | undefined;
return bundle != null && !bundle.includes('tracing') && !bundle.includes('esm') && !bundle.includes('cjs');
}

/**
* Waits until a number of requests matching urlRgx at the given URL arrive.
* If the timout option is configured, this function will abort waiting, even if it hasn't reveived the configured
Expand Down
2 changes: 0 additions & 2 deletions packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ export {
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
} from '@sentry/core';

export * from './metrics';

export { WINDOW } from './helpers';
export { BrowserClient } from './client';
export { makeFetchTransport } from './transports/fetch';
Expand Down
3 changes: 2 additions & 1 deletion packages/browser/src/index.bundle.feedback.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { browserTracingIntegrationShim, replayIntegrationShim } from '@sentry-internal/integration-shims';
import { browserTracingIntegrationShim, metricsShim, replayIntegrationShim } from '@sentry-internal/integration-shims';
import { feedbackAsyncIntegration } from './feedbackAsync';

export * from './index.bundle.base';
Expand All @@ -10,6 +10,7 @@ export {
feedbackAsyncIntegration as feedbackAsyncIntegration,
feedbackAsyncIntegration as feedbackIntegration,
replayIntegrationShim as replayIntegration,
metricsShim as metrics,
};

export { captureFeedback } from '@sentry/core';
7 changes: 6 additions & 1 deletion packages/browser/src/index.bundle.replay.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { browserTracingIntegrationShim, feedbackIntegrationShim } from '@sentry-internal/integration-shims';
import {
browserTracingIntegrationShim,
feedbackIntegrationShim,
metricsShim,
} from '@sentry-internal/integration-shims';

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

Expand All @@ -8,4 +12,5 @@ export {
browserTracingIntegrationShim as browserTracingIntegration,
feedbackIntegrationShim as feedbackAsyncIntegration,
feedbackIntegrationShim as feedbackIntegration,
metricsShim as metrics,
};
2 changes: 2 additions & 0 deletions packages/browser/src/index.bundle.tracing.replay.feedback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ registerSpanErrorInstrumentation();

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

export * from './metrics';

export {
getActiveSpan,
getRootSpan,
Expand Down
2 changes: 2 additions & 0 deletions packages/browser/src/index.bundle.tracing.replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ registerSpanErrorInstrumentation();

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

export * from './metrics';

export {
getActiveSpan,
getRootSpan,
Expand Down
2 changes: 2 additions & 0 deletions packages/browser/src/index.bundle.tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ registerSpanErrorInstrumentation();

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

export * from './metrics';

export {
getActiveSpan,
getRootSpan,
Expand Down
2 changes: 2 additions & 0 deletions packages/browser/src/index.bundle.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
browserTracingIntegrationShim,
feedbackIntegrationShim,
metricsShim,
replayIntegrationShim,
} from '@sentry-internal/integration-shims';

Expand All @@ -11,4 +12,5 @@ export {
feedbackIntegrationShim as feedbackAsyncIntegration,
feedbackIntegrationShim as feedbackIntegration,
replayIntegrationShim as replayIntegration,
metricsShim as metrics,
};
2 changes: 2 additions & 0 deletions packages/browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export {
sendFeedback,
} from '@sentry-internal/feedback';

export * from './metrics';

export {
defaultRequestInstrumentationOptions,
instrumentOutgoingRequests,
Expand Down
4 changes: 2 additions & 2 deletions packages/browser/src/metrics.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { MetricData } from '@sentry/core';
import { BrowserMetricsAggregator, metrics as metricsCore } from '@sentry/core';
import type { MetricData, Metrics } from '@sentry/types';

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

export const metrics = {
export const metrics: Metrics = {
increment,
distribution,
set,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export { rewriteFramesIntegration } from './integrations/rewriteframes';
export { sessionTimingIntegration } from './integrations/sessiontiming';
export { zodErrorsIntegration } from './integrations/zoderrors';
export { metrics } from './metrics/exports';
export type { MetricData } from './metrics/exports';
export type { MetricData } from '@sentry/types';
export { metricsDefault } from './metrics/exports-default';
export { BrowserMetricsAggregator } from './metrics/browser-aggregator';
export { getMetricSummaryJsonForSpan } from './metrics/metric-summary';
Expand Down
8 changes: 5 additions & 3 deletions packages/core/src/metrics/exports-default.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Client, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
import type { Client, MetricData, Metrics, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
import { MetricsAggregator } from './aggregator';
import type { MetricData } from './exports';
import { metrics as metricsCore } from './exports';

/**
Expand Down Expand Up @@ -46,11 +45,14 @@ function getMetricsAggregatorForClient(client: Client): MetricsAggregatorInterfa
return metricsCore.getMetricsAggregatorForClient(client, MetricsAggregator);
}

export const metricsDefault = {
export const metricsDefault: Metrics & {
getMetricsAggregatorForClient: typeof getMetricsAggregatorForClient;
} = {
increment,
distribution,
set,
gauge,

/**
* @ignore This is for internal use only.
*/
Expand Down
14 changes: 1 addition & 13 deletions packages/core/src/metrics/exports.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,11 @@
import type {
Client,
MeasurementUnit,
MetricsAggregator as MetricsAggregatorInterface,
Primitive,
} from '@sentry/types';
import type { Client, MetricData, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
import { getGlobalSingleton, logger } from '@sentry/utils';
import { getClient } from '../currentScopes';
import { DEBUG_BUILD } from '../debug-build';
import { getActiveSpan, getRootSpan, spanToJSON } from '../utils/spanUtils';
import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants';
import type { MetricType } from './types';

export interface MetricData {
unit?: MeasurementUnit;
tags?: Record<string, Primitive>;
timestamp?: number;
client?: Client;
}

type MetricsAggregatorConstructor = {
new (client: Client): MetricsAggregatorInterface;
};
Expand Down
1 change: 1 addition & 0 deletions packages/integration-shims/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { feedbackIntegrationShim } from './Feedback';
export { replayIntegrationShim } from './Replay';
export { browserTracingIntegrationShim } from './BrowserTracing';
export { metricsShim } from './metrics';
16 changes: 16 additions & 0 deletions packages/integration-shims/src/metrics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import type { Metrics } from '@sentry/types';
import { consoleSandbox } from '@sentry/utils';

function warn(): void {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn('You are using metrics even though this bundle does not include tracing.');
});
}

export const metricsShim: Metrics = {
increment: warn,
distribution: warn,
set: warn,
gauge: warn,
};
2 changes: 2 additions & 0 deletions packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ export type {
MetricsAggregator,
MetricBucketItem,
MetricInstance,
MetricData,
Metrics,
} from './metrics';
export type { ParameterizedString } from './parameterize';
export type { ViewHierarchyData, ViewHierarchyWindow } from './view-hierarchy';
38 changes: 38 additions & 0 deletions packages/types/src/metrics.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import type { Client } from './client';
import type { MeasurementUnit } from './measurement';
import type { Primitive } from './misc';

export interface MetricData {
unit?: MeasurementUnit;
tags?: Record<string, Primitive>;
timestamp?: number;
client?: Client;
}

/**
* An abstract definition of the minimum required API
* for a metric instance.
Expand Down Expand Up @@ -62,3 +70,33 @@ export interface MetricsAggregator {
*/
toString(): string;
}

export interface Metrics {
/**
* Adds a value to a counter metric
*
* @experimental This API is experimental and might have breaking changes in the future.
*/
increment(name: string, value?: number, data?: MetricData): void;

/**
* Adds a value to a distribution metric
*
* @experimental This API is experimental and might have breaking changes in the future.
*/
distribution(name: string, value: number, data?: MetricData): void;

/**
* Adds a value to a set metric. Value must be a string or integer.
*
* @experimental This API is experimental and might have breaking changes in the future.
*/
set(name: string, value: number | string, data?: MetricData): void;

/**
* Adds a value to a gauge metric
*
* @experimental This API is experimental and might have breaking changes in the future.
*/
gauge(name: string, value: number, data?: MetricData): void;
}
Loading