Skip to content

Commit

Permalink
feat(exporter-*-otlp-*)!: remove some environment variable code from …
Browse files Browse the repository at this point in the history 
…browser exporters (#4886)
  • Loading branch information
@pichlermarc
pichlermarc authored Jul 31, 2024
1 parent 08942ba commit 30a46ae
Show file tree
Hide file tree
Showing 9 changed files with 3 additions and 330 deletions.
2 changes: 2 additions & 0 deletions experimental/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ All notable changes to experimental packages in this project will be documented
* (user-facing) `hostname` was intended for use in tests and is not used by any exporters, it will be removed in a future release
* fix(exporter-*-otlp-*)!: ensure `User-Agent` header cannot be overwritten by the user [#4743](https://github.com/open-telemetry/opentelemetry-js/pull/4743) @pichlermarc
* allowing overrides of the `User-Agent` header was not specification compliant.
* feat(exporter-*-otlp*)!: remove environment-variable specific code from browser exporters
* (user-facing) removes the ability to configure browser exporters by using `process.env` polyfills

### :rocket: (Enhancement)

Expand Down
9 changes: 0 additions & 9 deletions experimental/packages/exporter-logs-otlp-http/src/platform/browser/OTLPLogExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import type {
import type { OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base';
import type { IExportLogsServiceResponse } from '@opentelemetry/otlp-transformer';
import { OTLPExporterBrowserBase } from '@opentelemetry/otlp-exporter-base';
import { baggageUtils, getEnv } from '@opentelemetry/core';
import { JsonLogsSerializer } from '@opentelemetry/otlp-transformer';

import { getDefaultUrl } from '../config';
Expand All @@ -34,21 +33,13 @@ export class OTLPLogExporter
implements LogRecordExporter
{
constructor(config: OTLPExporterConfigBase = {}) {
// load OTEL_EXPORTER_OTLP_LOGS_TIMEOUT env var
super(
{
timeoutMillis: getEnv().OTEL_EXPORTER_OTLP_LOGS_TIMEOUT,
...config,
},
JsonLogsSerializer,
'application/json'
);
this._headers = {
...this._headers,
...baggageUtils.parseKeyPairsIntoRecord(
getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS
),
};
}

getDefaultUrl(config: OTLPExporterConfigBase): string {
Expand Down
23 changes: 0 additions & 23 deletions experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,41 +24,18 @@ import { mockedReadableLogRecord } from '../logHelper';
import { ExportResultCode } from '@opentelemetry/core';

describe('OTLPLogExporter', () => {
let envSource: Record<string, any>;
let collectorExporter: OTLPLogExporter;
let collectorExporterConfig: OTLPExporterConfigBase;

afterEach(() => {
sinon.restore();
});

if (global.process?.versions?.node === undefined) {
envSource = globalThis as unknown as Record<string, any>;
} else {
envSource = process.env as Record<string, any>;
}

describe('constructor', () => {
it('should create an instance', () => {
const exporter = new OTLPLogExporter();
assert.ok(exporter instanceof OTLPLogExporter);
});

it('should use headers defined via env', () => {
envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=bar';
const exporter = new OTLPLogExporter();
assert.strictEqual(exporter['_headers'].foo, 'bar');
delete envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS;
});

it('should use timeout defined via env', () => {
envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = '';
envSource.OTEL_EXPORTER_OTLP_LOGS_TIMEOUT = 30000;
const exporter = new OTLPLogExporter();
assert.strictEqual(exporter.timeoutMillis, 30000);
delete envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS;
delete envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS;
});
});

describe('getDefaultUrl', () => {
Expand Down
20 changes: 0 additions & 20 deletions experimental/packages/exporter-logs-otlp-proto/src/platform/browser/OTLPLogExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,8 @@
* limitations under the License.
*/

import { getEnv, baggageUtils } from '@opentelemetry/core';
import {
OTLPExporterConfigBase,
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded,
OTLPExporterBrowserBase,
} from '@opentelemetry/otlp-exporter-base';
import {
Expand All @@ -40,30 +37,13 @@ export class OTLPLogExporter
{
constructor(config: OTLPExporterConfigBase = {}) {
super(config, ProtobufLogsSerializer, 'application/x-protobuf');
const env = getEnv();
this._headers = Object.assign(
this._headers,
baggageUtils.parseKeyPairsIntoRecord(env.OTEL_EXPORTER_OTLP_LOGS_HEADERS)
);
}

getDefaultUrl(config: OTLPExporterConfigBase): string {
if (typeof config.url === 'string') {
return config.url;
}

const env = getEnv();
if (env.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT.length > 0) {
return appendRootPathToUrlIfNeeded(env.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT);
}

if (env.OTEL_EXPORTER_OTLP_ENDPOINT.length > 0) {
return appendResourcePathToUrl(
env.OTEL_EXPORTER_OTLP_ENDPOINT,
DEFAULT_COLLECTOR_RESOURCE_PATH
);
}

return DEFAULT_COLLECTOR_URL;
}
}
26 changes: 1 addition & 25 deletions experimental/packages/exporter-trace-otlp-http/src/platform/browser/OTLPTraceExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@
*/

import { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base';
import { getEnv, baggageUtils } from '@opentelemetry/core';
import {
OTLPExporterConfigBase,
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded,
OTLPExporterBrowserBase,
} from '@opentelemetry/otlp-exporter-base';
import {
Expand All @@ -38,35 +35,14 @@ export class OTLPTraceExporter
implements SpanExporter
{
constructor(config: OTLPExporterConfigBase = {}) {
super(config, JsonTraceSerializer, 'application/json');
const env = getEnv();
this._headers = Object.assign(
this._headers,
baggageUtils.parseKeyPairsIntoRecord(
env.OTEL_EXPORTER_OTLP_TRACES_HEADERS
)
);
super(config, JsonTraceSerializer, `application/json`);
}

getDefaultUrl(config: OTLPExporterConfigBase): string {
if (typeof config.url === 'string') {
return config.url;
}

const env = getEnv();
if (env.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0) {
return appendRootPathToUrlIfNeeded(
env.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT
);
}

if (env.OTEL_EXPORTER_OTLP_ENDPOINT.length > 0) {
return appendResourcePathToUrl(
env.OTEL_EXPORTER_OTLP_ENDPOINT,
DEFAULT_COLLECTOR_RESOURCE_PATH
);
}

return DEFAULT_COLLECTOR_URL;
}
}
96 changes: 0 additions & 96 deletions experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,102 +490,6 @@ describe('OTLPTraceExporter - browser (getDefaultUrl)', () => {
});
});

describe('when configuring via environment', () => {
const envSource = window as any;
it('should use url defined in env that ends with root path and append version and signal path', () => {
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar/';
const collectorExporter = new OTLPTraceExporter();
assert.strictEqual(
collectorExporter.url,
`${envSource.OTEL_EXPORTER_OTLP_ENDPOINT}v1/traces`
);
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = '';
});
it('should use url defined in env without checking if path is already present', () => {
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar/v1/traces';
const collectorExporter = new OTLPTraceExporter();
assert.strictEqual(
collectorExporter.url,
`${envSource.OTEL_EXPORTER_OTLP_ENDPOINT}/v1/traces`
);
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = '';
});
it('should use url defined in env and append version and signal', () => {
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar';
const collectorExporter = new OTLPTraceExporter();
assert.strictEqual(
collectorExporter.url,
`${envSource.OTEL_EXPORTER_OTLP_ENDPOINT}/v1/traces`
);
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = '';
});
it('should override global exporter url with signal url defined in env', () => {
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar/';
envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = 'http://foo.traces/';
const collectorExporter = new OTLPTraceExporter();
assert.strictEqual(
collectorExporter.url,
envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT
);
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = '';
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();
assert.strictEqual(
collectorExporter.url,
`${envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT}/`
);
envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = '';
});
it('should not add root path when signal url defined in env contains root path but no path', () => {
envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = 'http://foo.bar/';
const collectorExporter = new OTLPTraceExporter();
assert.strictEqual(
collectorExporter.url,
`${envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT}`
);
envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = '';
});
it('should not add root path when signal url defined in env contains path', () => {
envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = 'http://foo.bar/v1/traces';
const collectorExporter = new OTLPTraceExporter();
assert.strictEqual(
collectorExporter.url,
`${envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT}`
);
envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = '';
});
it('should not add root path when signal url defined in env contains path and ends in /', () => {
envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = 'http://foo.bar/v1/traces/';
const collectorExporter = new OTLPTraceExporter();
assert.strictEqual(
collectorExporter.url,
`${envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT}`
);
envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = '';
});
it('should use headers defined via env', () => {
envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar';
const collectorExporter = new OTLPTraceExporter({ headers: {} });
// @ts-expect-error access internal property for testing
assert.strictEqual(collectorExporter._headers.foo, 'bar');
envSource.OTEL_EXPORTER_OTLP_HEADERS = '';
});
it('should override global headers config with signal headers defined via env', () => {
envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo';
envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = 'foo=boo';
const collectorExporter = new OTLPTraceExporter({ headers: {} });
// @ts-expect-error access internal property for testing
assert.strictEqual(collectorExporter._headers.foo, 'boo');
// @ts-expect-error access internal property for testing
assert.strictEqual(collectorExporter._headers.bar, 'foo');
envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = '';
envSource.OTEL_EXPORTER_OTLP_HEADERS = '';
});
});

describe('export with retry - real http request destroyed', () => {
let server: any;
let collectorTraceExporter: OTLPTraceExporter;
Expand Down
24 changes: 0 additions & 24 deletions experimental/packages/exporter-trace-otlp-proto/src/platform/browser/OTLPTraceExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@
*/

import { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base';
import { getEnv, baggageUtils } from '@opentelemetry/core';
import {
OTLPExporterConfigBase,
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded,
OTLPExporterBrowserBase,
} from '@opentelemetry/otlp-exporter-base';
import {
Expand All @@ -39,34 +36,13 @@ export class OTLPTraceExporter
{
constructor(config: OTLPExporterConfigBase = {}) {
super(config, ProtobufTraceSerializer, 'application/x-protobuf');
const env = getEnv();
this._headers = Object.assign(
this._headers,
baggageUtils.parseKeyPairsIntoRecord(
env.OTEL_EXPORTER_OTLP_TRACES_HEADERS
)
);
}

getDefaultUrl(config: OTLPExporterConfigBase): string {
if (typeof config.url === 'string') {
return config.url;
}

const env = getEnv();
if (env.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0) {
return appendRootPathToUrlIfNeeded(
env.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT
);
}

if (env.OTEL_EXPORTER_OTLP_ENDPOINT.length > 0) {
return appendResourcePathToUrl(
env.OTEL_EXPORTER_OTLP_ENDPOINT,
DEFAULT_COLLECTOR_RESOURCE_PATH
);
}

return DEFAULT_COLLECTOR_URL;
}
}
24 changes: 0 additions & 24 deletions ...kages/opentelemetry-exporter-metrics-otlp-http/src/platform/browser/OTLPMetricExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,11 @@
*/

import { ResourceMetrics } from '@opentelemetry/sdk-metrics';
import { baggageUtils, getEnv } from '@opentelemetry/core';
import { OTLPMetricExporterOptions } from '../../OTLPMetricExporterOptions';
import { OTLPMetricExporterBase } from '../../OTLPMetricExporterBase';
import {
OTLPExporterBrowserBase,
OTLPExporterConfigBase,
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded,
} from '@opentelemetry/otlp-exporter-base';
import {
IExportMetricsServiceResponse,
Expand All @@ -38,34 +35,13 @@ class OTLPExporterBrowserProxy extends OTLPExporterBrowserBase<
> {
constructor(config?: OTLPMetricExporterOptions & OTLPExporterConfigBase) {
super(config, JsonMetricsSerializer, 'application/json');
const env = getEnv();
this._headers = Object.assign(
this._headers,
baggageUtils.parseKeyPairsIntoRecord(
env.OTEL_EXPORTER_OTLP_METRICS_HEADERS
)
);
}

getDefaultUrl(config: OTLPExporterConfigBase): string {
if (typeof config.url === 'string') {
return config.url;
}

const env = getEnv();
if (env.OTEL_EXPORTER_OTLP_METRICS_ENDPOINT.length > 0) {
return appendRootPathToUrlIfNeeded(
env.OTEL_EXPORTER_OTLP_METRICS_ENDPOINT
);
}

if (env.OTEL_EXPORTER_OTLP_ENDPOINT.length > 0) {
return appendResourcePathToUrl(
env.OTEL_EXPORTER_OTLP_ENDPOINT,
DEFAULT_COLLECTOR_RESOURCE_PATH
);
}

return DEFAULT_COLLECTOR_URL;
}
}
Expand Down
Loading

0 comments on commit 30a46ae

Please sign in to comment.