diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 25c809467..9863ae771 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed env-driven OTLP trace export ignoring `OTEL_RESOURCE_ATTRIBUTES`: the exported resource only carried `service.name`, so backends that attribute spans via resource attributes never received them. The resource now merges the vendored `envDetector`, which parses `OTEL_RESOURCE_ATTRIBUTES` (percent-decoded, per spec) with `OTEL_SERVICE_NAME` taking precedence for `service.name` ([#7134](https://github.com/can1357/oh-my-pi/issues/7134)). + ## [17.2.1] - 2026-07-30 ### Added diff --git a/packages/coding-agent/src/telemetry-export.ts b/packages/coding-agent/src/telemetry-export.ts index 0dad0ecc7..62769c5ec 100644 --- a/packages/coding-agent/src/telemetry-export.ts +++ b/packages/coding-agent/src/telemetry-export.ts @@ -37,7 +37,7 @@ import { AsyncLocalStorageContextManager } from "@opentelemetry/context-async-ho import { OTLPLogExporter } from "@opentelemetry/exporter-logs-otlp-proto"; import { OTLPMetricExporter } from "@opentelemetry/exporter-metrics-otlp-proto"; import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-proto"; -import { resourceFromAttributes } from "@opentelemetry/resources"; +import { detectResources, envDetector, resourceFromAttributes } from "@opentelemetry/resources"; import { BatchLogRecordProcessor, LoggerProvider } from "@opentelemetry/sdk-logs"; import { MeterProvider, PeriodicExportingMetricReader } from "@opentelemetry/sdk-metrics"; import { BatchSpanProcessor } from "@opentelemetry/sdk-trace-base"; @@ -146,9 +146,13 @@ export async function initTelemetryExport(): Promise { } async function registerProviders(signalConfig: SignalConfig): Promise { - const resource = resourceFromAttributes({ - "service.name": process.env.OTEL_SERVICE_NAME ?? SERVICE_NAME, - }); + // `envDetector` parses OTEL_RESOURCE_ATTRIBUTES (percent-decoded, per spec) and + // OTEL_SERVICE_NAME; merged last so both take precedence over the fallback + // service.name — with OTEL_SERVICE_NAME still winning service.name inside the + // detector itself. + const resource = resourceFromAttributes({ "service.name": SERVICE_NAME }).merge( + detectResources({ detectors: [envDetector] }), + ); if (signalConfig.trace) { const exporter = new OTLPTraceExporter(); diff --git a/packages/coding-agent/test/otel-resource-probe.ts b/packages/coding-agent/test/otel-resource-probe.ts new file mode 100644 index 000000000..afca3b603 --- /dev/null +++ b/packages/coding-agent/test/otel-resource-probe.ts @@ -0,0 +1,64 @@ +/** + * Resource-attribute probe for the OTLP trace exporter, run as a subprocess by + * telemetry-export.test.ts. Isolated out-of-process for the same reason as the + * other probes: initTelemetryExport() registers a process-global provider. + * + * Stands up a loopback OTLP/proto receiver, sets OTEL_SERVICE_NAME plus + * OTEL_RESOURCE_ATTRIBUTES (including a service.name that must lose to + * OTEL_SERVICE_NAME), exports a span, and inspects the captured protobuf + * payload. Exits 0 only when the resource carries the OTEL_RESOURCE_ATTRIBUTES + * entries and service.name resolves to OTEL_SERVICE_NAME. + */ + +import { + flushTelemetryExport, + initTelemetryExport, + isTelemetryExportEnabled, +} from "@oh-my-pi/pi-coding-agent/telemetry-export"; +import { trace } from "@opentelemetry/api"; + +let body: Buffer | undefined; +const server = Bun.serve({ + port: 0, + async fetch(req) { + const path = new URL(req.url).pathname; + if (req.method === "POST" && path.endsWith("/v1/traces")) { + body = Buffer.from(await req.arrayBuffer()); + return new Response('{"partialSuccess":{}}', { + status: 200, + headers: { "content-type": "application/json" }, + }); + } + return new Response("not found", { status: 404 }); + }, +}); + +process.env.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = `http://localhost:${server.port}/v1/traces`; +process.env.OTEL_SERVICE_NAME = "svc-probe"; +process.env.OTEL_RESOURCE_ATTRIBUTES = "deployment.environment=staging,tenant.id=acme,service.name=should-lose"; + +await initTelemetryExport(); +if (!isTelemetryExportEnabled()) { + console.error("PROBE: provider did not register"); + await server.stop(true); + process.exit(2); +} + +const span = trace.getTracer("@oh-my-pi/pi-agent-core").startSpan("agent.llm_call"); +span.setAttribute("gen_ai.request.model", "claude-haiku-4-5"); +span.end(); + +await flushTelemetryExport(); +await server.stop(true); + +// Attribute keys/values are inline UTF-8 in the OTLP protobuf payload. +const payload = body ? body.toString("latin1") : ""; +const has = (s: string) => payload.includes(s); + +const merged = has("deployment.environment") && has("staging") && has("tenant.id") && has("acme"); +// OTEL_SERVICE_NAME must win over the service.name in OTEL_RESOURCE_ATTRIBUTES. +const precedence = has("svc-probe") && !has("should-lose"); + +console.log(merged && precedence ? "PROBE: RECEIVED" : "PROBE: NO_EXPORT"); +console.log("merged:", merged, "precedence:", precedence); +process.exit(merged && precedence ? 0 : 1); diff --git a/packages/coding-agent/test/telemetry-export.test.ts b/packages/coding-agent/test/telemetry-export.test.ts index cdc924a5e..ddab68694 100644 --- a/packages/coding-agent/test/telemetry-export.test.ts +++ b/packages/coding-agent/test/telemetry-export.test.ts @@ -116,4 +116,16 @@ describe("initTelemetryExport signals export path", () => { expect(stdout).toContain("PROBE: RECEIVED"); expect(code).toBe(0); }, 20_000); + + it("merges OTEL_RESOURCE_ATTRIBUTES into the exported resource", async () => { + // Regression for #7134: the resource only carried service.name, so + // OTEL_RESOURCE_ATTRIBUTES entries never reached the collector. The probe + // asserts the merged attributes land and that OTEL_SERVICE_NAME wins + // service.name over an OTEL_RESOURCE_ATTRIBUTES entry. + const probe = fileURLToPath(new URL("./otel-resource-probe.ts", import.meta.url)); + const proc = Bun.spawn(["bun", probe], { stdout: "pipe", stderr: "pipe" }); + const [code, stdout] = await Promise.all([proc.exited, new Response(proc.stdout).text()]); + expect(stdout).toContain("PROBE: RECEIVED"); + expect(code).toBe(0); + }, 20_000); });