dsmiley commented on code in PR #3719:
URL: https://github.com/apache/solr/pull/3719#discussion_r2392723815
##########
solr/core/src/java/org/apache/solr/update/UpdateLog.java:
##########
@@ -668,11 +662,14 @@ public void initializeMetrics(
toClose = Collections.unmodifiableList(observables);
- // NOCOMMIT: Do we want to keep this? Metric was just state with the
numeric enum value.
- // Without context this doesn't mean anything and can be very confusing.
Maybe keep the numeric
- // value and put the value meaning in the
- // description?
- // solrMetricsContext.gauge(() -> state.getValue(), true, "state",
scope);
+ solrMetricsContext.gauge(() -> state.getValue(), true, "state", scope);
+ observables.add(
+ solrMetricsContext.observableLongGauge(
+ "solr_core_update_log_state",
+ "The current state of the update log. Replaying (0), buffering
(1), applying buffered (2), active (3)",
Review Comment:
great description
##########
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 wonder if this is log-worthy. If someone disables something, they know
what they are doing and don't need to see a log about it.
##########
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:
a sneaky mistake any of us could make...
##########
solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java:
##########
Review Comment:
an observation: the types here show that the metric being counted is
fundamentally different -- may count docs, or tlog bytes, or a time unit. This
strongly suggest we should have separate metrics.
--
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]
