Skip to content
Open
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
1 change: 1 addition & 0 deletions packages/effect/src/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export type EffectClientLayerOptions = BrowserOptions;
* This layer provides Effect applications with full Sentry instrumentation including:
* - Effect spans traced as Sentry spans
* - Effect logs forwarded to Sentry (when `enableLogs` is set)
* - Effect metrics sent to Sentry (when `enableMetrics` is set)
*
* @example
* ```typescript
Expand Down
135 changes: 135 additions & 0 deletions packages/effect/src/metrics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import { metrics as sentryMetrics } from '@sentry/core';
import * as Effect from 'effect/Effect';
import type * as Layer from 'effect/Layer';
import { scopedDiscard } from 'effect/Layer';
import * as Metric from 'effect/Metric';
import * as MetricKeyType from 'effect/MetricKeyType';
import type * as MetricPair from 'effect/MetricPair';
import * as MetricState from 'effect/MetricState';
import * as Schedule from 'effect/Schedule';

type MetricAttributes = Record<string, string>;

function labelsToAttributes(labels: ReadonlyArray<{ key: string; value: string }>): MetricAttributes {
return labels.reduce((acc, label) => ({ ...acc, [label.key]: label.value }), {});
}

function sendMetricToSentry(pair: MetricPair.MetricPair.Untyped): void {
const { metricKey, metricState } = pair;
const name = metricKey.name;
const attributes = labelsToAttributes(metricKey.tags);

if (MetricState.isCounterState(metricState)) {
const value = typeof metricState.count === 'bigint' ? Number(metricState.count) : metricState.count;
sentryMetrics.count(name, value, { attributes });
} else if (MetricState.isGaugeState(metricState)) {
const value = typeof metricState.value === 'bigint' ? Number(metricState.value) : metricState.value;
sentryMetrics.gauge(name, value, { attributes });
} else if (MetricState.isHistogramState(metricState)) {
sentryMetrics.distribution(`${name}.sum`, metricState.sum, { attributes });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Histogram/summary cumulative sum re-emitted as new distribution points

Medium Severity

For histogram and summary states, metricState.sum (a cumulative value) is emitted via sentryMetrics.distribution() on each flush. Since distribution records a new data point per call, the cumulative sum gets repeatedly added as a fresh observation every 10 seconds, producing incorrect distribution data in Sentry. This likely needs to be a gauge or use delta tracking.

Additional Locations (1)

Fix in Cursor Fix in Web

sentryMetrics.gauge(`${name}.count`, metricState.count, { attributes });
sentryMetrics.gauge(`${name}.min`, metricState.min, { attributes });
sentryMetrics.gauge(`${name}.max`, metricState.max, { attributes });
} else if (MetricState.isSummaryState(metricState)) {
sentryMetrics.distribution(`${name}.sum`, metricState.sum, { attributes });
sentryMetrics.gauge(`${name}.count`, metricState.count, { attributes });
sentryMetrics.gauge(`${name}.min`, metricState.min, { attributes });
sentryMetrics.gauge(`${name}.max`, metricState.max, { attributes });
} else if (MetricState.isFrequencyState(metricState)) {
for (const [word, count] of metricState.occurrences) {
sentryMetrics.count(name, count, {
attributes: { ...attributes, word },
});
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frequency metrics double-counted on every flush cycle

High Severity

Frequency metrics use sentryMetrics.count() (which is additive) with cumulative occurrence counts from Effect's state, but lack delta tracking. Every 10-second flush re-sends the full cumulative counts, causing Sentry to multiply the actual values by the number of flushes. Counters correctly use sendDeltaMetricToSentry for this, but frequency metrics go through sendMetricToSentry without any delta logic.

Additional Locations (1)

Fix in Cursor Fix in Web

}

function getMetricId(pair: MetricPair.MetricPair.Untyped): string {
const tags = pair.metricKey.tags.map(t => `${t.key}=${t.value}`).join(',');
return `${pair.metricKey.name}:${tags}`;
}

function sendDeltaMetricToSentry(
pair: MetricPair.MetricPair.Untyped,
previousCounterValues: Map<string, number>,
): void {
const { metricKey, metricState } = pair;
const name = metricKey.name;
const attributes = labelsToAttributes(metricKey.tags);
const metricId = getMetricId(pair);

if (MetricState.isCounterState(metricState)) {
const currentValue = typeof metricState.count === 'bigint' ? Number(metricState.count) : metricState.count;

const previousValue = previousCounterValues.get(metricId) ?? 0;
const delta = currentValue - previousValue;

if (delta > 0) {
sentryMetrics.count(name, delta, { attributes });
}

previousCounterValues.set(metricId, currentValue);
} else {
sendMetricToSentry(pair);
}
}

/**
* Flushes all Effect metrics to Sentry.
* @param previousCounterValues - Map tracking previous counter values for delta calculation
*/
function flushMetricsToSentry(previousCounterValues: Map<string, number>): void {
const snapshot = Metric.unsafeSnapshot();

snapshot.forEach((pair: MetricPair.MetricPair.Untyped) => {
if (MetricKeyType.isCounterKey(pair.metricKey.keyType)) {
sendDeltaMetricToSentry(pair, previousCounterValues);
} else {
sendMetricToSentry(pair);
}
});
}

/**
* Creates a metrics flusher with its own isolated state for delta tracking.
* Useful for testing scenarios where you need to control the lifecycle.
* @internal
*/
export function createMetricsFlusher(): {
flush: () => void;
clear: () => void;
} {
const previousCounterValues = new Map<string, number>();
return {
flush: () => flushMetricsToSentry(previousCounterValues),
clear: () => previousCounterValues.clear(),
};
}

function createMetricsReporterEffect(previousCounterValues: Map<string, number>): Effect.Effect<void, never, never> {
const schedule = Schedule.spaced('10 seconds');

return Effect.repeat(
Effect.sync(() => flushMetricsToSentry(previousCounterValues)),
schedule,
).pipe(Effect.asVoid, Effect.interruptible);
}

/**
* Effect Layer that periodically flushes metrics to Sentry.
* The layer manages its own state for delta counter calculations,
* which is automatically cleaned up when the layer is finalized.
*/
export const SentryEffectMetricsLayer: Layer.Layer<never, never, never> = scopedDiscard(
Effect.gen(function* () {
const previousCounterValues = new Map<string, number>();

yield* Effect.acquireRelease(Effect.void, () =>
Effect.sync(() => {
previousCounterValues.clear();
}),
);

yield* Effect.forkScoped(createMetricsReporterEffect(previousCounterValues));
}),
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No final metrics flush before layer teardown

Medium Severity

The SentryEffectMetricsLayer release effect clears previousCounterValues but never performs a final flush. When the layer is torn down, the scoped fiber running the periodic reporter is interrupted, and any metrics accumulated since the last 10-second flush are silently lost.

Fix in Cursor Fix in Web

1 change: 1 addition & 0 deletions packages/effect/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type EffectServerLayerOptions = NodeOptions;
* This layer provides Effect applications with full Sentry instrumentation including:
* - Effect spans traced as Sentry spans
* - Effect logs forwarded to Sentry (when `enableLogs` is set)

*
* @example
* ```typescript
Expand Down
10 changes: 8 additions & 2 deletions packages/effect/src/utils/buildEffectLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import type * as EffectLayer from 'effect/Layer';
import { empty as emptyLayer, provideMerge } from 'effect/Layer';
import { defaultLogger, replace as replaceLogger } from 'effect/Logger';
import { SentryEffectLogger } from '../logger';
import { SentryEffectMetricsLayer } from '../metrics';
import { SentryEffectTracerLayer } from '../tracer';

