diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts index 22c34c62b64..6989da0b573 100644 --- a/packages/opentelemetry-plugin-http/src/http.ts +++ b/packages/opentelemetry-plugin-http/src/http.ts @@ -395,7 +395,7 @@ export class HttpPlugin extends BasePlugin { const plugin = this; return function outgoingRequest( this: {}, - options: RequestOptions | string, + options: url.URL | RequestOptions | string, ...args: unknown[] ): ClientRequest { if (!utils.isValidOptionsType(options)) { @@ -404,7 +404,8 @@ export class HttpPlugin extends BasePlugin { const { origin, pathname, method, optionsParsed } = utils.getRequestInfo( options, - typeof args[0] === 'object' && typeof options === 'string' + typeof args[0] === 'object' && + (typeof options === 'string' || options instanceof url.URL) ? (args.shift() as RequestOptions) : undefined ); diff --git a/packages/opentelemetry-plugin-http/src/utils.ts b/packages/opentelemetry-plugin-http/src/utils.ts index fbada00b1fa..adf112c3150 100644 --- a/packages/opentelemetry-plugin-http/src/utils.ts +++ b/packages/opentelemetry-plugin-http/src/utils.ts @@ -200,12 +200,12 @@ export const setSpanWithError = ( * @param [extraOptions] additional options for the request */ export const getRequestInfo = ( - options: RequestOptions | string, + options: url.URL | RequestOptions | string, extraOptions?: RequestOptions ) => { let pathname = '/'; let origin = ''; - let optionsParsed: url.URL | url.UrlWithStringQuery | RequestOptions; + let optionsParsed: RequestOptions; if (typeof options === 'string') { optionsParsed = url.parse(options); pathname = (optionsParsed as url.UrlWithStringQuery).pathname || '/'; @@ -213,8 +213,28 @@ export const getRequestInfo = ( if (extraOptions !== undefined) { Object.assign(optionsParsed, extraOptions); } + } else if (options instanceof url.URL) { + optionsParsed = { + protocol: options.protocol, + hostname: + typeof options.hostname === 'string' && options.hostname.startsWith('[') + ? options.hostname.slice(1, -1) + : options.hostname, + path: `${options.pathname || ''}${options.search || ''}`, + }; + if (optionsParsed.port !== '') { + optionsParsed.port = Number(options.port); + } + if (options.username || options.password) { + optionsParsed.auth = `${options.username}:${options.password}`; + } + pathname = options.pathname; + origin = options.origin; + if (extraOptions !== undefined) { + Object.assign(optionsParsed, extraOptions); + } } else { - optionsParsed = options as RequestOptions; + optionsParsed = options; pathname = (options as url.URL).pathname; if (!pathname && optionsParsed.path) { pathname = url.parse(optionsParsed.path).pathname || '/'; @@ -224,16 +244,13 @@ export const getRequestInfo = ( } if (hasExpectHeader(optionsParsed)) { - (optionsParsed as RequestOptions).headers = Object.assign( - {}, - (optionsParsed as RequestOptions).headers - ); - } else if (!(optionsParsed as RequestOptions).headers) { - (optionsParsed as RequestOptions).headers = {}; + optionsParsed.headers = Object.assign({}, optionsParsed.headers); + } else if (!optionsParsed.headers) { + optionsParsed.headers = {}; } // some packages return method in lowercase.. // ensure upperCase for consistency - let method = (optionsParsed as RequestOptions).method; + let method = optionsParsed.method; method = method ? method.toUpperCase() : 'GET'; return { origin, pathname, method, optionsParsed }; diff --git a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts index d64f9604812..6f49e45f575 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts @@ -23,6 +23,7 @@ import { NodeTracer } from '@opentelemetry/node'; import { CanonicalCode, Span as ISpan, SpanKind } from '@opentelemetry/types'; import * as assert from 'assert'; import * as http from 'http'; +import * as path from 'path'; import * as nock from 'nock'; import { HttpPlugin, plugin } from '../../src/http'; import { assertSpan } from '../utils/assertSpan'; @@ -393,7 +394,7 @@ describe('HttpPlugin', () => { }); } - for (const arg of ['string', '', {}, new Date()]) { + for (const arg of ['string', {}, new Date()]) { it(`should be tracable and not throw exception in http plugin when passing the following argument ${JSON.stringify( arg )}`, async () => { @@ -409,7 +410,7 @@ describe('HttpPlugin', () => { }); } - for (const arg of [true, 1, false, 0]) { + for (const arg of [true, 1, false, 0, '']) { it(`should not throw exception in http plugin when passing the following argument ${JSON.stringify( arg )}`, async () => { @@ -420,7 +421,9 @@ describe('HttpPlugin', () => { // http request has been made // nock throw assert.ok( - error.stack.indexOf('/node_modules/nock/lib/intercept.js') > 0 + error.stack.indexOf( + path.normalize('/node_modules/nock/lib/intercept.js') + ) > 0 ); } const spans = memoryExporter.getFinishedSpans(); diff --git a/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts b/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts index 0a453cdcae3..138cccac816 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts @@ -74,18 +74,25 @@ describe('Utility', () => { describe('getRequestInfo()', () => { it('should get options object', () => { - const webUrl = 'http://google.fr/'; + const webUrl = 'http://u:p@google.fr/aPath?qu=ry'; const urlParsed = url.parse(webUrl); const urlParsedWithoutPathname = { ...urlParsed, pathname: undefined, }; - for (const param of [webUrl, urlParsed, urlParsedWithoutPathname]) { + const whatWgUrl = new url.URL(webUrl); + for (const param of [ + webUrl, + urlParsed, + urlParsedWithoutPathname, + whatWgUrl, + ]) { const result = utils.getRequestInfo(param); assert.strictEqual(result.optionsParsed.hostname, 'google.fr'); assert.strictEqual(result.optionsParsed.protocol, 'http:'); - assert.strictEqual(result.optionsParsed.path, '/'); - assert.strictEqual(result.pathname, '/'); + assert.strictEqual(result.optionsParsed.path, '/aPath?qu=ry'); + assert.strictEqual(result.pathname, '/aPath'); + assert.strictEqual(result.origin, 'http://google.fr'); } }); }); diff --git a/packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts b/packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts index 8712ca58983..0e2ffcdc73a 100644 --- a/packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts +++ b/packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts @@ -113,6 +113,60 @@ describe('HttpPlugin Integration tests', () => { assertSpan(span, SpanKind.CLIENT, validations); }); + it('should create a rootSpan for GET requests and add propagation headers if URL is used', async () => { + let spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 0); + + const result = await httpRequest.get( + new url.URL('http://google.fr/?query=test') + ); + + spans = memoryExporter.getFinishedSpans(); + const span = spans[0]; + const validations = { + hostname: 'google.fr', + httpStatusCode: result.statusCode!, + httpMethod: 'GET', + pathname: '/', + path: '/?query=test', + resHeaders: result.resHeaders, + reqHeaders: result.reqHeaders, + component: plugin.component, + }; + + assert.strictEqual(spans.length, 1); + assert.ok(span.name.indexOf('GET /') >= 0); + assertSpan(span, SpanKind.CLIENT, validations); + }); + + it('should create a rootSpan for GET requests and add propagation headers if URL and options are used', async () => { + let spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 0); + + const result = await httpRequest.get( + new url.URL('http://google.fr/?query=test'), + { headers: { 'x-foo': 'foo' } } + ); + + spans = memoryExporter.getFinishedSpans(); + const span = spans[0]; + const validations = { + hostname: 'google.fr', + httpStatusCode: result.statusCode!, + httpMethod: 'GET', + pathname: '/', + path: '/?query=test', + resHeaders: result.resHeaders, + reqHeaders: result.reqHeaders, + component: plugin.component, + }; + + assert.strictEqual(spans.length, 1); + assert.ok(span.name.indexOf('GET /') >= 0); + assert.strictEqual(result.reqHeaders['x-foo'], 'foo'); + assertSpan(span, SpanKind.CLIENT, validations); + }); + it('custom attributes should show up on client spans', async () => { const result = await httpRequest.get(`http://google.fr/`); const spans = memoryExporter.getFinishedSpans(); diff --git a/packages/opentelemetry-plugin-http/test/utils/httpRequest.ts b/packages/opentelemetry-plugin-http/test/utils/httpRequest.ts index e53d09b9938..a88cc6aef05 100644 --- a/packages/opentelemetry-plugin-http/test/utils/httpRequest.ts +++ b/packages/opentelemetry-plugin-http/test/utils/httpRequest.ts @@ -18,54 +18,51 @@ import * as http from 'http'; import * as url from 'url'; import { RequestOptions } from 'https'; -export const httpRequest = { - get: ( - options: string | RequestOptions - ): Promise<{ - data: string; - statusCode: number | undefined; - resHeaders: http.IncomingHttpHeaders; - reqHeaders: http.OutgoingHttpHeaders; - method: string | undefined; - }> => { - const _options = - typeof options === 'string' - ? Object.assign(url.parse(options), { - headers: { - 'user-agent': 'http-plugin-test', - }, - }) - : options; - return new Promise((resolve, reject) => { - const req = http.get(_options, (resp: http.IncomingMessage) => { - const res = (resp as unknown) as http.IncomingMessage & { - req: http.IncomingMessage; - }; - let data = ''; - resp.on('data', chunk => { - data += chunk; - }); - resp.on('end', () => { - resolve({ - data, - statusCode: res.statusCode, - /* tslint:disable:no-any */ - reqHeaders: (res.req as any).getHeaders - ? (res.req as any).getHeaders() - : (res.req as any)._headers, - /* tslint:enable:no-any */ - resHeaders: res.headers, - method: res.req.method, - }); - }); - resp.on('error', err => { - reject(err); +type GetResult = Promise<{ + data: string; + statusCode: number | undefined; + resHeaders: http.IncomingHttpHeaders; + reqHeaders: http.OutgoingHttpHeaders; + method: string | undefined; +}>; + +function get(url: string | url.URL, options?: RequestOptions): GetResult; +function get(url: RequestOptions): GetResult; +function get(url: any, options?: any): GetResult { + return new Promise((resolve, reject) => { + let req: http.ClientRequest; + function getResponseCb(resp: http.IncomingMessage): void { + const res = (resp as unknown) as http.IncomingMessage & { + req: http.IncomingMessage; + }; + let data = ''; + resp.on('data', chunk => { + data += chunk; + }); + resp.on('end', () => { + resolve({ + data, + statusCode: res.statusCode, + reqHeaders: req.getHeaders ? req.getHeaders() : (req as any)._headers, + resHeaders: res.headers, + method: res.req.method, }); }); - req.on('error', err => { + resp.on('error', err => { reject(err); }); - return req; + } + req = + options != null + ? http.get(url, options, getResponseCb) + : http.get(url, getResponseCb); + req.on('error', err => { + reject(err); }); - }, + return req; + }); +} + +export const httpRequest = { + get, };