Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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,6 +1,10 @@
// @ts-check
import * as Sentry from '@sentry/effect';
import { Cause, Effect, Layer, Logger, LogLevel, Runtime } from 'effect';
import * as Logger from 'effect/Logger';
import * as Layer from 'effect/Layer';
import * as Runtime from 'effect/Runtime';
import * as LogLevel from 'effect/LogLevel';
import * as Effect from 'effect/Effect';

const LogLevelLive = Logger.minimumLogLevel(LogLevel.Debug);
const AppLayer = Layer.mergeAll(
Expand All @@ -16,8 +20,9 @@ const AppLayer = Layer.mergeAll(
environment: 'qa',
tunnel: 'http://localhost:3031',
enableLogs: true,
enableEffectLogs: true,
}),
Layer.setTracer(Sentry.SentryEffectTracer),
Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger),
LogLevelLive,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,6 @@ test('captures Effect spans with correct parent-child structure', async ({ page
expect(spans).toContainEqual(
expect.objectContaining({
description: 'custom-effect-span',
data: expect.objectContaining({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

l: just checking, no action required: Does effect add any other attributes we could/should assert on?

(also, removing the op here sounds correct to me. Span kind doesn't map well to sentry ops and there's no internal op defined)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added couple of more assertions to this specific test and removed the .toContainEqual in the file completely.

'sentry.op': 'internal',
}),
}),
);

Expand Down
27 changes: 17 additions & 10 deletions dev-packages/e2e-tests/test-applications/effect-node/src/app.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
import * as Sentry from '@sentry/effect';
import { HttpRouter, HttpServer, HttpServerResponse } from '@effect/platform';
import { NodeHttpServer, NodeRuntime } from '@effect/platform-node';
import { Cause, Effect, Layer, Logger, LogLevel } from 'effect';
import * as Effect from 'effect/Effect';
import * as Cause from 'effect/Cause';
import * as Layer from 'effect/Layer';
import * as Logger from 'effect/Logger';
import * as LogLevel from 'effect/LogLevel';
import { createServer } from 'http';

const SentryLive = Sentry.effectLayer({
dsn: process.env.E2E_TEST_DSN,
environment: 'qa',
debug: !!process.env.DEBUG,
tunnel: 'http://localhost:3031/',
tracesSampleRate: 1,
enableLogs: true,
enableEffectLogs: true,
});
const SentryLive = Layer.mergeAll(
Sentry.effectLayer({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is a fine setup and naming is also fine. To me (as an Effect noob), this signals that Sentry.effectLayer is the main one and the others are additional components. As long as we document this correctly, we should be good.

I have a question though: Does registering just the Sentry.effectLayer initialize error monitoring? This would be ideal, since errors should be part of the minimal Sentry setup. I believe you said for v4, we'll need an additional component for errors, which is also fine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Does registering just the Sentry.effectLayer initialize error monitoring?

Theoretically it would do everything a Sentry.init would do, so if the process crashes or someone would throw an unhandled error then yes. Once we get support for the ErrorReporter in v4, we could think of auto adding this or adding it manually with the layer (currently leaning towards auto adding, but this is another discussion)

dsn: process.env.E2E_TEST_DSN,
environment: 'qa',
debug: !!process.env.DEBUG,
tunnel: 'http://localhost:3031/',
tracesSampleRate: 1,
enableLogs: true,
}),
Layer.setTracer(Sentry.SentryEffectTracer),
Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger),
);

const router = HttpRouter.empty.pipe(
HttpRouter.get('/test-success', HttpServerResponse.json({ version: 'v1' })),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,29 @@ import { waitForTransaction } from '@sentry-internal/test-utils';

test('Sends an HTTP transaction', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('effect-node', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ambiguous E2E test transaction filters match wrong transactions

Medium Severity

The "Sends an HTTP transaction" and "Sends transaction for error route" tests both use the identical filter transactionEvent?.transaction === 'http.server GET' with no route-specific distinguishing criteria. Previously, filters included route paths like /test-success and /test-error. Now any http.server GET transaction satisfies both filters, making these tests flaky and unable to verify they received the transaction for the correct route.

Additional Locations (1)
Fix in Cursor Fix in Web

transactionEvent?.transaction?.includes('/test-success')
);
return transactionEvent?.transaction === 'http.server GET';
});

await fetch(`${baseURL}/test-success`);

const transactionEvent = await transactionEventPromise;

expect(transactionEvent.contexts?.trace?.op).toBe('http.server');
expect(transactionEvent.transaction).toContain('/test-success');
expect(transactionEvent.transaction).toBe('http.server GET');
});

