From 3d9b8228b536c99e09d36df7ac3386e3239f067a Mon Sep 17 00:00:00 2001 From: Olivier Albertini Date: Thu, 2 Jan 2020 02:07:39 -0500 Subject: [PATCH] feat(plugin-http): add/modify attributes (#643) * feat(plugin-http): add/modify attributes closes #373, #394 Signed-off-by: Olivier Albertini * fix: change remotePort to localPort refactor: remove useless checks test: add assertions Signed-off-by: Olivier Albertini * test(plugin-https): sync with http plugin Signed-off-by: Olivier Albertini Co-authored-by: Mayur Kale --- packages/opentelemetry-plugin-http/README.md | 2 +- .../src/enums/AttributeNames.ts | 18 +- .../opentelemetry-plugin-http/src/http.ts | 78 +++------ .../opentelemetry-plugin-http/src/types.ts | 8 + .../opentelemetry-plugin-http/src/utils.ts | 158 +++++++++++++++++- .../test/functionals/http-enable.test.ts | 49 +++++- .../test/integrations/http-enable.test.ts | 8 +- .../test/utils/assertSpan.ts | 47 ++++-- .../test/functionals/https-enable.test.ts | 50 +++++- .../test/integrations/https-enable.test.ts | 13 +- .../test/utils/assertSpan.ts | 48 ++++-- 11 files changed, 383 insertions(+), 96 deletions(-) diff --git a/packages/opentelemetry-plugin-http/README.md b/packages/opentelemetry-plugin-http/README.md index 73b5d4bad9a..c1e84e55a81 100644 --- a/packages/opentelemetry-plugin-http/README.md +++ b/packages/opentelemetry-plugin-http/README.md @@ -54,7 +54,7 @@ Http plugin has few options available to choose from. You can set the following: | [`applyCustomAttributesOnSpan`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L52) | `HttpCustomAttributeFunction` | Function for adding custom attributes | | [`ignoreIncomingPaths`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `IgnoreMatcher[]` | Http plugin will not trace all incoming requests that match paths | | [`ignoreOutgoingUrls`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `IgnoreMatcher[]` | Http plugin will not trace all outgoing requests that match urls | - +| [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `string` | The primary server name of the matched virtual host. | ## Useful links - For more information on OpenTelemetry, visit: - For more about OpenTelemetry JavaScript: diff --git a/packages/opentelemetry-plugin-http/src/enums/AttributeNames.ts b/packages/opentelemetry-plugin-http/src/enums/AttributeNames.ts index ac1bc03e081..dcc0ada8183 100644 --- a/packages/opentelemetry-plugin-http/src/enums/AttributeNames.ts +++ b/packages/opentelemetry-plugin-http/src/enums/AttributeNames.ts @@ -15,17 +15,29 @@ */ /** - * Attributes Names according to [OpenTelemetry attributes specs](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md#semantic-conventions) + * Attributes Names according to [OpenTelemetry attributes specs](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-http.md#common-attributes) */ export enum AttributeNames { - HTTP_HOSTNAME = 'http.hostname', + HTTP_HOST = 'http.host', COMPONENT = 'component', HTTP_METHOD = 'http.method', - HTTP_PATH = 'http.path', + HTTP_TARGET = 'http.target', HTTP_ROUTE = 'http.route', HTTP_URL = 'http.url', HTTP_STATUS_CODE = 'http.status_code', HTTP_STATUS_TEXT = 'http.status_text', + HTTP_FLAVOR = 'http.flavor', + NET_PEER_IP = 'net.peer.ip', + NET_PEER_PORT = 'net.peer.port', + NET_PEER_NAME = 'net.peer.name', + NET_HOST_IP = 'net.host.ip', + NET_HOST_PORT = 'net.host.port', + NET_HOST_NAME = 'net.host.name', + NET_TRANSPORT = 'net.transport', + IP_TCP = 'IP.TCP', + IP_UDP = 'IP.UDP', + HTTP_SERVER_NAME = 'http.server_name', + HTTP_CLIENT_IP = 'http.client_ip', // NOT ON OFFICIAL SPEC HTTP_ERROR_NAME = 'http.error_name', HTTP_ERROR_MESSAGE = 'http.error_message', diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts index 828a75f989f..feeab5a26ed 100644 --- a/packages/opentelemetry-plugin-http/src/http.ts +++ b/packages/opentelemetry-plugin-http/src/http.ts @@ -19,7 +19,6 @@ import { Span, SpanKind, SpanOptions, - Attributes, CanonicalCode, Status, } from '@opentelemetry/types'; @@ -45,6 +44,7 @@ import { import { Format } from './enums/Format'; import { AttributeNames } from './enums/AttributeNames'; import * as utils from './utils'; +import { Socket } from 'net'; /** * Http instrumentation plugin for Opentelemetry @@ -183,37 +183,24 @@ export class HttpPlugin extends BasePlugin { return (): ClientRequest => { this._logger.debug('makeRequestTrace by injecting context into header'); - const host = options.hostname || options.host || 'localhost'; - const method = options.method ? options.method.toUpperCase() : 'GET'; - const headers = options.headers || {}; - const userAgent = headers['user-agent']; - - span.setAttributes({ - [AttributeNames.HTTP_URL]: utils.getAbsoluteUrl( - options, - headers, - `${this.component}:` - ), - [AttributeNames.HTTP_HOSTNAME]: host, - [AttributeNames.HTTP_METHOD]: method, - [AttributeNames.HTTP_PATH]: options.path || '/', + const hostname = + options.hostname || + options.host?.replace(/^(.*)(\:[0-9]{1,5})/, '$1') || + 'localhost'; + const attributes = utils.getOutgoingRequestAttributes(options, { + component: this.component, + hostname, }); - - if (userAgent !== undefined) { - span.setAttribute(AttributeNames.HTTP_USER_AGENT, userAgent); - } + span.setAttributes(attributes); request.on( 'response', - ( - response: IncomingMessage & { aborted?: boolean; req: ClientRequest } - ) => { - if (response.statusCode) { - span.setAttributes({ - [AttributeNames.HTTP_STATUS_CODE]: response.statusCode, - [AttributeNames.HTTP_STATUS_TEXT]: response.statusMessage, - }); - } + (response: IncomingMessage & { aborted?: boolean }) => { + const attributes = utils.getOutgoingRequestAttributesOnResponse( + response, + { hostname } + ); + span.setAttributes(attributes); this._tracer.bind(response); this._logger.debug('outgoingRequest on response()'); @@ -280,7 +267,7 @@ export class HttpPlugin extends BasePlugin { } const request = args[0] as IncomingMessage; - const response = args[1] as ServerResponse; + const response = args[1] as ServerResponse & { socket: Socket }; const pathname = request.url ? url.parse(request.url).pathname || '/' : '/'; @@ -301,8 +288,13 @@ export class HttpPlugin extends BasePlugin { const propagation = plugin._tracer.getHttpTextFormat(); const headers = request.headers; + const spanOptions: SpanOptions = { kind: SpanKind.SERVER, + attributes: utils.getIncomingRequestAttributes(request, { + component: plugin.component, + serverName: plugin._config.serverName, + }), }; const spanContext = propagation.extract(Format.HTTP, headers); @@ -332,32 +324,10 @@ export class HttpPlugin extends BasePlugin { () => response.end.apply(this, arguments as any), true ); - const requestUrl = request.url ? url.parse(request.url) : null; - const hostname = headers.host - ? headers.host.replace(/^(.*)(\:[0-9]{1,5})/, '$1') - : 'localhost'; - const userAgent = headers['user-agent']; - - const attributes: Attributes = { - [AttributeNames.HTTP_URL]: utils.getAbsoluteUrl( - requestUrl, - headers, - `${plugin.component}:` - ), - [AttributeNames.HTTP_HOSTNAME]: hostname, - [AttributeNames.HTTP_METHOD]: method, - [AttributeNames.HTTP_STATUS_CODE]: response.statusCode, - [AttributeNames.HTTP_STATUS_TEXT]: response.statusMessage, - }; - - if (requestUrl) { - attributes[AttributeNames.HTTP_PATH] = requestUrl.path || '/'; - attributes[AttributeNames.HTTP_ROUTE] = requestUrl.pathname || '/'; - } - if (userAgent !== undefined) { - attributes[AttributeNames.HTTP_USER_AGENT] = userAgent; - } + const attributes = utils.getIncomingRequestAttributesOnResponse( + response + ); span .setAttributes(attributes) diff --git a/packages/opentelemetry-plugin-http/src/types.ts b/packages/opentelemetry-plugin-http/src/types.ts index e55c3583483..432235a05a1 100644 --- a/packages/opentelemetry-plugin-http/src/types.ts +++ b/packages/opentelemetry-plugin-http/src/types.ts @@ -57,10 +57,18 @@ export interface HttpCustomAttributeFunction { ): void; } +/** + * Options available for the HTTP Plugin (see [documentation](https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-plugin-http#http-plugin-options)) + */ export interface HttpPluginConfig extends PluginConfig { + /** Not trace all incoming requests that match paths */ ignoreIncomingPaths?: IgnoreMatcher[]; + /** Not trace all outgoing requests that match urls */ ignoreOutgoingUrls?: IgnoreMatcher[]; + /** Function for adding custom attributes */ applyCustomAttributesOnSpan?: HttpCustomAttributeFunction; + /** The primary server name of the matched virtual host. */ + serverName?: string; } export interface Err extends Error { diff --git a/packages/opentelemetry-plugin-http/src/utils.ts b/packages/opentelemetry-plugin-http/src/utils.ts index fafcec338d1..f49a9978d4b 100644 --- a/packages/opentelemetry-plugin-http/src/utils.ts +++ b/packages/opentelemetry-plugin-http/src/utils.ts @@ -14,17 +14,19 @@ * limitations under the License. */ -import { Status, CanonicalCode, Span } from '@opentelemetry/types'; +import { Status, CanonicalCode, Span, Attributes } from '@opentelemetry/types'; import { RequestOptions, IncomingMessage, ClientRequest, IncomingHttpHeaders, OutgoingHttpHeaders, + ServerResponse, } from 'http'; import { IgnoreMatcher, Err, ParsedRequestOptions } from './types'; import { AttributeNames } from './enums/AttributeNames'; import * as url from 'url'; +import { Socket } from 'net'; export const OT_REQUEST_HEADER = 'x-opentelemetry-outgoing-request'; /** @@ -280,3 +282,157 @@ export const isValidOptionsType = (options: unknown): boolean => { export const isOpenTelemetryRequest = (options: RequestOptions) => { return !!(options && options.headers && options.headers[OT_REQUEST_HEADER]); }; + +/** + * Returns outgoing request attributes scoped to the options passed to the request + * @param {ParsedRequestOptions} requestOptions the same options used to make the request + * @param {{ component: string, hostname: string }} options used to pass data needed to create attributes + */ +export const getOutgoingRequestAttributes = ( + requestOptions: ParsedRequestOptions, + options: { component: string; hostname: string } +): Attributes => { + const host = requestOptions.host; + const hostname = + requestOptions.hostname || + host?.replace(/^(.*)(\:[0-9]{1,5})/, '$1') || + 'localhost'; + const requestMethod = requestOptions.method; + const method = requestMethod ? requestMethod.toUpperCase() : 'GET'; + const headers = requestOptions.headers || {}; + const userAgent = headers['user-agent']; + + const attributes: Attributes = { + [AttributeNames.HTTP_URL]: getAbsoluteUrl( + requestOptions, + headers, + `${options.component}:` + ), + [AttributeNames.HTTP_METHOD]: method, + [AttributeNames.HTTP_TARGET]: requestOptions.path || '/', + [AttributeNames.NET_PEER_NAME]: hostname, + }; + + if (userAgent !== undefined) { + attributes[AttributeNames.HTTP_USER_AGENT] = userAgent; + } + return attributes; +}; + +/** + * Returns attributes related to the kind of HTTP protocol used + * @param {string} [kind] Kind of HTTP protocol used: "1.0", "1.1", "2", "SPDY" or "QUIC". + */ +export const getAttributesFromHttpKind = (kind?: string): Attributes => { + const attributes: Attributes = {}; + if (kind) { + attributes[AttributeNames.HTTP_FLAVOR] = kind; + if (kind.toUpperCase() !== 'QUIC') { + attributes[AttributeNames.NET_TRANSPORT] = AttributeNames.IP_TCP; + } else { + attributes[AttributeNames.NET_TRANSPORT] = AttributeNames.IP_UDP; + } + } + return attributes; +}; + +/** + * Returns outgoing request attributes scoped to the response data + * @param {IncomingMessage} response the response object + * @param {{ hostname: string }} options used to pass data needed to create attributes + */ +export const getOutgoingRequestAttributesOnResponse = ( + response: IncomingMessage, + options: { hostname: string } +): Attributes => { + const { statusCode, statusMessage, httpVersion, socket } = response; + const { remoteAddress, remotePort } = socket; + const attributes: Attributes = { + [AttributeNames.NET_PEER_IP]: remoteAddress, + [AttributeNames.NET_PEER_PORT]: remotePort, + [AttributeNames.HTTP_HOST]: `${options.hostname}:${remotePort}`, + }; + + if (statusCode) { + attributes[AttributeNames.HTTP_STATUS_CODE] = statusCode; + attributes[AttributeNames.HTTP_STATUS_TEXT] = ( + statusMessage || '' + ).toUpperCase(); + } + + const httpKindAttributes = getAttributesFromHttpKind(httpVersion); + return Object.assign(attributes, httpKindAttributes); +}; + +/** + * Returns incoming request attributes scoped to the request data + * @param {IncomingMessage} request the request object + * @param {{ component: string, serverName?: string }} options used to pass data needed to create attributes + */ +export const getIncomingRequestAttributes = ( + request: IncomingMessage, + options: { component: string; serverName?: string } +): Attributes => { + const headers = request.headers; + const userAgent = headers['user-agent']; + const ips = headers['x-forwarded-for']; + const method = request.method || 'GET'; + const httpVersion = request.httpVersion; + const requestUrl = request.url ? url.parse(request.url) : null; + const host = requestUrl?.host || headers.host; + const hostname = + requestUrl?.hostname || + host?.replace(/^(.*)(\:[0-9]{1,5})/, '$1') || + 'localhost'; + const serverName = options.serverName; + const attributes: Attributes = { + [AttributeNames.HTTP_URL]: getAbsoluteUrl( + requestUrl, + headers, + `${options.component}:` + ), + [AttributeNames.HTTP_HOST]: host, + [AttributeNames.NET_HOST_NAME]: hostname, + [AttributeNames.HTTP_METHOD]: method, + }; + + if (typeof ips === 'string') { + attributes[AttributeNames.HTTP_CLIENT_IP] = ips.split(',')[0]; + } + + if (typeof serverName === 'string') { + attributes[AttributeNames.HTTP_SERVER_NAME] = serverName; + } + + if (requestUrl) { + attributes[AttributeNames.HTTP_TARGET] = requestUrl.path || '/'; + attributes[AttributeNames.HTTP_ROUTE] = requestUrl.pathname || '/'; + } + + if (userAgent !== undefined) { + attributes[AttributeNames.HTTP_USER_AGENT] = userAgent; + } + + const httpKindAttributes = getAttributesFromHttpKind(httpVersion); + return Object.assign(attributes, httpKindAttributes); +}; + +/** + * Returns incoming request attributes scoped to the response data + * @param {(ServerResponse & { socket: Socket; })} response the response object + */ +export const getIncomingRequestAttributesOnResponse = ( + response: ServerResponse & { socket: Socket } +): Attributes => { + const { statusCode, statusMessage, socket } = response; + const { localAddress, localPort, remoteAddress, remotePort } = socket; + + return { + [AttributeNames.NET_HOST_IP]: localAddress, + [AttributeNames.NET_HOST_PORT]: localPort, + [AttributeNames.NET_PEER_IP]: remoteAddress, + [AttributeNames.NET_PEER_PORT]: remotePort, + [AttributeNames.HTTP_STATUS_CODE]: statusCode, + [AttributeNames.HTTP_STATUS_TEXT]: (statusMessage || '').toUpperCase(), + }; +}; 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 979d5622e5e..fcc49dc30d2 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts @@ -41,6 +41,7 @@ const serverPort = 22345; const protocol = 'http'; const hostname = 'localhost'; const pathname = '/test'; +const serverName = 'my.server.name'; const memoryExporter = new InMemorySpanExporter(); const httpTextFormat = new DummyPropagation(); const logger = new NoopLogger(); @@ -140,6 +141,14 @@ describe('HttpPlugin', () => { assert.strictEqual(spans.length, 2); assertSpan(incomingSpan, SpanKind.SERVER, validations); assertSpan(outgoingSpan, SpanKind.CLIENT, validations); + assert.strictEqual( + incomingSpan.attributes[AttributeNames.NET_HOST_PORT], + serverPort + ); + assert.strictEqual( + outgoingSpan.attributes[AttributeNames.NET_PEER_PORT], + serverPort + ); }); it(`should not trace requests with '${OT_REQUEST_HEADER}' header`, async () => { @@ -176,6 +185,7 @@ describe('HttpPlugin', () => { (url: string) => url.endsWith(`/ignored/function`), ], applyCustomAttributesOnSpan: customAttributeFunction, + serverName, }; plugin.enable(http, tracer, tracer.logger, config); server = http.createServer((request, response) => { @@ -204,7 +214,13 @@ describe('HttpPlugin', () => { it('should generate valid spans (client side and server side)', async () => { const result = await httpRequest.get( - `${protocol}://${hostname}:${serverPort}${pathname}` + `${protocol}://${hostname}:${serverPort}${pathname}`, + { + headers: { + 'x-forwarded-for': ', , ', + 'user-agent': 'chrome', + }, + } ); const spans = memoryExporter.getFinishedSpans(); const [incomingSpan, outgoingSpan] = spans; @@ -216,11 +232,36 @@ describe('HttpPlugin', () => { resHeaders: result.resHeaders, reqHeaders: result.reqHeaders, component: plugin.component, + serverName, }; assert.strictEqual(spans.length, 2); - assertSpan(incomingSpan, SpanKind.SERVER, validations); - assertSpan(outgoingSpan, SpanKind.CLIENT, validations); + assert.strictEqual( + incomingSpan.attributes[AttributeNames.HTTP_CLIENT_IP], + '' + ); + assert.strictEqual( + incomingSpan.attributes[AttributeNames.NET_HOST_PORT], + serverPort + ); + assert.strictEqual( + outgoingSpan.attributes[AttributeNames.NET_PEER_PORT], + serverPort + ); + [ + { span: incomingSpan, kind: SpanKind.SERVER }, + { span: outgoingSpan, kind: SpanKind.CLIENT }, + ].forEach(({ span, kind }) => { + assert.strictEqual( + span.attributes[AttributeNames.HTTP_FLAVOR], + '1.1' + ); + assert.strictEqual( + span.attributes[AttributeNames.NET_TRANSPORT], + AttributeNames.IP_TCP + ); + assertSpan(span, kind, validations); + }); }); it(`should not trace requests with '${OT_REQUEST_HEADER}' header`, async () => { @@ -545,7 +586,7 @@ describe('HttpPlugin', () => { const [span] = spans; assert.strictEqual(spans.length, 1); assert.strictEqual(span.status.code, CanonicalCode.ABORTED); - assert.ok(Object.keys(span.attributes).length > 6); + assert.ok(Object.keys(span.attributes).length >= 6); } }); 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 9f3eb84a7a5..7f79643e4fe 100644 --- a/packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts +++ b/packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts @@ -30,6 +30,7 @@ import { SimpleSpanProcessor, } from '@opentelemetry/tracing'; import { HttpPluginConfig } from '../../src/types'; +import { AttributeNames } from '../../src/enums/AttributeNames'; const protocol = 'http'; const serverPort = 32345; const hostname = 'localhost'; @@ -141,7 +142,7 @@ describe('HttpPlugin Integration tests', () => { assertSpan(span, SpanKind.CLIENT, validations); }); - it('should create a rootSpan for GET requests and add propagation headers if URL and options are used', async () => { + it('should create a valid rootSpan with propagation headers for GET requests if URL and options are used', async () => { let spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 0); @@ -166,6 +167,11 @@ describe('HttpPlugin Integration tests', () => { assert.strictEqual(spans.length, 1); assert.ok(span.name.indexOf('GET /') >= 0); assert.strictEqual(result.reqHeaders['x-foo'], 'foo'); + assert.strictEqual(span.attributes[AttributeNames.HTTP_FLAVOR], '1.1'); + assert.strictEqual( + span.attributes[AttributeNames.NET_TRANSPORT], + AttributeNames.IP_TCP + ); assertSpan(span, SpanKind.CLIENT, validations); }); diff --git a/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts b/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts index 7dd1537f692..ce9dc855b6f 100644 --- a/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts +++ b/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts @@ -35,6 +35,7 @@ export const assertSpan = ( reqHeaders?: http.OutgoingHttpHeaders; path?: string | null; forceStatus?: Status; + serverName?: string; component: string; } ) => { @@ -53,16 +54,12 @@ export const assertSpan = ( span.attributes[AttributeNames.HTTP_ERROR_MESSAGE], span.status.message ); - assert.strictEqual( - span.attributes[AttributeNames.HTTP_HOSTNAME], - validations.hostname - ); assert.strictEqual( span.attributes[AttributeNames.HTTP_METHOD], validations.httpMethod ); assert.strictEqual( - span.attributes[AttributeNames.HTTP_PATH], + span.attributes[AttributeNames.HTTP_TARGET], validations.path || validations.pathname ); assert.strictEqual( @@ -79,12 +76,6 @@ export const assertSpan = ( utils.parseResponseStatus(validations.httpStatusCode) ); - assert.ok( - (span.attributes[AttributeNames.HTTP_URL] as string).indexOf( - span.attributes[AttributeNames.HTTP_HOSTNAME] as string - ) > -1, - 'should be consistent' - ); assert.ok(span.endTime, 'must be finished'); assert.ok(hrTimeToNanoseconds(span.duration), 'must have positive duration'); @@ -97,8 +88,40 @@ export const assertSpan = ( ); } } - + if (span.kind === SpanKind.CLIENT) { + assert.strictEqual( + span.attributes[AttributeNames.NET_PEER_NAME], + validations.hostname, + 'must be consistent (PEER_NAME and hostname)' + ); + assert.ok(span.attributes[AttributeNames.NET_PEER_IP], 'must have PEER_IP'); + assert.ok( + span.attributes[AttributeNames.NET_PEER_PORT], + 'must have PEER_PORT' + ); + assert.ok( + (span.attributes[AttributeNames.HTTP_URL] as string).indexOf( + span.attributes[AttributeNames.NET_PEER_NAME] as string + ) > -1, + 'must be consistent' + ); + } if (span.kind === SpanKind.SERVER) { + if (validations.serverName) { + assert.strictEqual( + span.attributes[AttributeNames.HTTP_SERVER_NAME], + validations.serverName, + ' must have serverName attribute' + ); + assert.ok( + span.attributes[AttributeNames.NET_HOST_PORT], + 'must have HOST_PORT' + ); + assert.ok( + span.attributes[AttributeNames.NET_HOST_IP], + 'must have HOST_IP' + ); + } assert.strictEqual(span.parentSpanId, DummyPropagation.SPAN_CONTEXT_KEY); } else if (validations.reqHeaders) { assert.ok(validations.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY]); diff --git a/packages/opentelemetry-plugin-https/test/functionals/https-enable.test.ts b/packages/opentelemetry-plugin-https/test/functionals/https-enable.test.ts index 1bfcc94105a..c87f3635cac 100644 --- a/packages/opentelemetry-plugin-https/test/functionals/https-enable.test.ts +++ b/packages/opentelemetry-plugin-https/test/functionals/https-enable.test.ts @@ -45,6 +45,7 @@ let server: https.Server; const serverPort = 32345; const protocol = 'https'; const hostname = 'localhost'; +const serverName = 'my.server.name'; const pathname = '/test'; const memoryExporter = new InMemorySpanExporter(); const httpTextFormat = new DummyPropagation(); @@ -153,6 +154,14 @@ describe('HttpsPlugin', () => { assert.strictEqual(spans.length, 2); assertSpan(incomingSpan, SpanKind.SERVER, validations); assertSpan(outgoingSpan, SpanKind.CLIENT, validations); + assert.strictEqual( + incomingSpan.attributes[AttributeNames.NET_HOST_PORT], + serverPort + ); + assert.strictEqual( + outgoingSpan.attributes[AttributeNames.NET_PEER_PORT], + serverPort + ); }); it(`should not trace requests with '${OT_REQUEST_HEADER}' header`, async () => { @@ -189,6 +198,7 @@ describe('HttpsPlugin', () => { (url: string) => url.endsWith(`/ignored/function`), ], applyCustomAttributesOnSpan: customAttributeFunction, + serverName, }; plugin.enable( (https as unknown) as Http, @@ -230,7 +240,13 @@ describe('HttpsPlugin', () => { it('should generate valid spans (client side and server side)', async () => { const result = await httpsRequest.get( - `${protocol}://${hostname}:${serverPort}${pathname}` + `${protocol}://${hostname}:${serverPort}${pathname}`, + { + headers: { + 'x-forwarded-for': ', , ', + 'user-agent': 'chrome', + }, + } ); const spans = memoryExporter.getFinishedSpans(); const [incomingSpan, outgoingSpan] = spans; @@ -242,11 +258,37 @@ describe('HttpsPlugin', () => { resHeaders: result.resHeaders, reqHeaders: result.reqHeaders, component: plugin.component, + serverName, }; assert.strictEqual(spans.length, 2); - assertSpan(incomingSpan, SpanKind.SERVER, validations); - assertSpan(outgoingSpan, SpanKind.CLIENT, validations); + assert.strictEqual( + incomingSpan.attributes[AttributeNames.HTTP_CLIENT_IP], + '' + ); + assert.strictEqual( + incomingSpan.attributes[AttributeNames.NET_HOST_PORT], + serverPort + ); + assert.strictEqual( + outgoingSpan.attributes[AttributeNames.NET_PEER_PORT], + serverPort + ); + + [ + { span: incomingSpan, kind: SpanKind.SERVER }, + { span: outgoingSpan, kind: SpanKind.CLIENT }, + ].forEach(({ span, kind }) => { + assert.strictEqual( + span.attributes[AttributeNames.HTTP_FLAVOR], + '1.1' + ); + assert.strictEqual( + span.attributes[AttributeNames.NET_TRANSPORT], + AttributeNames.IP_TCP + ); + assertSpan(span, kind, validations); + }); }); it(`should not trace requests with '${OT_REQUEST_HEADER}' header`, async () => { @@ -571,7 +613,7 @@ describe('HttpsPlugin', () => { const [span] = spans; assert.strictEqual(spans.length, 1); assert.strictEqual(span.status.code, CanonicalCode.ABORTED); - assert.ok(Object.keys(span.attributes).length > 6); + assert.ok(Object.keys(span.attributes).length >= 6); } }); diff --git a/packages/opentelemetry-plugin-https/test/integrations/https-enable.test.ts b/packages/opentelemetry-plugin-https/test/integrations/https-enable.test.ts index 4c2ca192d25..e415816961c 100644 --- a/packages/opentelemetry-plugin-https/test/integrations/https-enable.test.ts +++ b/packages/opentelemetry-plugin-https/test/integrations/https-enable.test.ts @@ -15,7 +15,11 @@ */ import { NoopLogger } from '@opentelemetry/core'; -import { HttpPluginConfig, Http } from '@opentelemetry/plugin-http'; +import { + HttpPluginConfig, + Http, + AttributeNames, +} from '@opentelemetry/plugin-http'; import { SpanKind, Span } from '@opentelemetry/types'; import * as assert from 'assert'; import * as http from 'http'; @@ -143,7 +147,7 @@ describe('HttpsPlugin Integration tests', () => { assertSpan(span, SpanKind.CLIENT, validations); }); - it('should create a rootSpan for GET requests and add propagation headers if URL and options are used', async () => { + it('should create a valid rootSpan with propagation headers for GET requests if URL and options are used', async () => { let spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 0); @@ -168,6 +172,11 @@ describe('HttpsPlugin Integration tests', () => { assert.strictEqual(spans.length, 1); assert.ok(span.name.indexOf('GET /') >= 0); assert.strictEqual(result.reqHeaders['x-foo'], 'foo'); + assert.strictEqual(span.attributes[AttributeNames.HTTP_FLAVOR], '1.1'); + assert.strictEqual( + span.attributes[AttributeNames.NET_TRANSPORT], + AttributeNames.IP_TCP + ); assertSpan(span, SpanKind.CLIENT, validations); }); diff --git a/packages/opentelemetry-plugin-https/test/utils/assertSpan.ts b/packages/opentelemetry-plugin-https/test/utils/assertSpan.ts index 4e856a6876a..ceba19daea6 100644 --- a/packages/opentelemetry-plugin-https/test/utils/assertSpan.ts +++ b/packages/opentelemetry-plugin-https/test/utils/assertSpan.ts @@ -18,12 +18,12 @@ import { SpanKind, Status } from '@opentelemetry/types'; import { hrTimeToNanoseconds } from '@opentelemetry/core'; import * as assert from 'assert'; import * as http from 'http'; +import { DummyPropagation } from './DummyPropagation'; +import { ReadableSpan } from '@opentelemetry/tracing'; import { AttributeNames, parseResponseStatus, } from '@opentelemetry/plugin-http'; -import { DummyPropagation } from './DummyPropagation'; -import { ReadableSpan } from '@opentelemetry/tracing'; export const assertSpan = ( span: ReadableSpan, @@ -37,6 +37,7 @@ export const assertSpan = ( reqHeaders?: http.OutgoingHttpHeaders; path?: string | null; forceStatus?: Status; + serverName?: string; component: string; } ) => { @@ -55,16 +56,12 @@ export const assertSpan = ( span.attributes[AttributeNames.HTTP_ERROR_MESSAGE], span.status.message ); - assert.strictEqual( - span.attributes[AttributeNames.HTTP_HOSTNAME], - validations.hostname - ); assert.strictEqual( span.attributes[AttributeNames.HTTP_METHOD], validations.httpMethod ); assert.strictEqual( - span.attributes[AttributeNames.HTTP_PATH], + span.attributes[AttributeNames.HTTP_TARGET], validations.path || validations.pathname ); assert.strictEqual( @@ -80,12 +77,6 @@ export const assertSpan = ( validations.forceStatus || parseResponseStatus(validations.httpStatusCode) ); - assert.ok( - (span.attributes[AttributeNames.HTTP_URL] as string).indexOf( - span.attributes[AttributeNames.HTTP_HOSTNAME] as string - ) > -1, - 'should be consistent' - ); assert.ok(span.endTime, 'must be finished'); assert.ok(hrTimeToNanoseconds(span.duration), 'must have positive duration'); @@ -98,8 +89,37 @@ export const assertSpan = ( ); } } - + if (span.kind === SpanKind.CLIENT) { + assert.strictEqual( + span.attributes[AttributeNames.NET_PEER_NAME], + validations.hostname, + 'must be consistent (PEER_NAME and hostname)' + ); + assert.ok(span.attributes[AttributeNames.NET_PEER_IP], 'must have PEER_IP'); + assert.ok( + span.attributes[AttributeNames.NET_PEER_PORT], + 'must have PEER_PORT' + ); + assert.ok( + (span.attributes[AttributeNames.HTTP_URL] as string).indexOf( + span.attributes[AttributeNames.NET_PEER_NAME] as string + ) > -1, + 'must be consistent' + ); + } if (span.kind === SpanKind.SERVER) { + if (validations.serverName) { + assert.strictEqual( + span.attributes[AttributeNames.HTTP_SERVER_NAME], + validations.serverName, + ' must have serverName attribute' + ); + } + assert.ok( + span.attributes[AttributeNames.NET_HOST_PORT], + 'must have HOST_PORT' + ); + assert.ok(span.attributes[AttributeNames.NET_HOST_IP], 'must have HOST_IP'); assert.strictEqual(span.parentSpanId, DummyPropagation.SPAN_CONTEXT_KEY); } else if (validations.reqHeaders) { assert.ok(validations.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY]);