Skip to content

Commit

Permalink
fix regression from open-telemetry#2130 when host specified without p…
Browse files Browse the repository at this point in the history
…rotocol
  • Loading branch information
lizthegrey committed Jul 5, 2021
1 parent b3d3d86 commit 5c9f4e3
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 1 deletion.
8 changes: 8 additions & 0 deletions packages/opentelemetry-exporter-collector-grpc/package.json
Expand Up @@ -51,12 +51,20 @@
"@types/mocha": "8.2.2",
"@types/node": "14.17.4",
"@types/sinon": "10.0.2",
"@types/sinon-chai": "^3.2.5",
"@typescript-eslint/eslint-plugin": "^4.28.2",
"@typescript-eslint/parser": "^4.28.2",
"chai": "^4.3.4",
"codecov": "3.8.2",
"cpx": "1.5.0",
"eslint": "^7.30.0",
"eslint-plugin-header": "^3.1.1",
"eslint-plugin-node": "^11.1.0",
"mocha": "7.2.0",
"nyc": "15.1.0",
"rimraf": "3.0.2",
"sinon": "10.0.0",
"sinon-chai": "^3.7.0",
"ts-loader": "8.3.0",
"ts-mocha": "8.0.0",
"ts-node": "10.0.0",
Expand Down
5 changes: 4 additions & 1 deletion packages/opentelemetry-exporter-collector-grpc/src/util.ts
Expand Up @@ -108,8 +108,11 @@ export function send<ExportItem, ServiceRequest>(
}

export function validateAndNormalizeUrl(url: string): string {
if (!url.match(/:\/\//)) {
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
72 changes: 72 additions & 0 deletions packages/opentelemetry-exporter-collector-grpc/test/util.test.ts
@@ -0,0 +1,72 @@
/*
* 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 { expect, use, should } from 'chai';
import * as sinonChai from 'sinon-chai';
should();
use(sinonChai);

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.',
},
];
tests.forEach(test => {
it(test.name, () => {
const diagWarn = sinon.stub(diag, 'warn');
try {
validateAndNormalizeUrl(test.input).should.eq(test.expected);
if (test.warn) {
diagWarn.should.have.been.calledWith(test.warn);
} else {
diagWarn.should.not.have.been.called;
}
} finally {
diagWarn.restore();
}
});
});
});

0 comments on commit 5c9f4e3

Please sign in to comment.