From 88f698974af7fa43f89cfc149599981f8be5971b Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 19 Feb 2026 02:02:57 -0800 Subject: [PATCH] fix(otel): sanitize OTLP endpoint URL resolution (#13791) * fix(otel): sanitize OTLP endpoint signal URL resolution * fix(otel): preserve signal URLs with query params * fix(otel): accept case-insensitive signal paths --- .../diagnostics-otel/src/service.test.ts | 104 +++++++++++++++++- extensions/diagnostics-otel/src/service.ts | 3 +- 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/extensions/diagnostics-otel/src/service.test.ts b/extensions/diagnostics-otel/src/service.test.ts index 5d1f0ef9b88..299ba5a47c3 100644 --- a/extensions/diagnostics-otel/src/service.test.ts +++ b/extensions/diagnostics-otel/src/service.test.ts @@ -30,6 +30,7 @@ const sdkStart = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); const sdkShutdown = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); const logEmit = vi.hoisted(() => vi.fn()); const logShutdown = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); +const traceExporterCtor = vi.hoisted(() => vi.fn()); vi.mock("@opentelemetry/api", () => ({ metrics: { @@ -55,7 +56,11 @@ vi.mock("@opentelemetry/exporter-metrics-otlp-http", () => ({ })); vi.mock("@opentelemetry/exporter-trace-otlp-http", () => ({ - OTLPTraceExporter: class {}, + OTLPTraceExporter: class { + constructor(options?: unknown) { + traceExporterCtor(options); + } + }, })); vi.mock("@opentelemetry/exporter-logs-otlp-http", () => ({ @@ -119,6 +124,7 @@ describe("diagnostics-otel service", () => { sdkShutdown.mockClear(); logEmit.mockClear(); logShutdown.mockClear(); + traceExporterCtor.mockClear(); registerLogTransportMock.mockReset(); }); @@ -227,4 +233,100 @@ describe("diagnostics-otel service", () => { await service.stop?.(ctx); }); + + test("appends signal path when endpoint contains non-signal /v1 segment", async () => { + const service = createDiagnosticsOtelService(); + await service.start({ + config: { + diagnostics: { + enabled: true, + otel: { + enabled: true, + endpoint: "https://www.comet.com/opik/api/v1/private/otel", + protocol: "http/protobuf", + traces: true, + metrics: false, + logs: false, + }, + }, + }, + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + }); + + const options = traceExporterCtor.mock.calls[0]?.[0] as { url?: string } | undefined; + expect(options?.url).toBe("https://www.comet.com/opik/api/v1/private/otel/v1/traces"); + await service.stop?.(); + }); + + test("keeps already signal-qualified endpoint unchanged", async () => { + const service = createDiagnosticsOtelService(); + await service.start({ + config: { + diagnostics: { + enabled: true, + otel: { + enabled: true, + endpoint: "https://collector.example.com/v1/traces", + protocol: "http/protobuf", + traces: true, + metrics: false, + logs: false, + }, + }, + }, + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + }); + + const options = traceExporterCtor.mock.calls[0]?.[0] as { url?: string } | undefined; + expect(options?.url).toBe("https://collector.example.com/v1/traces"); + await service.stop?.(); + }); + + test("keeps signal-qualified endpoint unchanged when it has query params", async () => { + const service = createDiagnosticsOtelService(); + await service.start({ + config: { + diagnostics: { + enabled: true, + otel: { + enabled: true, + endpoint: "https://collector.example.com/v1/traces?timeout=30s", + protocol: "http/protobuf", + traces: true, + metrics: false, + logs: false, + }, + }, + }, + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + }); + + const options = traceExporterCtor.mock.calls[0]?.[0] as { url?: string } | undefined; + expect(options?.url).toBe("https://collector.example.com/v1/traces?timeout=30s"); + await service.stop?.(); + }); + + test("keeps signal-qualified endpoint unchanged when signal path casing differs", async () => { + const service = createDiagnosticsOtelService(); + await service.start({ + config: { + diagnostics: { + enabled: true, + otel: { + enabled: true, + endpoint: "https://collector.example.com/v1/Traces", + protocol: "http/protobuf", + traces: true, + metrics: false, + logs: false, + }, + }, + }, + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + }); + + const options = traceExporterCtor.mock.calls[0]?.[0] as { url?: string } | undefined; + expect(options?.url).toBe("https://collector.example.com/v1/Traces"); + await service.stop?.(); + }); }); diff --git a/extensions/diagnostics-otel/src/service.ts b/extensions/diagnostics-otel/src/service.ts index 101812b2e32..f1babf47327 100644 --- a/extensions/diagnostics-otel/src/service.ts +++ b/extensions/diagnostics-otel/src/service.ts @@ -23,7 +23,8 @@ function resolveOtelUrl(endpoint: string | undefined, path: string): string | un if (!endpoint) { return undefined; } - if (endpoint.includes("/v1/")) { + const endpointWithoutQueryOrFragment = endpoint.split(/[?#]/, 1)[0] ?? endpoint; + if (/\/v1\/(?:traces|metrics|logs)$/i.test(endpointWithoutQueryOrFragment)) { return endpoint; } return `${endpoint}/${path}`;