Repository navigation
refactor(metrics): phase 1 deprecation of the legacy monitor stack #6988
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
base: release_v4.8.3
Are you sure you want to change the base?
Changes from all commits
4e749e8
a1908f5
20ee8a9
e95c50c
6f65b8b
aeee8c7
bd4008d
833a7af
74d952e
56ef291
80740dd
c8f6bf6
10a8f2a
1f1a549
2fe5584
f7c9adf
b3e8dfd
4443724
feb287c
e3a8b12
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 |
|---|---|---|
|
|
@@ -21,6 +21,10 @@ public static class Counter { | |
| public static final String P2P_ERROR = "tron:p2p_error"; | ||
| public static final String P2P_DISCONNECT = "tron:p2p_disconnect"; | ||
| public static final String INTERNAL_SERVICE_FAIL = "tron:internal_service_fail"; | ||
| // verification counters for the bounded fetch latency estimator rollout | ||
| public static final String BLOCK_FETCH_ARMED = "tron:block_fetch_armed"; | ||
| public static final String BLOCK_FETCH_SECONDARY = "tron:block_fetch_secondary"; | ||
| public static final String BLOCK_ALREADY_KNOWN = "tron:block_already_known"; | ||
|
Contributor
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.
Contributor
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. Then all the below initialisation and calculation logic will looks very clear. Such as
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 — same as above:
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 the concrete sketch. The three names are pinned by the issue discussion (see the replies above), so we will keep them as-is. If you see wording in the init descriptions that could be clearer within the current names, we are happy to adopt that. |
||
|
|
||
| private Counter() { | ||
| throw new IllegalStateException("Counter"); | ||
|
|
@@ -44,6 +48,16 @@ private Gauge() { | |
|
|
||
| } | ||
|
|
||
| // Info | ||
| public static class Info { | ||
| public static final String NODE_INFO = "tron:node"; | ||
|
|
||
| private Info() { | ||
| throw new IllegalStateException("Info"); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| // Histogram | ||
| public static class Histogram { | ||
| public static final String HTTP_SERVICE_LATENCY = "tron:http_service_latency_seconds"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| package org.tron.common.prometheus; | ||
|
|
||
| import io.prometheus.client.Info; | ||
| import java.util.Map; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import lombok.extern.slf4j.Slf4j; | ||
|
|
||
| @Slf4j(topic = "metrics") | ||
| class MetricsInfo { | ||
|
|
||
| private static final Map<String, Info> container = new ConcurrentHashMap<>(); | ||
|
|
||
| static { | ||
| init(MetricKeys.Info.NODE_INFO, "tron node info.", | ||
| MetricLabels.Info.VERSION, MetricLabels.Info.CHAIN_ID); | ||
| } | ||
|
|
||
| private MetricsInfo() { | ||
| throw new IllegalStateException("MetricsInfo"); | ||
| } | ||
|
|
||
| private static void init(String name, String help, String... labels) { | ||
| container.put(name, Info.build() | ||
| .name(name) | ||
| .help(help) | ||
| .labelNames(labels) | ||
| .register()); | ||
| } | ||
|
|
||
| static void set(String key, String... labels) { | ||
| if (Metrics.enabled()) { | ||
| Info info = container.get(key); | ||
| if (info == null) { | ||
| logger.info("{} not exist", key); | ||
| return; | ||
| } | ||
| info.labels(labels); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest use a more easy to understand name like
tron:block_fetch_trackedinstead oftron:block_fetch_armed. Replacetron:block_fetch_secondarywithtron:block_fetch_failoverThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. These counter names and their semantics were discussed and agreed in the tracking issue before implementation: the counters were requested as unlabeled, monotonically increasing series for fetch trackings armed and secondary fetches sent (#6923 (comment)), the final names
tron:block_fetch_armedandtron:block_fetch_secondarywere confirmed there (#6923 (comment)), and the counters are expected to be kept beyond Phase 2 (#6923 (comment)). The verification data already posted in the issue references these names as well. To keep the public metric contract consistent with that discussion, we would prefer to keep the current names; happy to revisit if the issue discussion converges on different ones.