You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2023/01/11 09:17:38 UTC
[GitHub] [pinot] zhtaoxiang opened a new pull request, #10106: clean up the logic of reading guage metrics
zhtaoxiang opened a new pull request, #10106:
URL: https://github.com/apache/pinot/pull/10106
With `org.apache.pinot.plugin.metrics.yammer.YammerSettableGauge` and `org.apache.pinot.plugin.metrics.dropwizard.DropwizardSettableGauge`, gauge value stored in metric registry can be updated. Therefore, it is now possible to remove `_gaugeValues` defined in `org.apache.pinot.common.metrics.AbstractMetrics`, which is a walkaround to make gauge value changeble.
This PR removes the methods of reading guage metrics from `_gaugeValues` defined in `org.apache.pinot.common.metrics.AbstractMetrics`, so that we can remove `_gaugeValues` in a future PR. Notice that it's safe to delete those methods because they are only used in unit tests and integration tests.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] zhtaoxiang commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
zhtaoxiang commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1081609829
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java:
##########
@@ -481,20 +461,6 @@ public long getValueOfTableGauge(final String tableName, final G gauge) {
return gaugeValue == null ? 0 : gaugeValue.get();
}
- /**
- * Gets the value of a table partition gauge.
- *
- * @param tableName The table name
- * @param partitionId The partition name
- * @param gauge The gauge to use
- */
- public long getValueOfPartitionGauge(final String tableName, final int partitionId, final G gauge) {
- final String fullGaugeName = composeTableGaugeName(tableName, String.valueOf(partitionId), gauge);
-
- AtomicLong gaugeValue = _gaugeValues.get(fullGaugeName);
- return gaugeValue == null ? -1 : gaugeValue.get();
Review Comment:
This code is removed. So no way to add `@VisibleForTesting` 😄
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] zhtaoxiang commented on pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
zhtaoxiang commented on PR #10106:
URL: https://github.com/apache/pinot/pull/10106#issuecomment-1386272418
> Also can we test at least one metric in each category to make sure they are emitted properly on a live system?
Not sure if I understand you comment. There is no metric emission logic change in this PR, but only metric reading logic used in unit tests. Therefore, I think we don't need to test on a live system. What do you think?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
jugomezv commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1070080008
##########
pinot-controller/src/test/java/org/apache/pinot/controller/validation/StorageQuotaCheckerTest.java:
##########
@@ -104,33 +105,39 @@ public void testWithinQuota()
// No response from server, should pass without updating metrics
mockTableSizeResult(-1, 0);
assertTrue(isSegmentWithinQuota());
- assertEquals(
- controllerMetrics.getValueOfTableGauge(OFFLINE_TABLE_NAME, ControllerGauge.OFFLINE_TABLE_ESTIMATED_SIZE), 0);
+ assertFalse(
+ MetricValueUtils.gaugeExists(controllerMetrics,
Review Comment:
similar comment here: should we have a MetricValueUtils.tableGaugeExists() than pass name of table as an argument?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] snleee commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
snleee commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1072856278
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java:
##########
@@ -481,20 +461,6 @@ public long getValueOfTableGauge(final String tableName, final G gauge) {
return gaugeValue == null ? 0 : gaugeValue.get();
}
- /**
- * Gets the value of a table partition gauge.
- *
- * @param tableName The table name
- * @param partitionId The partition name
- * @param gauge The gauge to use
- */
- public long getValueOfPartitionGauge(final String tableName, final int partitionId, final G gauge) {
- final String fullGaugeName = composeTableGaugeName(tableName, String.valueOf(partitionId), gauge);
-
- AtomicLong gaugeValue = _gaugeValues.get(fullGaugeName);
- return gaugeValue == null ? -1 : gaugeValue.get();
Review Comment:
@jugomezv Is there a particular reason why we put the default value as `-1`, not `0`?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
jugomezv commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1072694982
##########
pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java:
##########
@@ -0,0 +1,128 @@
+/**
+ * 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.pinot.common.metrics;
+
+import com.yammer.metrics.core.MetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerMetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerSettableGauge;
+import org.apache.pinot.spi.metrics.PinotMetric;
+
+
+public class MetricValueUtils {
+ private MetricValueUtils() {
+ }
+
+ public static boolean gaugeExists(AbstractMetrics metrics, String metricName) {
+ return extractMetric(metrics, metricName) != null;
+ }
+
+ public static long getGaugeValue(AbstractMetrics metrics, String metricName) {
+ PinotMetric pinotMetric = extractMetric(metrics, metricName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName()) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName());
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + key) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean partitionGaugeExists(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId) != null;
+ }
+
+ public static long getPartitionGaugeValue(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ private static PinotMetric extractMetric(AbstractMetrics metrics, String metricName) {
+ String metricPrefix;
+ Class metricClass;
+ if (metrics instanceof ControllerMetrics) {
+ metricPrefix = "pinot.controller.";
+ metricClass = ControllerMetrics.class;
+ } else if (metrics instanceof BrokerMetrics) {
+ metricPrefix = "pinot.broker.";
+ metricClass = BrokerMetrics.class;
+ } else if (metrics instanceof ServerMetrics) {
+ metricPrefix = "pinot.server.";
+ metricClass = ServerMetrics.class;
+ } else if (metrics instanceof MinionMetrics) {
+ metricPrefix = "pinot.minion.";
+ metricClass = MinionMetrics.class;
+ } else {
+ throw new RuntimeException("unsupported AbstractMetrics type");
Review Comment:
Can the message include the type?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
jugomezv commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1070086597
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ControllerPeriodicTasksIntegrationTest.java:
##########
@@ -206,8 +207,8 @@ public void testSegmentStatusChecker()
int numTables = 5;
ControllerMetrics controllerMetrics = _controllerStarter.getControllerMetrics();
TestUtils.waitForCondition(aVoid -> {
- if (controllerMetrics
- .getValueOfGlobalGauge(ControllerGauge.PERIODIC_TASK_NUM_TABLES_PROCESSED, "SegmentStatusChecker")
+ if (MetricValueUtils.getGaugeValue(controllerMetrics,
+ ControllerGauge.PERIODIC_TASK_NUM_TABLES_PROCESSED.getGaugeName() + ".SegmentStatusChecker")
Review Comment:
Similar comment here, it is best to leave this as a task name argument, at least then you can enforce naming that sticks to some convention there and not let anybody who adds new stuff come up with a new way to concatenate names across the code
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] zhtaoxiang commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
zhtaoxiang commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1072961954
##########
pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java:
##########
@@ -0,0 +1,128 @@
+/**
+ * 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.pinot.common.metrics;
+
+import com.yammer.metrics.core.MetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerMetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerSettableGauge;
+import org.apache.pinot.spi.metrics.PinotMetric;
+
+
+public class MetricValueUtils {
+ private MetricValueUtils() {
+ }
+
+ public static boolean gaugeExists(AbstractMetrics metrics, String metricName) {
+ return extractMetric(metrics, metricName) != null;
+ }
+
+ public static long getGaugeValue(AbstractMetrics metrics, String metricName) {
+ PinotMetric pinotMetric = extractMetric(metrics, metricName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName()) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName());
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + key) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean partitionGaugeExists(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId) != null;
+ }
+
+ public static long getPartitionGaugeValue(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ private static PinotMetric extractMetric(AbstractMetrics metrics, String metricName) {
+ String metricPrefix;
+ Class metricClass;
+ if (metrics instanceof ControllerMetrics) {
+ metricPrefix = "pinot.controller.";
+ metricClass = ControllerMetrics.class;
+ } else if (metrics instanceof BrokerMetrics) {
+ metricPrefix = "pinot.broker.";
+ metricClass = BrokerMetrics.class;
+ } else if (metrics instanceof ServerMetrics) {
+ metricPrefix = "pinot.server.";
+ metricClass = ServerMetrics.class;
+ } else if (metrics instanceof MinionMetrics) {
+ metricPrefix = "pinot.minion.";
+ metricClass = MinionMetrics.class;
+ } else {
+ throw new RuntimeException("unsupported AbstractMetrics type");
Review Comment:
added
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] zhtaoxiang commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
zhtaoxiang commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1070549659
##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/periodictask/ControllerPeriodicTaskTest.java:
##########
@@ -128,7 +130,8 @@ public void testControllerPeriodicTaskCalls() {
assertTrue(_processTablesCalled.get());
assertEquals(_tablesProcessed.get(), _numTables);
assertEquals(
- _controllerMetrics.getValueOfGlobalGauge(ControllerGauge.PERIODIC_TASK_NUM_TABLES_PROCESSED, TASK_NAME),
+ MetricValueUtils.getGaugeValue(_controllerMetrics,
Review Comment:
thanks for the feedback!
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] zhtaoxiang commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
zhtaoxiang commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1080869461
##########
pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java:
##########
@@ -0,0 +1,128 @@
+/**
+ * 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.pinot.common.metrics;
+
+import com.yammer.metrics.core.MetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerMetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerSettableGauge;
+import org.apache.pinot.spi.metrics.PinotMetric;
+
+
+public class MetricValueUtils {
+ private MetricValueUtils() {
+ }
+
+ public static boolean gaugeExists(AbstractMetrics metrics, String metricName) {
+ return extractMetric(metrics, metricName) != null;
+ }
+
+ public static long getGaugeValue(AbstractMetrics metrics, String metricName) {
+ PinotMetric pinotMetric = extractMetric(metrics, metricName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName()) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName());
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + key) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean partitionGaugeExists(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId) != null;
+ }
+
+ public static long getPartitionGaugeValue(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId);
+ if (pinotMetric == null) {
Review Comment:
This `getValueOfPartitionGauge` is only used in unit tests and integration tests, as long as we don't break those tests, it should be fine even if we don't return `-1` in the new method
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java:
##########
@@ -481,20 +461,6 @@ public long getValueOfTableGauge(final String tableName, final G gauge) {
return gaugeValue == null ? 0 : gaugeValue.get();
}
- /**
- * Gets the value of a table partition gauge.
- *
- * @param tableName The table name
- * @param partitionId The partition name
- * @param gauge The gauge to use
- */
- public long getValueOfPartitionGauge(final String tableName, final int partitionId, final G gauge) {
- final String fullGaugeName = composeTableGaugeName(tableName, String.valueOf(partitionId), gauge);
-
- AtomicLong gaugeValue = _gaugeValues.get(fullGaugeName);
- return gaugeValue == null ? -1 : gaugeValue.get();
Review Comment:
This getValueOfPartitionGauge is only used in unit tests and integration tests, as long as we don't break those tests, it should be fine even if we don't return -1 in the new method
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] snleee merged pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
snleee merged PR #10106:
URL: https://github.com/apache/pinot/pull/10106
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
jugomezv commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1081597788
##########
pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java:
##########
@@ -0,0 +1,128 @@
+/**
+ * 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.pinot.common.metrics;
+
+import com.yammer.metrics.core.MetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerMetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerSettableGauge;
+import org.apache.pinot.spi.metrics.PinotMetric;
+
+
+public class MetricValueUtils {
+ private MetricValueUtils() {
+ }
+
+ public static boolean gaugeExists(AbstractMetrics metrics, String metricName) {
+ return extractMetric(metrics, metricName) != null;
+ }
+
+ public static long getGaugeValue(AbstractMetrics metrics, String metricName) {
+ PinotMetric pinotMetric = extractMetric(metrics, metricName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName()) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName());
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + key) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean partitionGaugeExists(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId) != null;
+ }
+
+ public static long getPartitionGaugeValue(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId);
+ if (pinotMetric == null) {
Review Comment:
This return -1 is strange but @zhtaoxiang is right on this being used only on testing, just nit would be marked as visible for testing only as suggested above
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
jugomezv commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1081595797
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java:
##########
@@ -481,20 +461,6 @@ public long getValueOfTableGauge(final String tableName, final G gauge) {
return gaugeValue == null ? 0 : gaugeValue.get();
}
- /**
- * Gets the value of a table partition gauge.
- *
- * @param tableName The table name
- * @param partitionId The partition name
- * @param gauge The gauge to use
- */
- public long getValueOfPartitionGauge(final String tableName, final int partitionId, final G gauge) {
- final String fullGaugeName = composeTableGaugeName(tableName, String.valueOf(partitionId), gauge);
-
- AtomicLong gaugeValue = _gaugeValues.get(fullGaugeName);
- return gaugeValue == null ? -1 : gaugeValue.get();
Review Comment:
So should this be Visible for testing only?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10106:
URL: https://github.com/apache/pinot/pull/10106#issuecomment-1378507927
# [Codecov](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10106](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e5a29eb) into [master](https://codecov.io/gh/apache/pinot/commit/8022c792f10c997335f510f1eea426ffc27468b5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8022c79) will **decrease** coverage by `45.74%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10106 +/- ##
=============================================
- Coverage 70.41% 24.66% -45.75%
+ Complexity 5745 44 -5701
=============================================
Files 1992 1980 -12
Lines 107981 107623 -358
Branches 16415 16374 -41
=============================================
- Hits 76031 26545 -49486
- Misses 26630 78306 +51676
+ Partials 5320 2772 -2548
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `24.66% <0.00%> (+0.01%)` | :arrow_up: |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==) | `78.53% <ø> (-2.78%)` | :arrow_down: |
| [...org/apache/pinot/spi/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `0.00% <0.00%> (-90.13%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/spi/utils/LoopUtils.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvTG9vcFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/user/RoleType.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3VzZXIvUm9sZVR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/stream/StreamMessage.java](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvc3RyZWFtL1N0cmVhbU1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1472 more](https://codecov.io/gh/apache/pinot/pull/10106?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
jugomezv commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1070076100
##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/periodictask/ControllerPeriodicTaskTest.java:
##########
@@ -128,7 +130,8 @@ public void testControllerPeriodicTaskCalls() {
assertTrue(_processTablesCalled.get());
assertEquals(_tablesProcessed.get(), _numTables);
assertEquals(
- _controllerMetrics.getValueOfGlobalGauge(ControllerGauge.PERIODIC_TASK_NUM_TABLES_PROCESSED, TASK_NAME),
+ MetricValueUtils.getGaugeValue(_controllerMetrics,
Review Comment:
Should we add a function named MetricValueUtils.getTaskGaugeValue(), seems like a common use case over controller code.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] zhtaoxiang commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
zhtaoxiang commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1081607958
##########
pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java:
##########
@@ -0,0 +1,128 @@
+/**
+ * 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.pinot.common.metrics;
+
+import com.yammer.metrics.core.MetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerMetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerSettableGauge;
+import org.apache.pinot.spi.metrics.PinotMetric;
+
+
+public class MetricValueUtils {
+ private MetricValueUtils() {
+ }
+
+ public static boolean gaugeExists(AbstractMetrics metrics, String metricName) {
+ return extractMetric(metrics, metricName) != null;
+ }
+
+ public static long getGaugeValue(AbstractMetrics metrics, String metricName) {
+ PinotMetric pinotMetric = extractMetric(metrics, metricName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName()) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName());
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + key) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean partitionGaugeExists(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId) != null;
+ }
+
+ public static long getPartitionGaugeValue(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId);
+ if (pinotMetric == null) {
Review Comment:
The code is defined under `src/test` path, so it's by default visible to tests only
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] snleee commented on a diff in pull request #10106: clean up the logic of reading guage metrics
Posted by GitBox <gi...@apache.org>.
snleee commented on code in PR #10106:
URL: https://github.com/apache/pinot/pull/10106#discussion_r1071949894
##########
pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java:
##########
@@ -0,0 +1,128 @@
+/**
+ * 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.pinot.common.metrics;
+
+import com.yammer.metrics.core.MetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerMetricName;
+import org.apache.pinot.plugin.metrics.yammer.YammerSettableGauge;
+import org.apache.pinot.spi.metrics.PinotMetric;
+
+
+public class MetricValueUtils {
+ private MetricValueUtils() {
+ }
+
+ public static boolean gaugeExists(AbstractMetrics metrics, String metricName) {
+ return extractMetric(metrics, metricName) != null;
+ }
+
+ public static long getGaugeValue(AbstractMetrics metrics, String metricName) {
+ PinotMetric pinotMetric = extractMetric(metrics, metricName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName()) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName());
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean globalGaugeExists(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + key) != null;
+ }
+
+ public static long getGlobalGaugeValue(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key) != null;
+ }
+
+ public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, String key,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key);
+ if (pinotMetric == null) {
+ return 0;
+ }
+ return ((YammerSettableGauge<Long>) pinotMetric.getMetric()).value();
+ }
+
+ public static boolean partitionGaugeExists(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId) != null;
+ }
+
+ public static long getPartitionGaugeValue(AbstractMetrics metrics, String tableName, int partitionId,
+ AbstractMetrics.Gauge gauge) {
+ PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId);
+ if (pinotMetric == null) {
Review Comment:
The previous implementation for `getValueOfPartitionGauge()` returned `-1`. Should we keep the backward compatibility..?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org