PDavid commented on code in PR #2328: URL: https://github.com/apache/zookeeper/pull/2328#discussion_r2707257494
########## zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerMetricsOpCountTest.java: ########## @@ -0,0 +1,198 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper.server; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.zookeeper.CreateMode; +import org.apache.zookeeper.ZooDefs.Ids; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.metrics.MetricsUtils; +import org.apache.zookeeper.test.ClientBase; +import org.junit.jupiter.api.Test; + +public class ServerMetricsOpCountTest extends ClientBase { Review Comment: Many thanks for adding the test for this. :+1: ########## zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java: ########## @@ -556,6 +584,35 @@ private ServerMetrics(MetricsProvider metricsProvider) { public final Counter TTL_NODE_DELETED_COUNT; public final Counter TTL_NODE_CREATED_COUNT; + /** + * Operation count metrics + */ Review Comment: Here as well. Do we need to have this as JavaDoc comment or rather some kind of divider? :thinking: As is right now the JavaDoc (documentation) of `TOTAL_OP_COUNT` field will be "Operation count metrics". Is this desired? If we need some kind if divider would it maybe make sense to use a normal (not JavaDoc comment)? I see that there are mixed examples in this class already. What do you all think? ########## zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java: ########## @@ -272,6 +272,34 @@ private ServerMetrics(MetricsProvider metricsProvider) { TTL_NODE_DELETED_COUNT = metricsContext.getCounter("ttl_node_deleted_count"); TTL_NODE_CREATED_COUNT = metricsContext.getCounter("ttl_node_created_count"); + + /** + * Operation count metrics + */ Review Comment: Just a small / nit question: Why do we need a JavaDoc comment here for this? I guess this would be some kind of a divider here and we have some examples above, but in this case shouldn't this be a normal (not JavaDoc comment)? (I can see that there are already some examples of this above but I think those are also most probably wrong.) -- 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]
