Skip to content
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
16 changes: 12 additions & 4 deletions packages/effect/src/client/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
import type { BrowserOptions } from '@sentry/browser';
import * as EffectLayer from 'effect/Layer';
import type * as EffectLayer from 'effect/Layer';
import { suspend as suspendLayer } from 'effect/Layer';
import { buildEffectLayer } from '../utils/buildEffectLayer';
import { init } from './sdk';

export { init } from './sdk';

/**
* Options for the Sentry Effect client layer.
*/
export type EffectClientLayerOptions = BrowserOptions;

/**
* Creates an empty Effect Layer
* Creates an Effect Layer that initializes Sentry for browser clients.
*
* This layer provides Effect applications with full Sentry instrumentation including:
* - Effect spans traced as Sentry spans
*
* @example
* ```typescript
Expand All @@ -25,6 +33,6 @@ export type EffectClientLayerOptions = BrowserOptions;
* Effect.runPromise(Effect.provide(myEffect, ApiClientWithSentry));
* ```
*/
export function effectLayer(_: EffectClientLayerOptions): EffectLayer.Layer<never, never, never> {
return EffectLayer.empty;
export function effectLayer(options: EffectClientLayerOptions): EffectLayer.Layer<never, never, never> {
return suspendLayer(() => buildEffectLayer(options, init(options)));
}
20 changes: 20 additions & 0 deletions packages/effect/src/client/sdk.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import type { BrowserOptions } from '@sentry/browser';
import { init as initBrowser } from '@sentry/browser';
import type { Client } from '@sentry/core';
import { applySdkMetadata } from '@sentry/core';

/**
* Initializes the Sentry Effect SDK for browser clients.
*
* @param options - Configuration options for the SDK
* @returns The initialized Sentry client, or undefined if initialization failed
*/
export function init(options: BrowserOptions): Client | undefined {
const opts = {
...options,
};

applySdkMetadata(opts, 'effect', ['effect', 'browser']);

return initBrowser(opts);
}
5 changes: 4 additions & 1 deletion packages/effect/src/index.client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// import/export got a false positive, and affects most of our index barrel files
// can be removed once following issue is fixed: https://github.com/import-js/eslint-plugin-import/issues/703
/* eslint-disable import/export */
export * from '@sentry/browser';

export { effectLayer } from './client/index';
export { effectLayer, init } from './client/index';
export type { EffectClientLayerOptions } from './client/index';
2 changes: 1 addition & 1 deletion packages/effect/src/index.server.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export * from '@sentry/node-core/light';

export { effectLayer } from './server/index';
export { effectLayer, init } from './server/index';
Copy link

Choose a reason for hiding this comment

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

Missing eslint-disable for duplicate init export in server

Low Severity

index.server.ts re-exports init from ./server/index while also doing export * from '@sentry/node-core/light', which also exports init. The equivalent index.client.ts file includes a /* eslint-disable import/export */ comment to suppress the duplicate-export lint error for this exact pattern, but index.server.ts is missing it.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Copy link
Member Author

Choose a reason for hiding this comment

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

It is missing because there is no lint error. Apparently the ESLint rule has its problems with sub exports (or it works properly there, but that I doubt highly)

export type { EffectServerLayerOptions } from './server/index';
17 changes: 12 additions & 5 deletions packages/effect/src/server/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
import type { NodeOptions } from '@sentry/node-core';
import * as EffectLayer from 'effect/Layer';
import type { NodeOptions } from '@sentry/node-core/light';
import type * as EffectLayer from 'effect/Layer';
import { buildEffectLayer } from '../utils/buildEffectLayer';
import { init } from './sdk';

export { init } from './sdk';

/**
* Options for the Sentry Effect server layer.
*/
export type EffectServerLayerOptions = NodeOptions;

/**
* Creates an empty Effect Layer
* Creates an Effect Layer that initializes Sentry for Node.js servers.
*
* This layer provides Effect applications with full Sentry instrumentation including:
* - Effect spans traced as Sentry spans
*
* @example
* ```typescript
Expand All @@ -27,6 +34,6 @@ export type EffectServerLayerOptions = NodeOptions;
* MainLive.pipe(Layer.launch, NodeRuntime.runMain);
* ```
*/
export function effectLayer(_: EffectServerLayerOptions): EffectLayer.Layer<never, never, never> {
return EffectLayer.empty;
export function effectLayer(options: EffectServerLayerOptions): EffectLayer.Layer<never, never, never> {
return buildEffectLayer(options, init(options));
Copy link

Choose a reason for hiding this comment

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

Server effectLayer eagerly calls init unlike client

Medium Severity

The server effectLayer calls init(options) eagerly at function-call time, while the client version wraps the call in suspendLayer to defer initialization until the layer is actually provided to an Effect program. This means on the server, calling effectLayer(options) immediately initializes Sentry as a side effect — even if the returned layer is never used. Multiple calls to effectLayer would also call init multiple times on the server, but only once (per layer usage) on the client. The server version likely needs suspendLayer wrapping for consistency.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is fine. Also, it already has suspendLayer as a wrapper

}
20 changes: 20 additions & 0 deletions packages/effect/src/server/sdk.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import type { Client } from '@sentry/core';
import { applySdkMetadata } from '@sentry/core';
import type { NodeOptions } from '@sentry/node-core/light';
import { init as initNode } from '@sentry/node-core/light';

/**
* Initializes the Sentry Effect SDK for Node.js servers.
*
* @param options - Configuration options for the SDK
* @returns The initialized Sentry client, or undefined if initialization failed
*/
export function init(options: NodeOptions): Client | undefined {
const opts = {
...options,
};

applySdkMetadata(opts, 'effect', ['effect', 'node-light']);

return initNode(opts);
}
201 changes: 201 additions & 0 deletions packages/effect/src/tracer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
import type { Span } from '@sentry/core';
import {
getActiveSpan,
getIsolationScope,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
startInactiveSpan,
withActiveSpan,
} from '@sentry/core';
import type * as Context from 'effect/Context';
import * as Exit from 'effect/Exit';
import type * as Layer from 'effect/Layer';
import { setTracer } from 'effect/Layer';
import * as Option from 'effect/Option';
import * as EffectTracer from 'effect/Tracer';

const KIND_MAP: Record<EffectTracer.SpanKind, 'internal' | 'server' | 'client' | 'producer' | 'consumer'> = {
internal: 'internal',
client: 'client',
server: 'server',
producer: 'producer',
consumer: 'consumer',
};

function deriveOp(name: string, kind: EffectTracer.SpanKind): string {
if (name.startsWith('http.server')) {
return 'http.server';
}

if (name.startsWith('http.client')) {
return 'http.client';
}

return KIND_MAP[kind];
}

function deriveOrigin(name: string): string {
if (name.startsWith('http.server') || name.startsWith('http.client')) {
return 'auto.http.effect';
}

return 'auto.function.effect';
}

function deriveSpanName(name: string, kind: EffectTracer.SpanKind): string {
if (name.startsWith('http.server') && kind === 'server') {
const isolationScope = getIsolationScope();
const transactionName = isolationScope.getScopeData().transactionName;
if (transactionName) {
return transactionName;
}
}
return name;
}

type HrTime = [number, number];

const SENTRY_SPAN_SYMBOL = Symbol.for('@sentry/effect.SentrySpan');

function nanosToHrTime(nanos: bigint): HrTime {
const seconds = Number(nanos / BigInt(1_000_000_000));
const remainingNanos = Number(nanos % BigInt(1_000_000_000));
return [seconds, remainingNanos];
}

interface SentrySpanLike extends EffectTracer.Span {
readonly [SENTRY_SPAN_SYMBOL]: true;
readonly sentrySpan: Span;
}

function isSentrySpan(span: EffectTracer.AnySpan): span is SentrySpanLike {
return SENTRY_SPAN_SYMBOL in span;
}

class SentrySpanWrapper implements SentrySpanLike {
public readonly [SENTRY_SPAN_SYMBOL]: true;
public readonly _tag: 'Span';
public readonly spanId: string;
public readonly traceId: string;
public readonly attributes: Map<string, unknown>;
public readonly sampled: boolean;
public readonly parent: Option.Option<EffectTracer.AnySpan>;
public readonly links: Array<EffectTracer.SpanLink>;
public status: EffectTracer.SpanStatus;
public readonly sentrySpan: Span;

public constructor(
public readonly name: string,
parent: Option.Option<EffectTracer.AnySpan>,
public readonly context: Context.Context<never>,
links: ReadonlyArray<EffectTracer.SpanLink>,
startTime: bigint,
public readonly kind: EffectTracer.SpanKind,
existingSpan: Span,
) {
this[SENTRY_SPAN_SYMBOL] = true as const;
this._tag = 'Span' as const;
this.attributes = new Map<string, unknown>();
this.parent = parent;
this.links = [...links];
this.sentrySpan = existingSpan;

const spanContext = this.sentrySpan.spanContext();
this.spanId = spanContext.spanId;
this.traceId = spanContext.traceId;
this.sampled = this.sentrySpan.isRecording();
this.status = {
_tag: 'Started',
startTime,
};
}

public attribute(key: string, value: unknown): void {
if (!this.sentrySpan.isRecording()) {
return;
}

this.sentrySpan.setAttribute(key, value as Parameters<Span['setAttribute']>[1]);
this.attributes.set(key, value);
}

public addLinks(links: ReadonlyArray<EffectTracer.SpanLink>): void {
this.links.push(...links);
}

public end(endTime: bigint, exit: Exit.Exit<unknown, unknown>): void {
this.status = {
_tag: 'Ended',
endTime,
exit,
startTime: this.status.startTime,
};
Comment on lines +126 to +131
Copy link
Member

Choose a reason for hiding this comment

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

q: Shouldn't we also move this to after the isRecording check?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a very good question indeed. I'm in between of changing and not changing it. For one reason:

When the span would end from the outside (for whatever reason), the this.status would never be set. It would hold a slightly false endTime, but it would set the _tag to Ended. If we would change it and the span would end from the outside the this.status would always be _tag: 'Started'. This doesn't have any downsides right now, as the information is not really used right now, but it would be false.

The good side of changing it is that endTime would always be correct (IF it was set). I still lean to not changing it and leaving it as is, for the sake of setting _tag to Ended in this edge case scenario.


if (!this.sentrySpan.isRecording()) {
return;
}

if (Exit.isFailure(exit)) {
const cause = exit.cause;
const message =
cause._tag === 'Fail' ? String(cause.error) : cause._tag === 'Die' ? String(cause.defect) : 'internal_error';
this.sentrySpan.setStatus({ code: 2, message });
} else {
this.sentrySpan.setStatus({ code: 1 });
}
Copy link

Choose a reason for hiding this comment

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

Interrupted spans incorrectly marked as errors

Medium Severity

Effect Interrupt causes (fiber cancellations) fall through the ternary chain and get span status code: 2 (ERROR) with message 'internal_error'. In Effect, interruptions are a normal control flow mechanism (cooperative cancellation), not errors. Marking interrupted spans as errors will inflate error rates in Sentry dashboards, producing false positive alerts. The cause._tag check only handles 'Fail' and 'Die', but 'Interrupt', 'Sequential', and 'Parallel' all silently become error spans with a misleading 'internal_error' message. Interruptions could use code: 1 (OK) or a 'cancelled' message, and composite causes (Sequential/Parallel) deserve more descriptive messages.

Fix in Cursor Fix in Web


this.sentrySpan.end(nanosToHrTime(endTime));
}

public event(name: string, startTime: bigint, attributes?: Record<string, unknown>): void {
if (!this.sentrySpan.isRecording()) {
return;
}

this.sentrySpan.addEvent(name, attributes as Parameters<Span['addEvent']>[1], nanosToHrTime(startTime));
}
}

function createSentrySpan(
name: string,
parent: Option.Option<EffectTracer.AnySpan>,
context: Context.Context<never>,
links: ReadonlyArray<EffectTracer.SpanLink>,
startTime: bigint,
kind: EffectTracer.SpanKind,
): SentrySpanLike {
const parentSentrySpan =
Option.isSome(parent) && isSentrySpan(parent.value) ? parent.value.sentrySpan : (getActiveSpan() ?? null);

const spanName = deriveSpanName(name, kind);

const newSpan = startInactiveSpan({
name: spanName,
op: deriveOp(name, kind),
startTime: nanosToHrTime(startTime),
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: deriveOrigin(name),
},
...(parentSentrySpan ? { parentSpan: parentSentrySpan } : {}),
});
Copy link

Choose a reason for hiding this comment

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

Missing SEMANTIC_ATTRIBUTE_SENTRY_OP attribute in startInactiveSpan

Low Severity

The startInactiveSpan call sets the op via the top-level op option but does not set SEMANTIC_ATTRIBUTE_SENTRY_OP as a span attribute. The review rules require that SEMANTIC_ATTRIBUTE_SENTRY_OP is set in span attributes when calling any startSpan API. Other packages in this codebase (e.g., nestjs) follow the convention of setting it explicitly in the attributes object using the SEMANTIC_ATTRIBUTE_SENTRY_OP constant.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Copy link
Member Author

Choose a reason for hiding this comment

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

Not for startInactiveSpan, at least based on my short research


return new SentrySpanWrapper(name, parent, context, links, startTime, kind, newSpan);
}

const makeSentryTracer = (): EffectTracer.Tracer =>
EffectTracer.make({
span(name, parent, context, links, startTime, kind) {
return createSentrySpan(name, parent, context, links, startTime, kind);
},
context(execution, fiber) {
const currentSpan = fiber.currentSpan;
if (currentSpan === undefined || !isSentrySpan(currentSpan)) {
return execution();
}
return withActiveSpan(currentSpan.sentrySpan, execution);
},
});

/**
* Effect Layer that sets up the Sentry tracer for Effect spans.
*/
export const SentryEffectTracerLayer: Layer.Layer<never, never, never> = setTracer(makeSentryTracer());
24 changes: 24 additions & 0 deletions packages/effect/src/utils/buildEffectLayer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type * as EffectLayer from 'effect/Layer';
import { empty as emptyLayer } from 'effect/Layer';
import { SentryEffectTracerLayer } from '../tracer';

// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface EffectLayerBaseOptions {}

/**
* Builds an Effect layer that integrates Sentry tracing.
*
* 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
* on the provided options.
*/
export function buildEffectLayer<T extends EffectLayerBaseOptions>(
options: T,
client: unknown,
): EffectLayer.Layer<never, never, never> {
if (!client) {
return emptyLayer;
}

return SentryEffectTracerLayer;
}
Loading
Loading