Resource attributes fix (#1720)

* Redoing changes from old PR1647 * Changelog Co-authored-by: Cijo Thomas <[email protected]>
open-telemetry · Jan 26, 2021 · 842daff · 842daff
1 parent 17dd4a7
commit 842daff
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 33 deletions.
diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md
@@ -22,6 +22,10 @@
 * Added check in `ActivitySourceAdapter` class for root activity if traceid is
   overridden by calling `SetParentId`
   ([#1355](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1355))
+* Resource Attributes now accept int, short, and float as values, converting
+  them to supported data types (long for int/short, double for float). For
+  invalid attributes we now throw an exception instead of logging an error.
+  ([#1720](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1720))
 
 ## 1.0.0-rc1.1
 

diff --git a/src/OpenTelemetry/Resources/Resource.cs b/src/OpenTelemetry/Resources/Resource.cs
@@ -101,28 +101,33 @@ private static KeyValuePair<string, object> SanitizeAttribute(KeyValuePair<strin
                 sanitizedKey = attribute.Key;
             }
 
-            object sanitizedValue;
-            if (!IsValidValue(attribute.Value))
-            {
-                OpenTelemetrySdkEventSource.Log.InvalidArgument("Create resource", "attribute value", "Attribute value should be a non-null string, long, bool or double.");
-                sanitizedValue = string.Empty;
-            }
-            else
-            {
-                sanitizedValue = attribute.Value;
-            }
-
+            object sanitizedValue = SanitizeValue(attribute.Value, sanitizedKey);
             return new KeyValuePair<string, object>(sanitizedKey, sanitizedValue);
         }
 
-        private static bool IsValidValue(object value)
+        private static object SanitizeValue(object value, string keyName)
         {
-            if (value != null && (value is string || value is bool || value is long || value is double))
+            if (value != null)
             {
-                return true;
+                if (value is string || value is bool || value is long || value is double)
+                {
+                    return value;
+                }
+
+                if (value is int || value is short)
+                {
+                    return System.Convert.ToInt64(value);
+                }
+
+                if (value is float)
+                {
+                    return System.Convert.ToDouble(value, System.Globalization.CultureInfo.InvariantCulture);
+                }
+
+                throw new System.ArgumentException("Attribute value type is not an accepted primitive", keyName);
             }
 
-            return false;
+            throw new System.ArgumentException("Attribute value is null", keyName);
         }
     }
 }
diff --git a/test/OpenTelemetry.Tests/Resources/ResourceTest.cs b/test/OpenTelemetry.Tests/Resources/ResourceTest.cs
@@ -46,15 +46,8 @@ public void CreateResource_NullAttributeValue()
             // Arrange
             var attributes = new Dictionary<string, object> { { "NullValue", null } };
 
-            // Act
-            var resource = new Resource(attributes);
-
-            // Assert
-            Assert.Single(resource.Attributes);
-
-            var attribute = resource.Attributes.Single();
-            Assert.Equal("NullValue", attribute.Key);
-            Assert.Empty((string)attribute.Value);
+            // Act and Assert
+            Assert.Throws<ArgumentException>(() => new Resource(attributes));
         }
 
         [Fact]
@@ -139,15 +132,25 @@ public void CreateResource_SupportedAttributeTypes()
                 { "long", 1L },
                 { "bool", true },
                 { "double", 0.1d },
+
+                // int and float supported by conversion to long and double
+                { "int", 1 },
+                { "short", (short)1 },
+                { "float", 0.1f },
             };
 
             var resource = new Resource(attributes);
 
-            Assert.Equal(4, resource.Attributes.Count());
+            Assert.Equal(7, resource.Attributes.Count());
             Assert.Contains(new KeyValuePair<string, object>("string", "stringValue"), resource.Attributes);
             Assert.Contains(new KeyValuePair<string, object>("long", 1L), resource.Attributes);
             Assert.Contains(new KeyValuePair<string, object>("bool", true), resource.Attributes);
             Assert.Contains(new KeyValuePair<string, object>("double", 0.1d), resource.Attributes);
+            Assert.Contains(new KeyValuePair<string, object>("int", 1L), resource.Attributes);
+            Assert.Contains(new KeyValuePair<string, object>("short", 1L), resource.Attributes);
+
+            double convertedFloat = Convert.ToDouble(0.1f, System.Globalization.CultureInfo.InvariantCulture);
+            Assert.Contains(new KeyValuePair<string, object>("float", convertedFloat), resource.Attributes);
         }
 
         [Fact]
@@ -158,16 +161,9 @@ public void CreateResource_NotSupportedAttributeTypes()
                 { "dynamic", new { } },
                 { "array", new int[1] },
                 { "complex", this },
-                { "float", 0.1f },
             };
 
-            var resource = new Resource(attributes);
-
-            Assert.Equal(4, resource.Attributes.Count());
-            Assert.Contains(new KeyValuePair<string, object>("dynamic", string.Empty), resource.Attributes);
-            Assert.Contains(new KeyValuePair<string, object>("array", string.Empty), resource.Attributes);
-            Assert.Contains(new KeyValuePair<string, object>("complex", string.Empty), resource.Attributes);
-            Assert.Contains(new KeyValuePair<string, object>("float", string.Empty), resource.Attributes);
+            Assert.Throws<ArgumentException>(() => new Resource(attributes));
         }
 
         [Fact]