Replace dimension allowlisting with blocklist in /metrics/v2

4 files changed, 81 insertions, 40 deletions

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java
index 3f93cdda61c..1d7f7000c5a 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java
@@ -41,8 +41,6 @@ public class NodeMetricsClient {
 
     private static final Logger log = Logger.getLogger(NodeMetricsClient.class.getName());
 
-    private static final int MAX_DIMENSIONS = 10;
-
     final Node node;
     private final CloseableHttpAsyncClient httpClient;
     private final Clock clock;
@@ -89,7 +87,7 @@ public class NodeMetricsClient {
         var metrics = processAndBuild(GenericJsonUtil.toMetricsPackets(respons),
                 new ServiceIdDimensionProcessor(),
                 new ClusterIdDimensionProcessor(),
-                new PublicDimensionsProcessor(MAX_DIMENSIONS));
+                new PublicDimensionsProcessor());
         snapshotsRetrieved.incrementAndGet();
         log.log(FINE, () -> "Successfully retrieved " + metrics.size() + " metrics packets from " + metricsUri);
         snapshots.put(consumer, new Snapshot(Instant.now(clock), metrics));
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessor.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessor.java
index 50eb74e8693..639f642778e 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessor.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessor.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package ai.vespa.metricsproxy.http.application;
 
+import ai.vespa.metricsproxy.metric.dimensions.BlocklistDimensions;
 import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions;
 import ai.vespa.metricsproxy.metric.model.DimensionId;
 import ai.vespa.metricsproxy.metric.model.MetricsPacket;
@@ -12,53 +13,23 @@ import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
- * Ensures that only whitelisted dimensions are retained in the given metrics packet, and that
- * there are no more dimensions than the given maximum number.
+ * Ensures that blocklisted dimensions are removed from the metric set
  *
  * @author gjoranv
  */
 public class PublicDimensionsProcessor implements MetricsProcessor {
 
-    private final int maxDimensions;
-    private Set<DimensionId> publicDimensions = getPublicDimensions();
-
-    public PublicDimensionsProcessor(int maxDimensions) {
-        int numCommonDimensions = PublicDimensions.commonDimensions.size();
-        if (numCommonDimensions > maxDimensions) {
-            throw new IllegalArgumentException(String.format(
-                    ("The maximum number of dimensions (%d) cannot be smaller than the number of " +
-                             "common metrics dimensions (%d)."), maxDimensions, numCommonDimensions));
-        }
-        this.maxDimensions = maxDimensions;
-    }
+    private final Set<DimensionId> blocklistDimensions = getBlocklistDimensions();
 
     @Override
     public void process(MetricsPacket.Builder builder) {
         Set<DimensionId> dimensionsToRetain = builder.getDimensionIds();
-        dimensionsToRetain.retainAll(publicDimensions);
-
-        if (dimensionsToRetain.size() > maxDimensions) {
-            for (var metricDim : getMetricDimensions()) {
-                dimensionsToRetain.remove(metricDim);
-                if (dimensionsToRetain.size() <= maxDimensions) break;
-            }
-        }
-
+        dimensionsToRetain.removeAll(blocklistDimensions);
         builder.retainDimensions(dimensionsToRetain);
-
-        // Extra safeguard, to make sure we don't exceed the limit of some metric systems.
-        if (builder.getDimensionIds().size() > maxDimensions) {
-            throw new IllegalStateException(String.format(
-                    "Metrics packet is only allowed to have %d dimensions, but has: %s", maxDimensions, builder.getDimensionIds()));
-        }
-    }
-
-    static Set<DimensionId> getPublicDimensions() {
-        return toDimensionIds(PublicDimensions.publicDimensions);
     }
 
-    static Set<DimensionId> getMetricDimensions() {
-        return toDimensionIds(PublicDimensions.metricDimensions);
+    static Set<DimensionId> getBlocklistDimensions() {
+        return toDimensionIds(BlocklistDimensions.blocklistDimensions);
     }
 
     static Set<DimensionId> toDimensionIds(Collection<String> dimensionNames) {
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java
index 339f06c67b8..1aa7244363b 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java
@@ -40,7 +40,6 @@ public class MetricsV2Handler  extends HttpHandlerBase {
 
     public static final String V2_PATH = "/metrics/v2";
     public static final String VALUES_PATH = V2_PATH + "/values";
-    private static final int MAX_DIMENSIONS = 10;
 
     private final ValuesFetcher valuesFetcher;
     private final NodeInfoConfig nodeInfoConfig;
@@ -68,7 +67,7 @@ public class MetricsV2Handler  extends HttpHandlerBase {
             List<MetricsPacket> metrics = processAndBuild(valuesFetcher.fetchMetricsAsBuilders(consumer),
                                                           new ServiceIdDimensionProcessor(),
                                                           new ClusterIdDimensionProcessor(),
-                                                          new PublicDimensionsProcessor(MAX_DIMENSIONS));
+                                                          new PublicDimensionsProcessor());
 
             Node localNode = new Node(nodeInfoConfig.role(), nodeInfoConfig.hostname(), 0, "");
             Map<Node, List<MetricsPacket>> metricsByNode = Map.of(localNode, metrics);
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/dimensions/BlocklistDimensions.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/dimensions/BlocklistDimensions.java
new file mode 100644
index 00000000000..5e72594759a
--- /dev/null
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/dimensions/BlocklistDimensions.java
@@ -0,0 +1,73 @@
+package ai.vespa.metricsproxy.metric.dimensions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * @author olaa
+ */
+public class BlocklistDimensions {
+
+    private BlocklistDimensions() {}
+
+    /**
+     * Deployment related metrics - most of which are redundant
+     * E.g. app/applicationName/tenantName/instanceName is already included in applicationId
+     */
+    public static final String APP = "app";
+    public static final String APPLICATION_NAME = "applicationName";
+    public static final String CLUSTER_NAME = "clustername";
+    public static final String CLUSTER_ID = "clusterid";
+    public static final String CLUSTER_TYPE = "clustertype";
+    public static final String DEPLOYMENT_CLUSTER = "deploymentCluster";
+    public static final String GROUP_ID = "groupId";
+    public static final String INSTANCE = "instance";
+    public static final String INSTANCE_NAME = "instanceName";
+    public static final String TENANT_NAME = "tenantName";
+
+    /**
+     * State related dimensions - will always be the same value for a given snapshot
+     */
+    public static final String METRIC_TYPE = "metrictype";
+    public static final String ORCHESTRATOR_STATE = "orchestratorState";
+    public static final String ROLE = "role";
+    public static final String STATE = "state";
+    public static final String SYSTEM = "system";
+    public static final String VESPA_VERSION = "vespaVersion";
+
+    /**  Metric specific dimensions  **/
+    public static final String ARCHITECTURE = "arch";
+    public static final String AUTHZ_REQUIRED = "authz-required";
+    public static final String HOME = "home";
+    public static final String PORT = "port";
+    public static final String SCHEME = "scheme";
+    public static final String DRYRUN = "dryrun";
+    public static final String VERSION = "version";
+
+    public static final List<String> blocklistDimensions = List.of(
+            APP,
+            APPLICATION_NAME,
+            CLUSTER_NAME,
+            CLUSTER_ID,
+            CLUSTER_TYPE,
+            DEPLOYMENT_CLUSTER,
+            GROUP_ID,
+            INSTANCE,
+            INSTANCE_NAME,
+            TENANT_NAME,
+            METRIC_TYPE,
+            ORCHESTRATOR_STATE,
+            ROLE,
+            STATE,
+            SYSTEM,
+            VESPA_VERSION,
+            ARCHITECTURE,
+            AUTHZ_REQUIRED,
+            HOME,
+            PORT,
+            SCHEME,
+            DRYRUN,
+            VERSION
+    );
+
+}