From b5d1011c6c2988b2d7ae11d181eed6cddfa6c708 Mon Sep 17 00:00:00 2001 From: Dudi Likvornik Date: Sat, 16 Nov 2024 13:46:03 +0200 Subject: [PATCH] [bugFix] Meter tags are now recognized as identified when reviewing meter identity uniqueness. --- src/OpenTelemetry/Metrics/Metric.cs | 2 +- .../Metrics/MetricStreamIdentity.cs | 6 +- src/OpenTelemetry/Metrics/Tags.cs | 66 +++++++++---------- .../Metrics/MetricApiTests.cs | 34 ++++++---- 4 files changed, 61 insertions(+), 47 deletions(-) diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index ac73955dda6..d7f1a3b1a43 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -234,7 +234,7 @@ internal Metric( /// /// Gets the attributes (tags) for the metric stream. /// - public IEnumerable>? MeterTags => this.InstrumentIdentity.MeterTags; + public IEnumerable>? MeterTags => this.InstrumentIdentity.MeterTags?.KeyValuePairs; /// /// Gets the for the metric stream. diff --git a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs index a9918fb6eaa..cabb63a3337 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs @@ -14,7 +14,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration? me { this.MeterName = instrument.Meter.Name; this.MeterVersion = instrument.Meter.Version ?? string.Empty; - this.MeterTags = instrument.Meter.Tags; + this.MeterTags = instrument.Meter.Tags != null ? new Tags(instrument.Meter.Tags.ToArray()) : null; this.InstrumentName = metricStreamConfiguration?.Name ?? instrument.Name; this.Unit = instrument.Unit ?? string.Empty; this.Description = metricStreamConfiguration?.Description ?? instrument.Description ?? string.Empty; @@ -32,6 +32,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration? me hashCode.Add(this.InstrumentType); hashCode.Add(this.MeterName); hashCode.Add(this.MeterVersion); + hashCode.Add(this.MeterTags); hashCode.Add(this.InstrumentName); hashCode.Add(this.HistogramRecordMinMax); hashCode.Add(this.Unit); @@ -91,7 +92,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration? me public string MeterVersion { get; } - public IEnumerable>? MeterTags { get; } + public Tags? MeterTags { get; } public string InstrumentName { get; } @@ -141,6 +142,7 @@ public bool Equals(MetricStreamIdentity other) && this.Unit == other.Unit && this.Description == other.Description && this.ViewId == other.ViewId + && this.MeterTags == other.MeterTags && this.HistogramRecordMinMax == other.HistogramRecordMinMax && this.ExponentialHistogramMaxSize == other.ExponentialHistogramMaxSize && this.ExponentialHistogramMaxScale == other.ExponentialHistogramMaxScale diff --git a/src/OpenTelemetry/Metrics/Tags.cs b/src/OpenTelemetry/Metrics/Tags.cs index ad166ebccad..5175f60cf5a 100644 --- a/src/OpenTelemetry/Metrics/Tags.cs +++ b/src/OpenTelemetry/Metrics/Tags.cs @@ -27,6 +27,39 @@ public Tags(KeyValuePair[] keyValuePairs) public static bool operator !=(Tags tag1, Tags tag2) => !tag1.Equals(tag2); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static int ComputeHashCode(KeyValuePair[] keyValuePairs) + { + Debug.Assert(keyValuePairs != null, "keyValuePairs was null"); + +#if NET + HashCode hashCode = default; + + for (int i = 0; i < keyValuePairs.Length; i++) + { + ref var item = ref keyValuePairs[i]; + hashCode.Add(item.Key.GetHashCode()); + hashCode.Add(item.Value); + } + + return hashCode.ToHashCode(); +#else + var hash = 17; + + for (int i = 0; i < keyValuePairs!.Length; i++) + { + ref var item = ref keyValuePairs[i]; + unchecked + { + hash = (hash * 31) + item.Key.GetHashCode(); + hash = (hash * 31) + (item.Value?.GetHashCode() ?? 0); + } + } + + return hash; +#endif + } + public override readonly bool Equals(object? obj) { return obj is Tags other && this.Equals(other); @@ -98,37 +131,4 @@ public readonly bool Equals(Tags other) } public override readonly int GetHashCode() => this.hashCode; - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static int ComputeHashCode(KeyValuePair[] keyValuePairs) - { - Debug.Assert(keyValuePairs != null, "keyValuePairs was null"); - -#if NET - HashCode hashCode = default; - - for (int i = 0; i < keyValuePairs.Length; i++) - { - ref var item = ref keyValuePairs[i]; - hashCode.Add(item.Key.GetHashCode()); - hashCode.Add(item.Value); - } - - return hashCode.ToHashCode(); -#else - var hash = 17; - - for (int i = 0; i < keyValuePairs!.Length; i++) - { - ref var item = ref keyValuePairs[i]; - unchecked - { - hash = (hash * 31) + item.Key.GetHashCode(); - hash = (hash * 31) + (item.Value?.GetHashCode() ?? 0); - } - } - - return hash; -#endif - } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricApiTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricApiTests.cs index ff7205e8009..9e9a753fac6 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricApiTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricApiTests.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.Diagnostics.Metrics; +using System.Threading.Tasks; using OpenTelemetry.Exporter; using OpenTelemetry.Internal; using OpenTelemetry.Tests; @@ -192,11 +193,10 @@ public void MetricInstrumentationScopeIsExportedCorrectly() } [Fact] - public void MetricInstrumentationScopeAttributesAreNotTreatedAsIdentifyingProperty() + public void MetricInstrumentationScopeAttributesAreTreatedAsIdentifyingProperty() { // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#get-a-meter - // Meters are identified by name, version, and schema_url fields - // and not with tags. + // Meters are identified by name, version, meter tags and schema_url fields. var exportedItems = new List(); var meterName = "MyMeter"; var meterVersion = "1.0"; @@ -228,16 +228,13 @@ public void MetricInstrumentationScopeAttributesAreNotTreatedAsIdentifyingProper // The first instrument's Meter.Tags is exported. // It is considered a user-error to create Meters with same name,version but with // different tags. TODO: See if we can emit an internal log about this. - Assert.Single(exportedItems); - var metric = exportedItems[0]; + Assert.Equal(2, exportedItems.Count); + + var meterTagHashed = Tags.ComputeHashCode(meterTags1.ToArray()); + var metric = exportedItems.First(m => Tags.ComputeHashCode(m.MeterTags?.ToArray() ?? Array.Empty>()) == meterTagHashed); Assert.Equal(meterName, metric.MeterName); Assert.Equal(meterVersion, metric.MeterVersion); - Assert.NotNull(metric.MeterTags); - - Assert.Single(metric.MeterTags.Where(kvp => kvp.Key == meterTags1[0].Key && kvp.Value == meterTags1[0].Value)); - Assert.DoesNotContain(metric.MeterTags, kvp => kvp.Key == meterTags2[0].Key && kvp.Value == meterTags2[0].Value); - List metricPoints = new List(); foreach (ref readonly var mp in metric.GetMetricPoints()) { @@ -246,7 +243,22 @@ public void MetricInstrumentationScopeAttributesAreNotTreatedAsIdentifyingProper Assert.Single(metricPoints); var metricPoint1 = metricPoints[0]; - Assert.Equal(25, metricPoint1.GetSumLong()); + Assert.Equal(10, metricPoint1.GetSumLong()); + + meterTagHashed = Tags.ComputeHashCode(meterTags2.ToArray()); + metric = exportedItems.First(m => Tags.ComputeHashCode(m.MeterTags?.ToArray() ?? Array.Empty>()) == meterTagHashed); + Assert.Equal(meterName, metric.MeterName); + Assert.Equal(meterVersion, metric.MeterVersion); + + metricPoints = new List(); + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + Assert.Single(metricPoints); + metricPoint1 = metricPoints[0]; + Assert.Equal(15, metricPoint1.GetSumLong()); } [Fact]