Skip to content

Commit 339e7f9

Browse files
committed
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).
1 parent b04fb90 commit 339e7f9

3 files changed

Lines changed: 137 additions & 11 deletions

File tree

docs/PERFTRACE.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,10 @@ Precedence:
5757
- **endpoint:** env overrides settings
5858
- **headers:** when `OTEL_EXPORTER_OTLP_HEADERS` is set, it fully replaces
5959
settings headers (prefer env so secrets stay out of the settings file)
60-
- **serviceName:** env > settings > `corbits-code`
60+
- **serviceName:** env `OTEL_SERVICE_NAME` > settings `serviceName` >
61+
`resourceAttributes["service.name"]` > `corbits-code`. After merge,
62+
`resourceAttributes["service.name"]` is always set to the resolved name so
63+
the two never diverge.
6164
- **resourceAttributes:** settings merged with env; env wins on key conflict
6265
- **`otel.enabled: false`:** disables export when only settings provide an
6366
endpoint; an explicit env endpoint still enables export
@@ -82,6 +85,10 @@ No endpoint and no half-config → export stays disabled (not an error).
8285
- Header **values** are secrets. Prefer env for them.
8386
- `otelConfigForDump()` exposes only: enabled flag, endpoint, service name,
8487
resource attributes, and header **names** — never values.
88+
- Resource attribute values whose keys match `/secret|token|key|password|auth/i`
89+
are replaced with `[redacted]` in the dump view (live export config is
90+
unchanged). Prefer non-secret labels in `resourceAttributes`; put auth in
91+
headers/env.
8592
- Local privacy-strict dump writers must call `otelConfigForDump` (or omit OTEL
8693
config entirely). Never serialize `OtelExportConfig.headers` into session
8794
artifacts, logs, or crash dumps.
@@ -104,7 +111,8 @@ Cloud / authenticated Phoenix: set the project endpoint and pass the API key as
104111
a header (exact header name follows Phoenix’s current docs):
105112

106113
```bash
107-
export OTEL_EXPORTER_OTLP_ENDPOINT="https://app.phoenix.arize.com/v1/traces"
114+
# Base URL only — the exporter appends /v1/traces (do not include the path here).
115+
export OTEL_EXPORTER_OTLP_ENDPOINT="https://app.phoenix.arize.com"
108116
export OTEL_EXPORTER_OTLP_HEADERS="Authorization=Bearer%20<phoenix-api-key>"
109117
export OTEL_SERVICE_NAME="corbits-code"
110118
```

src/perf/otel-config.test.ts

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ describe("resolveOtelExportConfig", () => {
121121
}
122122
});
123123

