Skip to content

Commit

Permalink
fix(otlp-http-exporter): update endpoint to match spec (#2895)
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Dyla <[email protected]>
Co-authored-by: Chengzhong Wu <[email protected]>
  • Loading branch information
3 people authored May 17, 2022
1 parent a9c59da commit 69cced2
Show file tree
Hide file tree
Showing 15 changed files with 428 additions and 69 deletions.
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ All notable changes to experimental packages in this project will be documented

* fix(opentelemetry-instrumentation-http): use correct origin when port is `null` #2948 @danielgblanco
* fix(otlp-exporter-base): include esm and esnext in package files #2952 @dyladan
* fix(otlp-http-exporter): update endpoint to match spec #2895 @svetlanabrennan

### :books: (Refine Doc)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,18 @@
* limitations under the License.
*/

import { appendResourcePathToUrlIfNotPresent, OTLPExporterBrowserBase } from '@opentelemetry/otlp-exporter-base';
import { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base';
import { getEnv, baggageUtils } from '@opentelemetry/core';
import { OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base';
import {
OTLPExporterConfigBase,
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded,
OTLPExporterBrowserBase
} from '@opentelemetry/otlp-exporter-base';
import { createExportTraceServiceRequest, IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer';

const DEFAULT_COLLECTOR_RESOURCE_PATH = '/v1/traces';
const DEFAULT_COLLECTOR_URL=`http://localhost:4318${DEFAULT_COLLECTOR_RESOURCE_PATH}`;
const DEFAULT_COLLECTOR_RESOURCE_PATH = 'v1/traces';
const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURCE_PATH}`;

/**
* Collector Trace Exporter for Web
Expand Down Expand Up @@ -49,9 +53,9 @@ export class OTLPTraceExporter
return typeof config.url === 'string'
? config.url
: getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0
? getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT
? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
: getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0
? appendResourcePathToUrlIfNotPresent(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
: DEFAULT_COLLECTOR_URL;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ import { getEnv, baggageUtils } from '@opentelemetry/core';
import { OTLPExporterNodeBase } from '@opentelemetry/otlp-exporter-base';
import {
OTLPExporterNodeConfigBase,
appendResourcePathToUrlIfNotPresent
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded
} from '@opentelemetry/otlp-exporter-base';
import { createExportTraceServiceRequest, IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer';

const DEFAULT_COLLECTOR_RESOURCE_PATH = '/v1/traces';
const DEFAULT_COLLECTOR_URL = `http://localhost:4318${DEFAULT_COLLECTOR_RESOURCE_PATH}`;
const DEFAULT_COLLECTOR_RESOURCE_PATH = 'v1/traces';
const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURCE_PATH}`;

/**
* Collector Trace Exporter for Node
Expand All @@ -51,9 +52,9 @@ export class OTLPTraceExporter
return typeof config.url === 'string'
? config.url
: getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0
? getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT
? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
: getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0
? appendResourcePathToUrlIfNotPresent(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
: DEFAULT_COLLECTOR_URL;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -508,16 +508,25 @@ describe('OTLPTraceExporter - browser (getDefaultUrl)', () => {

describe('when configuring via environment', () => {
const envSource = window as any;
it('should use url defined in env', () => {
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
`${envSource.OTEL_EXPORTER_OTLP_ENDPOINT}/v1/traces`
);
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = '';
});
it('should use url defined in env and append version and signal when not present', () => {
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(
Expand All @@ -527,8 +536,8 @@ describe('when configuring via environment', () => {
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';
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,
Expand All @@ -537,6 +546,42 @@ describe('when configuring via environment', () => {
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: {} });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,25 @@ describe('OTLPTraceExporter - node with json over http', () => {

describe('when configuring via environment', () => {
const envSource = process.env;
it('should use url defined in env', () => {
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
`${envSource.OTEL_EXPORTER_OTLP_ENDPOINT}/v1/traces`
);
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = '';
});
it('should use url defined in env and append version and signal when not present', () => {
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(
Expand All @@ -101,8 +110,8 @@ describe('OTLPTraceExporter - node with json over http', () => {
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';
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,
Expand All @@ -111,6 +120,42 @@ describe('OTLPTraceExporter - node with json over http', () => {
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,16 @@

import { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base';
import { getEnv, baggageUtils } from '@opentelemetry/core';
import { appendResourcePathToUrlIfNotPresent, OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base';
import {
OTLPExporterNodeConfigBase,
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded
} from '@opentelemetry/otlp-exporter-base';
import { OTLPProtoExporterNodeBase, ServiceClientType } from '@opentelemetry/otlp-proto-exporter-base';
import { createExportTraceServiceRequest, IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer';

const DEFAULT_COLLECTOR_RESOURCE_PATH = '/v1/traces';
const DEFAULT_COLLECTOR_URL=`http://localhost:4318${DEFAULT_COLLECTOR_RESOURCE_PATH}`;
const DEFAULT_COLLECTOR_RESOURCE_PATH = 'v1/traces';
const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURCE_PATH}`;

/**
* Collector Trace Exporter for Node with protobuf
Expand Down Expand Up @@ -50,9 +54,9 @@ export class OTLPTraceExporter
return typeof config.url === 'string'
? config.url
: getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0
? getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT
? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
: getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0
? appendResourcePathToUrlIfNotPresent(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
: DEFAULT_COLLECTOR_URL;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,25 @@ describe('OTLPTraceExporter - node with proto over http', () => {

describe('when configuring via environment', () => {
const envSource = process.env;
it('should use url defined in env', () => {
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
`${envSource.OTEL_EXPORTER_OTLP_ENDPOINT}/v1/traces`
);
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = '';
});
it('should use url defined in env and append version and signal when not present', () => {
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(
Expand All @@ -66,8 +75,8 @@ describe('OTLPTraceExporter - node with proto over http', () => {
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';
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,
Expand All @@ -76,6 +85,42 @@ describe('OTLPTraceExporter - node with proto over http', () => {
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ import { baggageUtils, getEnv } from '@opentelemetry/core';
import { defaultOptions, OTLPMetricExporterOptions } from '../../OTLPMetricExporterOptions';
import { OTLPMetricExporterBase } from '../../OTLPMetricExporterBase';
import {
appendResourcePathToUrlIfNotPresent,
OTLPExporterBrowserBase,
OTLPExporterConfigBase
OTLPExporterConfigBase,
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded
} from '@opentelemetry/otlp-exporter-base';
import { createExportMetricsServiceRequest, IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer';

const DEFAULT_COLLECTOR_RESOURCE_PATH = '/v1/metrics';
const DEFAULT_COLLECTOR_URL = `http://localhost:4318${DEFAULT_COLLECTOR_RESOURCE_PATH}`;
const DEFAULT_COLLECTOR_RESOURCE_PATH = 'v1/metrics';
const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURCE_PATH}`;

class OTLPExporterBrowserProxy extends OTLPExporterBrowserBase<ResourceMetrics, IExportMetricsServiceRequest> {

Expand All @@ -44,9 +45,9 @@ class OTLPExporterBrowserProxy extends OTLPExporterBrowserBase<ResourceMetrics,
return typeof config.url === 'string'
? config.url
: getEnv().OTEL_EXPORTER_OTLP_METRICS_ENDPOINT.length > 0
? getEnv().OTEL_EXPORTER_OTLP_METRICS_ENDPOINT
? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
: getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0
? appendResourcePathToUrlIfNotPresent(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH)
: DEFAULT_COLLECTOR_URL;
}

Expand Down
Loading

0 comments on commit 69cced2

Please sign in to comment.