Merge pull request #30928 from vespa-engine/balder/reduce-duplicats

- Remove duplicates of the timestamp.
vespa-engine · Apr 16, 2024 · 9ee70cf · 9ee70cf
2 parents 9151029 + df8244c
commit 9ee70cf
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 15 deletions.
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/DimensionId.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/DimensionId.java
@@ -2,6 +2,7 @@
 package ai.vespa.metricsproxy.metric.model;
 
 import com.yahoo.concurrent.CopyOnWriteHashMap;
+import io.prometheus.client.Collector;
 
 import java.util.Map;
 import java.util.Objects;
@@ -13,12 +14,18 @@ public final class DimensionId {
 
     private static final Map<String, DimensionId> dictionary = new CopyOnWriteHashMap<>();
     public final String id;
-    private DimensionId(String id) { this.id = id; }
+    private final String idForPrometheus;
+    private DimensionId(String id) {
+        this.id = id;
+        idForPrometheus = Collector.sanitizeMetricName(id);
+    }
 
     public static DimensionId toDimensionId(String id) {
         return dictionary.computeIfAbsent(id, key -> new DimensionId(key));
     }
 
+    public String getIdForPrometheus() { return idForPrometheus; }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricId.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricId.java
@@ -2,6 +2,7 @@
 package ai.vespa.metricsproxy.metric.model;
 
 import com.yahoo.concurrent.CopyOnWriteHashMap;
+import io.prometheus.client.Collector;
 
 import java.util.Map;
 import java.util.Objects;
@@ -14,11 +15,16 @@ public class MetricId {
     private static final Map<String, MetricId> dictionary = new CopyOnWriteHashMap<>();
     public static final MetricId empty = toMetricId("");
     public final String id;
-    private MetricId(String id) { this.id = id; }
+    private final String idForPrometheus;
+    private MetricId(String id) {
+        this.id = id;
+        idForPrometheus = Collector.sanitizeMetricName(id);
+    }
 
     public static MetricId toMetricId(String id) {
-        return dictionary.computeIfAbsent(id, key -> new MetricId(key));
+        return dictionary.computeIfAbsent(id, MetricId::new);
     }
+    public String getIdForPrometheus() { return idForPrometheus; }
 
     @Override
     public boolean equals(Object o) {
@@ -29,13 +35,9 @@ public boolean equals(Object o) {
     }
 
     @Override
-    public int hashCode() {
-        return Objects.hash(id);
-    }
+    public int hashCode() { return Objects.hash(id); }
 
     @Override
-    public String toString() {
-        return id;
-    }
+    public String toString() { return id; }
 
 }
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/ServiceId.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/ServiceId.java
@@ -1,6 +1,8 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package ai.vespa.metricsproxy.metric.model;
 
+import io.prometheus.client.Collector;
+
 import java.util.Objects;
 
 /**
@@ -9,10 +11,16 @@
 public class ServiceId {
 
     public final String id;
-    private ServiceId(String id) { this.id = id; }
+    private final String idForPrometheus;
+    private ServiceId(String id) {
+        this.id = id;
+        idForPrometheus = Collector.sanitizeMetricName(id);
+    }
 
     public static ServiceId toServiceId(String id) { return new ServiceId(id); }
 
+    public String getIdForPrometheus() { return idForPrometheus; }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;

diff --git a/...ics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusUtil.java b/...ics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/prometheus/PrometheusUtil.java
@@ -28,21 +28,22 @@ public static PrometheusModel toPrometheusModel(List<MetricsPacket> metricsPacke
         Map<String, List<Sample>> samples = new HashMap<>();
         packetsByService.forEach(((serviceId, packets) -> {
 
-            var serviceName = Collector.sanitizeMetricName(serviceId.id);
+            var serviceName = serviceId.getIdForPrometheus();
             for (var packet : packets) {
+                Long timeStamp = packet.timestamp * 1000;
                 var dimensions = packet.dimensions();
                 List<String> labels = new ArrayList<>(dimensions.size());
                 List<String> labelValues = new ArrayList<>(dimensions.size());
                 for (var entry : dimensions.entrySet()) {
-                    var labelName = Collector.sanitizeMetricName(entry.getKey().id);
+                    var labelName = entry.getKey().getIdForPrometheus();
                     labels.add(labelName);
                     labelValues.add(entry.getValue());
                 }
                 labels.add("vespa_service");
                 labelValues.add(serviceName);
 
                 for (var metric : packet.metrics().entrySet()) {
-                    var metricName = Collector.sanitizeMetricName(metric.getKey().id);
+                    var metricName = metric.getKey().getIdForPrometheus();
                     List<Sample> sampleList;
                     if (samples.containsKey(metricName)) {
                         sampleList = samples.get(metricName);
@@ -51,7 +52,7 @@ public static PrometheusModel toPrometheusModel(List<MetricsPacket> metricsPacke
                         samples.put(metricName, sampleList);
                         metricFamilySamples.add(new MetricFamilySamples(metricName, Collector.Type.UNKNOWN, "", sampleList));
                     }
-                    sampleList.add(new Sample(metricName, labels, labelValues, metric.getValue().doubleValue(), packet.timestamp * 1000));
+                    sampleList.add(new Sample(metricName, labels, labelValues, metric.getValue().doubleValue(), timeStamp));
                 }
             }
             if (!packets.isEmpty()) {

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
@@ -28,6 +28,7 @@
  * @author Jo Kristian Bergum
  */
 public class MetricsParser {
+    private static final Double ZERO_DOUBLE = 0d;
     public interface Collector {
         void accept(Metric metric);
     }
@@ -186,7 +187,8 @@ private static List<Map.Entry<String, Number>> parseValues(JsonParser parser) th
             if (token == JsonToken.VALUE_NUMBER_INT) {
                 metrics.add(Map.entry(metricName, parser.getLongValue()));
             } else if (token == JsonToken.VALUE_NUMBER_FLOAT) {
-                metrics.add(Map.entry(metricName, parser.getValueAsDouble()));
+                double value = parser.getValueAsDouble();
+                metrics.add(Map.entry(metricName, value == ZERO_DOUBLE ? ZERO_DOUBLE : value));
             } else {
                 throw new IllegalArgumentException("Value for aggregator '" + fieldName + "' is not a number");
             }