From 4fc70d1467e363f7d3e6459cb6cf7f393d5e5a7f Mon Sep 17 00:00:00 2001 From: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com> Date: Mon, 17 Oct 2022 09:48:32 +0530 Subject: [PATCH] Address comments from Discord Co-authored-by: Dave Kelsey Signed-off-by: CaptainIRS <36656347+CaptainIRS@users.noreply.github.com> --- .../prometheus-manager-tx-observer.js | 29 ++++++-- .../prometheus-manager-tx-observer.js | 73 +++++++++++-------- 2 files changed, 62 insertions(+), 40 deletions(-) diff --git a/packages/caliper-core/lib/worker/tx-observers/prometheus-manager-tx-observer.js b/packages/caliper-core/lib/worker/tx-observers/prometheus-manager-tx-observer.js index 2fea7f3bd..fde97e903 100644 --- a/packages/caliper-core/lib/worker/tx-observers/prometheus-manager-tx-observer.js +++ b/packages/caliper-core/lib/worker/tx-observers/prometheus-manager-tx-observer.js @@ -36,29 +36,42 @@ class PrometheusManagerTxObserver extends TxObserverInterface { super(messenger, workerIndex); this.method = (options && options.method) ? options.method : ConfigUtil.get(ConfigUtil.keys.Observer.PrometheusManager.Method); - if (this.method === 'periodic') { + + switch (this.method) { + case 'periodic': { this.updateInterval = (options && options.interval) ? options.interval : ConfigUtil.get(ConfigUtil.keys.Observer.PrometheusManager.Interval); this.intervalObject = undefined; if (this.updateInterval <= 0) { - Logger.error('Invalid update interval specified, using default value'); this.updateInterval = ConfigUtil.get(ConfigUtil.keys.Observer.PrometheusManager.Interval); + Logger.warn(`Invalid update interval specified, using default value of ${this.updateInterval}`); } if (options && options.collationCount) { Logger.warn('Collation count is ignored when using periodic method'); } - } else if (this.method === 'collate') { + break; + } + + case 'collate' : { this.collationCount = (options && options.collationCount) ? options.collationCount : ConfigUtil.get(ConfigUtil.keys.Observer.PrometheusManager.CollationCount); if (this.collationCount <= 0) { - Logger.error('Invalid collation count specified, using default value'); this.collationCount = ConfigUtil.get(ConfigUtil.keys.Observer.PrometheusManager.CollationCount); + Logger.warn(`Invalid collation count specified, using default value of ${this.collationCount}`); } if (options && options.interval) { Logger.warn('Update interval is ignored when using collate method'); } + break; } - this.pendingMessages = []; + default: { + const msg = `Unrecognised method '${this.method}' specified for prometheus manager, must be either 'collate' or 'periodic' `; + Logger.error(msg); + throw new Error(msg); + } + + } + this.pendingMessages = []; this.managerUuid = managerUuid; } /** @@ -117,7 +130,7 @@ class PrometheusManagerTxObserver extends TxObserverInterface { this.pendingMessages.push(message); if (this.method === 'collate' && this.pendingMessages.length === this.collationCount) { - await this._sendUpdate(); + this._sendUpdate(); } } @@ -125,7 +138,7 @@ class PrometheusManagerTxObserver extends TxObserverInterface { * Sends the current aggregated statistics to the manager node when triggered by "setInterval". * @private */ - async _sendUpdate() { + _sendUpdate() { for (const message of this.pendingMessages) { this.messenger.send(message); } @@ -141,7 +154,7 @@ class PrometheusManagerTxObserver extends TxObserverInterface { await super.activate(roundIndex, roundLabel); if (this.method === 'periodic') { - this.intervalObject = setInterval(async () => { await this._sendUpdate(); }, this.updateInterval); + this.intervalObject = setInterval(async () => { this._sendUpdate(); }, this.updateInterval); } } diff --git a/packages/caliper-core/test/worker/tx-observers/prometheus-manager-tx-observer.js b/packages/caliper-core/test/worker/tx-observers/prometheus-manager-tx-observer.js index 8645b3d2d..14efd586c 100644 --- a/packages/caliper-core/test/worker/tx-observers/prometheus-manager-tx-observer.js +++ b/packages/caliper-core/test/worker/tx-observers/prometheus-manager-tx-observer.js @@ -97,77 +97,86 @@ describe('When using a PrometheusManagerTxObserver', () => { it('should set managerUuid passed through constructor', () => { const observer = new PrometheusManagerTxObserver.createTxObserver(undefined, undefined, undefined, 'fakeUuid'); - observer.managerUuid.should.equal('fakeUuid'); + expect(observer.managerUuid).to.equal('fakeUuid'); }); - it ('should set the correct parameters when method is periodic', () => { + it('should set the correct parameters when method is periodic', () => { const options = { method: 'periodic', interval: 1000, }; const observer = new PrometheusManagerTxObserver.createTxObserver(options, undefined, undefined, 'fakeUuid'); - observer.method.should.equal('periodic'); + expect(observer.method).to.equal('periodic'); expect(observer.updateInterval).to.equal(1000); expect(observer.intervalObject).to.equal(undefined); }); - it ('should set the correct parameters when method is collate', () => { + it('should set the correct parameters when method is collate', () => { const options = { method: 'collate', collationCount: 10, }; const observer = new PrometheusManagerTxObserver.createTxObserver(options, undefined, undefined, 'fakeUuid'); - observer.method.should.equal('collate'); + expect(observer.method).to.equal('collate'); expect(observer.collationCount).to.equal(10); }); - it ('should set the default method when options are not provided', () => { + it('should set the default method when options are not provided', () => { const observer = new PrometheusManagerTxObserver.createTxObserver(undefined, undefined, undefined, 'fakeUuid'); - observer.method.should.equal('periodic'); + expect(observer.method).to.equal('periodic'); expect(observer.updateInterval).to.equal(1000); expect(observer.intervalObject).to.equal(undefined); }); - it ('should use default update interval and print error log when method is periodic and interval is invalid', () => { + it('should throw an error if an unknown method is specified', () => { + const options = { + method: 'profjgd' + }; + expect(() => { + new PrometheusManagerTxObserver.createTxObserver(options, undefined, undefined, 'fakeUuid'); + }).to.throw(/Unrecognised method 'profjgd' specified for prometheus manager, must be either 'collate' or 'periodic'/); + }); + + it('should use default update interval and print warning when method is periodic and interval is invalid', () => { const options = { method: 'periodic', interval: -1, }; const observer = new PrometheusManagerTxObserver.createTxObserver(options, undefined, undefined, 'fakeUuid'); - observer.method.should.equal('periodic'); + expect(observer.method).to.equal('periodic'); expect(observer.updateInterval).to.equal(1000); expect(observer.intervalObject).to.equal(undefined); - sinon.assert.calledOnce(errorLogger); + sinon.assert.calledOnce(warnLogger); }); - it ('should warn when collationCount is specified but method is periodic', () => { + it('should warn when collationCount is specified but method is periodic', () => { const options = { method: 'periodic', collationCount: 10, }; const observer = new PrometheusManagerTxObserver.createTxObserver(options, undefined, undefined, 'fakeUuid'); - observer.method.should.equal('periodic'); + expect(observer.method).to.equal('periodic'); sinon.assert.calledOnce(warnLogger); }); - it ('should use default collationCount and print error log when method is collate and collationCount is invalid', () => { + it('should use default collationCount and print warning when method is collate and collationCount is invalid', () => { const options = { method: 'collate', collationCount: -1, }; const observer = new PrometheusManagerTxObserver.createTxObserver(options, undefined, undefined, 'fakeUuid'); - observer.method.should.equal('collate'); + expect(observer.method).to.equal('collate'); expect(observer.collationCount).to.equal(10); - sinon.assert.calledOnce(errorLogger); + sinon.assert.calledOnce(warnLogger); }); - it ('should warn when interval is specified but method is collate', () => { + it('should warn when interval is specified but method is collate', () => { const options = { method: 'collate', interval: 1000, }; const observer = new PrometheusManagerTxObserver.createTxObserver(options, undefined, undefined, 'fakeUuid'); - observer.method.should.equal('collate'); + expect(observer.method).to.equal('collate'); sinon.assert.calledOnce(warnLogger); }); @@ -191,7 +200,7 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.txSubmitted(txCount); - observer.pendingMessages.should.have.lengthOf(1); + expect(observer.pendingMessages).to.have.lengthOf(1); }); it('should update the pending messages array when single TX is finished', async () => { @@ -220,7 +229,7 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.txFinished(result); - observer.pendingMessages.should.have.lengthOf(1); + expect(observer.pendingMessages).to.have.lengthOf(1); }); it('should update the pending messages array when multiple TXs are finished', async () => { @@ -249,7 +258,7 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.txFinished([result, result]); - observer.pendingMessages.should.have.lengthOf(2); + expect(observer.pendingMessages).to.have.lengthOf(2); }); it('should trigger update when collationCount is crossed with the collate method', async () => { @@ -285,7 +294,7 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.txFinished([result, result]); - observer._sendUpdate.should.have.been.calledOnce; + expect(observer._sendUpdate).to.have.been.calledOnce; }); it('should not trigger update until collation count is reached with method collate', async () => { @@ -321,7 +330,7 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.txFinished(result); - observer._sendUpdate.should.not.have.been.called; + expect(observer._sendUpdate).to.not.have.been.called; }); it('should send pending messages when collation count is reached with method collate', async () => { @@ -355,7 +364,7 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.txFinished([result, result]); - messenger.send.should.have.been.calledTwice; + expect(messenger.send).to.have.been.calledTwice; }); it('should clear pending messages when collation count is reached with method collate', async () => { @@ -389,7 +398,7 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.txFinished([result, result]); - observer.pendingMessages.should.have.lengthOf(0); + expect(observer.pendingMessages).to.have.lengthOf(0); }); it('should setup interval timer with method periodic', async () => { @@ -454,11 +463,11 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.activate(roundIndex, roundLabel); await observer.txFinished(result); - observer._sendUpdate.should.not.have.been.called; + expect(observer._sendUpdate).to.not.have.been.called; clock.tick(1000); - observer._sendUpdate.should.have.been.calledOnce; + expect(observer._sendUpdate).to.have.been.calledOnce; clock.restore(); }); @@ -497,11 +506,11 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.activate(roundIndex, roundLabel); await observer.txFinished(result); - messenger.send.should.not.have.been.called; + expect(messenger.send).to.not.have.been.called; clock.tick(1000); - messenger.send.should.have.been.calledOnce; + expect(messenger.send).to.have.been.calledOnce; clock.restore(); }); @@ -540,11 +549,11 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.activate(roundIndex, roundLabel); await observer.txFinished(result); - observer.pendingMessages.should.have.lengthOf(1); + expect(observer.pendingMessages).to.have.lengthOf(1); clock.tick(1000); - observer.pendingMessages.should.have.lengthOf(0); + expect(observer.pendingMessages).to.have.lengthOf(0); clock.restore(); }); @@ -614,11 +623,11 @@ describe('When using a PrometheusManagerTxObserver', () => { await observer.activate(roundIndex, roundLabel); await observer.txFinished(result); - messenger.send.should.not.have.been.called; + expect(messenger.send).to.not.have.been.called; await observer.deactivate(); - messenger.send.should.have.been.calledOnce; + expect(messenger.send).to.have.been.calledOnce; clock.restore(); });