diff options
author | gjoranv <gv@verizonmedia.com> | 2020-02-12 12:07:57 +0100 |
---|---|---|
committer | gjoranv <gv@verizonmedia.com> | 2020-02-12 12:07:57 +0100 |
commit | 280f17e3d62cc1973fc14c0fefdc19c7e6fdc85b (patch) | |
tree | 9214f4b7aff3ba42af215cba9aa5ec6c69285eb9 | |
parent | b0e5e2de5a2d2909c9f2a7668a449063a63149d2 (diff) |
Add test for public default metric set and non-existent set.
+ Fix bug that allowed adding an unknown (null) metric set.
2 files changed, 49 insertions, 1 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/MetricsBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/MetricsBuilder.java index b13fa4917e4..f029dad01a9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/MetricsBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/MetricsBuilder.java @@ -62,7 +62,7 @@ public class MetricsBuilder { .collect(Collectors.toCollection(LinkedList::new)); List<MetricSet> metricSets = XML.getChildren(consumerElement, "metric-set").stream() - .map(metricSetElement -> availableMetricSets.get(metricSetElement.getAttribute(ID_ATTRIBUTE))) + .map(metricSetElement -> getMetricSetOrThrow(metricSetElement.getAttribute(ID_ATTRIBUTE))) .collect(Collectors.toCollection(LinkedList::new)); metricSets.add(defaultVespaMetricSet); @@ -75,6 +75,11 @@ public class MetricsBuilder { return "user-metrics-" + consumerName; } + private MetricSet getMetricSetOrThrow(String id) { + if (! availableMetricSets.containsKey(id)) throw new IllegalArgumentException("No such metric-set: " + id); + return availableMetricSets.get(id); + } + private void throwIfIllegalConsumerId(Metrics metrics, String consumerId) { if (consumerId.equalsIgnoreCase(VESPA_CONSUMER_ID) && applicationType != ApplicationType.HOSTED_INFRASTRUCTURE) throw new IllegalArgumentException("'Vespa' is not allowed as metrics consumer id (case is ignored.)"); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java index de40e557265..9265e4437f1 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java @@ -67,6 +67,7 @@ import static org.junit.Assert.assertTrue; */ public class MetricsProxyContainerClusterTest { + private static int numPublicDefaultMetrics = defaultPublicMetricSet.getMetrics().size(); private static int numDefaultVespaMetrics = defaultVespaMetricSet.getMetrics().size(); private static int numVespaMetrics = vespaMetricSet.getMetrics().size(); private static int numSystemMetrics = systemMetricSet.getMetrics().size(); @@ -238,6 +239,25 @@ public class MetricsProxyContainerClusterTest { } @Test + public void non_existent_metric_set_causes_exception() { + String services = String.join("\n", + "<services>", + " <admin version='2.0'>", + " <adminserver hostalias='node1'/>", + " <metrics>", + " <consumer id='consumer-with-non-existent-default-set'>", + " <metric-set id='non-existent'/>", + " </consumer>", + " </metrics>", + " </admin>", + "</services>" + ); + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("No such metric-set: non-existent"); + consumersConfigFromXml(services, self_hosted); + } + + @Test public void consumer_with_no_metric_set_has_its_own_metrics_plus_system_metrics_plus_default_vespa_metrics() { String services = String.join("\n", "<services>", @@ -263,6 +283,29 @@ public class MetricsProxyContainerClusterTest { } @Test + public void consumer_with_default_public_metric_set_has_all_public_metrics_plus_all_system_metrics_plus_its_own() { + String services = String.join("\n", + "<services>", + " <admin version='2.0'>", + " <adminserver hostalias='node1'/>", + " <metrics>", + " <consumer id='consumer-with-public-default-set'>", + " <metric-set id='public'/>", + " <metric id='custom.metric'/>", + " </consumer>", + " </metrics>", + " </admin>", + "</services>" + ); + ConsumersConfig.Consumer consumer = getCustomConsumer(services); + + assertEquals(numPublicDefaultMetrics + numSystemMetrics + 1, consumer.metric().size()); + + Metric customMetric = new Metric("custom.metric"); + assertTrue("Did not contain metric: " + customMetric, checkMetric(consumer, customMetric)); + } + + @Test public void consumer_with_vespa_metric_set_has_all_vespa_metrics_plus_all_system_metrics_plus_its_own() { String services = String.join("\n", "<services>", |