diff --git a/lib/fragment.js b/lib/fragment.js index 73d34bd..f7755dd 100644 --- a/lib/fragment.js +++ b/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 => { diff --git a/lib/request-handler.js b/lib/request-handler.js index 7cc74dd..ea725ba 100644 --- a/lib/request-handler.js +++ b/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(); }); }; diff --git a/tests/fragment.events.js b/tests/fragment.events.js index 6ac4036..72f976f 100644 --- a/tests/fragment.events.js +++ b/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); }); }); diff --git a/tests/tailor.js b/tests/tailor.js index 0e3bf28..701afd5 100644 --- a/tests/tailor.js +++ b/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( + '' + ); + + 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( + '' + ); + + 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); + }); + }); });