| author | |
| committer | |
| log | 294dd123410c77eead2810a052c31fb0cfd3e0c1 |
| tree | 6ac2f8127870256cc8a2df2836c457885c55416e |
| parent | 6f97b0f66a51fbafabd54a5616da8827ebdf6020 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>5 files changed, 124 insertions(+), 9 deletions(-)
apps/server/src/http.ts+22-8| ... | @@ -311,9 +311,12 @@ export const serverEnvironmentHttpApiLayer = HttpApiBuilder.group( | ... | @@ -311,9 +311,12 @@ export const serverEnvironmentHttpApiLayer = HttpApiBuilder.group( |
| 311 | 311 | ||
| 312 | class DecodeOtlpTraceRecordsError extends Data.TaggedError("DecodeOtlpTraceRecordsError")<{ | 312 | class DecodeOtlpTraceRecordsError extends Data.TaggedError("DecodeOtlpTraceRecordsError")<{ |
| 313 | readonly cause: unknown; | 313 | readonly cause: unknown; |
| 314 | readonly bodyJson: OtlpTracer.TraceData; | ||
| 315 | }> {} | 314 | }> {} |
| 316 | 315 | ||
| 316 | // Renderers export up to once a second while they have spans buffered, so | ||
| 317 | // tracing this proxy would add more server spans than it forwards. | ||
| 318 | // withTracerEnabled(false) drops the handler's spans, including the forward. | ||
| 319 | // untracedRequestsLayer drops the HTTP server span. | ||
| 317 | export const otlpTracesProxyRouteLayer = HttpRouter.add( | 320 | export const otlpTracesProxyRouteLayer = HttpRouter.add( |
| 318 | "POST", | 321 | "POST", |
| 319 | OTLP_TRACES_PROXY_PATH, | 322 | OTLP_TRACES_PROXY_PATH, |
| ... | @@ -330,15 +333,10 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( | ... | @@ -330,15 +333,10 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( |
| 330 | 333 | ||
| 331 | yield* Effect.try({ | 334 | yield* Effect.try({ |
| 332 | try: () => decodeOtlpTraceRecords(bodyJson), | 335 | try: () => decodeOtlpTraceRecords(bodyJson), |
| 333 | catch: (cause) => new DecodeOtlpTraceRecordsError({ cause, bodyJson }), | 336 | catch: (cause) => new DecodeOtlpTraceRecordsError({ cause }), |
| 334 | }).pipe( | 337 | }).pipe( |
| 335 | Effect.flatMap((records) => browserTraceCollector.record(records)), | 338 | Effect.flatMap((records) => browserTraceCollector.record(records)), |
| 336 | Effect.catch((cause) => | 339 | Effect.catch((cause) => Effect.logWarning("Failed to decode browser OTLP traces", { cause })), |
| 337 | Effect.logWarning("Failed to decode browser OTLP traces", { | ||
| 338 | cause, | ||
| 339 | bodyJson, | ||
| 340 | }), | ||
| 341 | ), | ||
| 342 | ); | 340 | ); |
| 343 | 341 | ||
| 344 | if (otlpTracesUrl === undefined) { | 342 | if (otlpTracesUrl === undefined) { |
| ... | @@ -369,9 +367,25 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( | ... | @@ -369,9 +367,25 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( |
| 369 | EnvironmentInternalError: HttpServerRespondable.toResponse, | 367 | EnvironmentInternalError: HttpServerRespondable.toResponse, |
| 370 | EnvironmentScopeRequiredError: HttpServerRespondable.toResponse, | 368 | EnvironmentScopeRequiredError: HttpServerRespondable.toResponse, |
| 371 | }), | 369 | }), |
| 370 | Effect.withTracerEnabled(false), | ||
| 372 | ), | 371 | ), |
| 373 | ); | 372 | ); |
| 374 | 373 | ||
| 374 | const UNTRACED_REQUEST_PATHS: ReadonlySet<string> = new Set([OTLP_TRACES_PROXY_PATH]); | ||
| 375 | |||
| 376 | // Skips the HTTP server span for UNTRACED_REQUEST_PATHS. That span starts | ||
| 377 | // before routing, so a route handler cannot skip it. TracerDisabledWhen is one | ||
| 378 | // predicate for the whole server and the last layer to provide it wins, so | ||
| 379 | // makeRoutesLayer provides this one last. Add paths here instead of providing | ||
| 380 | // TracerDisabledWhen again; server.test.ts fails if a later layer replaces it. | ||
| 381 | // The query string is ignored, as in routing. | ||
| 382 | export const untracedRequestsLayer = Layer.succeed(HttpMiddleware.TracerDisabledWhen)((request) => { | ||
| 383 | const queryIndex = request.url.indexOf("?"); | ||
| 384 | return UNTRACED_REQUEST_PATHS.has( | ||
| 385 | queryIndex === -1 ? request.url : request.url.slice(0, queryIndex), | ||
| 386 | ); | ||
| 387 | }); | ||
| 388 | |||
| 375 | export const assetRouteLayer = HttpRouter.add( | 389 | export const assetRouteLayer = HttpRouter.add( |
| 376 | "GET", | 390 | "GET", |
| 377 | `${ASSET_ROUTE_PREFIX}/*`, | 391 | `${ASSET_ROUTE_PREFIX}/*`, |
apps/server/src/server.test.ts+50| ... | @@ -5772,6 +5772,56 @@ it.layer(NodeServices.layer)("server router seam", (it) => { | ... | @@ -5772,6 +5772,56 @@ it.layer(NodeServices.layer)("server router seam", (it) => { |
| 5772 | }).pipe(Effect.provide(NodeHttpServer.layerTest)), | 5772 | }).pipe(Effect.provide(NodeHttpServer.layerTest)), |
| 5773 | ); | 5773 | ); |
| 5774 | 5774 | ||
| 5775 | it.effect("does not trace browser OTLP trace exports on the server", () => | ||
| 5776 | Effect.gen(function* () { | ||
| 5777 | const spanNames: Array<string> = []; | ||
| 5778 | const forwardedUrls: Array<string> = []; | ||
| 5779 | yield* buildAppUnderTest({ | ||
| 5780 | config: { otlpTracesUrl: "http://collector.test/v1/traces" }, | ||
| 5781 | layers: { | ||
| 5782 | httpClient: HttpClient.make((request) => | ||
| 5783 | Effect.sync(() => { | ||
| 5784 | forwardedUrls.push(request.url); | ||
| 5785 | return HttpClientResponse.fromWeb(request, new Response(null, { status: 204 })); | ||
| 5786 | }), | ||
| 5787 | ), | ||
| 5788 | }, | ||
| 5789 | }).pipe( | ||
| 5790 | Effect.provideService( | ||
| 5791 | Tracer.Tracer, | ||
| 5792 | Tracer.make({ | ||
| 5793 | span: (options) => { | ||
| 5794 | spanNames.push(options.name); | ||
| 5795 | return new Tracer.NativeSpan(options); | ||
| 5796 | }, | ||
| 5797 | }), | ||
| 5798 | ), | ||
| 5799 | ); | ||
| 5800 | const cookie = yield* getAuthenticatedSessionCookieHeader(); | ||
| 5801 | spanNames.length = 0; | ||
| 5802 | |||
| 5803 | // The query string must not bring back the HTTP server span. | ||
| 5804 | for (const url of ["/api/observability/v1/traces", "/api/observability/v1/traces?x=1"]) { | ||
| 5805 | const response = yield* HttpClient.post(url, { | ||
| 5806 | headers: { cookie, "content-type": "application/json" }, | ||
| 5807 | body: yield* HttpBody.json({ resourceSpans: [] }), | ||
| 5808 | }); | ||
| 5809 | assert.equal(response.status, 204); | ||
| 5810 | } | ||
| 5811 | |||
| 5812 | assert.deepEqual(forwardedUrls, [ | ||
| 5813 | "http://collector.test/v1/traces", | ||
| 5814 | "http://collector.test/v1/traces", | ||
| 5815 | ]); | ||
| 5816 | assert.deepEqual(spanNames, []); | ||
| 5817 | |||
| 5818 | // Other routes keep their HTTP server span. | ||
| 5819 | const session = yield* HttpClient.get("/api/auth/session", { headers: { cookie } }); | ||
| 5820 | assert.equal(session.status, 200); | ||
| 5821 | assert.include(spanNames, "http.server GET"); | ||
| 5822 | }).pipe(Effect.provide(NodeHttpServer.layerTest)), | ||
| 5823 | ); | ||
| 5824 | |||
| 5775 | it.effect("routes websocket rpc server.upsertKeybinding", () => | 5825 | it.effect("routes websocket rpc server.upsertKeybinding", () => |
| 5776 | Effect.gen(function* () { | 5826 | Effect.gen(function* () { |
| 5777 | const rule: KeybindingRule = { | 5827 | const rule: KeybindingRule = { |
apps/server/src/server.ts+3| ... | @@ -33,6 +33,7 @@ import { | ... | @@ -33,6 +33,7 @@ import { |
| 33 | staticAndDevRouteLayer, | 33 | staticAndDevRouteLayer, |
| 34 | browserApiCorsLayer, | 34 | browserApiCorsLayer, |
| 35 | httpCompressionLayer, | 35 | httpCompressionLayer, |
| 36 | untracedRequestsLayer, | ||
| 36 | } from "./http.ts"; | 37 | } from "./http.ts"; |
| 37 | import { guardHttpResponseWriteErrors } from "./httpResponseErrorGuard.ts"; | 38 | import { guardHttpResponseWriteErrors } from "./httpResponseErrorGuard.ts"; |
| 38 | import { fixPath } from "./os-jank.ts"; | 39 | import { fixPath } from "./os-jank.ts"; |
| ... | @@ -605,6 +606,8 @@ export const makeRoutesLayer = Layer.mergeAll( | ... | @@ -605,6 +606,8 @@ export const makeRoutesLayer = Layer.mergeAll( |
| 605 | websocketRpcRouteLayer, | 606 | websocketRpcRouteLayer, |
| 606 | ), | 607 | ), |
| 607 | McpHttpServer.layer.pipe(Layer.provide(McpSessionRegistry.layer)), | 608 | McpHttpServer.layer.pipe(Layer.provide(McpSessionRegistry.layer)), |
| 609 | // Last, so no route layer can replace the server's one TracerDisabledWhen. | ||
| 610 | untracedRequestsLayer, | ||
| 608 | ).pipe( | 611 | ).pipe( |
| 609 | // Both transports consume the same service instance, so caches single-flight across clients | 612 | // Both transports consume the same service instance, so caches single-flight across clients |
| 610 | // and mutations observed on WebSocket invalidate patches subsequently read over HTTP. | 613 | // and mutations observed on WebSocket invalidate patches subsequently read over HTTP. |
packages/shared/src/observability.test.ts+48| ... | @@ -16,6 +16,7 @@ import * as Tracer from "effect/Tracer"; | ... | @@ -16,6 +16,7 @@ import * as Tracer from "effect/Tracer"; |
| 16 | import { | 16 | import { |
| 17 | causeErrorTag, | 17 | causeErrorTag, |
| 18 | compactTraceAttributes, | 18 | compactTraceAttributes, |
| 19 | decodeOtlpTraceRecords, | ||
| 19 | errorTag, | 20 | errorTag, |
| 20 | makeLocalFileTracer, | 21 | makeLocalFileTracer, |
| 21 | makeTraceSink, | 22 | makeTraceSink, |
| ... | @@ -138,6 +139,53 @@ describe("truncateTraceAttributes", () => { | ... | @@ -138,6 +139,53 @@ describe("truncateTraceAttributes", () => { |
| 138 | }); | 139 | }); |
| 139 | }); | 140 | }); |
| 140 | 141 | ||
| 142 | describe("decodeOtlpTraceRecords", () => { | ||
| 143 | it("clamps oversized renderer span and event attributes", () => { | ||
| 144 | const long = "x".repeat(2_000); | ||
| 145 | const clamped = `${"x".repeat(500)}…[truncated]`; | ||
| 146 | const [record] = decodeOtlpTraceRecords({ | ||
| 147 | resourceSpans: [ | ||
| 148 | { | ||
| 149 | resource: { attributes: [], droppedAttributesCount: 0 }, | ||
| 150 | scopeSpans: [ | ||
| 151 | { | ||
| 152 | scope: { name: "effect" }, | ||
| 153 | spans: [ | ||
| 154 | { | ||
| 155 | traceId: "11111111111111111111111111111111", | ||
| 156 | spanId: "2222222222222222", | ||
| 157 | parentSpanId: undefined, | ||
| 158 | name: "client.span", | ||
| 159 | kind: 1, | ||
| 160 | startTimeUnixNano: "1000000", | ||
| 161 | endTimeUnixNano: "2000000", | ||
| 162 | attributes: [{ key: "payload", value: { stringValue: long } }], | ||
| 163 | droppedAttributesCount: 0, | ||
| 164 | events: [ | ||
| 165 | { | ||
| 166 | name: "log", | ||
| 167 | timeUnixNano: "1500000", | ||
| 168 | attributes: [{ key: "effect.cause", value: { stringValue: long } }], | ||
| 169 | droppedAttributesCount: 0, | ||
| 170 | }, | ||
| 171 | ], | ||
| 172 | droppedEventsCount: 0, | ||
| 173 | status: { code: 1 }, | ||
| 174 | links: [], | ||
| 175 | droppedLinksCount: 0, | ||
| 176 | }, | ||
| 177 | ], | ||
| 178 | }, | ||
| 179 | ], | ||
| 180 | }, | ||
| 181 | ], | ||
| 182 | }); | ||
| 183 | |||
| 184 | assert.equal(record?.attributes["payload"], clamped); | ||
| 185 | assert.equal(record?.events[0]?.attributes["effect.cause"], clamped); | ||
| 186 | }); | ||
| 187 | }); | ||
| 188 | |||
| 141 | describe("observability", () => { | 189 | describe("observability", () => { |
| 142 | it("normalizes circular arrays, maps, and sets without recursing forever", () => { | 190 | it("normalizes circular arrays, maps, and sets without recursing forever", () => { |
| 143 | const array: Array<unknown> = ["alpha"]; | 191 | const array: Array<unknown> = ["alpha"]; |
packages/shared/src/observability.ts+1-1| ... | @@ -668,7 +668,7 @@ function decodeAttributes( | ... | @@ -668,7 +668,7 @@ function decodeAttributes( |
| 668 | entries[attribute.key] = decodeValue(attribute.value); | 668 | entries[attribute.key] = decodeValue(attribute.value); |
| 669 | } | 669 | } |
| 670 | 670 | ||
| 671 | return compactTraceAttributes(entries); | 671 | return truncateTraceAttributes(compactTraceAttributes(entries)); |
| 672 | } | 672 | } |
| 673 | 673 | ||
| 674 | function decodeValue(input: OtlpResource.AnyValue | null | undefined): unknown { | 674 | function decodeValue(input: OtlpResource.AnyValue | null | undefined): unknown { |