124-
test("service name: env > settings > default", () => {
124+
test("service name: env > settings > attrs > default", () => {
125125
const fromSettings = resolveOtelExportConfig(
126126
baseSettings({ endpoint: "https://c.example", serviceName: "from-settings" }),
127127
{},
@@ -137,6 +137,65 @@ describe("resolveOtelExportConfig", () => {
137137
expect(fromEnv.ok && fromEnv.config.enabled && fromEnv.config.serviceName).toBe("from-env");
138138
});
139139

140+
test("service.name: attrs used when env and settings serviceName unset", () => {
141+
const result = resolveOtelExportConfig(
142+
baseSettings({
143+
endpoint: "https://c.example",
144+
resourceAttributes: { "service.name": "from-attrs" },
145+
}),
146+
{},
147+
);
148+
expect(result.ok).toBe(true);
149+
if (result.ok && result.config.enabled) {
150+
expect(result.config.serviceName).toBe("from-attrs");
151+
expect(result.config.resourceAttributes["service.name"]).toBe("from-attrs");
152+
}
153+
});
154+
155+
test("service.name: env wins over settings and attrs, always synced to attrs", () => {
156+
const result = resolveOtelExportConfig(
157+
baseSettings({
158+
endpoint: "https://c.example",
159+
serviceName: "from-settings",
160+
resourceAttributes: { "service.name": "from-attrs" },
161+
}),
162+
{ [OTEL_ENV.serviceName]: "from-env" },
163+
);
164+
expect(result.ok).toBe(true);
165+
if (result.ok && result.config.enabled) {
166+
expect(result.config.serviceName).toBe("from-env");
167+
expect(result.config.resourceAttributes["service.name"]).toBe("from-env");
168+
}
169+
});
170+
171+
test("service.name: settings wins over attrs, always synced to attrs", () => {
172+
const result = resolveOtelExportConfig(
173+
baseSettings({
174+
endpoint: "https://c.example",
175+
serviceName: "from-settings",
176+
resourceAttributes: { "service.name": "from-attrs" },
177+
}),
178+
{},
179+
);
180+
expect(result.ok).toBe(true);
181+
if (result.ok && result.config.enabled) {
182+
expect(result.config.serviceName).toBe("from-settings");
183+
expect(result.config.resourceAttributes["service.name"]).toBe("from-settings");
184+
}
185+
});
186+
187+
test("service.name: env OTEL_RESOURCE_ATTRIBUTES service.name used when no env/settings name", () => {
188+
const result = resolveOtelExportConfig(baseSettings({ endpoint: "https://c.example" }), {
189+
[OTEL_ENV.resourceAttributes]: "service.name=from-env-attrs,team=corbits",
190+
});
191+
expect(result.ok).toBe(true);
192+
if (result.ok && result.config.enabled) {
193+
expect(result.config.serviceName).toBe("from-env-attrs");
194+
expect(result.config.resourceAttributes["service.name"]).toBe("from-env-attrs");
195+
expect(result.config.resourceAttributes.team).toBe("corbits");
196+
}
197+
});
198+
140199
test("resource attributes merge with env winning on conflict", () => {
141200
const result = resolveOtelExportConfig(
142201
baseSettings({
@@ -290,4 +349,38 @@ describe("otelConfigForDump", () => {
290349
test("disabled dump view is empty of secrets", () => {
291350
expect(otelConfigForDump({ enabled: false })).toEqual({ enabled: false });
292351
});
352+
353+
test("redacts high-risk resource attribute values in dump view", () => {
354+
const resolved = resolveOtelExportConfig(
355+
baseSettings({
356+
endpoint: "https://collector.example",
357+
resourceAttributes: {
358+
"deployment.environment": "prod",
359+
"api_key": "should-not-leak",
360+
"auth.token": "tok-secret",
361+
"db.password": "p@ss",
362+
team: "corbits",
363+
},
364+
}),
365+
{},
366+
);
367+
expect(resolved.ok).toBe(true);
368+
if (!resolved.ok || !resolved.config.enabled) throw new Error("expected enabled config");
369+
370+
// Live export config keeps raw values for the exporter.
371+
expect(resolved.config.resourceAttributes.api_key).toBe("should-not-leak");
372+
373+
const dump = otelConfigForDump(resolved.config);
374+
expect(dump.enabled).toBe(true);
375+
if (!dump.enabled) throw new Error("expected enabled dump");
376+
expect(dump.resourceAttributes["deployment.environment"]).toBe("prod");
377+
expect(dump.resourceAttributes.team).toBe("corbits");
378+
expect(dump.resourceAttributes.api_key).toBe("[redacted]");
379+
expect(dump.resourceAttributes["auth.token"]).toBe("[redacted]");
380+
expect(dump.resourceAttributes["db.password"]).toBe("[redacted]");
381+
const serialized = JSON.stringify(dump);
382+
expect(serialized).not.toContain("should-not-leak");
383+
expect(serialized).not.toContain("tok-secret");
384+
expect(serialized).not.toContain("p@ss");
385+
});
293386
});

src/perf/otel-config.ts

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,11 @@ function validateStringMap(
205205
* Precedence:
206206
* - endpoint: env > settings
207207
* - headers: env list fully replaces settings when env is set; else settings
208-
* - serviceName: env > settings > default
209-
* - resourceAttributes: settings then env (env wins on key conflict)
208+
* - serviceName: env OTEL_SERVICE_NAME > settings.serviceName >
209+
* resourceAttributes["service.name"] > default
210+
* - resourceAttributes: settings then env (env wins on key conflict); after
211+
* merge, resourceAttributes["service.name"] is always set to the resolved
212+
* serviceName so the two never diverge
210213
*
211214
* No endpoint → disabled (ok). Invalid partial/malformed config → not ok.
212215
*/
@@ -289,12 +292,17 @@ export function resolveOtelExportConfig(
289292
resourceAttributes = { ...resourceAttributes, ...parsed.value };
290293
}
291294

295+
// serviceName: env > settings > attrs["service.name"] > default, then always
296+
// write resourceAttributes["service.name"] so config.serviceName and attrs stay consistent.
297+
const attrServiceName = trimOrEmpty(resourceAttributes["service.name"]);
292298
const serviceName =
293299
envServiceNameRaw.length > 0
294300
? envServiceNameRaw
295301
: isNonEmptyString(otel?.serviceName)
296302
? otel.serviceName.trim()
297-
: DEFAULT_OTEL_SERVICE_NAME;
303+
: attrServiceName.length > 0
304+
? attrServiceName
305+
: DEFAULT_OTEL_SERVICE_NAME;
298306

299307
if (serviceName.length === 0) {
300308
return {
@@ -304,10 +312,7 @@ export function resolveOtelExportConfig(
304312
};
305313
}
306314

307-
// Ensure service.name is present in resource attributes (OTEL resource convention).
308-
if (resourceAttributes["service.name"] === undefined) {
309-
resourceAttributes["service.name"] = serviceName;
310-
}
315+
resourceAttributes["service.name"] = serviceName;
311316

312317
return {
313318
ok: true,
@@ -336,17 +341,37 @@ export function requireOtelExportConfig(
336341
return result.config;
337342
}
338343

344+
/** Attr keys that look secret-bearing — values redacted in dump views only. */
345+
const SENSITIVE_ATTR_KEY = /secret|token|key|password|auth/i;
346+
347+
/**
348+
* Redact high-risk resource attribute values for dump/log views.
349+
* Keys matching /secret|token|key|password|auth/i get a fixed placeholder.
350+
* Does not mutate the live export config.
351+
*/
352+
export function redactResourceAttributesForDump(
353+
attrs: Readonly<Record<string, string>>,
354+
): Readonly<Record<string, string>> {
355+
const out: Record<string, string> = {};
356+
for (const [key, value] of Object.entries(attrs)) {
357+
out[key] = SENSITIVE_ATTR_KEY.test(key) ? "[redacted]" : value;
358+
}
359+
return Object.freeze(out);
360+
}
361+
339362
/**
340363
* Strip secrets for local dumps and logs.
341364
* Never pass EnabledOtelExportConfig.headers into dump writers — use this.
365+
* Resource attribute values for high-risk keys are redacted; prefer non-secret
366+
* labels in resourceAttributes (auth belongs in headers/env).
342367
*/
343368
export function otelConfigForDump(config: OtelExportConfig): OtelExportConfigDumpView {
344369
if (!config.enabled) return { enabled: false };
345370
return {
346371
enabled: true,
347372
endpoint: config.endpoint,
348373
serviceName: config.serviceName,
349-
resourceAttributes: config.resourceAttributes,
374+
resourceAttributes: redactResourceAttributesForDump(config.resourceAttributes),
350375
headerNames: Object.freeze(Object.keys(config.headers).sort()),
351376
};
352377
}

0 commit comments

Comments
 (0)