mlbiscoc commented on code in PR #3719:
URL: https://github.com/apache/solr/pull/3719#discussion_r2394827660
##########
solr/core/src/java/org/apache/solr/core/ZkContainer.java:
##########
@@ -213,23 +213,24 @@ public void initializeMetrics(
"solr_zk_cumulative_multi_ops_total",
"Total cumulative multi-operations count",
measurement -> {
-
measurement.record(metricsListener.getCumulativeMultiOps());
+
measurement.record(metricsListener.getCumulativeMultiOps(), attributes);
}));
observables.add(
ctx.observableLongCounter(
"solr_zk_child_fetches",
"Total number of ZooKeeper child node fetches",
measurement -> {
-
measurement.record(metricsListener.getChildFetches());
+
measurement.record(metricsListener.getChildFetches(), attributes);
}));
observables.add(
ctx.observableLongCounter(
"solr_zk_cumulative_children_fetched",
"Total cumulative children fetched count",
measurement -> {
-
measurement.record(metricsListener.getCumulativeChildrenFetched());
+ measurement.record(
+ metricsListener.getCumulativeChildrenFetched(),
attributes);
Review Comment:
Luckily there was a test that helped me catch it! `testZkMetrics`
##########
solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java:
##########
Review Comment:
Yeah I agree now looking at it. Will address in another PR
##########
solr/core/src/java/org/apache/solr/metrics/OtelRuntimeJvmMetrics.java:
##########
@@ -27,15 +27,17 @@
public class OtelRuntimeJvmMetrics {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
- // Main feature flag to enable/disable all JVM metrics
- private static final String JVM_METRICS_ENABLED = "solr.metrics.jvm.enabled";
-
private RuntimeMetrics runtimeMetrics;
private boolean isInitialized = false;
+ // Main feature flag to enable/disable all JVM metrics
+ public static boolean isJvmMetricsEnabled() {
+ return EnvUtils.getPropertyAsBool("solr.metrics.jvm.enabled", true);
+ }
+
public OtelRuntimeJvmMetrics initialize(
SolrMetricManager solrMetricManager, String registryName) {
- if (!EnvUtils.getPropertyAsBool(JVM_METRICS_ENABLED, true)) {
+ if (!isJvmMetricsEnabled()) {
log.info("JVM metrics collection is disabled");
Review Comment:
I found it kind of useful and it was a one liner but I'm fine with removing
it.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
