diff --git a/CHANGELOG.md b/CHANGELOG.md index 40162b43a4..a24a014960 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - disable edit of built-in strategies - Strip uknown fields in client requests. - Disable x-powered-by header +- Improved client-metrics validation to avoid NaN - Add posibility to inject custom logger provider ## 3.0.0-alpha.1 diff --git a/lib/client-metrics/client-metrics.test.js b/lib/client-metrics/client-metrics.test.js index d972b11666..3db556efab 100644 --- a/lib/client-metrics/client-metrics.test.js +++ b/lib/client-metrics/client-metrics.test.js @@ -335,3 +335,45 @@ test('should have correct values for lastHour', t => { metrics.destroy(); clock.restore(); }); + +test('should not fail when toggle metrics is missing yes/no field', t => { + const store = new EventEmitter(); + const metrics = new UnleashClientMetrics(store); + store.emit('metrics', { + appName, + instanceId, + bucket: { + start: new Date(), + stop: new Date(), + toggles: { + toggleX: { + yes: 123, + no: 0, + }, + }, + }, + }); + + metrics.addPayload({ + appName, + instanceId, + bucket: { + start: new Date(), + stop: new Date(), + toggles: { + toggleX: { + blue: 10, + green: 10, + }, + }, + }, + }); + + t.is(metrics.globalCount, 123); + t.deepEqual(metrics.getTogglesMetrics().lastMinute.toggleX, { + yes: 123, + no: 0, + }); + + metrics.destroy(); +}); diff --git a/lib/client-metrics/index.js b/lib/client-metrics/index.js index dfb911cee4..a2e7feacd6 100644 --- a/lib/client-metrics/index.js +++ b/lib/client-metrics/index.js @@ -25,7 +25,7 @@ module.exports = class UnleashClientMetrics { Object.keys(toggles).forEach(toggleName => { this.lastHourProjection.substract( toggleName, - toggles[toggleName] + this.createCountObject(toggles[toggleName]) ); }); }); @@ -33,7 +33,7 @@ module.exports = class UnleashClientMetrics { Object.keys(toggles).forEach(toggleName => { this.lastMinuteProjection.substract( toggleName, - toggles[toggleName] + this.createCountObject(toggles[toggleName]) ); }); }); @@ -91,6 +91,12 @@ module.exports = class UnleashClientMetrics { return this.apps[appName]; } + createCountObject(entry) { + const yes = typeof entry.yes == 'number' ? entry.yes : 0; + const no = typeof entry.no == 'number' ? entry.no : 0; + return { yes, no }; + } + addBucket(app, bucket) { let count = 0; // TODO stop should be createdAt @@ -99,10 +105,10 @@ module.exports = class UnleashClientMetrics { const toggleNames = Object.keys(toggles); toggleNames.forEach(n => { - const entry = toggles[n]; - this.lastHourProjection.add(n, entry); - this.lastMinuteProjection.add(n, entry); - count += entry.yes + entry.no; + const countObj = this.createCountObject(toggles[n]); + this.lastHourProjection.add(n, countObj); + this.lastMinuteProjection.add(n, countObj); + count += countObj.yes + countObj.no; }); this.lastHourList.add(toggles, stop); diff --git a/lib/routes/client-api/metrics-schema.js b/lib/routes/client-api/metrics-schema.js index 4c25ade26a..96ac7e4c33 100644 --- a/lib/routes/client-api/metrics-schema.js +++ b/lib/routes/client-api/metrics-schema.js @@ -2,13 +2,18 @@ const joi = require('joi'); +const countSchema = joi.object().options({ stripUnknown: true }).keys({ + yes: joi.number().required().min(0).default(0), + no: joi.number().required().min(0).default(0), +}); + const clientMetricsSchema = joi.object().options({ stripUnknown: true }).keys({ appName: joi.string().required(), instanceId: joi.string().required(), bucket: joi.object().required().keys({ start: joi.date().required(), stop: joi.date().required(), - toggles: joi.object(), + toggles: joi.object().pattern(/.*/, countSchema), }), }); diff --git a/lib/routes/client-api/metrics.test.js b/lib/routes/client-api/metrics.test.js index 310fd0e0be..eb956aa53f 100644 --- a/lib/routes/client-api/metrics.test.js +++ b/lib/routes/client-api/metrics.test.js @@ -31,7 +31,7 @@ test('should validate client metrics', t => { .expect(400); }); -test('should accept client metrics', t => { +test('should accept empty client metrics', t => { t.plan(0); const { request } = getSetup(); return request @@ -47,3 +47,47 @@ test('should accept client metrics', t => { }) .expect(202); }); + +test('should accept client metrics with yes/no', t => { + t.plan(0); + const { request } = getSetup(); + return request + .post('/api/client/metrics') + .send({ + appName: 'demo', + instanceId: '1', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: { + toggleA: { + yes: 200, + no: 0, + }, + }, + }, + }) + .expect(202); +}); + +test('should not accept client metrics without yes/no', t => { + t.plan(0); + const { request } = getSetup(); + return request + .post('/api/client/metrics') + .send({ + appName: 'demo', + instanceId: '1', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: { + toggleA: { + blue: 200, + green: 0, + }, + }, + }, + }) + .expect(400); +}); diff --git a/test/examples/client-register.json b/test/examples/client-register.json new file mode 100644 index 0000000000..d13c06e995 --- /dev/null +++ b/test/examples/client-register.json @@ -0,0 +1,8 @@ +{ + "appName": "appName", + "instanceId": "instanceId", + "sdkVersion": "test:123", + "strategies": ["default", "some-strategy-1"], + "started": "2016-11-03T07:16:43.572Z", + "interval": 10000 +}