From b04fb903d9fb74cb687846d4cab91a1a189937d3 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 4 Aug 2026 02:13:55 -0700 Subject: [PATCH 1/2] Add settings and env surface for opt-in OTEL export Resolves CL-5175: otel-config.ts parses and validates endpoint, headers, service name, and resource attributes from settings + env; fails closed on invalid config with a stable OTEL_CONFIG_INVALID error. otelConfigForDump exposes header names only so secrets never reach privacy-strict dumps. Docs cover Phoenix, PostHog OTEL, and generic collectors. The actual OTLP transport is a follow-up (CL-5173). --- AGENTS.md | 1 + docs/PERFTRACE.md | 161 ++++++++++++++++ docs/TELEMETRY.md | 7 + src/config/settings.ts | 19 ++ src/perf/index.ts | 18 ++ src/perf/otel-config.test.ts | 293 ++++++++++++++++++++++++++++ src/perf/otel-config.ts | 359 +++++++++++++++++++++++++++++++++++ 7 files changed, 858 insertions(+) create mode 100644 docs/PERFTRACE.md create mode 100644 src/perf/otel-config.test.ts create mode 100644 src/perf/otel-config.ts diff --git a/AGENTS.md b/AGENTS.md index c216776c8..af891fd64 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,3 +71,4 @@ Interchange is the standard library for this repo, consumed as published `@intx/ - `docs/MCP.md` — connecting MCP servers - `docs/PLUGINS.md` — plugin manifest system and discovery - `docs/TELEMETRY.md` — what usage telemetry is collected and why +- `docs/PERFTRACE.md` — local PerfTrace and opt-in OTEL export settings diff --git a/docs/PERFTRACE.md b/docs/PERFTRACE.md new file mode 100644 index 000000000..fafda5d1f --- /dev/null +++ b/docs/PERFTRACE.md @@ -0,0 +1,161 @@ +# Performance tracing (PerfTrace) and OTEL export + +Corbits Code measures session performance with an always-on local tracer +(`src/perf/`). Optional OpenTelemetry export sends the same span tree to **your** +collector. This is separate from product analytics — see `docs/TELEMETRY.md` for +PostHog usage events. + +## Local sink (always on) + +- In-process ring buffer of phase spans (turn, inference, tools, …) +- Privacy-strict tags: enums, ids, and numbers only — no prompts, paths, tool + args, free-text errors, or credentials +- Future session dumps (CL-5169) use the same allowlist and must never include + OTEL auth headers + +Local measurement does not require any settings or env vars. + +## OTEL export (opt-in) + +Export is **off** until an OTLP endpoint is configured. When enabled, traces go +to the operator-owned backend you point at — not Corbits product analytics. + +The settings/env surface is implemented now (`src/perf/otel-config.ts`). The +actual OTLP transport lands in a follow-up (CL-5173). Invalid config fails +closed with a stable error code `OTEL_CONFIG_INVALID` and does not half-enable +export. + +### Configuration + +**Env vars (preferred for secrets; match OTEL conventions):** + +| Variable | Meaning | +|---|---| +| `OTEL_EXPORTER_OTLP_ENDPOINT` | OTLP base URL (`http` or `https` only) | +| `OTEL_EXPORTER_OTLP_HEADERS` | Comma-separated `key=value` headers (values may be percent-encoded) | +| `OTEL_SERVICE_NAME` | Resource `service.name` (default: `corbits-code`) | +| `OTEL_RESOURCE_ATTRIBUTES` | Comma-separated `key=value` resource attributes | + +**Global settings** (`~/.corbits/settings.json`), optional `otel` block: + +```json +{ + "otel": { + "enabled": true, + "endpoint": "https://collector.example/v1", + "headers": { "Authorization": "Bearer …" }, + "serviceName": "corbits-code", + "resourceAttributes": { + "deployment.environment": "dev" + } + } +} +``` + +Precedence: + +- **endpoint:** env overrides settings +- **headers:** when `OTEL_EXPORTER_OTLP_HEADERS` is set, it fully replaces + settings headers (prefer env so secrets stay out of the settings file) +- **serviceName:** env > settings > `corbits-code` +- **resourceAttributes:** settings merged with env; env wins on key conflict +- **`otel.enabled: false`:** disables export when only settings provide an + endpoint; an explicit env endpoint still enables export + +Do not put credentials in the endpoint URL (`https://user:pass@…` is rejected). +Use headers instead. + +### Fail closed + +Any of the following yields `OTEL_CONFIG_INVALID` and must not start export: + +- Endpoint that is not a valid `http`/`https` URL +- Credentials embedded in the endpoint URL +- Headers (settings or env) without an endpoint +- `otel.enabled: true` without an endpoint +- Malformed `key=value` lists for headers or resource attributes + +No endpoint and no half-config → export stays disabled (not an error). + +### Secrets and dumps + +- Header **values** are secrets. Prefer env for them. +- `otelConfigForDump()` exposes only: enabled flag, endpoint, service name, + resource attributes, and header **names** — never values. +- Local privacy-strict dump writers must call `otelConfigForDump` (or omit OTEL + config entirely). Never serialize `OtelExportConfig.headers` into session + artifacts, logs, or crash dumps. + +### Targeting common collectors + +Examples assume the OTLP HTTP base URL your collector documents. Paths such as +`/v1/traces` are appended by the exporter (CL-5173), not by this settings layer. + +#### Arize Phoenix + +Local Phoenix typically listens for OTLP HTTP on port 6006: + +```bash +export OTEL_EXPORTER_OTLP_ENDPOINT="http://127.0.0.1:6006" +export OTEL_SERVICE_NAME="corbits-code" +``` + +Cloud / authenticated Phoenix: set the project endpoint and pass the API key as +a header (exact header name follows Phoenix’s current docs): + +```bash +export OTEL_EXPORTER_OTLP_ENDPOINT="https://app.phoenix.arize.com/v1/traces" +export OTEL_EXPORTER_OTLP_HEADERS="Authorization=Bearer%20" +export OTEL_SERVICE_NAME="corbits-code" +``` + +#### PostHog OTEL + +PostHog can ingest OTLP independently of Corbits product telemetry. Use your +project’s OTEL endpoint and project API key as documented by PostHog: + +```bash +export OTEL_EXPORTER_OTLP_ENDPOINT="https://us.i.posthog.com/i/v0/otlp" +export OTEL_EXPORTER_OTLP_HEADERS="Authorization=Bearer%20" +export OTEL_SERVICE_NAME="corbits-code" +``` + +This does **not** expand the three PostHog product events in `docs/TELEMETRY.md`. +Product analytics opt-out (`CORBITS_TELEMETRY`, `DO_NOT_TRACK`, settings) does +not control OTEL export, and vice versa. + +#### Generic OTLP collector (Jaeger, Grafana Alloy, otel-collector, …) + +Point at any OTLP-compatible base URL: + +```bash +export OTEL_EXPORTER_OTLP_ENDPOINT="http://localhost:4318" +export OTEL_SERVICE_NAME="corbits-code" +export OTEL_RESOURCE_ATTRIBUTES="deployment.environment=local,service.namespace=dev" +``` + +Or in settings without secrets: + +```json +{ + "otel": { + "endpoint": "http://localhost:4318", + "serviceName": "corbits-code", + "resourceAttributes": { + "deployment.environment": "local" + } + } +} +``` + +Then supply auth only via env when needed. + +## Relationship to product telemetry + +| Pipe | Purpose | Default | Content | +|---|---|---|---| +| PostHog (`docs/TELEMETRY.md`) | Aggregate product usage | Opt-out | Three allowlisted events | +| Local PerfTrace | Operator/dev attribution | Always on | Privacy-strict phase spans | +| OTEL export | Your APM / Phoenix / collector | Opt-in | Full span tree when enabled | + +Do not enlarge the PostHog event schema for performance diagnostics. diff --git a/docs/TELEMETRY.md b/docs/TELEMETRY.md index efc269460..7e86f2779 100644 --- a/docs/TELEMETRY.md +++ b/docs/TELEMETRY.md @@ -86,3 +86,10 @@ it. Events are sent to PostHog. PostHog derives an approximate country from the request IP server-side; the client sends no location data itself. No self-hosted or third-party analytics beyond PostHog are used. + +## Not this document + +Local performance tracing and optional OpenTelemetry export to an operator-owned +collector (Phoenix, PostHog OTEL, Jaeger, generic OTLP) are documented in +`docs/PERFTRACE.md`. That pipe is separate: it does not expand these three +events, and product telemetry opt-out does not control OTEL export. diff --git a/src/config/settings.ts b/src/config/settings.ts index 147d8be4e..3629582c3 100644 --- a/src/config/settings.ts +++ b/src/config/settings.ts @@ -123,6 +123,16 @@ export type Settings = { installationId?: string; noticeShown?: boolean; }; + // Opt-in OTEL export (operator-owned collector). Separate from PostHog product + // telemetry. Prefer OTEL_* env vars for secrets; see docs/PERFTRACE.md. + // Local PerfTrace remains always-on regardless of this block. + otel?: { + enabled?: boolean; + endpoint?: string; + headers?: Record; + serviceName?: string; + resourceAttributes?: Record; + }; }; // Maps the settings shell block to the shape the shell-guard plugin expects. @@ -384,6 +394,13 @@ const SettingsSchema = type({ "installationId?": "string", "noticeShown?": "boolean", }), + "otel?": type({ + "enabled?": "boolean", + "endpoint?": "string", + "headers?": "Record", + "serviceName?": "string", + "resourceAttributes?": "Record", + }), }); // Per-entry MCP shape without the name key. The "exactly one transport" rule is @@ -543,6 +560,7 @@ export const GLOBAL_SETTINGS_OPTIONAL_KEYS = [ "shell", "tools", "telemetry", + "otel", ] as const satisfies readonly (keyof OptionalSettingsFields)[]; /** Optional local settings keys the load path is required to consider. */ @@ -615,6 +633,7 @@ export async function loadSettings(path: string): Promise { shell: s.shell as Settings["shell"] | undefined, tools: s.tools as Settings["tools"] | undefined, telemetry: s.telemetry as Settings["telemetry"] | undefined, + otel: s.otel as Settings["otel"] | undefined, }; return { providers: s.providers as Settings["providers"], diff --git a/src/perf/index.ts b/src/perf/index.ts index 0bd5f62b1..81a43fe19 100644 --- a/src/perf/index.ts +++ b/src/perf/index.ts @@ -22,6 +22,24 @@ export { type TransportKind, } from "./sanitize.js"; +export { + DEFAULT_OTEL_SERVICE_NAME, + OTEL_CONFIG_INVALID, + OTEL_ENV, + OtelConfigError, + isOtelConfigInvalid, + otelConfigForDump, + parseOtelKeyValueList, + requireOtelExportConfig, + resolveOtelExportConfig, + type DisabledOtelExportConfig, + type EnabledOtelExportConfig, + type OtelConfigResolution, + type OtelExportConfig, + type OtelExportConfigDumpView, + type OtelSettings, +} from "./otel-config.js"; + /** Core + adapter phase names. Adapters extend; they do not invent new sinks. */ export const SPAN_NAMES = [ "session", diff --git a/src/perf/otel-config.test.ts b/src/perf/otel-config.test.ts new file mode 100644 index 000000000..4dcaf7575 --- /dev/null +++ b/src/perf/otel-config.test.ts @@ -0,0 +1,293 @@ +import { describe, test, expect } from "bun:test"; + +import type { Settings } from "../config/settings.js"; +import { + DEFAULT_OTEL_SERVICE_NAME, + OTEL_CONFIG_INVALID, + OTEL_ENV, + OtelConfigError, + isOtelConfigInvalid, + otelConfigForDump, + parseOtelKeyValueList, + requireOtelExportConfig, + resolveOtelExportConfig, +} from "./otel-config.js"; + +const baseSettings = (otel?: Settings["otel"]): Settings => ({ + providers: {}, + ...(otel !== undefined ? { otel } : {}), +}); + +describe("parseOtelKeyValueList", () => { + test("parses key=value pairs", () => { + const result = parseOtelKeyValueList("Authorization=Bearer%20tok,x-api-key=abc", "headers"); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.value).toEqual({ + Authorization: "Bearer tok", + "x-api-key": "abc", + }); + } + }); + + test("rejects malformed entries", () => { + const result = parseOtelKeyValueList("noequals", "headers"); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.message).toContain("expected key=value"); + } + }); + + test("empty string yields empty map", () => { + const result = parseOtelKeyValueList("", "headers"); + expect(result.ok).toBe(true); + if (result.ok) expect(result.value).toEqual({}); + }); +}); + +describe("resolveOtelExportConfig", () => { + test("disabled when nothing is configured", () => { + const result = resolveOtelExportConfig(baseSettings(), {}); + expect(result).toEqual({ ok: true, config: { enabled: false } }); + }); + + test("disabled when only service name is set", () => { + const result = resolveOtelExportConfig(baseSettings({ serviceName: "demo" }), {}); + expect(result).toEqual({ ok: true, config: { enabled: false } }); + }); + + test("settings endpoint enables export with defaults", () => { + const result = resolveOtelExportConfig( + baseSettings({ endpoint: "https://collector.example/v1" }), + {}, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.endpoint).toBe("https://collector.example/v1"); + expect(result.config.serviceName).toBe(DEFAULT_OTEL_SERVICE_NAME); + expect(result.config.headers).toEqual({}); + expect(result.config.resourceAttributes["service.name"]).toBe(DEFAULT_OTEL_SERVICE_NAME); + } + }); + + test("strips trailing slash from endpoint", () => { + const result = resolveOtelExportConfig( + baseSettings({ endpoint: "https://collector.example/v1/" }), + {}, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.endpoint).toBe("https://collector.example/v1"); + } + }); + + test("env endpoint overrides settings", () => { + const result = resolveOtelExportConfig( + baseSettings({ endpoint: "https://settings.example" }), + { [OTEL_ENV.endpoint]: "https://env.example/otlp" }, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.endpoint).toBe("https://env.example/otlp"); + } + }); + + test("env headers replace settings headers", () => { + const result = resolveOtelExportConfig( + baseSettings({ + endpoint: "https://collector.example", + headers: { "x-settings": "secret-settings" }, + }), + { [OTEL_ENV.headers]: "Authorization=Bearer%20env-secret" }, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.headers).toEqual({ Authorization: "Bearer env-secret" }); + expect(result.config.headers["x-settings"]).toBeUndefined(); + } + }); + + test("settings headers used when env headers unset", () => { + const result = resolveOtelExportConfig( + baseSettings({ + endpoint: "https://collector.example", + headers: { "x-api-key": "from-settings" }, + }), + {}, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.headers).toEqual({ "x-api-key": "from-settings" }); + } + }); + + test("service name: env > settings > default", () => { + const fromSettings = resolveOtelExportConfig( + baseSettings({ endpoint: "https://c.example", serviceName: "from-settings" }), + {}, + ); + expect(fromSettings.ok && fromSettings.config.enabled && fromSettings.config.serviceName).toBe( + "from-settings", + ); + + const fromEnv = resolveOtelExportConfig( + baseSettings({ endpoint: "https://c.example", serviceName: "from-settings" }), + { [OTEL_ENV.serviceName]: "from-env" }, + ); + expect(fromEnv.ok && fromEnv.config.enabled && fromEnv.config.serviceName).toBe("from-env"); + }); + + test("resource attributes merge with env winning on conflict", () => { + const result = resolveOtelExportConfig( + baseSettings({ + endpoint: "https://c.example", + resourceAttributes: { "deployment.environment": "settings", team: "corbits" }, + }), + { [OTEL_ENV.resourceAttributes]: "deployment.environment=prod" }, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.resourceAttributes["deployment.environment"]).toBe("prod"); + expect(result.config.resourceAttributes.team).toBe("corbits"); + expect(result.config.resourceAttributes["service.name"]).toBe(DEFAULT_OTEL_SERVICE_NAME); + } + }); + + test("settings enabled false disables when only settings endpoint exists", () => { + const result = resolveOtelExportConfig( + baseSettings({ enabled: false, endpoint: "https://c.example" }), + {}, + ); + expect(result).toEqual({ ok: true, config: { enabled: false } }); + }); + + test("env endpoint still enables when settings enabled is false", () => { + const result = resolveOtelExportConfig( + baseSettings({ enabled: false, endpoint: "https://settings.example" }), + { [OTEL_ENV.endpoint]: "https://env.example" }, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.endpoint).toBe("https://env.example"); + } + }); + + test("fail closed: invalid endpoint URL", () => { + const result = resolveOtelExportConfig(baseSettings({ endpoint: "not a url" }), {}); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.code).toBe(OTEL_CONFIG_INVALID); + expect(result.message).toContain("not a valid URL"); + } + }); + + test("fail closed: non-http protocol", () => { + const result = resolveOtelExportConfig(baseSettings({ endpoint: "ftp://collector.example" }), {}); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.message).toContain("http or https"); + } + }); + + test("fail closed: credentials embedded in endpoint", () => { + const result = resolveOtelExportConfig( + baseSettings({ endpoint: "https://user:pass@collector.example" }), + {}, + ); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.message).toContain("must not embed credentials"); + } + }); + + test("fail closed: headers without endpoint", () => { + const result = resolveOtelExportConfig( + baseSettings({ headers: { Authorization: "Bearer x" } }), + {}, + ); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.code).toBe(OTEL_CONFIG_INVALID); + expect(result.message).toContain("no endpoint"); + } + }); + + test("fail closed: enabled true without endpoint", () => { + const result = resolveOtelExportConfig(baseSettings({ enabled: true }), {}); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.message).toContain("enabled but no endpoint"); + } + }); + + test("fail closed: malformed env headers", () => { + const result = resolveOtelExportConfig(baseSettings({ endpoint: "https://c.example" }), { + [OTEL_ENV.headers]: "bad", + }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.message).toContain(OTEL_ENV.headers); + } + }); + + test("fail closed: malformed env resource attributes", () => { + const result = resolveOtelExportConfig(baseSettings({ endpoint: "https://c.example" }), { + [OTEL_ENV.resourceAttributes]: "=novalue", + }); + expect(isOtelConfigInvalid(result)).toBe(true); + }); +}); + +describe("requireOtelExportConfig", () => { + test("throws OtelConfigError with stable code on invalid config", () => { + expect(() => requireOtelExportConfig(baseSettings({ endpoint: "://" }), {})).toThrow( + OtelConfigError, + ); + try { + requireOtelExportConfig(baseSettings({ endpoint: "://" }), {}); + } catch (err) { + expect(err).toBeInstanceOf(OtelConfigError); + if (err instanceof OtelConfigError) { + expect(err.code).toBe(OTEL_CONFIG_INVALID); + expect(err.message.length).toBeGreaterThan(0); + } + } + }); + + test("returns disabled config when unset", () => { + expect(requireOtelExportConfig(baseSettings(), {})).toEqual({ enabled: false }); + }); +}); + +describe("otelConfigForDump", () => { + test("never includes header values", () => { + const resolved = resolveOtelExportConfig( + baseSettings({ + endpoint: "https://collector.example", + headers: { Authorization: "Bearer super-secret", "x-api-key": "also-secret" }, + serviceName: "dump-test", + }), + {}, + ); + expect(resolved.ok).toBe(true); + if (!resolved.ok || !resolved.config.enabled) throw new Error("expected enabled config"); + + const dump = otelConfigForDump(resolved.config); + const serialized = JSON.stringify(dump); + expect(serialized).not.toContain("super-secret"); + expect(serialized).not.toContain("also-secret"); + expect(serialized).not.toContain("Bearer"); + expect(dump.enabled).toBe(true); + if (dump.enabled) { + expect(dump.headerNames).toEqual(["Authorization", "x-api-key"]); + expect(dump.endpoint).toBe("https://collector.example"); + expect(dump.serviceName).toBe("dump-test"); + // No headers field with values + expect("headers" in dump).toBe(false); + } + }); + + test("disabled dump view is empty of secrets", () => { + expect(otelConfigForDump({ enabled: false })).toEqual({ enabled: false }); + }); +}); diff --git a/src/perf/otel-config.ts b/src/perf/otel-config.ts new file mode 100644 index 000000000..617313f45 --- /dev/null +++ b/src/perf/otel-config.ts @@ -0,0 +1,359 @@ +/** + * Settings/env surface for opt-in OTEL export (CL-5175). + * + * Local PerfTrace stays always-on and independent. This module only resolves + * whether an OTLP exporter may be enabled later (CL-5173) — no SDK, no network. + * + * Fail closed: any invalid endpoint/headers/attrs yields a stable error; never + * half-enable export. Secrets (headers) must never enter privacy-strict dumps. + */ + +import type { Settings } from "../config/settings.js"; + +/** Stable operator-facing error code for invalid OTEL export config. */ +export const OTEL_CONFIG_INVALID = "OTEL_CONFIG_INVALID" as const; + +/** Default resource service name when neither settings nor env set one. */ +export const DEFAULT_OTEL_SERVICE_NAME = "corbits-code"; + +// Standard OTEL env keys (prefer conventions over CORBITS_* for export config). +export const OTEL_ENV = { + endpoint: "OTEL_EXPORTER_OTLP_ENDPOINT", + headers: "OTEL_EXPORTER_OTLP_HEADERS", + serviceName: "OTEL_SERVICE_NAME", + resourceAttributes: "OTEL_RESOURCE_ATTRIBUTES", +} as const; + +/** Non-secret settings block for OTEL export (global settings only). */ +export type OtelSettings = { + /** Explicit off switch. When false and no env endpoint, export stays disabled. */ + enabled?: boolean; + /** OTLP base URL (http/https). Env OTEL_EXPORTER_OTLP_ENDPOINT overrides. */ + endpoint?: string; + /** + * Extra OTLP headers (auth). Prefer OTEL_EXPORTER_OTLP_HEADERS env so secrets + * stay out of settings files when possible. Env fully replaces settings headers + * when the env var is set. + */ + headers?: Record; + /** Resource service.name. Env OTEL_SERVICE_NAME overrides. */ + serviceName?: string; + /** + * Extra resource attributes (merged; env keys win on conflict). + * Prefer non-secret labels only in settings. + */ + resourceAttributes?: Record; +}; + +export type EnabledOtelExportConfig = { + enabled: true; + endpoint: string; + headers: Readonly>; + serviceName: string; + resourceAttributes: Readonly>; +}; + +export type DisabledOtelExportConfig = { + enabled: false; +}; + +export type OtelExportConfig = EnabledOtelExportConfig | DisabledOtelExportConfig; + +/** + * Dump-safe view: never includes header values or other secrets. + * Local privacy-strict dumps must only ever receive this shape. + */ +export type OtelExportConfigDumpView = + | { enabled: false } + | { + enabled: true; + endpoint: string; + serviceName: string; + resourceAttributes: Readonly>; + /** Header *names* only — values are never included. */ + headerNames: readonly string[]; + }; + +export type OtelConfigResolution = + | { ok: true; config: OtelExportConfig } + | { ok: false; code: typeof OTEL_CONFIG_INVALID; message: string }; + +export class OtelConfigError extends Error { + readonly code = OTEL_CONFIG_INVALID; + + constructor(message: string) { + super(message); + this.name = "OtelConfigError"; + } +} + +function trimOrEmpty(value: string | undefined): string { + if (value === undefined) return ""; + return value.trim(); +} + +function isNonEmptyString(value: unknown): value is string { + return typeof value === "string" && value.trim().length > 0; +} + +/** + * Parse OTEL W3C-style `key=value,key2=value2` lists (headers / resource attrs). + * Values may be percent-encoded. Empty keys or malformed pairs fail closed. + */ +export function parseOtelKeyValueList( + raw: string, + label: string, +): { ok: true; value: Record } | { ok: false; message: string } { + const out: Record = {}; + const trimmed = raw.trim(); + if (trimmed.length === 0) return { ok: true, value: out }; + + const parts = trimmed.split(","); + for (const part of parts) { + const segment = part.trim(); + if (segment.length === 0) { + return { ok: false, message: `Invalid ${label}: empty entry in comma-separated list` }; + } + const eq = segment.indexOf("="); + if (eq <= 0) { + return { + ok: false, + message: `Invalid ${label}: expected key=value entries, got ${JSON.stringify(segment)}`, + }; + } + const key = segment.slice(0, eq).trim(); + const rawValue = segment.slice(eq + 1).trim(); + if (key.length === 0) { + return { ok: false, message: `Invalid ${label}: empty key` }; + } + let value: string; + try { + value = decodeURIComponent(rawValue); + } catch { + return { + ok: false, + message: `Invalid ${label}: could not decode value for key ${JSON.stringify(key)}`, + }; + } + out[key] = value; + } + return { ok: true, value: out }; +} + +function validateEndpoint( + raw: string, +): { ok: true; endpoint: string } | { ok: false; message: string } { + const endpoint = raw.trim(); + if (endpoint.length === 0) { + return { ok: false, message: "OTEL endpoint must be a non-empty http(s) URL" }; + } + + let url: URL; + try { + url = new URL(endpoint); + } catch { + return { + ok: false, + message: `OTEL endpoint is not a valid URL: ${JSON.stringify(endpoint)}`, + }; + } + + if (url.protocol !== "http:" && url.protocol !== "https:") { + return { + ok: false, + message: `OTEL endpoint must use http or https (got ${url.protocol.replace(":", "")})`, + }; + } + + if (url.username !== "" || url.password !== "") { + return { + ok: false, + message: + "OTEL endpoint must not embed credentials; pass auth via OTEL_EXPORTER_OTLP_HEADERS or settings.otel.headers", + }; + } + + // Normalize: drop trailing slash so exporters can append /v1/traces consistently. + const normalized = endpoint.replace(/\/+$/, ""); + return { ok: true, endpoint: normalized }; +} + +function validateStringMap( + map: Record | undefined, + label: string, +): { ok: true; value: Record } | { ok: false; message: string } { + if (map === undefined) return { ok: true, value: {} }; + const out: Record = {}; + for (const [key, value] of Object.entries(map)) { + if (typeof key !== "string" || key.trim().length === 0) { + return { ok: false, message: `Invalid ${label}: empty key` }; + } + if (typeof value !== "string") { + return { + ok: false, + message: `Invalid ${label}: value for ${JSON.stringify(key)} must be a string`, + }; + } + out[key.trim()] = value; + } + return { ok: true, value: out }; +} + +/** + * Resolve OTEL export config from global settings + process env. + * + * Precedence: + * - endpoint: env > settings + * - headers: env list fully replaces settings when env is set; else settings + * - serviceName: env > settings > default + * - resourceAttributes: settings then env (env wins on key conflict) + * + * No endpoint → disabled (ok). Invalid partial/malformed config → not ok. + */ +export function resolveOtelExportConfig( + settings?: Settings | null, + env: NodeJS.ProcessEnv = process.env, +): OtelConfigResolution { + const otel = settings?.otel; + const envEndpointRaw = trimOrEmpty(env[OTEL_ENV.endpoint]); + const envHeadersRaw = env[OTEL_ENV.headers]; + const envServiceNameRaw = trimOrEmpty(env[OTEL_ENV.serviceName]); + const envResourceAttrsRaw = env[OTEL_ENV.resourceAttributes]; + + const settingsEndpoint = isNonEmptyString(otel?.endpoint) ? otel.endpoint.trim() : ""; + const endpointRaw = envEndpointRaw.length > 0 ? envEndpointRaw : settingsEndpoint; + + // settings.otel.enabled === false forces off unless env explicitly sets an endpoint. + if (otel?.enabled === false && envEndpointRaw.length === 0) { + return { ok: true, config: { enabled: false } }; + } + + if (endpointRaw.length === 0) { + const settingsHeaders = otel?.headers !== undefined && Object.keys(otel.headers).length > 0; + const envHeadersSet = envHeadersRaw !== undefined && trimOrEmpty(envHeadersRaw).length > 0; + if (settingsHeaders || envHeadersSet) { + return { + ok: false, + code: OTEL_CONFIG_INVALID, + message: + "OTEL export headers are set but no endpoint is configured (set OTEL_EXPORTER_OTLP_ENDPOINT or settings.otel.endpoint)", + }; + } + if (otel?.enabled === true) { + return { + ok: false, + code: OTEL_CONFIG_INVALID, + message: + "OTEL export is enabled but no endpoint is configured (set OTEL_EXPORTER_OTLP_ENDPOINT or settings.otel.endpoint)", + }; + } + // Service name / resource attrs alone do not enable export. + return { ok: true, config: { enabled: false } }; + } + + const endpointResult = validateEndpoint(endpointRaw); + if (!endpointResult.ok) { + return { ok: false, code: OTEL_CONFIG_INVALID, message: endpointResult.message }; + } + + let headers: Record = {}; + if (envHeadersRaw !== undefined) { + const parsed = parseOtelKeyValueList(envHeadersRaw, OTEL_ENV.headers); + if (!parsed.ok) { + return { ok: false, code: OTEL_CONFIG_INVALID, message: parsed.message }; + } + headers = parsed.value; + } else { + const settingsHeaders = validateStringMap(otel?.headers, "settings.otel.headers"); + if (!settingsHeaders.ok) { + return { ok: false, code: OTEL_CONFIG_INVALID, message: settingsHeaders.message }; + } + headers = settingsHeaders.value; + } + + let resourceAttributes: Record = {}; + const settingsAttrs = validateStringMap( + otel?.resourceAttributes, + "settings.otel.resourceAttributes", + ); + if (!settingsAttrs.ok) { + return { ok: false, code: OTEL_CONFIG_INVALID, message: settingsAttrs.message }; + } + resourceAttributes = { ...settingsAttrs.value }; + + if (envResourceAttrsRaw !== undefined) { + const parsed = parseOtelKeyValueList(envResourceAttrsRaw, OTEL_ENV.resourceAttributes); + if (!parsed.ok) { + return { ok: false, code: OTEL_CONFIG_INVALID, message: parsed.message }; + } + resourceAttributes = { ...resourceAttributes, ...parsed.value }; + } + + const serviceName = + envServiceNameRaw.length > 0 + ? envServiceNameRaw + : isNonEmptyString(otel?.serviceName) + ? otel.serviceName.trim() + : DEFAULT_OTEL_SERVICE_NAME; + + if (serviceName.length === 0) { + return { + ok: false, + code: OTEL_CONFIG_INVALID, + message: "OTEL service name must be non-empty when set", + }; + } + + // Ensure service.name is present in resource attributes (OTEL resource convention). + if (resourceAttributes["service.name"] === undefined) { + resourceAttributes["service.name"] = serviceName; + } + + return { + ok: true, + config: { + enabled: true, + endpoint: endpointResult.endpoint, + headers: Object.freeze({ ...headers }), + serviceName, + resourceAttributes: Object.freeze({ ...resourceAttributes }), + }, + }; +} + +/** + * Resolve or throw. Callers that treat invalid config as a hard startup error + * use this; soft paths should call resolveOtelExportConfig and branch on ok. + */ +export function requireOtelExportConfig( + settings?: Settings | null, + env: NodeJS.ProcessEnv = process.env, +): OtelExportConfig { + const result = resolveOtelExportConfig(settings, env); + if (!result.ok) { + throw new OtelConfigError(result.message); + } + return result.config; +} + +/** + * Strip secrets for local dumps and logs. + * Never pass EnabledOtelExportConfig.headers into dump writers — use this. + */ +export function otelConfigForDump(config: OtelExportConfig): OtelExportConfigDumpView { + if (!config.enabled) return { enabled: false }; + return { + enabled: true, + endpoint: config.endpoint, + serviceName: config.serviceName, + resourceAttributes: config.resourceAttributes, + headerNames: Object.freeze(Object.keys(config.headers).sort()), + }; +} + +/** True when resolution failed closed (export must not start). */ +export function isOtelConfigInvalid( + result: OtelConfigResolution, +): result is Extract { + return result.ok === false; +} From 339e7f97eadfe0948bebb754fa6670143a00d45a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 4 Aug 2026 09:07:06 -0700 Subject: [PATCH 2/2] Fix OTEL service.name consistency and dump-safe attrs Resolve serviceName as env > settings > attrs["service.name"] > default, then always write resourceAttributes["service.name"] so the two never diverge. Redact high-risk resource attr values in otelConfigForDump. Document Phoenix cloud as a base OTLP URL (exporter appends /v1/traces). --- docs/PERFTRACE.md | 12 ++++- src/perf/otel-config.test.ts | 95 +++++++++++++++++++++++++++++++++++- src/perf/otel-config.ts | 41 +++++++++++++--- 3 files changed, 137 insertions(+), 11 deletions(-) diff --git a/docs/PERFTRACE.md b/docs/PERFTRACE.md index fafda5d1f..9709c3ef0 100644 --- a/docs/PERFTRACE.md +++ b/docs/PERFTRACE.md @@ -57,7 +57,10 @@ Precedence: - **endpoint:** env overrides settings - **headers:** when `OTEL_EXPORTER_OTLP_HEADERS` is set, it fully replaces settings headers (prefer env so secrets stay out of the settings file) -- **serviceName:** env > settings > `corbits-code` +- **serviceName:** env `OTEL_SERVICE_NAME` > settings `serviceName` > + `resourceAttributes["service.name"]` > `corbits-code`. After merge, + `resourceAttributes["service.name"]` is always set to the resolved name so + the two never diverge. - **resourceAttributes:** settings merged with env; env wins on key conflict - **`otel.enabled: false`:** disables export when only settings provide an endpoint; an explicit env endpoint still enables export @@ -82,6 +85,10 @@ No endpoint and no half-config → export stays disabled (not an error). - Header **values** are secrets. Prefer env for them. - `otelConfigForDump()` exposes only: enabled flag, endpoint, service name, resource attributes, and header **names** — never values. +- Resource attribute values whose keys match `/secret|token|key|password|auth/i` + are replaced with `[redacted]` in the dump view (live export config is + unchanged). Prefer non-secret labels in `resourceAttributes`; put auth in + headers/env. - Local privacy-strict dump writers must call `otelConfigForDump` (or omit OTEL config entirely). Never serialize `OtelExportConfig.headers` into session artifacts, logs, or crash dumps. @@ -104,7 +111,8 @@ Cloud / authenticated Phoenix: set the project endpoint and pass the API key as a header (exact header name follows Phoenix’s current docs): ```bash -export OTEL_EXPORTER_OTLP_ENDPOINT="https://app.phoenix.arize.com/v1/traces" +# Base URL only — the exporter appends /v1/traces (do not include the path here). +export OTEL_EXPORTER_OTLP_ENDPOINT="https://app.phoenix.arize.com" export OTEL_EXPORTER_OTLP_HEADERS="Authorization=Bearer%20" export OTEL_SERVICE_NAME="corbits-code" ``` diff --git a/src/perf/otel-config.test.ts b/src/perf/otel-config.test.ts index 4dcaf7575..168de61ca 100644 --- a/src/perf/otel-config.test.ts +++ b/src/perf/otel-config.test.ts @@ -121,7 +121,7 @@ describe("resolveOtelExportConfig", () => { } }); - test("service name: env > settings > default", () => { + test("service name: env > settings > attrs > default", () => { const fromSettings = resolveOtelExportConfig( baseSettings({ endpoint: "https://c.example", serviceName: "from-settings" }), {}, @@ -137,6 +137,65 @@ describe("resolveOtelExportConfig", () => { expect(fromEnv.ok && fromEnv.config.enabled && fromEnv.config.serviceName).toBe("from-env"); }); + test("service.name: attrs used when env and settings serviceName unset", () => { + const result = resolveOtelExportConfig( + baseSettings({ + endpoint: "https://c.example", + resourceAttributes: { "service.name": "from-attrs" }, + }), + {}, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.serviceName).toBe("from-attrs"); + expect(result.config.resourceAttributes["service.name"]).toBe("from-attrs"); + } + }); + + test("service.name: env wins over settings and attrs, always synced to attrs", () => { + const result = resolveOtelExportConfig( + baseSettings({ + endpoint: "https://c.example", + serviceName: "from-settings", + resourceAttributes: { "service.name": "from-attrs" }, + }), + { [OTEL_ENV.serviceName]: "from-env" }, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.serviceName).toBe("from-env"); + expect(result.config.resourceAttributes["service.name"]).toBe("from-env"); + } + }); + + test("service.name: settings wins over attrs, always synced to attrs", () => { + const result = resolveOtelExportConfig( + baseSettings({ + endpoint: "https://c.example", + serviceName: "from-settings", + resourceAttributes: { "service.name": "from-attrs" }, + }), + {}, + ); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.serviceName).toBe("from-settings"); + expect(result.config.resourceAttributes["service.name"]).toBe("from-settings"); + } + }); + + test("service.name: env OTEL_RESOURCE_ATTRIBUTES service.name used when no env/settings name", () => { + const result = resolveOtelExportConfig(baseSettings({ endpoint: "https://c.example" }), { + [OTEL_ENV.resourceAttributes]: "service.name=from-env-attrs,team=corbits", + }); + expect(result.ok).toBe(true); + if (result.ok && result.config.enabled) { + expect(result.config.serviceName).toBe("from-env-attrs"); + expect(result.config.resourceAttributes["service.name"]).toBe("from-env-attrs"); + expect(result.config.resourceAttributes.team).toBe("corbits"); + } + }); + test("resource attributes merge with env winning on conflict", () => { const result = resolveOtelExportConfig( baseSettings({ @@ -290,4 +349,38 @@ describe("otelConfigForDump", () => { test("disabled dump view is empty of secrets", () => { expect(otelConfigForDump({ enabled: false })).toEqual({ enabled: false }); }); + + test("redacts high-risk resource attribute values in dump view", () => { + const resolved = resolveOtelExportConfig( + baseSettings({ + endpoint: "https://collector.example", + resourceAttributes: { + "deployment.environment": "prod", + "api_key": "should-not-leak", + "auth.token": "tok-secret", + "db.password": "p@ss", + team: "corbits", + }, + }), + {}, + ); + expect(resolved.ok).toBe(true); + if (!resolved.ok || !resolved.config.enabled) throw new Error("expected enabled config"); + + // Live export config keeps raw values for the exporter. + expect(resolved.config.resourceAttributes.api_key).toBe("should-not-leak"); + + const dump = otelConfigForDump(resolved.config); + expect(dump.enabled).toBe(true); + if (!dump.enabled) throw new Error("expected enabled dump"); + expect(dump.resourceAttributes["deployment.environment"]).toBe("prod"); + expect(dump.resourceAttributes.team).toBe("corbits"); + expect(dump.resourceAttributes.api_key).toBe("[redacted]"); + expect(dump.resourceAttributes["auth.token"]).toBe("[redacted]"); + expect(dump.resourceAttributes["db.password"]).toBe("[redacted]"); + const serialized = JSON.stringify(dump); + expect(serialized).not.toContain("should-not-leak"); + expect(serialized).not.toContain("tok-secret"); + expect(serialized).not.toContain("p@ss"); + }); }); diff --git a/src/perf/otel-config.ts b/src/perf/otel-config.ts index 617313f45..a3a702473 100644 --- a/src/perf/otel-config.ts +++ b/src/perf/otel-config.ts @@ -205,8 +205,11 @@ function validateStringMap( * Precedence: * - endpoint: env > settings * - headers: env list fully replaces settings when env is set; else settings - * - serviceName: env > settings > default - * - resourceAttributes: settings then env (env wins on key conflict) + * - serviceName: env OTEL_SERVICE_NAME > settings.serviceName > + * resourceAttributes["service.name"] > default + * - resourceAttributes: settings then env (env wins on key conflict); after + * merge, resourceAttributes["service.name"] is always set to the resolved + * serviceName so the two never diverge * * No endpoint → disabled (ok). Invalid partial/malformed config → not ok. */ @@ -289,12 +292,17 @@ export function resolveOtelExportConfig( resourceAttributes = { ...resourceAttributes, ...parsed.value }; } + // serviceName: env > settings > attrs["service.name"] > default, then always + // write resourceAttributes["service.name"] so config.serviceName and attrs stay consistent. + const attrServiceName = trimOrEmpty(resourceAttributes["service.name"]); const serviceName = envServiceNameRaw.length > 0 ? envServiceNameRaw : isNonEmptyString(otel?.serviceName) ? otel.serviceName.trim() - : DEFAULT_OTEL_SERVICE_NAME; + : attrServiceName.length > 0 + ? attrServiceName + : DEFAULT_OTEL_SERVICE_NAME; if (serviceName.length === 0) { return { @@ -304,10 +312,7 @@ export function resolveOtelExportConfig( }; } - // Ensure service.name is present in resource attributes (OTEL resource convention). - if (resourceAttributes["service.name"] === undefined) { - resourceAttributes["service.name"] = serviceName; - } + resourceAttributes["service.name"] = serviceName; return { ok: true, @@ -336,9 +341,29 @@ export function requireOtelExportConfig( return result.config; } +/** Attr keys that look secret-bearing — values redacted in dump views only. */ +const SENSITIVE_ATTR_KEY = /secret|token|key|password|auth/i; + +/** + * Redact high-risk resource attribute values for dump/log views. + * Keys matching /secret|token|key|password|auth/i get a fixed placeholder. + * Does not mutate the live export config. + */ +export function redactResourceAttributesForDump( + attrs: Readonly>, +): Readonly> { + const out: Record = {}; + for (const [key, value] of Object.entries(attrs)) { + out[key] = SENSITIVE_ATTR_KEY.test(key) ? "[redacted]" : value; + } + return Object.freeze(out); +} + /** * Strip secrets for local dumps and logs. * Never pass EnabledOtelExportConfig.headers into dump writers — use this. + * Resource attribute values for high-risk keys are redacted; prefer non-secret + * labels in resourceAttributes (auth belongs in headers/env). */ export function otelConfigForDump(config: OtelExportConfig): OtelExportConfigDumpView { if (!config.enabled) return { enabled: false }; @@ -346,7 +371,7 @@ export function otelConfigForDump(config: OtelExportConfig): OtelExportConfigDum enabled: true, endpoint: config.endpoint, serviceName: config.serviceName, - resourceAttributes: config.resourceAttributes, + resourceAttributes: redactResourceAttributesForDump(config.resourceAttributes), headerNames: Object.freeze(Object.keys(config.headers).sort()), }; }