From 8c0bfbccaabc0ac755be46a3e4f6318b8045f70d Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 20 Aug 2026 22:14:55 -0400 Subject: [PATCH 01/10] feat(server): honor the standard OpenTelemetry environment variables A machine that already exports OTEL_* for every other service on it had to learn a second set of names before T3 Code would export anything, and headers, resource identity, and wire format had no names at all, so an authenticated or protobuf-only collector could not be reached. The failure is silent: the local trace file still looks healthy while nothing leaves the machine. Signed-off-by: Yordis Prieto --- apps/server/src/bin.test.ts | 2 + apps/server/src/cli/config.test.ts | 66 +++++ apps/server/src/cli/config.ts | 36 ++- apps/server/src/cli/pair.ts | 2 + apps/server/src/config.ts | 10 + .../src/environment/ServerEnvironment.test.ts | 2 + apps/server/src/http.ts | 2 + .../src/observability/Layers/Observability.ts | 58 ++-- .../src/observability/OtelEnvironment.test.ts | 237 +++++++++++++++++ .../src/observability/OtelEnvironment.ts | 247 ++++++++++++++++++ apps/server/src/server.test.ts | 2 + ...the-standard-otel-variables-are-honored.md | 55 ++++ docs/fork/README.md | 2 + docs/operations/observability.md | 56 ++++ 14 files changed, 748 insertions(+), 29 deletions(-) create mode 100644 apps/server/src/observability/OtelEnvironment.test.ts create mode 100644 apps/server/src/observability/OtelEnvironment.ts create mode 100644 docs/fork/0018-the-standard-otel-variables-are-honored.md diff --git a/apps/server/src/bin.test.ts b/apps/server/src/bin.test.ts index fcb662b9b780..8b8555a40574 100644 --- a/apps/server/src/bin.test.ts +++ b/apps/server/src/bin.test.ts @@ -41,6 +41,7 @@ import * as WorkspacePaths from "./workspace/WorkspacePaths.ts"; import * as ServerSecretStore from "./auth/ServerSecretStore.ts"; import * as EnvironmentAuth from "./auth/EnvironmentAuth.ts"; import { environmentAuthenticatedAuthLayer } from "./auth/http.ts"; +import * as OtelEnvironment from "./observability/OtelEnvironment.ts"; const CliRuntimeLayer = Layer.mergeAll(NodeServices.layer, NetService.layer); class ProjectCliHttpApi extends HttpApi.make("environment").add(EnvironmentOrchestrationHttpApi) {} @@ -76,6 +77,7 @@ const makeCliTestServerConfig = (baseDir: string) => otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, otlpServiceName: "t3-server", + otelEnvironment: OtelEnvironment.none, mode: "web", port: 0, host: "127.0.0.1", diff --git a/apps/server/src/cli/config.test.ts b/apps/server/src/cli/config.test.ts index def63b61fafe..fcb747c7143b 100644 --- a/apps/server/src/cli/config.test.ts +++ b/apps/server/src/cli/config.test.ts @@ -18,6 +18,7 @@ import { import * as NetService from "@t3tools/shared/Net"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { deriveServerPaths } from "../config.ts"; +import * as OtelEnvironment from "../observability/OtelEnvironment.ts"; import { resolveServerConfig } from "./config.ts"; const deriveExplicitServerPaths = (baseDir: string, devUrl: URL | undefined) => @@ -50,6 +51,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, otlpServiceName: "t3-server", + otelEnvironment: OtelEnvironment.none, devAllowedOrigins: [], } as const; @@ -488,6 +490,70 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { }), ); + const resolveWithEnv = (env: Record) => + resolveServerConfig( + { + mode: Option.some("web"), + port: Option.some(4888), + host: Option.none(), + baseDir: Option.some("/tmp/t3-otel-home"), + cwd: Option.none(), + devUrl: Option.none(), + noBrowser: Option.none(), + bootstrapFd: Option.none(), + autoBootstrapProjectFromCwd: Option.none(), + logWebSocketEvents: Option.none(), + tailscaleServeEnabled: Option.none(), + tailscaleServePort: Option.none(), + }, + Option.none(), + ).pipe( + Effect.provide( + Layer.mergeAll(ConfigProvider.layer(ConfigProvider.fromEnv({ env })), NetService.layer), + ), + ); + + it.effect("exports to the endpoint the rest of the machine already uses", () => + Effect.gen(function* () { + const resolved = yield* resolveWithEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_SERVICE_NAME: "t3", + }); + + expect(resolved.otlpTracesUrl).toBe("https://collector.example.com/v1/traces"); + expect(resolved.otlpMetricsUrl).toBe("https://collector.example.com/v1/metrics"); + expect(resolved.otlpServiceName).toBe("t3"); + }), + ); + + it.effect("keeps T3 Code's own names as the explicit answer", () => + Effect.gen(function* () { + const resolved = yield* resolveWithEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_SERVICE_NAME: "t3", + T3CODE_OTLP_TRACES_URL: "http://localhost:4318/v1/traces", + T3CODE_OTLP_SERVICE_NAME: "t3-local", + }); + + expect(resolved.otlpTracesUrl).toBe("http://localhost:4318/v1/traces"); + expect(resolved.otlpMetricsUrl).toBe("https://collector.example.com/v1/metrics"); + expect(resolved.otlpServiceName).toBe("t3-local"); + }), + ); + + it.effect("exports nothing at all once the SDK is switched off", () => + Effect.gen(function* () { + const resolved = yield* resolveWithEnv({ + OTEL_SDK_DISABLED: "true", + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + T3CODE_OTLP_TRACES_URL: "http://localhost:4318/v1/traces", + }); + + expect(resolved.otlpTracesUrl).toBeUndefined(); + expect(resolved.otlpMetricsUrl).toBeUndefined(); + }), + ); + it.effect("falls back to persisted observability settings when env vars are absent", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/cli/config.ts b/apps/server/src/cli/config.ts index 5b05b773b314..e5efeb2fb06c 100644 --- a/apps/server/src/cli/config.ts +++ b/apps/server/src/cli/config.ts @@ -16,6 +16,7 @@ import { Argument, Flag } from "effect/unstable/cli"; import { readBootstrapEnvelope } from "../bootstrap.ts"; import * as ServerConfig from "../config.ts"; import { expandHomePath, resolveBaseDir } from "../os-jank.ts"; +import * as OtelEnvironment from "../observability/OtelEnvironment.ts"; export const modeFlag = Flag.choice("mode", ServerConfig.RuntimeMode.literals).pipe( Flag.withDescription("Runtime mode. `desktop` keeps loopback defaults unless overridden."), @@ -95,9 +96,13 @@ const EnvServerConfig = Config.all({ Config.map(Option.getOrUndefined), ), otlpExportIntervalMs: Config.int("T3CODE_OTLP_EXPORT_INTERVAL_MS").pipe( - Config.withDefault(10_000), + Config.option, + Config.map(Option.getOrUndefined), + ), + otlpServiceName: Config.string("T3CODE_OTLP_SERVICE_NAME").pipe( + Config.option, + Config.map(Option.getOrUndefined), ), - otlpServiceName: Config.string("T3CODE_OTLP_SERVICE_NAME").pipe(Config.withDefault("t3-server")), mode: Config.schema(ServerConfig.RuntimeMode, "T3CODE_MODE").pipe( Config.option, Config.map(Option.getOrUndefined), @@ -220,6 +225,7 @@ export const resolveServerConfig = ( const path = yield* Path.Path; const fs = yield* FileSystem.FileSystem; const env = yield* EnvServerConfig; + const otelEnvironment = yield* OtelEnvironment.load; const normalizedFlags = { mode: flags.mode ?? Option.none(), port: flags.port ?? Option.none(), @@ -356,16 +362,22 @@ export const resolveServerConfig = ( traceBatchWindowMs: env.traceBatchWindowMs, traceMaxBytes: env.traceMaxBytes, traceMaxFiles: env.traceMaxFiles, - otlpTracesUrl: - env.otlpTracesUrl ?? - bootstrap?.otlpTracesUrl ?? - persistedObservabilitySettings.otlpTracesUrl, - otlpMetricsUrl: - env.otlpMetricsUrl ?? - bootstrap?.otlpMetricsUrl ?? - persistedObservabilitySettings.otlpMetricsUrl, - otlpExportIntervalMs: env.otlpExportIntervalMs, - otlpServiceName: env.otlpServiceName, + otlpTracesUrl: otelEnvironment.disabled + ? undefined + : (env.otlpTracesUrl ?? + bootstrap?.otlpTracesUrl ?? + persistedObservabilitySettings.otlpTracesUrl ?? + otelEnvironment.traces?.url), + otlpMetricsUrl: otelEnvironment.disabled + ? undefined + : (env.otlpMetricsUrl ?? + bootstrap?.otlpMetricsUrl ?? + persistedObservabilitySettings.otlpMetricsUrl ?? + otelEnvironment.metrics?.url), + otlpExportIntervalMs: + env.otlpExportIntervalMs ?? otelEnvironment.traces?.exportIntervalMs ?? 10_000, + otlpServiceName: env.otlpServiceName ?? otelEnvironment.resource.serviceName ?? "t3-server", + otelEnvironment, mode, port, cwd, diff --git a/apps/server/src/cli/pair.ts b/apps/server/src/cli/pair.ts index 38fa3be8bb57..ff1dab16ddaa 100644 --- a/apps/server/src/cli/pair.ts +++ b/apps/server/src/cli/pair.ts @@ -54,6 +54,7 @@ import { resolveHeadlessConnectionString, } from "../startupAccess.ts"; import { baseDirFlag, DurationFromString } from "./config.ts"; +import * as OtelEnvironment from "../observability/OtelEnvironment.ts"; const WELL_KNOWN_ENVIRONMENT_PATH = "/.well-known/t3/environment"; const PAIR_PROBE_TIMEOUT = Duration.millis(2_500); @@ -332,6 +333,7 @@ const makePairServerConfig = Effect.fn(function* (input: { otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, otlpServiceName: "t3-server", + otelEnvironment: OtelEnvironment.none, mode: "web", port: state.port, host: state.host, diff --git a/apps/server/src/config.ts b/apps/server/src/config.ts index e678264dde5f..52ae524d9a74 100644 --- a/apps/server/src/config.ts +++ b/apps/server/src/config.ts @@ -14,6 +14,8 @@ import * as LogLevel from "effect/LogLevel"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; +import * as OtelEnvironment from "./observability/OtelEnvironment.ts"; + export const DEFAULT_PORT = 3773; export const RuntimeMode = Schema.Literals(["web", "desktop"]); @@ -65,6 +67,13 @@ export class ServerConfig extends Context.Service< readonly otlpMetricsUrl: string | undefined; readonly otlpExportIntervalMs: number; readonly otlpServiceName: string; + /** + * What the standard `OTEL_*` variables asked for. The endpoints above are + * already resolved from it; this carries the rest, which T3 Code has no + * names of its own for: headers, wire format, resource attributes, and the + * batching knobs. + */ + readonly otelEnvironment: OtelEnvironment.OtelEnvironment; readonly mode: RuntimeMode; readonly port: number; readonly host: string | undefined; @@ -178,6 +187,7 @@ const makeTest = Effect.fn("ServerConfig.makeTest")(function* ( otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, otlpServiceName: "t3-server", + otelEnvironment: OtelEnvironment.none, cwd, baseDir, ...derivedPaths, diff --git a/apps/server/src/environment/ServerEnvironment.test.ts b/apps/server/src/environment/ServerEnvironment.test.ts index ee30d987591d..37cadf39dde0 100644 --- a/apps/server/src/environment/ServerEnvironment.test.ts +++ b/apps/server/src/environment/ServerEnvironment.test.ts @@ -15,6 +15,7 @@ import { } from "../cloud/config.ts"; import * as ServerConfig from "../config.ts"; import * as ServerEnvironment from "./ServerEnvironment.ts"; +import * as OtelEnvironment from "../observability/OtelEnvironment.ts"; const isServerEnvironmentIdPersistenceError = Schema.is( ServerEnvironment.ServerEnvironmentIdPersistenceError, @@ -52,6 +53,7 @@ const makeServerConfig = Effect.fn(function* (baseDir: string) { otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, otlpServiceName: "t3-server", + otelEnvironment: OtelEnvironment.none, cwd: process.cwd(), baseDir, mode: "web", diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index 0da55686b92f..290e4fa2bdab 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -147,6 +147,7 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( const request = yield* HttpServerRequest.HttpServerRequest; const config = yield* ServerConfig.ServerConfig; const otlpTracesUrl = config.otlpTracesUrl; + const otlpTracesHeaders = config.otelEnvironment.traces?.headers; const browserTraceCollector = yield* BrowserTraceCollector.BrowserTraceCollector; const httpClient = yield* HttpClient.HttpClient; const bodyJson = cast(yield* request.json); @@ -171,6 +172,7 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( return yield* httpClient .post(otlpTracesUrl, { body: HttpBody.jsonUnsafe(bodyJson), + ...(otlpTracesHeaders === undefined ? {} : { headers: otlpTracesHeaders }), }) .pipe( Effect.flatMap(HttpClientResponse.filterStatusOk), diff --git a/apps/server/src/observability/Layers/Observability.ts b/apps/server/src/observability/Layers/Observability.ts index 8aac0927534b..e196a85b52be 100644 --- a/apps/server/src/observability/Layers/Observability.ts +++ b/apps/server/src/observability/Layers/Observability.ts @@ -14,12 +14,34 @@ import * as ResourceAttribution from "../../resourceTelemetry/ResourceAttributio import { ServerLoggerLive } from "../../serverLogger.ts"; import * as BrowserTraceCollector from "../BrowserTraceCollector.ts"; -const otlpSerializationLayer = OtlpSerialization.layerJson; - export const ObservabilityLive = Layer.unwrap( Effect.gen(function* () { const config = yield* ServerConfig.ServerConfig; const attribution = yield* ResourceAttribution.ResourceAttribution; + const otel = config.otelEnvironment; + + if (otel.declined !== undefined) { + yield* Effect.logWarning(otel.declined); + } + + const configuredThroughOtelEnvironment = + otel.traces !== undefined || otel.metrics !== undefined; + const protocol = + otel.protocol ?? (configuredThroughOtelEnvironment ? "http/protobuf" : "http/json"); + const otlpSerializationLayer = + protocol === "http/protobuf" ? OtlpSerialization.layerProtobuf : OtlpSerialization.layerJson; + + const otlpResource = { + serviceName: config.otlpServiceName, + ...(otel.resource.serviceVersion === undefined + ? {} + : { serviceVersion: otel.resource.serviceVersion }), + attributes: { + ...otel.resource.attributes, + "service.runtime": "t3-server", + "service.mode": config.mode, + }, + }; const traceReferencesLayer = Layer.mergeAll( Layer.succeed(Tracer.MinimumTraceLevel, config.traceMinLevel), @@ -49,13 +71,14 @@ export const ObservabilityLive = Layer.unwrap( : yield* OtlpTracer.make({ url: config.otlpTracesUrl, exportInterval: `${config.otlpExportIntervalMs} millis`, - resource: { - serviceName: config.otlpServiceName, - attributes: { - "service.runtime": "t3-server", - "service.mode": config.mode, - }, - }, + resource: otlpResource, + ...(otel.traces?.headers === undefined ? {} : { headers: otel.traces.headers }), + ...(otel.traces?.maxBatchSize === undefined + ? {} + : { maxBatchSize: otel.traces.maxBatchSize }), + ...(otel.traces?.shutdownTimeoutMs === undefined + ? {} + : { shutdownTimeout: `${otel.traces.shutdownTimeoutMs} millis` as const }), }); const tracer = yield* makeLocalFileTracer({ @@ -79,14 +102,15 @@ export const ObservabilityLive = Layer.unwrap( ? Layer.empty : OtlpMetrics.layer({ url: config.otlpMetricsUrl, - exportInterval: `${config.otlpExportIntervalMs} millis`, - resource: { - serviceName: config.otlpServiceName, - attributes: { - "service.runtime": "t3-server", - "service.mode": config.mode, - }, - }, + exportInterval: `${otel.metrics?.exportIntervalMs ?? config.otlpExportIntervalMs} millis`, + resource: otlpResource, + ...(otel.metrics?.headers === undefined ? {} : { headers: otel.metrics.headers }), + ...(otel.metrics?.shutdownTimeoutMs === undefined + ? {} + : { shutdownTimeout: `${otel.metrics.shutdownTimeoutMs} millis` as const }), + ...(otel.metricsTemporality === undefined + ? {} + : { temporality: otel.metricsTemporality }), }).pipe(Layer.provideMerge(otlpSerializationLayer)); return Layer.mergeAll(ServerLoggerLive, traceReferencesLayer, tracerLayer, metricsLayer); diff --git a/apps/server/src/observability/OtelEnvironment.test.ts b/apps/server/src/observability/OtelEnvironment.test.ts new file mode 100644 index 000000000000..7063fa759f09 --- /dev/null +++ b/apps/server/src/observability/OtelEnvironment.test.ts @@ -0,0 +1,237 @@ +import { assert, describe, it } from "@effect/vitest"; +import * as ConfigProvider from "effect/ConfigProvider"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; + +import * as OtelEnvironment from "./OtelEnvironment.ts"; + +const withEnv = (env: Record) => + Effect.provide(Layer.mergeAll(ConfigProvider.layer(ConfigProvider.fromEnv({ env })))); + +describe("OtelEnvironment", () => { + it.effect("stays off when nothing is configured", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe(withEnv({})); + assert.strictEqual(resolved.traces, undefined); + assert.strictEqual(resolved.metrics, undefined); + assert.strictEqual(resolved.disabled, false); + }), + ); + + it.effect("appends the signal path to the generic endpoint", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com" }), + ); + assert.strictEqual(resolved.traces?.url, "https://collector.example.com/v1/traces"); + assert.strictEqual(resolved.metrics?.url, "https://collector.example.com/v1/metrics"); + }), + ); + + it.effect("does not double the slash on a generic endpoint that has one", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com/" }), + ); + assert.strictEqual(resolved.traces?.url, "https://collector.example.com/v1/traces"); + }), + ); + + it.effect("takes a signal endpoint exactly as written", () => + Effect.gen(function* () { + // The per-signal variable is a whole URL. Appending to it would send + // traces to a path the collector does not serve. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://generic.example.com", + OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: "https://traces.example.com/ingest", + }), + ); + assert.strictEqual(resolved.traces?.url, "https://traces.example.com/ingest"); + assert.strictEqual(resolved.metrics?.url, "https://generic.example.com/v1/metrics"); + }), + ); + + it.effect("exports without OTEL_TRACES_EXPORTER, because otlp is its default", () => + Effect.gen(function* () { + // A machine that sets OTEL_METRICS_EXPORTER and leaves the traces one + // alone still wants traces; the spec default is otlp, not none. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_METRICS_EXPORTER: "otlp", + }), + ); + assert.isDefined(resolved.traces); + }), + ); + + it.effect("honors a signal turned off by name", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_TRACES_EXPORTER: "none", + }), + ); + assert.strictEqual(resolved.traces, undefined); + assert.isDefined(resolved.metrics); + }), + ); + + it.effect("finds otlp in a list of exporters", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_TRACES_EXPORTER: "console, otlp", + }), + ); + assert.isDefined(resolved.traces); + }), + ); + + it.effect("exports nothing when the SDK is disabled", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_SDK_DISABLED: "true", + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + }), + ); + assert.strictEqual(resolved.disabled, true); + assert.strictEqual(resolved.traces, undefined); + assert.strictEqual(resolved.metrics, undefined); + }), + ); + + it.effect("carries the headers a collector needs to accept the request", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_HEADERS: "api-key=abc123,x-tenant=acme", + OTEL_EXPORTER_OTLP_TRACES_HEADERS: "api-key=traces-only", + }), + ); + // The per-signal header set replaces the generic one rather than + // merging with it, which is what the spec says and what a collector + // with two different keys depends on. + assert.deepStrictEqual(resolved.traces?.headers, { "api-key": "traces-only" }); + assert.deepStrictEqual(resolved.metrics?.headers, { + "api-key": "abc123", + "x-tenant": "acme", + }); + }), + ); + + it.effect("reads the service identity and the leftover resource attributes", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_RESOURCE_ATTRIBUTES: "org.name=Example,service.name=from-attributes,deployment=prod", + OTEL_SERVICE_VERSION: "1.2.3", + }), + ); + assert.strictEqual(resolved.resource.serviceName, "from-attributes"); + assert.strictEqual(resolved.resource.serviceVersion, "1.2.3"); + // service.name and service.version become the named fields, so leaving + // them in the attribute bag too would send each one twice. + assert.deepStrictEqual(resolved.resource.attributes, { + "org.name": "Example", + deployment: "prod", + }); + }), + ); + + it.effect("lets OTEL_SERVICE_NAME win over the resource attribute", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_SERVICE_NAME: "explicit", + OTEL_RESOURCE_ATTRIBUTES: "service.name=from-attributes", + }), + ); + assert.strictEqual(resolved.resource.serviceName, "explicit"); + }), + ); + + it.effect("declines grpc instead of posting a body it cannot frame", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_PROTOCOL: "grpc", + }), + ); + assert.strictEqual(resolved.traces, undefined); + assert.strictEqual(resolved.metrics, undefined); + assert.include(resolved.declined ?? "", "grpc"); + }), + ); + + it.effect("leaves the protocol unstated unless something states it", () => + Effect.gen(function* () { + const fallback = yield* OtelEnvironment.load.pipe(withEnv({})); + assert.strictEqual(fallback.protocol, undefined); + const json = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_EXPORTER_OTLP_PROTOCOL: "http/json" }), + ); + assert.strictEqual(json.protocol, "http/json"); + }), + ); + + it.effect("takes the batch and timeout knobs the exporter can act on", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_BSP_SCHEDULE_DELAY: "2500", + OTEL_BSP_MAX_EXPORT_BATCH_SIZE: "128", + OTEL_EXPORTER_OTLP_TIMEOUT: "7000", + OTEL_METRIC_EXPORT_INTERVAL: "15000", + }), + ); + assert.strictEqual(resolved.traces?.exportIntervalMs, 2500); + assert.strictEqual(resolved.traces?.maxBatchSize, 128); + assert.strictEqual(resolved.traces?.shutdownTimeoutMs, 7000); + assert.strictEqual(resolved.metrics?.exportIntervalMs, 15000); + }), + ); + + it.effect("leaves the intervals unset so T3 Code's own defaults still apply", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com" }), + ); + assert.strictEqual(resolved.traces?.exportIntervalMs, undefined); + assert.strictEqual(resolved.metrics?.exportIntervalMs, undefined); + }), + ); + + it.effect("lets the metric signal name its own timeout and temporality", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_METRIC_EXPORT_TIMEOUT: "9000", + OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "delta", + }), + ); + assert.strictEqual(resolved.metrics?.shutdownTimeoutMs, 9000); + assert.strictEqual(resolved.metricsTemporality, "delta"); + }), + ); + + it.effect("ignores a temporality this exporter cannot produce", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "lowmemory", + }), + ); + assert.strictEqual(resolved.metricsTemporality, undefined); + }), + ); +}); diff --git a/apps/server/src/observability/OtelEnvironment.ts b/apps/server/src/observability/OtelEnvironment.ts new file mode 100644 index 000000000000..eb0ae6d098c9 --- /dev/null +++ b/apps/server/src/observability/OtelEnvironment.ts @@ -0,0 +1,247 @@ +/** + * OtelEnvironment: the OpenTelemetry environment variables, read the way the + * specification says to read them. + * + * T3 Code has always had its own `T3CODE_OTLP_*` names, which stay the + * explicit answer when they are set. Everything here is the fallback for the + * far more common case: a machine that already exports `OTEL_*` for every + * other service on it and expects one more process to join in without being + * told twice. + * + * Only the variables this server can act on are read. The exporter speaks + * OTLP over HTTP, so `grpc` is declined loudly rather than answered with a + * body the endpoint cannot parse, and the log signal has no exporter here at + * all. + * + * @module observability/OtelEnvironment + */ +import * as Config from "effect/Config"; +import * as Effect from "effect/Effect"; +import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; + +/** The wire formats this server can produce. `grpc` is not one of them. */ +export type OtlpProtocol = "http/json" | "http/protobuf"; + +/** `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE`. */ +export type MetricsTemporality = "cumulative" | "delta"; + +/** Everything one signal's exporter needs, or `undefined` if it is off. */ +export interface OtlpSignalSettings { + readonly url: string; + readonly headers: Readonly> | undefined; + readonly exportIntervalMs: number | undefined; + readonly maxBatchSize: number | undefined; + readonly shutdownTimeoutMs: number | undefined; +} + +export interface OtlpResourceSettings { + readonly serviceName: string | undefined; + readonly serviceVersion: string | undefined; + readonly attributes: Readonly>; +} + +export interface OtelEnvironment { + /** `OTEL_SDK_DISABLED`. When set, nothing is exported by any route. */ + readonly disabled: boolean; + readonly traces: OtlpSignalSettings | undefined; + readonly metrics: OtlpSignalSettings | undefined; + readonly metricsTemporality: MetricsTemporality | undefined; + readonly resource: OtlpResourceSettings; + /** + * The wire format the environment asked for, or `undefined` when it said + * nothing. The spec's default is `http/protobuf`, which applies to an + * environment that configured OTLP through these variables; one that did not + * keeps whatever T3 Code already used. + */ + readonly protocol: OtlpProtocol | undefined; + /** + * Why a configured endpoint is not being used, if it is not. Carried rather + * than logged here so the caller can report it once, at startup, where a + * user is looking. + */ + readonly declined: string | undefined; +} + +const StringRecord = Config.Record(Schema.String, Schema.String); + +const optionalString = (name: string) => + Config.string(name).pipe(Config.option, Config.map(Option.getOrUndefined)); + +const optionalInt = (name: string) => + Config.int(name).pipe(Config.option, Config.map(Option.getOrUndefined)); + +const optionalRecord = (name: string) => + Config.schema(StringRecord, name).pipe(Config.option, Config.map(Option.getOrUndefined)); + +/** + * `OTEL_EXPORTER_OTLP__ENDPOINT` is a full URL and is used as given. + * The generic `OTEL_EXPORTER_OTLP_ENDPOINT` is a base, and the spec has each + * signal append its own path to it. + */ +const signalEndpoint = (signal: "TRACES" | "METRICS") => + Effect.gen(function* () { + const specific = yield* optionalString(`OTEL_EXPORTER_OTLP_${signal}_ENDPOINT`); + if (specific !== undefined) { + return specific; + } + const base = yield* optionalString("OTEL_EXPORTER_OTLP_ENDPOINT"); + if (base === undefined) { + return undefined; + } + const trimmed = base.endsWith("/") ? base.slice(0, -1) : base; + return `${trimmed}/v1/${signal.toLowerCase()}`; + }); + +/** + * `OTEL__EXPORTER` is a list, and `otlp` is its default. A value that + * names other exporters and not `otlp` is a deliberate "not this one". + */ +const signalWantsOtlp = (signal: "TRACES" | "METRICS") => + optionalString(`OTEL_${signal}_EXPORTER`).pipe( + Effect.map((value) => { + if (value === undefined) { + return true; + } + return value + .split(",") + .map((entry) => entry.trim().toLowerCase()) + .includes("otlp"); + }), + ); + +const signalSettings = (signal: "TRACES" | "METRICS") => + Effect.gen(function* () { + const url = yield* signalEndpoint(signal); + if (url === undefined || !(yield* signalWantsOtlp(signal))) { + return undefined; + } + const headers = + (yield* optionalRecord(`OTEL_EXPORTER_OTLP_${signal}_HEADERS`)) ?? + (yield* optionalRecord("OTEL_EXPORTER_OTLP_HEADERS")); + const timeoutMs = + (yield* optionalInt(`OTEL_EXPORTER_OTLP_${signal}_TIMEOUT`)) ?? + (yield* optionalInt("OTEL_EXPORTER_OTLP_TIMEOUT")) ?? + (signal === "METRICS" ? yield* optionalInt("OTEL_METRIC_EXPORT_TIMEOUT") : undefined); + const exportIntervalMs = + signal === "TRACES" + ? yield* optionalInt("OTEL_BSP_SCHEDULE_DELAY") + : yield* optionalInt("OTEL_METRIC_EXPORT_INTERVAL"); + return { + url, + headers, + exportIntervalMs, + maxBatchSize: + signal === "TRACES" ? yield* optionalInt("OTEL_BSP_MAX_EXPORT_BATCH_SIZE") : undefined, + shutdownTimeoutMs: timeoutMs, + } satisfies OtlpSignalSettings; + }); + +interface ProtocolDecision { + readonly protocol: OtlpProtocol | undefined; + readonly declined: string | undefined; +} + +/** + * Left unset when nothing named a protocol, so a machine that never mentioned + * OpenTelemetry keeps the wire format T3 Code has always used. `grpc` is the + * one value that cannot be quietly downgraded: its endpoint has no + * `/v1/traces` path and expects a framing this server does not produce, so + * posting anything there is worse than exporting nothing. + */ +const resolveProtocol = Effect.gen(function* () { + const raw = + (yield* optionalString("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL")) ?? + (yield* optionalString("OTEL_EXPORTER_OTLP_PROTOCOL")); + if (raw === undefined) { + return { protocol: undefined, declined: undefined } satisfies ProtocolDecision; + } + const value = raw.trim().toLowerCase(); + if (value === "http/json" || value === "http/protobuf") { + return { protocol: value, declined: undefined } satisfies ProtocolDecision; + } + return { + protocol: undefined, + declined: `OTEL_EXPORTER_OTLP_PROTOCOL=${value} is not supported; this server exports OTLP over HTTP only`, + } satisfies ProtocolDecision; +}); + +const resolveMetricsTemporality = optionalString( + "OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE", +).pipe( + Effect.map((value): MetricsTemporality | undefined => { + const preference = value?.trim().toLowerCase(); + return preference === "delta" || preference === "cumulative" ? preference : undefined; + }), +); + +const resolveResource = Effect.gen(function* () { + const attributes = (yield* optionalRecord("OTEL_RESOURCE_ATTRIBUTES")) ?? {}; + const { + "service.name": attributeName, + "service.version": attributeVersion, + ...rest + } = attributes; + return { + serviceName: (yield* optionalString("OTEL_SERVICE_NAME")) ?? attributeName, + serviceVersion: (yield* optionalString("OTEL_SERVICE_VERSION")) ?? attributeVersion, + attributes: rest, + } satisfies OtlpResourceSettings; +}); + +/** + * Read the environment. Never fails: a variable this server cannot honor + * leaves the corresponding setting unset and is reported through `declined`, + * because an unparseable telemetry knob is not a reason to refuse to start. + */ +export const load: Effect.Effect = Effect.gen(function* () { + const disabled = yield* Config.boolean("OTEL_SDK_DISABLED").pipe(Config.withDefault(false)); + const { protocol, declined } = yield* resolveProtocol; + const resource = yield* resolveResource; + if (disabled) { + return { + disabled, + traces: undefined, + metrics: undefined, + metricsTemporality: undefined, + resource, + protocol, + declined, + }; + } + const unsupportedProtocol = declined !== undefined; + return { + disabled, + traces: unsupportedProtocol ? undefined : yield* signalSettings("TRACES"), + metrics: unsupportedProtocol ? undefined : yield* signalSettings("METRICS"), + metricsTemporality: yield* resolveMetricsTemporality, + resource, + protocol, + declined, + }; +}).pipe( + Effect.catchCause((cause) => + Effect.logWarning("Could not read the OpenTelemetry environment", cause).pipe( + Effect.as({ + disabled: false, + traces: undefined, + metrics: undefined, + metricsTemporality: undefined, + resource: { serviceName: undefined, serviceVersion: undefined, attributes: {} }, + protocol: undefined, + declined: "the OpenTelemetry environment could not be read", + }), + ), + ), +); + +/** An environment that asked for nothing, for tests and for the pairing CLI. */ +export const none: OtelEnvironment = { + disabled: false, + traces: undefined, + metrics: undefined, + metricsTemporality: undefined, + resource: { serviceName: undefined, serviceVersion: undefined, attributes: {} }, + protocol: undefined, + declined: undefined, +}; diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 5a581d4e96a3..e58faa0b1aeb 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -181,6 +181,7 @@ import { type TransferBudgetRun, transferBudgetViolations, } from "../integration/TransferBudgetReport.integration.ts"; +import * as OtelEnvironment from "./observability/OtelEnvironment.ts"; const defaultProjectId = ProjectId.make("project-default"); const defaultThreadId = ThreadId.make("thread-default"); @@ -443,6 +444,7 @@ const buildAppUnderTest = (options?: { otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, otlpServiceName: "t3-server", + otelEnvironment: OtelEnvironment.none, mode: "desktop", port: 0, host: "127.0.0.1", diff --git a/docs/fork/0018-the-standard-otel-variables-are-honored.md b/docs/fork/0018-the-standard-otel-variables-are-honored.md new file mode 100644 index 000000000000..ea44634bdb7d --- /dev/null +++ b/docs/fork/0018-the-standard-otel-variables-are-honored.md @@ -0,0 +1,55 @@ +# 0018: The standard OTEL variables are honored + +- PR: [TrogonStack/t3code#31](https://github.com/TrogonStack/t3code/pull/31) +- Status: active + +## What you can do now + +- Point T3 Code at your collector the same way you point everything else at + it. A machine that already exports `OTEL_EXPORTER_OTLP_ENDPOINT` gets T3 Code + traces and metrics with no extra configuration, at the per-signal paths the + specification defines. +- Send the credentials your collector requires. `OTEL_EXPORTER_OTLP_HEADERS` + reaches the exporter, including the proxy that forwards browser traces, so an + authenticated endpoint stops rejecting the whole stream. +- Get one service identity across your fleet. `OTEL_SERVICE_NAME`, + `OTEL_SERVICE_VERSION`, and `OTEL_RESOURCE_ATTRIBUTES` are attached to every + span and metric, so T3 Code sits in the same dashboards as everything else + rather than under a name only it uses. +- Turn export off from the environment. `OTEL_SDK_DISABLED=true` stops every + export, including one configured in Settings, which is the one switch a + shared machine needs. +- Keep whatever you have. The `T3CODE_OTLP_*` names, the desktop bootstrap + envelope, and Settings all still win over the environment, and a setup that + never mentioned OpenTelemetry keeps the wire format it always used. + +## Why + +T3 Code has had a real OTLP exporter for a while, and it was unreachable for +almost everyone who wanted it. You had to learn a second set of names for +settings you had already configured once, and headers, resource attributes, and +the wire format had no names at all, so an authenticated collector or a +protobuf-only one simply could not be used. + +The cost of that shows up as silence rather than as an error. Someone with a +collector in their shell profile reasonably assumes the app found it, sees a +tidy local trace file, and never learns that nothing left the machine. Reading +the variables everyone else reads turns a feature that existed on paper into one +people can actually reach. + +Auto-enabling from an ambient endpoint is the deliberate part. Every other +OpenTelemetry SDK behaves this way, and a telemetry variable that some processes +honor and others quietly ignore is worse than either answer, so `OTEL_SDK_DISABLED` +is the way out rather than a requirement to opt in. + +## Upstream considerations + +Nothing here is fork-specific and it belongs upstream. The riskiest part for +them is the same part that makes it useful: an ambient endpoint starts an export +that includes thread ids, turn ids, and workspace paths, and upstream may prefer +an explicit opt-in for a product with this many users. + +The rebase burden is small. The reading lives in one module with no dependencies +on the rest of the server, and the wiring is a handful of fallbacks at the end of +existing precedence chains. A sync that rewrites those chains must keep the +environment as their last entry. diff --git a/docs/fork/README.md b/docs/fork/README.md index 898941e21b4f..f9167ec5ca5c 100644 --- a/docs/fork/README.md +++ b/docs/fork/README.md @@ -51,3 +51,5 @@ Each entry uses these sections: active, [#27](https://github.com/TrogonStack/t3code/pull/27) - **0017** [A revoked Claude token reads as revoked](./0017-a-revoked-claude-token-reads-as-revoked.md) active, [#28](https://github.com/TrogonStack/t3code/pull/28) +- **0018** [The standard OTEL variables are honored](./0018-the-standard-otel-variables-are-honored.md) + active, [#31](https://github.com/TrogonStack/t3code/pull/31) diff --git a/docs/operations/observability.md b/docs/operations/observability.md index 7341bfb5edac..86edd2861c64 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -173,6 +173,62 @@ Do not rely on launching from Finder, Spotlight, the dock, or the Start menu aft The backend reads observability config at process start. If you change OTLP env vars, stop the app completely and start it again. +### Option 3: The Standard `OTEL_*` Variables + +If your machine already exports the OpenTelemetry environment variables for everything else running on +it, T3 Code joins in without being told twice. Nothing above is required: + +```bash +export OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 +export OTEL_SERVICE_NAME=t3-local +``` + +The base endpoint is a base, not a full URL: traces go to `/v1/traces` and metrics to +`/v1/metrics`, exactly as the specification says. Set +`OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` or `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` when a signal needs a +full URL of its own. + +Ambient `OTEL_*` variables turn export on by themselves. A work collector in your shell profile means +T3 Code exports to it, so use `OTEL_SDK_DISABLED=true` if that is not what you want. + +#### Precedence + +For each setting, the first one that is present wins: + +1. `T3CODE_OTLP_*` +2. the desktop bootstrap envelope +3. Settings, under `observability` +4. `OTEL_*` + +`OTEL_SDK_DISABLED=true` outranks all four and stops every export, including one configured through +Settings. + +#### What Is Read + +| Variable | Effect | +| --------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------- | +| `OTEL_SDK_DISABLED` | Stops all export | +| `OTEL_EXPORTER_OTLP_ENDPOINT` | Base URL for both signals | +| `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_ENDPOINT` | Full URL for one signal | +| `OTEL_EXPORTER_OTLP_HEADERS`, `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_HEADERS` | Export headers, per signal overriding the shared ones | +| `OTEL_EXPORTER_OTLP_PROTOCOL`, `OTEL_EXPORTER_OTLP_TRACES_PROTOCOL` | `http/protobuf` (default) or `http/json` | +| `OTEL_{TRACES,METRICS}_EXPORTER` | A list; the signal is exported when it contains `otlp`, which is the default | +| `OTEL_SERVICE_NAME`, `OTEL_SERVICE_VERSION`, `OTEL_RESOURCE_ATTRIBUTES` | Resource identity attached to every span and metric | +| `OTEL_EXPORTER_OTLP_TIMEOUT`, `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_TIMEOUT`, `OTEL_METRIC_EXPORT_TIMEOUT` | Shutdown flush timeout | +| `OTEL_BSP_SCHEDULE_DELAY`, `OTEL_METRIC_EXPORT_INTERVAL` | Export interval | +| `OTEL_BSP_MAX_EXPORT_BATCH_SIZE` | Spans per batch | +| `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` | `cumulative` or `delta` | + +The wire format defaults to `http/protobuf` when the endpoint came from `OTEL_*`, matching the +specification, and stays `http/json` for a `T3CODE_OTLP_*` setup that never mentioned a protocol. + +`OTEL_EXPORTER_OTLP_PROTOCOL=grpc` is refused rather than downgraded, because this server has no gRPC +transport and posting an HTTP body to a gRPC endpoint fails in a way that is harder to read than +exporting nothing. The refusal is logged at startup and both signals stay off. + +Anything not listed is ignored, including the log signal, sampler variables, and propagator +variables. + ## How To Use Traces And Metrics To Debug The Server ### Start With The Local Trace File From d48d1c8c60752623d97536a1f9e2302b7a7c4eb2 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 20 Aug 2026 22:25:21 -0400 Subject: [PATCH 02/10] fix(server): a collector credential survives the environment it came from Header and resource attribute values are a W3C Baggage string, and reading them as a plain record broke exactly the values people put there: a base64 basic auth credential lost its padding at the first `=`, a percent encoded bearer token kept its literal `%20`, and a space after a comma became part of the next header name. All three reach the collector as an authentication failure, which reads like a bad credential rather than a parsing bug. Signed-off-by: Yordis Prieto --- .../src/observability/OtelEnvironment.test.ts | 63 +++++++++++++++++++ .../src/observability/OtelEnvironment.ts | 38 +++++++++-- docs/operations/observability.md | 33 +++++++++- 3 files changed, 128 insertions(+), 6 deletions(-) diff --git a/apps/server/src/observability/OtelEnvironment.test.ts b/apps/server/src/observability/OtelEnvironment.test.ts index 7063fa759f09..b824a6702d96 100644 --- a/apps/server/src/observability/OtelEnvironment.test.ts +++ b/apps/server/src/observability/OtelEnvironment.test.ts @@ -223,6 +223,69 @@ describe("OtelEnvironment", () => { }), ); + it.effect("decodes a header the way the specification encodes it", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_HEADERS: "Authorization=Bearer%20abc123, x-scope=team%2Fplatform", + }), + ); + assert.deepStrictEqual(resolved.traces?.headers, { + Authorization: "Bearer abc123", + "x-scope": "team/platform", + }); + }), + ); + + it.effect("keeps a credential that contains its own separator", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_HEADERS: "Authorization=Basic YWJjOmRlZg==", + }), + ); + assert.deepStrictEqual(resolved.traces?.headers, { Authorization: "Basic YWJjOmRlZg==" }); + }), + ); + + it.effect("decodes resource attributes too", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_RESOURCE_ATTRIBUTES: "team=platform%20eng, deployment.environment=prod" }), + ); + assert.deepStrictEqual(resolved.resource.attributes, { + team: "platform eng", + "deployment.environment": "prod", + }); + }), + ); + + it.effect("survives a value that is not valid percent encoding", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_HEADERS: "x-token=100%zz,x-other=100%25", + }), + ); + assert.deepStrictEqual(resolved.traces?.headers, { + "x-token": "100%zz", + "x-other": "100%", + }); + }), + ); + + it.effect("appends the signal path after a base that already has one", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com/otel" }), + ); + assert.strictEqual(resolved.traces?.url, "https://collector.example.com/otel/v1/traces"); + }), + ); + it.effect("ignores a temporality this exporter cannot produce", () => Effect.gen(function* () { const resolved = yield* OtelEnvironment.load.pipe( diff --git a/apps/server/src/observability/OtelEnvironment.ts b/apps/server/src/observability/OtelEnvironment.ts index eb0ae6d098c9..b9ee1b4b8e48 100644 --- a/apps/server/src/observability/OtelEnvironment.ts +++ b/apps/server/src/observability/OtelEnvironment.ts @@ -18,7 +18,6 @@ import * as Config from "effect/Config"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; -import * as Schema from "effect/Schema"; /** The wire formats this server can produce. `grpc` is not one of them. */ export type OtlpProtocol = "http/json" | "http/protobuf"; @@ -63,16 +62,47 @@ export interface OtelEnvironment { readonly declined: string | undefined; } -const StringRecord = Config.Record(Schema.String, Schema.String); - const optionalString = (name: string) => Config.string(name).pipe(Config.option, Config.map(Option.getOrUndefined)); const optionalInt = (name: string) => Config.int(name).pipe(Config.option, Config.map(Option.getOrUndefined)); +/** + * Headers and resource attributes are a W3C Baggage string: comma separated + * pairs, optional whitespace around each one, and percent encoded values. + * + * Splitting on every `=` rather than the first one truncates exactly the + * credentials people put here, since base64 basic auth ends in `=` padding, + * and leaving the encoding in place sends a literal `%20` as part of a bearer + * token. Both fail as an authentication error against the collector, which + * reads like a bad token rather than a parsing bug. + */ +const parseBaggage = (raw: string): Readonly> => { + const entries: Record = {}; + for (const member of raw.split(",")) { + const separator = member.indexOf("="); + if (separator === -1) { + continue; + } + const key = member.slice(0, separator).trim(); + if (key === "") { + continue; + } + const value = member.slice(separator + 1).trim(); + try { + entries[key] = decodeURIComponent(value); + } catch { + entries[key] = value; + } + } + return entries; +}; + const optionalRecord = (name: string) => - Config.schema(StringRecord, name).pipe(Config.option, Config.map(Option.getOrUndefined)); + optionalString(name).pipe( + Effect.map((raw) => (raw === undefined ? undefined : parseBaggage(raw))), + ); /** * `OTEL_EXPORTER_OTLP__ENDPOINT` is a full URL and is used as given. diff --git a/docs/operations/observability.md b/docs/operations/observability.md index 86edd2861c64..8068d09b28ba 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -226,8 +226,37 @@ specification, and stays `http/json` for a `T3CODE_OTLP_*` setup that never ment transport and posting an HTTP body to a gRPC endpoint fails in a way that is harder to read than exporting nothing. The refusal is logged at startup and both signals stay off. -Anything not listed is ignored, including the log signal, sampler variables, and propagator -variables. +Header and resource-attribute values are percent decoded, so +`OTEL_EXPORTER_OTLP_HEADERS=Authorization=Bearer%20abc` sends the space and a base64 credential keeps +its `=` padding. + +#### Known Gaps + +Not everything in the specification is implemented. These are the ones worth knowing about: + +- **No gRPC.** `OTEL_EXPORTER_OTLP_PROTOCOL=grpc` is refused rather than downgraded, because this + server has no gRPC transport and posting an HTTP body to a gRPC endpoint fails in a way that is + harder to read than exporting nothing. The refusal is logged at startup and both signals stay off. +- **No compression and no client TLS.** `OTEL_EXPORTER_OTLP_COMPRESSION`, + `OTEL_EXPORTER_OTLP_CERTIFICATE`, `OTEL_EXPORTER_OTLP_CLIENT_KEY`, and + `OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE` are ignored. A collector that requires mutual TLS needs a + proxy in front of it. +- **Timeouts flush at shutdown.** The specification's `OTEL_EXPORTER_OTLP_TIMEOUT` is a per-request + deadline. The exporter here has no per-request knob, so the value bounds the final flush instead. +- **Interval and batch defaults are T3 Code's, not the specification's.** Leaving + `OTEL_BSP_SCHEDULE_DELAY`, `OTEL_METRIC_EXPORT_INTERVAL`, or `OTEL_BSP_MAX_EXPORT_BATCH_SIZE` unset + keeps this server's own defaults rather than the specification's 5s, 60s, and 512. Set them + explicitly if you need the specification's numbers. +- **Browser traces are always JSON.** The proxy that forwards traces from the client posts + OTLP/HTTP JSON regardless of `OTEL_EXPORTER_OTLP_PROTOCOL`. Both are valid OTLP/HTTP, so a + collector accepts either. +- **`OTEL_SERVICE_VERSION` is not a specification variable.** It is read as a convenience because + the exporter library reads it too. `OTEL_RESOURCE_ATTRIBUTES=service.version=...` is the portable + spelling. + +Everything else not listed above is ignored, including the log signal, `OTEL_BSP_MAX_QUEUE_SIZE`, +`OTEL_BSP_EXPORT_TIMEOUT`, sampler variables, propagator variables, and the attribute and span +limit variables. ## How To Use Traces And Metrics To Debug The Server From 298bcdb3eeb6a971ad08dc747fc6e86ae2cb7641 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 20 Aug 2026 22:37:09 -0400 Subject: [PATCH 03/10] fix(server): an OTEL variable this server cannot use warns instead of guessing The specification requires a warning and a graceful fallback for a value the implementation does not recognize. A typo in the protocol was turning export off entirely, which loses the telemetry the typo was not about. Signed-off-by: Yordis Prieto --- .../src/observability/Layers/Observability.ts | 4 + .../src/observability/OtelEnvironment.test.ts | 89 ++++++- .../src/observability/OtelEnvironment.ts | 218 +++++++++++++----- ...the-standard-otel-variables-are-honored.md | 4 + docs/operations/observability.md | 26 ++- 5 files changed, 268 insertions(+), 73 deletions(-) diff --git a/apps/server/src/observability/Layers/Observability.ts b/apps/server/src/observability/Layers/Observability.ts index e196a85b52be..58e5acb6d2f4 100644 --- a/apps/server/src/observability/Layers/Observability.ts +++ b/apps/server/src/observability/Layers/Observability.ts @@ -20,6 +20,10 @@ export const ObservabilityLive = Layer.unwrap( const attribution = yield* ResourceAttribution.ResourceAttribution; const otel = config.otelEnvironment; + for (const warning of otel.warnings) { + yield* Effect.logWarning(warning); + } + if (otel.declined !== undefined) { yield* Effect.logWarning(otel.declined); } diff --git a/apps/server/src/observability/OtelEnvironment.test.ts b/apps/server/src/observability/OtelEnvironment.test.ts index b824a6702d96..c9587ab80a49 100644 --- a/apps/server/src/observability/OtelEnvironment.test.ts +++ b/apps/server/src/observability/OtelEnvironment.test.ts @@ -199,13 +199,16 @@ describe("OtelEnvironment", () => { }), ); - it.effect("leaves the intervals unset so T3 Code's own defaults still apply", () => + it.effect("falls back to the specification's own batching defaults", () => Effect.gen(function* () { + // Once this route is the one configuring the exporter, the numbers that + // apply are the specification's, not the ones T3 Code picked for itself. const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com" }), ); - assert.strictEqual(resolved.traces?.exportIntervalMs, undefined); - assert.strictEqual(resolved.metrics?.exportIntervalMs, undefined); + assert.strictEqual(resolved.traces?.exportIntervalMs, 5000); + assert.strictEqual(resolved.traces?.maxBatchSize, 512); + assert.strictEqual(resolved.metrics?.exportIntervalMs, 60000); }), ); @@ -262,18 +265,33 @@ describe("OtelEnvironment", () => { }), ); - it.effect("survives a value that is not valid percent encoding", () => + it.effect("discards a pair list that is not valid percent encoding", () => Effect.gen(function* () { + // Half a header set is worse than none: the collector answers a partial + // credential with the same 401 it gives a wrong one, and nothing says + // the variable was the problem. const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", OTEL_EXPORTER_OTLP_HEADERS: "x-token=100%zz,x-other=100%25", }), ); - assert.deepStrictEqual(resolved.traces?.headers, { - "x-token": "100%zz", - "x-other": "100%", - }); + assert.strictEqual(resolved.traces?.headers, undefined); + assert.isTrue( + resolved.warnings.some((warning) => warning.includes("OTEL_EXPORTER_OTLP_HEADERS")), + ); + }), + ); + + it.effect("discards resource attributes that do not decode", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_RESOURCE_ATTRIBUTES: "team=100%zz,deployment=prod" }), + ); + assert.deepStrictEqual(resolved.resource.attributes, {}); + assert.isTrue( + resolved.warnings.some((warning) => warning.includes("OTEL_RESOURCE_ATTRIBUTES")), + ); }), ); @@ -286,7 +304,7 @@ describe("OtelEnvironment", () => { }), ); - it.effect("ignores a temporality this exporter cannot produce", () => + it.effect("warns about a temporality this exporter cannot produce", () => Effect.gen(function* () { const resolved = yield* OtelEnvironment.load.pipe( withEnv({ @@ -295,6 +313,59 @@ describe("OtelEnvironment", () => { }), ); assert.strictEqual(resolved.metricsTemporality, undefined); + assert.isDefined(resolved.metrics); + assert.isTrue(resolved.warnings.some((warning) => warning.includes("lowmemory"))); + }), + ); + + it.effect("warns about a misspelled protocol and keeps exporting", () => + Effect.gen(function* () { + // The specification is explicit here: a value the implementation does + // not recognize gets a warning and is ignored. Switching export off over + // a typo loses the telemetry the typo was not about. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_PROTOCOL: "htp/json", + }), + ); + assert.isDefined(resolved.traces); + assert.strictEqual(resolved.protocol, undefined); + assert.strictEqual(resolved.declined, undefined); + assert.isTrue(resolved.warnings.some((warning) => warning.includes("htp/json"))); + }), + ); + + it.effect("warns when the two signals ask for different wire formats", () => + Effect.gen(function* () { + // One serializer covers both signals here, so the metric protocol cannot + // be honored separately and saying nothing would look like it was. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_PROTOCOL: "http/json", + OTEL_EXPORTER_OTLP_METRICS_PROTOCOL: "http/protobuf", + }), + ); + assert.strictEqual(resolved.protocol, "http/json"); + assert.isTrue( + resolved.warnings.some((warning) => + warning.includes("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"), + ), + ); + }), + ); + + it.effect("reads the metric protocol when it is the only one named", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_METRICS_PROTOCOL: "http/json", + }), + ); + assert.strictEqual(resolved.protocol, "http/json"); + assert.deepStrictEqual(resolved.warnings, []); }), ); }); diff --git a/apps/server/src/observability/OtelEnvironment.ts b/apps/server/src/observability/OtelEnvironment.ts index b9ee1b4b8e48..cfcc73331b1d 100644 --- a/apps/server/src/observability/OtelEnvironment.ts +++ b/apps/server/src/observability/OtelEnvironment.ts @@ -13,6 +13,10 @@ * body the endpoint cannot parse, and the log signal has no exporter here at * all. * + * Everything else the specification requires of an unusable value is a + * warning followed by the default, never a refusal to start and never a + * silently different behavior. + * * @module observability/OtelEnvironment */ import * as Config from "effect/Config"; @@ -43,6 +47,13 @@ export interface OtlpResourceSettings { export interface OtelEnvironment { /** `OTEL_SDK_DISABLED`. When set, nothing is exported by any route. */ readonly disabled: boolean; + /** + * Settings that were named but could not be used, each already phrased for a + * human. The specification requires a warning for a value the implementation + * does not recognize, and these are collected rather than logged here so the + * caller reports them once, at startup, where someone is looking. + */ + readonly warnings: ReadonlyArray; readonly traces: OtlpSignalSettings | undefined; readonly metrics: OtlpSignalSettings | undefined; readonly metricsTemporality: MetricsTemporality | undefined; @@ -78,7 +89,7 @@ const optionalInt = (name: string) => * token. Both fail as an authentication error against the collector, which * reads like a bad token rather than a parsing bug. */ -const parseBaggage = (raw: string): Readonly> => { +const parseBaggage = (raw: string): Readonly> | undefined => { const entries: Record = {}; for (const member of raw.split(",")) { const separator = member.indexOf("="); @@ -93,15 +104,34 @@ const parseBaggage = (raw: string): Readonly> => { try { entries[key] = decodeURIComponent(value); } catch { - entries[key] = value; + return undefined; } } return entries; }; +interface Parsed { + readonly value: A | undefined; + readonly warning: string | undefined; +} + +/** + * A pair list that fails to decode discards the whole variable, which is what + * the resource specification asks for and the safer answer for headers too: a + * half-parsed credential reaches the collector as an authentication error, + * while nothing plus a warning says where to look. + */ const optionalRecord = (name: string) => optionalString(name).pipe( - Effect.map((raw) => (raw === undefined ? undefined : parseBaggage(raw))), + Effect.map((raw) => { + if (raw === undefined) { + return { value: undefined, warning: undefined }; + } + const parsed = parseBaggage(raw); + return parsed === undefined + ? { value: undefined, warning: `${name} is not valid percent encoding and was ignored` } + : { value: parsed, warning: undefined }; + }), ); /** @@ -140,83 +170,146 @@ const signalWantsOtlp = (signal: "TRACES" | "METRICS") => }), ); +/** + * The specification's own defaults, which apply once this route is the one + * configuring the exporter. A `T3CODE_OTLP_*` setup never reaches here and + * keeps the numbers T3 Code has always used. + */ +const SPEC_DEFAULT_SCHEDULE_DELAY_MS = 5_000; +const SPEC_DEFAULT_MAX_EXPORT_BATCH_SIZE = 512; +const SPEC_DEFAULT_METRIC_EXPORT_INTERVAL_MS = 60_000; + const signalSettings = (signal: "TRACES" | "METRICS") => Effect.gen(function* () { const url = yield* signalEndpoint(signal); if (url === undefined || !(yield* signalWantsOtlp(signal))) { - return undefined; + return { value: undefined, warning: undefined }; } - const headers = - (yield* optionalRecord(`OTEL_EXPORTER_OTLP_${signal}_HEADERS`)) ?? - (yield* optionalRecord("OTEL_EXPORTER_OTLP_HEADERS")); + const specific = yield* optionalRecord(`OTEL_EXPORTER_OTLP_${signal}_HEADERS`); + const generic = yield* optionalRecord("OTEL_EXPORTER_OTLP_HEADERS"); + const headers = specific.value ?? generic.value; const timeoutMs = (yield* optionalInt(`OTEL_EXPORTER_OTLP_${signal}_TIMEOUT`)) ?? (yield* optionalInt("OTEL_EXPORTER_OTLP_TIMEOUT")) ?? (signal === "METRICS" ? yield* optionalInt("OTEL_METRIC_EXPORT_TIMEOUT") : undefined); const exportIntervalMs = signal === "TRACES" - ? yield* optionalInt("OTEL_BSP_SCHEDULE_DELAY") - : yield* optionalInt("OTEL_METRIC_EXPORT_INTERVAL"); + ? ((yield* optionalInt("OTEL_BSP_SCHEDULE_DELAY")) ?? SPEC_DEFAULT_SCHEDULE_DELAY_MS) + : ((yield* optionalInt("OTEL_METRIC_EXPORT_INTERVAL")) ?? + SPEC_DEFAULT_METRIC_EXPORT_INTERVAL_MS); return { - url, - headers, - exportIntervalMs, - maxBatchSize: - signal === "TRACES" ? yield* optionalInt("OTEL_BSP_MAX_EXPORT_BATCH_SIZE") : undefined, - shutdownTimeoutMs: timeoutMs, - } satisfies OtlpSignalSettings; + value: { + url, + headers, + exportIntervalMs, + maxBatchSize: + signal === "TRACES" + ? ((yield* optionalInt("OTEL_BSP_MAX_EXPORT_BATCH_SIZE")) ?? + SPEC_DEFAULT_MAX_EXPORT_BATCH_SIZE) + : undefined, + shutdownTimeoutMs: timeoutMs, + }, + warning: specific.warning ?? generic.warning, + } satisfies Parsed; }); interface ProtocolDecision { readonly protocol: OtlpProtocol | undefined; readonly declined: string | undefined; + readonly warnings: ReadonlyArray; } /** * Left unset when nothing named a protocol, so a machine that never mentioned - * OpenTelemetry keeps the wire format T3 Code has always used. `grpc` is the - * one value that cannot be quietly downgraded: its endpoint has no - * `/v1/traces` path and expects a framing this server does not produce, so - * posting anything there is worse than exporting nothing. + * OpenTelemetry keeps the wire format T3 Code has always used. + * + * `grpc` is the one value that turns export off rather than falling back. It + * is a real protocol this server does not speak, its endpoint has no + * `/v1/traces` path, and it expects a framing nothing here produces, so + * posting to it is worse than exporting nothing. A value that is not a + * protocol at all is a typo, and the specification is explicit that those get + * a warning and the default. + * + * One serializer covers both signals, so a per-signal protocol that disagrees + * with the trace protocol cannot be honored and says so. */ const resolveProtocol = Effect.gen(function* () { - const raw = - (yield* optionalString("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL")) ?? - (yield* optionalString("OTEL_EXPORTER_OTLP_PROTOCOL")); - if (raw === undefined) { - return { protocol: undefined, declined: undefined } satisfies ProtocolDecision; + const warnings: Array = []; + const read = function* (name: string) { + const raw = yield* optionalString(name); + if (raw === undefined) { + return undefined; + } + const value = raw.trim().toLowerCase(); + if (value === "http/json" || value === "http/protobuf" || value === "grpc") { + return value; + } + warnings.push(`${name}=${raw} is not a known OTLP protocol and was ignored`); + return undefined; + }; + + const traces = + (yield* read("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL")) ?? + (yield* read("OTEL_EXPORTER_OTLP_PROTOCOL")); + const metrics = yield* read("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"); + if (metrics !== undefined && traces !== undefined && metrics !== traces) { + warnings.push( + `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL=${metrics} cannot differ from the trace protocol here; ${traces} is used for both`, + ); } - const value = raw.trim().toLowerCase(); - if (value === "http/json" || value === "http/protobuf") { - return { protocol: value, declined: undefined } satisfies ProtocolDecision; + const chosen = traces ?? metrics; + if (chosen === "grpc") { + return { + protocol: undefined, + declined: + "OTEL_EXPORTER_OTLP_PROTOCOL=grpc is not supported; this server exports OTLP over HTTP only, so nothing is exported", + warnings, + } satisfies ProtocolDecision; } - return { - protocol: undefined, - declined: `OTEL_EXPORTER_OTLP_PROTOCOL=${value} is not supported; this server exports OTLP over HTTP only`, - } satisfies ProtocolDecision; + return { protocol: chosen, declined: undefined, warnings } satisfies ProtocolDecision; }); +/** + * `lowmemory` is a real preference in the specification that this exporter + * cannot produce, so it warns and falls back to the default rather than + * pretending it applied. + */ const resolveMetricsTemporality = optionalString( "OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE", ).pipe( - Effect.map((value): MetricsTemporality | undefined => { - const preference = value?.trim().toLowerCase(); - return preference === "delta" || preference === "cumulative" ? preference : undefined; + Effect.map((raw): Parsed => { + if (raw === undefined) { + return { value: undefined, warning: undefined }; + } + const preference = raw.trim().toLowerCase(); + if (preference === "delta" || preference === "cumulative") { + return { value: preference, warning: undefined }; + } + return { + value: undefined, + warning: + preference === "lowmemory" + ? "OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE=lowmemory is not supported here; cumulative is used" + : `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE=${raw} is not a known preference and was ignored`, + }; }), ); const resolveResource = Effect.gen(function* () { - const attributes = (yield* optionalRecord("OTEL_RESOURCE_ATTRIBUTES")) ?? {}; + const parsed = yield* optionalRecord("OTEL_RESOURCE_ATTRIBUTES"); const { "service.name": attributeName, "service.version": attributeVersion, ...rest - } = attributes; + } = parsed.value ?? {}; return { - serviceName: (yield* optionalString("OTEL_SERVICE_NAME")) ?? attributeName, - serviceVersion: (yield* optionalString("OTEL_SERVICE_VERSION")) ?? attributeVersion, - attributes: rest, - } satisfies OtlpResourceSettings; + value: { + serviceName: (yield* optionalString("OTEL_SERVICE_NAME")) ?? attributeName, + serviceVersion: (yield* optionalString("OTEL_SERVICE_VERSION")) ?? attributeVersion, + attributes: rest, + }, + warning: parsed.warning, + } satisfies Parsed; }); /** @@ -226,34 +319,38 @@ const resolveResource = Effect.gen(function* () { */ export const load: Effect.Effect = Effect.gen(function* () { const disabled = yield* Config.boolean("OTEL_SDK_DISABLED").pipe(Config.withDefault(false)); - const { protocol, declined } = yield* resolveProtocol; + const protocolDecision = yield* resolveProtocol; const resource = yield* resolveResource; - if (disabled) { - return { - disabled, - traces: undefined, - metrics: undefined, - metricsTemporality: undefined, - resource, - protocol, - declined, - }; - } - const unsupportedProtocol = declined !== undefined; + const temporality = yield* resolveMetricsTemporality; + const traces = disabled + ? { value: undefined, warning: undefined } + : yield* signalSettings("TRACES"); + const metrics = disabled + ? { value: undefined, warning: undefined } + : yield* signalSettings("METRICS"); + const exportable = !disabled && protocolDecision.declined === undefined; return { disabled, - traces: unsupportedProtocol ? undefined : yield* signalSettings("TRACES"), - metrics: unsupportedProtocol ? undefined : yield* signalSettings("METRICS"), - metricsTemporality: yield* resolveMetricsTemporality, - resource, - protocol, - declined, + warnings: [ + ...protocolDecision.warnings, + resource.warning, + temporality.warning, + traces.warning, + metrics.warning, + ].filter((warning) => warning !== undefined), + traces: exportable ? traces.value : undefined, + metrics: exportable ? metrics.value : undefined, + metricsTemporality: temporality.value, + resource: resource.value, + protocol: protocolDecision.protocol, + declined: protocolDecision.declined, }; }).pipe( Effect.catchCause((cause) => Effect.logWarning("Could not read the OpenTelemetry environment", cause).pipe( Effect.as({ disabled: false, + warnings: [], traces: undefined, metrics: undefined, metricsTemporality: undefined, @@ -268,6 +365,7 @@ export const load: Effect.Effect = Effect.gen(function* () { /** An environment that asked for nothing, for tests and for the pairing CLI. */ export const none: OtelEnvironment = { disabled: false, + warnings: [], traces: undefined, metrics: undefined, metricsTemporality: undefined, diff --git a/docs/fork/0018-the-standard-otel-variables-are-honored.md b/docs/fork/0018-the-standard-otel-variables-are-honored.md index ea44634bdb7d..0a42fcba4c7a 100644 --- a/docs/fork/0018-the-standard-otel-variables-are-honored.md +++ b/docs/fork/0018-the-standard-otel-variables-are-honored.md @@ -22,6 +22,10 @@ - Keep whatever you have. The `T3CODE_OTLP_*` names, the desktop bootstrap envelope, and Settings all still win over the environment, and a setup that never mentioned OpenTelemetry keeps the wire format it always used. +- Find out when a variable did not take. A misspelled protocol, a temporality + this exporter cannot produce, or a header list that is not valid percent + encoding is named in the startup log and then ignored, instead of silently + changing nothing or quietly turning export off. ## Why diff --git a/docs/operations/observability.md b/docs/operations/observability.md index 8068d09b28ba..e04a809909e4 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -243,13 +243,14 @@ Not everything in the specification is implemented. These are the ones worth kno proxy in front of it. - **Timeouts flush at shutdown.** The specification's `OTEL_EXPORTER_OTLP_TIMEOUT` is a per-request deadline. The exporter here has no per-request knob, so the value bounds the final flush instead. -- **Interval and batch defaults are T3 Code's, not the specification's.** Leaving - `OTEL_BSP_SCHEDULE_DELAY`, `OTEL_METRIC_EXPORT_INTERVAL`, or `OTEL_BSP_MAX_EXPORT_BATCH_SIZE` unset - keeps this server's own defaults rather than the specification's 5s, 60s, and 512. Set them - explicitly if you need the specification's numbers. +- **One wire format covers both signals.** `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL` is read, but it + cannot differ from the trace protocol. Setting the two to different values logs a warning and uses + the trace protocol for both. - **Browser traces are always JSON.** The proxy that forwards traces from the client posts OTLP/HTTP JSON regardless of `OTEL_EXPORTER_OTLP_PROTOCOL`. Both are valid OTLP/HTTP, so a collector accepts either. +- **`lowmemory` temporality is not available.** `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` + accepts `cumulative` and `delta`. `lowmemory` logs a warning and falls back to `cumulative`. - **`OTEL_SERVICE_VERSION` is not a specification variable.** It is read as a convenience because the exporter library reads it too. `OTEL_RESOURCE_ATTRIBUTES=service.version=...` is the portable spelling. @@ -258,6 +259,23 @@ Everything else not listed above is ignored, including the log signal, `OTEL_BSP `OTEL_BSP_EXPORT_TIMEOUT`, sampler variables, propagator variables, and the attribute and span limit variables. +#### When A Value Cannot Be Used + +A variable this server cannot act on never stops it from starting. Two things can happen instead, +and both are logged once at startup: + +- **A warning, then the default.** A misspelled protocol, an unavailable temporality, or a pair list + that is not valid percent encoding is reported and ignored, and everything else keeps exporting. +- **Export off.** Only `OTEL_EXPORTER_OTLP_PROTOCOL=grpc` does this, because it names a transport + this server does not speak rather than a value it failed to parse. + +A `OTEL_EXPORTER_OTLP_HEADERS` or `OTEL_RESOURCE_ATTRIBUTES` value that fails to decode is discarded +whole rather than partly. A half-parsed credential reaches the collector as the same authentication +error a wrong one would, which reads like a bad token instead of a bad variable. + +Once these variables are the ones configuring the exporter, the specification's own defaults apply: +`OTEL_BSP_SCHEDULE_DELAY` 5s, `OTEL_METRIC_EXPORT_INTERVAL` 60s, and `OTEL_BSP_MAX_EXPORT_BATCH_SIZE` 512. A `T3CODE_OTLP_*` setup keeps the numbers T3 Code has always used. + ## How To Use Traces And Metrics To Debug The Server ### Start With The Local Trace File From f2dd40d7a01248c7ad5fd671474bea68956e4be8 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 20 Aug 2026 22:51:08 -0400 Subject: [PATCH 04/10] fix(server): an ambient OTEL endpoint no longer reconfigures an export it did not point A gRPC metric endpoint says nothing about where traces go, and an endpoint that lost the URL should not still be choosing that URL's wire format, headers, and batching. Both let a variable reach past the setting that outranked it. Signed-off-by: Yordis Prieto --- apps/server/src/cli/config.test.ts | 18 ++++++ apps/server/src/cli/config.ts | 28 ++++++--- .../src/observability/Layers/Observability.ts | 10 +++ .../src/observability/OtelEnvironment.test.ts | 16 +++++ .../src/observability/OtelEnvironment.ts | 63 +++++++++++-------- docs/operations/observability.md | 14 ++++- 6 files changed, 110 insertions(+), 39 deletions(-) diff --git a/apps/server/src/cli/config.test.ts b/apps/server/src/cli/config.test.ts index fcb747c7143b..b2f69bec1e6b 100644 --- a/apps/server/src/cli/config.test.ts +++ b/apps/server/src/cli/config.test.ts @@ -541,6 +541,24 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { }), ); + it.effect("leaves a T3 Code endpoint alone when the environment names another", () => + Effect.gen(function* () { + // An ambient endpoint that lost the URL must not keep configuring the + // export around it: its wire format, headers, and batching belong to the + // endpoint it named, not to this one. + const resolved = yield* resolveWithEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + T3CODE_OTLP_TRACES_URL: "http://localhost:4318/v1/traces", + }); + + expect(resolved.otelEnvironment.traces).toBeUndefined(); + expect(resolved.otelEnvironment.metrics?.url).toBe( + "https://collector.example.com/v1/metrics", + ); + expect(resolved.otlpExportIntervalMs).toBe(10_000); + }), + ); + it.effect("exports nothing at all once the SDK is switched off", () => Effect.gen(function* () { const resolved = yield* resolveWithEnv({ diff --git a/apps/server/src/cli/config.ts b/apps/server/src/cli/config.ts index e5efeb2fb06c..51c6ecf76fbe 100644 --- a/apps/server/src/cli/config.ts +++ b/apps/server/src/cli/config.ts @@ -225,7 +225,7 @@ export const resolveServerConfig = ( const path = yield* Path.Path; const fs = yield* FileSystem.FileSystem; const env = yield* EnvServerConfig; - const otelEnvironment = yield* OtelEnvironment.load; + const otel = yield* OtelEnvironment.load; const normalizedFlags = { mode: flags.mode ?? Option.none(), port: flags.port ?? Option.none(), @@ -355,6 +355,22 @@ export const resolveServerConfig = ( ); const logLevel = Option.getOrElse(cliLogLevel, () => env.logLevel); + // A signal whose endpoint came from somewhere else is not this route's to + // configure. Leaving its settings in place would let an ambient + // OTEL_EXPORTER_OTLP_ENDPOINT change the wire format, headers, and batching + // of an export that a T3CODE_OTLP_* name or Settings already answered. + const namedTracesUrl = + env.otlpTracesUrl ?? bootstrap?.otlpTracesUrl ?? persistedObservabilitySettings.otlpTracesUrl; + const namedMetricsUrl = + env.otlpMetricsUrl ?? + bootstrap?.otlpMetricsUrl ?? + persistedObservabilitySettings.otlpMetricsUrl; + const otelEnvironment = { + ...otel, + traces: namedTracesUrl === undefined ? otel.traces : undefined, + metrics: namedMetricsUrl === undefined ? otel.metrics : undefined, + } satisfies OtelEnvironment.OtelEnvironment; + const config: ServerConfig.ServerConfig["Service"] = { logLevel, traceMinLevel: env.traceMinLevel, @@ -364,16 +380,10 @@ export const resolveServerConfig = ( traceMaxFiles: env.traceMaxFiles, otlpTracesUrl: otelEnvironment.disabled ? undefined - : (env.otlpTracesUrl ?? - bootstrap?.otlpTracesUrl ?? - persistedObservabilitySettings.otlpTracesUrl ?? - otelEnvironment.traces?.url), + : (namedTracesUrl ?? otelEnvironment.traces?.url), otlpMetricsUrl: otelEnvironment.disabled ? undefined - : (env.otlpMetricsUrl ?? - bootstrap?.otlpMetricsUrl ?? - persistedObservabilitySettings.otlpMetricsUrl ?? - otelEnvironment.metrics?.url), + : (namedMetricsUrl ?? otelEnvironment.metrics?.url), otlpExportIntervalMs: env.otlpExportIntervalMs ?? otelEnvironment.traces?.exportIntervalMs ?? 10_000, otlpServiceName: env.otlpServiceName ?? otelEnvironment.resource.serviceName ?? "t3-server", diff --git a/apps/server/src/observability/Layers/Observability.ts b/apps/server/src/observability/Layers/Observability.ts index 58e5acb6d2f4..52d7072139f4 100644 --- a/apps/server/src/observability/Layers/Observability.ts +++ b/apps/server/src/observability/Layers/Observability.ts @@ -35,6 +35,16 @@ export const ObservabilityLive = Layer.unwrap( const otlpSerializationLayer = protocol === "http/protobuf" ? OtlpSerialization.layerProtobuf : OtlpSerialization.layerJson; + // The proxy that forwards spans from the client encodes JSON and nothing + // else, so a protobuf server exporter means the two halves of a trace + // arrive in different encodings. Most collectors take either, and the ones + // that do not drop the browser half while the server half looks healthy. + if (protocol === "http/protobuf" && config.otlpTracesUrl !== undefined) { + yield* Effect.logWarning( + "Server telemetry uses http/protobuf, but browser traces are forwarded as OTLP/HTTP JSON; a collector that accepts only protobuf will drop them", + ); + } + const otlpResource = { serviceName: config.otlpServiceName, ...(otel.resource.serviceVersion === undefined diff --git a/apps/server/src/observability/OtelEnvironment.test.ts b/apps/server/src/observability/OtelEnvironment.test.ts index c9587ab80a49..d6b1837614c9 100644 --- a/apps/server/src/observability/OtelEnvironment.test.ts +++ b/apps/server/src/observability/OtelEnvironment.test.ts @@ -170,6 +170,22 @@ describe("OtelEnvironment", () => { }), ); + it.effect("declines only the signal that asked for grpc", () => + Effect.gen(function* () { + // A metric endpoint that speaks gRPC says nothing about where traces go, + // and turning traces off over it loses telemetry nobody asked to lose. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_METRICS_PROTOCOL: "grpc", + }), + ); + assert.isDefined(resolved.traces); + assert.strictEqual(resolved.metrics, undefined); + assert.include(resolved.declined ?? "", "OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"); + }), + ); + it.effect("leaves the protocol unstated unless something states it", () => Effect.gen(function* () { const fallback = yield* OtelEnvironment.load.pipe(withEnv({})); diff --git a/apps/server/src/observability/OtelEnvironment.ts b/apps/server/src/observability/OtelEnvironment.ts index cfcc73331b1d..bb25dcd59bb7 100644 --- a/apps/server/src/observability/OtelEnvironment.ts +++ b/apps/server/src/observability/OtelEnvironment.ts @@ -215,7 +215,8 @@ const signalSettings = (signal: "TRACES" | "METRICS") => interface ProtocolDecision { readonly protocol: OtlpProtocol | undefined; - readonly declined: string | undefined; + readonly declinedTraces: string | undefined; + readonly declinedMetrics: string | undefined; readonly warnings: ReadonlyArray; } @@ -226,12 +227,13 @@ interface ProtocolDecision { * `grpc` is the one value that turns export off rather than falling back. It * is a real protocol this server does not speak, its endpoint has no * `/v1/traces` path, and it expects a framing nothing here produces, so - * posting to it is worse than exporting nothing. A value that is not a - * protocol at all is a typo, and the specification is explicit that those get - * a warning and the default. + * posting to it is worse than exporting nothing. It turns off only the signal + * that named it, since a metric endpoint speaking gRPC says nothing about + * where traces go. A value that is not a protocol at all is a typo, and the + * specification is explicit that those get a warning and the default. * - * One serializer covers both signals, so a per-signal protocol that disagrees - * with the trace protocol cannot be honored and says so. + * One serializer covers both signals, so two HTTP protocols that disagree + * cannot both be honored and the mismatch says so. */ const resolveProtocol = Effect.gen(function* () { const warnings: Array = []; @@ -242,31 +244,36 @@ const resolveProtocol = Effect.gen(function* () { } const value = raw.trim().toLowerCase(); if (value === "http/json" || value === "http/protobuf" || value === "grpc") { - return value; + return { value, name } as const; } warnings.push(`${name}=${raw} is not a known OTLP protocol and was ignored`); return undefined; }; - const traces = - (yield* read("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL")) ?? - (yield* read("OTEL_EXPORTER_OTLP_PROTOCOL")); - const metrics = yield* read("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"); - if (metrics !== undefined && traces !== undefined && metrics !== traces) { + const generic = yield* read("OTEL_EXPORTER_OTLP_PROTOCOL"); + const traces = (yield* read("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL")) ?? generic; + const metrics = (yield* read("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL")) ?? generic; + + const decline = (named: typeof generic) => + named?.value === "grpc" + ? `${named.name}=grpc is not supported; this server exports OTLP over HTTP only, so this signal is not exported` + : undefined; + + const overHttp = [traces, metrics].flatMap((named) => + named === undefined || named.value === "grpc" ? [] : [named.value], + ); + if (overHttp.length === 2 && overHttp[0] !== overHttp[1]) { warnings.push( - `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL=${metrics} cannot differ from the trace protocol here; ${traces} is used for both`, + `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL=${overHttp[1]} cannot differ from the trace protocol here; ${overHttp[0]} is used for both`, ); } - const chosen = traces ?? metrics; - if (chosen === "grpc") { - return { - protocol: undefined, - declined: - "OTEL_EXPORTER_OTLP_PROTOCOL=grpc is not supported; this server exports OTLP over HTTP only, so nothing is exported", - warnings, - } satisfies ProtocolDecision; - } - return { protocol: chosen, declined: undefined, warnings } satisfies ProtocolDecision; + + return { + protocol: overHttp[0], + declinedTraces: decline(traces), + declinedMetrics: decline(metrics), + warnings, + } satisfies ProtocolDecision; }); /** @@ -328,7 +335,9 @@ export const load: Effect.Effect = Effect.gen(function* () { const metrics = disabled ? { value: undefined, warning: undefined } : yield* signalSettings("METRICS"); - const exportable = !disabled && protocolDecision.declined === undefined; + const declined = [protocolDecision.declinedTraces, protocolDecision.declinedMetrics].filter( + (reason) => reason !== undefined, + ); return { disabled, warnings: [ @@ -338,12 +347,12 @@ export const load: Effect.Effect = Effect.gen(function* () { traces.warning, metrics.warning, ].filter((warning) => warning !== undefined), - traces: exportable ? traces.value : undefined, - metrics: exportable ? metrics.value : undefined, + traces: protocolDecision.declinedTraces === undefined ? traces.value : undefined, + metrics: protocolDecision.declinedMetrics === undefined ? metrics.value : undefined, metricsTemporality: temporality.value, resource: resource.value, protocol: protocolDecision.protocol, - declined: protocolDecision.declined, + declined: declined.length === 0 ? undefined : [...new Set(declined)].join(" "), }; }).pipe( Effect.catchCause((cause) => diff --git a/docs/operations/observability.md b/docs/operations/observability.md index e04a809909e4..c80697ff01e5 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -236,7 +236,8 @@ Not everything in the specification is implemented. These are the ones worth kno - **No gRPC.** `OTEL_EXPORTER_OTLP_PROTOCOL=grpc` is refused rather than downgraded, because this server has no gRPC transport and posting an HTTP body to a gRPC endpoint fails in a way that is - harder to read than exporting nothing. The refusal is logged at startup and both signals stay off. + harder to read than exporting nothing. The refusal is logged at startup and turns off only the + signal that named gRPC, so `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL=grpc` leaves traces exporting. - **No compression and no client TLS.** `OTEL_EXPORTER_OTLP_COMPRESSION`, `OTEL_EXPORTER_OTLP_CERTIFICATE`, `OTEL_EXPORTER_OTLP_CLIENT_KEY`, and `OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE` are ignored. A collector that requires mutual TLS needs a @@ -247,8 +248,10 @@ Not everything in the specification is implemented. These are the ones worth kno cannot differ from the trace protocol. Setting the two to different values logs a warning and uses the trace protocol for both. - **Browser traces are always JSON.** The proxy that forwards traces from the client posts - OTLP/HTTP JSON regardless of `OTEL_EXPORTER_OTLP_PROTOCOL`. Both are valid OTLP/HTTP, so a - collector accepts either. + OTLP/HTTP JSON regardless of `OTEL_EXPORTER_OTLP_PROTOCOL`. Both are valid OTLP/HTTP and most + collectors accept either, so this only matters against one that takes protobuf and nothing else. + When the server exporter resolves to `http/protobuf`, a startup warning names the split rather + than letting the browser half disappear while the server half looks healthy. - **`lowmemory` temporality is not available.** `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` accepts `cumulative` and `delta`. `lowmemory` logs a warning and falls back to `cumulative`. - **`OTEL_SERVICE_VERSION` is not a specification variable.** It is read as a convenience because @@ -273,6 +276,11 @@ A `OTEL_EXPORTER_OTLP_HEADERS` or `OTEL_RESOURCE_ATTRIBUTES` value that fails to whole rather than partly. A half-parsed credential reaches the collector as the same authentication error a wrong one would, which reads like a bad token instead of a bad variable. +These variables configure a signal only when they also supplied its endpoint. A `T3CODE_OTLP_*` +name, the desktop bootstrap envelope, or Settings winning the URL takes the whole signal with it, so +an ambient `OTEL_EXPORTER_OTLP_ENDPOINT` cannot reach in and change the wire format, headers, or +batching of an export it did not point anywhere. + Once these variables are the ones configuring the exporter, the specification's own defaults apply: `OTEL_BSP_SCHEDULE_DELAY` 5s, `OTEL_METRIC_EXPORT_INTERVAL` 60s, and `OTEL_BSP_MAX_EXPORT_BATCH_SIZE` 512. A `T3CODE_OTLP_*` setup keeps the numbers T3 Code has always used. From 3c272f36622ab708c51ff43d72e88adf5d5b187c Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 20 Aug 2026 23:00:05 -0400 Subject: [PATCH 05/10] fix(server): the wire format belongs to the endpoint that named it Each signal builds its own serializer, so nothing forced traces and metrics to share a protocol. Sharing one let an OTLP protocol with no endpoint of its own decide the encoding for an export a different name had already configured. Signed-off-by: Yordis Prieto --- .../src/observability/Layers/Observability.ts | 24 +++--- .../src/observability/OtelEnvironment.test.ts | 44 ++++++----- .../src/observability/OtelEnvironment.ts | 75 +++++++++---------- docs/operations/observability.md | 12 +-- 4 files changed, 82 insertions(+), 73 deletions(-) diff --git a/apps/server/src/observability/Layers/Observability.ts b/apps/server/src/observability/Layers/Observability.ts index 52d7072139f4..012161f8ecec 100644 --- a/apps/server/src/observability/Layers/Observability.ts +++ b/apps/server/src/observability/Layers/Observability.ts @@ -28,18 +28,19 @@ export const ObservabilityLive = Layer.unwrap( yield* Effect.logWarning(otel.declined); } - const configuredThroughOtelEnvironment = - otel.traces !== undefined || otel.metrics !== undefined; - const protocol = - otel.protocol ?? (configuredThroughOtelEnvironment ? "http/protobuf" : "http/json"); - const otlpSerializationLayer = - protocol === "http/protobuf" ? OtlpSerialization.layerProtobuf : OtlpSerialization.layerJson; + // Each signal builds its own serializer, so the wire format travels with + // the settings of the endpoint that asked for it. A signal these variables + // did not supply keeps the JSON T3 Code has always sent. + const serializationFor = (signal: typeof otel.traces) => + signal?.protocol === "http/protobuf" + ? OtlpSerialization.layerProtobuf + : OtlpSerialization.layerJson; // The proxy that forwards spans from the client encodes JSON and nothing - // else, so a protobuf server exporter means the two halves of a trace + // else, so a protobuf trace exporter means the two halves of a trace // arrive in different encodings. Most collectors take either, and the ones // that do not drop the browser half while the server half looks healthy. - if (protocol === "http/protobuf" && config.otlpTracesUrl !== undefined) { + if (otel.traces?.protocol === "http/protobuf" && config.otlpTracesUrl !== undefined) { yield* Effect.logWarning( "Server telemetry uses http/protobuf, but browser traces are forwarded as OTLP/HTTP JSON; a collector that accepts only protobuf will drop them", ); @@ -109,7 +110,10 @@ export const ObservabilityLive = Layer.unwrap( BrowserTraceCollector.layer(sink), ); }), - ).pipe(Layer.provide(OtlpExporter.layerFlusher), Layer.provideMerge(otlpSerializationLayer)); + ).pipe( + Layer.provide(OtlpExporter.layerFlusher), + Layer.provideMerge(serializationFor(otel.traces)), + ); const metricsLayer = config.otlpMetricsUrl === undefined @@ -125,7 +129,7 @@ export const ObservabilityLive = Layer.unwrap( ...(otel.metricsTemporality === undefined ? {} : { temporality: otel.metricsTemporality }), - }).pipe(Layer.provideMerge(otlpSerializationLayer)); + }).pipe(Layer.provideMerge(serializationFor(otel.metrics))); return Layer.mergeAll(ServerLoggerLive, traceReferencesLayer, tracerLayer, metricsLayer); }), diff --git a/apps/server/src/observability/OtelEnvironment.test.ts b/apps/server/src/observability/OtelEnvironment.test.ts index d6b1837614c9..51305926b6db 100644 --- a/apps/server/src/observability/OtelEnvironment.test.ts +++ b/apps/server/src/observability/OtelEnvironment.test.ts @@ -186,14 +186,25 @@ describe("OtelEnvironment", () => { }), ); - it.effect("leaves the protocol unstated unless something states it", () => + it.effect("defaults each signal to the specification's wire format", () => Effect.gen(function* () { - const fallback = yield* OtelEnvironment.load.pipe(withEnv({})); - assert.strictEqual(fallback.protocol, undefined); - const json = yield* OtelEnvironment.load.pipe( - withEnv({ OTEL_EXPORTER_OTLP_PROTOCOL: "http/json" }), + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com" }), + ); + assert.strictEqual(resolved.traces?.protocol, "http/protobuf"); + assert.strictEqual(resolved.metrics?.protocol, "http/protobuf"); + }), + ); + + it.effect("keeps the wire format on the signal that named an endpoint", () => + Effect.gen(function* () { + // A protocol with no endpoint of its own describes nothing, so it must + // not reach an export configured by some other name. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_EXPORTER_OTLP_PROTOCOL: "http/protobuf" }), ); - assert.strictEqual(json.protocol, "http/json"); + assert.strictEqual(resolved.traces, undefined); + assert.strictEqual(resolved.metrics, undefined); }), ); @@ -345,17 +356,16 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_PROTOCOL: "htp/json", }), ); - assert.isDefined(resolved.traces); - assert.strictEqual(resolved.protocol, undefined); + assert.strictEqual(resolved.traces?.protocol, "http/protobuf"); assert.strictEqual(resolved.declined, undefined); assert.isTrue(resolved.warnings.some((warning) => warning.includes("htp/json"))); }), ); - it.effect("warns when the two signals ask for different wire formats", () => + it.effect("lets the two signals use different wire formats", () => Effect.gen(function* () { - // One serializer covers both signals here, so the metric protocol cannot - // be honored separately and saying nothing would look like it was. + // Each signal builds its own serializer, so the metric protocol is + // honored on its own rather than losing to the trace one. const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", @@ -363,12 +373,9 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_METRICS_PROTOCOL: "http/protobuf", }), ); - assert.strictEqual(resolved.protocol, "http/json"); - assert.isTrue( - resolved.warnings.some((warning) => - warning.includes("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"), - ), - ); + assert.strictEqual(resolved.traces?.protocol, "http/json"); + assert.strictEqual(resolved.metrics?.protocol, "http/protobuf"); + assert.deepStrictEqual(resolved.warnings, []); }), ); @@ -380,7 +387,8 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_METRICS_PROTOCOL: "http/json", }), ); - assert.strictEqual(resolved.protocol, "http/json"); + assert.strictEqual(resolved.metrics?.protocol, "http/json"); + assert.strictEqual(resolved.traces?.protocol, "http/protobuf"); assert.deepStrictEqual(resolved.warnings, []); }), ); diff --git a/apps/server/src/observability/OtelEnvironment.ts b/apps/server/src/observability/OtelEnvironment.ts index bb25dcd59bb7..c9e2d6b71121 100644 --- a/apps/server/src/observability/OtelEnvironment.ts +++ b/apps/server/src/observability/OtelEnvironment.ts @@ -32,6 +32,13 @@ export type MetricsTemporality = "cumulative" | "delta"; /** Everything one signal's exporter needs, or `undefined` if it is off. */ export interface OtlpSignalSettings { readonly url: string; + /** + * The wire format for this signal alone. Each signal builds its own + * serializer, so the two are free to differ, and keeping the choice on the + * signal is what stops it from reaching an endpoint these variables did not + * supply. + */ + readonly protocol: OtlpProtocol; readonly headers: Readonly> | undefined; readonly exportIntervalMs: number | undefined; readonly maxBatchSize: number | undefined; @@ -58,13 +65,6 @@ export interface OtelEnvironment { readonly metrics: OtlpSignalSettings | undefined; readonly metricsTemporality: MetricsTemporality | undefined; readonly resource: OtlpResourceSettings; - /** - * The wire format the environment asked for, or `undefined` when it said - * nothing. The spec's default is `http/protobuf`, which applies to an - * environment that configured OTLP through these variables; one that did not - * keeps whatever T3 Code already used. - */ - readonly protocol: OtlpProtocol | undefined; /** * Why a configured endpoint is not being used, if it is not. Carried rather * than logged here so the caller can report it once, at startup, where a @@ -175,11 +175,12 @@ const signalWantsOtlp = (signal: "TRACES" | "METRICS") => * configuring the exporter. A `T3CODE_OTLP_*` setup never reaches here and * keeps the numbers T3 Code has always used. */ +const SPEC_DEFAULT_PROTOCOL = "http/protobuf" as const; const SPEC_DEFAULT_SCHEDULE_DELAY_MS = 5_000; const SPEC_DEFAULT_MAX_EXPORT_BATCH_SIZE = 512; const SPEC_DEFAULT_METRIC_EXPORT_INTERVAL_MS = 60_000; -const signalSettings = (signal: "TRACES" | "METRICS") => +const signalSettings = (signal: "TRACES" | "METRICS", protocol: OtlpProtocol) => Effect.gen(function* () { const url = yield* signalEndpoint(signal); if (url === undefined || !(yield* signalWantsOtlp(signal))) { @@ -200,6 +201,7 @@ const signalSettings = (signal: "TRACES" | "METRICS") => return { value: { url, + protocol, headers, exportIntervalMs, maxBatchSize: @@ -213,10 +215,15 @@ const signalSettings = (signal: "TRACES" | "METRICS") => } satisfies Parsed; }); +/** What one signal should do about its wire format. */ +interface SignalProtocol { + readonly protocol: OtlpProtocol; + readonly declined: string | undefined; +} + interface ProtocolDecision { - readonly protocol: OtlpProtocol | undefined; - readonly declinedTraces: string | undefined; - readonly declinedMetrics: string | undefined; + readonly traces: SignalProtocol; + readonly metrics: SignalProtocol; readonly warnings: ReadonlyArray; } @@ -232,8 +239,8 @@ interface ProtocolDecision { * where traces go. A value that is not a protocol at all is a typo, and the * specification is explicit that those get a warning and the default. * - * One serializer covers both signals, so two HTTP protocols that disagree - * cannot both be honored and the mismatch says so. + * Each signal builds its own serializer, so the two are answered separately + * and are free to disagree. */ const resolveProtocol = Effect.gen(function* () { const warnings: Array = []; @@ -254,26 +261,17 @@ const resolveProtocol = Effect.gen(function* () { const traces = (yield* read("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL")) ?? generic; const metrics = (yield* read("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL")) ?? generic; - const decline = (named: typeof generic) => - named?.value === "grpc" - ? `${named.name}=grpc is not supported; this server exports OTLP over HTTP only, so this signal is not exported` - : undefined; + const decide = (named: typeof generic): SignalProtocol => + named === undefined + ? { protocol: SPEC_DEFAULT_PROTOCOL, declined: undefined } + : named.value === "grpc" + ? { + protocol: SPEC_DEFAULT_PROTOCOL, + declined: `${named.name}=grpc is not supported; this server exports OTLP over HTTP only, so this signal is not exported`, + } + : { protocol: named.value, declined: undefined }; - const overHttp = [traces, metrics].flatMap((named) => - named === undefined || named.value === "grpc" ? [] : [named.value], - ); - if (overHttp.length === 2 && overHttp[0] !== overHttp[1]) { - warnings.push( - `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL=${overHttp[1]} cannot differ from the trace protocol here; ${overHttp[0]} is used for both`, - ); - } - - return { - protocol: overHttp[0], - declinedTraces: decline(traces), - declinedMetrics: decline(metrics), - warnings, - } satisfies ProtocolDecision; + return { traces: decide(traces), metrics: decide(metrics), warnings } satisfies ProtocolDecision; }); /** @@ -331,11 +329,11 @@ export const load: Effect.Effect = Effect.gen(function* () { const temporality = yield* resolveMetricsTemporality; const traces = disabled ? { value: undefined, warning: undefined } - : yield* signalSettings("TRACES"); + : yield* signalSettings("TRACES", protocolDecision.traces.protocol); const metrics = disabled ? { value: undefined, warning: undefined } - : yield* signalSettings("METRICS"); - const declined = [protocolDecision.declinedTraces, protocolDecision.declinedMetrics].filter( + : yield* signalSettings("METRICS", protocolDecision.metrics.protocol); + const declined = [protocolDecision.traces.declined, protocolDecision.metrics.declined].filter( (reason) => reason !== undefined, ); return { @@ -347,11 +345,10 @@ export const load: Effect.Effect = Effect.gen(function* () { traces.warning, metrics.warning, ].filter((warning) => warning !== undefined), - traces: protocolDecision.declinedTraces === undefined ? traces.value : undefined, - metrics: protocolDecision.declinedMetrics === undefined ? metrics.value : undefined, + traces: protocolDecision.traces.declined === undefined ? traces.value : undefined, + metrics: protocolDecision.metrics.declined === undefined ? metrics.value : undefined, metricsTemporality: temporality.value, resource: resource.value, - protocol: protocolDecision.protocol, declined: declined.length === 0 ? undefined : [...new Set(declined)].join(" "), }; }).pipe( @@ -364,7 +361,6 @@ export const load: Effect.Effect = Effect.gen(function* () { metrics: undefined, metricsTemporality: undefined, resource: { serviceName: undefined, serviceVersion: undefined, attributes: {} }, - protocol: undefined, declined: "the OpenTelemetry environment could not be read", }), ), @@ -379,6 +375,5 @@ export const none: OtelEnvironment = { metrics: undefined, metricsTemporality: undefined, resource: { serviceName: undefined, serviceVersion: undefined, attributes: {} }, - protocol: undefined, declined: undefined, }; diff --git a/docs/operations/observability.md b/docs/operations/observability.md index c80697ff01e5..792b1a4e1795 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -244,14 +244,14 @@ Not everything in the specification is implemented. These are the ones worth kno proxy in front of it. - **Timeouts flush at shutdown.** The specification's `OTEL_EXPORTER_OTLP_TIMEOUT` is a per-request deadline. The exporter here has no per-request knob, so the value bounds the final flush instead. -- **One wire format covers both signals.** `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL` is read, but it - cannot differ from the trace protocol. Setting the two to different values logs a warning and uses - the trace protocol for both. - **Browser traces are always JSON.** The proxy that forwards traces from the client posts OTLP/HTTP JSON regardless of `OTEL_EXPORTER_OTLP_PROTOCOL`. Both are valid OTLP/HTTP and most collectors accept either, so this only matters against one that takes protobuf and nothing else. - When the server exporter resolves to `http/protobuf`, a startup warning names the split rather + When the trace exporter resolves to `http/protobuf`, a startup warning names the split rather than letting the browser half disappear while the server half looks healthy. +- **No protocol name of T3 Code's own.** `OTEL_EXPORTER_OTLP_PROTOCOL` describes the endpoint these + variables named. A `T3CODE_OTLP_*` endpoint always uses `http/json`, which is what it has always + used. - **`lowmemory` temporality is not available.** `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` accepts `cumulative` and `delta`. `lowmemory` logs a warning and falls back to `cumulative`. - **`OTEL_SERVICE_VERSION` is not a specification variable.** It is read as a convenience because @@ -279,7 +279,9 @@ error a wrong one would, which reads like a bad token instead of a bad variable. These variables configure a signal only when they also supplied its endpoint. A `T3CODE_OTLP_*` name, the desktop bootstrap envelope, or Settings winning the URL takes the whole signal with it, so an ambient `OTEL_EXPORTER_OTLP_ENDPOINT` cannot reach in and change the wire format, headers, or -batching of an export it did not point anywhere. +batching of an export it did not point anywhere. Traces and metrics are answered separately +throughout, so `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL` applies to metrics alone and leaves traces as +they were. Once these variables are the ones configuring the exporter, the specification's own defaults apply: `OTEL_BSP_SCHEDULE_DELAY` 5s, `OTEL_METRIC_EXPORT_INTERVAL` 60s, and `OTEL_BSP_MAX_EXPORT_BATCH_SIZE` 512. A `T3CODE_OTLP_*` setup keeps the numbers T3 Code has always used. From 51a7651fdbcf065c4a7ef85db1eb393ab1d5c7f7 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 20 Aug 2026 23:12:54 -0400 Subject: [PATCH 06/10] fix(server): a signal that took its endpoint elsewhere keeps nothing from the environment Aggregation, batch schedule, and the gRPC refusal each sat beside the signal they belong to rather than inside it, so narrowing away an ambient endpoint left them behind to reach an export the environment never pointed at, and to report a live signal as not exported. Signed-off-by: Yordis Prieto --- apps/server/src/bin.test.ts | 1 + apps/server/src/cli/config.test.ts | 49 ++++++++- apps/server/src/cli/config.ts | 23 ++-- apps/server/src/cli/pair.ts | 1 + apps/server/src/config.ts | 2 + .../src/environment/ServerEnvironment.test.ts | 1 + apps/server/src/http.ts | 2 +- .../src/observability/Layers/Observability.ts | 47 ++++---- .../src/observability/OtelEnvironment.test.ts | 100 ++++++++++-------- .../src/observability/OtelEnvironment.ts | 77 +++++++++----- apps/server/src/server.test.ts | 1 + docs/operations/observability.md | 14 ++- 12 files changed, 212 insertions(+), 106 deletions(-) diff --git a/apps/server/src/bin.test.ts b/apps/server/src/bin.test.ts index 8b8555a40574..81ce882dc721 100644 --- a/apps/server/src/bin.test.ts +++ b/apps/server/src/bin.test.ts @@ -76,6 +76,7 @@ const makeCliTestServerConfig = (baseDir: string) => otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, + otlpMetricsExportIntervalMs: 10_000, otlpServiceName: "t3-server", otelEnvironment: OtelEnvironment.none, mode: "web", diff --git a/apps/server/src/cli/config.test.ts b/apps/server/src/cli/config.test.ts index b2f69bec1e6b..756f25854584 100644 --- a/apps/server/src/cli/config.test.ts +++ b/apps/server/src/cli/config.test.ts @@ -50,6 +50,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, + otlpMetricsExportIntervalMs: 10_000, otlpServiceName: "t3-server", otelEnvironment: OtelEnvironment.none, devAllowedOrigins: [], @@ -551,14 +552,58 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { T3CODE_OTLP_TRACES_URL: "http://localhost:4318/v1/traces", }); - expect(resolved.otelEnvironment.traces).toBeUndefined(); - expect(resolved.otelEnvironment.metrics?.url).toBe( + expect(resolved.otelEnvironment.traces.settings).toBeUndefined(); + expect(resolved.otelEnvironment.metrics.settings?.url).toBe( "https://collector.example.com/v1/metrics", ); expect(resolved.otlpExportIntervalMs).toBe(10_000); }), ); + it.effect("keeps an ambient aggregation off a T3 Code metric endpoint", () => + Effect.gen(function* () { + const resolved = yield* resolveWithEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "delta", + T3CODE_OTLP_METRICS_URL: "http://localhost:4318/v1/metrics", + }); + + expect(resolved.otelEnvironment.metrics.settings).toBeUndefined(); + expect(resolved.otelEnvironment.traces.settings?.temporality).toBeUndefined(); + }), + ); + + it.effect("keeps one signal's schedule off the other one", () => + Effect.gen(function* () { + // Traces take the specification's five second batch delay from the + // ambient endpoint. Metrics went somewhere else and keep T3 Code's own + // interval rather than inheriting a number meant for spans. + const resolved = yield* resolveWithEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + T3CODE_OTLP_METRICS_URL: "http://localhost:4318/v1/metrics", + }); + + expect(resolved.otlpExportIntervalMs).toBe(5_000); + expect(resolved.otlpMetricsExportIntervalMs).toBe(10_000); + }), + ); + + it.effect("does not report a signal as declined while it is exporting", () => + Effect.gen(function* () { + // grpc turns off the export these variables asked for, and says nothing + // about a signal whose endpoint came from a T3 Code name. + const resolved = yield* resolveWithEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_PROTOCOL: "grpc", + T3CODE_OTLP_TRACES_URL: "http://localhost:4318/v1/traces", + }); + + expect(resolved.otlpTracesUrl).toBe("http://localhost:4318/v1/traces"); + expect(resolved.otelEnvironment.traces.declined).toBeUndefined(); + expect(resolved.otelEnvironment.metrics.declined).toContain("grpc"); + }), + ); + it.effect("exports nothing at all once the SDK is switched off", () => Effect.gen(function* () { const resolved = yield* resolveWithEnv({ diff --git a/apps/server/src/cli/config.ts b/apps/server/src/cli/config.ts index 51c6ecf76fbe..2f92100ee5fa 100644 --- a/apps/server/src/cli/config.ts +++ b/apps/server/src/cli/config.ts @@ -356,9 +356,11 @@ export const resolveServerConfig = ( const logLevel = Option.getOrElse(cliLogLevel, () => env.logLevel); // A signal whose endpoint came from somewhere else is not this route's to - // configure. Leaving its settings in place would let an ambient - // OTEL_EXPORTER_OTLP_ENDPOINT change the wire format, headers, and batching - // of an export that a T3CODE_OTLP_* name or Settings already answered. + // configure. Dropping the whole signal, rather than the endpoint alone, + // is what stops an ambient OTEL_EXPORTER_OTLP_ENDPOINT from changing the + // wire format, headers, batching, or aggregation of an export that a + // T3CODE_OTLP_* name or Settings already answered, and stops startup from + // reporting that signal as declined while it is exporting. const namedTracesUrl = env.otlpTracesUrl ?? bootstrap?.otlpTracesUrl ?? persistedObservabilitySettings.otlpTracesUrl; const namedMetricsUrl = @@ -367,8 +369,8 @@ export const resolveServerConfig = ( persistedObservabilitySettings.otlpMetricsUrl; const otelEnvironment = { ...otel, - traces: namedTracesUrl === undefined ? otel.traces : undefined, - metrics: namedMetricsUrl === undefined ? otel.metrics : undefined, + traces: namedTracesUrl === undefined ? otel.traces : OtelEnvironment.noSignal, + metrics: namedMetricsUrl === undefined ? otel.metrics : OtelEnvironment.noSignal, } satisfies OtelEnvironment.OtelEnvironment; const config: ServerConfig.ServerConfig["Service"] = { @@ -380,12 +382,17 @@ export const resolveServerConfig = ( traceMaxFiles: env.traceMaxFiles, otlpTracesUrl: otelEnvironment.disabled ? undefined - : (namedTracesUrl ?? otelEnvironment.traces?.url), + : (namedTracesUrl ?? otelEnvironment.traces.settings?.url), otlpMetricsUrl: otelEnvironment.disabled ? undefined - : (namedMetricsUrl ?? otelEnvironment.metrics?.url), + : (namedMetricsUrl ?? otelEnvironment.metrics.settings?.url), + // Each signal gets its own, because the environment names them + // separately and a signal that took its endpoint elsewhere must not + // inherit the other one's schedule. otlpExportIntervalMs: - env.otlpExportIntervalMs ?? otelEnvironment.traces?.exportIntervalMs ?? 10_000, + env.otlpExportIntervalMs ?? otelEnvironment.traces.settings?.exportIntervalMs ?? 10_000, + otlpMetricsExportIntervalMs: + env.otlpExportIntervalMs ?? otelEnvironment.metrics.settings?.exportIntervalMs ?? 10_000, otlpServiceName: env.otlpServiceName ?? otelEnvironment.resource.serviceName ?? "t3-server", otelEnvironment, mode, diff --git a/apps/server/src/cli/pair.ts b/apps/server/src/cli/pair.ts index ff1dab16ddaa..16a46d097b87 100644 --- a/apps/server/src/cli/pair.ts +++ b/apps/server/src/cli/pair.ts @@ -332,6 +332,7 @@ const makePairServerConfig = Effect.fn(function* (input: { otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, + otlpMetricsExportIntervalMs: 10_000, otlpServiceName: "t3-server", otelEnvironment: OtelEnvironment.none, mode: "web", diff --git a/apps/server/src/config.ts b/apps/server/src/config.ts index 52ae524d9a74..07e309421921 100644 --- a/apps/server/src/config.ts +++ b/apps/server/src/config.ts @@ -66,6 +66,7 @@ export class ServerConfig extends Context.Service< readonly otlpTracesUrl: string | undefined; readonly otlpMetricsUrl: string | undefined; readonly otlpExportIntervalMs: number; + readonly otlpMetricsExportIntervalMs: number; readonly otlpServiceName: string; /** * What the standard `OTEL_*` variables asked for. The endpoints above are @@ -186,6 +187,7 @@ const makeTest = Effect.fn("ServerConfig.makeTest")(function* ( otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, + otlpMetricsExportIntervalMs: 10_000, otlpServiceName: "t3-server", otelEnvironment: OtelEnvironment.none, cwd, diff --git a/apps/server/src/environment/ServerEnvironment.test.ts b/apps/server/src/environment/ServerEnvironment.test.ts index 37cadf39dde0..17ab600257b0 100644 --- a/apps/server/src/environment/ServerEnvironment.test.ts +++ b/apps/server/src/environment/ServerEnvironment.test.ts @@ -52,6 +52,7 @@ const makeServerConfig = Effect.fn(function* (baseDir: string) { otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, + otlpMetricsExportIntervalMs: 10_000, otlpServiceName: "t3-server", otelEnvironment: OtelEnvironment.none, cwd: process.cwd(), diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index 290e4fa2bdab..2436d7e85e4b 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -147,7 +147,7 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( const request = yield* HttpServerRequest.HttpServerRequest; const config = yield* ServerConfig.ServerConfig; const otlpTracesUrl = config.otlpTracesUrl; - const otlpTracesHeaders = config.otelEnvironment.traces?.headers; + const otlpTracesHeaders = config.otelEnvironment.traces.settings?.headers; const browserTraceCollector = yield* BrowserTraceCollector.BrowserTraceCollector; const httpClient = yield* HttpClient.HttpClient; const bodyJson = cast(yield* request.json); diff --git a/apps/server/src/observability/Layers/Observability.ts b/apps/server/src/observability/Layers/Observability.ts index 012161f8ecec..ca7b2b298f10 100644 --- a/apps/server/src/observability/Layers/Observability.ts +++ b/apps/server/src/observability/Layers/Observability.ts @@ -24,15 +24,20 @@ export const ObservabilityLive = Layer.unwrap( yield* Effect.logWarning(warning); } - if (otel.declined !== undefined) { - yield* Effect.logWarning(otel.declined); + // One variable can decline both signals, and saying so twice reads like + // two separate problems. + const declined = new Set( + [otel.traces.declined, otel.metrics.declined].filter((reason) => reason !== undefined), + ); + for (const reason of declined) { + yield* Effect.logWarning(reason); } // Each signal builds its own serializer, so the wire format travels with // the settings of the endpoint that asked for it. A signal these variables // did not supply keeps the JSON T3 Code has always sent. - const serializationFor = (signal: typeof otel.traces) => - signal?.protocol === "http/protobuf" + const serializationFor = (settings: typeof otel.traces.settings) => + settings?.protocol === "http/protobuf" ? OtlpSerialization.layerProtobuf : OtlpSerialization.layerJson; @@ -40,7 +45,7 @@ export const ObservabilityLive = Layer.unwrap( // else, so a protobuf trace exporter means the two halves of a trace // arrive in different encodings. Most collectors take either, and the ones // that do not drop the browser half while the server half looks healthy. - if (otel.traces?.protocol === "http/protobuf" && config.otlpTracesUrl !== undefined) { + if (otel.traces.settings?.protocol === "http/protobuf" && config.otlpTracesUrl !== undefined) { yield* Effect.logWarning( "Server telemetry uses http/protobuf, but browser traces are forwarded as OTLP/HTTP JSON; a collector that accepts only protobuf will drop them", ); @@ -87,13 +92,17 @@ export const ObservabilityLive = Layer.unwrap( url: config.otlpTracesUrl, exportInterval: `${config.otlpExportIntervalMs} millis`, resource: otlpResource, - ...(otel.traces?.headers === undefined ? {} : { headers: otel.traces.headers }), - ...(otel.traces?.maxBatchSize === undefined + ...(otel.traces.settings?.headers === undefined + ? {} + : { headers: otel.traces.settings.headers }), + ...(otel.traces.settings?.maxBatchSize === undefined ? {} - : { maxBatchSize: otel.traces.maxBatchSize }), - ...(otel.traces?.shutdownTimeoutMs === undefined + : { maxBatchSize: otel.traces.settings.maxBatchSize }), + ...(otel.traces.settings?.shutdownTimeoutMs === undefined ? {} - : { shutdownTimeout: `${otel.traces.shutdownTimeoutMs} millis` as const }), + : { + shutdownTimeout: `${otel.traces.settings.shutdownTimeoutMs} millis` as const, + }), }); const tracer = yield* makeLocalFileTracer({ @@ -112,7 +121,7 @@ export const ObservabilityLive = Layer.unwrap( }), ).pipe( Layer.provide(OtlpExporter.layerFlusher), - Layer.provideMerge(serializationFor(otel.traces)), + Layer.provideMerge(serializationFor(otel.traces.settings)), ); const metricsLayer = @@ -120,16 +129,18 @@ export const ObservabilityLive = Layer.unwrap( ? Layer.empty : OtlpMetrics.layer({ url: config.otlpMetricsUrl, - exportInterval: `${otel.metrics?.exportIntervalMs ?? config.otlpExportIntervalMs} millis`, + exportInterval: `${config.otlpMetricsExportIntervalMs} millis`, resource: otlpResource, - ...(otel.metrics?.headers === undefined ? {} : { headers: otel.metrics.headers }), - ...(otel.metrics?.shutdownTimeoutMs === undefined + ...(otel.metrics.settings?.headers === undefined + ? {} + : { headers: otel.metrics.settings.headers }), + ...(otel.metrics.settings?.shutdownTimeoutMs === undefined ? {} - : { shutdownTimeout: `${otel.metrics.shutdownTimeoutMs} millis` as const }), - ...(otel.metricsTemporality === undefined + : { shutdownTimeout: `${otel.metrics.settings.shutdownTimeoutMs} millis` as const }), + ...(otel.metrics.settings?.temporality === undefined ? {} - : { temporality: otel.metricsTemporality }), - }).pipe(Layer.provideMerge(serializationFor(otel.metrics))); + : { temporality: otel.metrics.settings.temporality }), + }).pipe(Layer.provideMerge(serializationFor(otel.metrics.settings))); return Layer.mergeAll(ServerLoggerLive, traceReferencesLayer, tracerLayer, metricsLayer); }), diff --git a/apps/server/src/observability/OtelEnvironment.test.ts b/apps/server/src/observability/OtelEnvironment.test.ts index 51305926b6db..d147a902fb18 100644 --- a/apps/server/src/observability/OtelEnvironment.test.ts +++ b/apps/server/src/observability/OtelEnvironment.test.ts @@ -12,8 +12,8 @@ describe("OtelEnvironment", () => { it.effect("stays off when nothing is configured", () => Effect.gen(function* () { const resolved = yield* OtelEnvironment.load.pipe(withEnv({})); - assert.strictEqual(resolved.traces, undefined); - assert.strictEqual(resolved.metrics, undefined); + assert.strictEqual(resolved.traces.settings, undefined); + assert.strictEqual(resolved.metrics.settings, undefined); assert.strictEqual(resolved.disabled, false); }), ); @@ -23,8 +23,11 @@ describe("OtelEnvironment", () => { const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com" }), ); - assert.strictEqual(resolved.traces?.url, "https://collector.example.com/v1/traces"); - assert.strictEqual(resolved.metrics?.url, "https://collector.example.com/v1/metrics"); + assert.strictEqual(resolved.traces.settings?.url, "https://collector.example.com/v1/traces"); + assert.strictEqual( + resolved.metrics.settings?.url, + "https://collector.example.com/v1/metrics", + ); }), ); @@ -33,7 +36,7 @@ describe("OtelEnvironment", () => { const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com/" }), ); - assert.strictEqual(resolved.traces?.url, "https://collector.example.com/v1/traces"); + assert.strictEqual(resolved.traces.settings?.url, "https://collector.example.com/v1/traces"); }), ); @@ -47,8 +50,8 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: "https://traces.example.com/ingest", }), ); - assert.strictEqual(resolved.traces?.url, "https://traces.example.com/ingest"); - assert.strictEqual(resolved.metrics?.url, "https://generic.example.com/v1/metrics"); + assert.strictEqual(resolved.traces.settings?.url, "https://traces.example.com/ingest"); + assert.strictEqual(resolved.metrics.settings?.url, "https://generic.example.com/v1/metrics"); }), ); @@ -62,7 +65,7 @@ describe("OtelEnvironment", () => { OTEL_METRICS_EXPORTER: "otlp", }), ); - assert.isDefined(resolved.traces); + assert.isDefined(resolved.traces.settings); }), ); @@ -74,8 +77,8 @@ describe("OtelEnvironment", () => { OTEL_TRACES_EXPORTER: "none", }), ); - assert.strictEqual(resolved.traces, undefined); - assert.isDefined(resolved.metrics); + assert.strictEqual(resolved.traces.settings, undefined); + assert.isDefined(resolved.metrics.settings); }), ); @@ -87,7 +90,7 @@ describe("OtelEnvironment", () => { OTEL_TRACES_EXPORTER: "console, otlp", }), ); - assert.isDefined(resolved.traces); + assert.isDefined(resolved.traces.settings); }), ); @@ -100,8 +103,8 @@ describe("OtelEnvironment", () => { }), ); assert.strictEqual(resolved.disabled, true); - assert.strictEqual(resolved.traces, undefined); - assert.strictEqual(resolved.metrics, undefined); + assert.strictEqual(resolved.traces.settings, undefined); + assert.strictEqual(resolved.metrics.settings, undefined); }), ); @@ -117,8 +120,8 @@ describe("OtelEnvironment", () => { // The per-signal header set replaces the generic one rather than // merging with it, which is what the spec says and what a collector // with two different keys depends on. - assert.deepStrictEqual(resolved.traces?.headers, { "api-key": "traces-only" }); - assert.deepStrictEqual(resolved.metrics?.headers, { + assert.deepStrictEqual(resolved.traces.settings?.headers, { "api-key": "traces-only" }); + assert.deepStrictEqual(resolved.metrics.settings?.headers, { "api-key": "abc123", "x-tenant": "acme", }); @@ -164,9 +167,9 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_PROTOCOL: "grpc", }), ); - assert.strictEqual(resolved.traces, undefined); - assert.strictEqual(resolved.metrics, undefined); - assert.include(resolved.declined ?? "", "grpc"); + assert.strictEqual(resolved.traces.settings, undefined); + assert.strictEqual(resolved.metrics.settings, undefined); + assert.include(resolved.traces.declined ?? "", "grpc"); }), ); @@ -180,9 +183,9 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_METRICS_PROTOCOL: "grpc", }), ); - assert.isDefined(resolved.traces); - assert.strictEqual(resolved.metrics, undefined); - assert.include(resolved.declined ?? "", "OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"); + assert.isDefined(resolved.traces.settings); + assert.strictEqual(resolved.metrics.settings, undefined); + assert.include(resolved.metrics.declined ?? "", "OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"); }), ); @@ -191,8 +194,8 @@ describe("OtelEnvironment", () => { const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com" }), ); - assert.strictEqual(resolved.traces?.protocol, "http/protobuf"); - assert.strictEqual(resolved.metrics?.protocol, "http/protobuf"); + assert.strictEqual(resolved.traces.settings?.protocol, "http/protobuf"); + assert.strictEqual(resolved.metrics.settings?.protocol, "http/protobuf"); }), ); @@ -203,8 +206,8 @@ describe("OtelEnvironment", () => { const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_PROTOCOL: "http/protobuf" }), ); - assert.strictEqual(resolved.traces, undefined); - assert.strictEqual(resolved.metrics, undefined); + assert.strictEqual(resolved.traces.settings, undefined); + assert.strictEqual(resolved.metrics.settings, undefined); }), ); @@ -219,10 +222,10 @@ describe("OtelEnvironment", () => { OTEL_METRIC_EXPORT_INTERVAL: "15000", }), ); - assert.strictEqual(resolved.traces?.exportIntervalMs, 2500); - assert.strictEqual(resolved.traces?.maxBatchSize, 128); - assert.strictEqual(resolved.traces?.shutdownTimeoutMs, 7000); - assert.strictEqual(resolved.metrics?.exportIntervalMs, 15000); + assert.strictEqual(resolved.traces.settings?.exportIntervalMs, 2500); + assert.strictEqual(resolved.traces.settings?.maxBatchSize, 128); + assert.strictEqual(resolved.traces.settings?.shutdownTimeoutMs, 7000); + assert.strictEqual(resolved.metrics.settings?.exportIntervalMs, 15000); }), ); @@ -233,9 +236,9 @@ describe("OtelEnvironment", () => { const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com" }), ); - assert.strictEqual(resolved.traces?.exportIntervalMs, 5000); - assert.strictEqual(resolved.traces?.maxBatchSize, 512); - assert.strictEqual(resolved.metrics?.exportIntervalMs, 60000); + assert.strictEqual(resolved.traces.settings?.exportIntervalMs, 5000); + assert.strictEqual(resolved.traces.settings?.maxBatchSize, 512); + assert.strictEqual(resolved.metrics.settings?.exportIntervalMs, 60000); }), ); @@ -248,8 +251,8 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "delta", }), ); - assert.strictEqual(resolved.metrics?.shutdownTimeoutMs, 9000); - assert.strictEqual(resolved.metricsTemporality, "delta"); + assert.strictEqual(resolved.metrics.settings?.shutdownTimeoutMs, 9000); + assert.strictEqual(resolved.metrics.settings?.temporality, "delta"); }), ); @@ -261,7 +264,7 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_HEADERS: "Authorization=Bearer%20abc123, x-scope=team%2Fplatform", }), ); - assert.deepStrictEqual(resolved.traces?.headers, { + assert.deepStrictEqual(resolved.traces.settings?.headers, { Authorization: "Bearer abc123", "x-scope": "team/platform", }); @@ -276,7 +279,9 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_HEADERS: "Authorization=Basic YWJjOmRlZg==", }), ); - assert.deepStrictEqual(resolved.traces?.headers, { Authorization: "Basic YWJjOmRlZg==" }); + assert.deepStrictEqual(resolved.traces.settings?.headers, { + Authorization: "Basic YWJjOmRlZg==", + }); }), ); @@ -303,7 +308,7 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_HEADERS: "x-token=100%zz,x-other=100%25", }), ); - assert.strictEqual(resolved.traces?.headers, undefined); + assert.strictEqual(resolved.traces.settings?.headers, undefined); assert.isTrue( resolved.warnings.some((warning) => warning.includes("OTEL_EXPORTER_OTLP_HEADERS")), ); @@ -327,7 +332,10 @@ describe("OtelEnvironment", () => { const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com/otel" }), ); - assert.strictEqual(resolved.traces?.url, "https://collector.example.com/otel/v1/traces"); + assert.strictEqual( + resolved.traces.settings?.url, + "https://collector.example.com/otel/v1/traces", + ); }), ); @@ -339,8 +347,8 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "lowmemory", }), ); - assert.strictEqual(resolved.metricsTemporality, undefined); - assert.isDefined(resolved.metrics); + assert.strictEqual(resolved.metrics.settings?.temporality, undefined); + assert.isDefined(resolved.metrics.settings); assert.isTrue(resolved.warnings.some((warning) => warning.includes("lowmemory"))); }), ); @@ -356,8 +364,8 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_PROTOCOL: "htp/json", }), ); - assert.strictEqual(resolved.traces?.protocol, "http/protobuf"); - assert.strictEqual(resolved.declined, undefined); + assert.strictEqual(resolved.traces.settings?.protocol, "http/protobuf"); + assert.strictEqual(resolved.traces.declined, undefined); assert.isTrue(resolved.warnings.some((warning) => warning.includes("htp/json"))); }), ); @@ -373,8 +381,8 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_METRICS_PROTOCOL: "http/protobuf", }), ); - assert.strictEqual(resolved.traces?.protocol, "http/json"); - assert.strictEqual(resolved.metrics?.protocol, "http/protobuf"); + assert.strictEqual(resolved.traces.settings?.protocol, "http/json"); + assert.strictEqual(resolved.metrics.settings?.protocol, "http/protobuf"); assert.deepStrictEqual(resolved.warnings, []); }), ); @@ -387,8 +395,8 @@ describe("OtelEnvironment", () => { OTEL_EXPORTER_OTLP_METRICS_PROTOCOL: "http/json", }), ); - assert.strictEqual(resolved.metrics?.protocol, "http/json"); - assert.strictEqual(resolved.traces?.protocol, "http/protobuf"); + assert.strictEqual(resolved.metrics.settings?.protocol, "http/json"); + assert.strictEqual(resolved.traces.settings?.protocol, "http/protobuf"); assert.deepStrictEqual(resolved.warnings, []); }), ); diff --git a/apps/server/src/observability/OtelEnvironment.ts b/apps/server/src/observability/OtelEnvironment.ts index c9e2d6b71121..40ebd48f098f 100644 --- a/apps/server/src/observability/OtelEnvironment.ts +++ b/apps/server/src/observability/OtelEnvironment.ts @@ -43,6 +43,24 @@ export interface OtlpSignalSettings { readonly exportIntervalMs: number | undefined; readonly maxBatchSize: number | undefined; readonly shutdownTimeoutMs: number | undefined; + /** Metrics only. Traces have no aggregation to prefer. */ + readonly temporality: MetricsTemporality | undefined; +} + +/** + * One signal's whole answer from these variables: how to export it, or why it + * is not exported from here. Everything a signal decides lives under it, so a + * caller whose endpoint came from somewhere else drops this one value and + * leaves nothing behind that could reach an export it did not configure. + */ +export interface OtlpSignal { + readonly settings: OtlpSignalSettings | undefined; + /** + * Why a configured endpoint is not being used, if it is not. Carried rather + * than logged here so the caller can report it once, at startup, where a + * user is looking. + */ + readonly declined: string | undefined; } export interface OtlpResourceSettings { @@ -61,16 +79,9 @@ export interface OtelEnvironment { * caller reports them once, at startup, where someone is looking. */ readonly warnings: ReadonlyArray; - readonly traces: OtlpSignalSettings | undefined; - readonly metrics: OtlpSignalSettings | undefined; - readonly metricsTemporality: MetricsTemporality | undefined; + readonly traces: OtlpSignal; + readonly metrics: OtlpSignal; readonly resource: OtlpResourceSettings; - /** - * Why a configured endpoint is not being used, if it is not. Carried rather - * than logged here so the caller can report it once, at startup, where a - * user is looking. - */ - readonly declined: string | undefined; } const optionalString = (name: string) => @@ -180,7 +191,11 @@ const SPEC_DEFAULT_SCHEDULE_DELAY_MS = 5_000; const SPEC_DEFAULT_MAX_EXPORT_BATCH_SIZE = 512; const SPEC_DEFAULT_METRIC_EXPORT_INTERVAL_MS = 60_000; -const signalSettings = (signal: "TRACES" | "METRICS", protocol: OtlpProtocol) => +const signalSettings = ( + signal: "TRACES" | "METRICS", + protocol: OtlpProtocol, + temporality: MetricsTemporality | undefined, +) => Effect.gen(function* () { const url = yield* signalEndpoint(signal); if (url === undefined || !(yield* signalWantsOtlp(signal))) { @@ -210,6 +225,7 @@ const signalSettings = (signal: "TRACES" | "METRICS", protocol: OtlpProtocol) => SPEC_DEFAULT_MAX_EXPORT_BATCH_SIZE) : undefined, shutdownTimeoutMs: timeoutMs, + temporality: signal === "METRICS" ? temporality : undefined, }, warning: specific.warning ?? generic.warning, } satisfies Parsed; @@ -317,10 +333,13 @@ const resolveResource = Effect.gen(function* () { } satisfies Parsed; }); +const UNREADABLE = "the OpenTelemetry environment could not be read"; + /** * Read the environment. Never fails: a variable this server cannot honor - * leaves the corresponding setting unset and is reported through `declined`, - * because an unparseable telemetry knob is not a reason to refuse to start. + * leaves the corresponding setting unset and is reported through the signal's + * `declined`, because an unparseable telemetry knob is not a reason to refuse + * to start. */ export const load: Effect.Effect = Effect.gen(function* () { const disabled = yield* Config.boolean("OTEL_SDK_DISABLED").pipe(Config.withDefault(false)); @@ -329,13 +348,10 @@ export const load: Effect.Effect = Effect.gen(function* () { const temporality = yield* resolveMetricsTemporality; const traces = disabled ? { value: undefined, warning: undefined } - : yield* signalSettings("TRACES", protocolDecision.traces.protocol); + : yield* signalSettings("TRACES", protocolDecision.traces.protocol, undefined); const metrics = disabled ? { value: undefined, warning: undefined } - : yield* signalSettings("METRICS", protocolDecision.metrics.protocol); - const declined = [protocolDecision.traces.declined, protocolDecision.metrics.declined].filter( - (reason) => reason !== undefined, - ); + : yield* signalSettings("METRICS", protocolDecision.metrics.protocol, temporality.value); return { disabled, warnings: [ @@ -345,11 +361,15 @@ export const load: Effect.Effect = Effect.gen(function* () { traces.warning, metrics.warning, ].filter((warning) => warning !== undefined), - traces: protocolDecision.traces.declined === undefined ? traces.value : undefined, - metrics: protocolDecision.metrics.declined === undefined ? metrics.value : undefined, - metricsTemporality: temporality.value, + traces: { + settings: protocolDecision.traces.declined === undefined ? traces.value : undefined, + declined: protocolDecision.traces.declined, + }, + metrics: { + settings: protocolDecision.metrics.declined === undefined ? metrics.value : undefined, + declined: protocolDecision.metrics.declined, + }, resource: resource.value, - declined: declined.length === 0 ? undefined : [...new Set(declined)].join(" "), }; }).pipe( Effect.catchCause((cause) => @@ -357,23 +377,22 @@ export const load: Effect.Effect = Effect.gen(function* () { Effect.as({ disabled: false, warnings: [], - traces: undefined, - metrics: undefined, - metricsTemporality: undefined, + traces: { settings: undefined, declined: UNREADABLE }, + metrics: { settings: undefined, declined: UNREADABLE }, resource: { serviceName: undefined, serviceVersion: undefined, attributes: {} }, - declined: "the OpenTelemetry environment could not be read", }), ), ), ); +/** A signal these variables said nothing usable about. */ +export const noSignal: OtlpSignal = { settings: undefined, declined: undefined }; + /** An environment that asked for nothing, for tests and for the pairing CLI. */ export const none: OtelEnvironment = { disabled: false, warnings: [], - traces: undefined, - metrics: undefined, - metricsTemporality: undefined, + traces: noSignal, + metrics: noSignal, resource: { serviceName: undefined, serviceVersion: undefined, attributes: {} }, - declined: undefined, }; diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index e58faa0b1aeb..5f3e2ebe72c9 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -443,6 +443,7 @@ const buildAppUnderTest = (options?: { otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpExportIntervalMs: 10_000, + otlpMetricsExportIntervalMs: 10_000, otlpServiceName: "t3-server", otelEnvironment: OtelEnvironment.none, mode: "desktop", diff --git a/docs/operations/observability.md b/docs/operations/observability.md index 792b1a4e1795..16fa4069250a 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -193,13 +193,22 @@ T3 Code exports to it, so use `OTEL_SDK_DISABLED=true` if that is not what you w #### Precedence -For each setting, the first one that is present wins: +For each signal, the first source that names its endpoint wins: 1. `T3CODE_OTLP_*` 2. the desktop bootstrap envelope 3. Settings, under `observability` 4. `OTEL_*` +Whichever source wins takes the whole signal, not just the URL. Traces sent to a +`T3CODE_OTLP_TRACES_URL` endpoint keep T3 Code's own wire format, headers, batching, and export +interval even when `OTEL_*` variables are set, because those variables describe the collector they +named rather than this one. `T3CODE_OTLP_EXPORT_INTERVAL_MS` is the exception, and applies to both +signals wherever they go. + +The two signals are resolved separately, so traces can come from one source and metrics from +another. + `OTEL_SDK_DISABLED=true` outranks all four and stops every export, including one configured through Settings. @@ -224,7 +233,8 @@ specification, and stays `http/json` for a `T3CODE_OTLP_*` setup that never ment `OTEL_EXPORTER_OTLP_PROTOCOL=grpc` is refused rather than downgraded, because this server has no gRPC transport and posting an HTTP body to a gRPC endpoint fails in a way that is harder to read than -exporting nothing. The refusal is logged at startup and both signals stay off. +exporting nothing. The refusal is logged at startup and turns off only the signal that named gRPC, +and only when that signal had no other endpoint to go to. Header and resource-attribute values are percent decoded, so `OTEL_EXPORTER_OTLP_HEADERS=Authorization=Bearer%20abc` sends the space and a base64 credential keeps From 3aa225d9547105608faf4247ab21d3dc112f082d Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 20 Aug 2026 23:22:54 -0400 Subject: [PATCH 07/10] fix(server): one unparseable OTEL value no longer costs you every other one A present-but-invalid number failed the read outright rather than falling back, so a typo on a batch delay declined both signals and exported nothing. The boolean reader also accepted values the specification says are false while rejecting the capitalized true it says is true. Signed-off-by: Yordis Prieto --- .../src/observability/OtelEnvironment.test.ts | 54 +++++++++++ .../src/observability/OtelEnvironment.ts | 94 +++++++++++++------ ...the-standard-otel-variables-are-honored.md | 7 +- docs/operations/observability.md | 11 ++- 4 files changed, 134 insertions(+), 32 deletions(-) diff --git a/apps/server/src/observability/OtelEnvironment.test.ts b/apps/server/src/observability/OtelEnvironment.test.ts index d147a902fb18..6f7920ff8624 100644 --- a/apps/server/src/observability/OtelEnvironment.test.ts +++ b/apps/server/src/observability/OtelEnvironment.test.ts @@ -387,6 +387,60 @@ describe("OtelEnvironment", () => { }), ); + it.effect("keeps exporting when a number is not a number", () => + Effect.gen(function* () { + // A typo on one knob must not take the rest of the telemetry with it. + // Before this, the read failed outright and nothing was exported. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_BSP_SCHEDULE_DELAY: "abc", + }), + ); + assert.strictEqual(resolved.traces.settings?.exportIntervalMs, 5000); + assert.strictEqual( + resolved.metrics.settings?.url, + "https://collector.example.com/v1/metrics", + ); + assert.isTrue( + resolved.warnings.some((warning) => warning.includes("OTEL_BSP_SCHEDULE_DELAY")), + ); + }), + ); + + it.effect("reads a boolean the way the specification defines one", () => + Effect.gen(function* () { + // Case insensitive `true` and nothing else. `yes` is affirmative in + // other config systems and false here, which the specification is + // explicit about. + const upper = yield* OtelEnvironment.load.pipe(withEnv({ OTEL_SDK_DISABLED: "True" })); + assert.isTrue(upper.disabled); + + const affirmative = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_SDK_DISABLED: "yes", + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + }), + ); + assert.isFalse(affirmative.disabled); + assert.isDefined(affirmative.traces.settings); + }), + ); + + it.effect("treats an empty value as an unset one", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_SERVICE_NAME: "", + OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: "", + }), + ); + assert.strictEqual(resolved.resource.serviceName, undefined); + assert.strictEqual(resolved.traces.settings?.url, "https://collector.example.com/v1/traces"); + }), + ); + it.effect("reads the metric protocol when it is the only one named", () => Effect.gen(function* () { const resolved = yield* OtelEnvironment.load.pipe( diff --git a/apps/server/src/observability/OtelEnvironment.ts b/apps/server/src/observability/OtelEnvironment.ts index 40ebd48f098f..a04430b624e6 100644 --- a/apps/server/src/observability/OtelEnvironment.ts +++ b/apps/server/src/observability/OtelEnvironment.ts @@ -84,11 +84,49 @@ export interface OtelEnvironment { readonly resource: OtlpResourceSettings; } +/** + * An empty value means the same thing as an unset one. The specification says + * so, and it is how a machine clears a variable it inherited without being + * able to unset it. + */ const optionalString = (name: string) => - Config.string(name).pipe(Config.option, Config.map(Option.getOrUndefined)); + Config.string(name).pipe( + Config.option, + Config.map((value) => { + const raw = Option.getOrUndefined(value); + return raw === undefined || raw.trim() === "" ? undefined : raw; + }), + ); -const optionalInt = (name: string) => - Config.int(name).pipe(Config.option, Config.map(Option.getOrUndefined)); +/** + * The specification defines exactly one true value: the case-insensitive + * string `true`. Everything else is false, including values that read as + * affirmative elsewhere, because implementations are told not to extend the + * list. + */ +const specBoolean = (name: string) => + optionalString(name).pipe(Effect.map((raw) => raw?.trim().toLowerCase() === "true")); + +/** + * A number that is not a number is warned about and dropped, which is what the + * specification asks for anywhere a value is unrecognized. Letting the read + * fail instead would take every other variable down with it and turn one typo + * into no telemetry at all. + */ +const readInt = (name: string, warnings: Array) => + optionalString(name).pipe( + Effect.map((raw) => { + if (raw === undefined) { + return undefined; + } + const value = Number(raw.trim()); + if (!Number.isSafeInteger(value) || value < 0) { + warnings.push(`${name}=${raw} is not a whole number and was ignored`); + return undefined; + } + return value; + }), + ); /** * Headers and resource attributes are a W3C Baggage string: comma separated @@ -123,7 +161,7 @@ const parseBaggage = (raw: string): Readonly> | undefined interface Parsed { readonly value: A | undefined; - readonly warning: string | undefined; + readonly warnings: ReadonlyArray; } /** @@ -136,12 +174,12 @@ const optionalRecord = (name: string) => optionalString(name).pipe( Effect.map((raw) => { if (raw === undefined) { - return { value: undefined, warning: undefined }; + return { value: undefined, warnings: [] }; } const parsed = parseBaggage(raw); return parsed === undefined - ? { value: undefined, warning: `${name} is not valid percent encoding and was ignored` } - : { value: parsed, warning: undefined }; + ? { value: undefined, warnings: [`${name} is not valid percent encoding and was ignored`] } + : { value: parsed, warnings: [] }; }), ); @@ -199,19 +237,20 @@ const signalSettings = ( Effect.gen(function* () { const url = yield* signalEndpoint(signal); if (url === undefined || !(yield* signalWantsOtlp(signal))) { - return { value: undefined, warning: undefined }; + return { value: undefined, warnings: [] }; } + const numbers: Array = []; const specific = yield* optionalRecord(`OTEL_EXPORTER_OTLP_${signal}_HEADERS`); const generic = yield* optionalRecord("OTEL_EXPORTER_OTLP_HEADERS"); const headers = specific.value ?? generic.value; const timeoutMs = - (yield* optionalInt(`OTEL_EXPORTER_OTLP_${signal}_TIMEOUT`)) ?? - (yield* optionalInt("OTEL_EXPORTER_OTLP_TIMEOUT")) ?? - (signal === "METRICS" ? yield* optionalInt("OTEL_METRIC_EXPORT_TIMEOUT") : undefined); + (yield* readInt(`OTEL_EXPORTER_OTLP_${signal}_TIMEOUT`, numbers)) ?? + (yield* readInt("OTEL_EXPORTER_OTLP_TIMEOUT", numbers)) ?? + (signal === "METRICS" ? yield* readInt("OTEL_METRIC_EXPORT_TIMEOUT", numbers) : undefined); const exportIntervalMs = signal === "TRACES" - ? ((yield* optionalInt("OTEL_BSP_SCHEDULE_DELAY")) ?? SPEC_DEFAULT_SCHEDULE_DELAY_MS) - : ((yield* optionalInt("OTEL_METRIC_EXPORT_INTERVAL")) ?? + ? ((yield* readInt("OTEL_BSP_SCHEDULE_DELAY", numbers)) ?? SPEC_DEFAULT_SCHEDULE_DELAY_MS) + : ((yield* readInt("OTEL_METRIC_EXPORT_INTERVAL", numbers)) ?? SPEC_DEFAULT_METRIC_EXPORT_INTERVAL_MS); return { value: { @@ -221,13 +260,13 @@ const signalSettings = ( exportIntervalMs, maxBatchSize: signal === "TRACES" - ? ((yield* optionalInt("OTEL_BSP_MAX_EXPORT_BATCH_SIZE")) ?? + ? ((yield* readInt("OTEL_BSP_MAX_EXPORT_BATCH_SIZE", numbers)) ?? SPEC_DEFAULT_MAX_EXPORT_BATCH_SIZE) : undefined, shutdownTimeoutMs: timeoutMs, temporality: signal === "METRICS" ? temporality : undefined, }, - warning: specific.warning ?? generic.warning, + warnings: [...specific.warnings, ...generic.warnings, ...numbers], } satisfies Parsed; }); @@ -300,18 +339,19 @@ const resolveMetricsTemporality = optionalString( ).pipe( Effect.map((raw): Parsed => { if (raw === undefined) { - return { value: undefined, warning: undefined }; + return { value: undefined, warnings: [] }; } const preference = raw.trim().toLowerCase(); if (preference === "delta" || preference === "cumulative") { - return { value: preference, warning: undefined }; + return { value: preference, warnings: [] }; } return { value: undefined, - warning: + warnings: [ preference === "lowmemory" ? "OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE=lowmemory is not supported here; cumulative is used" : `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE=${raw} is not a known preference and was ignored`, + ], }; }), ); @@ -329,7 +369,7 @@ const resolveResource = Effect.gen(function* () { serviceVersion: (yield* optionalString("OTEL_SERVICE_VERSION")) ?? attributeVersion, attributes: rest, }, - warning: parsed.warning, + warnings: parsed.warnings, } satisfies Parsed; }); @@ -342,25 +382,25 @@ const UNREADABLE = "the OpenTelemetry environment could not be read"; * to start. */ export const load: Effect.Effect = Effect.gen(function* () { - const disabled = yield* Config.boolean("OTEL_SDK_DISABLED").pipe(Config.withDefault(false)); + const disabled = yield* specBoolean("OTEL_SDK_DISABLED"); const protocolDecision = yield* resolveProtocol; const resource = yield* resolveResource; const temporality = yield* resolveMetricsTemporality; const traces = disabled - ? { value: undefined, warning: undefined } + ? { value: undefined, warnings: [] } : yield* signalSettings("TRACES", protocolDecision.traces.protocol, undefined); const metrics = disabled - ? { value: undefined, warning: undefined } + ? { value: undefined, warnings: [] } : yield* signalSettings("METRICS", protocolDecision.metrics.protocol, temporality.value); return { disabled, warnings: [ ...protocolDecision.warnings, - resource.warning, - temporality.warning, - traces.warning, - metrics.warning, - ].filter((warning) => warning !== undefined), + ...resource.warnings, + ...temporality.warnings, + ...traces.warnings, + ...metrics.warnings, + ], traces: { settings: protocolDecision.traces.declined === undefined ? traces.value : undefined, declined: protocolDecision.traces.declined, diff --git a/docs/fork/0018-the-standard-otel-variables-are-honored.md b/docs/fork/0018-the-standard-otel-variables-are-honored.md index 0a42fcba4c7a..5ee42491364b 100644 --- a/docs/fork/0018-the-standard-otel-variables-are-honored.md +++ b/docs/fork/0018-the-standard-otel-variables-are-honored.md @@ -23,9 +23,10 @@ envelope, and Settings all still win over the environment, and a setup that never mentioned OpenTelemetry keeps the wire format it always used. - Find out when a variable did not take. A misspelled protocol, a temporality - this exporter cannot produce, or a header list that is not valid percent - encoding is named in the startup log and then ignored, instead of silently - changing nothing or quietly turning export off. + this exporter cannot produce, a batch size that is not a number, or a header + list that is not valid percent encoding is named in the startup log and then + ignored, instead of silently changing nothing or quietly turning export off. + One bad value costs you that value and nothing else. ## Why diff --git a/docs/operations/observability.md b/docs/operations/observability.md index 16fa4069250a..3dd2d22b3eca 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -277,11 +277,18 @@ limit variables. A variable this server cannot act on never stops it from starting. Two things can happen instead, and both are logged once at startup: -- **A warning, then the default.** A misspelled protocol, an unavailable temporality, or a pair list - that is not valid percent encoding is reported and ignored, and everything else keeps exporting. +- **A warning, then the default.** A misspelled protocol, an unavailable temporality, a timeout or + batch size that is not a whole number, or a pair list that is not valid percent encoding is + reported and ignored, and everything else keeps exporting. One bad value never costs you the + other variables. - **Export off.** Only `OTEL_EXPORTER_OTLP_PROTOCOL=grpc` does this, because it names a transport this server does not speak rather than a value it failed to parse. +An empty value means the same thing as an unset one, so `OTEL_SERVICE_NAME=` reads as if the +variable were not there at all. `OTEL_SDK_DISABLED` follows the specification's one rule for +booleans: the case-insensitive string `true` is the only value that switches export off, and +anything else, including `yes` and `1`, leaves it on. + A `OTEL_EXPORTER_OTLP_HEADERS` or `OTEL_RESOURCE_ATTRIBUTES` value that fails to decode is discarded whole rather than partly. A half-parsed credential reaches the collector as the same authentication error a wrong one would, which reads like a bad token instead of a bad variable. From e122ca4a0ea13ea649a573c54ae90b0e17efb8b6 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 20 Aug 2026 23:41:30 -0400 Subject: [PATCH 08/10] fix(server): a value the exporter cannot act on no longer changes an export A per-request timeout spent on the shutdown flush would hold a restart open for as long as the collector was allowed to be slow, and a header list with no pair in it read as a request for no headers rather than as the malformed value it is. Signed-off-by: Yordis Prieto --- apps/server/src/cli/config.test.ts | 17 +++- apps/server/src/cli/config.ts | 7 +- .../src/observability/Layers/Observability.ts | 14 ++- .../src/observability/OtelEnvironment.test.ts | 98 +++++++++++++++++-- .../src/observability/OtelEnvironment.ts | 40 +++++--- docs/operations/observability.md | 32 +++--- 6 files changed, 158 insertions(+), 50 deletions(-) diff --git a/apps/server/src/cli/config.test.ts b/apps/server/src/cli/config.test.ts index 756f25854584..1e834555c852 100644 --- a/apps/server/src/cli/config.test.ts +++ b/apps/server/src/cli/config.test.ts @@ -1,6 +1,7 @@ // @effect-diagnostics nodeBuiltinImport:off import * as NodeFS from "node:fs"; import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; import { assert, expect, it } from "@effect/vitest"; import * as ConfigProvider from "effect/ConfigProvider"; @@ -491,13 +492,17 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { }), ); - const resolveWithEnv = (env: Record) => - resolveServerConfig( + // Resolving a config reads the settings file and creates the trace + // directory, so a shared home would let one case see another's writes and + // would race when these run in parallel. + const resolveWithEnv = (env: Record) => { + const baseDir = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3code-otel-config-")); + return resolveServerConfig( { mode: Option.some("web"), port: Option.some(4888), host: Option.none(), - baseDir: Option.some("/tmp/t3-otel-home"), + baseDir: Option.some(baseDir), cwd: Option.none(), devUrl: Option.none(), noBrowser: Option.none(), @@ -512,7 +517,13 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { Effect.provide( Layer.mergeAll(ConfigProvider.layer(ConfigProvider.fromEnv({ env })), NetService.layer), ), + Effect.ensuring( + Effect.sync(() => { + NodeFS.rmSync(baseDir, { recursive: true, force: true }); + }), + ), ); + }; it.effect("exports to the endpoint the rest of the machine already uses", () => Effect.gen(function* () { diff --git a/apps/server/src/cli/config.ts b/apps/server/src/cli/config.ts index 2f92100ee5fa..eb0739fe9d88 100644 --- a/apps/server/src/cli/config.ts +++ b/apps/server/src/cli/config.ts @@ -386,9 +386,10 @@ export const resolveServerConfig = ( otlpMetricsUrl: otelEnvironment.disabled ? undefined : (namedMetricsUrl ?? otelEnvironment.metrics.settings?.url), - // Each signal gets its own, because the environment names them - // separately and a signal that took its endpoint elsewhere must not - // inherit the other one's schedule. + // T3 Code has one interval variable and it deliberately covers both + // signals. The per-signal part is the fallback under it: the environment + // names a trace delay and a metric interval separately, so a signal that + // took its endpoint elsewhere must not inherit the other one's. otlpExportIntervalMs: env.otlpExportIntervalMs ?? otelEnvironment.traces.settings?.exportIntervalMs ?? 10_000, otlpMetricsExportIntervalMs: diff --git a/apps/server/src/observability/Layers/Observability.ts b/apps/server/src/observability/Layers/Observability.ts index ca7b2b298f10..4e5e2cfad644 100644 --- a/apps/server/src/observability/Layers/Observability.ts +++ b/apps/server/src/observability/Layers/Observability.ts @@ -24,6 +24,12 @@ export const ObservabilityLive = Layer.unwrap( yield* Effect.logWarning(warning); } + if (otel.disabled) { + yield* Effect.logWarning( + "OTEL_SDK_DISABLED is set, so no telemetry is exported; this overrides T3CODE_OTLP_* and Settings too", + ); + } + // One variable can decline both signals, and saying so twice reads like // two separate problems. const declined = new Set( @@ -98,11 +104,6 @@ export const ObservabilityLive = Layer.unwrap( ...(otel.traces.settings?.maxBatchSize === undefined ? {} : { maxBatchSize: otel.traces.settings.maxBatchSize }), - ...(otel.traces.settings?.shutdownTimeoutMs === undefined - ? {} - : { - shutdownTimeout: `${otel.traces.settings.shutdownTimeoutMs} millis` as const, - }), }); const tracer = yield* makeLocalFileTracer({ @@ -134,9 +135,6 @@ export const ObservabilityLive = Layer.unwrap( ...(otel.metrics.settings?.headers === undefined ? {} : { headers: otel.metrics.settings.headers }), - ...(otel.metrics.settings?.shutdownTimeoutMs === undefined - ? {} - : { shutdownTimeout: `${otel.metrics.settings.shutdownTimeoutMs} millis` as const }), ...(otel.metrics.settings?.temporality === undefined ? {} : { temporality: otel.metrics.settings.temporality }), diff --git a/apps/server/src/observability/OtelEnvironment.test.ts b/apps/server/src/observability/OtelEnvironment.test.ts index 6f7920ff8624..361bdde75d06 100644 --- a/apps/server/src/observability/OtelEnvironment.test.ts +++ b/apps/server/src/observability/OtelEnvironment.test.ts @@ -211,24 +211,47 @@ describe("OtelEnvironment", () => { }), ); - it.effect("takes the batch and timeout knobs the exporter can act on", () => + it.effect("takes the batch knobs the exporter can act on", () => Effect.gen(function* () { const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", OTEL_BSP_SCHEDULE_DELAY: "2500", OTEL_BSP_MAX_EXPORT_BATCH_SIZE: "128", - OTEL_EXPORTER_OTLP_TIMEOUT: "7000", OTEL_METRIC_EXPORT_INTERVAL: "15000", }), ); assert.strictEqual(resolved.traces.settings?.exportIntervalMs, 2500); assert.strictEqual(resolved.traces.settings?.maxBatchSize, 128); - assert.strictEqual(resolved.traces.settings?.shutdownTimeoutMs, 7000); assert.strictEqual(resolved.metrics.settings?.exportIntervalMs, 15000); }), ); + it.effect("leaves the request timeouts alone rather than spending them on shutdown", () => + Effect.gen(function* () { + // These name a per-request deadline and the exporter has no such knob. + // Bounding the final flush with them instead would hold a restart open + // for as long as the collector was allowed to be slow. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_TIMEOUT: "600000", + OTEL_EXPORTER_OTLP_TRACES_TIMEOUT: "600000", + OTEL_METRIC_EXPORT_TIMEOUT: "600000", + }), + ); + assert.deepStrictEqual(Object.keys(resolved.traces.settings ?? {}).sort(), [ + "exportIntervalMs", + "headers", + "maxBatchSize", + "protocol", + "temporality", + "url", + ]); + assert.deepStrictEqual(resolved.warnings, []); + }), + ); + it.effect("falls back to the specification's own batching defaults", () => Effect.gen(function* () { // Once this route is the one configuring the exporter, the numbers that @@ -242,17 +265,16 @@ describe("OtelEnvironment", () => { }), ); - it.effect("lets the metric signal name its own timeout and temporality", () => + it.effect("lets the metric signal name its own aggregation", () => Effect.gen(function* () { const resolved = yield* OtelEnvironment.load.pipe( withEnv({ OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", - OTEL_METRIC_EXPORT_TIMEOUT: "9000", OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: "delta", }), ); - assert.strictEqual(resolved.metrics.settings?.shutdownTimeoutMs, 9000); assert.strictEqual(resolved.metrics.settings?.temporality, "delta"); + assert.strictEqual(resolved.traces.settings?.temporality, undefined); }), ); @@ -441,6 +463,70 @@ describe("OtelEnvironment", () => { }), ); + it.effect("falls back to the generic headers when the signal's own list is junk", () => + Effect.gen(function* () { + // A list with no pair in it is malformed, not a request for no headers. + // Reading it as an answer would shadow the generic variable and send an + // unauthenticated stream to a collector that was told how to authorize. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_TRACES_HEADERS: "junk", + OTEL_EXPORTER_OTLP_HEADERS: "Authorization=Bearer%20abc123", + }), + ); + assert.deepStrictEqual(resolved.traces.settings?.headers, { + Authorization: "Bearer abc123", + }); + assert.isTrue( + resolved.warnings.some((warning) => warning.includes("OTEL_EXPORTER_OTLP_TRACES_HEADERS")), + ); + }), + ); + + it.effect("names a shared variable once even though both signals read it", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_HEADERS: "x-token=100%zz", + }), + ); + assert.strictEqual( + resolved.warnings.filter((warning) => warning.includes("OTEL_EXPORTER_OTLP_HEADERS")) + .length, + 1, + ); + }), + ); + + it.effect("does not blame gRPC for a signal that was never going to export", () => + Effect.gen(function* () { + // Nothing named an endpoint, so the protocol is beside the point and + // reporting it would send someone looking for a collector problem. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ OTEL_EXPORTER_OTLP_PROTOCOL: "grpc" }), + ); + assert.strictEqual(resolved.traces.declined, undefined); + assert.strictEqual(resolved.metrics.declined, undefined); + }), + ); + + it.effect("stays quiet about the protocol once the SDK is off", () => + Effect.gen(function* () { + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_SDK_DISABLED: "true", + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_EXPORTER_OTLP_PROTOCOL: "grpc", + }), + ); + assert.isTrue(resolved.disabled); + assert.strictEqual(resolved.traces.declined, undefined); + assert.strictEqual(resolved.metrics.declined, undefined); + }), + ); + it.effect("reads the metric protocol when it is the only one named", () => Effect.gen(function* () { const resolved = yield* OtelEnvironment.load.pipe( diff --git a/apps/server/src/observability/OtelEnvironment.ts b/apps/server/src/observability/OtelEnvironment.ts index a04430b624e6..41c8badf1113 100644 --- a/apps/server/src/observability/OtelEnvironment.ts +++ b/apps/server/src/observability/OtelEnvironment.ts @@ -42,7 +42,6 @@ export interface OtlpSignalSettings { readonly headers: Readonly> | undefined; readonly exportIntervalMs: number | undefined; readonly maxBatchSize: number | undefined; - readonly shutdownTimeoutMs: number | undefined; /** Metrics only. Traces have no aggregation to prefer. */ readonly temporality: MetricsTemporality | undefined; } @@ -156,7 +155,11 @@ const parseBaggage = (raw: string): Readonly> | undefined return undefined; } } - return entries; + // A value that produced no pair at all is a malformed list, not a request + // for no headers. Returning `{}` here would count as a supplied value and + // silently shadow the generic variable the signal should have fallen back + // to. + return Object.keys(entries).length === 0 ? undefined : entries; }; interface Parsed { @@ -178,7 +181,10 @@ const optionalRecord = (name: string) => } const parsed = parseBaggage(raw); return parsed === undefined - ? { value: undefined, warnings: [`${name} is not valid percent encoding and was ignored`] } + ? { + value: undefined, + warnings: [`${name} is not a valid list of key=value pairs and was ignored`], + } : { value: parsed, warnings: [] }; }), ); @@ -243,10 +249,6 @@ const signalSettings = ( const specific = yield* optionalRecord(`OTEL_EXPORTER_OTLP_${signal}_HEADERS`); const generic = yield* optionalRecord("OTEL_EXPORTER_OTLP_HEADERS"); const headers = specific.value ?? generic.value; - const timeoutMs = - (yield* readInt(`OTEL_EXPORTER_OTLP_${signal}_TIMEOUT`, numbers)) ?? - (yield* readInt("OTEL_EXPORTER_OTLP_TIMEOUT", numbers)) ?? - (signal === "METRICS" ? yield* readInt("OTEL_METRIC_EXPORT_TIMEOUT", numbers) : undefined); const exportIntervalMs = signal === "TRACES" ? ((yield* readInt("OTEL_BSP_SCHEDULE_DELAY", numbers)) ?? SPEC_DEFAULT_SCHEDULE_DELAY_MS) @@ -263,7 +265,6 @@ const signalSettings = ( ? ((yield* readInt("OTEL_BSP_MAX_EXPORT_BATCH_SIZE", numbers)) ?? SPEC_DEFAULT_MAX_EXPORT_BATCH_SIZE) : undefined, - shutdownTimeoutMs: timeoutMs, temporality: signal === "METRICS" ? temporality : undefined, }, warnings: [...specific.warnings, ...generic.warnings, ...numbers], @@ -394,20 +395,29 @@ export const load: Effect.Effect = Effect.gen(function* () { : yield* signalSettings("METRICS", protocolDecision.metrics.protocol, temporality.value); return { disabled, + // Both signals read the generic `OTEL_EXPORTER_OTLP_*` variables, so one + // bad value arrives here twice and would be logged twice. warnings: [ - ...protocolDecision.warnings, - ...resource.warnings, - ...temporality.warnings, - ...traces.warnings, - ...metrics.warnings, + ...new Set([ + ...protocolDecision.warnings, + ...resource.warnings, + ...temporality.warnings, + ...traces.warnings, + ...metrics.warnings, + ]), ], + // `value` is set only for a signal that resolved an endpoint and asked for + // OTLP, so it is also the test for whether a decline is worth reporting. A + // signal nothing pointed anywhere, one switched off by name, and every + // signal once the SDK is disabled were never going to export, and saying + // gRPC is why would name the wrong cause. traces: { settings: protocolDecision.traces.declined === undefined ? traces.value : undefined, - declined: protocolDecision.traces.declined, + declined: traces.value === undefined ? undefined : protocolDecision.traces.declined, }, metrics: { settings: protocolDecision.metrics.declined === undefined ? metrics.value : undefined, - declined: protocolDecision.metrics.declined, + declined: metrics.value === undefined ? undefined : protocolDecision.metrics.declined, }, resource: resource.value, }; diff --git a/docs/operations/observability.md b/docs/operations/observability.md index 3dd2d22b3eca..cd0c74e87f8a 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -214,19 +214,18 @@ Settings. #### What Is Read -| Variable | Effect | -| --------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------- | -| `OTEL_SDK_DISABLED` | Stops all export | -| `OTEL_EXPORTER_OTLP_ENDPOINT` | Base URL for both signals | -| `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_ENDPOINT` | Full URL for one signal | -| `OTEL_EXPORTER_OTLP_HEADERS`, `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_HEADERS` | Export headers, per signal overriding the shared ones | -| `OTEL_EXPORTER_OTLP_PROTOCOL`, `OTEL_EXPORTER_OTLP_TRACES_PROTOCOL` | `http/protobuf` (default) or `http/json` | -| `OTEL_{TRACES,METRICS}_EXPORTER` | A list; the signal is exported when it contains `otlp`, which is the default | -| `OTEL_SERVICE_NAME`, `OTEL_SERVICE_VERSION`, `OTEL_RESOURCE_ATTRIBUTES` | Resource identity attached to every span and metric | -| `OTEL_EXPORTER_OTLP_TIMEOUT`, `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_TIMEOUT`, `OTEL_METRIC_EXPORT_TIMEOUT` | Shutdown flush timeout | -| `OTEL_BSP_SCHEDULE_DELAY`, `OTEL_METRIC_EXPORT_INTERVAL` | Export interval | -| `OTEL_BSP_MAX_EXPORT_BATCH_SIZE` | Spans per batch | -| `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` | `cumulative` or `delta` | +| Variable | Effect | +| ----------------------------------------------------------------------------- | ---------------------------------------------------------------------------- | +| `OTEL_SDK_DISABLED` | Stops all export | +| `OTEL_EXPORTER_OTLP_ENDPOINT` | Base URL for both signals | +| `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_ENDPOINT` | Full URL for one signal | +| `OTEL_EXPORTER_OTLP_HEADERS`, `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_HEADERS` | Export headers, per signal overriding the shared ones | +| `OTEL_EXPORTER_OTLP_PROTOCOL`, `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_PROTOCOL` | `http/protobuf` (default) or `http/json` | +| `OTEL_{TRACES,METRICS}_EXPORTER` | A list; the signal is exported when it contains `otlp`, which is the default | +| `OTEL_SERVICE_NAME`, `OTEL_SERVICE_VERSION`, `OTEL_RESOURCE_ATTRIBUTES` | Resource identity attached to every span and metric | +| `OTEL_BSP_SCHEDULE_DELAY`, `OTEL_METRIC_EXPORT_INTERVAL` | Export interval | +| `OTEL_BSP_MAX_EXPORT_BATCH_SIZE` | Spans per batch | +| `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` | `cumulative` or `delta` | The wire format defaults to `http/protobuf` when the endpoint came from `OTEL_*`, matching the specification, and stays `http/json` for a `T3CODE_OTLP_*` setup that never mentioned a protocol. @@ -252,8 +251,11 @@ Not everything in the specification is implemented. These are the ones worth kno `OTEL_EXPORTER_OTLP_CERTIFICATE`, `OTEL_EXPORTER_OTLP_CLIENT_KEY`, and `OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE` are ignored. A collector that requires mutual TLS needs a proxy in front of it. -- **Timeouts flush at shutdown.** The specification's `OTEL_EXPORTER_OTLP_TIMEOUT` is a per-request - deadline. The exporter here has no per-request knob, so the value bounds the final flush instead. +- **No export timeouts.** `OTEL_EXPORTER_OTLP_TIMEOUT`, + `OTEL_EXPORTER_OTLP_{TRACES,METRICS}_TIMEOUT`, and `OTEL_METRIC_EXPORT_TIMEOUT` are per-request + deadlines, and this exporter has no per-request knob, so they are ignored. Spending them on the + shutdown flush instead would be the wrong meaning and would let a generous collector timeout hold + the server open on every restart. - **Browser traces are always JSON.** The proxy that forwards traces from the client posts OTLP/HTTP JSON regardless of `OTEL_EXPORTER_OTLP_PROTOCOL`. Both are valid OTLP/HTTP and most collectors accept either, so this only matters against one that takes protobuf and nothing else. From 8e76646916a575a66c2599ef1c89adfc38705ede Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 21 Aug 2026 00:06:40 -0400 Subject: [PATCH 09/10] fix(server): an empty T3 Code name no longer outranks the endpoint the machine already has A variable that is set to nothing is not an answer, and taking it as one both publishes an endpoint nothing can reach and hides the ambient one that would have worked. Signed-off-by: Yordis Prieto --- apps/server/src/cli/config.test.ts | 19 +++++++++++++++++++ apps/server/src/cli/config.ts | 26 ++++++++++++++++++++------ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/apps/server/src/cli/config.test.ts b/apps/server/src/cli/config.test.ts index 1e834555c852..9298f98e7fb0 100644 --- a/apps/server/src/cli/config.test.ts +++ b/apps/server/src/cli/config.test.ts @@ -538,6 +538,25 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { }), ); + it.effect("does not let an empty T3 Code name stand in for an answer", () => + Effect.gen(function* () { + // An empty variable is set without saying anything. Reading it as an + // answer would publish an endpoint nothing can reach and would suppress + // the ambient one that could have been used instead. + const resolved = yield* resolveWithEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com", + OTEL_SERVICE_NAME: "t3", + T3CODE_OTLP_TRACES_URL: "", + T3CODE_OTLP_METRICS_URL: " ", + T3CODE_OTLP_SERVICE_NAME: "", + }); + + expect(resolved.otlpTracesUrl).toBe("https://collector.example.com/v1/traces"); + expect(resolved.otlpMetricsUrl).toBe("https://collector.example.com/v1/metrics"); + expect(resolved.otlpServiceName).toBe("t3"); + }), + ); + it.effect("keeps T3 Code's own names as the explicit answer", () => Effect.gen(function* () { const resolved = yield* resolveWithEnv({ diff --git a/apps/server/src/cli/config.ts b/apps/server/src/cli/config.ts index eb0739fe9d88..91c7deb54a6b 100644 --- a/apps/server/src/cli/config.ts +++ b/apps/server/src/cli/config.ts @@ -201,6 +201,17 @@ const resolveOptionPrecedence = ( ...values: ReadonlyArray> ): Option.Option => Option.firstSomeOf(values); +/** + * Reads a source that names an OTLP destination, treating a blank one as + * nobody having named it. An empty variable is set in the environment but is + * not an answer, and taking it as one both publishes an endpoint that cannot + * be reached and suppresses the ambient variable that could have been. + */ +const named = (value: string | undefined) => { + const trimmed = value?.trim(); + return trimmed === undefined || trimmed === "" ? undefined : trimmed; +}; + const loadPersistedObservabilitySettings = Effect.fn(function* (settingsPath: string) { const fs = yield* FileSystem.FileSystem; const exists = yield* fs.exists(settingsPath).pipe(Effect.orElseSucceed(() => false)); @@ -361,12 +372,14 @@ export const resolveServerConfig = ( // wire format, headers, batching, or aggregation of an export that a // T3CODE_OTLP_* name or Settings already answered, and stops startup from // reporting that signal as declined while it is exporting. - const namedTracesUrl = - env.otlpTracesUrl ?? bootstrap?.otlpTracesUrl ?? persistedObservabilitySettings.otlpTracesUrl; - const namedMetricsUrl = + const namedTracesUrl = named( + env.otlpTracesUrl ?? bootstrap?.otlpTracesUrl ?? persistedObservabilitySettings.otlpTracesUrl, + ); + const namedMetricsUrl = named( env.otlpMetricsUrl ?? - bootstrap?.otlpMetricsUrl ?? - persistedObservabilitySettings.otlpMetricsUrl; + bootstrap?.otlpMetricsUrl ?? + persistedObservabilitySettings.otlpMetricsUrl, + ); const otelEnvironment = { ...otel, traces: namedTracesUrl === undefined ? otel.traces : OtelEnvironment.noSignal, @@ -394,7 +407,8 @@ export const resolveServerConfig = ( env.otlpExportIntervalMs ?? otelEnvironment.traces.settings?.exportIntervalMs ?? 10_000, otlpMetricsExportIntervalMs: env.otlpExportIntervalMs ?? otelEnvironment.metrics.settings?.exportIntervalMs ?? 10_000, - otlpServiceName: env.otlpServiceName ?? otelEnvironment.resource.serviceName ?? "t3-server", + otlpServiceName: + named(env.otlpServiceName) ?? otelEnvironment.resource.serviceName ?? "t3-server", otelEnvironment, mode, port, From d8312bc80887c0e2577dc87571af7edc8afe36e4 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Fri, 21 Aug 2026 00:14:47 -0400 Subject: [PATCH 10/10] fix(server): a padded OTEL value no longer lands in the middle of a URL Whitespace around an inherited variable is formatting, not part of the endpoint, and appending the signal path buries it where nothing would report it. Signed-off-by: Yordis Prieto --- .../src/observability/OtelEnvironment.test.ts | 16 ++++++++++++++++ apps/server/src/observability/OtelEnvironment.ts | 12 +++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/apps/server/src/observability/OtelEnvironment.test.ts b/apps/server/src/observability/OtelEnvironment.test.ts index 361bdde75d06..2e4c2baf98a3 100644 --- a/apps/server/src/observability/OtelEnvironment.test.ts +++ b/apps/server/src/observability/OtelEnvironment.test.ts @@ -527,6 +527,22 @@ describe("OtelEnvironment", () => { }), ); + it.effect("does not carry a padded variable into the URL it builds", () => + Effect.gen(function* () { + // A shell profile that lined up its exports did not mean the padding to + // become part of the endpoint, and the appended signal path would put it + // in the middle of the URL where nothing would report it. + const resolved = yield* OtelEnvironment.load.pipe( + withEnv({ + OTEL_EXPORTER_OTLP_ENDPOINT: " https://collector.example.com/ ", + OTEL_SERVICE_NAME: " t3 ", + }), + ); + assert.strictEqual(resolved.traces.settings?.url, "https://collector.example.com/v1/traces"); + assert.strictEqual(resolved.resource.serviceName, "t3"); + }), + ); + it.effect("reads the metric protocol when it is the only one named", () => Effect.gen(function* () { const resolved = yield* OtelEnvironment.load.pipe( diff --git a/apps/server/src/observability/OtelEnvironment.ts b/apps/server/src/observability/OtelEnvironment.ts index 41c8badf1113..6f8b9de3d647 100644 --- a/apps/server/src/observability/OtelEnvironment.ts +++ b/apps/server/src/observability/OtelEnvironment.ts @@ -86,14 +86,16 @@ export interface OtelEnvironment { /** * An empty value means the same thing as an unset one. The specification says * so, and it is how a machine clears a variable it inherited without being - * able to unset it. + * able to unset it. Surrounding whitespace is dropped for the same reason a + * blank value is: a shell profile that padded a line did not mean the padding + * to become part of an endpoint or a service name. */ const optionalString = (name: string) => Config.string(name).pipe( Config.option, Config.map((value) => { - const raw = Option.getOrUndefined(value); - return raw === undefined || raw.trim() === "" ? undefined : raw; + const raw = Option.getOrUndefined(value)?.trim(); + return raw === undefined || raw === "" ? undefined : raw; }), ); @@ -104,7 +106,7 @@ const optionalString = (name: string) => * list. */ const specBoolean = (name: string) => - optionalString(name).pipe(Effect.map((raw) => raw?.trim().toLowerCase() === "true")); + optionalString(name).pipe(Effect.map((raw) => raw?.toLowerCase() === "true")); /** * A number that is not a number is warned about and dropped, which is what the @@ -118,7 +120,7 @@ const readInt = (name: string, warnings: Array) => if (raw === undefined) { return undefined; } - const value = Number(raw.trim()); + const value = Number(raw); if (!Number.isSafeInteger(value) || value < 0) { warnings.push(`${name}=${raw} is not a whole number and was ignored`); return undefined;