Skip to content

[Exporter.Geneva] Fix exporter histogram negative values #3676

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Dytroxx wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Exporter.Geneva] Fix exporter histogram negative values #3676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/OpenTelemetry.Exporter.Geneva/Metrics/MetricSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ internal enum PayloadType
DoubleMetric = 6,
ExternallyAggregatedULongDistributionMetric = 8,
HistogramULongValueCountPairs = 12,
ExternallyAggregatedDoubleDistributionMetric = 9,
HistogramDoubleScaledToUInt64ValueCountPairs = 13,
Exemplars = 15,
}

Expand Down
105 changes: 93 additions & 12 deletions src/OpenTelemetry.Exporter.Geneva/Metrics/TlvMetricExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ internal ExportResult Export(in Batch<Metric> batch)

case MetricType.Histogram:
{
var sum = Convert.ToUInt64(metricPoint.GetHistogramSum());
var sum = metricPoint.GetHistogramSum();
var count = Convert.ToUInt32(metricPoint.GetHistogramCount());
if (!metricPoint.TryGetHistogramMinMaxValues(out var min, out var max))
{
Expand Down Expand Up @@ -525,7 +525,12 @@ private static void SerializeNonHistogramMetricData(MetricEventType eventType, M
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void SerializeHistogramMetricData(HistogramBuckets buckets, double sum, uint count, double min, double max, long timestamp, byte[] buffer, ref int bufferIndex)
{
MetricSerializer.SerializeByte(buffer, ref bufferIndex, (byte)PayloadType.ExternallyAggregatedULongDistributionMetric);
var useDoubleAggregation = sum < 0 || min < 0 || max < 0;

MetricSerializer.SerializeByte(
buffer,
ref bufferIndex,
(byte)(useDoubleAggregation ? PayloadType.ExternallyAggregatedDoubleDistributionMetric : PayloadType.ExternallyAggregatedULongDistributionMetric));

// Get a placeholder to add the payloadType length
var payloadTypeStartIndex = bufferIndex;
Expand All @@ -535,15 +540,29 @@ private static void SerializeHistogramMetricData(HistogramBuckets buckets, doubl
MetricSerializer.SerializeUInt32(buffer, ref bufferIndex, count); // histogram count
MetricSerializer.SerializeUInt32(buffer, ref bufferIndex, 0); // padding
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, (ulong)timestamp); // timestamp
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, Convert.ToUInt64(sum)); // histogram sum
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, Convert.ToUInt64(min)); // histogram min
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, Convert.ToUInt64(max)); // histogram max

if (useDoubleAggregation)
{
MetricSerializer.SerializeFloat64(buffer, ref bufferIndex, sum); // histogram sum
MetricSerializer.SerializeFloat64(buffer, ref bufferIndex, min); // histogram min
MetricSerializer.SerializeFloat64(buffer, ref bufferIndex, max); // histogram max
}
else
{
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, SafeConvertDoubleToUInt64(sum)); // histogram sum
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, SafeConvertDoubleToUInt64(min)); // histogram min
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, SafeConvertDoubleToUInt64(max)); // histogram max
}

var payloadTypeLength = (ushort)(bufferIndex - payloadTypeStartIndex - 2);
MetricSerializer.SerializeUInt16(buffer, ref payloadTypeStartIndex, payloadTypeLength);

// Serialize histogram buckets as value-count pairs
MetricSerializer.SerializeByte(buffer, ref bufferIndex, (byte)PayloadType.HistogramULongValueCountPairs);
var useDoubleEncodedBounds = useDoubleAggregation || !AreAllBoundsUInt64Compatible(buckets);
MetricSerializer.SerializeByte(
buffer,
ref bufferIndex,
(byte)(useDoubleEncodedBounds ? PayloadType.HistogramDoubleScaledToUInt64ValueCountPairs : PayloadType.HistogramULongValueCountPairs));

