diff --git a/src/OpenTelemetry/Metrics/InstrumentIdentity.cs b/src/OpenTelemetry/Metrics/InstrumentIdentity.cs index 411a53eb1a1..ff69da59259 100644 --- a/src/OpenTelemetry/Metrics/InstrumentIdentity.cs +++ b/src/OpenTelemetry/Metrics/InstrumentIdentity.cs @@ -23,7 +23,7 @@ namespace OpenTelemetry.Metrics { private readonly int hashCode; - public InstrumentIdentity(Meter meter, string instrumentName, string unit, string description, Type instrumentType) + public InstrumentIdentity(Meter meter, string instrumentName, string unit, string description, Type instrumentType, bool derivedFromView) { this.MeterName = meter.Name; this.MeterVersion = meter.Version ?? string.Empty; @@ -31,6 +31,7 @@ public InstrumentIdentity(Meter meter, string instrumentName, string unit, strin this.Unit = unit ?? string.Empty; this.Description = description ?? string.Empty; this.InstrumentType = instrumentType; + this.DerivedFromView = derivedFromView; unchecked { @@ -41,6 +42,7 @@ public InstrumentIdentity(Meter meter, string instrumentName, string unit, strin hash = (hash * 31) + this.InstrumentName.GetHashCode(); hash = this.Unit == null ? hash : (hash * 31) + this.Unit.GetHashCode(); hash = this.Description == null ? hash : (hash * 31) + this.Description.GetHashCode(); + hash = (hash * 31) + this.DerivedFromView.GetHashCode(); this.hashCode = hash; } } @@ -57,6 +59,8 @@ public InstrumentIdentity(Meter meter, string instrumentName, string unit, strin public readonly Type InstrumentType { get; } + public readonly bool DerivedFromView { get; } + public static bool operator ==(InstrumentIdentity metricIdentity1, InstrumentIdentity metricIdentity2) => metricIdentity1.Equals(metricIdentity2); public static bool operator !=(InstrumentIdentity metricIdentity1, InstrumentIdentity metricIdentity2) => !metricIdentity1.Equals(metricIdentity2); @@ -73,7 +77,8 @@ public bool Equals(InstrumentIdentity other) && this.MeterVersion == other.MeterVersion && this.InstrumentName == other.InstrumentName && this.Unit == other.Unit - && this.Description == other.Description; + && this.Description == other.Description + && this.DerivedFromView == other.DerivedFromView; } public override readonly int GetHashCode() => this.hashCode; diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index 2603da6d64d..2152064824e 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -42,7 +42,7 @@ internal Metric AddMetricWithNoViews(Instrument instrument) var meterVersion = instrument.Meter.Version; var metricName = instrument.Name; var metricStreamName = $"{meterName}.{meterVersion}.{metricName}"; - var instrumentIdentity = new InstrumentIdentity(instrument.Meter, metricName, instrument.Unit, instrument.Description, instrument.GetType()); + var instrumentIdentity = new InstrumentIdentity(instrument.Meter, metricName, instrument.Unit, instrument.Description, instrument.GetType(), derivedFromView: false); lock (this.instrumentCreationLock) { if (this.instrumentIdentityToMetric.TryGetValue(instrumentIdentity, out var existingMetric)) @@ -119,7 +119,7 @@ internal List AddMetricsListWithViews(Instrument instrument, List AddMetricsListWithViews(Instrument instrument, List AddMetricsListWithViews(Instrument instrument, List= this.maxMetricStreams) { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index fb7406bd732..ef0b29ddd3d 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -469,6 +469,282 @@ long GetAggregatedValue(Metric metric) } } + [Fact] + public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_SameRename() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => new MetricStreamConfiguration { Name = "newname" }) + .AddView((instrument) => new MetricStreamConfiguration { Name = "newname" }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name"); + var instrument2 = meter.CreateCounter("name"); + + instrument1.Add(10); + instrument2.Add(10); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + // The second view results in a name conflict, so it is ignored + Assert.Single(exportedItems); + var metric1 = new List() { exportedItems[0] }; + Assert.Equal("newname", exportedItems[0].Name); + + // Both instruments record + Assert.Equal(20, GetLongSum(metric1)); + } + + [Fact] + public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_DifferentTags() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; + }) + .AddView((instrument) => + { + return new MetricStreamConfiguration { TagKeys = new[] { "key2" } }; + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name"); + var instrument2 = meter.CreateCounter("name"); + + var tags = new KeyValuePair[] + { + new("key1", "value"), + new("key2", "value"), + }; + + instrument1.Add(10, tags); + instrument2.Add(10, tags); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + // The second view results in a name conflict, so it is ignored + Assert.Single(exportedItems); + var metric1 = new List() { exportedItems[0] }; + var tag1 = new List> { tags[0] }; + + Assert.Equal("name", exportedItems[0].Name); + + // Both instruments record + Assert.Equal(20, GetLongSum(metric1)); + + // Only key1 is kept not key2 + CheckTagsForNthMetricPoint(metric1, tag1, 1); + } + + [Fact] + public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_SameTags() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; + }) + .AddView((instrument) => + { + return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name"); + var instrument2 = meter.CreateCounter("name"); + + var tags = new KeyValuePair[] + { + new("key1", "value"), + new("key2", "value"), + }; + + instrument1.Add(10, tags); + instrument2.Add(10, tags); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + // The second view results in a name conflict, so it is ignored + Assert.Single(exportedItems); + var metric1 = new List() { exportedItems[0] }; + + var tag1 = new List> { tags[0] }; + + Assert.Equal("name", exportedItems[0].Name); + + // Both instruments record + Assert.Equal(20, GetLongSum(metric1)); + CheckTagsForNthMetricPoint(metric1, tag1, 1); + } + + [Fact] + public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_DifferentHistogramBounds() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + return new ExplicitBucketHistogramConfiguration { Boundaries = new[] { 5.0, 10.0 } }; + }) + .AddView((instrument) => + { + return new ExplicitBucketHistogramConfiguration { Boundaries = new[] { 10.0, 20.0 } }; + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateHistogram("name"); + var instrument2 = meter.CreateHistogram("name"); + + instrument1.Record(15); + instrument2.Record(15); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + // The second view results in a name conflict, so it is ignored + Assert.Single(exportedItems); + var metric1 = exportedItems[0]; + + Assert.Equal("name", exportedItems[0].Name); + + var metricPoints = new List(); + foreach (ref readonly var mp in metric1.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + Assert.Single(metricPoints); + var metricPoint = metricPoints[0]; + + // Both instruments record + Assert.Equal(2, metricPoint.GetHistogramCount()); + Assert.Equal(30, metricPoint.GetHistogramSum()); + + var index = 0; + var actualCount = 0; + var expectedBucketCounts = new long[] { 0, 0, 2 }; + foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) + { + Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); + index++; + actualCount++; + } + } + + [Fact] + public void DuplicateInstrumentRegistration_WithViews_TwoInstruments_OneView_ResultsInConflict() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + if (instrument.Name == "name") + { + return new MetricStreamConfiguration { Name = "othername", TagKeys = new[] { "key1" } }; + } + else + { + return null; + } + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name"); + var instrument2 = meter.CreateCounter("othername"); + + var tags = new KeyValuePair[] + { + new("key1", "value"), + new("key2", "value"), + }; + + instrument1.Add(10, tags); + instrument2.Add(10, tags); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.Equal(2, exportedItems.Count); + var metric1 = new List() { exportedItems[0] }; + var metric2 = new List() { exportedItems[1] }; + + var tags1 = new List> { tags[0] }; + var tags2 = new List> { tags[0], tags[1] }; + + Assert.Equal("othername", exportedItems[0].Name); + Assert.Equal("othername", exportedItems[1].Name); + + Assert.Equal(10, GetLongSum(metric1)); + Assert.Equal(10, GetLongSum(metric2)); + + CheckTagsForNthMetricPoint(metric1, tags1, 1); + CheckTagsForNthMetricPoint(metric2, tags2, 1); + } + + [Fact] + public void DuplicateInstrumentRegistration_WithViews_TwoInstruments_WouldResultInConflictButSecondInstrumentIsDropped() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + if (instrument.Name == "name") + { + return new MetricStreamConfiguration { Name = "othername" }; + } + else + { + return MetricStreamConfiguration.Drop; + } + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name"); + var instrument2 = meter.CreateCounter("othername"); + + instrument1.Add(10); + instrument2.Add(20); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.Single(exportedItems); + var metric1 = new List() { exportedItems[0] }; + + Assert.Equal("othername", exportedItems[0].Name); + Assert.Equal(10, GetLongSum(metric1)); + } + [Fact] public void DuplicateInstrumentNamesFromDifferentMetersWithSameNameDifferentVersion() {