test('Sends transaction with manual Effect span', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('effect-node', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction?.includes('/test-transaction')
transactionEvent?.transaction === 'http.server GET' &&
transactionEvent?.spans?.some(span => span.description === 'test-span')
);
});

await fetch(`${baseURL}/test-transaction`);

const transactionEvent = await transactionEventPromise;

expect(transactionEvent.contexts?.trace?.op).toBe('http.server');
expect(transactionEvent.transaction).toContain('/test-transaction');
expect(transactionEvent.transaction).toBe('http.server GET');

const spans = transactionEvent.spans || [];
expect(spans).toContainEqual(
Expand All @@ -43,24 +38,22 @@ test('Sends transaction with manual Effect span', async ({ baseURL }) => {
test('Sends Effect spans with correct parent-child structure', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('effect-node', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction?.includes('/test-effect-span')
transactionEvent?.transaction === 'http.server GET' &&
transactionEvent?.spans?.some(span => span.description === 'custom-effect-span')
);
});

await fetch(`${baseURL}/test-effect-span`);

const transactionEvent = await transactionEventPromise;

expect(transactionEvent.contexts?.trace?.op).toBe('http.server');
expect(transactionEvent.transaction).toContain('/test-effect-span');
expect(transactionEvent.transaction).toBe('http.server GET');

const spans = transactionEvent.spans || [];

expect(spans).toContainEqual(
expect.objectContaining({
description: 'custom-effect-span',
op: 'internal',
}),
);

Expand All @@ -77,15 +70,12 @@ test('Sends Effect spans with correct parent-child structure', async ({ baseURL

test('Sends transaction for error route', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('effect-node', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction?.includes('/test-error')
);
return transactionEvent?.transaction === 'http.server GET';
});

await fetch(`${baseURL}/test-error`);

const transactionEvent = await transactionEventPromise;

expect(transactionEvent.contexts?.trace?.op).toBe('http.server');
expect(transactionEvent.transaction).toContain('/test-error');
expect(transactionEvent.transaction).toBe('http.server GET');
});
30 changes: 14 additions & 16 deletions packages/effect/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,28 @@ This SDK does not have docs yet. Stay tuned.
```typescript
import * as Sentry from '@sentry/effect/server';
import { NodeRuntime } from '@effect/platform-node';
import { Layer } from 'effect';
import { Layer, Logger } from 'effect';
import { HttpLive } from './Http.js';

const MainLive = HttpLive.pipe(
Layer.provide(
Sentry.effectLayer({
dsn: '__DSN__',
tracesSampleRate: 1.0,
enableLogs: true,
enableEffectLogs: true,
enableEffectMetrics: true,
}),
),
const SentryLive = Layer.mergeAll(
Sentry.effectLayer({
dsn: '__DSN__',
tracesSampleRate: 1.0,
enableLogs: true,
}),
Layer.setTracer(Sentry.SentryEffectTracer),
Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

l: should we add the metrics layer here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

well said. Will add

);

const MainLive = HttpLive.pipe(Layer.provide(SentryLive));
MainLive.pipe(Layer.launch, NodeRuntime.runMain);
```

The `effectLayer` function initializes Sentry and returns an Effect Layer that provides:
The `effectLayer` function initializes Sentry. To enable Effect instrumentation, compose with:

- Distributed tracing with automatic HTTP header extraction/injection
- Effect spans traced as Sentry spans
- Effect logs forwarded to Sentry (when `enableEffectLogs` is set)
- Effect metrics sent to Sentry (when `enableEffectMetrics` is set)
- `Layer.setTracer(Sentry.SentryEffectTracer)` - Effect spans traced as Sentry spans
- `Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger)` - Effect logs forwarded to Sentry
- `Sentry.SentryEffectMetricsLayer` - Effect metrics sent to Sentry

## Links

Expand Down
32 changes: 17 additions & 15 deletions packages/effect/src/client/index.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,43 @@
import type { BrowserOptions } from '@sentry/browser';
import type * as EffectLayer from 'effect/Layer';
import { suspend as suspendLayer } from 'effect/Layer';
import type { EffectLayerBaseOptions } from '../utils/buildEffectLayer';
import { buildEffectLayer } from '../utils/buildEffectLayer';
import { empty as emptyLayer, suspend as suspendLayer } from 'effect/Layer';
import { init } from './sdk';

export { init } from './sdk';

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

/**
* 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
* - Effect logs forwarded to Sentry (when `enableEffectLogs` is set)
* - Effect metrics sent to Sentry (when `enableEffectMetrics` is set)
* To enable Effect tracing, logs, or metrics, compose with the respective layers:
* - `Layer.setTracer(Sentry.SentryEffectTracer)` for tracing
* - `Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger)` for logs
* - `Sentry.SentryEffectMetricsLayer` for metrics
*
* @example
* ```typescript
* import * as Sentry from '@sentry/effect/client';
* import { Layer, Effect } from 'effect';
* import { Layer, Logger, LogLevel } from 'effect';
*
* const ApiClientWithSentry = ApiClientLive.pipe(
* Layer.provide(Sentry.effectLayer({
* const SentryLive = Layer.mergeAll(
* Sentry.effectLayer({
* dsn: '__DSN__',
* integrations: [Sentry.browserTracingIntegration()],
* tracesSampleRate: 1.0,
* })),
* }),
* Layer.setTracer(Sentry.SentryEffectTracer),
* Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger),
* );
*
* Effect.runPromise(Effect.provide(myEffect, ApiClientWithSentry));
* ```
*/
export function effectLayer(options: EffectClientLayerOptions): EffectLayer.Layer<never, never, never> {
return suspendLayer(() => buildEffectLayer(options, init(options)));
return suspendLayer(() => {
init(options);

return emptyLayer;
});
}
31 changes: 16 additions & 15 deletions packages/effect/src/server/index.ts
Original file line number Diff line number Diff line change
@@ -1,42 +1,43 @@
import type { NodeOptions } from '@sentry/node-core/light';
import type * as EffectLayer from 'effect/Layer';
import type { EffectLayerBaseOptions } from '../utils/buildEffectLayer';
import { buildEffectLayer } from '../utils/buildEffectLayer';
import { empty as emptyLayer, suspend as suspendLayer } from 'effect/Layer';
import { init } from './sdk';

export { init } from './sdk';

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

/**
* 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
* - Effect logs forwarded to Sentry (when `enableEffectLogs` is set)
* - Effect metrics sent to Sentry (when `enableEffectMetrics` is set)
* To enable Effect tracing, logs, or metrics, compose with the respective layers:
* - `Layer.setTracer(Sentry.SentryEffectTracer)` for tracing
* - `Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger)` for logs
* - `Sentry.SentryEffectMetricsLayer` for metrics
*
* @example
* ```typescript
* import * as Sentry from '@sentry/effect/server';
* import { NodeRuntime } from '@effect/platform-node';
* import { Layer } from 'effect';
* import { Layer, Logger } from 'effect';
* import { HttpLive } from './Http.js';
*
* const MainLive = HttpLive.pipe(
* Layer.provide(Sentry.effectLayer({
* dsn: '__DSN__',
* enableEffectLogs: true,
* enableEffectMetrics: true,
* })),
* const SentryLive = Layer.mergeAll(
* Sentry.effectLayer({ dsn: '__DSN__' }),
* Layer.setTracer(Sentry.SentryEffectTracer),
* Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger),
* );
*
* const MainLive = HttpLive.pipe(Layer.provide(SentryLive));
* MainLive.pipe(Layer.launch, NodeRuntime.runMain);
* ```
*/
export function effectLayer(options: EffectServerLayerOptions): EffectLayer.Layer<never, never, never> {
return buildEffectLayer(options, init(options));
return suspendLayer(() => {
init(options);
return emptyLayer;
});
}
44 changes: 2 additions & 42 deletions packages/effect/src/tracer.ts
Original file line number Diff line number Diff line change
@@ -1,36 +1,10 @@
import type { Span } from '@sentry/core';
import {
getActiveSpan,
getIsolationScope,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
startInactiveSpan,
withActiveSpan,
} from '@sentry/core';
import { getActiveSpan, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan, withActiveSpan } from '@sentry/core';
import type * as Context from 'effect/Context';
import * as Exit from 'effect/Exit';
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';
Expand All @@ -39,17 +13,6 @@ function deriveOrigin(name: string): string {
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');
Expand Down Expand Up @@ -164,11 +127,8 @@ function createSentrySpan(
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),
name,
Copy link
Copy Markdown

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 in startInactiveSpan call

Medium Severity

The startInactiveSpan call no longer sets SEMANTIC_ATTRIBUTE_SENTRY_OP ('sentry.op'). The deriveOp function and the op parameter were removed as part of this PR. Per project rules, when calling any startSpan API, SEMANTIC_ATTRIBUTE_SENTRY_OP must always be set with a proper span op. The origin attribute is still correctly set, but the op attribute is missing entirely from the attributes object.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think not adding an op is fine for the initial release. However, as discussed we should refine our deriveSpanName and deriveOp logic to make Effects' spans more Sentry-esque in the long run. For example, we need sentry.op and certain span names to make our Insights pages work correctly. This especially concerns http.server, http.client and db spans.

Again, for this release, I'd rather not have ops than incorrect ones though.

startTime: nanosToHrTime(startTime),
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: deriveOrigin(name),
Expand Down
Loading
Loading