From 51569b4326802018342e95eaff6b29dbd3a67851 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 25 Sep 2026 22:11:32 -0700 Subject: [PATCH] perf(observability): stop writing empty spans on spawns, projected events, and idle polls Five leaf spans carried no information but were 26-34% of all spans in a steady-state bench. The idle port poll added one more every 3 s. - createTrace2Monitor runs on every git spawn and returns at once without hook callbacks. It is now untraced. Its errors still fail the runGitCommand span. - shell.resolveSpawnCommand and the two processRunner.collectText spans run on every processRunner spawn (lsof, gh, VCS, providers). resolveSpawnCommand returns at once off Windows, and collectText has no attributes. They are now untraced. Their errors still fail the runProcessCore span. - applyAttachmentSideEffects wrote a span for every projected event and then returned early. projectEventDeferred now skips the call when there is no attachment cleanup. - PortScanner now checks retainCount before the pollTick span starts, so an idle server no longer writes a span every 3 s. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Layers/ProjectionPipeline.test.ts | 61 +++++++++++++++++++ .../Layers/ProjectionPipeline.ts | 15 +++-- apps/server/src/preview/PortScanner.test.ts | 21 +++++++ apps/server/src/preview/PortScanner.ts | 10 +-- apps/server/src/processRunner.ts | 3 +- apps/server/src/vcs/GitVcsDriverCore.ts | 4 +- packages/shared/src/shell.ts | 3 +- 7 files changed, 102 insertions(+), 15 deletions(-) diff --git a/apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts b/apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts index 179d04843c7e..e8c7154f38e7 100644 --- a/apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts +++ b/apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts @@ -21,6 +21,7 @@ import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; +import * as Tracer from "effect/Tracer"; import * as SqlClient from "effect/unstable/sql/SqlClient"; import { makeSqlStatementCounter } from "../../../integration/SqlStatementCounter.integration.ts"; @@ -114,6 +115,66 @@ it.layer(Layer.fresh(makeProjectionPipelinePrefixedTestLayer("t3-projection-curs }, ); +it.layer(Layer.fresh(makeProjectionPipelinePrefixedTestLayer("t3-projection-cleanup-span-")))( + "OrchestrationProjectionPipeline attachment cleanup span", + (it) => { + it.effect("runs attachment cleanup only for events that remove attachments", () => + Effect.gen(function* () { + const projectionPipeline = yield* OrchestrationProjectionPipeline; + const eventStore = yield* OrchestrationEventStore; + let cleanupSpans = 0; + const tracer = Tracer.make({ + span: (options) => { + if (options.name === "applyAttachmentSideEffects") cleanupSpans += 1; + return new Tracer.NativeSpan(options); + }, + }); + const now = "2026-01-01T00:00:00.000Z"; + const projectId = ProjectId.make("project-cleanup-span"); + const threadId = ThreadId.make("thread-cleanup-span"); + + const projectCreated = yield* eventStore.append({ + type: "project.created", + eventId: EventId.make("evt-cleanup-span-project"), + aggregateKind: "project", + aggregateId: projectId, + occurredAt: now, + commandId: CommandId.make("cmd-cleanup-span-project"), + causationEventId: null, + correlationId: null, + metadata: {}, + payload: { + projectId, + title: "Cleanup span project", + workspaceRoot: "/tmp/project-cleanup-span", + defaultModelSelection: null, + scripts: [], + createdAt: now, + updatedAt: now, + }, + }); + yield* projectionPipeline.projectEvent(projectCreated).pipe(Effect.withTracer(tracer)); + assert.strictEqual(cleanupSpans, 0); + + const threadDeleted = yield* eventStore.append({ + type: "thread.deleted", + eventId: EventId.make("evt-cleanup-span-thread-delete"), + aggregateKind: "thread", + aggregateId: threadId, + occurredAt: now, + commandId: CommandId.make("cmd-cleanup-span-thread-delete"), + causationEventId: null, + correlationId: null, + metadata: {}, + payload: { threadId, deletedAt: now }, + }); + yield* projectionPipeline.projectEvent(threadDeleted).pipe(Effect.withTracer(tracer)); + assert.strictEqual(cleanupSpans, 1); + }), + ); + }, +); + it.layer(Layer.fresh(makeProjectionPipelinePrefixedTestLayer("t3-import-shell-")))( "imported thread shell projection", (it) => { diff --git a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts index 4163168157e7..1acb3c360b15 100644 --- a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts +++ b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts @@ -1991,13 +1991,6 @@ const makeOrchestrationProjectionPipeline = Effect.fn("makeOrchestrationProjecti const applyAttachmentSideEffects = Effect.fn("applyAttachmentSideEffects")( function* (event: OrchestrationEvent, sideEffects: AttachmentSideEffects) { - if ( - sideEffects.deletedThreadIds.size === 0 && - sideEffects.prunedThreadRelativePaths.size === 0 - ) { - return; - } - const deletedThreadIds = new Set(); for (const threadId of sideEffects.deletedThreadIds) { const recreatedLater = yield* eventStore.hasEventAfter({ @@ -2113,9 +2106,15 @@ const makeOrchestrationProjectionPipeline = Effect.fn("makeOrchestrationProjecti ); }), ); + const hasCleanup = + attachmentSideEffects.deletedThreadIds.size > 0 || + attachmentSideEffects.prunedThreadRelativePaths.size > 0; // Return the cleanup effect so the caller runs it after the outer transaction commits. + // Most events have no cleanup, so they skip the call and write no cleanup span. // @effect-diagnostics-next-line returnEffectInGen:off - return applyAttachmentSideEffects(event, attachmentSideEffects).pipe(Effect.asVoid); + return hasCleanup + ? applyAttachmentSideEffects(event, attachmentSideEffects).pipe(Effect.asVoid) + : Effect.void; }, Effect.provideService(FileSystem.FileSystem, fileSystem), Effect.provideService(Path.Path, path), diff --git a/apps/server/src/preview/PortScanner.test.ts b/apps/server/src/preview/PortScanner.test.ts index 7fa15defeca9..d790002f95f1 100644 --- a/apps/server/src/preview/PortScanner.test.ts +++ b/apps/server/src/preview/PortScanner.test.ts @@ -18,6 +18,7 @@ import * as Layer from "effect/Layer"; import * as PlatformError from "effect/PlatformError"; import * as Scope from "effect/Scope"; import * as TestClock from "effect/testing/TestClock"; +import * as Tracer from "effect/Tracer"; import { expect } from "vite-plus/test"; import { FetchHttpClient } from "effect/unstable/http"; @@ -441,6 +442,26 @@ effectIt.effect("stops probing a subscriber's configured paths after its scope c }).pipe(Effect.scoped, Effect.provide(layer)); }); +effectIt.effect("writes no poll span while no client retains the scanner", () => { + let pollSpans = 0; + const tracer = Tracer.make({ + span: (options) => { + if (options.name === "PortDiscovery.pollTick") pollSpans += 1; + return new Tracer.NativeSpan(options); + }, + }); + const layer = makeProbeFailureLayer(processProbeFailure); + + return Effect.gen(function* () { + const scanner = yield* PortScanner.PortDiscovery; + yield* TestClock.adjust(Duration.seconds(15)); + expect(pollSpans).toBe(0); + + yield* scanner.retain; + expect(pollSpans).toBe(1); + }).pipe(Effect.scoped, Effect.provide(layer), Effect.withTracer(tracer)); +}); + effectIt.effect("uses the current configured fragment when readiness comes from cache", () => { const requests: string[] = []; const fetchFn = ((input: Parameters[0]) => { diff --git a/apps/server/src/preview/PortScanner.ts b/apps/server/src/preview/PortScanner.ts index f4d73d62320d..9eee1a3e215e 100644 --- a/apps/server/src/preview/PortScanner.ts +++ b/apps/server/src/preview/PortScanner.ts @@ -550,7 +550,6 @@ export const make = Effect.gen(function* PortDiscoveryMake() { const pollTick = Effect.fn("PortDiscovery.pollTick")( function* () { - if ((yield* Ref.get(stateRef)).retainCount <= 0) return; const configuredUrls = [ ...new Set( [...(yield* Ref.get(stateRef)).listeners.values()].flatMap( @@ -579,9 +578,12 @@ export const make = Effect.gen(function* PortDiscoveryMake() { ), ); - // Single layer-scoped polling fiber. Ticks are no-ops when no client is - // currently retained, so the cost is one Ref.get every POLL_INTERVAL. - yield* Effect.forkScoped(pollTick().pipe(Effect.repeat(Schedule.spaced(POLL_INTERVAL)))); + // Single layer-scoped polling fiber. Ticks skip the scan and its span when no + // client is currently retained, so the cost is one Ref.get every POLL_INTERVAL. + const pollIfRetained = Ref.get(stateRef).pipe( + Effect.flatMap((state) => (state.retainCount > 0 ? pollTick() : Effect.void)), + ); + yield* Effect.forkScoped(pollIfRetained.pipe(Effect.repeat(Schedule.spaced(POLL_INTERVAL)))); const acquireRetention = Effect.fn("PortDiscovery.retain")(function* () { const wasIdle = yield* Ref.modify(stateRef, (state) => [ diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index 36bb5b649f06..049125de3fbf 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -171,7 +171,8 @@ export const isWindowsCommandNotFound = Effect.fn("processRunner.isWindowsComman }, ); -const collectText = Effect.fn("processRunner.collectText")(function* (input: { +// Untraced: no attributes, and its time is the runProcessCore span. Errors fail that span. +const collectText = Effect.fnUntraced(function* (input: { readonly command: string; readonly args: ReadonlyArray; readonly cwd?: string | undefined; diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 8ec274a46611..1985a2d31aee 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -549,7 +549,9 @@ function trace2ChildKey(record: Record): string | null { const Trace2Record = Schema.Record(Schema.String, Schema.Unknown); const decodeTrace2Record = decodeJsonResult(Trace2Record); -const createTrace2Monitor = Effect.fn("createTrace2Monitor")(function* ( +// Untraced because it runs on every git spawn and returns at once without hook +// callbacks. Its errors fail the runGitCommand span. +const createTrace2Monitor = Effect.fnUntraced(function* ( input: Pick, progress: GitVcsDriver.ExecuteGitProgress | undefined, ): Effect.fn.Return< diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 0a25785d916d..6cb08d2890be 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -681,7 +681,8 @@ export const resolveCommandPath = Effect.fn("shell.resolveCommandPath")(function }); }); -export const resolveSpawnCommand = Effect.fn("shell.resolveSpawnCommand")(function* ( +// Untraced because it runs before most spawns and returns at once off Windows. +export const resolveSpawnCommand = Effect.fnUntraced(function* ( command: string, args: ReadonlyArray, options: CommandAvailabilityOptions = {},