You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ke...@apache.org on 2021/03/26 02:19:00 UTC

[skywalking-nodejs] branch master updated: express uses http wrap explicitly if http disabled (#42)

This is an automated email from the ASF dual-hosted git repository.

kezhenxu94 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking-nodejs.git


The following commit(s) were added to refs/heads/master by this push:
     new 23e827d  express uses http wrap explicitly if http disabled (#42)
23e827d is described below

commit 23e827df71a7e01ae01e1b6b819f5fcf99caeb01
Author: Tomasz Pytel <to...@gmail.com>
AuthorDate: Thu Mar 25 23:18:13 2021 -0300

    express uses http wrap explicitly if http disabled (#42)
---
 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);