diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-12-18 15:42:51 +0100 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2022-12-18 15:42:51 +0100 |
commit | a8751ad4b039b958f29da98e074103ba437c50d9 (patch) | |
tree | 90015298d05eadde523518f6472b76025c88bdf6 /metrics-proxy | |
parent | 020eb72d22540f5392730fe5173ca2da9520372d (diff) |
Clean up test as implementation has changed to use a set.
Diffstat (limited to 'metrics-proxy')
-rw-r--r-- | metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java | 11 | ||||
-rw-r--r-- | metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsParserTest.java | 83 |
2 files changed, 53 insertions, 41 deletions
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 e24c7428157..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 @@ -7,11 +7,9 @@ import ai.vespa.metricsproxy.metric.model.MetricId; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.databind.ObjectMapper; -import com.yahoo.compress.Hasher; import java.io.IOException; import java.io.InputStream; -import java.nio.charset.StandardCharsets; import java.time.Instant; import java.util.ArrayList; import java.util.HashMap; @@ -167,15 +165,6 @@ public class MetricsParser { dim -> toDimensionId(dim.id), dim -> dim.value))); } - static long dimensionsHashCode(List<Dimension> dimensions) { - long hash = 0; - - for (Dimension dim : dimensions) { - hash += Hasher.xxh3(dim.id.getBytes(StandardCharsets.UTF_8)) ^ Hasher.xxh3(dim.value.getBytes(StandardCharsets.UTF_8)); - } - return hash; - } - record Dimension(String id, String value) { static Dimension of(String id, String value) { return new Dimension(id, value); 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 index 5fc260e9912..17dc9e643f0 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsParserTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsParserTest.java @@ -6,7 +6,6 @@ import org.junit.Test; import java.util.ArrayList; import java.util.List; -import static ai.vespa.metricsproxy.service.MetricsParser.dimensionsHashCode; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -25,44 +24,27 @@ public class MetricsParserTest { } @Test - public void dimensions_hashcode_is_different_for_distinct_but_duplicate_dimension_values() { - var dimensions1 = List.of( - new MetricsParser.Dimension("cluster", "CLUSTER-1"), - new MetricsParser.Dimension("clusterid", "CLUSTER-1")); - - var dimensions2 = List.of( - new MetricsParser.Dimension("cluster", "CLUSTER-2"), - new MetricsParser.Dimension("clusterid", "CLUSTER-2")); - - assertNotEquals(dimensionsHashCode(dimensions1), dimensionsHashCode(dimensions2)); - } - - @Test - public void dimensions_hashcode_is_different_for_distinct_with_swapped_dimension_values() { - var dimensions1 = List.of( - new MetricsParser.Dimension("cluster", "CLUSTER-1"), - new MetricsParser.Dimension("clusterid", "CLUSTER-2")); - - var dimensions2 = List.of( - new MetricsParser.Dimension("cluster", "CLUSTER-2"), - new MetricsParser.Dimension("clusterid", "CLUSTER-1")); - - assertNotEquals(dimensionsHashCode(dimensions1), dimensionsHashCode(dimensions2)); - } - - - @Test public void different_dimension_values_are_not_treated_as_equal() throws Exception { var collector = new MetricsCollector(); - MetricsParser.parse(metricsJson(), collector); + 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 metricsJson() { + private String metricsJsonDistinctButDuplicateDimensionDalues() { return """ { "time": 1671035366573, @@ -102,5 +84,46 @@ public class MetricsParserTest { } """; } + // 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" + } + } + ] + } + } + """; + } } |