Add node authority status metric #4699

sandreim · 2022-01-12T10:13:50Z

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

eskimor

While there is no "perfect" subsystem to add these generic logs, I would prefer to use one where we are already doing the check. gossip-support would make the "most" sense to me. Any reason why you added the logs to chain selection in particular?

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim · 2022-01-12T10:53:19Z

While there is no "perfect" subsystem to add these generic logs, I would prefer to use one where we are already doing the check. gossip-support would make the "most" sense to me. Any reason why you added the logs to chain selection in particular?

It was path of least resistance given the fact that I tried overseer, runtime-api and a new tiny subsystem just for this.

That being said, you are right and I totally missed this - gossip support seems the way to go here as the check is already done in there.

bkchr

I'm still not convinced that we need this. However, if you really insist on this, why not add a prometheus metric for this?

node/core/chain-selection/src/lib.rs

ordian · 2022-01-12T11:24:30Z

That being said, you are right and I totally missed this - gossip support seems the way to go here as the check is already done in there.

Note that in gossip-support we're querying discovery_keys, which also past session validators. To check only the current session, we can also query the current n_validators (e.g. by querying util::request_validators) and check if our_index is < n_validators.

The previous suggestion was incorrect since we query not from session_info. Validator::new is the way to go.

ordian · 2022-01-12T11:26:08Z

Would it be good to add a prometheus metric for this as well?

sandreim · 2022-01-12T11:31:56Z

Would it be good to add a prometheus metric for this as well?

IMO the log is enough, but if folks think this is useful I'll add it.

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

This reverts commit 5bd56bb.

This reverts commit ffea18f.

This reverts commit 55a30ac.

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/network/gossip-support/src/metrics.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/network/gossip-support/src/lib.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

ordian

LGTM

node/network/gossip-support/src/lib.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/network/gossip-support/src/metrics.rs

node/network/gossip-support/src/lib.rs

drahnr

Minor bike shed nit, other than that, LGTM 👍

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

primitives/src/v2/mod.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/network/gossip-support/src/tests.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/network/gossip-support/src/metrics.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

ordian · 2022-01-13T16:42:21Z

bot merge

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

paritytech-processbot · 2022-01-13T16:42:30Z

Waiting for commit status.

paritytech-processbot · 2022-01-13T16:43:22Z

Merge cancelled due to error. Error: Head SHA changed from 4633e8c to 541ad6d

ordian · 2022-01-13T16:43:32Z

bot merge

paritytech-processbot · 2022-01-13T16:43:40Z

Waiting for commit status.

* Check authority status on active leaves update Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * cargo changes Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Add metric for authority status Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Revert "Fix tests" This reverts commit 5bd56bb. * Revert "cargo changes" This reverts commit ffea18f. * Revert "Check authority status on active leaves update" This reverts commit 55a30ac. * Test fixups Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * update Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * undo damage Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * dont update status on runtime errors Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix inconsistency Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Review feedback Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Dont derive primitive Default Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * add dummy_session_info helper Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * unset parachain validator status if no longer authority Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * update Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * damn Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * 🤦 Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim added 2 commits January 12, 2022 10:12

Check authority status on active leaves update

55a30ac

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

cargo changes

ffea18f

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jan 12, 2022

sandreim requested review from drahnr and eskimor January 12, 2022 10:14

eskimor reviewed Jan 12, 2022

View reviewed changes

Fix tests

5bd56bb

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

bkchr reviewed Jan 12, 2022

View reviewed changes

node/core/chain-selection/src/lib.rs Outdated Show resolved Hide resolved

node/core/chain-selection/src/lib.rs Outdated Show resolved Hide resolved

sandreim added 4 commits January 12, 2022 14:33

Add metric for authority status

1234ab1

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Revert "Fix tests"

a5d3b6e

This reverts commit 5bd56bb.

Revert "cargo changes"

cf824f3

This reverts commit ffea18f.

Revert "Check authority status on active leaves update"

625cc2f

This reverts commit 55a30ac.

sandreim changed the title ~~Check if the node is an authority on each block import and log changes~~ Add node authority status metric Jan 12, 2022

Test fixups

d8304fd

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

drahnr reviewed Jan 12, 2022

View reviewed changes

node/network/gossip-support/src/metrics.rs Outdated Show resolved Hide resolved

fix

a44d8bf

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

