Skip to content

Commit

Permalink
fix(@opentelemetry/exporter-collector-grpc) regression from #2130 whe…
Browse files Browse the repository at this point in the history
…n 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 <naseemkullah@gmail.com>

* 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 <naseemkullah@gmail.com>

Co-authored-by: Naseem <naseemkullah@gmail.com>
  • Loading branch information
lizthegrey and naseemkullah committed Jul 21, 2021
1 parent 11719ed commit c55142f
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 1 deletion.
6 changes: 5 additions & 1 deletion packages/opentelemetry-exporter-collector-grpc/src/util.ts
Expand Up @@ -108,8 +108,12 @@ export function send<ExportItem, ServiceRequest>(
}

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.'
);
Expand Down
81 changes: 81 additions & 0 deletions 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();
}
});
});
});

0 comments on commit c55142f

Please sign in to comment.