Fix NullReferenceException caught by SDK when metric has a tag with a…

… null value (#3325)
open-telemetry · Jun 3, 2022 · a2aa305 · a2aa305
1 parent 5006370
commit a2aa305
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 14 deletions.
diff --git 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
@@ -20,10 +20,23 @@ namespace OpenTelemetry.Metrics
 {
     internal readonly struct Tags : IEquatable<Tags>
     {
+        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
@@ -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<Metric>();
+            using var meterProvider = Sdk.CreateMeterProviderBuilder()
+                .AddMeter(meter.Name)
+                .AddInMemoryExporter(exportedItems)
+                .Build();
+
+            var counter = meter.CreateCounter<long>("myCounter");
+            counter.Add(100, new KeyValuePair<string, object>("tagWithNullValue", null));
+
+            meterProvider.ForceFlush(MaxTimeToAllowForFlush);
+            Assert.Single(exportedItems);
+            var metric = exportedItems[0];
+            Assert.Equal("myCounter", 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(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()
         {