-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(metrics): add block transaction count and SR set change monitoring #6624
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
Changes from all commits
9b80f91
1d2de1a
1c0c1b7
bc87e2e
6bece43
39dd708
e92b8e2
846e3d3
ff17a19
7f351bf
7fdb3c5
7791944
a12378d
96bf315
033019c
4ca7d6e
d9aab3e
8ef3e91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ class MetricsCounter { | |
| init(MetricKeys.Counter.TXS, "tron txs info .", "type", "detail"); | ||
| init(MetricKeys.Counter.MINER, "tron miner info .", "miner", "type"); | ||
| init(MetricKeys.Counter.BLOCK_FORK, "tron block fork info .", "type"); | ||
| init(MetricKeys.Counter.SR_SET_CHANGE, "tron sr set change .", "action", "witness"); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Practical impact assessment:
Possible approaches:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for raising the high-cardinality concern — it's a valid Prometheus best practice to watch for. However, I believe the witness label is acceptable here given TRON's specific constraints:
In summary, the worst-case time series count is roughly candidate_pool_size × 2 (add/remove), which stays well within Prometheus's comfortable operating range. That said, I agree it's worth documenting this characteristic. Could you suggest what form you'd prefer — a code comment at the metric registration site, a note in the PR description, or something else?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prefer to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @halibobo1205 Added a comment at the
|
||
| init(MetricKeys.Counter.P2P_ERROR, "tron p2p error info .", "type"); | ||
| init(MetricKeys.Counter.P2P_DISCONNECT, "tron p2p disconnect .", "type"); | ||
| init(MetricKeys.Counter.INTERNAL_SERVICE_FAIL, "internal Service fail.", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package org.tron.common.prometheus; | ||
|
|
||
| import com.google.protobuf.ByteString; | ||
| import java.util.List; | ||
| import org.tron.common.utils.StringUtil; | ||
|
|
||
| public class SRMetrics { | ||
|
|
||
| private SRMetrics() { | ||
| throw new IllegalStateException("SRMetrics"); | ||
| } | ||
|
|
||
| public static void recordSrSetChange(List<ByteString> currentWits, List<ByteString> newWits) { | ||
| if (!Metrics.enabled()) { | ||
| return; | ||
| } | ||
| newWits.stream() | ||
| .filter(w -> !currentWits.contains(w)) | ||
| .forEach(w -> Metrics.counterInc(MetricKeys.Counter.SR_SET_CHANGE, 1, | ||
| MetricLabels.Counter.SR_ADD, StringUtil.encode58Check(w.toByteArray()))); | ||
| currentWits.stream() | ||
| .filter(w -> !newWits.contains(w)) | ||
| .forEach(w -> Metrics.counterInc(MetricKeys.Counter.SR_SET_CHANGE, 1, | ||
| MetricLabels.Counter.SR_REMOVE, StringUtil.encode58Check(w.toByteArray()))); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,206 @@ | ||
| package org.tron.common.prometheus; | ||
|
|
||
| import com.google.protobuf.ByteString; | ||
| import io.prometheus.client.CollectorRegistry; | ||
| import java.util.Arrays; | ||
| import java.util.Collections; | ||
| import java.util.concurrent.atomic.AtomicInteger; | ||
| import javax.annotation.Resource; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.junit.After; | ||
| import org.junit.Assert; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
| import org.tron.common.BaseTest; | ||
| import org.tron.common.TestConstants; | ||
| import org.tron.common.utils.StringUtil; | ||
| import org.tron.consensus.dpos.MaintenanceManager; | ||
| import org.tron.core.capsule.AccountCapsule; | ||
| import org.tron.core.capsule.VotesCapsule; | ||
| import org.tron.core.capsule.WitnessCapsule; | ||
| import org.tron.core.config.args.Args; | ||
| import org.tron.core.consensus.ConsensusService; | ||
| import org.tron.protos.Protocol; | ||
| import org.tron.protos.Protocol.Vote; | ||
|
|
||
| @Slf4j(topic = "metric") | ||
| public class SRMetricsTest extends BaseTest { | ||
|
|
||
| private static final AtomicInteger PORT = new AtomicInteger(0); | ||
| private static final AtomicInteger UNIQUE = new AtomicInteger(0); | ||
|
|
||
| @Resource | ||
| private MaintenanceManager maintenanceManager; | ||
| @Resource | ||
| private ConsensusService consensusService; | ||
|
|
||
| static { | ||
| Args.setParam(new String[]{"-d", dbPath()}, TestConstants.TEST_CONF); | ||
| Args.getInstance().setNodeListenPort(20000 + PORT.incrementAndGet()); | ||
| Args.getInstance().setMetricsPrometheusEnable(true); | ||
| Metrics.init(); | ||
| } | ||
|
|
||
| @Before | ||
| public void setUp() { | ||
| Args.getInstance().setMetricsPrometheusEnable(true); | ||
| consensusService.start(); | ||
| } | ||
|
|
||
| @After | ||
| public void tearDown() { | ||
| Args.getInstance().setMetricsPrometheusEnable(true); | ||
| } | ||
|
|
||
| /** | ||
| * Drive the full maintenance flow: starting with a single active witness while WitnessStore | ||
| * contains additional ones, doMaintenance() should expand active witnesses to the full set and | ||
| * emit SR_ADD for each newly active witness. | ||
| */ | ||
| @Test | ||
| public void testSrAddViaMaintenance() { | ||
| ByteString stableWit = registerWitness(); | ||
| ByteString newWit1 = registerWitness(); | ||
| ByteString newWit2 = registerWitness(); | ||
|
|
||
| chainBaseManager.getWitnessScheduleStore() | ||
| .saveActiveWitnesses(Collections.singletonList(stableWit)); | ||
|
|
||
| seedVote(stableWit); | ||
|
|
||
| maintenanceManager.doMaintenance(); | ||
|
|
||
| Assert.assertEquals(1, sample(MetricLabels.Counter.SR_ADD, newWit1).intValue()); | ||
| Assert.assertEquals(1, sample(MetricLabels.Counter.SR_ADD, newWit2).intValue()); | ||
| Assert.assertNull(sample(MetricLabels.Counter.SR_ADD, stableWit)); | ||
| Assert.assertNull(sample(MetricLabels.Counter.SR_REMOVE, stableWit)); | ||
| } | ||
|
|
||
| /** | ||
| * Active witness set already matches WitnessStore → no metric emitted. | ||
| */ | ||
| @Test | ||
| public void testNoMetricWhenSetUnchanged() { | ||
| ByteString witA = registerWitness(); | ||
| ByteString witB = registerWitness(); | ||
|
|
||
| chainBaseManager.getWitnessScheduleStore() | ||
| .saveActiveWitnesses(Arrays.asList(witA, witB)); | ||
|
|
||
| seedVote(witA); | ||
|
|
||
| maintenanceManager.doMaintenance(); | ||
|
|
||
| Assert.assertNull(sample(MetricLabels.Counter.SR_ADD, witA)); | ||
| Assert.assertNull(sample(MetricLabels.Counter.SR_ADD, witB)); | ||
| Assert.assertNull(sample(MetricLabels.Counter.SR_REMOVE, witA)); | ||
| Assert.assertNull(sample(MetricLabels.Counter.SR_REMOVE, witB)); | ||
| } | ||
|
|
||
| /** | ||
| * Empty VotesStore → countVote() is empty → SR change check is skipped, even when the active | ||
| * set differs from the full witness store. | ||
| */ | ||
| @Test | ||
| public void testNoMetricWhenNoVotes() { | ||
| ByteString stableWit = registerWitness(); | ||
| ByteString newWit = registerWitness(); | ||
|
|
||
| chainBaseManager.getWitnessScheduleStore() | ||
| .saveActiveWitnesses(Collections.singletonList(stableWit)); | ||
|
|
||
| maintenanceManager.doMaintenance(); | ||
|
|
||
| Assert.assertNull(sample(MetricLabels.Counter.SR_ADD, newWit)); | ||
| } | ||
|
|
||
| /** | ||
| * Metrics disabled → record() short-circuits even though the active set changes. | ||
| */ | ||
| @Test | ||
| public void testNoMetricWhenMetricsDisabled() { | ||
| Args.getInstance().setMetricsPrometheusEnable(false); | ||
| try { | ||
| ByteString stableWit = registerWitness(); | ||
| ByteString newWit = registerWitness(); | ||
|
|
||
| chainBaseManager.getWitnessScheduleStore() | ||
| .saveActiveWitnesses(Collections.singletonList(stableWit)); | ||
|
|
||
| seedVote(stableWit); | ||
|
|
||
| maintenanceManager.doMaintenance(); | ||
|
|
||
| Assert.assertNull(sample(MetricLabels.Counter.SR_ADD, newWit)); | ||
| } finally { | ||
| Args.getInstance().setMetricsPrometheusEnable(true); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * SR_REMOVE is verified by directly calling record() instead of going through doMaintenance(), | ||
| * because driving a removal through the real flow is impractical here: | ||
| * | ||
| * <p>Inside doMaintenance(), the block before SRMetrics.recordSrSetChange() iterates currentWits | ||
| * and calls setIsJobs(false) on each WitnessCapsule fetched from WitnessStore. If currentWits | ||
| * contains any address that is not present in WitnessStore, getWitness() returns null and the | ||
| * code NPEs — so SR_REMOVE cannot be triggered by simply pointing the active set at an | ||
| * "obsolete" address. | ||
| * | ||
| * <p>The only other path to SR_REMOVE is rank-based eviction: with more than | ||
| * MAX_ACTIVE_WITNESS_NUM (27) witnesses, sorting drops the lowest-ranked one. Building that | ||
| * setup just to exercise this branch is heavy and adds little value, since SR_ADD and | ||
| * SR_REMOVE share the exact same emit logic in record() — verifying SR_ADD via doMaintenance | ||
| * already proves the wiring is correct, and this direct call covers the symmetric branch. | ||
| */ | ||
| @Test | ||
| public void testSrRemoveDirect() { | ||
| ByteString stableWit = uniqueAddress(); | ||
| ByteString removedWit = uniqueAddress(); | ||
|
|
||
| SRMetrics.recordSrSetChange( | ||
| Arrays.asList(stableWit, removedWit), | ||
| Collections.singletonList(stableWit)); | ||
|
|
||
| Assert.assertEquals(1, sample(MetricLabels.Counter.SR_REMOVE, removedWit).intValue()); | ||
| Assert.assertNull(sample(MetricLabels.Counter.SR_ADD, removedWit)); | ||
| Assert.assertNull(sample(MetricLabels.Counter.SR_REMOVE, stableWit)); | ||
| } | ||
|
|
||
| private ByteString registerWitness() { | ||
| ByteString address = uniqueAddress(); | ||
| chainBaseManager.getWitnessStore().put(address.toByteArray(), new WitnessCapsule(address)); | ||
| chainBaseManager.addWitness(address); | ||
| chainBaseManager.getAccountStore().put(address.toByteArray(), | ||
| new AccountCapsule(Protocol.Account.newBuilder().setAddress(address).build())); | ||
| return address; | ||
| } | ||
|
|
||
| private void seedVote(ByteString voteFor) { | ||
| ByteString voter = uniqueAddress(); | ||
| VotesCapsule votes = new VotesCapsule(voter, Collections.emptyList(), | ||
| Collections.singletonList(Vote.newBuilder() | ||
| .setVoteAddress(voteFor) | ||
| .setVoteCount(1L) | ||
| .build())); | ||
| chainBaseManager.getVotesStore().put(voter.toByteArray(), votes); | ||
| } | ||
|
|
||
| private ByteString uniqueAddress() { | ||
| int n = UNIQUE.incrementAndGet(); | ||
| byte[] bytes = new byte[21]; | ||
| bytes[0] = 0x41; | ||
| bytes[17] = (byte) ((n >> 16) & 0xFF); | ||
| bytes[18] = (byte) ((n >> 8) & 0xFF); | ||
| bytes[19] = (byte) (n & 0xFF); | ||
| bytes[20] = 0x01; | ||
| return ByteString.copyFrom(bytes); | ||
| } | ||
|
|
||
| private Double sample(String action, ByteString witness) { | ||
| return CollectorRegistry.defaultRegistry.getSampleValue( | ||
| MetricKeys.Counter.SR_SET_CHANGE + "_total", | ||
| new String[]{"action", "witness"}, | ||
| new String[]{action, StringUtil.encode58Check(witness.toByteArray())}); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.