Map dictionary values to maps, consistent with Serilog's JsonValueFormatter (#88)

Author: nblumhardt

Build.ps1

@@ -24,14 +24,14 @@ foreach ($src in Get-ChildItem src/*) {
 	Write-Output "build: Packaging project in $src"
 
     & dotnet build -c Release --version-suffix=$buildSuffix
-    if($LASTEXITCODE -ne 0) { exit 1 }
+    if($LASTEXITCODE -ne 0) { throw "Failed" }
 
     if($suffix) {
         & dotnet pack -c Release --include-source --no-build -o ../../artifacts --version-suffix=$suffix
     } else {
         & dotnet pack -c Release --include-source --no-build -o ../../artifacts
     }
-    if($LASTEXITCODE -ne 0) { exit 1 }
+    if($LASTEXITCODE -ne 0) { throw "Failed" }
 
     Pop-Location
 }
@@ -42,7 +42,7 @@ foreach ($test in Get-ChildItem test/*.Tests) {
 	Write-Output "build: Testing project in $test"
 
     & dotnet test -c Release
-    if($LASTEXITCODE -ne 0) { exit 3 }
+    if($LASTEXITCODE -ne 0) { throw "Failed" }
 
     Pop-Location
 }
@@ -53,7 +53,7 @@ foreach ($test in ls test/*.PerformanceTests) {
 	Write-Output "build: Building performance test project in $test"
 
     & dotnet build -c Release
-    if($LASTEXITCODE -ne 0) { exit 2 }
+    if($LASTEXITCODE -ne 0) { throw "Failed" }
 
     Pop-Location
 }

src/Serilog.Sinks.OpenTelemetry/Sinks/OpenTelemetry/ProtocolHelpers/PrimitiveConversions.cs

@@ -120,8 +120,17 @@ public static AnyValue ToOpenTelemetryMap(StructureValue value)
         var map = new AnyValue();
         var kvList = new KeyValueList();
         map.KvlistValue = kvList;
+        
+        // Per the OTLP protos, attribute keys MUST be unique.
+        var seen = new HashSet<string>();
+        
         foreach (var prop in value.Properties)
         {
+            if (seen.Contains(prop.Name))
+                continue;
+
+            seen.Add(prop.Name);
+
             var v = ToOpenTelemetryAnyValue(prop.Value);
             var kv = new KeyValue
             {
@@ -134,29 +143,24 @@ public static AnyValue ToOpenTelemetryMap(StructureValue value)
         return map;
     }
 
-    public static AnyValue ToOpenTelemetryArray(DictionaryValue value)
+    public static AnyValue ToOpenTelemetryMap(DictionaryValue value)
     {
-        var array = new AnyValue();
-        var values = new ArrayValue();
-        array.ArrayValue = values;
+        var map = new AnyValue();
+        var kvList = new KeyValueList();
+        map.KvlistValue = kvList;
+
         foreach (var element in value.Elements)
         {
-            var v = new AnyValue();
-            var kvList = new KeyValueList();
-            v.KvlistValue = kvList;
-            kvList.Values.Add(new KeyValue
-            {
-                Key = "Key",
-                Value = ToOpenTelemetryAnyValue(element.Key)
-            });
+            var k = element.Key.Value?.ToString() ?? "null";
+            var v = ToOpenTelemetryAnyValue(element.Value);
             kvList.Values.Add(new KeyValue
             {
-                Key = "Value",
-                Value = ToOpenTelemetryAnyValue(element.Value)
+                Key = k,
+                Value = v
             });
-            values.Values.Add(v);
         }
-        return array;
+        
+        return map;
     }
 
     public static AnyValue ToOpenTelemetryArray(SequenceValue value)
@@ -177,9 +181,9 @@ internal static AnyValue ToOpenTelemetryAnyValue(LogEventPropertyValue value)
         return value switch
         {
             ScalarValue scalar => ToOpenTelemetryScalar(scalar),
-            StructureValue map => ToOpenTelemetryMap(map),
-            SequenceValue array => ToOpenTelemetryArray(array),
-            DictionaryValue d => ToOpenTelemetryArray(d),
+            StructureValue structure => ToOpenTelemetryMap(structure),
+            SequenceValue sequence => ToOpenTelemetryArray(sequence),
+            DictionaryValue dictionary => ToOpenTelemetryMap(dictionary),
             _ => ToOpenTelemetryPrimitive(value.ToString()),
         };
     }

test/Serilog.Sinks.OpenTelemetry.Tests/PrimitiveConversionsTests.cs

@@ -285,4 +285,37 @@ public void TestMd5Hash()
         Assert.Equal(PrimitiveConversions.Md5Hash("alpha"), PrimitiveConversions.Md5Hash("alpha"));
         Assert.NotEqual(PrimitiveConversions.Md5Hash("alpha"), PrimitiveConversions.Md5Hash("beta"));
     }
-}
+
+    [Fact]
+    public void DictionariesMapToMaps()
+    {
+        var dict = new DictionaryValue(new[]
+        {
+            new KeyValuePair<ScalarValue, LogEventPropertyValue>(new ScalarValue(0), new ScalarValue("test"))
+        });
+
+        var any = PrimitiveConversions.ToOpenTelemetryAnyValue(dict);
+
+        Assert.NotNull(any.KvlistValue);
+        var value = Assert.Single(any.KvlistValue.Values);
+        Assert.Equal("0", value.Key);
+        Assert.Equal("test", value.Value.StringValue);
+    }
+    
+    [Fact]
+    public void StructureKeysAreDeduplicated()
+    {
+        var structure = new StructureValue(new[]
+        {
+            new LogEventProperty("a", new ScalarValue("test")),
+            new LogEventProperty("a", new ScalarValue("test")),
+            new LogEventProperty("b", new ScalarValue("test"))
+        });
+
+        Assert.Equal(3, structure.Properties.Count);
+        
+        var any = PrimitiveConversions.ToOpenTelemetryAnyValue(structure);
+
+        Assert.Equal(2, any.KvlistValue.Values.Count);
+    }
+}

test/Serilog.Sinks.OpenTelemetry.Tests/PublicApiVisibilityTests.approved.txt

@@ -4,8 +4,8 @@
     {
         public static Serilog.LoggerConfiguration OpenTelemetry(this Serilog.Configuration.LoggerAuditSinkConfiguration loggerAuditSinkConfiguration, System.Action<Serilog.Sinks.OpenTelemetry.OpenTelemetrySinkOptions> configure) { }
         public static Serilog.LoggerConfiguration OpenTelemetry(this Serilog.Configuration.LoggerSinkConfiguration loggerSinkConfiguration, System.Action<Serilog.Sinks.OpenTelemetry.BatchedOpenTelemetrySinkOptions> configure) { }
-        public static Serilog.LoggerConfiguration OpenTelemetry(this Serilog.Configuration.LoggerAuditSinkConfiguration loggerAuditSinkConfiguration, string endpoint = "http://localhost:4317/v1/logs", Serilog.Sinks.OpenTelemetry.OtlpProtocol protocol = 0) { }
-        public static Serilog.LoggerConfiguration OpenTelemetry(this Serilog.Configuration.LoggerSinkConfiguration loggerSinkConfiguration, string endpoint = "http://localhost:4317/v1/logs", Serilog.Sinks.OpenTelemetry.OtlpProtocol protocol = 0) { }
+        public static Serilog.LoggerConfiguration OpenTelemetry(this Serilog.Configuration.LoggerAuditSinkConfiguration loggerAuditSinkConfiguration, string endpoint = "http://localhost:4317", Serilog.Sinks.OpenTelemetry.OtlpProtocol protocol = 0) { }
+        public static Serilog.LoggerConfiguration OpenTelemetry(this Serilog.Configuration.LoggerSinkConfiguration loggerSinkConfiguration, string endpoint = "http://localhost:4317", Serilog.Sinks.OpenTelemetry.OtlpProtocol protocol = 0) { }
     }
 }
 namespace Serilog.Sinks.OpenTelemetry

test/Serilog.Sinks.OpenTelemetry.Tests/RequiredResourceAttributeTests.cs

@@ -25,8 +25,7 @@ public void MissingServiceNameDefaultsToExecutableName()
     {
         var actual = RequiredResourceAttributes.AddDefaults(new Dictionary<string, object>());
 
-        // .exe on Windows; true for all of our current targets, but this test may need some tuning in the future.
-        Assert.StartsWith("unknown_service:dotnet", (string)actual["service.name"]);
+        Assert.StartsWith("unknown_service:", (string)actual["service.name"]);
     }
 
     [Fact]