Allow adding metrics to the default consumer, for model amenders.

8 files changed, 70 insertions, 24 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
index e0956097c84..e03873515d6 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
@@ -68,7 +68,7 @@ public class Admin extends AbstractConfigProducer implements Serializable {
      // Cluster of logserver containers. If enabled, exactly one container is running on each logserver host.
     private Optional<LogserverContainerCluster> logServerContainerCluster = Optional.empty();
 
-    // Cluster of metricsproxy containers. Exactly one container is set up on all hosts.
+    // Cluster of metrics-proxy containers. Exactly one container is set up on all hosts.
     private MetricsProxyContainerCluster metricsProxyContainerCluster;
 
     private ZooKeepersConfigProvider zooKeepersConfigProvider;
@@ -98,6 +98,8 @@ public class Admin extends AbstractConfigProducer implements Serializable {
 
     public Metrics getUserMetrics() { return metrics; }
 
+    public MetricsProxyContainerCluster getMetricsProxyContainerCluster() {return metricsProxyContainerCluster;}
+
     /** Returns a list of all config servers */
     public List<Configserver> getConfigservers() {
         return configservers;
@@ -204,6 +206,8 @@ public class Admin extends AbstractConfigProducer implements Serializable {
     }
 
     private void addMetricsProxyCluster(List<HostResource> hosts, DeployState deployState) {
+        if (metricsProxyContainerCluster != null) throw new RuntimeException("Metrics proxy cluster is already added.");
+
         var metricsProxyCluster = new MetricsProxyContainerCluster(this, "metrics", deployState);
         int index = 0;
         for (var host : hosts) {
@@ -211,6 +215,7 @@ public class Admin extends AbstractConfigProducer implements Serializable {
             addAndInitializeService(deployState.getDeployLogger(), host, container);
             metricsProxyCluster.addContainer(container);
         }
+        metricsProxyContainerCluster = metricsProxyCluster;
     }
 
     private void addCommonServices(HostResource host, DeployState deployState) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/ConsumersConfigGenerator.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/ConsumersConfigGenerator.java
index 2df4008214a..fe40e6ffa84 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/ConsumersConfigGenerator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/ConsumersConfigGenerator.java
@@ -30,10 +30,11 @@ class ConsumersConfigGenerator {
      * @param userConsumers The consumers set up by the user in services.xml
      * @return A list of consumer builders (a mapping from consumer to its metrics)
      */
-    static List<Consumer.Builder> generate(Map<String, MetricsConsumer> userConsumers) {
+    static List<Consumer.Builder> generateConsumers(MetricsConsumer defaultConsumer,
+                                                    Map<String, MetricsConsumer> userConsumers) {
         // Normally, the user given consumers should not contain VESPA_CONSUMER_ID, but it's allowed for some internally used applications.
         var allConsumers = new LinkedHashMap<>(userConsumers);
-        allConsumers.put(VESPA_CONSUMER_ID, combineConsumers(getDefaultMetricsConsumer(), allConsumers.get(VESPA_CONSUMER_ID)));
+        allConsumers.put(VESPA_CONSUMER_ID, combineConsumers(defaultConsumer, allConsumers.get(VESPA_CONSUMER_ID)));
 
         return allConsumers.values().stream()
                 .map(ConsumersConfigGenerator::toConsumerBuilder)
@@ -52,7 +53,9 @@ class ConsumersConfigGenerator {
         return addMetrics(original, overriding.getMetrics());
     }
 
-    private static MetricsConsumer addMetrics(MetricsConsumer original, Map<String, Metric> metrics) {
+    static MetricsConsumer addMetrics(MetricsConsumer original, Map<String, Metric> metrics) {
+        if (metrics == null) return original;
+
         Map<String, Metric> combinedMetrics = new LinkedHashMap<>(original.getMetrics());
         metrics.forEach((name, newMetric) ->
                                 combinedMetrics.put(name, combineMetrics(original.getMetrics().get(name), newMetric)));
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
index d9a969ab7c9..19c1ae4aa92 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
@@ -22,6 +22,7 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.admin.Admin;
+import com.yahoo.vespa.model.admin.monitoring.MetricSet;
 import com.yahoo.vespa.model.admin.monitoring.MetricsConsumer;
 import com.yahoo.vespa.model.admin.monitoring.Monitoring;
 import com.yahoo.vespa.model.admin.monitoring.builder.Metrics;
@@ -34,12 +35,16 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.logging.Logger;
 
+import static com.yahoo.vespa.model.admin.metricsproxy.ConsumersConfigGenerator.addMetrics;
+import static com.yahoo.vespa.model.admin.metricsproxy.ConsumersConfigGenerator.generateConsumers;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.APPLICATION;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.APPLICATION_ID;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.INSTANCE;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.LEGACY_APPLICATION;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.TENANT;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.ZONE;
+import static com.yahoo.vespa.model.admin.monitoring.DefaultMetricsConsumer.getDefaultMetricsConsumer;
+import static com.yahoo.vespa.model.admin.monitoring.MetricSet.emptyMetricSet;
 import static com.yahoo.vespa.model.container.xml.BundleMapper.JarSuffix.JAR_WITH_DEPS;
 import static com.yahoo.vespa.model.container.xml.BundleMapper.bundlePathFromName;
 
@@ -74,6 +79,8 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC
     private final AbstractConfigProducer<?> parent;
     private final ApplicationId applicationId;
 
+    private MetricSet additionalDefaultMetrics = emptyMetricSet();
+
     public MetricsProxyContainerCluster(AbstractConfigProducer<?> parent, String name, DeployState deployState) {
         super(parent, name, name, deployState);
         this.parent = parent;
@@ -97,6 +104,12 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC
         addMetricsProxyComponent(VespaMetrics.class);
     }
 
+    @SuppressWarnings("WeakerAccess") // Used by amenders
+    public void setAdditionalDefaultMetrics(MetricSet additionalDefaultMetrics) {
+        if (additionalDefaultMetrics == null) return;
+        this.additionalDefaultMetrics = additionalDefaultMetrics;
+    }
+
     @Override
     protected void doPrepare(DeployState deployState) { }
 
@@ -108,7 +121,8 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC
 
     @Override
     public void getConfig(ConsumersConfig.Builder builder) {
-        builder.consumer.addAll(ConsumersConfigGenerator.generate(getUserMetricsConsumers()));
+        var amendedDefaultConsumer = addMetrics(getDefaultMetricsConsumer(), additionalDefaultMetrics.getMetrics());
+        builder.consumer.addAll(generateConsumers(amendedDefaultConsumer, getUserMetricsConsumers()));
     }
 
     @Override
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/DefaultMetricsConsumer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/DefaultMetricsConsumer.java
index 67abe302335..e9eca67a55a 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/DefaultMetricsConsumer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/DefaultMetricsConsumer.java
@@ -25,7 +25,6 @@ public class DefaultMetricsConsumer {
                                                                                            systemMetricSet,
                                                                                            networkMetricSet));
 
-    @SuppressWarnings("UnusedDeclaration")
     public static MetricsConsumer getDefaultMetricsConsumer() {
         return new MetricsConsumer(VESPA_CONSUMER_ID, defaultConsumerMetrics);
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricSet.java
index f9b59d4f32a..d879a6f445d 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricSet.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricSet.java
@@ -37,6 +37,10 @@ public class MetricSet {
         this(id, metrics, Collections.emptySet());
     }
 
+    public static MetricSet emptyMetricSet() {
+        return new MetricSet("empty", Collections.emptySet());
+    }
+
     public final String getId() { return id; }
 
     /**
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricsConsumer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricsConsumer.java
index ec1f66b418f..529ed6ecf67 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricsConsumer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricsConsumer.java
@@ -3,11 +3,13 @@ package com.yahoo.vespa.model.admin.monitoring;
 
 import javax.annotation.concurrent.Immutable;
 import java.util.Map;
+import java.util.Objects;
 
 /**
  * Represents an arbitrary metric consumer
  *
  * @author trygve
+ * @author gjoranv
  */
 @Immutable
 public class MetricsConsumer {
@@ -19,8 +21,8 @@ public class MetricsConsumer {
      * @param metricSet  The metrics for this consumer
      */
     public MetricsConsumer(String id, MetricSet metricSet) {
-        this.id = id;
-        this.metricSet = metricSet;
+        this.id = Objects.requireNonNull(id, "A consumer must have a non-null id.");;
+        this.metricSet = Objects.requireNonNull(metricSet, "A consumer must have a non-null metric set.");
     }
 
     public String getId() {
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 59a72aedaaf..665e36c1760 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
@@ -10,24 +10,29 @@ import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames;
 import com.yahoo.vespa.model.admin.monitoring.Metric;
+import com.yahoo.vespa.model.admin.monitoring.MetricSet;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.zoneString;
+import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.CLUSTER_CONFIG_ID;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.MY_APPLICATION;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.MY_INSTANCE;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.MY_TENANT;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.checkMetric;
+import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.consumersConfigFromModel;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getApplicationDimensionsConfig;
-import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getConsumersConfig;
+import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.consumersConfigFromXml;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getCustomConsumer;
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getHostedModel;
+import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getModel;
 import static com.yahoo.vespa.model.admin.monitoring.DefaultMetricsConsumer.VESPA_CONSUMER_ID;
 import static com.yahoo.vespa.model.admin.monitoring.DefaultVespaMetrics.defaultVespaMetricSet;
 import static com.yahoo.vespa.model.admin.monitoring.NetworkMetrics.networkMetricSet;
 import static com.yahoo.vespa.model.admin.monitoring.SystemMetrics.systemMetricSet;
 import static com.yahoo.vespa.model.admin.monitoring.VespaMetricSet.vespaMetricSet;
+import static java.util.Collections.singleton;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
@@ -48,9 +53,23 @@ public class MetricsProxyContainerClusterTest {
 
     @Test
     public void default_consumer_is_always_present_and_has_all_vespa_metrics_and_all_system_metrics() {
-        ConsumersConfig consumersConfig = getConsumersConfig(servicesWithAdminOnly());
-        assertEquals(consumersConfig.consumer(0).name(), VESPA_CONSUMER_ID);
-        assertEquals(numMetricsForDefaultConsumer, consumersConfig.consumer(0).metric().size());
+        ConsumersConfig config = consumersConfigFromXml(servicesWithAdminOnly());
+        assertEquals(config.consumer(0).name(), VESPA_CONSUMER_ID);
+        assertEquals(numMetricsForDefaultConsumer, config.consumer(0).metric().size());
+    }
+
+    @Test
+    public void default_consumer_can_be_amended() {
+        VespaModel model = getModel(servicesWithAdminOnly());
+        var cluster = model.getAdmin().getMetricsProxyContainerCluster();
+        var additionalMetric = new Metric("additional-metric");
+        cluster.setAdditionalDefaultMetrics(new MetricSet("amender-metrics", singleton(additionalMetric)));
+
+        ConsumersConfig config = consumersConfigFromModel(model);
+        assertEquals(numMetricsForDefaultConsumer + 1, config.consumer(0).metric().size());
+
+        ConsumersConfig.Consumer vespaConsumer = config.consumer(0);
+        assertTrue("Did not contain additional metric", checkMetric(vespaConsumer, additionalMetric));
     }
 
     @Test
@@ -67,7 +86,7 @@ public class MetricsProxyContainerClusterTest {
         );
         thrown.expect(IllegalArgumentException.class);
         thrown.expectMessage("'Vespa' is not allowed as metrics consumer id");
-        getConsumersConfig(services);
+        consumersConfigFromXml(services);
     }
 
     @Test
@@ -84,7 +103,7 @@ public class MetricsProxyContainerClusterTest {
                 "    </admin>",
                 "</services>"
         );
-        ConsumersConfig config = getConsumersConfig(services);
+        ConsumersConfig config = consumersConfigFromXml(services);
         assertEquals(1, config.consumer().size());
 
         // All default metrics are retained
@@ -110,7 +129,7 @@ public class MetricsProxyContainerClusterTest {
         );
         thrown.expect(IllegalArgumentException.class);
         thrown.expectMessage("'a' is used as id for two metrics consumers");
-        getConsumersConfig(services);
+        consumersConfigFromXml(services);
     }
 
     @Test
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java
index 3e5c8a6ef0d..7fdd0205b9c 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java
@@ -28,8 +28,10 @@ class MetricsProxyModelTester {
     static final String MY_INSTANCE = "myinstance";
     static final String MY_FLAVOR = "myflavor";
 
+    static final String CLUSTER_CONFIG_ID = "admin/metrics";
+
     // Used for all configs that are produced by the container, not the cluster.
-    static final String CONTAINER_CONFIG_ID = "admin/metrics/0";
+    static final String CONTAINER_CONFIG_ID = CLUSTER_CONFIG_ID + "/0";
 
     static VespaModel getModel(String servicesXml) {
         var numberOfHosts = 1;
@@ -59,7 +61,7 @@ class MetricsProxyModelTester {
     }
 
     static ConsumersConfig.Consumer getCustomConsumer(String servicesXml) {
-        ConsumersConfig config = getConsumersConfig(servicesXml);
+        ConsumersConfig config = consumersConfigFromXml(servicesXml);
         assertEquals(2, config.consumer().size());
         for (ConsumersConfig.Consumer consumer : config.consumer()) {
             if (! consumer.name().equals(VESPA_CONSUMER_ID))
@@ -68,18 +70,16 @@ class MetricsProxyModelTester {
         throw new RuntimeException("Two consumers with the reserved id - this cannot happen.");
     }
 
-    static ConsumersConfig getConsumersConfig(String servicesXml) {
-        return getConsumersConfig(getModel(servicesXml));
+    static ConsumersConfig consumersConfigFromXml(String servicesXml) {
+        return consumersConfigFromModel(getModel(servicesXml));
     }
 
-    private static ConsumersConfig getConsumersConfig(VespaModel model) {
-        String configId = "admin/metrics";
-        return new ConsumersConfig((ConsumersConfig.Builder) model.getConfig(new ConsumersConfig.Builder(), configId));
+    static ConsumersConfig consumersConfigFromModel(VespaModel model) {
+        return new ConsumersConfig((ConsumersConfig.Builder) model.getConfig(new ConsumersConfig.Builder(), CLUSTER_CONFIG_ID));
     }
 
     static ApplicationDimensionsConfig getApplicationDimensionsConfig(VespaModel model) {
-        String configId = "admin/metrics";
-        return new ApplicationDimensionsConfig((ApplicationDimensionsConfig.Builder) model.getConfig(new ApplicationDimensionsConfig.Builder(), configId));
+        return new ApplicationDimensionsConfig((ApplicationDimensionsConfig.Builder) model.getConfig(new ApplicationDimensionsConfig.Builder(), CLUSTER_CONFIG_ID));
     }
 
     static NodeDimensionsConfig getNodeDimensionsConfig(VespaModel model) {