Repository navigation
Conversation
node_info gains address and role labels; add is_active, votes and rank gauges refreshed at startup and on maintenance blocks.
Stub BackupManager and MaintenanceManager lookups in both mocked startup contexts. Verify startup, node info, witness gauges and shutdown ordering without changing production code.
Address review feedback: keep node_info in its original form
(version, genesis_block_id) recorded in FullNode, and move the
witness address and role into a dedicated one-hot gauge
tron:node_role{address, role} where the full role set
(init/master/slaver) is always written and only the current role is
1. The gauge is exposed on witness-configured nodes only.
BackupManager now derives the role: with no backup election
configured the node is master; with backup configured but the
election not yet initialized it reports init, covering the pre-init
window where the status field default previously read as an elected
master. MetricsInfo is reverted to its original two-label
implementation, removing the mutable-label mechanism and the
label-swap race with concurrent scrapes.
tron:node_role now exposes one series per witness address whose value encodes the role: 0=init, 1=master, 2=slaver. This replaces the previous one-hot form that published three series per address with a role label. - SRMetrics owns the mapping as package-visible constants shared with tests; unknown role names fall back to init so a misderived state cannot masquerade as an elected one. - The gauge is registered with the single label 'address' and its help text documents the numeric mapping. - docs/metrics-changelog.md updated with the 0/1/2 mapping table. - Tests updated to assert the numeric encoding; a plain fullnode with no local witness still exposes no series.
Review follow-ups on the backup role gauge: - Post-init the election status is a real outcome, so map it 1:1: INIT -> init(0), MASTER -> master(1), SLAVER -> slaver(2). Previously a post-init INIT was misreported as slaver(2). The pre-init window is still gated by isInit, since the status field initializer (MASTER) is not an election result and cannot change: BlockHandleImpl.getState() relies on the MASTER default to decide whether this node may produce a block, and a node without backup configured never runs init(). - Multi-key setups (testnets) get one node_role series per configured witness identity: the witness account address when set, otherwise one address derived per private key, all carrying the same node-level role value.
- sr_is_active / sr_votes / sr_rank now carry an address label and emit one series per configured local witness address, mirroring tron:node_role; a node without a local witness keeps the existing empty-address zero-value series - SRMetrics caches the full address set resolved by BackupManager (explicit witness account address, or one address per configured private key); MaintenanceManager scans the witness list per address - a malformed address no longer skips recording for the remaining addresses until the next maintenance round (per-address catch + warn) - gauge help text: rank is 0 when the witness is unknown
localWitnessAddresses() preferred getWitnessAccountAddress(), but the production assembly path (WitnessInitializer -> LocalWitnesses.initWitnessAccountAddress) always derives it from the first private key, leaving it non-null even when multiple keys are configured. Multi-key nodes therefore published role/sr series only for the first key, while every key is its own SR identity. Branch on the private key count instead: with several keys (an explicit witness account address is rejected at startup in that case) derive one address per key; with a single key keep using the witness account address, covering multisig setups where it differs from the derived one. Add a regression test that builds LocalWitnesses through the production assembly path (WitnessInitializer.initFromCFGPrivateKey) with two keys and asserts both derived addresses get role series. Also fix a stale NODE_ROLE comment that still said one-hot after the numeric encoding change.
halibobo1205
approved these changes
Oct 23, 2026
halibobo1205
left a comment
halibobo1205
left a comment
Collaborator
There was a problem hiding this comment.
LGTM.
Before merge: the title still says "extend node_info", but node_info is unchanged now. Since it becomes the squash subject, could it be updated, e.g. feat(metrics): add node role and witness address gauges?
| } | ||
| for (Miner miner : miners) { | ||
| ByteString witnessAddress = miner.getWitnessAddress(); | ||
| if (witnessAddress == null) { |
Contributor
There was a problem hiding this comment.
Can delete this if (witnessAddress == null) {, since the below already has a try/catch.
Contributor
Author
There was a problem hiding this comment.
Agreed. The existing per-miner try/catch already handles address conversion failures, so I removed the redundant null check locally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds four low-cost node-status metrics and removes the earlier design that computed SR status during maintenance periods.
tron:node_info(Info, labelsversion,genesis_block_id): static node identity — the running version string plus the full genesis block hash as the canonical chain identifier.tron:node_type(Gauge, no labels): startup identity,0 = fullnode,1 = witness. Published once inConsensusService.start(); the identity is fixed at startup and cannot change at runtime.tron:node_role(Gauge, no labels): backup-election role,0 = init,1 = master,2 = slaver. Published byBackupManagerat startup and refreshed on every status transition. Independent of node type: both fullnodes and witness nodes report their backup role. A node without backup configured reportsmaster; a node with backup configured but not yet initialized reportsinit.tron:witness_address{address="..."}(Gauge, value1): one series per configured consensus witness account address, Base58Check-encoded. Published once inConsensusService.start(); a plain fullnode publishes no series. When a single key signs for an explicitly configured witness account, the witness account address is used rather than the signing-key address.Why are these changes required?
Operators need to distinguish chain/version, startup identity, and primary/backup role from metrics alone. Identity and role must be separate series: encoding fullnode as a role value would mask the backup state of a fullnode that has backup configured. All values are published at startup or on status transitions — no maintenance-period computation and no database access.
This PR has been tested by:
SRMetricsTest(19),BackupManagerTest(10), andPrometheusApiServiceTest(6), covering metric values and text export, identity/role independence, partial and absent backup configuration, disabled metrics, and single-node recovery frominittomasterthrough the real backup initialization and keep-alive timer path.:framework:checkstyleMain,:framework:checkstyleTest, and:plugins:checkstyleMainpassed.Follow up
None.
Extra details
No election, consensus, or maintenance-period behavior changes: the backup state machine, keep-alive loop, and member resolution are untouched; metric publication failures are isolated from startup and status transitions. When backup is configured with no reachable peer, the node reports
inituntil the keep-alive timeout (default ~18s) promotes it tomaster.