export interface EffectLayerBaseOptions {
enableLogs?: boolean;
enableMetrics?: boolean;
}

/**
* Builds an Effect layer that integrates Sentry tracing and logging.
* Builds an Effect layer that integrates Sentry tracing, logging, and metrics.
*
* Returns an empty layer if no Sentry client is available. Otherwise, starts with
* the Sentry tracer layer and optionally merges logging and metrics layers based
Expand All @@ -23,13 +25,17 @@ export function buildEffectLayer<T extends EffectLayerBaseOptions>(
return emptyLayer;
}

const { enableLogs = false } = options;
const { enableLogs = false, enableMetrics = true } = options;
let layer: EffectLayer.Layer<never, never, never> = SentryEffectTracerLayer;

if (enableLogs) {
const effectLogger = replaceLogger(defaultLogger, SentryEffectLogger);
layer = layer.pipe(provideMerge(effectLogger));
}

if (enableMetrics) {
layer = layer.pipe(provideMerge(SentryEffectMetricsLayer));
}

return layer;
}
35 changes: 24 additions & 11 deletions packages/effect/test/buildEffectLayer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,22 @@ describe('buildEffectLayer', () => {
expect(Layer.isLayer(layer)).toBe(true);
});

it('returns a valid layer with enableMetrics: false', () => {
const layer = buildEffectLayer({ enableMetrics: false }, mockClient);

expect(layer).toBeDefined();
expect(Layer.isLayer(layer)).toBe(true);
});

it('returns a valid layer with enableMetrics: true', () => {
const layer = buildEffectLayer({ enableMetrics: true }, mockClient);

expect(layer).toBeDefined();
expect(Layer.isLayer(layer)).toBe(true);
});

it('returns a valid layer with all features enabled', () => {
const layer = buildEffectLayer({ enableLogs: true }, mockClient);
const layer = buildEffectLayer({ enableLogs: true, enableMetrics: true }, mockClient);

expect(layer).toBeDefined();
expect(Layer.isLayer(layer)).toBe(true);
Expand All @@ -71,20 +85,18 @@ describe('buildEffectLayer', () => {
}).pipe(Effect.provide(buildEffectLayer({ enableLogs: true }, mockClient))),
);

it.effect('layer with logs disabled routes Effect does not log to Sentry logger', () =>
Effect.gen(function* () {
const infoSpy = vi.spyOn(sentryLogger, 'info');
yield* Effect.log('test log message');
expect(infoSpy).not.toHaveBeenCalled();
infoSpy.mockRestore();
}).pipe(Effect.provide(buildEffectLayer({ enableLogs: false }, mockClient))),
);
it('returns different layer when enableMetrics is true vs false', () => {
const layerWithMetrics = buildEffectLayer({ enableMetrics: true }, mockClient);
const layerWithoutMetrics = buildEffectLayer({ enableMetrics: false }, mockClient);

expect(layerWithMetrics).not.toBe(layerWithoutMetrics);
});

it.effect('layer with all features enabled can be provided to an Effect program', () =>
Effect.gen(function* () {
const result = yield* Effect.succeed('all-features');
expect(result).toBe('all-features');
}).pipe(Effect.provide(buildEffectLayer({ enableLogs: true }, mockClient))),
}).pipe(Effect.provide(buildEffectLayer({ enableLogs: true, enableMetrics: true }, mockClient))),
);

it.effect('layer enables tracing for Effect spans via Sentry tracer', () =>
Expand All @@ -109,9 +121,10 @@ describe('buildEffectLayer', () => {
const layer = buildEffectLayer(
{
enableLogs: true,
enableMetrics: true,
dsn: 'https://test@sentry.io/123',
debug: true,
} as { enableLogs?: boolean; dsn?: string; debug?: boolean },
} as { enableLogs?: boolean; enableMetrics?: boolean; dsn?: string; debug?: boolean },
mockClient,
);

Expand Down
1 change: 1 addition & 0 deletions packages/effect/test/layer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ describe.each([
dsn: TEST_DSN,
transport: getMockTransport(),
enableLogs: true,
enableMetrics: true,
});

expect(layer).toBeDefined();
Expand Down
Loading
Loading