diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index 7533b8c1db19b24a43dc3e8ddef826cab463b4dd..30b368e62167fcd0ac18ad9655497995fc91f528 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -311,9 +311,12 @@ export const serverEnvironmentHttpApiLayer = HttpApiBuilder.group( class DecodeOtlpTraceRecordsError extends Data.TaggedError("DecodeOtlpTraceRecordsError")<{ readonly cause: unknown; - readonly bodyJson: OtlpTracer.TraceData; }> {} +// Renderers export up to once a second while they have spans buffered, so +// tracing this proxy would add more server spans than it forwards. +// withTracerEnabled(false) drops the handler's spans, including the forward. +// untracedRequestsLayer drops the HTTP server span. export const otlpTracesProxyRouteLayer = HttpRouter.add( "POST", OTLP_TRACES_PROXY_PATH, @@ -330,15 +333,10 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( yield* Effect.try({ try: () => decodeOtlpTraceRecords(bodyJson), - catch: (cause) => new DecodeOtlpTraceRecordsError({ cause, bodyJson }), + catch: (cause) => new DecodeOtlpTraceRecordsError({ cause }), }).pipe( Effect.flatMap((records) => browserTraceCollector.record(records)), - Effect.catch((cause) => - Effect.logWarning("Failed to decode browser OTLP traces", { - cause, - bodyJson, - }), - ), + Effect.catch((cause) => Effect.logWarning("Failed to decode browser OTLP traces", { cause })), ); if (otlpTracesUrl === undefined) { @@ -369,9 +367,25 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( EnvironmentInternalError: HttpServerRespondable.toResponse, EnvironmentScopeRequiredError: HttpServerRespondable.toResponse, }), + Effect.withTracerEnabled(false), ), ); +const UNTRACED_REQUEST_PATHS: ReadonlySet = new Set([OTLP_TRACES_PROXY_PATH]); + +// Skips the HTTP server span for UNTRACED_REQUEST_PATHS. That span starts +// before routing, so a route handler cannot skip it. TracerDisabledWhen is one +// predicate for the whole server and the last layer to provide it wins, so +// makeRoutesLayer provides this one last. Add paths here instead of providing +// TracerDisabledWhen again; server.test.ts fails if a later layer replaces it. +// The query string is ignored, as in routing. +export const untracedRequestsLayer = Layer.succeed(HttpMiddleware.TracerDisabledWhen)((request) => { + const queryIndex = request.url.indexOf("?"); + return UNTRACED_REQUEST_PATHS.has( + queryIndex === -1 ? request.url : request.url.slice(0, queryIndex), + ); +}); + export const assetRouteLayer = HttpRouter.add( "GET", `${ASSET_ROUTE_PREFIX}/*`, diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 5dc793b5cdf1ed54c79599ba9982404b0d9a3c52..192cdd3191f0efc4118017abdf5338bdecfa3719 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -5772,6 +5772,56 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); + it.effect("does not trace browser OTLP trace exports on the server", () => + Effect.gen(function* () { + const spanNames: Array = []; + const forwardedUrls: Array = []; + yield* buildAppUnderTest({ + config: { otlpTracesUrl: "http://collector.test/v1/traces" }, + layers: { + httpClient: HttpClient.make((request) => + Effect.sync(() => { + forwardedUrls.push(request.url); + return HttpClientResponse.fromWeb(request, new Response(null, { status: 204 })); + }), + ), + }, + }).pipe( + Effect.provideService( + Tracer.Tracer, + Tracer.make({ + span: (options) => { + spanNames.push(options.name); + return new Tracer.NativeSpan(options); + }, + }), + ), + ); + const cookie = yield* getAuthenticatedSessionCookieHeader(); + spanNames.length = 0; + + // The query string must not bring back the HTTP server span. + for (const url of ["/api/observability/v1/traces", "/api/observability/v1/traces?x=1"]) { + const response = yield* HttpClient.post(url, { + headers: { cookie, "content-type": "application/json" }, + body: yield* HttpBody.json({ resourceSpans: [] }), + }); + assert.equal(response.status, 204); + } + + assert.deepEqual(forwardedUrls, [ + "http://collector.test/v1/traces", + "http://collector.test/v1/traces", + ]); + assert.deepEqual(spanNames, []); + + // Other routes keep their HTTP server span. + const session = yield* HttpClient.get("/api/auth/session", { headers: { cookie } }); + assert.equal(session.status, 200); + assert.include(spanNames, "http.server GET"); + }).pipe(Effect.provide(NodeHttpServer.layerTest)), + ); + it.effect("routes websocket rpc server.upsertKeybinding", () => Effect.gen(function* () { const rule: KeybindingRule = { diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index b7020458160de64a1ac14c1451656b50436c11a4..88c08fd3bdee9e98bcaba572a5547b5ec68df69a 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -33,6 +33,7 @@ import { staticAndDevRouteLayer, browserApiCorsLayer, httpCompressionLayer, + untracedRequestsLayer, } from "./http.ts"; import { guardHttpResponseWriteErrors } from "./httpResponseErrorGuard.ts"; import { fixPath } from "./os-jank.ts"; @@ -605,6 +606,8 @@ export const makeRoutesLayer = Layer.mergeAll( websocketRpcRouteLayer, ), McpHttpServer.layer.pipe(Layer.provide(McpSessionRegistry.layer)), + // Last, so no route layer can replace the server's one TracerDisabledWhen. + untracedRequestsLayer, ).pipe( // Both transports consume the same service instance, so caches single-flight across clients // and mutations observed on WebSocket invalidate patches subsequently read over HTTP. diff --git a/packages/shared/src/observability.test.ts b/packages/shared/src/observability.test.ts index 081291f34b86377754a2d9659a2580c7a3b08a33..b9bf4ef1375da4ab9640a7c55ab6a2ed406367f5 100644 --- a/packages/shared/src/observability.test.ts +++ b/packages/shared/src/observability.test.ts @@ -16,6 +16,7 @@ import * as Tracer from "effect/Tracer"; import { causeErrorTag, compactTraceAttributes, + decodeOtlpTraceRecords, errorTag, makeLocalFileTracer, makeTraceSink, @@ -138,6 +139,53 @@ describe("truncateTraceAttributes", () => { }); }); +describe("decodeOtlpTraceRecords", () => { + it("clamps oversized renderer span and event attributes", () => { + const long = "x".repeat(2_000); + const clamped = `${"x".repeat(500)}…[truncated]`; + const [record] = decodeOtlpTraceRecords({ + resourceSpans: [ + { + resource: { attributes: [], droppedAttributesCount: 0 }, + scopeSpans: [ + { + scope: { name: "effect" }, + spans: [ + { + traceId: "11111111111111111111111111111111", + spanId: "2222222222222222", + parentSpanId: undefined, + name: "client.span", + kind: 1, + startTimeUnixNano: "1000000", + endTimeUnixNano: "2000000", + attributes: [{ key: "payload", value: { stringValue: long } }], + droppedAttributesCount: 0, + events: [ + { + name: "log", + timeUnixNano: "1500000", + attributes: [{ key: "effect.cause", value: { stringValue: long } }], + droppedAttributesCount: 0, + }, + ], + droppedEventsCount: 0, + status: { code: 1 }, + links: [], + droppedLinksCount: 0, + }, + ], + }, + ], + }, + ], + }); + + assert.equal(record?.attributes["payload"], clamped); + assert.equal(record?.events[0]?.attributes["effect.cause"], clamped); + }); +}); + describe("observability", () => { it("normalizes circular arrays, maps, and sets without recursing forever", () => { const array: Array = ["alpha"]; diff --git a/packages/shared/src/observability.ts b/packages/shared/src/observability.ts index 6393ed0879db862133a04f87418763d180132982..b4dbdf88651a7ec1e6c9c2fc6718a5e1f8b69ece 100644 --- a/packages/shared/src/observability.ts +++ b/packages/shared/src/observability.ts @@ -668,7 +668,7 @@ function decodeAttributes( entries[attribute.key] = decodeValue(attribute.value); } - return compactTraceAttributes(entries); + return truncateTraceAttributes(compactTraceAttributes(entries)); } function decodeValue(input: OtlpResource.AnyValue | null | undefined): unknown {