zipkin-instrumentation-fetch - npm Package Compare versions

Comparing version 0.18.4 to 0.18.5

package.json

 {
   "name": "zipkin-instrumentation-fetch",
-  "version": "0.18.4",
+  "version": "0.18.5",
   "description": "Interceptor for HTTP clients using the 'fetch' API",
@@ -8,3 +8,4 @@ "main": "lib/index.js",
     "build": "babel src -d lib",
-    "test": "mocha --exit --require ../../test/helper.js",
+    "test": "mocha --exit --require ../../test/helper.js --require @babel/register && karma start --single-run --browsers ChromeHeadless,FirefoxHeadless ../../karma.conf.js",
+    "test-browser": "karma start --single-run --browsers ChromeHeadless ../../karma.conf.js",
     "test-debug": "mocha --inspect-brk --exit --require ../../test/helper.js",
@@ -16,7 +17,9 @@ "prepublish": "npm run build"
   "repository": "https://github.com/openzipkin/zipkin-js",
+  "dependencies": {
+    "zipkin": "^0.18.5"
+  },
   "devDependencies": {
-    "node-fetch": "^1.5.3",
-    "zipkin": "^0.18.4"
+    "node-fetch": "^1.5.3"
   },
-  "gitHead": "2c9ea42842e03343fa6b2066dedf6e760f38e9f6"
+  "gitHead": "201c6f6360d7ae79eaa04d5e53660908e3602358"
 }

src/wrapFetch.js

