Skip to content

Commit

Permalink
Avoid eager read of the stack when captured by a span (#2798)
Browse files Browse the repository at this point in the history
  • Loading branch information
mikearnaldi committed May 21, 2024
1 parent 5d9266e commit 6a6f670
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/gorgeous-ties-tan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"effect": patch
---

Avoid eager read of the stack when captured by a span
3 changes: 2 additions & 1 deletion packages/effect/src/Tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type * as Context from "./Context.js"
import type * as Effect from "./Effect.js"
import type * as Exit from "./Exit.js"
import type * as Fiber from "./Fiber.js"
import type { LazyArg } from "./Function.js"
import * as defaultServices from "./internal/defaultServices.js"
import * as internal from "./internal/tracer.js"
import type * as Option from "./Option.js"
Expand Down Expand Up @@ -92,7 +93,7 @@ export interface SpanOptions {
readonly root?: boolean | undefined
readonly context?: Context.Context<never> | undefined
readonly kind?: SpanKind | undefined
readonly captureStackTrace?: boolean | string | undefined
readonly captureStackTrace?: boolean | LazyArg<string | undefined> | undefined
}

/**
Expand Down
9 changes: 7 additions & 2 deletions packages/effect/src/internal/cause.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as Either from "../Either.js"
import * as Equal from "../Equal.js"
import type * as FiberId from "../FiberId.js"
import { constFalse, constTrue, dual, identity, pipe } from "../Function.js"
import { globalValue } from "../GlobalValue.js"
import * as Hash from "../Hash.js"
import * as HashSet from "../HashSet.js"
import { NodeInspectSymbol, toJSON } from "../Inspectable.js"
Expand Down Expand Up @@ -1052,6 +1053,9 @@ export const prettyErrorMessage = (u: unknown): string => {

const locationRegex = /\((.*)\)/

/** @internal */
export const spanToTrace = globalValue("effect/Tracer/spanToTrace", () => new WeakMap())

const prettyErrorStack = (message: string, stack: string, span?: Span | undefined): string => {
const out: Array<string> = [message]
const lines = stack.split("\n")
Expand All @@ -1072,8 +1076,9 @@ const prettyErrorStack = (message: string, stack: string, span?: Span | undefine
let current: Span | AnySpan | undefined = span
let i = 0
while (current && current._tag === "Span" && i < 10) {
const stack = current.attributes.get("code.stacktrace")
if (typeof stack === "string") {
const stackFn = spanToTrace.get(current)
if (typeof stackFn === "function") {
const stack = stackFn()
const locationMatch = stack.match(locationRegex)
const location = locationMatch ? locationMatch[1] : stack.replace(/^at /, "")
out.push(` at ${current.name} (${location})`)
Expand Down
22 changes: 16 additions & 6 deletions packages/effect/src/internal/core-effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2054,8 +2054,8 @@ export const unsafeMakeSpan = <XA, XE>(
options.kind ?? "internal"
)

if (typeof options.captureStackTrace === "string") {
span.attribute("code.stacktrace", options.captureStackTrace)
if (typeof options.captureStackTrace === "function") {
internalCause.spanToTrace.set(span, options.captureStackTrace)
}

if (annotationsFromEnv._tag === "Some") {
Expand Down Expand Up @@ -2112,6 +2112,9 @@ export const useSpan: {
if (span.status._tag === "Ended") {
return
}
if (core.exitIsFailure(exit) && internalCause.spanToTrace.has(span)) {
span.attribute("code.stacktrace", internalCause.spanToTrace.get(span)!())
}
span.end(timingEnabled ? clock.unsafeCurrentTimeNanos() : bigint0, exit)
}))
})
Expand Down Expand Up @@ -2155,15 +2158,22 @@ export const functionWithSpan = <Args extends Array<any>, Ret extends Effect.Eff
}
): (...args: Args) => Unify<Ret> =>
(function(this: any) {
let captureStackTrace: string | boolean = options.captureStackTrace ?? false
let captureStackTrace: LazyArg<string | undefined> | boolean = options.captureStackTrace ?? false
if (options.captureStackTrace !== false) {
const limit = Error.stackTraceLimit
Error.stackTraceLimit = 2
const error = new Error()
Error.stackTraceLimit = limit
if (error.stack !== undefined) {
const stack = error.stack.trim().split("\n")
captureStackTrace = stack.slice(2).join("\n").trim()
let cache: false | string = false
captureStackTrace = () => {
if (cache !== false) {
return cache
}
if (error.stack) {
const stack = error.stack.trim().split("\n")
cache = stack.slice(2).join("\n").trim()
return cache
}
}
}
// eslint-disable-next-line @typescript-eslint/no-this-alias
Expand Down
22 changes: 15 additions & 7 deletions packages/effect/src/internal/tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,20 @@ export const addSpanStackTrace = (options: Tracer.SpanOptions | undefined): Trac
Error.stackTraceLimit = 3
const traceError = new Error()
Error.stackTraceLimit = limit
if (traceError.stack === undefined) {
return { ...options, captureStackTrace: false }
}
const stack = traceError.stack.split("\n")
if (!stack[3]) {
return { ...options, captureStackTrace: false }
let cache: false | string = false
return {
...options,
captureStackTrace: () => {
if (cache !== false) {
return cache
}
if (traceError.stack !== undefined) {
const stack = traceError.stack.split("\n")
if (stack[3] !== undefined) {
cache = stack[3].trim()
return cache
}
}
}
}
return { ...options, captureStackTrace: stack[3].trim() }
}
33 changes: 26 additions & 7 deletions packages/effect/test/Tracer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe("Tracer", () => {
const span = yield* Effect.withSpan("A")(Effect.currentSpan)
assert.deepEqual(span.name, "A")
assert.deepEqual(span.parent, Option.none())
assert.include(span.attributes.get("code.stacktrace"), "Tracer.test.ts:18")
assert.strictEqual(span.attributes.get("code.stacktrace"), undefined)
}))

it.effect("parent", () =>
Expand Down Expand Up @@ -139,7 +139,7 @@ describe("Tracer", () => {
Effect.gen(function*() {
const span = yield* Effect.scoped(Effect.makeSpanScoped("A"))
assert.deepEqual(span.status._tag, "Ended")
assert.include(span.attributes.get("code.stacktrace"), "Tracer.test.ts:140")
assert.strictEqual(span.attributes.get("code.stacktrace"), undefined)
}))

it.effect("annotateCurrentSpan", () =>
Expand Down Expand Up @@ -176,8 +176,8 @@ describe("Tracer", () => {
const span = yield* Effect.makeSpan("child")
const parent = yield* Option.filter(span.parent, (span): span is Span => span._tag === "Span")
assert.deepEqual(parent.name, "parent")
assert.include(span.attributes.get("code.stacktrace"), "Tracer.test.ts:176")
assert.include(parent.attributes.get("code.stacktrace"), "Tracer.test.ts:184")
assert.strictEqual(span.attributes.get("code.stacktrace"), undefined)
assert.strictEqual(parent.attributes.get("code.stacktrace"), undefined)
}).pipe(
Effect.provide(Layer.unwrapScoped(
Effect.map(
Expand All @@ -195,7 +195,7 @@ describe("Tracer", () => {
Option.getOrThrow
)
assert.strictEqual(parent.name, "parent")
assert.include(parent.attributes.get("code.stacktrace"), "Tracer.test.ts:200")
assert.strictEqual(parent.attributes.get("code.stacktrace"), undefined)
}).pipe(
Effect.provide(Layer.span("parent"))
))
Expand Down Expand Up @@ -238,7 +238,7 @@ describe("Tracer", () => {
const layer = Layer.effectDiscard(Effect.gen(function*() {
const span = yield* Effect.currentSpan
assert.strictEqual(span.name, "span")
assert.include(span.attributes.get("code.stacktrace"), "Tracer.test.ts:243")
assert.strictEqual(span.attributes.get("code.stacktrace"), undefined)
})).pipe(
Layer.withSpan("span", {
onEnd: (span, _exit) =>
Expand Down Expand Up @@ -274,6 +274,25 @@ it.effect("withTracerEnabled", () =>
assert.deepEqual(spanB.name, "B")
}))

it.effect("includes trace when errored", () =>
Effect.gen(function*() {
let maybeSpan: undefined | Span
const getSpan = Effect.functionWithSpan({
body: (_id: string) =>
Effect.currentSpan.pipe(Effect.flatMap((span) => {
maybeSpan = span
return Effect.fail("error")
})),
options: (id) => ({
name: `span-${id}`,
attributes: { id }
})
})
yield* Effect.flip(getSpan("fail"))
assert.isDefined(maybeSpan)
assert.include(maybeSpan!.attributes.get("code.stacktrace"), "Tracer.test.ts:291:24")
}))

describe("functionWithSpan", () => {
const getSpan = Effect.functionWithSpan({
body: (_id: string) => Effect.currentSpan,
Expand All @@ -288,7 +307,7 @@ describe("functionWithSpan", () => {
const span = yield* getSpan("A")
assert.deepEqual(span.name, "span-A")
assert.deepEqual(span.parent, Option.none())
assert.include(span.attributes.get("code.stacktrace"), "Tracer.test.ts:288")
assert.strictEqual(span.attributes.get("code.stacktrace"), undefined)
}))

it.effect("parent", () =>
Expand Down

0 comments on commit 6a6f670

Please sign in to comment.