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 2020/12/31 09:55:22 UTC

[skywalking-nodejs] branch bugfix/express created (now 7338fca)

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

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


      at 7338fca  Fix bug that some requests of express are not close

This branch includes the following new commits:

     new 7338fca  Fix bug that some requests of express are not close

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



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

Posted by 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'