eskimor reviewed Jan 12, 2022

View reviewed changes

node/network/gossip-support/src/lib.rs Outdated Show resolved Hide resolved

sandreim added 2 commits January 12, 2022 20:05

update

404bae8

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

undo damage

d30205f

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

ordian approved these changes Jan 12, 2022

View reviewed changes

ordian reviewed Jan 12, 2022

View reviewed changes

node/network/gossip-support/src/lib.rs Outdated Show resolved Hide resolved

dont update status on runtime errors

b0a3428

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

drahnr reviewed Jan 13, 2022

View reviewed changes

node/network/gossip-support/src/metrics.rs Show resolved Hide resolved

drahnr reviewed Jan 13, 2022

View reviewed changes

node/network/gossip-support/src/metrics.rs Show resolved Hide resolved

drahnr reviewed Jan 13, 2022

View reviewed changes

node/network/gossip-support/src/lib.rs Outdated Show resolved Hide resolved

drahnr reviewed Jan 13, 2022

View reviewed changes

node/network/gossip-support/src/lib.rs Outdated Show resolved Hide resolved

drahnr approved these changes Jan 13, 2022

View reviewed changes

sandreim added 3 commits January 13, 2022 11:35

Fix tests

65aa234

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix inconsistency

8a29350

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Review feedback

d05bfc9

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim requested review from eskimor and ordian January 13, 2022 15:51

drahnr reviewed Jan 13, 2022

View reviewed changes

primitives/src/v2/mod.rs Outdated Show resolved Hide resolved

eskimor reviewed Jan 13, 2022

View reviewed changes

primitives/src/v2/mod.rs Outdated Show resolved Hide resolved

Dont derive primitive Default

2d75603

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

ordian reviewed Jan 13, 2022

View reviewed changes

node/network/gossip-support/src/tests.rs Outdated Show resolved Hide resolved

add dummy_session_info helper

cac69c6

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

ordian approved these changes Jan 13, 2022

View reviewed changes

node/network/gossip-support/src/metrics.rs Show resolved Hide resolved

eskimor approved these changes Jan 13, 2022

View reviewed changes

sandreim added 3 commits January 13, 2022 16:35

unset parachain validator status if no longer authority

a0930ec

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

update

c953489

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

damn

4633e8c

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

🤦

541ad6d

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

paritytech-processbot bot merged commit 095cc29 into master Jan 13, 2022

paritytech-processbot bot deleted the sandreim/log_auth_status_change_try2 branch January 13, 2022 17:41

stakeworld mentioned this pull request Jun 12, 2022

Prometheus endpoint "polkadot_node_is_authority" does not switch back when getting out of the active set #5664

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add node authority status metric #4699

Add node authority status metric #4699

sandreim commented Jan 12, 2022

eskimor left a comment

sandreim commented Jan 12, 2022 •

edited

Loading

bkchr left a comment

ordian commented Jan 12, 2022 •

edited

Loading

ordian commented Jan 12, 2022

sandreim commented Jan 12, 2022 •

edited

Loading

ordian left a comment

drahnr left a comment

ordian commented Jan 13, 2022

paritytech-processbot bot commented Jan 13, 2022

paritytech-processbot bot commented Jan 13, 2022

ordian commented Jan 13, 2022

paritytech-processbot bot commented Jan 13, 2022

Add node authority status metric #4699

Add node authority status metric #4699

Conversation

sandreim commented Jan 12, 2022

eskimor left a comment

Choose a reason for hiding this comment

sandreim commented Jan 12, 2022 • edited Loading

bkchr left a comment

Choose a reason for hiding this comment

ordian commented Jan 12, 2022 • edited Loading

ordian commented Jan 12, 2022

sandreim commented Jan 12, 2022 • edited Loading

ordian left a comment

Choose a reason for hiding this comment

drahnr left a comment

Choose a reason for hiding this comment

ordian commented Jan 13, 2022

paritytech-processbot bot commented Jan 13, 2022

paritytech-processbot bot commented Jan 13, 2022

ordian commented Jan 13, 2022

paritytech-processbot bot commented Jan 13, 2022

sandreim commented Jan 12, 2022 •

edited

Loading

ordian commented Jan 12, 2022 •

edited

Loading

sandreim commented Jan 12, 2022 •

edited

Loading