From c55142f88a1bca6999e9ac3d3463f312f1525b42 Mon Sep 17 00:00:00 2001 From: Liz Fong-Jones Date: Wed, 21 Jul 2021 11:40:48 -0400 Subject: [PATCH] fix(@opentelemetry/exporter-collector-grpc) regression from #2130 when host specified without protocol (#2322) * fix regression from #2130 when host specified without protocol * Update util.test.ts * Apply suggestions from code review Co-authored-by: Naseem * revert package.json changes * Update util.ts * add tests as per @msnev request * Update packages/opentelemetry-exporter-collector-grpc/src/util.ts Co-authored-by: Naseem Co-authored-by: Naseem --- .../src/util.ts | 6 +- .../test/util.test.ts | 81 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 packages/opentelemetry-exporter-collector-grpc/test/util.test.ts diff --git a/packages/opentelemetry-exporter-collector-grpc/src/util.ts b/packages/opentelemetry-exporter-collector-grpc/src/util.ts index 39330afc3b..299a6e7e81 100644 --- a/packages/opentelemetry-exporter-collector-grpc/src/util.ts +++ b/packages/opentelemetry-exporter-collector-grpc/src/util.ts @@ -108,8 +108,12 @@ export function send( } export function validateAndNormalizeUrl(url: string): string { + const hasProtocol = url.match(/^([\w]{1,8}):\/\//); + if (!hasProtocol) { + url = `https://${url}`; + } const target = new URL(url); - if (target.pathname !== '/') { + if (target.pathname && target.pathname !== '/') { diag.warn( 'URL path should not be set when using grpc, the path part of the URL will be ignored.' ); diff --git a/packages/opentelemetry-exporter-collector-grpc/test/util.test.ts b/packages/opentelemetry-exporter-collector-grpc/test/util.test.ts new file mode 100644 index 0000000000..ba08009cd5 --- /dev/null +++ b/packages/opentelemetry-exporter-collector-grpc/test/util.test.ts @@ -0,0 +1,81 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as sinon from 'sinon'; +import * as assert from 'assert'; + +import { diag } from '@opentelemetry/api'; +import { validateAndNormalizeUrl } from '../src/util'; + +// Tests added to detect breakage released in #2130 +describe('validateAndNormalizeUrl()', () => { + const tests = [ + { + name: 'bare hostname should return same value', + input: 'api.datacat.io', + expected: 'api.datacat.io', + }, + { + name: 'host:port should return same value', + input: 'api.datacat.io:1234', + expected: 'api.datacat.io:1234', + }, + { + name: 'grpc://host:port should trim off protocol', + input: 'grpc://api.datacat.io:1234', + expected: 'api.datacat.io:1234', + }, + { + name: 'bad protocol should warn but return host:port', + input: 'badproto://api.datacat.io:1234', + expected: 'api.datacat.io:1234', + warn: 'URL protocol should be http(s):// or grpc(s)://. Using grpc://.', + }, + { + name: 'path on end of url should warn but return host:port', + input: 'grpc://api.datacat.io:1234/a/b/c', + expected: 'api.datacat.io:1234', + warn: 'URL path should not be set when using grpc, the path part of the URL will be ignored.', + }, + { + name: ':// in path should not be used for protocol even if protocol not specified', + input: 'api.datacat.io/a/b://c', + expected: 'api.datacat.io', + warn: 'URL path should not be set when using grpc, the path part of the URL will be ignored.', + }, + { + name: ':// in path is valid when a protocol is specified', + input: 'grpc://api.datacat.io/a/b://c', + expected: 'api.datacat.io', + warn: 'URL path should not be set when using grpc, the path part of the URL will be ignored.', + }, + ]; + tests.forEach(test => { + it(test.name, () => { + const diagWarn = sinon.stub(diag, 'warn'); + try { + assert.strictEqual(validateAndNormalizeUrl(test.input), (test.expected)); + if (test.warn) { + sinon.assert.calledWith(diagWarn, test.warn); + } else { + sinon.assert.notCalled(diagWarn); + } + } finally { + diagWarn.restore(); + } + }); + }); +});