refactor(opentracing): add tests and pass correct span contexts

lib/fragment.js

@@ -86,7 +86,7 @@ module.exports = class Fragment extends EventEmitter {
      * @param {object} parentSpan - opentracing Span that will be the parent of the current operation
      * @returns {object} Fragment response streams in case of synchronous fragment or buffer in case of async fragment
      */
-    fetch(request, isFallback, parentSpan = null) {
+    fetch(request, isFallback = false, parentSpan = null) {
         if (!isFallback) {
             this.emit('start');
         }
@@ -97,43 +97,41 @@ module.exports = class Fragment extends EventEmitter {
         const spanOptions = parentSpan ? { childOf: parentSpan } : {};
         const span = tracer.startSpan('fetch-fragment', spanOptions);
         span.addTags({
-            'fragment-url': url,
-            'fragment-id': this.attributes.id || 'unnamed',
+            [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT,
+            url: url,
+            id: this.attributes.id || 'unnamed',
             fallback: isFallback,
             primary: this.attributes.primary,
             async: this.attributes.async,
-            public: this.attributes.public,
-            [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT
+            public: this.attributes.public
         });
-        this.requestFragment(url, this.attributes, request, span)
-            .then(
-                res => this.onResponse(res, isFallback),
-                err => {
-                    if (!isFallback) {
-                        const { fallbackUrl } = this.attributes;
-                        if (fallbackUrl) {
-                            this.emit('fallback', err);
-                            this.fetch(request, true, span);
-                        } else {
-                            this.emit('error', err);
-                            span.setTag(Tags.ERROR, true);
-                            span.log({
-                                message: err.message,
-                                stack: err.stack
-                            });
-                            this.stream.end();
-                        }
+
+        this.requestFragment(url, this.attributes, request, span).then(
+            res => this.onResponse(res, isFallback, span),
+            err => {
+                if (!isFallback) {
+                    const { fallbackUrl } = this.attributes;
+                    if (fallbackUrl) {
+                        this.emit('fallback', err);
+                        this.fetch(request, true, span);
                     } else {
                         span.setTag(Tags.ERROR, true);
                         span.log({
-                            message: err.message,
-                            stack: err.stack
+                            message: err.message
                         });
+                        this.emit('error', err);
                         this.stream.end();
                     }
+                } else {
+                    span.setTag(Tags.ERROR, true);
+                    span.log({
+                        message: err.message
+                    });
+                    this.stream.end();
                 }
-            )
-            .then(() => span.finish());
+                span.finish();
+            }
+        );
         // Async fragments are piped later on the page
         if (this.attributes.async) {
             return Buffer.from(
@@ -147,9 +145,10 @@ module.exports = class Fragment extends EventEmitter {
     /**
      * Handle the fragment response
      * @param {object} response - HTTP response stream from fragment
-     * @param {boolean} isFallback - decides between response from fragment or fallback URL
+     * @param {boolean} isFallback - decides between response from fragment src or fallback src
+     * @param {object} span - fetch-fragment opentracing span
      */
-    onResponse(response, isFallback) {
+    onResponse(response, isFallback, span) {
         const { statusCode, headers } = response;
 
         if (!isFallback) {
@@ -180,6 +179,7 @@ module.exports = class Fragment extends EventEmitter {
         contentLengthStream.on('end', () => {
             this.insertEnd();
             this.stream.end();
+            span.finish();
         });
 
         const handleError = err => {

lib/request-handler.js

@@ -153,9 +153,17 @@ module.exports = function processRequest(options, request, response) {
 
         fragment.once('error', err => {
             this.emit('error', request, err);
-            span.setTag(Tags.HTTP_STATUS_CODE, 500);
+            span.addTags({
+                [Tags.ERROR]: true,
+                [Tags.HTTP_STATUS_CODE]: 500
+            });
+            span.log({
+                message: err.message,
+                stack: err.stack
+            });
             response.writeHead(500, responseHeaders);
             response.end(INTERNAL_SERVER_ERROR);
+            span.finish();
         });
     };
 

tests/fragment.events.js

@@ -1,7 +1,10 @@
 'use strict';
-const Fragment = require('../lib/fragment');
 const assert = require('assert');
 const nock = require('nock');
+const sinon = require('sinon');
+const Fragment = require('../lib/fragment');
+const requestFragment = require('../lib/request-fragment');
+
 const TAG = { attributes: { src: 'https://fragment' } };
 const TAG_FALLBACK = {
     attributes: {
@@ -12,8 +15,6 @@ const TAG_FALLBACK = {
 const REQUEST = { headers: {} };
 const RESPONSE_HEADERS = { connection: 'close' };
 const filterHeaderFn = () => ({});
-const sinon = require('sinon');
-const requestFragment = require('../lib/request-fragment');
 const getOptions = tag => {
     return {
         tag,
@@ -30,7 +31,7 @@ describe('Fragment events', () => {
             .reply(200, 'OK');
         const fragment = new Fragment(getOptions(TAG));
         fragment.on('start', done);
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
     });
 
     it('triggers `fallback` event', done => {
@@ -44,7 +45,7 @@ describe('Fragment events', () => {
         fragment.on('fallback', () => {
             done();
         });
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
     });
 
     it('should not trigger error and response event when fallback is triggered', done => {
@@ -65,7 +66,7 @@ describe('Fragment events', () => {
             assert.equal(onError.callCount, 0);
             done();
         });
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
         fragment.stream.resume();
     });
 
@@ -79,7 +80,7 @@ describe('Fragment events', () => {
             assert.deepEqual(headers, RESPONSE_HEADERS);
             done();
         });
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
     });
 
     it('triggers `end(contentSize)` when the content is succesfully retreived', done => {
@@ -91,7 +92,7 @@ describe('Fragment events', () => {
             assert.equal(contentSize, 5);
             done();
         });
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
         fragment.stream.resume();
     });
 
@@ -104,7 +105,7 @@ describe('Fragment events', () => {
             assert.ok(error);
             done();
         });
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
     });
 
     it('should not trigger `response` and `end` for fallback fragment', done => {
@@ -118,7 +119,7 @@ describe('Fragment events', () => {
         fragment.on('response', onResponse);
         fragment.on('end', onEnd);
         fragment.on('fallback', onFallback);
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
         fragment.stream.on('end', () => {
             assert.equal(onResponse.callCount, 0);
             assert.equal(onEnd.callCount, 0);
@@ -139,7 +140,7 @@ describe('Fragment events', () => {
         fragment.on('response', onResponse);
         fragment.on('end', onEnd);
         fragment.on('error', onError);
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
         fragment.stream.on('end', () => {
             assert.equal(onResponse.callCount, 0);
             assert.equal(onEnd.callCount, 0);
@@ -162,7 +163,7 @@ describe('Fragment events', () => {
             assert.equal(error.message, ERROR.message);
             done();
         });
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
     });
 
     it('triggers `error(error)` when fragment times out', done => {
@@ -176,6 +177,6 @@ describe('Fragment events', () => {
             assert.equal(err.message, 'socket hang up');
             done();
         });
-        fragment.fetch(REQUEST, false);
+        fragment.fetch(REQUEST);
     });
 });

tests/tailor.js

@@ -10,9 +10,17 @@ const { TEMPLATE_NOT_FOUND } = require('../lib/fetch-template');
 const Tailor = require('../index');
 const processTemplate = require('../lib/process-template');
 const PIPE_DEFINITION = readFileSync(resolve(__dirname, '../src/pipe.min.js'));
+const { MockTracer } = require('opentracing');
+
+//Custom mock tracer for Unit tests
+class CustomTracer extends MockTracer {
+    inject() {}
+    extract() {}
+}
 
 describe('Tailor', () => {
     let server;
+    const tracer = new CustomTracer();
     const mockTemplate = sinon.stub();
     const mockChildTemplate = sinon.stub();
     const mockContext = sinon.stub();
@@ -85,7 +93,8 @@ describe('Tailor', () => {
                 },
                 pipeInstanceName,
                 pipeAttributes: attributes => ({ id: attributes.id }),
-                filterResponseHeaders: (attributes, headers) => headers
+                filterResponseHeaders: (attributes, headers) => headers,
+                tracer
             },
             pipeDefinition !== undefined ? { pipeDefinition } : {}
         );
@@ -1297,4 +1306,103 @@ describe('Tailor', () => {
                 .then(done, done);
         });
     });
+
+    describe('OpenTracing', () => {
+        beforeEach(() => {
+            tracer.clear();
+        });
+
+        function traceResults() {
+            const { spans } = tracer.report();
+            const tags = spans.map(s => s.tags());
+            const logs = spans.map(s => s._logs[0]);
+            return { tags, logs };
+        }
+
+        it('process request spans', done => {
+            mockTemplate.returns('Test');
+            getResponse('http://localhost:8080/test')
+                .then(() => {
+                    const { tags } = traceResults();
+                    assert.equal(tags.length, 1);
+                    assert.deepEqual(tags[0], {
+                        'http.url': '/test',
+                        'span.kind': 'server'
+                    });
+                })
+                .then(done, done);
+        });
+
+        it('template error request spans & logs', done => {
+            mockTemplate.returns('');
+            getResponse('http://localhost:8080/error')
+                .then(() => {
+                    const { tags, logs } = traceResults();
+                    assert.deepEqual(tags[0], {
+                        'http.url': '/error',
+                        'span.kind': 'server',
+                        error: true,
+                        'http.status_code': 500
+                    });
+                    assert.equal(logs.length, 1);
+                })
+                .then(done, done);
+        });
+
+        it('process request + primary fragment error spans', done => {
+            nock('https://fragment')
+                .get('/1')
+                .reply(500);
+
+            mockTemplate.returns(
+                '<fragment primary src="https://fragment/1"></fragment>'
+            );
+
+            getResponse('http://localhost:8080/test')
+                .then(() => {
+                    const { tags } = traceResults();
+                    // Tailor should return error
+                    assert.equal(tags[0].error, true);
+                    // Primary fragment error
+                    const primaryTag = {
+                        primary: tags[1].primary,
+                        error: tags[1].error
+                    };
+                    assert.deepEqual(primaryTag, {
+                        error: true,
+                        primary: true
+                    });
+                })
+                .then(done, done);
+        });
+
+        it('process request + fragment error & fallback spans', done => {
+            nock('https://fragment')
+                .get('/1')
+                .reply(500);
+
+            nock('http://fragment:9000')
+                .get('/2')
+                .reply(500);
+
+            mockTemplate.returns(
+                '<fragment id="test" src="https://fragment/1" fallback-src="http://localhost:9000/2"></fragment>'
+            );
+
+            getResponse('http://localhost:8080/test')
+                .then(() => {
+                    const { tags } = traceResults();
+                    assert.deepEqual(tags[1], {
+                        'span.kind': 'client',
+                        url: 'https://fragment/1',
+                        id: 'test',
+                        fallback: false,
+                        primary: false,
+                        async: false,
+                        public: false
+                    });
+                })
+                .then(done, done);
+        });
+    });
 });