Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distinguish different workers in Prometheus PushGateway #1427

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

CaptainIRS
Copy link
Contributor

In this PR:

  • The worker index of each worker is added as a label when pushing metrics to PushGateway to distinguish workers.

Fixes #1354

Results:

/metrics endpoint of PushGateway

image

Grafana dashboard showing metrics from two workers simultaneously

image

Copy link
Contributor

@davidkel davidkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With every code change in caliper we need to add unit tests, and so we need a new unit test for this change as I don't think there is a unit test that covers this code

Copy link
Contributor Author

@CaptainIRS CaptainIRS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have looked at https://github.com/hyperledger/caliper/blob/main/packages/caliper-core/test/worker/tx-observers/prometheus-push-tx-observer.js, but there doesn't seem to be any tests covering _sendUpdate as it is being stubbed out in unit tests. I'm not sure how a unit test for this would look like as pushAdd is a method from the prom-client library. Should the unit test just ensure that workerIndex is being passed to the pushAdd method (by using mocks)?

@davidkel
Copy link
Contributor

davidkel commented Aug 4, 2022

We would need a stub of the method this.prometheusPushGateway.pushAdd so we can track that it is being called and passing the expected parameters. This will then give us code coverage of that line and we know that it executes and passes the correct parameters, also checks that the call is likely to succeed and not fail when executed for real.

* Remove redundant labels
* Add roundIndex to groupings
* Update unit tests utilizing workerIndex and roundIndex
* Write new unit test for _sendUpdate

Signed-off-by: CaptainIRS <[email protected]>
@CaptainIRS CaptainIRS requested a review from davidkel August 4, 2022 11:24
Copy link
Contributor

@davidkel davidkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@davidkel davidkel merged commit c648fa9 into hyperledger-caliper:main Aug 4, 2022
eravatee pushed a commit to eravatee/caliper that referenced this pull request Oct 1, 2022
…caliper#1427)

* Distinguish different workers in Prometheus PushGateway

Signed-off-by: CaptainIRS <[email protected]>

* Address review comments

* Remove redundant labels
* Add roundIndex to groupings
* Update unit tests utilizing workerIndex and roundIndex
* Write new unit test for _sendUpdate

Signed-off-by: CaptainIRS <[email protected]>
Signed-off-by: eravatee <[email protected]>
eravatee pushed a commit to eravatee/caliper that referenced this pull request Oct 4, 2022
…caliper#1427)

* Distinguish different workers in Prometheus PushGateway

Signed-off-by: CaptainIRS <[email protected]>

* Address review comments

* Remove redundant labels
* Add roundIndex to groupings
* Update unit tests utilizing workerIndex and roundIndex
* Write new unit test for _sendUpdate

Signed-off-by: CaptainIRS <[email protected]>
Signed-off-by: eravatee <[email protected]>
eravatee pushed a commit to eravatee/caliper that referenced this pull request Oct 4, 2022
…caliper#1427)

* Distinguish different workers in Prometheus PushGateway

Signed-off-by: CaptainIRS <[email protected]>

* Address review comments

* Remove redundant labels
* Add roundIndex to groupings
* Update unit tests utilizing workerIndex and roundIndex
* Write new unit test for _sendUpdate

Signed-off-by: CaptainIRS <[email protected]>
Signed-off-by: eravatee <[email protected]>
eravatee pushed a commit to eravatee/caliper that referenced this pull request Oct 4, 2022
…caliper#1427)

* Distinguish different workers in Prometheus PushGateway

Signed-off-by: CaptainIRS <[email protected]>

* Address review comments

* Remove redundant labels
* Add roundIndex to groupings
* Update unit tests utilizing workerIndex and roundIndex
* Write new unit test for _sendUpdate

Signed-off-by: CaptainIRS <[email protected]>
Signed-off-by: eravatee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prometheus push gateway can't distinguish different workers and other issues
2 participants