-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(effect): Add tracing to the effectLayer #19655
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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); | ||
| } |
| 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'; |
| 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'; | ||
| export type { EffectServerLayerOptions } from './server/index'; | ||
| 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 | ||
|
|
@@ -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)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Server effectLayer eagerly calls init unlike clientMedium Severity The server Additional Locations (1)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is fine. Also, it already has |
||
| } | ||
| 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); | ||
| } |
| 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. q: Shouldn't we also move this to after the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 The good side of changing it is that |
||
|
|
||
| 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 }); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interrupted spans incorrectly marked as errorsMedium Severity Effect |
||
|
|
||
| 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 } : {}), | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing
|
||
|
|
||
| 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()); | ||
| 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; | ||
| } |


There was a problem hiding this comment.
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.tsre-exportsinitfrom./server/indexwhile also doingexport * from '@sentry/node-core/light', which also exportsinit. The equivalentindex.client.tsfile includes a/* eslint-disable import/export */comment to suppress the duplicate-export lint error for this exact pattern, butindex.server.tsis missing it.Triggered by project rule: PR Review Guidelines for Cursor Bot
There was a problem hiding this comment.
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)