express-http-proxy ^1.6.0 compatibility

openzipkin · Jan 23, 2020 · 7d7adbf · 7d7adbf
1 parent b3b5a0c
commit 7d7adbf
Show file tree

Hide file tree

Showing 4 changed files with 256 additions and 64 deletions.
diff --git a/packages/zipkin-instrumentation-express/package.json b/packages/zipkin-instrumentation-express/package.json
@@ -17,7 +17,7 @@
     "zipkin": "^0.19.1"
   },
   "devDependencies": {
-    "express-http-proxy": "^0.11.0",
+    "express-http-proxy": "^1.6.0",
     "node-fetch": "^2.6.0"
   }
 }
diff --git a/packages/zipkin-instrumentation-express/src/wrapExpressHttpProxy.js b/packages/zipkin-instrumentation-express/src/wrapExpressHttpProxy.js
@@ -50,33 +50,33 @@ class ExpressHttpProxyInstrumentation {
 
 function wrapProxy(proxy, {tracer, serviceName, remoteServiceName}) {
   return function zipkinProxy(host, options = {}) {
-    function wrapDecorateRequest(instrumentation, decorateRequest) {
+    function wrapProxyReqOptDecorator(instrumentation, proxyReqOptDecorator) {
       return (proxyReq, serverReq) => {
         const serverTraceId = serverReq._trace_id;
         let wrappedProxyReq = proxyReq;
-        if (typeof decorateRequest === 'function') {
+        if (typeof proxyReqOptDecorator === 'function') {
           tracer.letId(serverTraceId, () => {
-            wrappedProxyReq = decorateRequest(proxyReq, serverReq);
+            wrappedProxyReq = proxyReqOptDecorator(proxyReq, serverReq);
           });
         }
 
         return instrumentation.decorateAndRecordRequest(serverReq, wrappedProxyReq, serverTraceId);
       };
     }
 
-    function wrapIntercept(instrumentation, intercept) {
-      return (rsp, data, serverReq, res, callback) => {
-        const instrumentedCallback = (err, rspd, sent) => {
-          instrumentation.recordResponse(rsp, serverReq._trace_id_proxy);
-          return callback(err, rspd, sent);
-        };
-
+    function wrapUserResDecorator(instrumentation, userResDecorator) {
+      return (rsp, data, serverReq, res) => {
         const serverTraceId = serverReq._trace_id;
-        if (typeof intercept === 'function') {
-          tracer.letId(serverTraceId,
-            () => intercept(rsp, data, serverReq, res, instrumentedCallback));
+        if (typeof userResDecorator === 'function') {
+          let decoratedResponse;
+          tracer.letId(serverTraceId, () => {
+            decoratedResponse = userResDecorator(rsp, data, serverReq, res);
+            instrumentation.recordResponse(rsp, serverReq._trace_id_proxy);
+          });
+          return decoratedResponse;
         } else {
-          instrumentedCallback(null, data);
+          instrumentation.recordResponse(rsp, serverReq._trace_id_proxy);
+          return data;
         }
       };
     }
@@ -87,11 +87,13 @@ function wrapProxy(proxy, {tracer, serviceName, remoteServiceName}) {
 
     const wrappedOptions = options;
 
-    const {decorateRequest} = wrappedOptions;
-    wrappedOptions.decorateRequest = wrapDecorateRequest(instrumentation, decorateRequest);
+    const {proxyReqOptDecorator} = wrappedOptions;
+    wrappedOptions.proxyReqOptDecorator = wrapProxyReqOptDecorator(
+      instrumentation, proxyReqOptDecorator
+    );
 
-    const {intercept} = wrappedOptions;
-    wrappedOptions.intercept = wrapIntercept(instrumentation, intercept);
+    const {userResDecorator} = wrappedOptions;
+    wrappedOptions.userResDecorator = wrapUserResDecorator(instrumentation, userResDecorator);
 
     return proxy(host, wrappedOptions);
   };

diff --git a/packages/zipkin-instrumentation-express/test/wrapExpressHttpProxyIntegrationTest.js b/packages/zipkin-instrumentation-express/test/wrapExpressHttpProxyIntegrationTest.js
@@ -31,13 +31,13 @@ describe('express proxy instrumentation - integration test', () => {
       const zipkinProxy = wrapProxy(proxy, {tracer: tracer.tracer(), remoteServiceName});
       frontendApp.use(middleware({tracer: tracer.tracer()}));
       frontendApp.use(zipkinProxy(`127.0.0.1:${backend.address().port}`, {
-        decorateRequest: (proxyReq) => {
-          tracer.tracer().recordBinary('decorateRequest', '');
+        proxyReqOptDecorator: (proxyReq) => {
+          tracer.tracer().recordBinary('proxyReqOptDecorator', '');
           return proxyReq;
         },
-        intercept: (rsp, data, serverReq, res, callback) => {
-          tracer.tracer().recordBinary('intercept', '');
-          callback(null, data);
+        userResDecorator: (rsp, data, serverReq, res) => {
+          tracer.tracer().recordBinary('userResDecorator', '');
+          return data;
         }
       }));
 
@@ -61,8 +61,8 @@ describe('express proxy instrumentation - integration test', () => {
       tags: {
         'http.path': path,
         'http.status_code': '200',
-        intercept: '',
-        decorateRequest: ''
+        userResDecorator: '',
+        proxyReqOptDecorator: ''
       }
     });
   }
@@ -154,7 +154,7 @@ describe('express proxy instrumentation - integration test', () => {
         'http.path': path,
         'http.status_code': '500',
         error: '500', // TODO: better error message
-        decorateRequest: ''
+        proxyReqOptDecorator: ''
       }
     }));
   });