Skip to content

feat(core): Decouple metrics aggregation from client #10628

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 9 commits into from
Feb 16, 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
4 changes: 4 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Upgrading from 7.x to 8.x

## Removal of the `MetricsAggregator` integration class and `metricsAggregatorIntegration`

The SDKs now support metrics features without any additional configuration.

## Updated behaviour of `tracePropagationTargets` in the browser (HTTP tracing headers & CORS)

We updated the behaviour of the SDKs when no `tracePropagationTargets` option was defined. As a reminder, you can
Expand Down
16 changes: 16 additions & 0 deletions dev-packages/browser-integration-tests/suites/metrics/init.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

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

Sentry.metrics.increment('increment');
Sentry.metrics.increment('increment');
Sentry.metrics.distribution('distribution', 42);
Sentry.metrics.distribution('distribution', 45);
Sentry.metrics.gauge('gauge', 5);
Sentry.metrics.gauge('gauge', 15);
Sentry.metrics.set('set', 'nope');
Sentry.metrics.set('set', 'another');
17 changes: 17 additions & 0 deletions dev-packages/browser-integration-tests/suites/metrics/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { expect } from '@playwright/test';

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

sentryTest('collects metrics', async ({ getLocalTestUrl, page }) => {
const url = await getLocalTestUrl({ testDir: __dirname });

const statsdBuffer = await getFirstSentryEnvelopeRequest<Uint8Array>(page, url, properEnvelopeRequestParser);
const statsdString = new TextDecoder().decode(statsdBuffer);
// Replace all the Txxxxxx to remove the timestamps
const normalisedStatsdString = statsdString.replace(/T\d+\n?/g, 'T000000');

expect(normalisedStatsdString).toEqual(
'increment@none:2|c|T000000distribution@none:42:45|d|T000000gauge@none:15:5:15:20:2|g|T000000set@none:3387254:3443787523|s|T000000',
);
});
24 changes: 23 additions & 1 deletion dev-packages/browser-integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Page, Request } from '@playwright/test';
import type { EnvelopeItemType, Event, EventEnvelopeHeaders } from '@sentry/types';
import type { EnvelopeItem, EnvelopeItemType, Event, EventEnvelopeHeaders } from '@sentry/types';
import { parseEnvelope } from '@sentry/utils';

export const envelopeUrlRegex = /\.sentry\.io\/api\/\d+\/envelope\//;

Expand All @@ -21,6 +22,27 @@ export const envelopeRequestParser = <T = Event>(request: Request | null, envelo
return envelopeParser(request)[envelopeIndex] as T;
};

/**
* The above envelope parser does not follow the envelope spec...
* ...but modifying it to follow the spec breaks a lot of the test which rely on the current indexing behavior.
*
* This parser is a temporary solution to allow us to test metrics with statsd envelopes.
*
* Eventually, all the tests should be migrated to use this 'proper' envelope parser!
*/
export const properEnvelopeParser = (request: Request | null): EnvelopeItem[] => {
// https://develop.sentry.dev/sdk/envelopes/
const envelope = request?.postData() || '';

const [, items] = parseEnvelope(envelope, new TextEncoder(), new TextDecoder());

return items;
};

export const properEnvelopeRequestParser = <T = Event>(request: Request | null, envelopeIndex = 1): T => {
return properEnvelopeParser(request)[0][envelopeIndex] as T;
};

export const envelopeHeaderRequestParser = (request: Request | null): EventEnvelopeHeaders => {
// https://develop.sentry.dev/sdk/envelopes/
const envelope = request?.postData() || '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ Sentry.init({
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
_experiments: {
metricsAggregator: true,
},
});

// Stop the process from exiting before the transaction is sent
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/index.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,5 @@ export declare const defaultStackParser: StackParser;
export declare function close(timeout?: number | undefined): PromiseLike<boolean>;
export declare function flush(timeout?: number | undefined): PromiseLike<boolean>;

