From 7f0d2e348f83b25f266da4a914c79beeca827884 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Thu, 25 Mar 2021 11:49:37 -0300 Subject: [PATCH] express uses http wrap explicitly if http disabled --- src/plugins/ExpressPlugin.ts | 66 +++++----------------------- src/plugins/HttpPlugin.ts | 85 +++++++++++++++++++----------------- src/trace/span/EntrySpan.ts | 12 +---- 3 files changed, 59 insertions(+), 104 deletions(-) diff --git a/src/plugins/ExpressPlugin.ts b/src/plugins/ExpressPlugin.ts index 90f86b7..10ae4f9 100644 --- a/src/plugins/ExpressPlugin.ts +++ b/src/plugins/ExpressPlugin.ts @@ -23,9 +23,9 @@ import ContextManager from '../trace/context/ContextManager'; import { Component } from '../trace/Component'; import Tag from '../Tag'; import EntrySpan from '../trace/span/EntrySpan'; -import { SpanLayer } from '../proto/language-agent/Tracing_pb'; import { ContextCarrier } from '../trace/context/ContextCarrier'; import PluginInstaller from '../core/PluginInstaller'; +import HttpPlugin from './HttpPlugin'; class ExpressPlugin implements SwPlugin { readonly module = 'express'; @@ -49,59 +49,17 @@ class ExpressPlugin implements SwPlugin { if (span.depth) // if we inherited from http then just change component ID and let http do the work return _handle.apply(this, arguments); - // all the rest of this code is only needed to make express tracing work if the http plugin is disabled - - let copyStatusErrorAndStopIfNotStopped = (err: Error | undefined) => { - copyStatusErrorAndStopIfNotStopped = () => undefined; - - span.tag(Tag.httpStatusCode(res.statusCode)); - - if (res.statusCode && res.statusCode >= 400) - span.errored = true; - - if (res.statusMessage) - span.tag(Tag.httpStatusMsg(res.statusMessage)); - - if (err instanceof Error) - span.error(err); - - span.stop(); - }; - - span.start(); - - try { - span.layer = SpanLayer.HTTP; - span.peer = - (typeof req.headers['x-forwarded-for'] === 'string' && req.headers['x-forwarded-for'].split(',').shift()) - || (req.connection.remoteFamily === 'IPv6' - ? `[${req.connection.remoteAddress}]:${req.connection.remotePort}` - : `${req.connection.remoteAddress}:${req.connection.remotePort}`); - - span.tag(Tag.httpMethod(req.method)); - - const ret = _handle.call(this, req, res, (err: Error) => { - span.error(err); - next.call(this, err); - }); - - if (process.version < 'v12') - req.on('end', copyStatusErrorAndStopIfNotStopped); // this insead of req or res.close because Node 10 doesn't emit those - else - res.on('close', copyStatusErrorAndStopIfNotStopped); // this works better - - span.async(); - - return ret; - - } catch (err) { - copyStatusErrorAndStopIfNotStopped(err); - - throw err; - - } finally { // req.protocol is only possibly available after call to _handle() - span.tag(Tag.httpURL(((req as any).protocol ? (req as any).protocol + '://' : '') + (req.headers.host || '') + req.url)); - } + return HttpPlugin.wrapHttpResponse(span, req, res, () => { // http plugin disabled, we use its mechanism anyway + try { + return _handle.call(this, req, res, (err: Error) => { + span.error(err); + next.call(this, err); + }); + + } finally { // req.protocol is only possibly available after call to _handle() + span.tag(Tag.httpURL(((req as any).protocol ? (req as any).protocol + '://' : '') + (req.headers.host || '') + req.url)); + } + }); }; } } diff --git a/src/plugins/HttpPlugin.ts b/src/plugins/HttpPlugin.ts index 8d82371..c9936e3 100644 --- a/src/plugins/HttpPlugin.ts +++ b/src/plugins/HttpPlugin.ts @@ -23,6 +23,7 @@ import { ClientRequest, IncomingMessage, RequestOptions, ServerResponse } from ' import ContextManager from '../trace/context/ContextManager'; import { Component } from '../trace/Component'; import Tag from '../Tag'; +import Span from '../trace/span/Span'; import ExitSpan from '../trace/span/ExitSpan'; import { SpanLayer } from '../proto/language-agent/Tracing_pb'; import { ContextCarrier } from '../trace/context/ContextCarrier'; @@ -136,8 +137,8 @@ class HttpPlugin implements SwPlugin { } private interceptServerRequest(module: any, protocol: string) { - /// TODO? full event protocol support not currently implemented (prependListener(), removeListener(), etc...) - const _addListener = module.Server.prototype.addListener; + const plugin = this; + const _addListener = module.Server.prototype.addListener; // TODO? full event protocol support not currently implemented (prependListener(), removeListener(), etc...) module.Server.prototype.addListener = module.Server.prototype.on = function (event: any, handler: any, ...addArgs: any[]) { return _addListener.call(this, event, event === 'request' ? _sw_request : handler, ...addArgs); @@ -147,57 +148,63 @@ class HttpPlugin implements SwPlugin { const operation = (req.url || '/').replace(/\?.*/g, ''); const span = ContextManager.current.newEntrySpan(operation, carrier); - span.start(); + span.component = Component.HTTP_SERVER; - try { - span.component = Component.HTTP_SERVER; - span.layer = SpanLayer.HTTP; - span.peer = - (typeof req.headers['x-forwarded-for'] === 'string' && req.headers['x-forwarded-for'].split(',').shift()) - || (req.connection.remoteFamily === 'IPv6' - ? `[${req.connection.remoteAddress}]:${req.connection.remotePort}` - : `${req.connection.remoteAddress}:${req.connection.remotePort}`); + span.tag(Tag.httpURL(protocol + '://' + (req.headers.host || '') + req.url)); - span.tag(Tag.httpURL(protocol + '://' + (req.headers.host || '') + req.url)); - span.tag(Tag.httpMethod(req.method)); + return plugin.wrapHttpResponse(span, req, res, () => handler.call(this, req, res, ...reqArgs)); + } + }; + } - const ret = handler.call(this, req, res, ...reqArgs); + wrapHttpResponse(span: Span, req: IncomingMessage, res: ServerResponse, handler: any): any { + span.start(); - const stopper = (stopEvent: any) => { - const stop = (emittedEvent: any) => { - if (emittedEvent === stopEvent) { - span.tag(Tag.httpStatusCode(res.statusCode)); + try { + span.layer = SpanLayer.HTTP; + span.peer = + (typeof req.headers['x-forwarded-for'] === 'string' && req.headers['x-forwarded-for'].split(',').shift()) + || (req.connection.remoteFamily === 'IPv6' + ? `[${req.connection.remoteAddress}]:${req.connection.remotePort}` + : `${req.connection.remoteAddress}:${req.connection.remotePort}`); - if (res.statusCode && res.statusCode >= 400) - span.errored = true; + span.tag(Tag.httpMethod(req.method)); - if (res.statusMessage) - span.tag(Tag.httpStatusMsg(res.statusMessage)); + const ret = handler(); - return true; - } - }; + const stopper = (stopEvent: any) => { + const stop = (emittedEvent: any) => { + if (emittedEvent === stopEvent) { + span.tag(Tag.httpStatusCode(res.statusCode)); - return stop; - }; + if (res.statusCode && res.statusCode >= 400) + span.errored = true; - const isSub12 = process.version < 'v12'; + if (res.statusMessage) + span.tag(Tag.httpStatusMsg(res.statusMessage)); - wrapEmit(span, req, true, isSub12 ? stopper('end') : NaN); - wrapEmit(span, res, true, isSub12 ? NaN : stopper('close')); + return true; + } + }; - span.async(); + return stop; + }; - return ret; + const isSub12 = process.version < 'v12'; - } catch (err) { - span.error(err); - span.stop(); + wrapEmit(span, req, true, isSub12 ? stopper('end') : NaN); + wrapEmit(span, res, true, isSub12 ? NaN : stopper('close')); - throw err; - } - } - }; + span.async(); + + return ret; + + } catch (err) { + span.error(err); + span.stop(); + + throw err; + } } } diff --git a/src/trace/span/EntrySpan.ts b/src/trace/span/EntrySpan.ts index 6c49e9a..511b0a5 100644 --- a/src/trace/span/EntrySpan.ts +++ b/src/trace/span/EntrySpan.ts @@ -18,10 +18,9 @@ */ import StackedSpan from '../../trace/span/StackedSpan'; -import { Component } from '../Component'; import { SpanCtorOptions } from './Span'; import SegmentRef from '../../trace/context/SegmentRef'; -import { SpanLayer, SpanType } from '../../proto/language-agent/Tracing_pb'; +import { SpanType } from '../../proto/language-agent/Tracing_pb'; import { ContextCarrier } from '../context/ContextCarrier'; export default class EntrySpan extends StackedSpan { @@ -33,15 +32,6 @@ export default class EntrySpan extends StackedSpan { ); } - start(): this { - super.start(); - this.layer = SpanLayer.UNKNOWN; - this.component = Component.UNKNOWN; - this.logs.splice(0, this.logs.length); - this.tags.splice(0, this.tags.length); - return this; - } - extract(carrier: ContextCarrier): this { super.extract(carrier);