@@ -12,7 +12,6 @@ const {
         const method = opts.method || 'GET';
-        const zipkinOpts =
-          instrumentation.recordRequest(opts, url, method);
+        const zipkinOpts = instrumentation.recordRequest(opts, url, method);
         const traceId = tracer.id;
 
-        fetch(url, zipkinOpts).then(res => {
+        fetch(url, zipkinOpts).then((res) => {
           tracer.scoped(() => {
@@ -22,3 +21,3 @@ instrumentation.recordResponse(traceId, res.status);
           resolve(res);
-        }).catch(err => {
+        }).catch((err) => {
           tracer.scoped(() => {
@@ -25,0 +24,0 @@ instrumentation.recordError(traceId, err);

test/integrationTest.js

@@ -1,170 +0,223 @@
-const {Tracer, ExplicitContext, createNoopTracer} = require('zipkin');
-const express = require('express');
-const nodeFetch = require('node-fetch');
-const sinon = require('sinon');
+const {expect} = require('chai');
+const {ExplicitContext, Tracer} = require('zipkin');
+const {
+  maybeMiddleware,
+  newSpanRecorder,
+  expectB3Headers,
+  expectSpan
+} = require('../../../test/testFixture');
+
+// defer lookup of node fetch until we know if we are node
 const wrapFetch = require('../src/wrapFetch');
 
-describe('wrapFetch', () => {
-  before(function(done) {
-    const app = express();
-    app.post('/user', (req, res) => res.status(202).json({
-      traceId: req.header('X-B3-TraceId') || '?',
-      spanId: req.header('X-B3-SpanId') || '?'
-    }));
-    app.get('/user', (req, res) => res.status(202).json({}));
-    this.server = app.listen(0, () => {
-      this.port = this.server.address().port;
+describe('fetch instrumentation - integration test', () => {
+  const serviceName = 'weather-app';
+  const remoteServiceName = 'weather-api';
+
+  let server;
+  let baseURL = ''; // default to relative path, for browser-based tests
+
+  before((done) => {
+    const middleware = maybeMiddleware();
+    if (middleware !== null) {
+      server = middleware.listen(0, () => {
+        baseURL = `http://127.0.0.1:${server.address().port}`;
+        done();
+      });
+    } else { // Inside a browser
       done();
-    });
+    }
   });
 
-  after(function(done) {
-    this.server.close(done);
+  after(() => {
+    if (server) server.close();
   });
 
-  it('should add instrumentation to "fetch"', function(done) {
-    const record = sinon.spy();
-    const recorder = {record};
-    const ctxImpl = new ExplicitContext();
-    const tracer = new Tracer({recorder, ctxImpl});
+  let spans;
+  let tracer;
 
-    const fetch = wrapFetch(nodeFetch, {
-      tracer,
-      serviceName: 'caller',
-      remoteServiceName: 'callee'
+  beforeEach(() => {
+    spans = [];
+    tracer = new Tracer({
+      ctxImpl: new ExplicitContext(),
+      localServiceName: serviceName,
+      recorder: newSpanRecorder(spans)
     });
+  });
 
-    ctxImpl.scoped(() => {
-      const id = tracer.createChildId();
-      tracer.setId(id);
+  function popSpan() {
+    expect(spans).to.not.be.empty; // eslint-disable-line no-unused-expressions
+    return spans.pop();
+  }
 
-      const host = '127.0.0.1';
-      const urlPath = '/user';
-      const url = `http://${host}:${this.port}${urlPath}`;
-      fetch(url, {method: 'post'})
-        .then(res => res.json())
-        .then(data => {
-          const annotations = record.args.map(args => args[0]);
+  function wrappedFetch() {
+    let fetch;
+    if (server) { // defer loading node-fetch
+      fetch = require('node-fetch'); // eslint-disable-line global-require
+    } else {
+      fetch = window.fetch; // eslint-disable-line
+    }
+    return wrapFetch(fetch, {tracer, remoteServiceName});
+  }
 
-          // All annotations should have the same trace id and span id
-          const traceId = annotations[0].traceId.traceId;
-          const spanId = annotations[0].traceId.spanId;
-          annotations.forEach(ann => expect(ann.traceId.traceId).to.equal(traceId));
-          annotations.forEach(ann => expect(ann.traceId.spanId).to.equal(spanId));
+  function url(path) {
+    return `${baseURL}${path}?index=10&count=300`;
+  }
 
-          expect(annotations[0].annotation.annotationType).to.equal('ServiceName');
-          expect(annotations[0].annotation.serviceName).to.equal('caller');
+  function successSpan(path) {
+    return ({
+      name: 'get',
+      kind: 'CLIENT',
+      localEndpoint: {serviceName},
+      remoteEndpoint: {serviceName: remoteServiceName},
+      tags: {
+        'http.path': path,
+        'http.status_code': '200'
+      }
+    });
+  }
 
-          expect(annotations[1].annotation.annotationType).to.equal('Rpc');
-          expect(annotations[1].annotation.name).to.equal('POST');
+  it('should not interfere with errors that precede a call', (done) => {
+    // Here we are passing a function instead of the value of it. This ensures our error callback
+    // doesn't make assumptions about a span in progress: there won't be if there was a config error
+    wrappedFetch()(url)
+      .then((response) => {
+        done(new Error(`expected an invalid url parameter to error. status: ${response.status}`));
+      })
+      .catch((error) => {
+        const {message} = error;
+        const expected = ['must be of type string', 'must be a string']; // messages can vary in CI
+        if (message.indexOf(expected[0]) !== -1 || message.indexOf(expected[1]) !== -1) {
+          done();
+        } else {
+          done(new Error(`expected error message to match [${expected.toString()}]: ${message}`));
+        }
+      });
+  });
 
-          expect(annotations[2].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[2].annotation.key).to.equal('http.path');
-          expect(annotations[2].annotation.value).to.equal(urlPath);
+  it('should add headers to requests', () => {
+    const path = '/weather/wuhan';
+    return wrappedFetch()(url(path))
+      .then(response => response.json()) // json() returns a promise
+      .then(json => expectB3Headers(popSpan(), json));
+  });
 
-          expect(annotations[3].annotation.annotationType).to.equal('ClientSend');
 
-          expect(annotations[4].annotation.annotationType).to.equal('ServerAddr');
-          expect(annotations[4].annotation.serviceName).to.equal('callee');
+  it('should support get request', () => {
+    const path = '/weather/wuhan';
+    return wrappedFetch()(url(path))
+      .then(() => expectSpan(popSpan(), successSpan(path)));
+  });
 
-          expect(annotations[5].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[5].annotation.key).to.equal('http.status_code');
-          expect(annotations[5].annotation.value).to.equal('202');
+  it('should support options request', () => {
+    const path = '/weather/wuhan';
+    return wrappedFetch()({url: url(path), method: 'GET'})
+      .then(() => expectSpan(popSpan(), successSpan(path)));
+  });
 
-          expect(annotations[6].annotation.annotationType).to.equal('ClientRecv');
+  it('should report 404 in tags', () => {
+    const path = '/pathno';
+    return wrappedFetch()(url(path))
+      .then(() => expectSpan(popSpan(), {
+        name: 'get',
+        kind: 'CLIENT',
+        localEndpoint: {serviceName},
+        remoteEndpoint: {serviceName: remoteServiceName},
+        tags: {
+          'http.path': path,
+          'http.status_code': '404',
+          error: '404'
+        }
+      }));
+  });
 
-          const traceIdOnServer = data.traceId;
-          expect(traceIdOnServer).to.equal(traceId);
+  it('should report 401 in tags', () => {
+    const path = '/weather/securedTown';
+    return wrappedFetch()(url(path))
+      .then(() => expectSpan(popSpan(), {
+        name: 'get',
+        kind: 'CLIENT',
+        localEndpoint: {serviceName},
+        remoteEndpoint: {serviceName: remoteServiceName},
+        tags: {
+          'http.path': path,
+          'http.status_code': '401',
+          error: '401'
+        }
+      }));
+  });
 
-          const spanIdOnServer = data.spanId;
-          expect(spanIdOnServer).to.equal(spanId);
-        })
-        .then(done)
-        .catch(done);
-    });
+  it('should report 500 in tags', () => {
+    const path = '/weather/bagCity';
+    return wrappedFetch()(url(path))
+      .then(() => expectSpan(popSpan(), {
+        name: 'get',
+        kind: 'CLIENT',
+        localEndpoint: {serviceName},
+        remoteEndpoint: {serviceName: remoteServiceName},
+        tags: {
+          'http.path': path,
+          'http.status_code': '500',
+          error: '500'
+        }
+      }));
   });
 
-  it('should not throw when using fetch without options', function(done) {
-    const tracer = createNoopTracer();
-    const fetch = wrapFetch(nodeFetch, {serviceName: 'user-service', tracer});
-
-    const path = `http://127.0.0.1:${this.port}/user`;
-    fetch(path)
-      .then(res => res.json())
-      .then(() => {
+  it('should report when endpoint doesnt exist in tags', (done) => {
+    const path = '/badHost';
+    const badUrl = `http://localhost:12345${path}`;
+    wrappedFetch()(badUrl)
+      .then((response) => {
+        done(new Error(`expected an invalid host to error. status: ${response.status}`));
+      })
+      .catch((error) => {
+        expectSpan(popSpan(), {
+          name: 'get',
+          kind: 'CLIENT',
+          localEndpoint: {serviceName},
+          remoteEndpoint: {serviceName: remoteServiceName},
+          tags: {
+            'http.path': path,
+            error: error.toString()
+          }
+        });
         done();
-      })
-      .catch(done);
+      });
   });
 
-  it('should not throw when using fetch with a request object', function(done) {
-    const tracer = createNoopTracer();
-    const fetch = wrapFetch(nodeFetch, {serviceName: 'user-service', tracer});
+  it('should support nested get requests', () => {
+    const client = wrappedFetch();
 
-    const path = `http://127.0.0.1:${this.port}/user`;
-    const request = {url: path};
-    fetch(request)
-      .then(res => res.json())
-      .then(() => {
-        done();
-      })
-      .catch(done);
-  });
+    const beijing = '/weather/beijing';
+    const wuhan = '/weather/wuhan';
 
-  it('should record error', (done) => {
-    const record = sinon.spy();
-    const recorder = {record};
-    const ctxImpl = new ExplicitContext();
-    const tracer = new Tracer({recorder, ctxImpl});
+    const getBeijingWeather = client(url(beijing));
+    const getWuhanWeather = client(url(wuhan));
 
-    const fetch = wrapFetch(nodeFetch, {
-      tracer,
-      serviceName: 'caller',
-      remoteServiceName: 'callee'
+    return getBeijingWeather.then(() => {
+      getWuhanWeather.then(() => {
+        // since these are sequential, we should have an expected order
+        expectSpan(popSpan(), successSpan(wuhan));
+        expectSpan(popSpan(), successSpan(beijing));
+      });
     });
+  });
 
-    ctxImpl.scoped(() => {
-      const id = tracer.createChildId();
-      tracer.setId(id);
+  it('should support parallel get requests', () => {
+    const client = wrappedFetch();
 
-      const host = 'domain.invalid';
-      const url = `http://${host}`;
-      fetch(url, {method: 'post'})
-        .then(() => expect.fail())
-        .catch(() => {
-          const annotations = record.args.map(args => args[0]);
+    const beijing = '/weather/beijing';
+    const wuhan = '/weather/wuhan';
 
-          // All annotations should have the same trace id and span id
-          const traceId = annotations[0].traceId.traceId;
-          const spanId = annotations[0].traceId.spanId;
-          annotations.forEach(ann => expect(ann.traceId.traceId).to.equal(traceId));
-          annotations.forEach(ann => expect(ann.traceId.spanId).to.equal(spanId));
+    const getBeijingWeather = client(url(beijing));
+    const getWuhanWeather = client(url(wuhan));
 
-          expect(annotations[0].annotation.annotationType).to.equal('ServiceName');
-          expect(annotations[0].annotation.serviceName).to.equal('caller');
-
-          expect(annotations[1].annotation.annotationType).to.equal('Rpc');
-          expect(annotations[1].annotation.name).to.equal('POST');
-
-          expect(annotations[2].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[2].annotation.key).to.equal('http.path');
-          expect(annotations[2].annotation.value).to.equal('/');
-
-          expect(annotations[3].annotation.annotationType).to.equal('ClientSend');
-
-          expect(annotations[4].annotation.annotationType).to.equal('ServerAddr');
-          expect(annotations[4].annotation.serviceName).to.equal('callee');
-
-          expect(annotations[5].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[5].annotation.key).to.equal('error');
-          expect(annotations[5].annotation.value)
-            .to.contain('getaddrinfo ENOTFOUND domain.invalid');
-
-          expect(annotations[6].annotation.annotationType).to.equal('ClientRecv');
-
-          expect(annotations[7]).to.be.undefined; // eslint-disable-line no-unused-expressions
-          done();
-        });
+    return Promise.all([getBeijingWeather, getWuhanWeather]).then(() => {
+      // since these are parallel, we have an unexpected order
+      const firstPath = spans[0].tags['http.path'] === wuhan ? beijing : wuhan;
+      const secondPath = firstPath === wuhan ? beijing : wuhan;
+      expectSpan(popSpan(), successSpan(firstPath));
+      expectSpan(popSpan(), successSpan(secondPath));
     });
   });
 });

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Major refactor

Supply chain risk

Package has recently undergone a major refactor. It may be unstable or indicate significant internal changes. Use caution when updating to versions that include significant changes.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

10662

increased by5.46%

Dev dependency count

1

decreased by-50%

Lines of code

260

increased by28.08%

Number of medium supply chain risk alerts

4

decreased by-33.33%

Worsened metrics

Dependency count

1

increased byInfinity%

Dependency changes

• + Addedzipkin@^0.18.5

• + Addedbase64-js@1.5.1(transitive)

• + Addedis-promise@2.2.2(transitive)

• + Addedzipkin@0.18.6(transitive)