li4wang commented on a change in pull request #1644:
URL: https://github.com/apache/zookeeper/pull/1644#discussion_r598217478
##########
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
##########
@@ -259,6 +260,8 @@ private ServerMetrics(MetricsProvider metricsProvider) {
WATCH_BYTES = metricsContext.getCounter("watch_bytes");
JVM_PAUSE_TIME = metricsContext.getSummary("jvm_pause_time_ms",
DetailLevel.ADVANCED);
+
+ QUOTA_EXCEEDED_ERROR_PER_NAMESPACE =
metricsContext.getSummarySet(QuotaMetricsUtils.QUOTA_EXCEEDED_ERROR_PER_NAMESPACE,
DetailLevel.BASIC);
Review comment:
SummarySet is used because it provides count (i.e. cnt) and the grouping
capability. Yes, you are right, in this case, since the count is always added
by 1, the cnt and sum are the same with PrometheusMetricsProvider and the
min/max/avg with DefaultMetricsProvider are always 1 in this case. User can
just use the cnt and ignore the others.
Is there any reason that CounterSet is not supported? Are you think we
should add a CounterSet in this PR?
I added GaugeSet for other quota metrics, as SummarySet really can not be
used for them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
