agura commented on code in PR #1766: URL: https://github.com/apache/ignite-3/pull/1766#discussion_r1143976368
########## modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientHandlerMetricSource.java: ########## @@ -0,0 +1,399 @@ +/* + * 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.ignite.client.handler; + +import java.util.Arrays; +import java.util.List; +import org.apache.ignite.internal.metrics.AbstractMetricSource; +import org.apache.ignite.internal.metrics.AtomicLongMetric; +import org.apache.ignite.internal.metrics.Metric; +import org.apache.ignite.internal.metrics.MetricSetBuilder; + +/** + * Server-side client handler metrics. + */ +public class ClientHandlerMetricSource extends AbstractMetricSource<ClientHandlerMetricSource.Holder> { + /** + * Constructor. + */ + public ClientHandlerMetricSource() { + super("client-handler"); + } + + /** + * Gets total initiated connections. + * + * @return Total initiated connections. + */ + public long connectionsInitiated() { + Holder h = holder(); + + return h == null ? 0 : h.connectionsInitiated.value(); + } + + void connectionsInitiatedIncrement() { + Holder h = holder(); + + if (h != null) { + h.connectionsInitiated.increment(); + } + } + + /** + * Gets total accepted sessions. + * + * @return Total accepted sessions. + */ + public long sessionsAccepted() { + Holder h = holder(); + + return h == null ? 0 : h.sessionsAccepted.value(); + } + + void sessionsAcceptedIncrement() { + Holder h = holder(); + + if (h != null) { + h.sessionsAccepted.increment(); + } + } + + /** + * Gets active sessions. + * + * @return Active sessions. + */ + public long sessionsActive() { + Holder h = holder(); + + return h == null ? 0 : h.sessionsActive.value(); + } + + void sessionsActiveIncrement() { + Holder h = holder(); + + if (h != null) { + h.sessionsActive.increment(); + } + } + + void sessionsActiveDecrement() { + Holder h = holder(); + + if (h != null) { + h.sessionsActive.decrement(); + } + } + + /** + * Gets total rejected sessions. + * + * @return Total rejected sessions. + */ + public long sessionsRejected() { + Holder h = holder(); + + return h == null ? 0 : h.sessionsRejected.value(); + } + + void sessionsRejectedIncrement() { + Holder h = holder(); + + if (h != null) { + h.sessionsRejected.increment(); + } + } + + /** + * Gets sessions rejected due to TLS errors. + * + * @return Sessions rejected due to TLS errors. + */ + public long sessionsRejectedTls() { + Holder h = holder(); + + return h == null ? 0 : h.sessionsRejectedTls.value(); + } + + void sessionsRejectedTlsIncrement() { + Holder h = holder(); + + if (h != null) { + h.sessionsRejectedTls.increment(); + } + } + + /** + * Gets sent bytes. + * + * @return Sent bytes. + */ + public long bytesSent() { + Holder h = holder(); + + return h == null ? 0 : h.bytesSent.value(); + } + + void bytesSentAdd(long bytes) { + Holder h = holder(); + + if (h != null) { + h.bytesSent.add(bytes); + } + } + + /** + * Gets received bytes. + * + * @return Received bytes. + */ + public long bytesReceived() { + Holder h = holder(); + + return h == null ? 0 : h.bytesReceived.value(); + } + + void bytesReceivedAdd(long bytes) { + Holder h = holder(); + + if (h != null) { + h.bytesReceived.add(bytes); + } + } + + /** + * Gets sessions rejected due to a timeout. + * + * @return Sessions rejected due to a timeout. + */ + public long sessionsRejectedTimeout() { + Holder h = holder(); + + return h == null ? 0 : h.sessionsRejectedTimeout.value(); + } + + void sessionsRejectedTimeoutIncrement() { + Holder h = holder(); + + if (h != null) { + h.sessionsRejectedTimeout.increment(); + } + } + + /** + * Gets active requests. + * + * @return Active requests. + */ + public long requestsActive() { + Holder h = holder(); + + return h == null ? 0 : h.requestsActive.value(); + } + + void requestsActiveIncrement() { + Holder h = holder(); + + if (h != null) { + h.requestsActive.increment(); + } + } + + void requestsActiveDecrement() { + Holder h = holder(); + + if (h != null) { + h.requestsActive.decrement(); + } + } + + /** + * Gets processed requests. + * + * @return Processed requests. + */ + public long requestsProcessed() { + Holder h = holder(); + + return h == null ? 0 : h.requestsProcessed.value(); + } + + void requestsProcessedIncrement() { + Holder h = holder(); + + if (h != null) { + h.requestsProcessed.increment(); + } + } + + /** + * Gets failed requests. + * + * @return Failed requests. + */ + public long requestsFailed() { + Holder h = holder(); + + return h == null ? 0 : h.requestsFailed.value(); + } + + void requestsFailedIncrement() { + Holder h = holder(); + + if (h != null) { + h.requestsFailed.increment(); + } + } + + /** + * Gets active transactions. + * + * @return Active transactions. + */ + public long transactionsActive() { + Holder h = holder(); + + return h == null ? 0 : h.transactionsActive.value(); + } + + /** + * Increments active transactions. + */ + public void transactionsActiveIncrement() { + Holder h = holder(); + + if (h != null) { + h.transactionsActive.increment(); + } + } + + /** + * Decrements active transactions. + */ + public void transactionsActiveDecrement() { + Holder h = holder(); + + if (h != null) { + h.transactionsActive.decrement(); + } + } + + /** + * Gets active cursors. + * + * @return Active cursors. + */ + public long cursorsActive() { + Holder h = holder(); + + return h == null ? 0 : h.cursorsActive.value(); + } + + /** + * Increments active cursors. + */ + public void cursorsActiveIncrement() { + Holder h = holder(); + + if (h != null) { + h.cursorsActive.increment(); + } + } + + /** + * Decrements active cursors. + */ + public void cursorsActiveDecrement() { + Holder h = holder(); + + if (h != null) { + h.cursorsActive.decrement(); + } + } + + /** {@inheritDoc} */ + @Override + protected void init(MetricSetBuilder bldr, Holder holder) { + holder.register(bldr); + } + + /** {@inheritDoc} */ + @Override + protected Holder createHolder() { + return new Holder(); + } + + /** + * Holder. + */ + protected static class Holder implements AbstractMetricSource.Holder<Holder> { + private final AtomicLongMetric connectionsInitiated = + new AtomicLongMetric("connections.Initiated", "Total initiated connections"); + + private final AtomicLongMetric sessionsAccepted = + new AtomicLongMetric("sessions.Accepted", "Total accepted sessions"); Review Comment: IMHO: redundant grouping here. The amount of metrics is not large so there is no need to additional grouping like `sessions.Active` and `sessions.Rejected`. So my proposal is to rename metrics to something l ike `SessionsActive`, `SessionsRejected`, `BytesSent`, etc. ########## modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientHandlerMetricSource.java: ########## @@ -0,0 +1,399 @@ +/* + * 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.ignite.client.handler; + +import java.util.Arrays; +import java.util.List; +import org.apache.ignite.internal.metrics.AbstractMetricSource; +import org.apache.ignite.internal.metrics.AtomicLongMetric; +import org.apache.ignite.internal.metrics.Metric; +import org.apache.ignite.internal.metrics.MetricSetBuilder; + +/** + * Server-side client handler metrics. + */ +public class ClientHandlerMetricSource extends AbstractMetricSource<ClientHandlerMetricSource.Holder> { + /** + * Constructor. + */ + public ClientHandlerMetricSource() { + super("client-handler"); Review Comment: It is not stated explicitly but I think that usage of dash (`-`) in a source or a metric name is not good idea because some 3rd-party tools can treat dash as some operator. But this is the recommendation, not a requirement -- 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]
