From 39bf4d224838d3ed5f138b581bce8f7fa34c06e8 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Fri, 7 May 2021 19:24:37 -0300 Subject: [PATCH 01/17] Daily telemetry --- webapp/src/ts/services/telemetry.service.ts | 53 ++++++++++++++++----- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/webapp/src/ts/services/telemetry.service.ts b/webapp/src/ts/services/telemetry.service.ts index 228352801ef..d0dd2bca504 100644 --- a/webapp/src/ts/services/telemetry.service.ts +++ b/webapp/src/ts/services/telemetry.service.ts @@ -9,7 +9,7 @@ import { SessionService } from '@mm-services/session.service'; providedIn: 'root' }) /** - * TelemetryService: Records, aggregates, and submits telemetry data + * TelemetryService: Records, aggregates, and submits telemetry data. */ export class TelemetryService { // Intentionally scoped to the whole browser (for this domain). We can then tell if multiple users use the same device @@ -54,7 +54,16 @@ export class TelemetryService { return uniqueDeviceId; } - private getLastAggregatedDate() { + /** + * Returns the time in milliseconds (since Unix epoch) when the first telemetry + * record was created within the day that is going to be aggregated. + * + * The date is stored locally once computed by this method, either because is + * the first time is called or because the aggregation was performed last time + * a record was created, therefore `reset(db)' deleted it, and next time this + * method is called the date need to be fetched from the system again. + */ + private getFirstAggregatedDate() { let date = parseInt(window.localStorage.getItem(this.LAST_AGGREGATED_DATE_KEY)); if (!date) { @@ -73,14 +82,26 @@ export class TelemetryService { }); } - private submitIfNeeded(db) { - const monthStart = moment().startOf('month'); - const dbDate = moment(this.getLastAggregatedDate()); + // moment when the aggregation starts (the beginning of the current day) + private aggregateStartsAt() { + return moment().startOf('day'); + //return moment().startOf('minute'); + } - if (dbDate.isBefore(monthStart)) { - return this - .aggregate(db) - .then(() => this.reset(db)); + // if there is telemetry data to aggregate and is before the current date, + // aggregation is performed and the data destroyed + private submitIfNeeded(db) { + const startOf = this.aggregateStartsAt(); + const dbDate = moment(this.getFirstAggregatedDate()); + + if (dbDate.isBefore(startOf)) { + return db.info() + .then(info => { + if (info.doc_count > 0) { + return this.aggregate(db) + .then(() => this.reset(db)); + } + }); } } @@ -97,6 +118,9 @@ export class TelemetryService { 'telemetry', metadata.year, metadata.month, + metadata.day, + //metadata.hour, + //metadata.minute, metadata.user, metadata.deviceId, ].join('-'); @@ -109,7 +133,7 @@ export class TelemetryService { this.dbService.get().query('medic-client/doc_by_type', { key: ['form'], include_docs: true }) ]) .then(([ddoc, formResults]) => { - const date = moment(this.getLastAggregatedDate()); + const date = moment(this.getFirstAggregatedDate()); const version = (ddoc.deploy_info && ddoc.deploy_info.version) || 'unknown'; const forms = formResults.rows.reduce((keyToVersion, row) => { keyToVersion[row.doc.internalId] = row.doc._rev; @@ -120,6 +144,9 @@ export class TelemetryService { return { year: date.year(), month: date.month() + 1, + day: date.date(), + //hour: date.hour(), + //minute: date.minute(), user: this.sessionService.userCtx().name, deviceId: this.getUniqueDeviceId(), versions: { @@ -225,7 +252,7 @@ export class TelemetryService { * metric_b: { sum: -16, min: -4, max: -4, count: 4, sumsqr: 64 } * } * - * See: https://wiki.apache.org/couchdb/Built-In_Reduce_Functions#A_stats + * See: https://docs.couchdb.org/en/stable/ddocs/ddocs.html#_stats * * This single month aggregate document is of type 'telemetry', and is * stored in the user's meta DB (which replicates up to the main server) @@ -255,14 +282,14 @@ export class TelemetryService { let db; this.queue = this.queue .then(() => db = this.getDb()) - .then(() => this.storeIt(db, key, value)) .then(() => this.submitIfNeeded(db)) + .then(() => db = this.getDb()) // db is fetched again in case submitIfNeeded dropped the old reference + .then(() => this.storeIt(db, key, value)) .catch(err => console.error('Error in telemetry service', err)) .finally(() => { if (!db || db._destroyed || db._closed) { return; } - try { db.close(); } catch (err) { From b9eca760ee1415109ef3e70a34620856fd820c32 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Mon, 10 May 2021 18:44:48 -0300 Subject: [PATCH 02/17] Add telemetry tests. Refactor --- .../ts/services/telemetry.service.spec.ts | 282 +++++++++++------- 1 file changed, 172 insertions(+), 110 deletions(-) diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index 29ee09fb1b7..56251790bc0 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -8,7 +8,7 @@ import { DbService } from '@mm-services/db.service'; import { SessionService } from '@mm-services/session.service'; describe('TelemetryService', () => { - const NOW = new Date(2018, 10, 10, 12, 33).getTime(); + const NOW = new Date(2018, 10, 10, 12, 33).getTime(); // -> 2018-11-10T12:33:00 let service: TelemetryService; let dbService; let dbInstance; @@ -18,7 +18,9 @@ describe('TelemetryService', () => { let storageGetItemStub; let storageSetItemStub; let consoleErrorSpy; + const windowPouchOriginal = window.PouchDB; + const windowScreenOriginal = { availWidth: window.screen.availWidth, availHeight: window.screen.availHeight @@ -28,7 +30,43 @@ describe('TelemetryService', () => { hardwareConcurrency: window.navigator.hardwareConcurrency }; + function defineWindow() { + Object.defineProperty(window.navigator, 'userAgent', + { value: 'Agent Smith', configurable: true }); + Object.defineProperty(window.navigator, 'hardwareConcurrency', + { value: 4, configurable: true }); + Object.defineProperty(window.screen, 'availWidth', + { value: 768, configurable: true }); + Object.defineProperty(window.screen, 'availHeight', + { value: 1024, configurable: true }); + } + + function restoreWindow() { + Object.defineProperty(window.navigator, 'userAgent', + { value: windowNavigatorOriginal.userAgent, configurable: true }); + Object.defineProperty(window.navigator, 'hardwareConcurrency', + { value: windowNavigatorOriginal.hardwareConcurrency, configurable: true }); + Object.defineProperty(window.screen, 'availWidth', + { value: windowScreenOriginal.availWidth, configurable: true }); + Object.defineProperty(window.screen, 'availHeight', + { value: windowScreenOriginal.availHeight, configurable: true }); + } + + function subtractDays(numDays) { + return moment() + .subtract(numDays, 'days') + .valueOf() + .toString(); + } + + function sameDay() { + return moment() + .valueOf() + .toString(); + } + beforeEach(() => { + defineWindow(); dbInstance = { info: sinon.stub(), put: sinon.stub(), @@ -38,8 +76,13 @@ describe('TelemetryService', () => { dbService = { get: () => dbInstance }; consoleErrorSpy = sinon.spy(console, 'error'); pouchDb = { + info: sinon.stub().resolves({doc_count: 10}), post: sinon.stub().resolves(), - close: sinon.stub() + close: sinon.stub(), + destroy: sinon.stub().callsFake(() => { + pouchDb._destroyed = true; + return Promise.resolve(); + }) }; sessionService = { userCtx: sinon.stub().returns({ name: 'greg' }) }; storageGetItemStub = sinon.stub(window.localStorage, 'getItem'); @@ -61,38 +104,13 @@ describe('TelemetryService', () => { clock.restore(); sinon.restore(); window.PouchDB = windowPouchOriginal; - Object.defineProperty( - window.navigator, - 'userAgent', - { value: windowNavigatorOriginal.userAgent, configurable: true } - ); - Object.defineProperty( - window.navigator, - 'hardwareConcurrency', - { value: windowNavigatorOriginal.hardwareConcurrency, configurable: true } - ); - Object.defineProperty( - window.screen, - 'availWidth', - { value: windowScreenOriginal.availWidth, configurable: true } - ); - Object.defineProperty( - window.screen, - 'availHeight', - { value: windowScreenOriginal.availHeight, configurable: true } - ); + restoreWindow(); }); describe('record()', () => { it('should record a piece of telemetry', async () => { - pouchDb.post = sinon.stub().resolves(); - pouchDb.close = sinon.stub(); - storageGetItemStub - .withArgs('medic-greg-telemetry-db') - .returns('dbname'); - storageGetItemStub - .withArgs('medic-greg-telemetry-date') - .returns(Date.now().toString()); + storageGetItemStub.withArgs('medic-greg-telemetry-db').returns('dbname'); + storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(Date.now().toString()); await service.record('test', 100); @@ -100,19 +118,13 @@ describe('TelemetryService', () => { expect(pouchDb.post.callCount).to.equal(1); expect(pouchDb.post.args[0][0]).to.deep.include({ key: 'test', value: 100 }); expect(pouchDb.post.args[0][0].date_recorded).to.be.above(0); - expect(storageGetItemStub.callCount).to.equal(2); + expect(storageGetItemStub.callCount).to.equal(3); expect(pouchDb.close.callCount).to.equal(1); }); it('should default the value to 1 if not passed', async () => { - pouchDb.post = sinon.stub().resolves(); - pouchDb.close = sinon.stub(); - storageGetItemStub - .withArgs('medic-greg-telemetry-db') - .returns('dbname'); - storageGetItemStub - .withArgs('medic-greg-telemetry-date') - .returns(Date.now().toString()); + storageGetItemStub.withArgs('medic-greg-telemetry-db').returns('dbname'); + storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(Date.now().toString()); await service.record('test'); @@ -121,52 +133,14 @@ describe('TelemetryService', () => { expect(pouchDb.close.callCount).to.equal(1); }); - it('should set localStorage values', async () => { - pouchDb.post = sinon.stub().resolves(); - pouchDb.close = sinon.stub(); - storageGetItemStub - .withArgs('medic-greg-telemetry-db') - .returns(undefined); - storageGetItemStub - .withArgs('medic-greg-telemetry-date') - .returns(undefined); - - await service.record('test', 1); - - expect(consoleErrorSpy.callCount).to.equal(0); - expect(storageSetItemStub.callCount).to.equal(2); - expect(storageSetItemStub.args[0][0]).to.equal('medic-greg-telemetry-db'); - expect(storageSetItemStub.args[0][1]).to.match(/medic-user-greg-telemetry-/); // ends with a UUID - expect(storageSetItemStub.args[1][0]).to.equal('medic-greg-telemetry-date'); - expect(storageSetItemStub.args[1][1]).to.equal(NOW.toString()); - }); - - it('should aggregate once a month and resets the db', async () => { - storageGetItemStub - .withArgs('medic-greg-telemetry-db') - .returns('dbname'); - storageGetItemStub - .withArgs('medic-greg-telemetry-date') - .returns( - moment() - .subtract(5, 'weeks') - .valueOf() - .toString() - ); - - pouchDb.post = sinon.stub().resolves(); + function setupDbMocks() { + storageGetItemStub.returns('dbname'); pouchDb.query = sinon.stub().resolves({ rows: [ - { key: 'foo', value: 'stats' }, - { key: 'bar', value: 'more stats' }, + { key: 'foo', value: {sum:2876, min:581, max:2295, count:2, sumsqr:5604586} }, + { key: 'bar', value: {sum:93, min:43, max:50, count:2, sumsqr:4349} }, ], }); - pouchDb.destroy = sinon.stub().callsFake(() => { - pouchDb._destroyed = true; - return Promise.resolve(); - }); - pouchDb.close = sinon.stub(); - dbInstance.info.resolves({ some: 'stats' }); dbInstance.put.resolves(); dbInstance.get @@ -188,28 +162,28 @@ describe('TelemetryService', () => { } ] }); + } - Object.defineProperty(window.navigator, 'userAgent', { value: 'Agent Smith', configurable: true }); - Object.defineProperty(window.navigator, 'hardwareConcurrency', { value: 4, configurable: true }); - Object.defineProperty(window.screen, 'availWidth', { value: 768, configurable: true }); - Object.defineProperty(window.screen, 'availHeight', { value: 1024, configurable: true }); + it('should aggregate once a day and resets the db first', async () => { + setupDbMocks(); + storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(subtractDays(5)); await service.record('test', 1); - expect(consoleErrorSpy.callCount).to.equal(0); expect(pouchDb.post.callCount).to.equal(1); expect(pouchDb.post.args[0][0]).to.deep.include({ key: 'test', value: 1 }); - expect(dbInstance.put.callCount).to.equal(1); + expect(dbInstance.put.callCount).to.equal(1); const aggregatedDoc = dbInstance.put.args[0][0]; - expect(aggregatedDoc._id).to.match(/telemetry-2018-10-greg/); + expect(aggregatedDoc._id).to.match(/telemetry-2018-11-5-greg/); expect(aggregatedDoc.metrics).to.deep.equal({ - foo: 'stats', - bar: 'more stats', + foo: {sum:2876, min:581, max:2295, count:2, sumsqr:5604586}, + bar: {sum:93, min:43, max:50, count:2, sumsqr:4349}, }); expect(aggregatedDoc.type).to.equal('telemetry'); expect(aggregatedDoc.metadata.year).to.equal(2018); - expect(aggregatedDoc.metadata.month).to.equal(10); + expect(aggregatedDoc.metadata.month).to.equal(11); + expect(aggregatedDoc.metadata.day).to.equal(5); expect(aggregatedDoc.metadata.user).to.equal('greg'); expect(aggregatedDoc.metadata.versions).to.deep.equal({ app: '3.0.0', @@ -227,23 +201,122 @@ describe('TelemetryService', () => { }, deviceInfo: {} }); + expect(dbInstance.query.callCount).to.equal(1); expect(dbInstance.query.args[0][0]).to.equal('medic-client/doc_by_type'); expect(dbInstance.query.args[0][1]).to.deep.equal({ key: ['form'], include_docs: true }); expect(pouchDb.destroy.callCount).to.equal(1); expect(pouchDb.close.callCount).to.equal(0); + + expect(consoleErrorSpy.callCount).to.equal(0); // no errors }); + it('should not aggregate when recording the day the db was created and next day it should aggregate', async () => { + setupDbMocks(); + storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(sameDay()); + + await service.record('test', 10); + + expect(pouchDb.post.callCount).to.equal(1); + expect(pouchDb.post.args[0][0]).to.deep.include({ key: 'test', value: 10 }); + expect(dbInstance.put.callCount).to.equal(0); // NO telemetry has been recorded yet + + clock = sinon.useFakeTimers(moment(NOW).add(1, 'minutes').valueOf()); // 1 min later ... + await service.record('test', 5); + + expect(pouchDb.post.callCount).to.equal(2); // second call + expect(pouchDb.post.args[1][0]).to.deep.include({ key: 'test', value: 5 }); + expect(dbInstance.put.callCount).to.equal(0); // still NO telemetry has been recorded (same day) + + clock = sinon.useFakeTimers(moment(NOW).add(1, 'days').valueOf()); // 1 day later ... + await service.record('test', 2); + + expect(pouchDb.post.callCount).to.equal(3); // third call + expect(pouchDb.post.args[2][0]).to.deep.include({ key: 'test', value: 2 }); + expect(dbInstance.put.callCount).to.equal(1); // Now telemetry has been recorded + + const aggregatedDoc = dbInstance.put.args[0][0]; + expect(aggregatedDoc._id).to.match(/telemetry-2018-11-10-greg/); // Now is 2018-11-11 but aggregation + expect(pouchDb.destroy.callCount).to.equal(1); // is from from previous day + + expect(consoleErrorSpy.callCount).to.equal(0); // no errors + }); + + it('should aggregate from days with records skipping days without records', async () => { + setupDbMocks(); + storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(sameDay()); + + await service.record('datapoint', 12); + + expect(pouchDb.post.callCount).to.equal(1); + expect(dbInstance.put.callCount).to.equal(0); // NO telemetry has been recorded yet + + clock = sinon.useFakeTimers(moment(NOW).add(1, 'minutes').valueOf()); // 1 min later ... + await service.record('another.datapoint'); + + expect(pouchDb.post.callCount).to.equal(2); // second call + expect(dbInstance.put.callCount).to.equal(0); // still NO telemetry has been recorded (same day) + + storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(sameDay()); + clock = sinon.useFakeTimers(moment(NOW).add(2, 'days').valueOf()); // 2 days later ... + await service.record('test', 2); + + expect(pouchDb.post.callCount).to.equal(3); // third call + expect(dbInstance.put.callCount).to.equal(1); // Now telemetry IS recorded + + let aggregatedDoc = dbInstance.put.args[0][0]; + expect(aggregatedDoc._id).to.match(/telemetry-2018-11-10-greg/); // Today is 2018-11-12 but aggregation + expect(pouchDb.destroy.callCount).to.equal(1); // is from from 2 days ago (not Yesterday) + + storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(sameDay()); // same day now is 2 days ahead + + clock = sinon.useFakeTimers(moment(NOW).add(7, 'days').valueOf()); // 7 days later ... + await service.record('point.a', 1); + + expect(pouchDb.post.callCount).to.equal(4); // 4th call + expect(dbInstance.put.callCount).to.equal(2); // Telemetry IS recorded again + aggregatedDoc = dbInstance.put.args[1][0]; + expect(aggregatedDoc._id).to.match(/telemetry-2018-11-12-greg/); // Today is 2018-11-19 but aggregation + // is from 2018-11-12 + + // A new record is added ... + clock = sinon.useFakeTimers(moment(NOW).add(2, 'hours').valueOf()); // ... 2 hours later ... + await service.record('point.b', 0); // 1 record added + // ...the aggregation count is the same because + // the aggregation was already performed 2 hours ago within the same day + expect(pouchDb.post.callCount).to.equal(5); // 5th call + expect(dbInstance.put.callCount).to.equal(2); // Telemetry count is the same + + expect(consoleErrorSpy.callCount).to.equal(0); // no errors + }); + }); + + describe('getDb()', () => { + it('should set localStorage values', async () => { + storageGetItemStub + .withArgs('medic-greg-telemetry-db') + .returns(undefined); + storageGetItemStub + .withArgs('medic-greg-telemetry-date') + .returns(undefined); + + await service.record('test', 1); + + expect(consoleErrorSpy.callCount).to.equal(0); + expect(storageSetItemStub.callCount).to.equal(3); + expect(storageSetItemStub.args[0][0]).to.equal('medic-greg-telemetry-db'); + expect(storageSetItemStub.args[0][1]).to.match(/medic-user-greg-telemetry-/); // ends with a UUID + expect(storageSetItemStub.args[1][0]).to.equal('medic-greg-telemetry-date'); + expect(storageSetItemStub.args[1][1]).to.equal(NOW.toString()); + }); + }); + + describe('storeConflictedAggregate()', () => { + it('should deal with conflicts by making the ID unique and noting the conflict in the new document', async () => { storageGetItemStub.withArgs('medic-greg-telemetry-db').returns('dbname'); - storageGetItemStub.withArgs('medic-greg-telemetry-date').returns( - moment() - .subtract(5, 'weeks') - .valueOf() - .toString() - ); - - pouchDb.post = sinon.stub().resolves(); + storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(subtractDays(5)); + pouchDb.query = sinon.stub().resolves({ rows: [ { @@ -266,16 +339,6 @@ describe('TelemetryService', () => { } }); dbInstance.query.resolves({ rows: [] }); - pouchDb.destroy = sinon.stub().callsFake(() => { - pouchDb._destroyed = true; - return Promise.resolve(); - }); - pouchDb.close = sinon.stub(); - - Object.defineProperty(window.navigator, 'userAgent', { value: 'Agent Smith', configurable: true }); - Object.defineProperty(window.navigator, 'hardwareConcurrency', { value: 4, configurable: true }); - Object.defineProperty(window.screen, 'availWidth', { value: 768, configurable: true }); - Object.defineProperty(window.screen, 'availHeight', { value: 1024, configurable: true }); await service.record('test', 1); @@ -287,5 +350,4 @@ describe('TelemetryService', () => { expect(pouchDb.close.callCount).to.equal(0); }); }); - }); From bb02f44480d3a36c7b59830dede383d6434c83f7 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Wed, 12 May 2021 17:41:45 -0300 Subject: [PATCH 03/17] Improvements in the script to collect meta data from a terminal - Output errors in the standard error stream to see the errors in the console while piping right results to a JSON file - Add Unix exec permission and header to execute the script without the `node ` prefix --- scripts/get_users_meta_docs.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) mode change 100644 => 100755 scripts/get_users_meta_docs.js diff --git a/scripts/get_users_meta_docs.js b/scripts/get_users_meta_docs.js old mode 100644 new mode 100755 index fe7b906cfde..82f01fe4e42 --- a/scripts/get_users_meta_docs.js +++ b/scripts/get_users_meta_docs.js @@ -1,3 +1,5 @@ +#!/usr/bin/env node + const inquirer = require('inquirer'); const PouchDB = require('pouchdb-core'); const fs = require('fs'); @@ -86,7 +88,7 @@ const actionQuestions = [{ } docs.forEach(doc => console.log(JSON.stringify(doc, null, 2) + ',')); } else if (i === 0) { - console.log('\x1b[31m%s\x1b[0m', `There are no documents of type ${type}`); + console.error('\x1b[31m%s\x1b[0m', `There are no documents of type ${type}`); break; } else { console.log('{}]'); @@ -100,7 +102,7 @@ const actionQuestions = [{ let docIndex = 0; if (docs.length === 0) { - console.log('\x1b[31m%s\x1b[0m', `There are no documents of type ${type}`); + console.error('\x1b[31m%s\x1b[0m', `There are no documents of type ${type}`); } else { console.log(JSON.stringify(docs[docIndex], null, 2)); @@ -125,7 +127,7 @@ const actionQuestions = [{ console.log(JSON.stringify(docs[docIndex], null, 2)); if (printMessage) { - console.log('\x1b[31m%s\x1b[0m', `No next document. This is the last one.`); + console.error('\x1b[31m%s\x1b[0m', `No next document. This is the last one.`); } } else if (response.action === 'save_current') { const filePath = path.join(path.resolve(__dirname), docs[docIndex]._id + '.json'); @@ -154,6 +156,6 @@ const actionQuestions = [{ } } } catch(err) { - console.log(err); + console.error(err); } })(); From 2793e5bcd7c3f90ca4abb143c650c93ecd7953b0 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Wed, 12 May 2021 17:47:37 -0300 Subject: [PATCH 04/17] Fix lint error --- webapp/tests/karma/ts/services/telemetry.service.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index 56251790bc0..0c24c2862bb 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -276,8 +276,7 @@ describe('TelemetryService', () => { expect(pouchDb.post.callCount).to.equal(4); // 4th call expect(dbInstance.put.callCount).to.equal(2); // Telemetry IS recorded again aggregatedDoc = dbInstance.put.args[1][0]; - expect(aggregatedDoc._id).to.match(/telemetry-2018-11-12-greg/); // Today is 2018-11-19 but aggregation - // is from 2018-11-12 + expect(aggregatedDoc._id).to.match(/telemetry-2018-11-12-greg/); // It's Nov 19 but aggregation is from Nov 12 // A new record is added ... clock = sinon.useFakeTimers(moment(NOW).add(2, 'hours').valueOf()); // ... 2 hours later ... From d1957d7a1160ca9b34fa4164ba8778d1df913536 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 13 May 2021 15:30:49 -0300 Subject: [PATCH 05/17] Update jsdoc Co-authored-by: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> --- webapp/src/ts/services/telemetry.service.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/webapp/src/ts/services/telemetry.service.ts b/webapp/src/ts/services/telemetry.service.ts index d0dd2bca504..faf6475352d 100644 --- a/webapp/src/ts/services/telemetry.service.ts +++ b/webapp/src/ts/services/telemetry.service.ts @@ -55,8 +55,7 @@ export class TelemetryService { } /** - * Returns the time in milliseconds (since Unix epoch) when the first telemetry - * record was created within the day that is going to be aggregated. + * Returns the time in milliseconds (since Unix epoch) when the first telemetry record was created * * The date is stored locally once computed by this method, either because is * the first time is called or because the aggregation was performed last time From 9e26a4a9d7914c730d5ef9f323024cd022e9a8fa Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 13 May 2021 15:31:35 -0300 Subject: [PATCH 06/17] Update regex Co-authored-by: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> --- webapp/tests/karma/ts/services/telemetry.service.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index 0c24c2862bb..7b8dce226cd 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -304,7 +304,7 @@ describe('TelemetryService', () => { expect(consoleErrorSpy.callCount).to.equal(0); expect(storageSetItemStub.callCount).to.equal(3); expect(storageSetItemStub.args[0][0]).to.equal('medic-greg-telemetry-db'); - expect(storageSetItemStub.args[0][1]).to.match(/medic-user-greg-telemetry-/); // ends with a UUID + expect(storageSetItemStub.args[0][1]).to.match(/^medic-user-greg-telemetry-/) expect(storageSetItemStub.args[1][0]).to.equal('medic-greg-telemetry-date'); expect(storageSetItemStub.args[1][1]).to.equal(NOW.toString()); }); From c0d8e80a7b161501b98b949ef35f0d34e838090c Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 13 May 2021 15:33:46 -0300 Subject: [PATCH 07/17] Update jsdoc Co-authored-by: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> --- webapp/src/ts/services/telemetry.service.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/webapp/src/ts/services/telemetry.service.ts b/webapp/src/ts/services/telemetry.service.ts index faf6475352d..7255d16f45f 100644 --- a/webapp/src/ts/services/telemetry.service.ts +++ b/webapp/src/ts/services/telemetry.service.ts @@ -57,10 +57,7 @@ export class TelemetryService { /** * Returns the time in milliseconds (since Unix epoch) when the first telemetry record was created * - * The date is stored locally once computed by this method, either because is - * the first time is called or because the aggregation was performed last time - * a record was created, therefore `reset(db)' deleted it, and next time this - * method is called the date need to be fetched from the system again. + * This date is computed and stored when we call this method for the first time and after every aggregation. */ private getFirstAggregatedDate() { let date = parseInt(window.localStorage.getItem(this.LAST_AGGREGATED_DATE_KEY)); From 618a5804d3c6cc293ff8299e8dab3e8720a2a622 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 13 May 2021 15:34:40 -0300 Subject: [PATCH 08/17] Update comment Co-authored-by: Diana Barsan <35681649+dianabarsan@users.noreply.github.com> --- webapp/src/ts/services/telemetry.service.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/webapp/src/ts/services/telemetry.service.ts b/webapp/src/ts/services/telemetry.service.ts index 7255d16f45f..09b13045c5d 100644 --- a/webapp/src/ts/services/telemetry.service.ts +++ b/webapp/src/ts/services/telemetry.service.ts @@ -84,8 +84,7 @@ export class TelemetryService { //return moment().startOf('minute'); } - // if there is telemetry data to aggregate and is before the current date, - // aggregation is performed and the data destroyed + // if there is telemetry data from previous days, aggregation is performed and the data destroyed private submitIfNeeded(db) { const startOf = this.aggregateStartsAt(); const dbDate = moment(this.getFirstAggregatedDate()); From c6ad957472406fe2c1f5282e134a36c8a7ad0e62 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 13 May 2021 15:54:52 -0300 Subject: [PATCH 09/17] More precise matches in tests --- .../karma/ts/services/telemetry.service.spec.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index 7b8dce226cd..208a2785ebf 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -175,7 +175,7 @@ describe('TelemetryService', () => { expect(dbInstance.put.callCount).to.equal(1); const aggregatedDoc = dbInstance.put.args[0][0]; - expect(aggregatedDoc._id).to.match(/telemetry-2018-11-5-greg/); + expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-5-greg-[\w-]+$/); expect(aggregatedDoc.metrics).to.deep.equal({ foo: {sum:2876, min:581, max:2295, count:2, sumsqr:5604586}, bar: {sum:93, min:43, max:50, count:2, sumsqr:4349}, @@ -236,8 +236,8 @@ describe('TelemetryService', () => { expect(dbInstance.put.callCount).to.equal(1); // Now telemetry has been recorded const aggregatedDoc = dbInstance.put.args[0][0]; - expect(aggregatedDoc._id).to.match(/telemetry-2018-11-10-greg/); // Now is 2018-11-11 but aggregation - expect(pouchDb.destroy.callCount).to.equal(1); // is from from previous day + expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-10-greg-[\w-]+$/); // Now is 2018-11-11 but aggregation + expect(pouchDb.destroy.callCount).to.equal(1); // is from from previous day expect(consoleErrorSpy.callCount).to.equal(0); // no errors }); @@ -265,8 +265,8 @@ describe('TelemetryService', () => { expect(dbInstance.put.callCount).to.equal(1); // Now telemetry IS recorded let aggregatedDoc = dbInstance.put.args[0][0]; - expect(aggregatedDoc._id).to.match(/telemetry-2018-11-10-greg/); // Today is 2018-11-12 but aggregation - expect(pouchDb.destroy.callCount).to.equal(1); // is from from 2 days ago (not Yesterday) + expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-10-greg-[\w-]+$/); // Today 2018-11-12 but aggregation is + expect(pouchDb.destroy.callCount).to.equal(1); // from from 2 days ago (not Yesterday) storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(sameDay()); // same day now is 2 days ahead @@ -276,7 +276,7 @@ describe('TelemetryService', () => { expect(pouchDb.post.callCount).to.equal(4); // 4th call expect(dbInstance.put.callCount).to.equal(2); // Telemetry IS recorded again aggregatedDoc = dbInstance.put.args[1][0]; - expect(aggregatedDoc._id).to.match(/telemetry-2018-11-12-greg/); // It's Nov 19 but aggregation is from Nov 12 + expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-12-greg-[\w-]+$/); // Now is Nov 19 but agg. is from Nov 12 // A new record is added ... clock = sinon.useFakeTimers(moment(NOW).add(2, 'hours').valueOf()); // ... 2 hours later ... @@ -304,7 +304,7 @@ describe('TelemetryService', () => { expect(consoleErrorSpy.callCount).to.equal(0); expect(storageSetItemStub.callCount).to.equal(3); expect(storageSetItemStub.args[0][0]).to.equal('medic-greg-telemetry-db'); - expect(storageSetItemStub.args[0][1]).to.match(/^medic-user-greg-telemetry-/) + expect(storageSetItemStub.args[0][1]).to.match(/^medic-user-greg-telemetry-[\w-]+$/); expect(storageSetItemStub.args[1][0]).to.equal('medic-greg-telemetry-date'); expect(storageSetItemStub.args[1][1]).to.equal(NOW.toString()); }); @@ -343,7 +343,7 @@ describe('TelemetryService', () => { expect(consoleErrorSpy.callCount).to.equal(0); expect(dbInstance.put.callCount).to.equal(2); - expect(dbInstance.put.args[1][0]._id).to.match(/conflicted/); + expect(dbInstance.put.args[1][0]._id).to.match(/^telemetry-2018-11-5-greg-[\w-]+-conflicted-[\w-]+$/); expect(dbInstance.put.args[1][0].metadata.conflicted).to.equal(true); expect(pouchDb.destroy.callCount).to.equal(1); expect(pouchDb.close.callCount).to.equal(0); From 49c54d59d53316dcd21a8eca9fd1625a39ad0179 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Fri, 14 May 2021 13:07:56 -0300 Subject: [PATCH 10/17] Remove unnecessary check of docs count --- webapp/src/ts/services/telemetry.service.ts | 15 +++------------ .../karma/ts/services/telemetry.service.spec.ts | 1 - 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/webapp/src/ts/services/telemetry.service.ts b/webapp/src/ts/services/telemetry.service.ts index 09b13045c5d..daa2631da41 100644 --- a/webapp/src/ts/services/telemetry.service.ts +++ b/webapp/src/ts/services/telemetry.service.ts @@ -81,7 +81,6 @@ export class TelemetryService { // moment when the aggregation starts (the beginning of the current day) private aggregateStartsAt() { return moment().startOf('day'); - //return moment().startOf('minute'); } // if there is telemetry data from previous days, aggregation is performed and the data destroyed @@ -90,13 +89,9 @@ export class TelemetryService { const dbDate = moment(this.getFirstAggregatedDate()); if (dbDate.isBefore(startOf)) { - return db.info() - .then(info => { - if (info.doc_count > 0) { - return this.aggregate(db) - .then(() => this.reset(db)); - } - }); + return this + .aggregate(db) + .then(() => this.reset(db)); } } @@ -114,8 +109,6 @@ export class TelemetryService { metadata.year, metadata.month, metadata.day, - //metadata.hour, - //metadata.minute, metadata.user, metadata.deviceId, ].join('-'); @@ -140,8 +133,6 @@ export class TelemetryService { year: date.year(), month: date.month() + 1, day: date.date(), - //hour: date.hour(), - //minute: date.minute(), user: this.sessionService.userCtx().name, deviceId: this.getUniqueDeviceId(), versions: { diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index 208a2785ebf..1a7a301690a 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -76,7 +76,6 @@ describe('TelemetryService', () => { dbService = { get: () => dbInstance }; consoleErrorSpy = sinon.spy(console, 'error'); pouchDb = { - info: sinon.stub().resolves({doc_count: 10}), post: sinon.stub().resolves(), close: sinon.stub(), destroy: sinon.stub().callsFake(() => { From 5b5be639a608db66410d5e3245ebb41e1e972fac Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 27 May 2021 11:23:00 -0300 Subject: [PATCH 11/17] Rename constant --- webapp/src/ts/services/telemetry.service.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/webapp/src/ts/services/telemetry.service.ts b/webapp/src/ts/services/telemetry.service.ts index daa2631da41..869486bd71c 100644 --- a/webapp/src/ts/services/telemetry.service.ts +++ b/webapp/src/ts/services/telemetry.service.ts @@ -15,7 +15,7 @@ export class TelemetryService { // Intentionally scoped to the whole browser (for this domain). We can then tell if multiple users use the same device private readonly DEVICE_ID_KEY = 'medic-telemetry-device-id'; private DB_ID_KEY; - private LAST_AGGREGATED_DATE_KEY; + private FIRST_AGGREGATED_DATE_KEY; private queue = Promise.resolve(); @@ -27,7 +27,7 @@ export class TelemetryService { // Intentionally scoped to the specific user, as they may perform a // different role (online vs. offline being being the most obvious) with different performance implications this.DB_ID_KEY = ['medic', this.sessionService.userCtx().name, 'telemetry-db'].join('-'); - this.LAST_AGGREGATED_DATE_KEY = ['medic', this.sessionService.userCtx().name, 'telemetry-date'].join('-'); + this.FIRST_AGGREGATED_DATE_KEY = ['medic', this.sessionService.userCtx().name, 'telemetry-date'].join('-'); } private getDb() { @@ -55,16 +55,16 @@ export class TelemetryService { } /** - * Returns the time in milliseconds (since Unix epoch) when the first telemetry record was created + * Returns the time in milliseconds (since Unix epoch) when the first telemetry record was created. * * This date is computed and stored when we call this method for the first time and after every aggregation. */ private getFirstAggregatedDate() { - let date = parseInt(window.localStorage.getItem(this.LAST_AGGREGATED_DATE_KEY)); + let date = parseInt(window.localStorage.getItem(this.FIRST_AGGREGATED_DATE_KEY)); if (!date) { date = Date.now(); - window.localStorage.setItem(this.LAST_AGGREGATED_DATE_KEY, date.toString()); + window.localStorage.setItem(this.FIRST_AGGREGATED_DATE_KEY, date.toString()); } return date; @@ -217,7 +217,7 @@ export class TelemetryService { private reset(db) { window.localStorage.removeItem(this.DB_ID_KEY); - window.localStorage.removeItem(this.LAST_AGGREGATED_DATE_KEY); + window.localStorage.removeItem(this.FIRST_AGGREGATED_DATE_KEY); return db.destroy(); } From 7cc1a9144627f6fbb59fb378413594f91524e979 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 27 May 2021 11:36:46 -0300 Subject: [PATCH 12/17] getFirstAggregatedDate() returns a Moment object --- webapp/src/ts/services/telemetry.service.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/webapp/src/ts/services/telemetry.service.ts b/webapp/src/ts/services/telemetry.service.ts index 869486bd71c..e6f8f3fafbf 100644 --- a/webapp/src/ts/services/telemetry.service.ts +++ b/webapp/src/ts/services/telemetry.service.ts @@ -55,9 +55,10 @@ export class TelemetryService { } /** - * Returns the time in milliseconds (since Unix epoch) when the first telemetry record was created. + * Returns a Moment object when the first telemetry record was created. * - * This date is computed and stored when we call this method for the first time and after every aggregation. + * This date is computed and stored in milliseconds (since Unix epoch) + * when we call this method for the first time and after every aggregation. */ private getFirstAggregatedDate() { let date = parseInt(window.localStorage.getItem(this.FIRST_AGGREGATED_DATE_KEY)); @@ -67,7 +68,7 @@ export class TelemetryService { window.localStorage.setItem(this.FIRST_AGGREGATED_DATE_KEY, date.toString()); } - return date; + return moment(date); } private storeIt(db, key, value) { @@ -86,7 +87,7 @@ export class TelemetryService { // if there is telemetry data from previous days, aggregation is performed and the data destroyed private submitIfNeeded(db) { const startOf = this.aggregateStartsAt(); - const dbDate = moment(this.getFirstAggregatedDate()); + const dbDate = this.getFirstAggregatedDate(); if (dbDate.isBefore(startOf)) { return this @@ -121,7 +122,7 @@ export class TelemetryService { this.dbService.get().query('medic-client/doc_by_type', { key: ['form'], include_docs: true }) ]) .then(([ddoc, formResults]) => { - const date = moment(this.getFirstAggregatedDate()); + const date = this.getFirstAggregatedDate(); const version = (ddoc.deploy_info && ddoc.deploy_info.version) || 'unknown'; const forms = formResults.rows.reduce((keyToVersion, row) => { keyToVersion[row.doc.internalId] = row.doc._rev; From ca116c852d17299931bc3b8be02810c3bcaab718 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 27 May 2021 11:52:25 -0300 Subject: [PATCH 13/17] Indent fixes --- .../ts/services/telemetry.service.spec.ts | 56 +++++++++++++------ 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index 1a7a301690a..4b178ae6dc6 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -31,25 +31,49 @@ describe('TelemetryService', () => { }; function defineWindow() { - Object.defineProperty(window.navigator, 'userAgent', - { value: 'Agent Smith', configurable: true }); - Object.defineProperty(window.navigator, 'hardwareConcurrency', - { value: 4, configurable: true }); - Object.defineProperty(window.screen, 'availWidth', - { value: 768, configurable: true }); - Object.defineProperty(window.screen, 'availHeight', - { value: 1024, configurable: true }); + Object.defineProperty( + window.navigator, + 'userAgent', + { value: 'Agent Smith', configurable: true } + ); + Object.defineProperty( + window.navigator, + 'hardwareConcurrency', + { value: 4, configurable: true } + ); + Object.defineProperty( + window.screen, + 'availWidth', + { value: 768, configurable: true } + ); + Object.defineProperty( + window.screen, + 'availHeight', + { value: 1024, configurable: true } + ); } function restoreWindow() { - Object.defineProperty(window.navigator, 'userAgent', - { value: windowNavigatorOriginal.userAgent, configurable: true }); - Object.defineProperty(window.navigator, 'hardwareConcurrency', - { value: windowNavigatorOriginal.hardwareConcurrency, configurable: true }); - Object.defineProperty(window.screen, 'availWidth', - { value: windowScreenOriginal.availWidth, configurable: true }); - Object.defineProperty(window.screen, 'availHeight', - { value: windowScreenOriginal.availHeight, configurable: true }); + Object.defineProperty( + window.navigator, + 'userAgent', + { value: windowNavigatorOriginal.userAgent, configurable: true } + ); + Object.defineProperty( + window.navigator, + 'hardwareConcurrency', + { value: windowNavigatorOriginal.hardwareConcurrency, configurable: true } + ); + Object.defineProperty( + window.screen, + 'availWidth', + { value: windowScreenOriginal.availWidth, configurable: true } + ); + Object.defineProperty( + window.screen, + 'availHeight', + { value: windowScreenOriginal.availHeight, configurable: true } + ); } function subtractDays(numDays) { From ad7cfd0c8e88329a5add49a3f01cc63cee01589f Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 27 May 2021 16:32:30 -0300 Subject: [PATCH 14/17] Checks args --- .../karma/ts/services/telemetry.service.spec.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index 4b178ae6dc6..c0e44284070 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -142,6 +142,9 @@ describe('TelemetryService', () => { expect(pouchDb.post.args[0][0]).to.deep.include({ key: 'test', value: 100 }); expect(pouchDb.post.args[0][0].date_recorded).to.be.above(0); expect(storageGetItemStub.callCount).to.equal(3); + expect(storageGetItemStub.args[0]).to.deep.equal(['medic-greg-telemetry-db']); + expect(storageGetItemStub.args[1]).to.deep.equal(["medic-greg-telemetry-date"]); + expect(storageGetItemStub.args[2]).to.deep.equal(['medic-greg-telemetry-db']); expect(pouchDb.close.callCount).to.equal(1); }); @@ -242,25 +245,25 @@ describe('TelemetryService', () => { expect(pouchDb.post.callCount).to.equal(1); expect(pouchDb.post.args[0][0]).to.deep.include({ key: 'test', value: 10 }); - expect(dbInstance.put.callCount).to.equal(0); // NO telemetry has been recorded yet + expect(dbInstance.put.callCount).to.equal(0); // NO telemetry aggregation has been recorded yet clock = sinon.useFakeTimers(moment(NOW).add(1, 'minutes').valueOf()); // 1 min later ... await service.record('test', 5); - expect(pouchDb.post.callCount).to.equal(2); // second call + expect(pouchDb.post.callCount).to.equal(2); // second call expect(pouchDb.post.args[1][0]).to.deep.include({ key: 'test', value: 5 }); - expect(dbInstance.put.callCount).to.equal(0); // still NO telemetry has been recorded (same day) + expect(dbInstance.put.callCount).to.equal(0); // still NO aggregation has been recorded (same day) clock = sinon.useFakeTimers(moment(NOW).add(1, 'days').valueOf()); // 1 day later ... await service.record('test', 2); - expect(pouchDb.post.callCount).to.equal(3); // third call + expect(pouchDb.post.callCount).to.equal(3); // third call expect(pouchDb.post.args[2][0]).to.deep.include({ key: 'test', value: 2 }); expect(dbInstance.put.callCount).to.equal(1); // Now telemetry has been recorded const aggregatedDoc = dbInstance.put.args[0][0]; expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-10-greg-[\w-]+$/); // Now is 2018-11-11 but aggregation - expect(pouchDb.destroy.callCount).to.equal(1); // is from from previous day + expect(pouchDb.destroy.callCount).to.equal(1); // is from the previous day expect(consoleErrorSpy.callCount).to.equal(0); // no errors }); From 86b09d05a7a58d7e44fb85883f0e964f483bd9d5 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 27 May 2021 17:04:55 -0300 Subject: [PATCH 15/17] Rename db names in tests --- .../ts/services/telemetry.service.spec.ts | 137 +++++++++--------- 1 file changed, 69 insertions(+), 68 deletions(-) diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index c0e44284070..4ff6c692935 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -11,10 +11,10 @@ describe('TelemetryService', () => { const NOW = new Date(2018, 10, 10, 12, 33).getTime(); // -> 2018-11-10T12:33:00 let service: TelemetryService; let dbService; - let dbInstance; + let metaDb; let sessionService; let clock; - let pouchDb; + let telemetryDb; let storageGetItemStub; let storageSetItemStub; let consoleErrorSpy; @@ -91,19 +91,20 @@ describe('TelemetryService', () => { beforeEach(() => { defineWindow(); - dbInstance = { + metaDb = { info: sinon.stub(), put: sinon.stub(), get: sinon.stub(), query: sinon.stub() }; - dbService = { get: () => dbInstance }; + dbService = { get: () => metaDb }; consoleErrorSpy = sinon.spy(console, 'error'); - pouchDb = { + telemetryDb = { post: sinon.stub().resolves(), close: sinon.stub(), + query: sinon.stub(), destroy: sinon.stub().callsFake(() => { - pouchDb._destroyed = true; + telemetryDb._destroyed = true; return Promise.resolve(); }) }; @@ -120,7 +121,7 @@ describe('TelemetryService', () => { service = TestBed.inject(TelemetryService); clock = sinon.useFakeTimers(NOW); - window.PouchDB = () => pouchDb; + window.PouchDB = () => telemetryDb; }); afterEach(() => { @@ -138,14 +139,14 @@ describe('TelemetryService', () => { await service.record('test', 100); expect(consoleErrorSpy.callCount).to.equal(0); - expect(pouchDb.post.callCount).to.equal(1); - expect(pouchDb.post.args[0][0]).to.deep.include({ key: 'test', value: 100 }); - expect(pouchDb.post.args[0][0].date_recorded).to.be.above(0); + expect(telemetryDb.post.callCount).to.equal(1); + expect(telemetryDb.post.args[0][0]).to.deep.include({ key: 'test', value: 100 }); + expect(telemetryDb.post.args[0][0].date_recorded).to.be.above(0); expect(storageGetItemStub.callCount).to.equal(3); expect(storageGetItemStub.args[0]).to.deep.equal(['medic-greg-telemetry-db']); expect(storageGetItemStub.args[1]).to.deep.equal(["medic-greg-telemetry-date"]); expect(storageGetItemStub.args[2]).to.deep.equal(['medic-greg-telemetry-db']); - expect(pouchDb.close.callCount).to.equal(1); + expect(telemetryDb.close.callCount).to.equal(1); }); it('should default the value to 1 if not passed', async () => { @@ -155,27 +156,27 @@ describe('TelemetryService', () => { await service.record('test'); expect(consoleErrorSpy.callCount).to.equal(0); - expect(pouchDb.post.args[0][0].value).to.equal(1); - expect(pouchDb.close.callCount).to.equal(1); + expect(telemetryDb.post.args[0][0].value).to.equal(1); + expect(telemetryDb.close.callCount).to.equal(1); }); function setupDbMocks() { storageGetItemStub.returns('dbname'); - pouchDb.query = sinon.stub().resolves({ + telemetryDb.query.resolves({ rows: [ { key: 'foo', value: {sum:2876, min:581, max:2295, count:2, sumsqr:5604586} }, { key: 'bar', value: {sum:93, min:43, max:50, count:2, sumsqr:4349} }, ], }); - dbInstance.info.resolves({ some: 'stats' }); - dbInstance.put.resolves(); - dbInstance.get + metaDb.info.resolves({ some: 'stats' }); + metaDb.put.resolves(); + metaDb.get .withArgs('_design/medic-client') .resolves({ _id: '_design/medic-client', deploy_info: { version: '3.0.0' } }); - dbInstance.query.resolves({ + metaDb.query.resolves({ rows: [ { id: 'form:anc_followup', @@ -196,11 +197,11 @@ describe('TelemetryService', () => { await service.record('test', 1); - expect(pouchDb.post.callCount).to.equal(1); - expect(pouchDb.post.args[0][0]).to.deep.include({ key: 'test', value: 1 }); + expect(telemetryDb.post.callCount).to.equal(1); + expect(telemetryDb.post.args[0][0]).to.deep.include({ key: 'test', value: 1 }); - expect(dbInstance.put.callCount).to.equal(1); - const aggregatedDoc = dbInstance.put.args[0][0]; + expect(metaDb.put.callCount).to.equal(1); + const aggregatedDoc = metaDb.put.args[0][0]; expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-5-greg-[\w-]+$/); expect(aggregatedDoc.metrics).to.deep.equal({ foo: {sum:2876, min:581, max:2295, count:2, sumsqr:5604586}, @@ -228,11 +229,11 @@ describe('TelemetryService', () => { deviceInfo: {} }); - expect(dbInstance.query.callCount).to.equal(1); - expect(dbInstance.query.args[0][0]).to.equal('medic-client/doc_by_type'); - expect(dbInstance.query.args[0][1]).to.deep.equal({ key: ['form'], include_docs: true }); - expect(pouchDb.destroy.callCount).to.equal(1); - expect(pouchDb.close.callCount).to.equal(0); + expect(metaDb.query.callCount).to.equal(1); + expect(metaDb.query.args[0][0]).to.equal('medic-client/doc_by_type'); + expect(metaDb.query.args[0][1]).to.deep.equal({ key: ['form'], include_docs: true }); + expect(telemetryDb.destroy.callCount).to.equal(1); + expect(telemetryDb.close.callCount).to.equal(0); expect(consoleErrorSpy.callCount).to.equal(0); // no errors }); @@ -243,29 +244,29 @@ describe('TelemetryService', () => { await service.record('test', 10); - expect(pouchDb.post.callCount).to.equal(1); - expect(pouchDb.post.args[0][0]).to.deep.include({ key: 'test', value: 10 }); - expect(dbInstance.put.callCount).to.equal(0); // NO telemetry aggregation has been recorded yet + expect(telemetryDb.post.callCount).to.equal(1); + expect(telemetryDb.post.args[0][0]).to.deep.include({ key: 'test', value: 10 }); + expect(metaDb.put.callCount).to.equal(0); // NO telemetry aggregation has been recorded yet clock = sinon.useFakeTimers(moment(NOW).add(1, 'minutes').valueOf()); // 1 min later ... await service.record('test', 5); - expect(pouchDb.post.callCount).to.equal(2); // second call - expect(pouchDb.post.args[1][0]).to.deep.include({ key: 'test', value: 5 }); - expect(dbInstance.put.callCount).to.equal(0); // still NO aggregation has been recorded (same day) + expect(telemetryDb.post.callCount).to.equal(2); // second call + expect(telemetryDb.post.args[1][0]).to.deep.include({ key: 'test', value: 5 }); + expect(metaDb.put.callCount).to.equal(0); // still NO aggregation has been recorded (same day) clock = sinon.useFakeTimers(moment(NOW).add(1, 'days').valueOf()); // 1 day later ... await service.record('test', 2); - expect(pouchDb.post.callCount).to.equal(3); // third call - expect(pouchDb.post.args[2][0]).to.deep.include({ key: 'test', value: 2 }); - expect(dbInstance.put.callCount).to.equal(1); // Now telemetry has been recorded + expect(telemetryDb.post.callCount).to.equal(3); // third call + expect(telemetryDb.post.args[2][0]).to.deep.include({ key: 'test', value: 2 }); + expect(metaDb.put.callCount).to.equal(1); // Now telemetry has been recorded - const aggregatedDoc = dbInstance.put.args[0][0]; - expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-10-greg-[\w-]+$/); // Now is 2018-11-11 but aggregation - expect(pouchDb.destroy.callCount).to.equal(1); // is from the previous day + const aggregatedDoc = metaDb.put.args[0][0]; + expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-10-greg-[\w-]+$/); // Now is 2018-11-11 but aggregation + expect(telemetryDb.destroy.callCount).to.equal(1); // is from the previous day - expect(consoleErrorSpy.callCount).to.equal(0); // no errors + expect(consoleErrorSpy.callCount).to.equal(0); // no errors }); it('should aggregate from days with records skipping days without records', async () => { @@ -274,34 +275,34 @@ describe('TelemetryService', () => { await service.record('datapoint', 12); - expect(pouchDb.post.callCount).to.equal(1); - expect(dbInstance.put.callCount).to.equal(0); // NO telemetry has been recorded yet + expect(telemetryDb.post.callCount).to.equal(1); + expect(metaDb.put.callCount).to.equal(0); // NO telemetry has been recorded yet clock = sinon.useFakeTimers(moment(NOW).add(1, 'minutes').valueOf()); // 1 min later ... await service.record('another.datapoint'); - expect(pouchDb.post.callCount).to.equal(2); // second call - expect(dbInstance.put.callCount).to.equal(0); // still NO telemetry has been recorded (same day) + expect(telemetryDb.post.callCount).to.equal(2); // second call + expect(metaDb.put.callCount).to.equal(0); // still NO telemetry has been recorded (same day) storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(sameDay()); clock = sinon.useFakeTimers(moment(NOW).add(2, 'days').valueOf()); // 2 days later ... await service.record('test', 2); - expect(pouchDb.post.callCount).to.equal(3); // third call - expect(dbInstance.put.callCount).to.equal(1); // Now telemetry IS recorded + expect(telemetryDb.post.callCount).to.equal(3); // third call + expect(metaDb.put.callCount).to.equal(1); // Now telemetry IS recorded - let aggregatedDoc = dbInstance.put.args[0][0]; - expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-10-greg-[\w-]+$/); // Today 2018-11-12 but aggregation is - expect(pouchDb.destroy.callCount).to.equal(1); // from from 2 days ago (not Yesterday) + let aggregatedDoc = metaDb.put.args[0][0]; + expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-10-greg-[\w-]+$/); // Today 2018-11-12 but aggregation is + expect(telemetryDb.destroy.callCount).to.equal(1); // from from 2 days ago (not Yesterday) - storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(sameDay()); // same day now is 2 days ahead + storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(sameDay()); // same day now is 2 days ahead - clock = sinon.useFakeTimers(moment(NOW).add(7, 'days').valueOf()); // 7 days later ... + clock = sinon.useFakeTimers(moment(NOW).add(7, 'days').valueOf()); // 7 days later ... await service.record('point.a', 1); - expect(pouchDb.post.callCount).to.equal(4); // 4th call - expect(dbInstance.put.callCount).to.equal(2); // Telemetry IS recorded again - aggregatedDoc = dbInstance.put.args[1][0]; + expect(telemetryDb.post.callCount).to.equal(4); // 4th call + expect(metaDb.put.callCount).to.equal(2); // Telemetry IS recorded again + aggregatedDoc = metaDb.put.args[1][0]; expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-12-greg-[\w-]+$/); // Now is Nov 19 but agg. is from Nov 12 // A new record is added ... @@ -309,10 +310,10 @@ describe('TelemetryService', () => { await service.record('point.b', 0); // 1 record added // ...the aggregation count is the same because // the aggregation was already performed 2 hours ago within the same day - expect(pouchDb.post.callCount).to.equal(5); // 5th call - expect(dbInstance.put.callCount).to.equal(2); // Telemetry count is the same + expect(telemetryDb.post.callCount).to.equal(5); // 5th call + expect(metaDb.put.callCount).to.equal(2); // Telemetry count is the same - expect(consoleErrorSpy.callCount).to.equal(0); // no errors + expect(consoleErrorSpy.callCount).to.equal(0); // no errors }); }); @@ -342,7 +343,7 @@ describe('TelemetryService', () => { storageGetItemStub.withArgs('medic-greg-telemetry-db').returns('dbname'); storageGetItemStub.withArgs('medic-greg-telemetry-date').returns(subtractDays(5)); - pouchDb.query = sinon.stub().resolves({ + telemetryDb.query = sinon.stub().resolves({ rows: [ { key: 'foo', @@ -354,25 +355,25 @@ describe('TelemetryService', () => { }, ], }); - dbInstance.info.resolves({ some: 'stats' }); - dbInstance.put.onFirstCall().rejects({ status: 409 }); - dbInstance.put.onSecondCall().resolves(); - dbInstance.get.withArgs('_design/medic-client').resolves({ + metaDb.info.resolves({ some: 'stats' }); + metaDb.put.onFirstCall().rejects({ status: 409 }); + metaDb.put.onSecondCall().resolves(); + metaDb.get.withArgs('_design/medic-client').resolves({ _id: '_design/medic-client', deploy_info: { version: '3.0.0' } }); - dbInstance.query.resolves({ rows: [] }); + metaDb.query.resolves({ rows: [] }); await service.record('test', 1); expect(consoleErrorSpy.callCount).to.equal(0); - expect(dbInstance.put.callCount).to.equal(2); - expect(dbInstance.put.args[1][0]._id).to.match(/^telemetry-2018-11-5-greg-[\w-]+-conflicted-[\w-]+$/); - expect(dbInstance.put.args[1][0].metadata.conflicted).to.equal(true); - expect(pouchDb.destroy.callCount).to.equal(1); - expect(pouchDb.close.callCount).to.equal(0); + expect(metaDb.put.callCount).to.equal(2); + expect(metaDb.put.args[1][0]._id).to.match(/^telemetry-2018-11-5-greg-[\w-]+-conflicted-[\w-]+$/); + expect(metaDb.put.args[1][0].metadata.conflicted).to.equal(true); + expect(telemetryDb.destroy.callCount).to.equal(1); + expect(telemetryDb.close.callCount).to.equal(0); }); }); }); From 9bb8ac5b1092f2d5065e802cf657c8f45ac99a1b Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 27 May 2021 18:32:58 -0300 Subject: [PATCH 16/17] Test that aggregation is performed before the recording of the new telemetry entry --- .../karma/ts/services/telemetry.service.spec.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index 4ff6c692935..27045390239 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -244,23 +244,32 @@ describe('TelemetryService', () => { await service.record('test', 10); - expect(telemetryDb.post.callCount).to.equal(1); + expect(telemetryDb.post.callCount).to.equal(1); // Telemetry entry has been recorded expect(telemetryDb.post.args[0][0]).to.deep.include({ key: 'test', value: 10 }); - expect(metaDb.put.callCount).to.equal(0); // NO telemetry aggregation has been recorded yet + expect(telemetryDb.query.called).to.be.false; // NO telemetry aggregation has + expect(metaDb.put.callCount).to.equal(0); // been recorded yet clock = sinon.useFakeTimers(moment(NOW).add(1, 'minutes').valueOf()); // 1 min later ... await service.record('test', 5); expect(telemetryDb.post.callCount).to.equal(2); // second call expect(telemetryDb.post.args[1][0]).to.deep.include({ key: 'test', value: 5 }); - expect(metaDb.put.callCount).to.equal(0); // still NO aggregation has been recorded (same day) + expect(telemetryDb.query.called).to.be.false; // still NO aggregation has + expect(metaDb.put.callCount).to.equal(0); // been recorded (same day) + let postCalledAfterQuery = false; + telemetryDb.post.callsFake(async () => postCalledAfterQuery = telemetryDb.query.called); clock = sinon.useFakeTimers(moment(NOW).add(1, 'days').valueOf()); // 1 day later ... await service.record('test', 2); expect(telemetryDb.post.callCount).to.equal(3); // third call expect(telemetryDb.post.args[2][0]).to.deep.include({ key: 'test', value: 2 }); - expect(metaDb.put.callCount).to.equal(1); // Now telemetry has been recorded + expect(telemetryDb.query.callCount).to.equal(1); // Now aggregation HAS been performed + expect(metaDb.put.callCount).to.equal(1); // and the stats recorded + + // The telemetry record has been recorded after aggregation to not being included in the stats, + // because the record belong to the current date, not the day aggregated (yesterday) + expect(postCalledAfterQuery).to.be.true; const aggregatedDoc = metaDb.put.args[0][0]; expect(aggregatedDoc._id).to.match(/^telemetry-2018-11-10-greg-[\w-]+$/); // Now is 2018-11-11 but aggregation From 378fea5157c16cfe3ceb20bf303c15a0c210b9f9 Mon Sep 17 00:00:00 2001 From: Mariano Ruiz Date: Thu, 27 May 2021 18:41:03 -0300 Subject: [PATCH 17/17] Fix quotes in string --- webapp/tests/karma/ts/services/telemetry.service.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/tests/karma/ts/services/telemetry.service.spec.ts b/webapp/tests/karma/ts/services/telemetry.service.spec.ts index 27045390239..c6898aff9d3 100644 --- a/webapp/tests/karma/ts/services/telemetry.service.spec.ts +++ b/webapp/tests/karma/ts/services/telemetry.service.spec.ts @@ -144,7 +144,7 @@ describe('TelemetryService', () => { expect(telemetryDb.post.args[0][0].date_recorded).to.be.above(0); expect(storageGetItemStub.callCount).to.equal(3); expect(storageGetItemStub.args[0]).to.deep.equal(['medic-greg-telemetry-db']); - expect(storageGetItemStub.args[1]).to.deep.equal(["medic-greg-telemetry-date"]); + expect(storageGetItemStub.args[1]).to.deep.equal(['medic-greg-telemetry-date']); expect(storageGetItemStub.args[2]).to.deep.equal(['medic-greg-telemetry-db']); expect(telemetryDb.close.callCount).to.equal(1); });