Skip to content

Do not apply ExplicitBucketHistogramConfiguration to non-histograms #3126

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

Merged
cijothomas merged 10 commits into from
Mar 31, 2022
Merged

Do not apply ExplicitBucketHistogramConfiguration to non-histograms #3126

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
da0d53a
Add failing test demonstrating that histogram config currently applie…
alanwest Mar 31, 2022
daa2915
Do not apply histogram stream config to non-histograms
alanwest Mar 31, 2022
9f2668c
Update changelog
alanwest Mar 31, 2022
aa23358
Merge branch 'main' into alanwest/its-a-bird-its-a-plane-its-not-a-hi…
alanwest Mar 31, 2022
6936c7e
Log warning message
alanwest Mar 31, 2022
646232e
Merge branch 'alanwest/its-a-bird-its-a-plane-its-not-a-histogram' of…
alanwest Mar 31, 2022
221d3f8
Update changelog
alanwest Mar 31, 2022
6760658
Make it nicer
alanwest Mar 31, 2022
1601764
Merge branch 'main' into alanwest/its-a-bird-its-a-plane-its-not-a-hi…
cijothomas Mar 31, 2022
67371e4
Undo deleted line
alanwest Mar 31, 2022
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: 4 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Fix issue where `ExplicitBucketHistogramConfiguration` could be used to
Comment thread
cijothomas marked this conversation as resolved.
configure metric streams for instruments that are not histograms.
([#3126](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3126))

## 1.2.0-rc4

Released 2022-Mar-30
Expand Down
6 changes: 6 additions & 0 deletions src/OpenTelemetry/Metrics/MeterProviderSdk.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,12 @@ internal MeterProviderSdk(
try
{
metricStreamConfig = viewConfig(instrument);

if (metricStreamConfig is ExplicitBucketHistogramConfiguration
&& instrument.GetType().GetGenericTypeDefinition() != typeof(Histogram<>))
{
Comment thread
cijothomas marked this conversation as resolved.
metricStreamConfig = null;
Comment thread
cijothomas marked this conversation as resolved.
}
}
catch (Exception ex)
{
Expand Down
31 changes: 31 additions & 0 deletions test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,37 @@ public void ViewToProduceMultipleStreamsWithDuplicatesFromInstrument()
Assert.Equal("renamedStream2", exportedItems[1].Name);
}

[Fact]
public void ViewWithHistogramConfiguraionIgnoredWhenAppliedToNonHistogram()
{
using var meter = new Meter(Utils.GetCurrentMethodName());
var exportedItems = new List<Metric>();
using var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddMeter(meter.Name)
.AddView("NotAHistogram", new ExplicitBucketHistogramConfiguration() { Name = "ImAHistogram" })
.AddInMemoryExporter(exportedItems)
.Build();

var counter = meter.CreateCounter<long>("NotAHistogram");
counter.Add(10);
meterProvider.ForceFlush(MaxTimeToAllowForFlush);

Assert.Single(exportedItems);
var metric = exportedItems[0];

Assert.Equal("NotAHistogram", metric.Name);

List<MetricPoint> metricPoints = new List<MetricPoint>();
foreach (ref readonly var mp in metric.GetMetricPoints())
{
metricPoints.Add(mp);
}

Assert.Single(metricPoints);
var metricPoint = metricPoints[0];
Assert.Equal(10, metricPoint.GetSumLong());
}

[Fact]
public void ViewToProduceCustomHistogramBound()
{
Expand Down