Skip to content
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.

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

Histogram MinMax API fixes #3822

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ public override ExportResult Export(in Batch<Metric> batch)
var sum = metricPoint.GetHistogramSum();
var count = metricPoint.GetHistogramCount();
bucketsBuilder.Append($"Sum: {sum} Count: {count} ");
if (metricPoint.HasMinMax())
if (metricPoint.TryGetHistogramMinMaxValues(out double min, out double max))
{
bucketsBuilder.Append($"Min: {metricPoint.GetHistogramMin()} Max: {metricPoint.GetHistogramMax()} ");
bucketsBuilder.Append($"Min: {min} Max: {max} ");
}

bucketsBuilder.AppendLine();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,10 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric)
dataPoint.Count = (ulong)metricPoint.GetHistogramCount();
dataPoint.Sum = metricPoint.GetHistogramSum();

if (metricPoint.HasMinMax())
if (metricPoint.TryGetHistogramMinMaxValues(out double min, out double max))
{
dataPoint.Min = metricPoint.GetHistogramMin();
dataPoint.Max = metricPoint.GetHistogramMax();
dataPoint.Min = min;
dataPoint.Max = max;
}

foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ OpenTelemetry.Metrics.HistogramConfiguration
OpenTelemetry.Metrics.HistogramConfiguration.HistogramConfiguration() -> void
OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.get -> bool
OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.set -> void
OpenTelemetry.Metrics.MetricPoint.GetHistogramMax() -> double
OpenTelemetry.Metrics.MetricPoint.GetHistogramMin() -> double
OpenTelemetry.Metrics.MetricPoint.HasMinMax() -> bool
OpenTelemetry.Metrics.MetricPoint.TryGetHistogramMinMaxValues(out double min, out double max) -> bool
OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider?, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions(Microsoft.Extensions.Configuration.IConfiguration! configuration) -> void
static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ OpenTelemetry.Metrics.HistogramConfiguration
OpenTelemetry.Metrics.HistogramConfiguration.HistogramConfiguration() -> void
OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.get -> bool
OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.set -> void
OpenTelemetry.Metrics.MetricPoint.GetHistogramMax() -> double
OpenTelemetry.Metrics.MetricPoint.GetHistogramMin() -> double
OpenTelemetry.Metrics.MetricPoint.HasMinMax() -> bool
OpenTelemetry.Metrics.MetricPoint.TryGetHistogramMinMaxValues(out double min, out double max) -> bool
OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider?, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions(Microsoft.Extensions.Configuration.IConfiguration! configuration) -> void
static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ OpenTelemetry.Metrics.HistogramConfiguration
OpenTelemetry.Metrics.HistogramConfiguration.HistogramConfiguration() -> void
OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.get -> bool
OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.set -> void
OpenTelemetry.Metrics.MetricPoint.GetHistogramMax() -> double
OpenTelemetry.Metrics.MetricPoint.GetHistogramMin() -> double
OpenTelemetry.Metrics.MetricPoint.HasMinMax() -> bool
OpenTelemetry.Metrics.MetricPoint.TryGetHistogramMinMaxValues(out double min, out double max) -> bool
OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider?, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions(Microsoft.Extensions.Configuration.IConfiguration! configuration) -> void
static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ OpenTelemetry.Metrics.HistogramConfiguration
OpenTelemetry.Metrics.HistogramConfiguration.HistogramConfiguration() -> void
OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.get -> bool
OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.set -> void
OpenTelemetry.Metrics.MetricPoint.GetHistogramMax() -> double
OpenTelemetry.Metrics.MetricPoint.GetHistogramMin() -> double
OpenTelemetry.Metrics.MetricPoint.HasMinMax() -> bool
OpenTelemetry.Metrics.MetricPoint.TryGetHistogramMinMaxValues(out double min, out double max) -> bool
OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider?, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions(Microsoft.Extensions.Configuration.IConfiguration! configuration) -> void
static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
Expand Down
49 changes: 16 additions & 33 deletions src/OpenTelemetry/Metrics/MetricPoint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,44 +243,27 @@ internal MetricPointStatus MetricPointStatus
}

/// <summary>
/// Gets the minimum value of the histogram associated with the metric
/// point if present. Check by using <see cref="HasMinMax"/>.
/// Gets the Histogram Min and Max values.
/// </summary>
/// <remarks>
/// Applies to <see cref="MetricType.Histogram"/> metric type.
/// </remarks>
/// <returns>A histogram's maximum value.</returns>
/// <param name="min"> The histogram minimum value.</param>
/// <param name="max"> The histogram maximum value.</param>
/// <returns>True if minimum and maximum value exist, false otherwise.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public double GetHistogramMin()
public bool TryGetHistogramMinMaxValues(out double min, out double max)
{
return this.histogramBuckets.SnapshotMin;
}
if (this.aggType == AggregationType.HistogramWithMinMax ||
this.aggType == AggregationType.HistogramWithMinMaxBuckets)
{
Debug.Assert(this.histogramBuckets != null, "histogramBuckets was null");

/// <summary>
/// Gets the maximum value of the histogram associated with the metric
/// point if present. Check by using <see cref="HasMinMax"/>.
/// </summary>
/// <remarks>
/// Applies to <see cref="MetricType.Histogram"/> metric type.
/// </remarks>
/// <returns>A histogram's maximum value.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public double GetHistogramMax()
{
return this.histogramBuckets.SnapshotMax;
}
min = this.histogramBuckets.SnapshotMin;
max = this.histogramBuckets.SnapshotMax;
return true;
}

/// <summary>
/// Gets whether the histogram has a valid Min and Max value.
/// Should be used before
/// <see cref="GetHistogramMin"/> and <see cref="GetHistogramMax"/> to
/// ensure a valid value is returned.
/// </summary>
/// <returns>A minimum and maximum value exist.</returns>
public bool HasMinMax()
{
return this.aggType == AggregationType.HistogramWithMinMaxBuckets ||
this.aggType == AggregationType.HistogramWithMinMax;
min = 0;
max = 0;
return false;
}

internal readonly MetricPoint Copy()
Expand Down
20 changes: 10 additions & 10 deletions test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -599,14 +599,15 @@ public void HistogramMinMax(double[] values, HistogramConfiguration histogramCon
}

var histogramPoint = metricPoints[0];
var hasMinMax = histogramPoint.HasMinMax();
Assert.True(hasMinMax);

var min = histogramPoint.GetHistogramMin();
var max = histogramPoint.GetHistogramMax();

Assert.Equal(expectedMin, min);
Assert.Equal(expectedMax, max);
if (histogramPoint.TryGetHistogramMinMaxValues(out double min, out double max))
{
Assert.Equal(expectedMin, min);
Assert.Equal(expectedMax, max);
}
else
{
Assert.Fail("MinMax expected");
}
}

[Theory]
Expand Down Expand Up @@ -636,8 +637,7 @@ public void HistogramMinMaxNotPresent(double[] values, HistogramConfiguration hi
}

var histogramPoint = metricPoints[0];

Assert.False(histogramPoint.HasMinMax());
Assert.False(histogramPoint.TryGetHistogramMinMaxValues(out double _, out double _));
}

[Fact]
Expand Down