From ae0a3c522d0b802de264e4d8b9566d3677a36ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladimir=20Adami=C4=87?= <441333+Vunovati@users.noreply.github.com> Date: Wed, 3 Jan 2024 17:42:17 +0100 Subject: [PATCH] =?UTF-8?q?fix(exporter-logs-otlp-proto):=20programatic=20?= =?UTF-8?q?headers=20take=20precedence=20ov=E2=80=A6=20(#4351)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(exporter-logs-otlp-proto): programatic headers take precedence over environment variables * chore: update PR url in changelog * chore: fix deletion of env var * fix(exporter-logs-otlp-http): programatic headers take precedence over environment variables * fix(exporter-trace-otlp-http): programatic headers take precedence over environment variables * fix(exporter-trace-otlp-proto): programatic headers take precedence over environment variable * chore: update CHANGELOG --------- Co-authored-by: Marc Pichler --- experimental/CHANGELOG.md | 5 +++++ .../src/platform/node/OTLPLogExporter.ts | 9 +++++---- .../test/node/OTLPLogExporter.test.ts | 12 +++++++++++ .../src/platform/node/OTLPLogExporter.ts | 3 ++- .../test/node/OTLPLogExporter.test.ts | 20 +++++++++++++++++++ .../src/platform/node/OTLPTraceExporter.ts | 1 + .../test/node/CollectorTraceExporter.test.ts | 20 +++++++++++++++++++ .../src/platform/node/OTLPTraceExporter.ts | 1 + .../test/node/OTLPTraceExporter.test.ts | 20 +++++++++++++++++++ 9 files changed, 86 insertions(+), 5 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index abec890a38..845a7e7ad6 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -6,6 +6,11 @@ All notable changes to experimental packages in this project will be documented ### :boom: Breaking Change +* fix(exporter-logs-otlp-http): programatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4351) @Vunovati +* fix(exporter-logs-otlp-proto): programatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4351) @Vunovati +* fix(exporter-trace-otlp-http): programatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4351) @Vunovati +* fix(exporter-trace-otlp-proto): programatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4351) @Vunovati + ### :rocket: (Enhancement) ### :bug: (Bug Fix) diff --git a/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts b/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts index 25a1b194ab..56e4bad3d6 100644 --- a/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts +++ b/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts @@ -39,12 +39,13 @@ export class OTLPLogExporter timeoutMillis: getEnv().OTEL_EXPORTER_OTLP_LOGS_TIMEOUT, ...config, }); - this.headers = { - ...this.headers, - ...baggageUtils.parseKeyPairsIntoRecord( + this.headers = Object.assign( + this.headers, + baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS ), - }; + config.headers + ); } convert(logRecords: ReadableLogRecord[]): IExportLogsServiceRequest { diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index 6dac23b580..83b389628a 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -94,6 +94,18 @@ describe('OTLPLogExporter', () => { delete envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS; delete envSource.OTEL_EXPORTER_OTLP_LOGS_TIMEOUT; }); + + it('should override headers defined via env with headers defined in constructor', () => { + envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; + const collectorExporter = new OTLPLogExporter({ + headers: { + foo: 'constructor', + }, + }); + assert.strictEqual(collectorExporter.headers.foo, 'constructor'); + assert.strictEqual(collectorExporter.headers.bar, 'foo'); + envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; + }); }); describe('getDefaultUrl', () => { diff --git a/experimental/packages/exporter-logs-otlp-proto/src/platform/node/OTLPLogExporter.ts b/experimental/packages/exporter-logs-otlp-proto/src/platform/node/OTLPLogExporter.ts index 53191c0625..a8cdfc4c96 100644 --- a/experimental/packages/exporter-logs-otlp-proto/src/platform/node/OTLPLogExporter.ts +++ b/experimental/packages/exporter-logs-otlp-proto/src/platform/node/OTLPLogExporter.ts @@ -50,7 +50,8 @@ export class OTLPLogExporter this.headers, baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS - ) + ), + config.headers ); } convert(logs: ReadableLogRecord[]): IExportLogsServiceRequest { diff --git a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts index 0810109e81..ab918c992b 100644 --- a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts @@ -92,6 +92,15 @@ describe('OTLPLogExporter - node with proto over http', () => { envSource.OTEL_EXPORTER_OTLP_ENDPOINT = ''; envSource.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT = ''; }); + it('should override url defined in env with url defined in constructor', () => { + envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar/'; + const constructorDefinedEndpoint = 'http://constructor/v1/logs'; + const collectorExporter = new OTLPLogExporter({ + url: constructorDefinedEndpoint, + }); + assert.strictEqual(collectorExporter.url, constructorDefinedEndpoint); + envSource.OTEL_EXPORTER_OTLP_ENDPOINT = ''; + }); it('should add root path when signal url defined in env contains no path and no root path', () => { envSource.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT = 'http://foo.bar'; const collectorExporter = new OTLPLogExporter(); @@ -143,6 +152,17 @@ describe('OTLPLogExporter - node with proto over http', () => { envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); + it('should override headers defined via env with headers defined in constructor', () => { + envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; + const collectorExporter = new OTLPLogExporter({ + headers: { + foo: 'constructor', + }, + }); + assert.strictEqual(collectorExporter.headers.foo, 'constructor'); + assert.strictEqual(collectorExporter.headers.bar, 'foo'); + envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; + }); }); describe('export', () => { diff --git a/experimental/packages/exporter-trace-otlp-http/src/platform/node/OTLPTraceExporter.ts b/experimental/packages/exporter-trace-otlp-http/src/platform/node/OTLPTraceExporter.ts index e4d3273239..aeb3b94ca7 100644 --- a/experimental/packages/exporter-trace-otlp-http/src/platform/node/OTLPTraceExporter.ts +++ b/experimental/packages/exporter-trace-otlp-http/src/platform/node/OTLPTraceExporter.ts @@ -49,6 +49,7 @@ export class OTLPTraceExporter ...baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS ), + ...config.headers, }; } diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 3de60027dc..e9af4ec37b 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -118,6 +118,15 @@ describe('OTLPTraceExporter - node with json over http', () => { envSource.OTEL_EXPORTER_OTLP_ENDPOINT = ''; envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = ''; }); + it('should override url defined in env with url defined in constructor', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = 'http://foo.bar'; + const constructorDefinedEndpoint = 'http://constructor/v1/traces'; + const collectorExporter = new OTLPTraceExporter({ + url: constructorDefinedEndpoint, + }); + assert.strictEqual(collectorExporter.url, constructorDefinedEndpoint); + envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = ''; + }); it('should add root path when signal url defined in env contains no path and no root path', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = 'http://foo.bar'; const collectorExporter = new OTLPTraceExporter(); @@ -177,6 +186,17 @@ describe('OTLPTraceExporter - node with json over http', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); + it('should override headers defined via env with headers defined in constructor', () => { + envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; + const collectorExporter = new OTLPTraceExporter({ + headers: { + foo: 'constructor', + }, + }); + assert.strictEqual(collectorExporter.headers.foo, 'constructor'); + assert.strictEqual(collectorExporter.headers.bar, 'foo'); + envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; + }); it('should use compression defined via env', () => { envSource.OTEL_EXPORTER_OTLP_COMPRESSION = 'gzip'; const collectorExporter = new OTLPTraceExporter(); diff --git a/experimental/packages/exporter-trace-otlp-proto/src/platform/node/OTLPTraceExporter.ts b/experimental/packages/exporter-trace-otlp-proto/src/platform/node/OTLPTraceExporter.ts index 210a16145a..be115583cd 100644 --- a/experimental/packages/exporter-trace-otlp-proto/src/platform/node/OTLPTraceExporter.ts +++ b/experimental/packages/exporter-trace-otlp-proto/src/platform/node/OTLPTraceExporter.ts @@ -52,6 +52,7 @@ export class OTLPTraceExporter ...baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS ), + ...config.headers, }; } diff --git a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts index c0a604ce90..b18c5a39de 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts @@ -103,6 +103,15 @@ describe('OTLPTraceExporter - node with proto over http', () => { envSource.OTEL_EXPORTER_OTLP_ENDPOINT = ''; envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = ''; }); + it('should override url defined in env with url defined in constructor', () => { + envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar/'; + const constructorDefinedEndpoint = 'http://constructor/v1/traces'; + const collectorExporter = new OTLPTraceExporter({ + url: constructorDefinedEndpoint, + }); + assert.strictEqual(collectorExporter.url, constructorDefinedEndpoint); + envSource.OTEL_EXPORTER_OTLP_ENDPOINT = ''; + }); it('should add root path when signal url defined in env contains no path and no root path', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = 'http://foo.bar'; const collectorExporter = new OTLPTraceExporter(); @@ -155,6 +164,17 @@ describe('OTLPTraceExporter - node with proto over http', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); + it('should override headers defined via env with headers defined in constructor', () => { + envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; + const collectorExporter = new OTLPTraceExporter({ + headers: { + foo: 'constructor', + }, + }); + assert.strictEqual(collectorExporter.headers.foo, 'constructor'); + assert.strictEqual(collectorExporter.headers.bar, 'foo'); + envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; + }); }); describe('export', () => {