From a2aa3057a6d55e409855f2f97fb4db0c983e4b62 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Fri, 3 Jun 2022 15:38:51 -0700 Subject: [PATCH] Fix NullReferenceException caught by SDK when metric has a tag with a null value (#3325) --- src/OpenTelemetry/CHANGELOG.md | 4 +++ src/OpenTelemetry/Metrics/Tags.cs | 28 ++++++++-------- .../Metrics/MetricAPITest.cs | 33 +++++++++++++++++++ 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 883ff74ab3a..6f1df629e44 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Fix issue where a measurement would be dropped when recording it with a + null-valued tag. + ([#3325](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3325)) + ## 1.3.0 Released 2022-Jun-03 diff --git a/src/OpenTelemetry/Metrics/Tags.cs b/src/OpenTelemetry/Metrics/Tags.cs index 6f3b4ba62dd..e7996bd846a 100644 --- a/src/OpenTelemetry/Metrics/Tags.cs +++ b/src/OpenTelemetry/Metrics/Tags.cs @@ -20,10 +20,23 @@ namespace OpenTelemetry.Metrics { internal readonly struct Tags : IEquatable { + private readonly int hashCode; + public Tags(string[] keys, object[] values) { this.Keys = keys; this.Values = values; + + unchecked + { + var hash = 17; + for (int i = 0; i < this.Keys.Length; i++) + { + hash = (hash * 31) + this.Keys[i].GetHashCode() + this.Values[i]?.GetHashCode() ?? 0; + } + + this.hashCode = hash; + } } public readonly string[] Keys { get; } @@ -78,19 +91,6 @@ public readonly bool Equals(Tags other) return true; } - public override readonly int GetHashCode() - { - int hash = 17; - - unchecked - { - for (int i = 0; i < this.Keys.Length; i++) - { - hash = (hash * 31) + this.Keys[i].GetHashCode() + this.Values[i].GetHashCode(); - } - } - - return hash; - } + public override readonly int GetHashCode() => this.hashCode; } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 3b70cf5f022..a4357f5e8bd 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -42,6 +42,39 @@ public MetricApiTest(ITestOutputHelper output) this.output = output; } + [Fact] + public void MeasurementWithNullValuedTag() + { + using var meter = new Meter(Utils.GetCurrentMethodName()); + var exportedItems = new List(); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + var counter = meter.CreateCounter("myCounter"); + counter.Add(100, new KeyValuePair("tagWithNullValue", null)); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + Assert.Single(exportedItems); + var metric = exportedItems[0]; + Assert.Equal("myCounter", metric.Name); + List metricPoints = new List(); + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + Assert.Single(metricPoints); + var metricPoint = metricPoints[0]; + Assert.Equal(100, metricPoint.GetSumLong()); + Assert.Equal(1, metricPoint.Tags.Count); + var tagEnumerator = metricPoint.Tags.GetEnumerator(); + tagEnumerator.MoveNext(); + Assert.Equal("tagWithNullValue", tagEnumerator.Current.Key); + Assert.Null(tagEnumerator.Current.Value); + } + [Fact] public void ObserverCallbackTest() {