Merge f3f0ffd into 2fb1b30

open-telemetry · May 29, 2023 · bdab866 · bdab866
2 parents 2fb1b30 + f3f0ffd
commit bdab866
Show file tree

Hide file tree

Showing 28 changed files with 602 additions and 971 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -17,11 +17,13 @@ All notable changes to experimental packages in this project will be documented
 * feat(exporter-logs-otlp-http): otlp-http exporter for logs. [#3764](https://github.com/open-telemetry/opentelemetry-js/pull/3764/) @fuaiyi
 * feat(otlp-trace-exporters): Add User-Agent header to OTLP trace exporters. [#3790](https://github.com/open-telemetry/opentelemetry-js/pull/3790) @JamieDanielson
 * feat(otlp-metric-exporters): Add User-Agent header to OTLP metric exporters. [#3806](https://github.com/open-telemetry/opentelemetry-js/pull/3806) @JamieDanielson
+* feat(otlp-exporters): Move User-Agent header to OTLP exporter base packages. [#3811](https://github.com/open-telemetry/opentelemetry-js/pull/3811) @llc1123
 
 ### :bug: (Bug Fix)
 
 * fix(sdk-node): use resource interface instead of concrete class [#3803](https://github.com/open-telemetry/opentelemetry-js/pull/3803) @blumamir
 * fix(sdk-logs): remove includeTraceContext configuration and use LogRecord context when available  [#3817](https://github.com/open-telemetry/opentelemetry-js/pull/3817) @hectorhdzg
+* fix(otlp-exporters): correct applying order of http headers/gRPC metadata [#3811](https://github.com/open-telemetry/opentelemetry-js/pull/3811) @llc1123
 
 ### :books: (Refine Doc)
 

diff --git a/experimental/packages/exporter-logs-otlp-grpc/src/OTLPLogExporter.ts b/experimental/packages/exporter-logs-otlp-grpc/src/OTLPLogExporter.ts
@@ -37,14 +37,22 @@ export class OTLPLogExporter
   implements LogRecordExporter
 {
   constructor(config: OTLPGRPCExporterConfigNode = {}) {
-    super(config);
     const headers = baggageUtils.parseKeyPairsIntoRecord(
       getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS
     );
-    this.metadata ||= new Metadata();
+    const metadata = new Metadata();
     for (const [k, v] of Object.entries(headers)) {
-      this.metadata.set(k, v);
+      metadata.set(k, v);
     }
+    if (config.metadata) {
+      for (const [k, v] of Object.entries(config.metadata.getMap())) {
+        metadata.set(k, v);
+      }
+    }
+    super({
+      ...config,
+      metadata,
+    });
   }
 
   convert(logRecords: ReadableLogRecord[]): IExportLogsServiceRequest {

diff --git a/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts
@@ -37,6 +37,7 @@ import {
   IExportLogsServiceRequest,
   IResourceLogs,
 } from '@opentelemetry/otlp-transformer';
+import { VERSION } from '../src/version';
 
 const logsServiceProtoPath =
   'opentelemetry/proto/collector/logs/v1/logs_service.proto';
@@ -326,16 +327,19 @@ describe('when configuring via environment', () => {
     assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['bar']);
     envSource.OTEL_EXPORTER_OTLP_HEADERS = '';
   });
-  it('should override global headers config with signal headers defined via env', () => {
+  it('should override global headers config with signal headers defined via env but not config from parameters', () => {
     const metadata = new grpc.Metadata();
-    metadata.set('foo', 'bar');
-    metadata.set('goo', 'lol');
-    envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=jar,bar=foo';
-    envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=boo';
+    metadata.set('foo', 'jar');
+    envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo,goo=loo';
+    envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS =
+      'foo=boo,bar=loo,User-Agent=baz';
     const collectorExporter = new OTLPLogExporter({ metadata });
-    assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['boo']);
-    assert.deepStrictEqual(collectorExporter.metadata?.get('bar'), ['foo']);
-    assert.deepStrictEqual(collectorExporter.metadata?.get('goo'), ['lol']);
+    assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['jar']);
+    assert.deepStrictEqual(collectorExporter.metadata?.get('bar'), ['loo']);
+    assert.deepStrictEqual(collectorExporter.metadata?.get('goo'), ['loo']);
+    assert.deepStrictEqual(collectorExporter.metadata?.get('User-Agent'), [
+      `OTel-OTLP-Exporter-JavaScript/${VERSION}`,
+    ]);
     envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = '';
     envSource.OTEL_EXPORTER_OTLP_HEADERS = '';
   });

diff --git a/experimental/packages/exporter-trace-otlp-grpc/src/OTLPTraceExporter.ts b/experimental/packages/exporter-trace-otlp-grpc/src/OTLPTraceExporter.ts
@@ -28,11 +28,6 @@ import {
   createExportTraceServiceRequest,
   IExportTraceServiceRequest,
 } from '@opentelemetry/otlp-transformer';
-import { VERSION } from './version';
-
-const USER_AGENT = {
-  'User-Agent': `OTel-OTLP-Exporter-JavaScript/${VERSION}`,
-};
 
 /**
  * OTLP Trace Exporter for Node
@@ -42,17 +37,22 @@ export class OTLPTraceExporter
   implements SpanExporter
 {
   constructor(config: OTLPGRPCExporterConfigNode = {}) {
-    super(config);
-    const headers = {
-      ...USER_AGENT,
-      ...baggageUtils.parseKeyPairsIntoRecord(
-        getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS
-      ),
-    };
-    this.metadata ||= new Metadata();
+    const headers = baggageUtils.parseKeyPairsIntoRecord(
+      getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS
+    );
+    const metadata = new Metadata();
     for (const [k, v] of Object.entries(headers)) {
-      this.metadata.set(k, v);
+      metadata.set(k, v);
+    }
+    if (config.metadata) {
+      for (const [k, v] of Object.entries(config.metadata.getMap())) {
+        metadata.set(k, v);
+      }
     }
+    super({
+      ...config,
+      metadata,
+    });
   }
 
   convert(spans: ReadableSpan[]): IExportTraceServiceRequest {

diff --git a/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts
@@ -27,7 +27,6 @@ import * as grpc from '@grpc/grpc-js';
 import * as path from 'path';
 import * as sinon from 'sinon';
 import { OTLPTraceExporter } from '../src';
-import { VERSION } from '../src/version';
 
 import {
   ensureExportedSpanIsCorrect,
@@ -42,6 +41,7 @@ import {
   IExportTraceServiceRequest,
   IResourceSpans,
 } from '@opentelemetry/otlp-transformer';
+import { VERSION } from '../src/version';
 
 const traceServiceProtoPath =
   'opentelemetry/proto/collector/trace/v1/trace_service.proto';
@@ -337,22 +337,19 @@ describe('when configuring via environment', () => {
     assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['bar']);
     envSource.OTEL_EXPORTER_OTLP_HEADERS = '';
   });
-  it('should include user agent in header', () => {
-    const collectorExporter = new OTLPTraceExporter();
+  it('should override global headers config with signal headers defined via env but not config from parameters', () => {
+    const metadata = new grpc.Metadata();
+    metadata.set('foo', 'jar');
+    envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo,goo=loo';
+    envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS =
+      'foo=boo,bar=loo,User-Agent=baz';
+    const collectorExporter = new OTLPTraceExporter({ metadata });
+    assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['jar']);
+    assert.deepStrictEqual(collectorExporter.metadata?.get('bar'), ['loo']);
+    assert.deepStrictEqual(collectorExporter.metadata?.get('goo'), ['loo']);
     assert.deepStrictEqual(collectorExporter.metadata?.get('User-Agent'), [
       `OTel-OTLP-Exporter-JavaScript/${VERSION}`,
     ]);
-  });
-  it('should override global headers config with signal headers defined via env', () => {
-    const metadata = new grpc.Metadata();
-    metadata.set('foo', 'bar');
-    metadata.set('goo', 'lol');
-    envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=jar,bar=foo';
-    envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = 'foo=boo';
-    const collectorExporter = new OTLPTraceExporter({ metadata });
-    assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['boo']);
-    assert.deepStrictEqual(collectorExporter.metadata?.get('bar'), ['foo']);
-    assert.deepStrictEqual(collectorExporter.metadata?.get('goo'), ['lol']);
     envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = '';
     envSource.OTEL_EXPORTER_OTLP_HEADERS = '';
   });

diff --git a/experimental/packages/exporter-trace-otlp-http/src/platform/browser/OTLPTraceExporter.ts b/experimental/packages/exporter-trace-otlp-http/src/platform/browser/OTLPTraceExporter.ts
@@ -38,13 +38,15 @@ export class OTLPTraceExporter
   implements SpanExporter
 {
   constructor(config: OTLPExporterConfigBase = {}) {
-    super(config);
-    this._headers = Object.assign(
-      this._headers,
-      baggageUtils.parseKeyPairsIntoRecord(
-        getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS
-      )
-    );
+    super({
+      ...config,
+      headers: {
+        ...baggageUtils.parseKeyPairsIntoRecord(
+          getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS
+        ),
+        ...config.headers,
+      },
+    });
   }
   convert(spans: ReadableSpan[]): IExportTraceServiceRequest {
     return createExportTraceServiceRequest(spans, true);

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
@@ -26,13 +26,9 @@ import {
   createExportTraceServiceRequest,
   IExportTraceServiceRequest,
 } from '@opentelemetry/otlp-transformer';
-import { VERSION } from '../../version';
 
 const DEFAULT_COLLECTOR_RESOURCE_PATH = 'v1/traces';
 const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURCE_PATH}`;
-const USER_AGENT = {
-  'User-Agent': `OTel-OTLP-Exporter-JavaScript/${VERSION}`,
-};
 
 /**
  * Collector Trace Exporter for Node
@@ -42,14 +38,15 @@ export class OTLPTraceExporter
   implements SpanExporter
 {
   constructor(config: OTLPExporterNodeConfigBase = {}) {
-    super(config);
-    this.headers = {
-      ...this.headers,
-      ...USER_AGENT,
-      ...baggageUtils.parseKeyPairsIntoRecord(
-        getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS
-      ),
-    };
+    super({
+      ...config,
+      headers: {
+        ...baggageUtils.parseKeyPairsIntoRecord(
+          getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS
+        ),
+        ...config.headers,
+      },
+    });
   }
 
   convert(spans: ReadableSpan[]): IExportTraceServiceRequest {