From 609ecc59af384afea980902820631e3cadfc4378 Mon Sep 17 00:00:00 2001 From: "marc.pichler" Date: Fri, 23 Sep 2022 11:20:42 +0200 Subject: [PATCH 1/3] fix(exporters): do not append trailing '/' when URL contains path --- .../src/platform/browser/OTLPTraceExporter.ts | 2 +- .../src/platform/node/OTLPTraceExporter.ts | 2 +- .../src/OTLPTraceExporter.ts | 2 +- .../platform/browser/OTLPMetricExporter.ts | 2 +- .../src/platform/node/OTLPMetricExporter.ts | 2 +- .../src/OTLPMetricExporter.ts | 2 +- .../packages/otlp-exporter-base/src/util.ts | 17 +++++++---- .../test/common/util.test.ts | 30 +++++++++++++------ 8 files changed, 38 insertions(+), 21 deletions(-) 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 index ff478ead7c..ec12def8f2 100644 --- a/experimental/packages/exporter-trace-otlp-http/src/platform/browser/OTLPTraceExporter.ts +++ b/experimental/packages/exporter-trace-otlp-http/src/platform/browser/OTLPTraceExporter.ts @@ -53,7 +53,7 @@ export class OTLPTraceExporter return typeof config.url === 'string' ? config.url : getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0 - ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) + ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) : getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0 ? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) : DEFAULT_COLLECTOR_URL; 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 aeec62259b..528a2735c3 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 @@ -52,7 +52,7 @@ export class OTLPTraceExporter return typeof config.url === 'string' ? config.url : getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0 - ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) + ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) : getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0 ? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) : DEFAULT_COLLECTOR_URL; diff --git a/experimental/packages/exporter-trace-otlp-proto/src/OTLPTraceExporter.ts b/experimental/packages/exporter-trace-otlp-proto/src/OTLPTraceExporter.ts index f523110024..d5c40a8214 100644 --- a/experimental/packages/exporter-trace-otlp-proto/src/OTLPTraceExporter.ts +++ b/experimental/packages/exporter-trace-otlp-proto/src/OTLPTraceExporter.ts @@ -54,7 +54,7 @@ export class OTLPTraceExporter return typeof config.url === 'string' ? config.url : getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0 - ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) + ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) : getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0 ? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) : DEFAULT_COLLECTOR_URL; diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/browser/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/browser/OTLPMetricExporter.ts index 2a7b14d219..afc42de108 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/browser/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/browser/OTLPMetricExporter.ts @@ -45,7 +45,7 @@ class OTLPExporterBrowserProxy extends OTLPExporterBrowserBase 0 - ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) + ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_METRICS_ENDPOINT) : getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0 ? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) : DEFAULT_COLLECTOR_URL; diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts index 2008b88662..579fe95696 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts @@ -49,7 +49,7 @@ class OTLPExporterNodeProxy extends OTLPExporterNodeBase 0 - ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) + ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_METRICS_ENDPOINT) : getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0 ? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) : DEFAULT_COLLECTOR_URL; diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts index dec361e522..e3d8a60155 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts @@ -52,7 +52,7 @@ class OTLPMetricExporterNodeProxy extends OTLPProtoExporterNodeBase 0 - ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) + ? appendRootPathToUrlIfNeeded(getEnv().OTEL_EXPORTER_OTLP_METRICS_ENDPOINT) : getEnv().OTEL_EXPORTER_OTLP_ENDPOINT.length > 0 ? appendResourcePathToUrl(getEnv().OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_COLLECTOR_RESOURCE_PATH) : DEFAULT_COLLECTOR_URL; diff --git a/experimental/packages/otlp-exporter-base/src/util.ts b/experimental/packages/otlp-exporter-base/src/util.ts index bfe9209758..6290153bcb 100644 --- a/experimental/packages/otlp-exporter-base/src/util.ts +++ b/experimental/packages/otlp-exporter-base/src/util.ts @@ -53,14 +53,19 @@ export function appendResourcePathToUrl(url: string, path: string): string { /** * Adds root path to signal specific endpoint when endpoint contains no path part and no root path * @param url - * @param path * @returns url */ -export function appendRootPathToUrlIfNeeded(url: string, path: string): string { - if (!url.includes(path) && !url.endsWith('/')) { - url = url + '/'; +export function appendRootPathToUrlIfNeeded(url: string): string { + try { + const parsedUrl = new URL(url); + if (parsedUrl.pathname === '') { + parsedUrl.pathname = parsedUrl.pathname + '/'; + } + return parsedUrl.toString(); + } catch { + diag.warn(`Could not parse export URL: '${url}'`); + return url; } - return url; } /** @@ -83,7 +88,7 @@ export function configureExporterTimeout(timeoutMillis: number | undefined): num function getExporterTimeoutFromEnv(): number { const definedTimeout = Number(getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT ?? - getEnv().OTEL_EXPORTER_OTLP_TIMEOUT); + getEnv().OTEL_EXPORTER_OTLP_TIMEOUT); if (definedTimeout <= 0) { // OTLP exporter configured timeout - using default value of 10000ms diff --git a/experimental/packages/otlp-exporter-base/test/common/util.test.ts b/experimental/packages/otlp-exporter-base/test/common/util.test.ts index cc29de641d..955e8d1773 100644 --- a/experimental/packages/otlp-exporter-base/test/common/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/common/util.test.ts @@ -20,7 +20,7 @@ import { diag } from '@opentelemetry/api'; import { parseHeaders, appendResourcePathToUrl, - appendRootPathToUrlIfNeeded + appendRootPathToUrlIfNeeded, } from '../../src/util'; describe('utils', () => { @@ -84,23 +84,35 @@ describe('utils', () => { describe('appendRootPathToUrlIfNeeded - specifc signal http endpoint', () => { it('should append root path when missing', () => { const url = 'http://foo.bar'; - const resourcePath = 'v1/traces'; - const finalUrl = appendRootPathToUrlIfNeeded(url, resourcePath); + const finalUrl = appendRootPathToUrlIfNeeded(url); assert.strictEqual(finalUrl, url + '/'); }); it('should not append root path and return same url', () => { const url = 'http://foo.bar/'; - const resourcePath = 'v1/traces'; - const finalUrl = appendRootPathToUrlIfNeeded(url, resourcePath); + const finalUrl = appendRootPathToUrlIfNeeded(url); assert.strictEqual(finalUrl, url); }); - it('should append root path when url contains resource path', () => { - const url = 'http://foo.bar/v1/traces'; - const resourcePath = 'v1/traces'; + it('should not append root path when url contains resource path', () => { + { + const url = 'http://foo.bar/v1/traces'; + + const finalUrl = appendRootPathToUrlIfNeeded(url); + assert.strictEqual(finalUrl, url); + } + { + const url = 'https://endpoint/something'; + + const finalUrl = appendRootPathToUrlIfNeeded(url); + assert.strictEqual(finalUrl, url); + } + }); + + it('should not change string when url is not parseable', () => { + const url = 'this is not an URL'; - const finalUrl = appendRootPathToUrlIfNeeded(url, resourcePath); + const finalUrl = appendRootPathToUrlIfNeeded(url); assert.strictEqual(finalUrl, url); }); }); From f6f4031678bd567861d64fbb2b22727ad6a30e97 Mon Sep 17 00:00:00 2001 From: "marc.pichler" Date: Fri, 23 Sep 2022 11:28:48 +0200 Subject: [PATCH 2/3] fix(changlog): add changelog entry. --- experimental/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index d62009288d..28b04b3157 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -10,6 +10,8 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) +* fix(exporters): do not append trailing '/' when URL contains path [#3274](https://github.com/open-telemetry/opentelemetry-js/pull/3274) @pichlermarc + ### :books: (Refine Doc) ### :house: (Internal) From 4fe34f563f02957b9e6ecc579a5d9ad591e4c9b0 Mon Sep 17 00:00:00 2001 From: "marc.pichler" Date: Fri, 23 Sep 2022 11:30:26 +0200 Subject: [PATCH 3/3] fix(exporter): fix typo. --- .../packages/otlp-exporter-base/test/common/util.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/otlp-exporter-base/test/common/util.test.ts b/experimental/packages/otlp-exporter-base/test/common/util.test.ts index 955e8d1773..d78b719faa 100644 --- a/experimental/packages/otlp-exporter-base/test/common/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/common/util.test.ts @@ -110,7 +110,7 @@ describe('utils', () => { }); it('should not change string when url is not parseable', () => { - const url = 'this is not an URL'; + const url = 'this is not a URL'; const finalUrl = appendRootPathToUrlIfNeeded(url); assert.strictEqual(finalUrl, url);