From c0d9852d9af109718d7e3618126b33b2d7b383ba Mon Sep 17 00:00:00 2001 From: "chris.smith" Date: Fri, 6 Jul 2018 16:21:46 -0400 Subject: [PATCH 1/2] record an error if there was one in the express middleware --- .../zipkin-instrumentation-express/src/expressMiddleware.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/zipkin-instrumentation-express/src/expressMiddleware.js b/packages/zipkin-instrumentation-express/src/expressMiddleware.js index 707f4a76..7cb93038 100644 --- a/packages/zipkin-instrumentation-express/src/expressMiddleware.js +++ b/packages/zipkin-instrumentation-express/src/expressMiddleware.js @@ -32,7 +32,9 @@ module.exports = function expressMiddleware({tracer, serviceName, port = 0}) { res.on('finish', () => { tracer.scoped(() => { - instrumentation.recordResponse(id, res.statusCode); + const error = res.statusCode >= 400 ? res.statusCode : null; + + instrumentation.recordResponse(id, res.statusCode, error); }); }); From b706671b196cbd09d0f091c30d516b5d763f14db Mon Sep 17 00:00:00 2001 From: "chris.smith" Date: Fri, 6 Jul 2018 17:48:29 -0400 Subject: [PATCH 2/2] add a test for the error annotation --- .../test/expressMiddlewareIntegrationTest.js | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/zipkin-instrumentation-express/test/expressMiddlewareIntegrationTest.js b/packages/zipkin-instrumentation-express/test/expressMiddlewareIntegrationTest.js index bbdd1a76..e2a86e69 100644 --- a/packages/zipkin-instrumentation-express/test/expressMiddlewareIntegrationTest.js +++ b/packages/zipkin-instrumentation-express/test/expressMiddlewareIntegrationTest.js @@ -130,4 +130,48 @@ describe('express middleware - integration test', () => { }); }); }); + + it('should mark 500 respones as errors', done => { + const record = sinon.spy(); + const recorder = {record}; + const ctxImpl = new ExplicitContext(); + const tracer = new Tracer({recorder, ctxImpl}); + + ctxImpl.scoped(() => { + const app = express(); + app.use(middleware({ + tracer, + serviceName: 'service-a' + })); + app.get('/error', (req, res) => { + tracer.recordBinary('message', 'hello from within app'); + res.status(500).send({status: 'An Error Occurred'}); + }); + const server = app.listen(0, () => { + const port = server.address().port; + const urlPath = `http://127.0.0.1:${port}/error`; + + fetch(urlPath, { + method: 'get' + }).then(res => { + server.close(); + + expect(res.status).to.equal(500); + + const annotations = record.args.map(args => args[0]); + + expect(annotations[6].annotation.key).to.equal('http.status_code'); + expect(annotations[6].annotation.value).to.equal('500'); + expect(annotations[7].annotation.key).to.equal('error'); + expect(annotations[7].annotation.value).to.equal('500'); + + done(); + }) + .catch(err => { + server.close(); + done(err); + }); + }); + }); + }); });