// Get a placeholder to add the payloadType length
payloadTypeStartIndex = bufferIndex;
Expand All @@ -562,7 +581,7 @@ private static void SerializeHistogramMetricData(HistogramBuckets buckets, doubl
{
if (bucket.BucketCount > 0)
{
SerializeHistogramBucketWithTLV(bucket, buffer, ref bufferIndex, lastExplicitBound);
SerializeHistogramBucketWithTLV(bucket, buffer, ref bufferIndex, lastExplicitBound, useDoubleEncodedBounds);
bucketCount++;
}

Expand All @@ -577,21 +596,83 @@ private static void SerializeHistogramMetricData(HistogramBuckets buckets, doubl
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void SerializeHistogramBucketWithTLV(in HistogramBucket bucket, byte[] buffer, ref int bufferIndex, double lastExplicitBound)
private static void SerializeHistogramBucketWithTLV(in HistogramBucket bucket, byte[] buffer, ref int bufferIndex, double lastExplicitBound, bool useDoubleEncodedBound)
{
if (bucket.ExplicitBound != double.PositiveInfinity)
var bound = bucket.ExplicitBound != double.PositiveInfinity ? bucket.ExplicitBound : lastExplicitBound + 1;

if (useDoubleEncodedBound)
{
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, Convert.ToUInt64(bucket.ExplicitBound));
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, EncodeDoubleToSortableUInt64(bound));
}
else
{
// The bucket to catch the overflows is one greater than the last bound provided
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, Convert.ToUInt64(lastExplicitBound + 1));
MetricSerializer.SerializeUInt64(buffer, ref bufferIndex, SafeConvertDoubleToUInt64(bound));
}

MetricSerializer.SerializeUInt32(buffer, ref bufferIndex, Convert.ToUInt32(bucket.BucketCount));
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static ulong SafeConvertDoubleToUInt64(double value)
{
if (double.IsNaN(value))
{
return 0;
}

if (value <= 0)
{
return 0;
}

if (value >= (double)ulong.MaxValue)
{
return ulong.MaxValue;
}

// For positive values in-range, truncation is OK; histogram aggregates should be integral for long instruments.
return (ulong)value;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool AreAllBoundsUInt64Compatible(HistogramBuckets buckets)
{
double lastExplicitBound = default;
foreach (var bucket in buckets)
{
var bound = bucket.ExplicitBound != double.PositiveInfinity ? bucket.ExplicitBound : lastExplicitBound + 1;
if (!IsUInt64CompatibleBound(bound))
{
return false;
}

lastExplicitBound = bucket.ExplicitBound;
}

return true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool IsUInt64CompatibleBound(double bound)
{
return !double.IsNaN(bound)
&& !double.IsInfinity(bound)
&& bound >= 0
&& bound < (double)ulong.MaxValue
&& bound == Math.Truncate(bound);
}

// Encodes a double into an unsigned 64-bit integer that preserves numeric ordering when
// compared as an unsigned integer. This is a common "sortable double" encoding.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static ulong EncodeDoubleToSortableUInt64(double value)
{
var bits = (ulong)BitConverter.DoubleToInt64Bits(value);
var signMask = (ulong)((long)bits >> 63);

return bits ^ (signMask | 0x8000_0000_0000_0000UL);
}

private static string? ConvertTagValueToString(object? value)
{
string? repr;
Expand Down
12 changes: 6 additions & 6 deletions test/OpenTelemetry.Exporter.Geneva.Benchmarks/Exporter/MetricExporterBenchmarks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ public class MetricExporterBenchmarks
private Metric? histogramMetricWith4Dimensions;
private MetricPoint histogramMetricPointWith3Dimensions;
private MetricPoint histogramMetricPointWith4Dimensions;
private ulong histogramSumWith3Dimensions;
private ulong histogramSumWith4Dimensions;
private double histogramSumWith3Dimensions;
private double histogramSumWith4Dimensions;
private double histogramMinWith3Dimensions;
private double histogramMinWith4Dimensions;
private double histogramMaxWith3Dimensions;
Expand Down Expand Up @@ -305,7 +305,7 @@ private Batch<Metric> GenerateCounterBatchWith4Dimensions()
return batchGeneratorExporter.Batch;
}

private MetricPoint GenerateHistogramMetricItemWith3Dimensions(out ulong sum, out uint count, out double min, out double max)
private MetricPoint GenerateHistogramMetricItemWith3Dimensions(out double sum, out uint count, out double min, out double max)
{
using var meterWithInMemoryExporter = new Meter("GenerateHistogramMetricItemWith3Dimensions", "0.0.1");
var histogram = meterWithInMemoryExporter.CreateHistogram<long>("HistogramWith3Dimensions");
Expand Down Expand Up @@ -336,7 +336,7 @@ private MetricPoint GenerateHistogramMetricItemWith3Dimensions(out ulong sum, ou
var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator();
metricPointsEnumerator.MoveNext();
var metricPoint = metricPointsEnumerator.Current;
sum = Convert.ToUInt64(metricPoint.GetHistogramSum());
sum = metricPoint.GetHistogramSum();
count = Convert.ToUInt32(metricPoint.GetHistogramCount());

if (!metricPoint.TryGetHistogramMinMaxValues(out min, out max))
Expand All @@ -348,7 +348,7 @@ private MetricPoint GenerateHistogramMetricItemWith3Dimensions(out ulong sum, ou
return metricPoint;
}

private MetricPoint GenerateHistogramMetricItemWith4Dimensions(out ulong sum, out uint count, out double min, out double max)
private MetricPoint GenerateHistogramMetricItemWith4Dimensions(out double sum, out uint count, out double min, out double max)
{
using var meterWithInMemoryExporter = new Meter("GenerateHistogramMetricItemWith4Dimensions", "0.0.1");
var histogram = meterWithInMemoryExporter.CreateHistogram<long>("HistogramWith4Dimensions");
Expand Down Expand Up @@ -383,7 +383,7 @@ private MetricPoint GenerateHistogramMetricItemWith4Dimensions(out ulong sum, ou
var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator();
metricPointsEnumerator.MoveNext();
var metricPoint = metricPointsEnumerator.Current;
sum = Convert.ToUInt64(metricPoint.GetHistogramSum());
sum = metricPoint.GetHistogramSum();
count = Convert.ToUInt32(metricPoint.GetHistogramCount());

if (!metricPoint.TryGetHistogramMinMaxValues(out min, out max))
Expand Down
Loading