export declare const metrics: typeof clientSdk.metrics & typeof serverSdk.metrics;
export default sentryAstro;
3 changes: 2 additions & 1 deletion packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ export {
FunctionToString,
// eslint-disable-next-line deprecation/deprecation
InboundFilters,
metrics,
functionToStringIntegration,
inboundFiltersIntegration,
parameterize,
Expand All @@ -77,6 +76,8 @@ export {
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
} from '@sentry/core';

export * from './metrics';

export { WINDOW } from './helpers';
export { BrowserClient } from './client';
export { makeFetchTransport, makeXHRTransport } from './transports';
Expand Down
45 changes: 45 additions & 0 deletions packages/browser/src/metrics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import type { MetricData } from '@sentry/core';
import { BrowserMetricsAggregator, metrics as metricsCore } from '@sentry/core';

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

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

/**
* 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.
*/
function set(name: string, value: number | string, data?: MetricData): void {
metricsCore.set(BrowserMetricsAggregator, name, value, data);
}

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

export const metrics = {
increment,
distribution,
set,
gauge,
};
2 changes: 1 addition & 1 deletion packages/bun/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export {
startInactiveSpan,
startSpanManual,
continueTrace,
metrics,
metricsDefault as metrics,
functionToStringIntegration,
inboundFiltersIntegration,
linkedErrorsIntegration,
Expand Down
26 changes: 12 additions & 14 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import type {
Integration,
IntegrationClass,
MetricBucketItem,
MetricsAggregator,
Outcome,
ParameterizedString,
SdkMetadata,
Expand Down Expand Up @@ -93,13 +92,6 @@ const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been ca
* }
*/
export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/**
* A reference to a metrics aggregator
*
* @experimental Note this is alpha API. It may experience breaking changes in the future.
*/
public metricsAggregator?: MetricsAggregator;

/** Options passed to the SDK. */
protected readonly _options: O;

Expand Down Expand Up @@ -280,9 +272,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
public flush(timeout?: number): PromiseLike<boolean> {
const transport = this._transport;
if (transport) {
if (this.metricsAggregator) {
this.metricsAggregator.flush();
}
this.emit('flush');
return this._isClientDoneProcessing(timeout).then(clientFinished => {
return transport.flush(timeout).then(transportFlushed => clientFinished && transportFlushed);
});
Expand All @@ -297,9 +287,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
public close(timeout?: number): PromiseLike<boolean> {
return this.flush(timeout).then(result => {
this.getOptions().enabled = false;
if (this.metricsAggregator) {
this.metricsAggregator.close();
}
this.emit('close');
return result;
});
}
Expand Down Expand Up @@ -495,6 +483,10 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/** @inheritdoc */
public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): void;

public on(hook: 'flush', callback: () => void): void;

public on(hook: 'close', callback: () => void): void;

/** @inheritdoc */
public on(hook: string, callback: unknown): void {
if (!this._hooks[hook]) {
Expand Down Expand Up @@ -541,6 +533,12 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/** @inheritdoc */
public emit(hook: 'startNavigationSpan', options: StartSpanOptions): void;

/** @inheritdoc */
public emit(hook: 'flush'): void;

/** @inheritdoc */
public emit(hook: 'close'): void;

/** @inheritdoc */
public emit(hook: string, ...rest: unknown[]): void {
if (this._hooks[hook]) {
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ export { linkedErrorsIntegration } from './integrations/linkederrors';
export { moduleMetadataIntegration } from './integrations/metadata';
export { requestDataIntegration } from './integrations/requestdata';
export { metrics } from './metrics/exports';
export type { MetricData } from './metrics/exports';
export { metricsDefault } from './metrics/exports-default';
export { BrowserMetricsAggregator } from './metrics/browser-aggregator';

/** @deprecated Import the integration function directly, e.g. `inboundFiltersIntegration()` instead of `new Integrations.InboundFilter(). */
const Integrations = INTEGRATIONS;
Expand Down
46 changes: 46 additions & 0 deletions packages/core/src/metrics/exports-default.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { MetricsAggregator } from './aggregator';
import type { MetricData } from './exports';
import { metrics as metricsCore } from './exports';

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

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

/**
* 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.
*/
function set(name: string, value: number | string, data?: MetricData): void {
metricsCore.set(MetricsAggregator, name, value, data);
}

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

export const metricsDefault = {
increment,
distribution,
set,
gauge,
};
Loading