[Exporter.Geneva] Fix exemplar value serialization for TLV serializat…

…ion format (#1698)
open-telemetry · Apr 26, 2024 · 9d4b5f1 · 9d4b5f1
1 parent 59b5c3b
commit 9d4b5f1
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 16 deletions.
diff --git a/src/OpenTelemetry.Exporter.Geneva/Metrics/TlvMetricExporter.cs b/src/OpenTelemetry.Exporter.Geneva/Metrics/TlvMetricExporter.cs
@@ -90,8 +90,8 @@ internal ExportResult Export(in Batch<Metric> batch)
 #if EXPOSE_EXPERIMENTAL_FEATURES
                     metricPoint.TryGetExemplars(out var exemplars);
 #endif
-
-                    switch (metric.MetricType)
+                    var metricType = metric.MetricType;
+                    switch (metricType)
                     {
                         case MetricType.LongSum:
                             {
@@ -104,6 +104,7 @@ internal ExportResult Export(in Batch<Metric> batch)
                                     metricPoint.Tags,
                                     metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                                    metricType,
                                     exemplars,
 #endif
                                     out monitoringAccount,
@@ -126,6 +127,7 @@ internal ExportResult Export(in Batch<Metric> batch)
                                     metricPoint.Tags,
                                     metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                                    metricType,
                                     exemplars,
 #endif
                                     out monitoringAccount,
@@ -148,6 +150,7 @@ internal ExportResult Export(in Batch<Metric> batch)
                                     metricPoint.Tags,
                                     metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                                    metricType,
                                     exemplars,
 #endif
                                     out monitoringAccount,
@@ -168,6 +171,7 @@ internal ExportResult Export(in Batch<Metric> batch)
                                     metricPoint.Tags,
                                     metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                                    metricType,
                                     exemplars,
 #endif
                                     out monitoringAccount,
@@ -187,6 +191,7 @@ internal ExportResult Export(in Batch<Metric> batch)
                                     metricPoint.Tags,
                                     metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                                    metricType,
                                     exemplars,
 #endif
                                     out monitoringAccount,
@@ -215,6 +220,7 @@ internal ExportResult Export(in Batch<Metric> batch)
                                     min,
                                     max,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                                    metricType,
                                     exemplars,
 #endif
                                     out monitoringAccount,
@@ -266,6 +272,7 @@ internal unsafe ushort SerializeMetricWithTLV(
         in ReadOnlyTagCollection tags,
         MetricData value,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+        MetricType metricType,
         ReadOnlyExemplarCollection exemplars,
 #endif
         out string monitoringAccount,
@@ -295,7 +302,7 @@ internal unsafe ushort SerializeMetricWithTLV(
                 out metricNamespace);
 
 #if EXPOSE_EXPERIMENTAL_FEATURES
-            SerializeExemplars(exemplars, this.buffer, ref bufferIndex);
+            SerializeExemplars(exemplars, metricType, this.buffer, ref bufferIndex);
 #endif
 
             SerializeMonitoringAccount(monitoringAccount, this.buffer, ref bufferIndex);
@@ -330,6 +337,7 @@ internal unsafe ushort SerializeHistogramMetricWithTLV(
         double min,
         double max,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+        MetricType metricType,
         ReadOnlyExemplarCollection exemplars,
 #endif
         out string monitoringAccount,
@@ -359,7 +367,7 @@ internal unsafe ushort SerializeHistogramMetricWithTLV(
                 out metricNamespace);
 
 #if EXPOSE_EXPERIMENTAL_FEATURES
-            SerializeExemplars(exemplars, this.buffer, ref bufferIndex);
+            SerializeExemplars(exemplars, metricType, this.buffer, ref bufferIndex);
 #endif
 
             SerializeMonitoringAccount(monitoringAccount, this.buffer, ref bufferIndex);
@@ -407,7 +415,7 @@ private static void SerializeMonitoringAccount(string monitoringAccount, byte[]
 
 #if EXPOSE_EXPERIMENTAL_FEATURES
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    private static void SerializeExemplars(in ReadOnlyExemplarCollection exemplars, byte[] buffer, ref int bufferIndex)
+    private static void SerializeExemplars(in ReadOnlyExemplarCollection exemplars, MetricType metricType, byte[] buffer, ref int bufferIndex)
     {
         var exemplarsCount = 0;
         foreach (ref readonly var exemplar in exemplars)
@@ -429,7 +437,7 @@ private static void SerializeExemplars(in ReadOnlyExemplarCollection exemplars,
 
             foreach (ref readonly var exemplar in exemplars)
             {
-                SerializeSingleExemplar(exemplar, buffer, ref bufferIndex);
+                SerializeSingleExemplar(exemplar, metricType, buffer, ref bufferIndex);
             }
 
             var payloadTypeLength = (ushort)(bufferIndex - payloadTypeStartIndex - 2);
@@ -438,7 +446,7 @@ private static void SerializeExemplars(in ReadOnlyExemplarCollection exemplars,
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    private static void SerializeSingleExemplar(Exemplar exemplar, byte[] buffer, ref int bufferIndex)
+    private static void SerializeSingleExemplar(Exemplar exemplar, MetricType metricType, byte[] buffer, ref int bufferIndex)
     {
         MetricSerializer.SerializeByte(buffer, ref bufferIndex, 0); // version
 
@@ -450,20 +458,14 @@ private static void SerializeSingleExemplar(Exemplar exemplar, byte[] buffer, re
 
         var flags = ExemplarFlags.IsTimestampAvailable; // we only serialize exemplars with Timestamp != default
 
-        // TODO: Update the code when Exemplars support long values
-        var value = exemplar.DoubleValue;
-
-        // Check if the double value is actually a whole number that can be serialized as a long instead
-        var valueAsLong = (long)value;
-        bool isWholeNumber = valueAsLong == value;
-        if (isWholeNumber)
+        if (metricType.IsLong())
         {
             flags |= ExemplarFlags.IsMetricValueDoubleStoredAsLong;
-            MetricSerializer.SerializeInt64AsBase128(buffer, ref bufferIndex, valueAsLong); // serialize long value
+            MetricSerializer.SerializeInt64AsBase128(buffer, ref bufferIndex, exemplar.LongValue); // serialize long value
         }
         else
         {
-            MetricSerializer.SerializeFloat64(buffer, ref bufferIndex, value); // serialize double value
+            MetricSerializer.SerializeFloat64(buffer, ref bufferIndex, exemplar.DoubleValue); // serialize double value
         }
 
         var bufferIndexForNumberOfLabels = bufferIndex;

diff --git a/test/OpenTelemetry.Exporter.Geneva.Benchmark/Exporter/MetricExporterBenchmarks.cs b/test/OpenTelemetry.Exporter.Geneva.Benchmark/Exporter/MetricExporterBenchmarks.cs
@@ -584,6 +584,7 @@ public void SerializeCounterMetricItemWith3Dimensions()
             this.counterMetricPointWith3Dimensions.Tags,
             this.counterMetricDataWith3Dimensions,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+            MetricType.LongSum,
             exemplars,
 #endif
             out _,
@@ -603,6 +604,7 @@ public void SerializeCounterMetricItemWith4Dimensions()
             this.counterMetricPointWith4Dimensions.Tags,
             this.counterMetricDataWith4Dimensions,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+            MetricType.LongSum,
             exemplars,
 #endif
             out _,
@@ -637,6 +639,7 @@ public void SerializeHistogramMetricItemWith3Dimensions()
             this.histogramMinWith3Dimensions,
             this.histogramMaxWith3Dimensions,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+            MetricType.Histogram,
             exemplars,
 #endif
             out _,
@@ -659,6 +662,7 @@ public void SerializeHistogramMetricItemWith4Dimensions()
             this.histogramMinWith4Dimensions,
             this.histogramMaxWith4Dimensions,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+            MetricType.Histogram,
             exemplars,
 #endif
             out _,

diff --git a/test/OpenTelemetry.Exporter.Geneva.Tests/GenevaMetricExporterTests.cs b/test/OpenTelemetry.Exporter.Geneva.Tests/GenevaMetricExporterTests.cs
@@ -195,6 +195,7 @@ public void SuccessfulExportOnLinux()
                     metricPoint.Tags,
                     metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                    MetricType.LongSum,
                     exemplars,
 #endif
                     out _,
@@ -1000,6 +1001,7 @@ private static void CheckSerializationWithTLVForSingleMetricPoint(Metric metric,
                 metricPoint.Tags,
                 metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,
@@ -1028,6 +1030,7 @@ private static void CheckSerializationWithTLVForSingleMetricPoint(Metric metric,
                 metricPoint.Tags,
                 metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,
@@ -1058,6 +1061,7 @@ private static void CheckSerializationWithTLVForSingleMetricPoint(Metric metric,
                 metricPoint.Tags,
                 metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,
@@ -1088,6 +1092,7 @@ private static void CheckSerializationWithTLVForSingleMetricPoint(Metric metric,
                 metricPoint.Tags,
                 metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,
@@ -1125,6 +1130,7 @@ private static void CheckSerializationWithTLVForSingleMetricPoint(Metric metric,
                 min,
                 max,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,
@@ -1276,6 +1282,9 @@ private static void AssertExemplarFilteredTagSerialization(Exemplar expectedExem
                 .TotalMilliseconds * 1000000;
 
         // TODO: Test for exemplar values stored as long
+        // Ideally we could assert the long on serializedExemplarBody.Value.ValueAsVlq
+        // But the result returned by it is not correct so skipping the long check for now.
+        // TODO: follow up to see if this is a bug in KaitaiStruct.Runtime.CSharp.
         if (!serializedExemplarBody.Value.IsDoubleStoredAsLong)
         {
             Assert.Equal(expectedExemplar.DoubleValue, serializedExemplarBody.Value.ValueAsDouble);
@@ -1345,6 +1354,7 @@ private static UserdataV2 GetSerializedData(Metric metric, TlvMetricExporter exp
                 metricPoint.Tags,
                 metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,
@@ -1366,6 +1376,7 @@ private static UserdataV2 GetSerializedData(Metric metric, TlvMetricExporter exp
                 metricPoint.Tags,
                 metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,
@@ -1389,6 +1400,7 @@ private static UserdataV2 GetSerializedData(Metric metric, TlvMetricExporter exp
                 metricPoint.Tags,
                 metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,
@@ -1412,6 +1424,7 @@ private static UserdataV2 GetSerializedData(Metric metric, TlvMetricExporter exp
                 metricPoint.Tags,
                 metricData,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,
@@ -1442,6 +1455,7 @@ private static UserdataV2 GetSerializedData(Metric metric, TlvMetricExporter exp
                 min,
                 max,
 #if EXPOSE_EXPERIMENTAL_FEATURES
+                metricType,
                 exemplars,
 #endif
                 out _,