Skip to content

Commit

Permalink
refactor(instrumentation-http): fix eslint warnings
Browse files Browse the repository at this point in the history
```
/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-package.test.ts
  86:27  warning  'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead  node/no-deprecated-api

/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-package.test.ts
  86:27  warning  'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead  node/no-deprecated-api

/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts
  81:25  warning  'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead  node/no-deprecated-api
  156:43  warning  'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead  node/no-deprecated-api
  161:43  warning  'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead  node/no-deprecated-api
  213:35  warning  'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead  node/no-deprecated-api

/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts
  300:9  warning  'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead  node/no-deprecated-api

/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts
  274:9  warning  'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead  node/no-deprecated-api
```

Generally speaking, `new URL()` is not a direct replacement for the
deprecated `url.parse()`, so this type of change requires careful
considerations.

However, in this instance, these are all found in test code, which
cuts out a lot of the typically associated issues, and the tests
passing after the change is a good indication for correctness.

Ref open-telemetry#5365
  • Loading branch information
chancancode committed Jan 29, 2025
1 parent 83ad899 commit e606a4c
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
} from '@opentelemetry/sdk-trace-base';
import * as assert from 'assert';
import * as path from 'path';
import * as url from 'url';
import { HttpInstrumentation } from '../../src/http';
import { assertSpan } from '../utils/assertSpan';
import { DummyPropagation } from '../utils/DummyPropagation';
Expand Down Expand Up @@ -83,22 +82,22 @@ describe('Packages', () => {
it(`should create a span for GET requests and add propagation headers by using ${name} package`, async () => {
nock.load(path.join(__dirname, '../', '/fixtures/google-http.json'));

const urlparsed = url.parse(
const url = new URL(
`${protocol}://www.google.com/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8`
);
const result = await httpPackage.get(urlparsed.href!);
const result = await httpPackage.get(url.href);
if (!resHeaders) {
const res = result as axios.AxiosResponse<unknown>;
resHeaders = res.headers as any;
}
const spans = memoryExporter.getFinishedSpans();
const span = spans[0];
const validations = {
hostname: urlparsed.hostname!,
hostname: 'www.google.com',
httpStatusCode: 200,
httpMethod: 'GET',
pathname: urlparsed.pathname!,
path: urlparsed.path,
pathname: '/search',
path: '/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8',
resHeaders,
component: 'http',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
} from '@opentelemetry/sdk-trace-base';
import * as assert from 'assert';
import * as path from 'path';
import * as url from 'url';
import { HttpInstrumentation } from '../../src/http';
import { assertSpan } from '../utils/assertSpan';
import { DummyPropagation } from '../utils/DummyPropagation';
Expand Down Expand Up @@ -83,22 +82,22 @@ describe('Packages', () => {
it(`should create a span for GET requests and add propagation headers by using ${name} package`, async () => {
nock.load(path.join(__dirname, '../', '/fixtures/google-https.json'));

const urlparsed = url.parse(
const url = new URL(
'https://www.google.com/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8'
);
const result = await httpPackage.get(urlparsed.href!);
const result = await httpPackage.get(url.href!);
if (!resHeaders) {
const res = result as axios.AxiosResponse<unknown>;
resHeaders = res.headers as any;
}
const spans = memoryExporter.getFinishedSpans();
const span = spans[0];
const validations = {
hostname: urlparsed.hostname!,
hostname: 'www.google.com',
httpStatusCode: 200,
httpMethod: 'GET',
pathname: urlparsed.pathname!,
path: urlparsed.path,
pathname: '/search',
path: '/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8',
resHeaders,
component: 'https',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ describe('Utility', () => {
describe('getRequestInfo()', () => {
it('should get options object', () => {
const webUrl = 'http://u:[email protected]/aPath?qu=ry';
// This is explicitly testing interop with the legacy API
// eslint-disable-next-line node/no-deprecated-api
const urlParsed = url.parse(webUrl);
const urlParsedWithoutPathname = {
...urlParsed,
Expand Down Expand Up @@ -152,13 +154,15 @@ describe('Utility', () => {

describe('getAbsoluteUrl()', () => {
it('should return absolute url with localhost', () => {
const path = '/test/1';
const result = utils.getAbsoluteUrl(url.parse(path), {});
assert.strictEqual(result, `http://localhost${path}`);
const result = utils.getAbsoluteUrl({ path: '/test/1' }, {});
assert.strictEqual(result, 'http://localhost/test/1');
});
it('should return absolute url', () => {
const absUrl = 'http://www.google/test/1?query=1';
const result = utils.getAbsoluteUrl(url.parse(absUrl), {});
const absUrl = 'http://www.google.com/test/1?query=1';
const result = utils.getAbsoluteUrl(
{ protocol: 'http:', host: 'www.google.com', path: '/test/1?query=1' },
{}
);
assert.strictEqual(result, absUrl);
});
it('should return default url', () => {
Expand Down Expand Up @@ -210,7 +214,7 @@ describe('Utility', () => {
assert.strictEqual(utils.isValidOptionsType(options), false);
});
});
for (const options of ['url', url.parse('http://url.com'), {}]) {
for (const options of ['url', new URL('http://url.com'), {}]) {
it(`should return true with the following value: ${JSON.stringify(
options
)}`, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
SEMATTRS_NET_TRANSPORT,
} from '@opentelemetry/semantic-conventions';
import * as assert from 'assert';
import * as url from 'url';
import { HttpInstrumentation } from '../../src/http';
import { assertSpan } from '../utils/assertSpan';
import * as utils from '../utils/utils';
Expand Down Expand Up @@ -180,7 +179,7 @@ describe('HttpInstrumentation Integration tests', () => {
assert.strictEqual(spans.length, 0);

const result = await httpRequest.get(
new url.URL(`${protocol}://localhost:${mockServerPort}/?query=test`)
new URL(`${protocol}://localhost:${mockServerPort}/?query=test`)
);

spans = memoryExporter.getFinishedSpans();
Expand All @@ -207,7 +206,7 @@ describe('HttpInstrumentation Integration tests', () => {
assert.strictEqual(spans.length, 0);

const result = await httpRequest.get(
new url.URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
new URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
{
headers: { 'x-foo': 'foo' },
}
Expand Down Expand Up @@ -270,7 +269,7 @@ describe('HttpInstrumentation Integration tests', () => {

const headers = { 'x-foo': 'foo' };
const result = await httpRequest.get(
new url.URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
new URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
{ headers }
);
assert.deepStrictEqual(headers, { 'x-foo': 'foo' });
Expand All @@ -284,7 +283,7 @@ describe('HttpInstrumentation Integration tests', () => {

const headers = { 'x-foo': 'foo', forwarded: 'malformed' };
const result = await httpRequest.get(
new url.URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
new URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
{ headers }
);

Expand All @@ -295,12 +294,14 @@ describe('HttpInstrumentation Integration tests', () => {
it('should create a span for GET requests and add propagation headers with Expect headers', async () => {
let spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 0);
const options = Object.assign(
{ headers: { Expect: '100-continue' } },
url.parse(`${protocol}://localhost:${mockServerPort}/`)
);

const result = await httpRequest.get(options);
const result = await httpRequest.get({
protocol: `${protocol}:`,
host: 'localhost',
port: mockServerPort,
path: '/',
headers: { Expect: '100-continue' },
});
spans = memoryExporter.getFinishedSpans();
const span = spans.find(s => s.kind === SpanKind.CLIENT);
assert.ok(span);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import * as fs from 'fs';
import * as path from 'path';
import { Socket } from 'net';
import { assertSpan } from '../utils/assertSpan';
import * as url from 'url';
import * as utils from '../utils/utils';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import {
Expand Down Expand Up @@ -182,7 +181,7 @@ describe('HttpsInstrumentation Integration tests', () => {
assert.strictEqual(spans.length, 0);

const result = await httpsRequest.get(
new url.URL(`${protocol}://localhost:${mockServerPort}/?query=test`)
new URL(`${protocol}://localhost:${mockServerPort}/?query=test`)
);

spans = memoryExporter.getFinishedSpans();
Expand All @@ -209,7 +208,7 @@ describe('HttpsInstrumentation Integration tests', () => {
assert.strictEqual(spans.length, 0);

const result = await httpsRequest.get(
new url.URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
new URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
{
headers: { 'x-foo': 'foo' },
}
Expand Down Expand Up @@ -269,12 +268,14 @@ describe('HttpsInstrumentation Integration tests', () => {
it('should create a span for GET requests and add propagation headers with Expect headers', async () => {
let spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 0);
const options = Object.assign(
{ headers: { Expect: '100-continue' } },
url.parse(`${protocol}://localhost:${mockServerPort}/`)
);

const result = await httpsRequest.get(options);
const result = await httpsRequest.get({
protocol: `${protocol}:`,
host: 'localhost',
port: mockServerPort,
path: '/',
headers: { Expect: '100-continue' },
});
spans = memoryExporter.getFinishedSpans();
const span = spans.find(s => s.kind === SpanKind.CLIENT);
assert.ok(span);
Expand Down

0 comments on commit e606a4c

Please sign in to comment.