diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 0dd9f188196..587c74d6803 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -50,6 +50,8 @@ Improvements properly regardless of the mode (standalone, distributed). The API has been stripped of ancient, unused, interfaces and simplified. (Dawid Weiss) +* SOLR-14683: Metrics API should ensure consistent placeholders for missing values. (ab) + Other Changes ---------------------- * SOLR-14656: Autoscaling framework removed (Ishan Chattopadhyaya, noble, Ilan Ginzburg) diff --git a/solr/core/src/java/org/apache/solr/core/MetricsConfig.java b/solr/core/src/java/org/apache/solr/core/MetricsConfig.java index cdaa56d7a2c..17449fd1636 100644 --- a/solr/core/src/java/org/apache/solr/core/MetricsConfig.java +++ b/solr/core/src/java/org/apache/solr/core/MetricsConfig.java @@ -32,13 +32,18 @@ public class MetricsConfig { private final PluginInfo timerSupplier; private final PluginInfo histogramSupplier; private final PluginInfo historyHandler; + private final Object nullNumber; + private final Object notANumber; + private final Object nullString; + private final Object nullObject; private final boolean enabled; private MetricsConfig(boolean enabled, PluginInfo[] metricReporters, Set hiddenSysProps, PluginInfo counterSupplier, PluginInfo meterSupplier, PluginInfo timerSupplier, PluginInfo histogramSupplier, - PluginInfo historyHandler) { + PluginInfo historyHandler, + Object nullNumber, Object notANumber, Object nullString, Object nullObject) { this.enabled = enabled; this.metricReporters = metricReporters; this.hiddenSysProps = hiddenSysProps; @@ -47,6 +52,10 @@ public class MetricsConfig { this.timerSupplier = timerSupplier; this.histogramSupplier = histogramSupplier; this.historyHandler = historyHandler; + this.nullNumber = nullNumber; + this.notANumber = notANumber; + this.nullString = nullString; + this.nullObject = nullObject; } public boolean isEnabled() { @@ -63,6 +72,22 @@ public class MetricsConfig { } } + public Object getNullNumber() { + return nullNumber; + } + + public Object getNotANumber() { + return notANumber; + } + + public Object getNullString() { + return nullString; + } + + public Object getNullObject() { + return nullObject; + } + public Set getHiddenSysProps() { if (enabled) { return hiddenSysProps; @@ -127,6 +152,10 @@ public class MetricsConfig { private PluginInfo timerSupplier; private PluginInfo histogramSupplier; private PluginInfo historyHandler; + private Object nullNumber = null; + private Object notANumber = null; + private Object nullString = null; + private Object nullObject = null; // default to metrics enabled private boolean enabled = true; @@ -177,9 +206,30 @@ public class MetricsConfig { return this; } + public MetricsConfigBuilder setNullNumber(Object nullNumber) { + this.nullNumber = nullNumber; + return this; + } + + public MetricsConfigBuilder setNotANumber(Object notANumber) { + this.notANumber = notANumber; + return this; + } + + public MetricsConfigBuilder setNullString(Object nullString) { + this.nullString = nullString; + return this; + } + + public MetricsConfigBuilder setNullObject(Object nullObject) { + this.nullObject = nullObject; + return this; + } + public MetricsConfig build() { return new MetricsConfig(enabled, metricReporterPlugins, hiddenSysProps, counterSupplier, meterSupplier, - timerSupplier, histogramSupplier, historyHandler); + timerSupplier, histogramSupplier, historyHandler, + nullNumber, notANumber, nullString, nullObject); } } diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index e3489d02160..a388bf3a3d9 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -1195,13 +1195,13 @@ public final class SolrCore implements SolrInfoBean, Closeable { newSearcherMaxReachedCounter = parentContext.counter("maxReached", Category.SEARCHER.toString(), "new"); newSearcherOtherErrorsCounter = parentContext.counter("errors", Category.SEARCHER.toString(), "new"); - parentContext.gauge(() -> name == null ? "(null)" : name, true, "coreName", Category.CORE.toString()); + parentContext.gauge(() -> name == null ? parentContext.nullString() : name, true, "coreName", Category.CORE.toString()); parentContext.gauge(() -> startTime, true, "startTime", Category.CORE.toString()); parentContext.gauge(() -> getOpenCount(), true, "refCount", Category.CORE.toString()); parentContext.gauge(() -> getInstancePath().toString(), true, "instanceDir", Category.CORE.toString()); - parentContext.gauge(() -> isClosed() ? "(closed)" : getIndexDir(), true, "indexDir", Category.CORE.toString()); - parentContext.gauge(() -> isClosed() ? 0 : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString()); - parentContext.gauge(() -> isClosed() ? "(closed)" : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString()); + parentContext.gauge(() -> isClosed() ? parentContext.nullString() : getIndexDir(), true, "indexDir", Category.CORE.toString()); + parentContext.gauge(() -> isClosed() ? parentContext.nullNumber() : getIndexSize(), true, "sizeInBytes", Category.INDEX.toString()); + parentContext.gauge(() -> isClosed() ? parentContext.nullString() : NumberUtils.readableSize(getIndexSize()), true, "size", Category.INDEX.toString()); if (coreContainer != null) { final CloudDescriptor cd = getCoreDescriptor().getCloudDescriptor(); if (cd != null) { @@ -1209,7 +1209,7 @@ public final class SolrCore implements SolrInfoBean, Closeable { if (cd.getCollectionName() != null) { return cd.getCollectionName(); } else { - return "_notset_"; + return parentContext.nullString(); } }, true, "collection", Category.CORE.toString()); @@ -1217,7 +1217,7 @@ public final class SolrCore implements SolrInfoBean, Closeable { if (cd.getShardId() != null) { return cd.getShardId(); } else { - return "_auto_"; + return parentContext.nullString(); } }, true, "shard", Category.CORE.toString()); } diff --git a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java index e926e6d3127..69522951faf 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java +++ b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java @@ -45,6 +45,7 @@ import org.apache.solr.common.SolrException; import org.apache.solr.common.util.DOMUtil; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.PropertiesUtil; +import org.apache.solr.common.util.Utils; import org.apache.solr.logging.LogWatcherConfig; import org.apache.solr.metrics.reporters.SolrJmxReporter; import org.apache.solr.update.UpdateShardHandlerConfig; @@ -531,6 +532,15 @@ public class SolrXmlConfig { if (node != null) { builder = builder.setHistoryHandler(new PluginInfo(node, "history", false, false)); } + node = config.getNode("solr/metrics/missingValues", false);; + if (node != null) { + NamedList missingValues = DOMUtil.childNodesToNamedList(node); + builder.setNullNumber(decodeNullValue(missingValues.get("nullNumber"))); + builder.setNotANumber(decodeNullValue(missingValues.get("notANumber"))); + builder.setNullString(decodeNullValue(missingValues.get("nullString"))); + builder.setNullObject(decodeNullValue(missingValues.get("nullObject"))); + } + PluginInfo[] reporterPlugins = getMetricReporterPluginInfos(config); Set hiddenSysProps = getHiddenSysProps(config); return builder @@ -539,6 +549,20 @@ public class SolrXmlConfig { .build(); } + private static Object decodeNullValue(Object o) { + if (o instanceof String) { // check if it's a JSON object + String str = (String) o; + if (!str.isBlank() && (str.startsWith("{") || str.startsWith("["))) { + try { + o = Utils.fromJSONString((String) o); + } catch (Exception e) { + // ignore + } + } + } + return o; + } + private static PluginInfo[] getMetricReporterPluginInfos(XmlConfigFile config) { NodeList nodes = (NodeList) config.evaluate("solr/metrics/reporter", XPathConstants.NODESET); List configs = new ArrayList<>(); diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java index e41242966f7..3b136271da5 100644 --- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java @@ -883,13 +883,13 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw @Override public void initializeMetrics(SolrMetricsContext parentContext, String scope) { super.initializeMetrics(parentContext, scope); - solrMetricsContext.gauge(() -> (core != null && !core.isClosed() ? NumberUtils.readableSize(core.getIndexSize()) : ""), + solrMetricsContext.gauge(() -> (core != null && !core.isClosed() ? NumberUtils.readableSize(core.getIndexSize()) : parentContext.nullNumber()), true, "indexSize", getCategory().toString(), scope); - solrMetricsContext.gauge(() -> (core != null && !core.isClosed() ? getIndexVersion().toString() : ""), + solrMetricsContext.gauge(() -> (core != null && !core.isClosed() ? getIndexVersion().toString() : parentContext.nullString()), true, "indexVersion", getCategory().toString(), scope); - solrMetricsContext.gauge(() -> (core != null && !core.isClosed() ? getIndexVersion().generation : 0), + solrMetricsContext.gauge(() -> (core != null && !core.isClosed() ? getIndexVersion().generation : parentContext.nullNumber()), true, GENERATION, getCategory().toString(), scope); - solrMetricsContext.gauge(() -> (core != null && !core.isClosed() ? core.getIndexDir() : ""), + solrMetricsContext.gauge(() -> (core != null && !core.isClosed() ? core.getIndexDir() : parentContext.nullString()), true, "indexPath", getCategory().toString(), scope); solrMetricsContext.gauge(() -> isLeader, true, "isLeader", getCategory().toString(), scope); diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java index 7e6a933fdaa..d0ebc26c99d 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java @@ -149,6 +149,35 @@ public class SolrMetricManager { return histogramSupplier; } + /** + * Return an object used for representing a null (missing) numeric value. + */ + public Object nullNumber() { + return metricsConfig.getNullNumber(); + } + + /** + * Return an object used for representing a "Not A Number" (NaN) value. + */ + public Object notANumber() { + return metricsConfig.getNotANumber(); + } + + /** + * Return an object used for representing a null (missing) string value. + */ + public Object nullString() { + return metricsConfig.getNullString(); + } + + /** + * Return an object used for representing a null (missing) object value. + */ + public Object nullObject() { + return metricsConfig.getNullObject(); + } + + /** * An implementation of {@link MetricFilter} that selects metrics * with names that start with one of prefixes. diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java index 6bd856eb7d4..85217b9bc87 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java @@ -47,6 +47,34 @@ public class SolrMetricsContext { this.tag = tag; } + /** + * See {@link SolrMetricManager#nullNumber()}. + */ + public Object nullNumber() { + return metricManager.nullNumber(); + } + + /** + * See {@link SolrMetricManager#notANumber()}. + */ + public Object notANumber() { + return metricManager.notANumber(); + } + + /** + * See {@link SolrMetricManager#nullString()}. + */ + public Object nullString() { + return metricManager.nullString(); + } + + /** + * See {@link SolrMetricManager#nullObject()}. + */ + public Object nullObject() { + return metricManager.nullObject(); + } + /** * Metrics tag that represents objects with the same life-cycle. */ diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java index 5d1af6d1865..b5112075d43 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java +++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java @@ -2277,12 +2277,12 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI parentContext.gauge(() -> warmupTime, true, "warmupTime", Category.SEARCHER.toString(), scope); parentContext.gauge(() -> registerTime, true, "registeredAt", Category.SEARCHER.toString(), scope); // reader stats - parentContext.gauge(rgauge(-1, () -> reader.numDocs()), true, "numDocs", Category.SEARCHER.toString(), scope); - parentContext.gauge(rgauge(-1, () -> reader.maxDoc()), true, "maxDoc", Category.SEARCHER.toString(), scope); - parentContext.gauge(rgauge(-1, () -> reader.maxDoc() - reader.numDocs()), true, "deletedDocs", Category.SEARCHER.toString(), scope); - parentContext.gauge(rgauge(-1, () -> reader.toString()), true, "reader", Category.SEARCHER.toString(), scope); - parentContext.gauge(rgauge("", () -> reader.directory().toString()), true, "readerDir", Category.SEARCHER.toString(), scope); - parentContext.gauge(rgauge(-1, () -> reader.getVersion()), true, "indexVersion", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(parentContext.nullNumber(), () -> reader.numDocs()), true, "numDocs", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(parentContext.nullNumber(), () -> reader.maxDoc()), true, "maxDoc", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(parentContext.nullNumber(), () -> reader.maxDoc() - reader.numDocs()), true, "deletedDocs", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(parentContext.nullString(), () -> reader.toString()), true, "reader", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(parentContext.nullString(), () -> reader.directory().toString()), true, "readerDir", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(parentContext.nullNumber(), () -> reader.getVersion()), true, "indexVersion", Category.SEARCHER.toString(), scope); // size of the currently opened commit parentContext.gauge(() -> { try { @@ -2293,7 +2293,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI } return total; } catch (Exception e) { - return -1; + return parentContext.nullNumber(); } }, true, "indexCommitSize", Category.SEARCHER.toString(), scope); // statsCache metrics diff --git a/solr/core/src/test-files/solr/solr-metricsconfig1.xml b/solr/core/src/test-files/solr/solr-metricsconfig1.xml new file mode 100644 index 00000000000..4782e87e19f --- /dev/null +++ b/solr/core/src/test-files/solr/solr-metricsconfig1.xml @@ -0,0 +1,28 @@ + + + + + + + + -1 + + {"value":"missing"} + + + diff --git a/solr/core/src/test/org/apache/solr/metrics/MetricsConfigTest.java b/solr/core/src/test/org/apache/solr/metrics/MetricsConfigTest.java index 7ef5895b8b4..2cefedf3858 100644 --- a/solr/core/src/test/org/apache/solr/metrics/MetricsConfigTest.java +++ b/solr/core/src/test/org/apache/solr/metrics/MetricsConfigTest.java @@ -18,6 +18,7 @@ package org.apache.solr.metrics; import java.io.File; import java.io.InputStream; +import java.util.Map; import java.util.Properties; import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule; @@ -58,7 +59,7 @@ public class MetricsConfigTest extends SolrTestCaseJ4 { @Test public void testDefaults() throws Exception { - NodeConfig cfg = loadNodeConfig(); + NodeConfig cfg = loadNodeConfig("solr-metricsconfig.xml"); SolrMetricManager mgr = new SolrMetricManager(cfg.getSolrResourceLoader(), cfg.getMetricsConfig()); assertTrue(mgr.getCounterSupplier() instanceof MetricSuppliers.DefaultCounterSupplier); assertTrue(mgr.getMeterSupplier() instanceof MetricSuppliers.DefaultMeterSupplier); @@ -76,7 +77,7 @@ public class MetricsConfigTest extends SolrTestCaseJ4 { System.setProperty("histogram.size", "2048"); System.setProperty("histogram.window", "600"); System.setProperty("histogram.reservoir", SlidingTimeWindowReservoir.class.getName()); - NodeConfig cfg = loadNodeConfig(); + NodeConfig cfg = loadNodeConfig("solr-metricsconfig.xml"); SolrMetricManager mgr = new SolrMetricManager(cfg.getSolrResourceLoader(), cfg.getMetricsConfig()); assertTrue(mgr.getCounterSupplier() instanceof MetricSuppliers.DefaultCounterSupplier); assertTrue(mgr.getMeterSupplier() instanceof MetricSuppliers.DefaultMeterSupplier); @@ -94,7 +95,7 @@ public class MetricsConfigTest extends SolrTestCaseJ4 { System.setProperty("meter.class", MockMeterSupplier.class.getName()); System.setProperty("timer.class", MockTimerSupplier.class.getName()); System.setProperty("histogram.class", MockHistogramSupplier.class.getName()); - NodeConfig cfg = loadNodeConfig(); + NodeConfig cfg = loadNodeConfig("solr-metricsconfig.xml"); SolrMetricManager mgr = new SolrMetricManager(cfg.getSolrResourceLoader(), cfg.getMetricsConfig()); assertTrue(mgr.getCounterSupplier() instanceof MockCounterSupplier); assertTrue(mgr.getMeterSupplier() instanceof MockMeterSupplier); @@ -119,7 +120,7 @@ public class MetricsConfigTest extends SolrTestCaseJ4 { @Test public void testDisabledMetrics() throws Exception { System.setProperty("metricsEnabled", "false"); - NodeConfig cfg = loadNodeConfig(); + NodeConfig cfg = loadNodeConfig("solr-metricsconfig.xml"); SolrMetricManager mgr = new SolrMetricManager(cfg.getSolrResourceLoader(), cfg.getMetricsConfig()); assertTrue(mgr.getCounterSupplier() instanceof MetricSuppliers.NoOpCounterSupplier); assertTrue(mgr.getMeterSupplier() instanceof MetricSuppliers.NoOpMeterSupplier); @@ -128,8 +129,21 @@ public class MetricsConfigTest extends SolrTestCaseJ4 { } - private NodeConfig loadNodeConfig() throws Exception { - InputStream is = MetricsConfigTest.class.getResourceAsStream("/solr/solr-metricsconfig.xml"); + @Test + public void testMissingValuesConfig() throws Exception { + NodeConfig cfg = loadNodeConfig("solr-metricsconfig1.xml"); + SolrMetricManager mgr = new SolrMetricManager(cfg.getSolrResourceLoader(), cfg.getMetricsConfig()); + assertEquals("nullNumber", null, mgr.nullNumber()); + assertEquals("notANumber", -1, mgr.notANumber()); + assertEquals("nullNumber", "", mgr.nullString()); + assertTrue("nullObject", mgr.nullObject() instanceof Map); + @SuppressWarnings("unchecked") + Map map = (Map) mgr.nullObject(); + assertEquals("missing", map.get("value")); + } + + private NodeConfig loadNodeConfig(String config) throws Exception { + InputStream is = MetricsConfigTest.class.getResourceAsStream("/solr/" + config); return SolrXmlConfig.fromInputStream(TEST_PATH(), is, new Properties()); //TODO pass in props } } diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java index 96777693548..e0c9e110e2b 100644 --- a/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java @@ -116,6 +116,11 @@ public class SolrMetricsIntegrationTest extends SolrTestCaseJ4 { SolrCoreMetricManager coreMetricManager = h.getCore().getCoreMetricManager(); Map reporters = metricManager.getReporters(coreMetricManager.getRegistryName()); + Gauge gauge = (Gauge) coreMetricManager.getRegistry().getMetrics().get("CORE.indexDir"); + assertNotNull(gauge.getValue()); + h.getCore().close(); + assertEquals(metricManager.nullString(), gauge.getValue()); + deleteCore(); for (String reporterName : RENAMED_REPORTERS) { diff --git a/solr/solr-ref-guide/src/metrics-reporting.adoc b/solr/solr-ref-guide/src/metrics-reporting.adoc index 35164754b29..67203597a71 100644 --- a/solr/solr-ref-guide/src/metrics-reporting.adoc +++ b/solr/solr-ref-guide/src/metrics-reporting.adoc @@ -32,6 +32,45 @@ For each group (and/or for each registry) there can be several *reporters*, whic There is also a dedicated `/admin/metrics` handler that can be queried to report all or a subset of the current metrics from multiple registries. +=== Missing metrics +Long-lived metrics values are still reported when the underlying value is unavailable (eg. "INDEX.sizeInBytes" when +IndexReader is closed). Short-lived transient metrics (such as cache entries) that are properties of complex gauges +(internally represented as `MetricsMap`) are simply skipped when not available, and neither their names nor values +appear in registries (or in /admin/metrics reports). + +When a missing value is encountered by default it's reported as null value, regardless of the metrics type. +This can be configured in the `solr.xml:/solr/metrics/missingValues` element, which recognizes the following child elements +(for string elements a JSON payload is supported): + +`nullNumber`:: +value to use when a missing (null) numeric metrics value is encountered. + +`notANumber`:: +value to use when an invalid numeric value is encountered. + +`nullString`:: +value to use when a missing (null) string metrics is encountered. + +`nullObject`:: +value to use when a missing (null) complex object is encountered. + +Example configuration that returns null for missing numbers, -1 for +invalid numeric values, empty string for missing strings, and a Map for missing +complex objects: + +[source,xml] +---- + + + + -1 + + {"value":"missing"} + + +---- + + == Metric Registries Solr includes multiple metric registries, which group related metrics.