Skip to content

Commit 2c0bca4

Browse files
cjihrigcjihrig
authored
test(instrumentation-http): avoid deprecated url.parse() (#6102)
1 parent 823251f commit 2c0bca4

6 files changed

Lines changed: 76 additions & 27 deletions

File tree

‎experimental/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
5959
* test(sdk-node): use process.env consistently in tests [#6052](https://github.com/open-telemetry/opentelemetry-js/pull/6052) @cjihrig
6060
* test(sdk-node): ensure process.env is cleaned up between tests [#6066](https://github.com/open-telemetry/opentelemetry-js/pull/6066) @cjihrig
6161
* refactor(instrumentation-http): avoid deprecated url.parse() in getAbsoluteUrl() [#6089](https://github.com/open-telemetry/opentelemetry-js/pull/6089) @cjihrig
62+
* test(instrumentation-http): avoid deprecated url.parse() [#6102](https://github.com/open-telemetry/opentelemetry-js/pull/6102) @cjihrig
6263

6364
## 0.207.0
6465

‎experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-package.test.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import {
2323
} from '@opentelemetry/sdk-trace-base';
2424
import * as assert from 'assert';
2525
import * as path from 'path';
26-
import * as url from 'url';
2726
import { HttpInstrumentation } from '../../src/http';
2827
import { assertSpan } from '../utils/assertSpan';
2928
import { DummyPropagation } from '../utils/DummyPropagation';
@@ -83,22 +82,22 @@ describe('Packages', () => {
8382
it(`should create a span for GET requests and add propagation headers by using ${name} package`, async () => {
8483
nock.load(path.join(__dirname, '../', '/fixtures/google-http.json'));
8584

86-
const urlparsed = url.parse(
85+
const urlparsed = new URL(
8786
`${protocol}://www.google.com/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8`
8887
);
89-
const result = await httpPackage.get(urlparsed.href!);
88+
const result = await httpPackage.get(urlparsed.href);
9089
if (!resHeaders) {
9190
const res = result as axios.AxiosResponse<unknown>;
9291
resHeaders = res.headers as any;
9392
}
9493
const spans = memoryExporter.getFinishedSpans();
9594
const span = spans[0];
9695
const validations = {
97-
hostname: urlparsed.hostname!,
96+
hostname: urlparsed.hostname,
9897
httpStatusCode: 200,
9998
httpMethod: 'GET',
100-
pathname: urlparsed.pathname!,
101-
path: urlparsed.path,
99+
pathname: urlparsed.pathname,
100+
path: urlparsed.pathname + urlparsed.search,
102101
resHeaders,
103102
component: 'http',
104103
};

‎experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-package.test.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import {
2323
} from '@opentelemetry/sdk-trace-base';
2424
import * as assert from 'assert';
2525
import * as path from 'path';
26-
import * as url from 'url';
2726
import { HttpInstrumentation } from '../../src/http';
2827
import { assertSpan } from '../utils/assertSpan';
2928
import { DummyPropagation } from '../utils/DummyPropagation';
@@ -83,22 +82,22 @@ describe('Packages', () => {
8382
it(`should create a span for GET requests and add propagation headers by using ${name} package`, async () => {
8483
nock.load(path.join(__dirname, '../', '/fixtures/google-https.json'));
8584

86-
const urlparsed = url.parse(
85+
const urlparsed = new URL(
8786
'https://www.google.com/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8'
8887
);
89-
const result = await httpPackage.get(urlparsed.href!);
88+
const result = await httpPackage.get(urlparsed.href);
9089
if (!resHeaders) {
9190
const res = result as axios.AxiosResponse<unknown>;
9291
resHeaders = res.headers as any;
9392
}
9493
const spans = memoryExporter.getFinishedSpans();
9594
const span = spans[0];
9695
const validations = {
97-
hostname: urlparsed.hostname!,
96+
hostname: urlparsed.hostname,
9897
httpStatusCode: 200,
9998
httpMethod: 'GET',
100-
pathname: urlparsed.pathname!,
101-
path: urlparsed.path,
99+
pathname: urlparsed.pathname,
100+
path: urlparsed.pathname + urlparsed.search,
102101
resHeaders,
103102
component: 'https',
104103
};

‎experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts‎

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
4646
import { SemconvStability } from '@opentelemetry/instrumentation';
4747
import { extractHostnameAndPort } from '../../src/utils';
4848
import { AttributeNames } from '../../src/enums/AttributeNames';
49+
import { ParsedUrlQuery } from 'node:querystring';
4950

5051
describe('Utility', () => {
5152
describe('parseResponseStatus()', () => {
@@ -80,7 +81,20 @@ describe('Utility', () => {
8081
describe('getRequestInfo()', () => {
8182
it('should get options object', () => {
8283
const webUrl = 'http://u:p@google.fr/aPath?qu=ry';
83-
const urlParsed = url.parse(webUrl);
84+
const urlParsed = {
85+
protocol: 'http:',
86+
slashes: true,
87+
auth: 'u:p',
88+
host: 'google.fr',
89+
port: null,
90+
hostname: 'google.fr',
91+
hash: null,
92+
search: '?qu=ry',
93+
query: 'qu=ry',
94+
pathname: '/aPath',
95+
path: '/aPath?qu=ry',
96+
href: 'http://u:p@google.fr/aPath?qu=ry',
97+
};
8498
const urlParsedWithoutPathname = {
8599
...urlParsed,
86100
pathname: undefined,
@@ -95,7 +109,7 @@ describe('Utility', () => {
95109
host: undefined,
96110
port: null,
97111
};
98-
const whatWgUrl = new url.URL(webUrl);
112+
const whatWgUrl = new URL(webUrl);
99113
for (const param of [
100114
webUrl,
101115
urlParsed,
@@ -155,12 +169,44 @@ describe('Utility', () => {
155169
describe('getAbsoluteUrl()', () => {
156170
it('should return absolute url with localhost', () => {
157171
const path = '/test/1';
158-
const result = utils.getAbsoluteUrl(url.parse(path), {});
172+
const result = utils.getAbsoluteUrl(
173+
{
174+
protocol: null,
175+
slashes: null,
176+
auth: null,
177+
host: null,
178+
port: null,
179+
hostname: null,
180+
hash: null,
181+
search: null,
182+
query: null as unknown as undefined,
183+
pathname: '/test/1',
184+
path: '/test/1',
185+
href: '/test/1',
186+
},
187+
{}
188+
);
159189
assert.strictEqual(result, `http://localhost${path}`);
160190
});
161191
it('should return absolute url', () => {
162192
const absUrl = 'http://www.google/test/1?query=1';
163-
const result = utils.getAbsoluteUrl(url.parse(absUrl), {});
193+
const result = utils.getAbsoluteUrl(
194+
{
195+
protocol: 'http:',
196+
slashes: true,
197+
auth: null,
198+
host: 'www.google',
199+
port: null,
200+
hostname: 'www.google',
201+
hash: null,
202+
search: '?query=1',
203+
query: 'query=1' as unknown as ParsedUrlQuery,
204+
pathname: '/test/1',
205+
path: '/test/1?query=1',
206+
href: 'http://www.google/test/1?query=1',
207+
},
208+
{}
209+
);
164210
assert.strictEqual(result, absUrl);
165211
});
166212
it('should return default url', () => {
@@ -258,7 +304,11 @@ describe('Utility', () => {
258304
assert.strictEqual(utils.isValidOptionsType(options), false);
259305
});
260306
});
261-
for (const options of ['url', url.parse('http://url.com'), {}]) {
307+
for (const options of [
308+
'url',
309+
url.urlToHttpOptions(new URL('http://url.com')),
310+
{},
311+
]) {
262312
it(`should return true with the following value: ${JSON.stringify(
263313
options
264314
)}`, () => {

‎experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
NET_TRANSPORT_VALUE_IP_TCP,
2424
} from '../../src/semconv';
2525
import * as assert from 'assert';
26-
import * as url from 'url';
26+
import { urlToHttpOptions } from 'url';
2727
import { HttpInstrumentation } from '../../src/http';
2828
import { assertSpan } from '../utils/assertSpan';
2929
import * as utils from '../utils/utils';
@@ -180,7 +180,7 @@ describe('HttpInstrumentation Integration tests', () => {
180180
assert.strictEqual(spans.length, 0);
181181

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

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

209209
const result = await httpRequest.get(
210-
new url.URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
210+
new URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
211211
{
212212
headers: { 'x-foo': 'foo' },
213213
}
@@ -270,7 +270,7 @@ describe('HttpInstrumentation Integration tests', () => {
270270

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

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

@@ -297,7 +297,7 @@ describe('HttpInstrumentation Integration tests', () => {
297297
assert.strictEqual(spans.length, 0);
298298
const options = Object.assign(
299299
{ headers: { Expect: '100-continue' } },
300-
url.parse(`${protocol}://localhost:${mockServerPort}/`)
300+
urlToHttpOptions(new URL(`${protocol}://localhost:${mockServerPort}/`))
301301
);
302302

303303
const result = await httpRequest.get(options);

‎experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import * as fs from 'fs';
2727
import * as path from 'path';
2828
import { Socket } from 'net';
2929
import { assertSpan } from '../utils/assertSpan';
30-
import * as url from 'url';
30+
import { urlToHttpOptions } from 'url';
3131
import * as utils from '../utils/utils';
3232
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
3333
import {
@@ -182,7 +182,7 @@ describe('HttpsInstrumentation Integration tests', () => {
182182
assert.strictEqual(spans.length, 0);
183183

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

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

211211
const result = await httpsRequest.get(
212-
new url.URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
212+
new URL(`${protocol}://localhost:${mockServerPort}/?query=test`),
213213
{
214214
headers: { 'x-foo': 'foo' },
215215
}
@@ -271,7 +271,7 @@ describe('HttpsInstrumentation Integration tests', () => {
271271
assert.strictEqual(spans.length, 0);
272272
const options = Object.assign(
273273
{ headers: { Expect: '100-continue' } },
274-
url.parse(`${protocol}://localhost:${mockServerPort}/`)
274+
urlToHttpOptions(new URL(`${protocol}://localhost:${mockServerPort}/`))
275275
);
276276

277277
const result = await httpsRequest.get(options);

0 commit comments

Comments
 (0)