[skywalking-nodejs] 01/01: Fix bug that some requests of express are not close

[ke...@apache.org]

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

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

commit 7338fcadf912c656bcbcf422a17c56f8e47b1795
Author: kezhenxu94 <ke...@apache.org>
AuthorDate: Thu Dec 31 17:55:06 2020 +0800

    Fix bug that some requests of express are not close
---
 src/core/PluginInstaller.ts  |  4 ++--
 src/core/SwPlugin.ts         |  4 +++-
 src/plugins/AxiosPlugin.ts   | 19 ++++++++++---------
 src/plugins/ExpressPlugin.ts | 10 ++++++----
 4 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/src/core/PluginInstaller.ts b/src/core/PluginInstaller.ts
index e959501..820b2bc 100644
--- a/src/core/PluginInstaller.ts
+++ b/src/core/PluginInstaller.ts
@@ -32,7 +32,7 @@ while (topModule.parent) {
 
 export default class PluginInstaller {
   private readonly pluginDir: string;
-  private readonly require: (name: string) => any = topModule.require.bind(topModule);
+  readonly require: (name: string) => any = topModule.require.bind(topModule);
   private readonly resolve = (request: string) => (module.constructor as any)._resolveFilename(request, topModule);
 
   constructor() {
@@ -90,7 +90,7 @@ export default class PluginInstaller {
 
         logger.info(`Installing plugin ${plugin.module} ${plugin.versions}`);
 
-        plugin.install();
+        plugin.install(this);
       } catch (e) {
         if (plugin) {
           logger.error(`Error installing plugin ${plugin.module} ${plugin.versions}`);
diff --git a/src/core/SwPlugin.ts b/src/core/SwPlugin.ts
index 86b81a2..5dbbfe7 100644
--- a/src/core/SwPlugin.ts
+++ b/src/core/SwPlugin.ts
@@ -17,9 +17,11 @@
  *
  */
 
+import PluginInstaller from './PluginInstaller';
+
 export default interface SwPlugin {
   readonly module: string;
   readonly versions: string;
 
-  install(): void;
+  install(installer: PluginInstaller): void;
 }
diff --git a/src/plugins/AxiosPlugin.ts b/src/plugins/AxiosPlugin.ts
index 1ce7191..6289be4 100644
--- a/src/plugins/AxiosPlugin.ts
+++ b/src/plugins/AxiosPlugin.ts
@@ -25,22 +25,23 @@ import Span from '../trace/span/Span';
 import Tag from '../Tag';
 import { SpanLayer } from '../proto/language-agent/Tracing_pb';
 import { createLogger } from '../logging';
+import PluginInstaller from '../core/PluginInstaller';
 
 const logger = createLogger(__filename);
 
 class AxiosPlugin implements SwPlugin {
   readonly module = 'axios';
   readonly versions = '*';
-  axios = require('axios').default;
 
-  install(): void {
+  install(installer: PluginInstaller): void {
     if (logger.isDebugEnabled()) {
       logger.debug('installing axios plugin');
     }
-    this.interceptClientRequest();
+    const axios = installer.require('axios').default;
+    this.interceptClientRequest(axios);
   }
 
-  private interceptClientRequest() {
+  private interceptClientRequest(axios: any) {
     const copyStatusAndStop = (span: Span, response: any) => {
       if (response) {
         if (response.status) {
@@ -54,7 +55,7 @@ class AxiosPlugin implements SwPlugin {
       span.stop();
     };
 
-    this.axios.interceptors.request.use(
+    axios.interceptors.request.use(
       (config: any) => {
         config.span.resync();
 
@@ -73,7 +74,7 @@ class AxiosPlugin implements SwPlugin {
       },
     );
 
-    this.axios.interceptors.response.use(
+    axios.interceptors.response.use(
       (response: any) => {
         copyStatusAndStop(response.config.span, response);
 
@@ -89,14 +90,14 @@ class AxiosPlugin implements SwPlugin {
       },
     );
 
-    const _request = this.axios.Axios.prototype.request;
+    const _request = axios.Axios.prototype.request;
 
-    this.axios.Axios.prototype.request = function(config: any) {
+    axios.Axios.prototype.request = function(config: any) {
       const { host, pathname: operation } = new URL(config.url);  // TODO: this may throw invalid URL
       const span = ContextManager.current.newExitSpan(operation, host).start();
 
       try {
-        span.component = Component.AXIOS;  // TODO: add Component.AXIOS (to main Skywalking project)
+        span.component = Component.AXIOS;
         span.layer = SpanLayer.HTTP;
         span.peer = host;
         span.tag(Tag.httpURL(host + operation));
diff --git a/src/plugins/ExpressPlugin.ts b/src/plugins/ExpressPlugin.ts
index 2ca1e8a..15bde6e 100644
--- a/src/plugins/ExpressPlugin.ts
+++ b/src/plugins/ExpressPlugin.ts
@@ -25,17 +25,18 @@ import Tag from '../Tag';
 import { SpanLayer } from '../proto/language-agent/Tracing_pb';
 import { ContextCarrier } from '../trace/context/ContextCarrier';
 import onFinished from 'on-finished';
+import PluginInstaller from '../core/PluginInstaller';
 
 class ExpressPlugin implements SwPlugin {
   readonly module = 'express';
   readonly versions = '*';
 
-  install(): void {
-    this.interceptServerRequest();
+  install(installer: PluginInstaller): void {
+    this.interceptServerRequest(installer);
   }
 
-  private interceptServerRequest() {
-    const router = require('express/lib/router');
+  private interceptServerRequest(installer: PluginInstaller) {
+    const router = installer.require('express/lib/router');
     const _handle = router.handle;
 
     router.handle = function(req: IncomingMessage, res: ServerResponse, out: any) {
@@ -81,6 +82,7 @@ class ExpressPlugin implements SwPlugin {
           }
           out.call(this, err);
           stopped -= 1;  // skip first stop attempt, make sure stop executes once status code and message is set
+          onFinished(res, stopIfNotStopped);  // this must run after any handlers deferred in 'out'
         });
         onFinished(res, stopIfNotStopped); // this must run after any handlers deferred in 'out'