From 1864a9a50c66024c2b78452d3ea6f691dd83692d Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 5 Oct 2022 17:26:15 +0200 Subject: [PATCH] fix(exporters): do not append trailing '/' when URL contains path (#3274) --- experimental/CHANGELOG.md | 1 + .../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 +++++++++++++------ 9 files changed, 39 insertions(+), 21 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 1c4a3fd8ed..f14d274c65 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) * fix(node-sdk): move `@opentelemetry/semantic-conventions` to `dependencies` [#3283](https://github.com/open-telemetry/opentelemetry-js/pull/3283) @mhassan1 +* fix(exporters): do not append trailing '/' when URL contains path [#3274](https://github.com/open-telemetry/opentelemetry-js/pull/3274) @pichlermarc ### :books: (Refine Doc) 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..d78b719faa 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 a URL'; - const finalUrl = appendRootPathToUrlIfNeeded(url, resourcePath); + const finalUrl = appendRootPathToUrlIfNeeded(url); assert.strictEqual(finalUrl, url); }); });