Skip to content

Commit

Permalink
Address comments from Discord
Browse files Browse the repository at this point in the history 
Co-authored-by: Dave Kelsey <[email protected]>
Signed-off-by: CaptainIRS <[email protected]>
  • Loading branch information
@CaptainIRS
CaptainIRS and Dave Kelsey committed Oct 17, 2022
1 parent 0ded275 commit 4fc70d1
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 40 deletions.
29 changes: 21 additions & 8 deletions packages/caliper-core/lib/worker/tx-observers/prometheus-manager-tx-observer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
/**
Expand Down Expand Up @@ -117,15 +130,15 @@ class PrometheusManagerTxObserver extends TxObserverInterface {
this.pendingMessages.push(message);

if (this.method === 'collate' && this.pendingMessages.length === this.collationCount) {
await this._sendUpdate();
this._sendUpdate();
}
}

/**
* 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);
}
Expand All @@ -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);
}
}

Expand Down
73 changes: 41 additions & 32 deletions packages/caliper-core/test/worker/tx-observers/prometheus-manager-tx-observer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -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();
});
Expand Down

0 comments on commit 4fc70d1

Please sign in to comment.