codings-dan commented on code in PR #742:
URL: https://github.com/apache/ratis/pull/742#discussion_r971878690
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/metrics/MessageMetrics.java:
##########
@@ -17,49 +17,71 @@
*/
package org.apache.ratis.grpc.metrics;
+import org.apache.ratis.metrics.LongCounter;
import org.apache.ratis.metrics.MetricRegistryInfo;
import org.apache.ratis.metrics.RatisMetrics;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import java.util.Map;
public class MessageMetrics extends RatisMetrics {
- static final Logger LOG = LoggerFactory.getLogger(MessageMetrics.class);
public static final String GRPC_MESSAGE_METRICS = "%s_message_metrics";
public static final String GRPC_MESSAGE_METRICS_DESC = "Outbound/Inbound
message counters";
+ private enum Type {
+ STARTED("_started_total"),
+ COMPLETED("_completed_total"),
+ RECEIVED("_received_executed");
Review Comment:
_received_executed -> _received_total or _received_executed_total
##########
ratis-server/src/main/java/org/apache/ratis/server/metrics/LeaderElectionMetrics.java:
##########
@@ -43,12 +44,20 @@ public final class LeaderElectionMetrics extends
RatisMetrics {
public static final String TRANSFER_LEADERSHIP_COUNT_METRIC =
"transferLeadershipCount";
public static final String LAST_LEADER_ELECTION_ELAPSED_TIME =
"lastLeaderElectionElapsedTime";
+
+ private final LongCounter electionCount =
getRegistry().counter(LEADER_ELECTION_COUNT_METRIC);
+ private final LongCounter timeoutCount =
getRegistry().counter(LEADER_ELECTION_TIMEOUT_COUNT_METRIC);
+ private final LongCounter transferLeadershipCount =
getRegistry().counter(TRANSFER_LEADERSHIP_COUNT_METRIC);
+
+ private final Timekeeper electionTime =
getRegistry().timer(LEADER_ELECTION_TIME_TAKEN);
+
private volatile Timestamp lastElectionTime;
private LeaderElectionMetrics(RaftGroupMemberId serverId, LongSupplier
getLastLeaderElapsedTimeMs) {
- this.registry = getMetricRegistryForLeaderElection(serverId);
- registry.gauge(LAST_LEADER_ELAPSED_TIME, () ->
getLastLeaderElapsedTimeMs::getAsLong);
- registry.gauge(LAST_LEADER_ELECTION_ELAPSED_TIME,
+ super(getMetricRegistryForLeaderElection(serverId));
Review Comment:
rename `getMetricRegistryForLeaderElection` -> `createRegistry`
##########
ratis-netty/src/main/java/org/apache/ratis/netty/metrics/NettyServerStreamRpcMetrics.java:
##########
@@ -88,11 +92,32 @@ public void stop(Timekeeper.Context context, boolean
success) {
}
}
+ private enum Op {
+ Create(RequestType::getNumRequestsString),
+ Success(RequestType::getSuccessCountString),
+ Fail(RequestType::getFailCountString);
+
+ private final Function<RequestType, String> stringFunction;
+
+ Op(Function<RequestType, String> stringFunction) {
+ this.stringFunction = stringFunction;
+ }
+
+ String getString(RequestType type) {
+ return stringFunction.apply(type);
+ }
+ }
+
+ private final Map<String, Timekeeper> latencyTimers = new
ConcurrentHashMap<>();
+ private final Map<Op, Map<String, LongCounter>> ops;
+
public NettyServerStreamRpcMetrics(String serverId) {
- registry = getMetricRegistryForGrpcServer(serverId);
+ super(getMetricRegistryForGrpcServer(serverId));
Review Comment:
rename `getMetricRegistryForGrpcServer` -> `createRegistry`
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/metrics/MessageMetrics.java:
##########
@@ -17,49 +17,71 @@
*/
package org.apache.ratis.grpc.metrics;
+import org.apache.ratis.metrics.LongCounter;
import org.apache.ratis.metrics.MetricRegistryInfo;
import org.apache.ratis.metrics.RatisMetrics;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import java.util.Map;
public class MessageMetrics extends RatisMetrics {
- static final Logger LOG = LoggerFactory.getLogger(MessageMetrics.class);
public static final String GRPC_MESSAGE_METRICS = "%s_message_metrics";
public static final String GRPC_MESSAGE_METRICS_DESC = "Outbound/Inbound
message counters";
+ private enum Type {
+ STARTED("_started_total"),
+ COMPLETED("_completed_total"),
+ RECEIVED("_received_executed");
+
+ private final String suffix;
+
+ Type(String suffix) {
+ this.suffix = suffix;
+ }
+
+ String getSuffix() {
+ return suffix;
+ }
+ }
+
+ private final Map<Type, Map<String, LongCounter>> types;
+
public MessageMetrics(String endpointId, String endpointType) {
- this.registry = create(
+ super(create(
new MetricRegistryInfo(endpointId,
RATIS_APPLICATION_NAME_METRICS,
String.format(GRPC_MESSAGE_METRICS, endpointType),
GRPC_MESSAGE_METRICS_DESC)
- );
+ ));
+
+ this.types = newCounterMaps(Type.class);
+ }
+
+ private void inc(Type t, String rpcType) {
+ types.get(t)
+ .computeIfAbsent(rpcType, key -> getRegistry().counter(key +
t.getSuffix()))
+ .inc();
}
/**
* Increments the count of RPCs that are started.
* Both client and server use this.
- * @param rpcType
*/
public void rpcStarted(String rpcType){
Review Comment:
The variable name here is `rpcType` Inappropriate, I think it should be
`rpcName`
##########
ratis-server/src/main/java/org/apache/ratis/server/metrics/RaftLogMetricsBase.java:
##########
@@ -33,8 +34,12 @@ public class RaftLogMetricsBase extends RatisMetrics
implements RaftLogMetrics {
public static final String CONFIG_LOG_ENTRY_COUNT = "configLogEntryCount";
public static final String STATE_MACHINE_LOG_ENTRY_COUNT =
"stateMachineLogEntryCount";
+ private final LongCounter configLogEntryCount =
getRegistry().counter(CONFIG_LOG_ENTRY_COUNT);
+ private final LongCounter metadataLogEntryCount =
getRegistry().counter(METADATA_LOG_ENTRY_COUNT);
+ private final LongCounter stateMachineLogEntryCount =
getRegistry().counter(STATE_MACHINE_LOG_ENTRY_COUNT);
+
public RaftLogMetricsBase(RaftGroupMemberId serverId) {
- this.registry = getLogWorkerMetricRegistry(serverId);
+ super(getLogWorkerMetricRegistry(serverId));
Review Comment:
rename `getLogWorkerMetricRegistry` -> `createRegistry`
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/metrics/MessageMetrics.java:
##########
@@ -17,49 +17,71 @@
*/
package org.apache.ratis.grpc.metrics;
+import org.apache.ratis.metrics.LongCounter;
import org.apache.ratis.metrics.MetricRegistryInfo;
import org.apache.ratis.metrics.RatisMetrics;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import java.util.Map;
public class MessageMetrics extends RatisMetrics {
- static final Logger LOG = LoggerFactory.getLogger(MessageMetrics.class);
public static final String GRPC_MESSAGE_METRICS = "%s_message_metrics";
public static final String GRPC_MESSAGE_METRICS_DESC = "Outbound/Inbound
message counters";
+ private enum Type {
+ STARTED("_started_total"),
+ COMPLETED("_completed_total"),
+ RECEIVED("_received_executed");
+
+ private final String suffix;
+
+ Type(String suffix) {
+ this.suffix = suffix;
+ }
+
+ String getSuffix() {
+ return suffix;
+ }
+ }
+
+ private final Map<Type, Map<String, LongCounter>> types;
+
public MessageMetrics(String endpointId, String endpointType) {
- this.registry = create(
+ super(create(
Review Comment:
Let's align with GrpcServerMetrics and create a new method
```
private static RatisMetricRegistry createRegistry(String endpointId) {
return create(new MetricRegistryInfo(endpointId,
RATIS_APPLICATION_NAME_METRICS,
String.format(GRPC_MESSAGE_METRICS, endpointType),
GRPC_MESSAGE_METRICS_DESC);
}
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
