From cc857482b85c46d08de6eebce243ebe3a43de544 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 17 Dec 2021 22:03:07 +0100 Subject: Handle that metrics can arrive with a dimension set to null. Skip and log it as debug. Let exceptions through in the tests so that we do not hide errors there. --- .../vespa/metricsproxy/service/MetricsParser.java | 17 ++++++++-- .../metricsproxy/service/RemoteMetricsFetcher.java | 8 ++--- .../metricsproxy/service/MetricsFetcherTest.java | 36 +++++++++++++++------- 3 files changed, 41 insertions(+), 20 deletions(-) (limited to 'metrics-proxy') diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java index f9443c46bad..701af8730d7 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java @@ -14,6 +14,8 @@ import java.io.InputStream; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.logging.Level; +import java.util.logging.Logger; import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; @@ -23,6 +25,7 @@ import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; * @author Jo Kristian Bergum */ public class MetricsParser { + private final static Logger log = Logger.getLogger(MetricsParser.class.getName()); public interface Consumer { void consume(Metric metric); } @@ -128,15 +131,23 @@ public class MetricsParser { StringBuilder sb = new StringBuilder(); for (Iterator it = dimensions.fieldNames(); it.hasNext(); ) { String k = (String) it.next(); - String v = dimensions.get(k).asText(); - sb.append(toDimensionId(k)).append(v); + String v = dimensions.get(k).textValue(); + if (v != null) { + sb.append(toDimensionId(k)).append(v); + } } if ( ! uniqueDimensions.containsKey(sb.toString())) { dim = new HashMap<>(); for (Iterator it = dimensions.fieldNames(); it.hasNext(); ) { String k = (String) it.next(); String v = dimensions.get(k).textValue(); - dim.put(toDimensionId(k), v); + if (v != null) { + dim.put(toDimensionId(k), v); + } else { + // TODO This should never happen, but it has been seen. This should be flagged as warning, + // but will try to find root cause before flooding the log. + log.log(Level.FINE, "Metric '" + name + "': dimension '" + k + "' is null"); + } } uniqueDimensions.put(sb.toString(), Map.copyOf(dim)); } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java index f375fd1586f..5ca9e6fd950 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java @@ -28,12 +28,8 @@ public class RemoteMetricsFetcher extends HttpMetricFetcher { } } - void createMetrics(String data, MetricsParser.Consumer consumer, int fetchCount) { - try { - MetricsParser.parse(data, consumer); - } catch (Exception e) { - handleException(e, data, fetchCount); - } + void createMetrics(String data, MetricsParser.Consumer consumer, int fetchCount) throws IOException { + MetricsParser.parse(data, consumer); } private void createMetrics(InputStream data, MetricsParser.Consumer consumer, int fetchCount) throws IOException { try { diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java index b25d3af2c20..56e78e3930c 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java @@ -6,24 +6,27 @@ import ai.vespa.metricsproxy.metric.Metric; import ai.vespa.metricsproxy.metric.Metrics; import org.junit.Test; +import java.io.IOException; + import static ai.vespa.metricsproxy.metric.model.MetricId.toMetricId; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; /** */ public class MetricsFetcherTest { private static final int port = 9; //port number is not used in this test - private static final double EPSILON = 0.00000000001; - private class MetricsConsumer implements MetricsParser.Consumer { + private static class MetricsConsumer implements MetricsParser.Consumer { Metrics metrics = new Metrics(); @Override public void consume(Metric metric) { metrics.add(metric); } } - Metrics fetch(String data) { + Metrics fetch(String data) throws IOException { RemoteMetricsFetcher fetcher = new RemoteMetricsFetcher(new DummyService(0, "dummy/id/0"), port); MetricsConsumer consumer = new MetricsConsumer(); fetcher.createMetrics(data, consumer, 0); @@ -31,7 +34,7 @@ public class MetricsFetcherTest { } @Test - public void testStateFormatMetricsParse() { + public void testStateFormatMetricsParse() throws IOException { String jsonData = TestUtil.getFileContents("metrics-state.json"); Metrics metrics = fetch(jsonData); assertEquals(10, metrics.size()); @@ -41,20 +44,25 @@ public class MetricsFetcherTest { } @Test - public void testEmptyJson() { + public void testEmptyJson() throws IOException { String jsonData = "{}"; Metrics metrics = fetch(jsonData); assertEquals(0, metrics.size()); } @Test - public void testErrors() { + public void testErrors() throws IOException { String jsonData; - Metrics metrics; + Metrics metrics = null; jsonData = ""; - metrics = fetch(jsonData); - assertEquals(0, metrics.size()); + try { + metrics = fetch(jsonData); + fail("Should have an IOException instead"); + } catch (IOException e) { + assertEquals("Expected start of object, got null", e.getMessage()); + } + assertNull(metrics); jsonData = "{\n" + "\"status\" : {\n" + @@ -91,8 +99,14 @@ public class MetricsFetcherTest { "}\n" + "}"; - metrics = fetch(jsonData); - assertEquals(0, metrics.size()); + metrics = null; + try { + metrics = fetch(jsonData); + fail("Should have an IOException instead"); + } catch (IllegalArgumentException e) { + assertEquals("Value for aggregator 'count' is not a number", e.getMessage()); + } + assertNull(metrics); } public Metric getMetric(String metric, Metrics metrics) { -- cgit v1.2.3