Merge pull request #25280 from vespa-engine/fix-dimension-hashing-bugv8.101.28

Fix dimension hashing bug

10 files changed, 228 insertions, 83 deletions

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java
index ebb5d2fe8fb..641e771aafa 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java
@@ -75,9 +75,9 @@ public class VespaMetrics {
             systemCheck.ifPresent(metricsPackets::add);
 
             MetricAggregator aggregator = new MetricAggregator(service.getDimensions());
-            MetricsParser.Consumer metricsConsumer = (consumerId != null)
-                    ? new GetServiceMetricsConsumer(metricsConsumers, aggregator, consumerId)
-                    : new GetServiceMetricsConsumerForAll(metricsConsumers, aggregator);
+            MetricsParser.Collector metricsConsumer = (consumerId != null)
+                    ? new ServiceMetricsCollector(metricsConsumers, aggregator, consumerId)
+                    : new ServiceMetricsCollectorForAll(metricsConsumers, aggregator);
             service.consumeMetrics(metricsConsumer);
 
             if (! aggregator.getAggregated().isEmpty()) {
@@ -118,10 +118,10 @@ public class VespaMetrics {
      * In order to include a metric, it must exist in the given map of metric to consumers.
      * Each returned metric will contain a collection of consumers that it should be routed to.
      */
-    private static abstract class GetServiceMetricsConsumerBase implements MetricsParser.Consumer {
+    private static abstract class ServiceMetricsCollectorBase implements MetricsParser.Collector {
         protected final MetricAggregator aggregator;
 
-        GetServiceMetricsConsumerBase(MetricAggregator aggregator) {
+        ServiceMetricsCollectorBase(MetricAggregator aggregator) {
             this.aggregator = aggregator;
         }
 
@@ -150,18 +150,18 @@ public class VespaMetrics {
         }
     }
 
-    private static class GetServiceMetricsConsumer extends GetServiceMetricsConsumerBase {
+    private static class ServiceMetricsCollector extends ServiceMetricsCollectorBase {
         private final Map<MetricId, ConfiguredMetric> configuredMetrics;
         private final Set<ConsumerId> consumerId;
 
-        GetServiceMetricsConsumer(MetricsConsumers metricsConsumers, MetricAggregator aggregator, ConsumerId consumerId) {
+        ServiceMetricsCollector(MetricsConsumers metricsConsumers, MetricAggregator aggregator, ConsumerId consumerId) {
             super(aggregator);
             this.consumerId = Set.of(consumerId);
             this.configuredMetrics = metricsConsumers.getMetricsForConsumer(consumerId);
         }
 
         @Override
-        public void consume(Metric candidate) {
+        public void accept(Metric candidate) {
             ConfiguredMetric configuredMetric = configuredMetrics.get(candidate.getName());
             if (configuredMetric != null) {
                 aggregator.aggregate(
@@ -170,16 +170,16 @@ public class VespaMetrics {
         }
     }
 
-    private static class GetServiceMetricsConsumerForAll extends GetServiceMetricsConsumerBase {
+    private static class ServiceMetricsCollectorForAll extends ServiceMetricsCollectorBase {
         private final MetricsConsumers metricsConsumers;
 
-        GetServiceMetricsConsumerForAll(MetricsConsumers metricsConsumers, MetricAggregator aggregator) {
+        ServiceMetricsCollectorForAll(MetricsConsumers metricsConsumers, MetricAggregator aggregator) {
             super(aggregator);
             this.metricsConsumers = metricsConsumers;
         }
 
         @Override
-        public void consume(Metric candidate) {
+        public void accept(Metric candidate) {
             Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric = metricsConsumers.getConsumersByMetric(candidate.getName());
             if (consumersByMetric != null) {
                 consumersByMetric.keySet().forEach(
@@ -234,11 +234,11 @@ public class VespaMetrics {
                 .statusMessage("Data collected successfully");
     }
 
-    private class MetricStringBuilder implements MetricsParser.Consumer {
+    private class MetricStringBuilder implements MetricsParser.Collector {
         private final StringBuilder sb = new StringBuilder();
         private VespaService service;
         @Override
-        public void consume(Metric metric) {
+        public void accept(Metric metric) {
             MetricId key = metric.getName();
             MetricId alias = key;
 
@@ -272,7 +272,7 @@ public class VespaMetrics {
         return msb.toString();
     }
 
-    private class MetricNamesBuilder implements MetricsParser.Consumer {
+    private class MetricNamesBuilder implements MetricsParser.Collector {
         private final StringBuilder bufferOn = new StringBuilder();
         private final StringBuilder bufferOff = new StringBuilder();
         private final ConsumerId consumer;
@@ -280,7 +280,7 @@ public class VespaMetrics {
             this.consumer = consumer;
         }
         @Override
-        public void consume(Metric m) {
+        public void accept(Metric m) {
             String description = m.getDescription();
             MetricId alias = MetricId.empty;
             boolean isForwarded = false;
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/DummyMetricsFetcher.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/DummyMetricsFetcher.java
index db7caa84bac..3d5ced63ab9 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/DummyMetricsFetcher.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/DummyMetricsFetcher.java
@@ -19,6 +19,6 @@ public class DummyMetricsFetcher extends RemoteMetricsFetcher {
     /**
      * Connect to remote service over http and fetch metrics
      */
-    public void getMetrics(MetricsParser.Consumer consumer, int fetchCount) {
+    public void getMetrics(MetricsParser.Collector consumer, int fetchCount) {
     }
 }
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 bb21485e0e7..e8f6459a1bc 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
@@ -13,8 +13,10 @@ import java.io.InputStream;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId;
@@ -25,20 +27,22 @@ import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId;
  * @author Jo Kristian Bergum
  */
 public class MetricsParser {
-    public interface Consumer {
-        void consume(Metric metric);
+    public interface Collector {
+        void accept(Metric metric);
     }
 
     private static final ObjectMapper jsonMapper = new ObjectMapper();
 
-    public static void parse(String data, Consumer consumer) throws IOException {
+    public static void parse(String data, Collector consumer) throws IOException {
         parse(jsonMapper.createParser(data), consumer);
     }
 
-    static void parse(InputStream data, Consumer consumer) throws IOException {
+    static void parse(InputStream data, Collector consumer) throws IOException {
         parse(jsonMapper.createParser(data), consumer);
     }
-    private static void parse(JsonParser parser, Consumer consumer) throws IOException {
+
+    // Top level 'metrics' object, with e.g. 'time', 'status' and 'metrics'.
+    private static void parse(JsonParser parser, Collector consumer) throws IOException {
         if (parser.nextToken() != JsonToken.START_OBJECT) {
             throw new IOException("Expected start of object, got " + parser.currentToken());
         }
@@ -55,6 +59,7 @@ public class MetricsParser {
             }
         }
     }
+
     static private Instant parseSnapshot(JsonParser parser) throws IOException {
         if (parser.getCurrentToken() != JsonToken.START_OBJECT) {
             throw new IOException("Expected start of 'snapshot' object, got " + parser.currentToken());
@@ -75,59 +80,97 @@ public class MetricsParser {
         return timestamp;
     }
 
-    static private void parseMetricValues(JsonParser parser, Instant timestamp, Consumer consumer) throws IOException {
+    // 'metrics' object with 'snapshot' and 'values' arrays
+    static private void parseMetrics(JsonParser parser, Collector consumer) throws IOException {
+        if (parser.getCurrentToken() != JsonToken.START_OBJECT) {
+            throw new IOException("Expected start of 'metrics' object, got " + parser.currentToken());
+        }
+        Instant timestamp = Instant.now();
+        for (parser.nextToken(); parser.getCurrentToken() != JsonToken.END_OBJECT; parser.nextToken()) {
+            String fieldName = parser.getCurrentName();
+            JsonToken token = parser.nextToken();
+            if (fieldName.equals("snapshot")) {
+                timestamp = parseSnapshot(parser);
+            } else if (fieldName.equals("values")) {
+                parseMetricValues(parser, timestamp, consumer);
+            } else {
+                if (token == JsonToken.START_OBJECT || token == JsonToken.START_ARRAY) {
+                    parser.skipChildren();
+                }
+            }
+        }
+    }
+
+    // 'values' array
+    static private void parseMetricValues(JsonParser parser, Instant timestamp, Collector consumer) throws IOException {
         if (parser.getCurrentToken() != JsonToken.START_ARRAY) {
             throw new IOException("Expected start of 'metrics:values' array, got " + parser.currentToken());
         }
 
-        Map<Long, Map<DimensionId, String>> uniqueDimensions = new HashMap<>();
+        Map<Set<Dimension>, Map<DimensionId, String>> uniqueDimensions = new HashMap<>();
         while (parser.nextToken() == JsonToken.START_OBJECT) {
             handleValue(parser, timestamp, consumer, uniqueDimensions);
         }
     }
 
-    static private void parseMetrics(JsonParser parser, Consumer consumer) throws IOException {
-        if (parser.getCurrentToken() != JsonToken.START_OBJECT) {
-            throw new IOException("Expected start of 'metrics' object, got " + parser.currentToken());
-        }
-        Instant timestamp = Instant.now();
+    // One item in the 'values' array, where each item has 'name', 'values' and 'dimensions'
+    static private void handleValue(JsonParser parser, Instant timestamp, Collector consumer,
+                                    Map<Set<Dimension>, Map<DimensionId, String>> uniqueDimensions) throws IOException {
+        String name = "";
+        String description = "";
+        Map<DimensionId, String> dim = Map.of();
+        List<Map.Entry<String, Number>> values = List.of();
         for (parser.nextToken(); parser.getCurrentToken() != JsonToken.END_OBJECT; parser.nextToken()) {
             String fieldName = parser.getCurrentName();
             JsonToken token = parser.nextToken();
-            if (fieldName.equals("snapshot")) {
-                timestamp = parseSnapshot(parser);
+            if (fieldName.equals("name")) {
+                name = parser.getText();
+            } else if (fieldName.equals("description")) {
+                description = parser.getText();
+            } else if (fieldName.equals("dimensions")) {
+                dim = parseDimensions(parser, uniqueDimensions);
             } else if (fieldName.equals("values")) {
-                parseMetricValues(parser, timestamp, consumer);
+                values = parseValues(name+".", parser);
             } else {
                 if (token == JsonToken.START_OBJECT || token == JsonToken.START_ARRAY) {
                     parser.skipChildren();
                 }
             }
         }
+        for (Map.Entry<String, Number> value : values) {
+            consumer.accept(new Metric(MetricId.toMetricId(value.getKey()), value.getValue(), timestamp, dim, description));
+        }
     }
 
     private static Map<DimensionId, String> parseDimensions(JsonParser parser,
-                                                            Map<Long, Map<DimensionId, String>> uniqueDimensions) throws IOException {
-        List<Map.Entry<String, String>> dims = new ArrayList<>();
-        int keyHash = 0;
-        int valueHash = 0;
+                                                            Map<Set<Dimension>, Map<DimensionId, String>> uniqueDimensions) throws IOException {
+
+        Set<Dimension> dimensions = new HashSet<>();
+
         for (parser.nextToken(); parser.getCurrentToken() != JsonToken.END_OBJECT; parser.nextToken()) {
             String fieldName = parser.getCurrentName();
             JsonToken token = parser.nextToken();
+
             if (token == JsonToken.VALUE_STRING){
                 String value = parser.getValueAsString();
-                dims.add(Map.entry(fieldName, value));
-                keyHash ^= fieldName.hashCode();
-                valueHash ^= value.hashCode();
+                dimensions.add(Dimension.of(fieldName, value));
             } else if (token == JsonToken.VALUE_NULL) {
                 // TODO Should log a warning if this happens
             } else {
                 throw new IllegalArgumentException("Dimension '" + fieldName + "' must be a string");
             }
         }
-        Long uniqueKey = (((long) keyHash) << 32) | (valueHash & 0xffffffffL);
-        return uniqueDimensions.computeIfAbsent(uniqueKey, key -> dims.stream().collect(Collectors.toUnmodifiableMap(e -> toDimensionId(e.getKey()), Map.Entry::getValue)));
+        return uniqueDimensions.computeIfAbsent(dimensions,
+                                                key -> dimensions.stream().collect(Collectors.toUnmodifiableMap(
+                                                        dim -> toDimensionId(dim.id), dim -> dim.value)));
+    }
+
+    record Dimension(String id, String value) {
+        static Dimension of(String id, String value) {
+            return new Dimension(id, value);
+        }
     }
+
     private static List<Map.Entry<String, Number>> parseValues(String prefix, JsonParser parser) throws IOException {
         List<Map.Entry<String, Number>> metrics = new ArrayList<>();
         for (parser.nextToken(); parser.getCurrentToken() != JsonToken.END_OBJECT; parser.nextToken()) {
@@ -144,31 +187,5 @@ public class MetricsParser {
         }
         return metrics;
     }
-    static private void handleValue(JsonParser parser, Instant timestamp, Consumer consumer,
-                                    Map<Long, Map<DimensionId, String>> uniqueDimensions) throws IOException {
-        String name = "";
-        String description = "";
-        Map<DimensionId, String> dim = Map.of();
-        List<Map.Entry<String, Number>> values = List.of();
-        for (parser.nextToken(); parser.getCurrentToken() != JsonToken.END_OBJECT; parser.nextToken()) {
-            String fieldName = parser.getCurrentName();
-            JsonToken token = parser.nextToken();
-            if (fieldName.equals("name")) {
-                name = parser.getText();
-            } else if (fieldName.equals("description")) {
-                description = parser.getText();
-            } else if (fieldName.equals("dimensions")) {
-                dim = parseDimensions(parser, uniqueDimensions);
-            } else if (fieldName.equals("values")) {
-                values = parseValues(name+".", parser);
-            } else {
-                if (token == JsonToken.START_OBJECT || token == JsonToken.START_ARRAY) {
-                    parser.skipChildren();
-                }
-            }
-        }
-        for (Map.Entry<String, Number> value : values) {
-            consumer.consume(new Metric(MetricId.toMetricId(value.getKey()), value.getValue(), timestamp, dim, description));
-        }
-    }
+
 }
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 cd05945f4f1..7a0cca26afe 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
@@ -24,7 +24,7 @@ public class RemoteMetricsFetcher extends HttpMetricFetcher {
     /**
      * Connect to remote service over http and fetch metrics
      */
-    public void getMetrics(MetricsParser.Consumer consumer, int fetchCount) {
+    public void getMetrics(MetricsParser.Collector consumer, int fetchCount) {
         try (CloseableHttpResponse response = getResponse()) {
             HttpEntity entity = response.getEntity();
             try {
@@ -37,7 +37,7 @@ public class RemoteMetricsFetcher extends HttpMetricFetcher {
         } catch (IOException ignored) {}
     }
 
-    void createMetrics(String data, MetricsParser.Consumer consumer, int fetchCount) throws IOException {
+    void createMetrics(String data, MetricsParser.Collector consumer, int fetchCount) throws IOException {
         MetricsParser.parse(data, consumer);
     }
 }
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/VespaService.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/VespaService.java
index 666fa31b7ed..8dd8d002c84 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/VespaService.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/VespaService.java
@@ -137,15 +137,15 @@ public class VespaService implements Comparable<VespaService> {
      * Get the Metrics registered for this service. Metrics are fetched over HTTP
      * if a metric http port has been defined, otherwise from log file
      */
-    public void consumeMetrics(MetricsParser.Consumer consumer) {
+    public void consumeMetrics(MetricsParser.Collector consumer) {
         remoteMetricsFetcher.getMetrics(consumer, metricsFetchCount.get());
         metricsFetchCount.getAndIncrement();
     }
 
-    private static class CollectMetrics implements MetricsParser.Consumer {
+    private static class CollectMetrics implements MetricsParser.Collector {
         private final Metrics metrics = new Metrics();
         @Override
-        public void consume(Metric metric) {
+        public void accept(Metric metric) {
             metrics.add(metric);
         }
     }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DownService.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DownService.java
index 7b76c2fbd37..ccd1351f2d5 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DownService.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DownService.java
@@ -2,7 +2,6 @@
 package ai.vespa.metricsproxy.service;
 
 import ai.vespa.metricsproxy.metric.HealthMetric;
-import ai.vespa.metricsproxy.metric.Metrics;
 
 /**
  * @author gjoranv
@@ -18,7 +17,7 @@ public class DownService extends VespaService {
     }
 
     @Override
-    public void consumeMetrics(MetricsParser.Consumer consumer) {
+    public void consumeMetrics(MetricsParser.Collector consumer) {
     }
 
    @Override
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DummyService.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DummyService.java
index b7b41f4a4f8..b4607bd4376 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DummyService.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/DummyService.java
@@ -20,10 +20,10 @@ public class DummyService extends VespaService {
     }
 
     @Override
-    public void consumeMetrics(MetricsParser.Consumer consumer) {
+    public void consumeMetrics(MetricsParser.Collector consumer) {
         long timestamp = System.currentTimeMillis() / 1000;
-        consumer.consume(new Metric(MetricId.toMetricId(METRIC_1), 5 * num + 1, timestamp));
-        consumer.consume(new Metric(MetricId.toMetricId(METRIC_2), 1.3 * num + 1.05, timestamp));
+        consumer.accept(new Metric(MetricId.toMetricId(METRIC_1), 5 * num + 1, timestamp));
+        consumer.accept(new Metric(MetricId.toMetricId(METRIC_2), 1.3 * num + 1.05, timestamp));
     }
 
 }
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 5d4512276d4..a252543ca1d 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
@@ -21,18 +21,18 @@ public class MetricsFetcherTest {
 
     private static final int port = 9;  //port number is not used in this test
 
-    private static class MetricsConsumer implements MetricsParser.Consumer {
+    private static class MetricsCollector implements MetricsParser.Collector {
         Metrics metrics = new Metrics();
         @Override
-        public void consume(Metric metric) {
+        public void accept(Metric metric) {
             metrics.add(metric);
         }
     }
     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);
-        return consumer.metrics;
+        MetricsCollector collector = new MetricsCollector();
+        fetcher.createMetrics(data, collector, 0);
+        return collector.metrics;
     }
 
     @Test
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsParserTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsParserTest.java
new file mode 100644
index 00000000000..17dc9e643f0
--- /dev/null
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsParserTest.java
@@ -0,0 +1,129 @@
+package ai.vespa.metricsproxy.service;
+
+import ai.vespa.metricsproxy.metric.Metric;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+/**
+ * @author gjoranv
+ */
+public class MetricsParserTest {
+
+    private static class MetricsCollector implements MetricsParser.Collector {
+        List<Metric> metrics = new ArrayList<>();
+
+        @Override
+        public void accept(Metric metric) {
+            metrics.add(metric);
+        }
+    }
+
+    @Test
+    public void different_dimension_values_are_not_treated_as_equal() throws Exception {
+        var collector = new MetricsCollector();
+        MetricsParser.parse(metricsJsonDistinctButDuplicateDimensionDalues(), collector);
+        assertEquals(2, collector.metrics.size());
+        assertNotEquals("Dimensions should not be equal",
+                        collector.metrics.get(0).getDimensions(),
+                        collector.metrics.get(1).getDimensions());
+    }
+
+    @Test
+    public void different_swapped_dimension_values_are_not_treated_as_equal() throws Exception {
+        var collector = new MetricsCollector();
+        MetricsParser.parse(metricsJsonDistinctButDuplicateSwappedDimensionDalues(), collector);
+        assertEquals(2, collector.metrics.size());
+        assertNotEquals("Dimensions should not be equal",
+                collector.metrics.get(0).getDimensions(),
+                collector.metrics.get(1).getDimensions());
+    }
+
+    // The duplicate dimension values for 'cluster' and 'clusterid' exposed a bug in a previously used hashing algo for dimensions.
+    private String metricsJsonDistinctButDuplicateDimensionDalues() {
+        return """
+                {
+                  "time": 1671035366573,
+                  "status": {
+                    "code": "up"
+                  },
+                  "metrics": {
+                    "snapshot": {
+                      "from": 1671035306.562,
+                      "to": 1671035366.562
+                    },
+                    "values": [
+                      {
+                        "name": "cluster-controller.resource_usage.nodes_above_limit",
+                        "values": {
+                          "last": 1.0
+                        },
+                        "dimensions": {
+                          "controller-index": "0",
+                          "cluster": "CLUSTER-1",
+                          "clusterid": "CLUSTER-1"
+                        }
+                      },
+                      {
+                        "name": "cluster-controller.resource_usage.nodes_above_limit",
+                        "values": {
+                          "last": 2.0
+                        },
+                        "dimensions": {
+                          "controller-index": "0",
+                          "cluster": "CLUSTER-2",
+                          "clusterid": "CLUSTER-2"
+                        }
+                      }
+                    ]
+                  }
+                }
+                """;
+    }
+    // The duplicate dimension values for 'cluster' and 'clusterid' exposed a bug in a previously used hashing algo for dimensions.
+    private String metricsJsonDistinctButDuplicateSwappedDimensionDalues() {
+        return """
+                {
+                  "time": 1671035366573,
+                  "status": {
+                    "code": "up"
+                  },
+                  "metrics": {
+                    "snapshot": {
+                      "from": 1671035306.562,
+                      "to": 1671035366.562
+                    },
+                    "values": [
+                      {
+                        "name": "cluster-controller.resource_usage.nodes_above_limit",
+                        "values": {
+                          "last": 1.0
+                        },
+                        "dimensions": {
+                          "controller-index": "0",
+                          "cluster": "CLUSTER-1",
+                          "clusterid": "CLUSTER-2"
+                        }
+                      },
+                      {
+                        "name": "cluster-controller.resource_usage.nodes_above_limit",
+                        "values": {
+                          "last": 2.0
+                        },
+                        "dimensions": {
+                          "controller-index": "0",
+                          "cluster": "CLUSTER-2",
+                          "clusterid": "CLUSTER-1"
+                        }
+                      }
+                    ]
+                  }
+                }
+                """;
+    }
+
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcher.java
index 8f06a7bb30a..5d3d8d40ad4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcher.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsV2MetricsFetcher.java
@@ -60,7 +60,7 @@ public class MetricsV2MetricsFetcher extends AbstractComponent implements Metric
             return CompletableFuture.completedFuture(MetricsResponse.empty());
         }
         else {
-            // Consumer 'autoscaling' defined in com.yahoo.vespa.model.admin.monitoring.MetricConsumer
+            // Collector 'autoscaling' defined in com.yahoo.vespa.model.admin.monitoring.MetricConsumer
             String url = "http://" + metricsV2Container.get().hostname() + ":" + 4080 + apiPath + "?consumer=autoscaling";
             return httpClient.get(url)
                              .thenApply(response -> new MetricsResponse(response, applicationNodes, nodeRepository));