Merge pull request #164 from serilog/dev

nblumhardt · web-flow · commit b894851cb04a · 2024-09-25T09:52:23.000+10:00
4.1.1 Release
diff --git a/.gitignore b/.gitignore
@@ -37,6 +37,12 @@ bld/
 # Uncomment if you have tasks that create the project's static files in wwwroot
 #wwwroot/
 
+# JetBrains Rider cache
+.idea/
+
+# VSCode cache
+.vscode/
+
 # Visual Studio 2017 auto generated files
 Generated\ Files/
 
diff --git a/.idea/.idea.serilog-sinks-opentelemetry/.idea/.gitignore b/.idea/.idea.serilog-sinks-opentelemetry/.idea/.gitignore
diff --git a/.idea/.idea.serilog-sinks-opentelemetry/.idea/encodings.xml b/.idea/.idea.serilog-sinks-opentelemetry/.idea/encodings.xml
diff --git a/.idea/.idea.serilog-sinks-opentelemetry/.idea/indexLayout.xml b/.idea/.idea.serilog-sinks-opentelemetry/.idea/indexLayout.xml
diff --git a/.idea/.idea.serilog-sinks-opentelemetry/.idea/vcs.xml b/.idea/.idea.serilog-sinks-opentelemetry/.idea/vcs.xml
diff --git a/.vscode/launch.json b/.vscode/launch.json
diff --git a/.vscode/tasks.json b/.vscode/tasks.json
diff --git a/README.md b/README.md
@@ -203,4 +203,10 @@ The `example/Example` subdirectory contains an example application that logs
 to a local [OpenTelemetry collector](https://opentelemetry.io/docs/collector/).
 See the README in that directory for instructions on how to run the example.
 
+## .NET Framework Activity Traces
+
+In .NET 5 and later versions, the `Activity.DefaultIdFormat` is `ActivityIdFormat.W3C`. In previous versions, the default format is `ActivityIdFormat.Hierarchical`.
+To make use of the **Activity**'s traces and spans, you should set the global `Activity.DefaultIdFormat` to `ActivityIdFormat.W3C` in .NET Framework environments.
+Read more: [Default ActivityIdFormat is W3C](https://learn.microsoft.com/en-us/dotnet/core/compatibility/core-libraries/5.0/default-activityidformat-changed)
+
 _Copyright &copy; Serilog Contributors - Provided under the [Apache License, Version 2.0](http://apache.org/licenses/LICENSE-2.0.html)._
diff --git a/global.json b/global.json
@@ -1,6 +1,6 @@
 {
   "sdk": {
-    "version": "8.0.204",
+    "version": "8.0.401",
     "allowPrerelease": false,
     "rollForward": "latestFeature"
   }
diff --git a/src/Serilog.Sinks.OpenTelemetry/Serilog.Sinks.OpenTelemetry.csproj b/src/Serilog.Sinks.OpenTelemetry/Serilog.Sinks.OpenTelemetry.csproj
@@ -2,7 +2,7 @@
     <PropertyGroup>
         <Description>This Serilog sink transforms Serilog events into OpenTelemetry
             logs and sends them to an OTLP (gRPC or HTTP) endpoint.</Description>
-        <VersionPrefix>4.1.0</VersionPrefix>
+        <VersionPrefix>4.1.1</VersionPrefix>
         <Authors>Serilog Contributors</Authors>
         <!-- .NET Framework version targeting is frozen at these two TFMs. -->
         <TargetFrameworks Condition=" '$(OS)' == 'Windows_NT'">net471;net462</TargetFrameworks>
diff --git a/src/Serilog.Sinks.OpenTelemetry/Sinks/OpenTelemetry/Configuration/BaggageFormat.cs b/src/Serilog.Sinks.OpenTelemetry/Sinks/OpenTelemetry/Configuration/BaggageFormat.cs
@@ -0,0 +1,40 @@
+namespace Serilog.Sinks.OpenTelemetry.Configuration;
+
+static class BaggageFormat
+{
+    /// <summary>
+    /// Decode W3C Baggage-formatted key-value pairs as specified for handling of the `OTEL_EXPORTER_OTLP_HEADERS` and
+    /// `OTEL_RESOURCE_ATTRIBUTES` environment variables.
+    /// </summary>
+    /// <returns>The property names and values encoded in the supplied <paramref name="baggageString"/>.</returns>
+    public static IEnumerable<(string, string)> DecodeBaggageString(string baggageString, string environmentVariableName)
+    {
+        // See: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#specifying-resource-information-via-an-environment-variable
+        // See: https://www.w3.org/TR/baggage/#header-content
+        
+        if (string.IsNullOrWhiteSpace(baggageString)) yield break;
+        
+        foreach (var listMember in baggageString.Split(','))
+        {
+            // The baggage spec allows list members to carry additional key-value pair metadata after the initial
+            // key and value and a trailing semicolon, but this is disallowed by the OTel spec. We're pretty loose with
+            // validation, here, but could tighten up handling of invalid values in the future.
+
+            var eq = listMember.IndexOf('=');
+            if (eq == -1) RejectInvalidListMember(listMember, environmentVariableName, nameof(baggageString));
+
+            var key = listMember.Substring(0, eq).Trim();
+            if (string.IsNullOrEmpty(key)) RejectInvalidListMember(listMember, environmentVariableName, nameof(baggageString));
+
+            var escapedValue = eq == listMember.Length - 1 ? "" : listMember.Substring(eq + 1).Trim();
+            var value = Uri.UnescapeDataString(escapedValue);
+
+            yield return (key, value);
+        }
+    }
+
+    static void RejectInvalidListMember(string listMember, string environmentVariableName, string paramName)
+    {
+        throw new ArgumentException($"Invalid item format `{listMember}` in {environmentVariableName} environment variable.", paramName);
+    }
+}
diff --git a/src/Serilog.Sinks.OpenTelemetry/Sinks/OpenTelemetry/Configuration/OpenTelemetryEnvironment.cs b/src/Serilog.Sinks.OpenTelemetry/Sinks/OpenTelemetry/Configuration/OpenTelemetryEnvironment.cs
@@ -46,23 +46,19 @@ public static void Configure(BatchedOpenTelemetrySinkOptions options, Func<strin
 
     static void FillHeadersIfPresent(string? config, IDictionary<string, string> headers)
     {
-        foreach (var part in config?.Split(',') ?? [])
+        if (config == null) return;
+        foreach (var (key, value) in BaggageFormat.DecodeBaggageString(config, HeaderVarName))
         {
-            if (part.Split('=') is { Length: 2 } parts)
-                headers[parts[0]] = parts[1];
-            else
-                throw new InvalidOperationException($"Invalid header format `{part}` in {HeaderVarName} environment variable.");
+            headers[key] = value;
         }
     }
 
     static void FillHeadersResourceAttributesIfPresent(string? config, IDictionary<string, object> resourceAttributes)
     {
-        foreach (var part in config?.Split(',') ?? [])
+        if (config == null) return;
+        foreach (var (key, value) in BaggageFormat.DecodeBaggageString(config, ResourceAttributesVarName))
         {
-            if (part.Split('=') is { Length: 2 } parts)
-                resourceAttributes[parts[0]] = parts[1];
-            else
-                throw new InvalidOperationException($"Invalid resource attributes format `{part}` in {ResourceAttributesVarName} environment variable.");
+            resourceAttributes[key] = value;
         }
     }
 }
diff --git a/test/Serilog.Sinks.OpenTelemetry.Tests/BaggageFormatTests.cs b/test/Serilog.Sinks.OpenTelemetry.Tests/BaggageFormatTests.cs
@@ -0,0 +1,39 @@
+using Serilog.Sinks.OpenTelemetry.Configuration;
+using Serilog.Sinks.OpenTelemetry.Tests.Support;
+using Xunit;
+
+namespace Serilog.Sinks.OpenTelemetry.Tests;
+
+public class BaggageFormatTests
+{
+    public static TheoryData<string, (string, string)[]> Cases => new()
+    {
+        { "", [] },
+        { " ", [] },
+        { "a=", [("a", "")] },
+        { "abc=def", [("abc", "def")] },
+        { "abc= def ", [("abc", "def")] },
+        { "abc=def,ghi=jkl", [("abc", "def"), ("ghi", "jkl")] },
+        { "a=1%202", [("a", "1 2")] },
+        { "a=b=c,d=e", [("a", "b=c"), ("d", "e")] },
+        { "a=%2C,b=c", [("a", ","), ("b", "c")] }
+    };
+    
+    [Theory, MemberData(nameof(Cases))]
+    public void BaggageStringsAreDecoded(string baggageString, IEnumerable<(string, string)> expected)
+    {
+        var actual = BaggageFormat.DecodeBaggageString(baggageString, Some.String());
+        Assert.Equal(expected, actual);
+    }
+
+    [Theory]
+    [InlineData(",")]
+    [InlineData(", ")]
+    [InlineData("a")]
+    [InlineData("=")]
+    [InlineData(",=")]
+    public void InvalidBaggageStringsAreRejected(string baggageString)
+    {
+        Assert.Throws<ArgumentException>(() => BaggageFormat.DecodeBaggageString(baggageString, Some.String()).ToList());
+    }
+}
diff --git a/test/Serilog.Sinks.OpenTelemetry.Tests/OpenTelemetryEnvironmentTests.cs b/test/Serilog.Sinks.OpenTelemetry.Tests/OpenTelemetryEnvironmentTests.cs
@@ -88,9 +88,9 @@ public void ConfigureThrowsIfHeaderEnvIsInvalidFormat()
         BatchedOpenTelemetrySinkOptions options = new();
         var headers = "header1";
 
-        var exception = Assert.Throws<InvalidOperationException>(() => OpenTelemetryEnvironment.Configure(options, GetEnvVar));
+        var exception = Assert.Throws<ArgumentException>(() => OpenTelemetryEnvironment.Configure(options, GetEnvVar));
 
-        Assert.Equal("Invalid header format `header1` in OTEL_EXPORTER_OTLP_HEADERS environment variable.", exception.Message);
+        Assert.StartsWith("Invalid item format `header1` in OTEL_EXPORTER_OTLP_HEADERS environment variable.", exception.Message);
 
         string? GetEnvVar(string name)
              => name switch
@@ -106,9 +106,9 @@ public void ConfigureThrowsIfResourceAttributesEnvIsInvalidFormat()
         BatchedOpenTelemetrySinkOptions options = new();
         var resourceAttributes = "resource1";
 
-        var exception = Assert.Throws<InvalidOperationException>(() => OpenTelemetryEnvironment.Configure(options, GetEnvVar));
+        var exception = Assert.Throws<ArgumentException>(() => OpenTelemetryEnvironment.Configure(options, GetEnvVar));
 
-        Assert.Equal("Invalid resource attributes format `resource1` in OTEL_RESOURCE_ATTRIBUTES environment variable.", exception.Message);
+        Assert.StartsWith("Invalid item format `resource1` in OTEL_RESOURCE_ATTRIBUTES environment variable.", exception.Message);
 
         string? GetEnvVar(string name)
              => name switch
diff --git a/test/Serilog.Sinks.OpenTelemetry.Tests/OtlpEventBuilderTests.cs b/test/Serilog.Sinks.OpenTelemetry.Tests/OtlpEventBuilderTests.cs
@@ -161,7 +161,9 @@ public void IncludeTraceIdAndSpanId()
         using var listener = new ActivityListener();
         listener.ShouldListenTo = _ => true;
         listener.Sample = (ref ActivityCreationOptions<ActivityContext> _) => ActivitySamplingResult.AllData;
-
+        #if NETFRAMEWORK
+        Activity.DefaultIdFormat = ActivityIdFormat.W3C;
+        #endif
         ActivitySource.AddActivityListener(listener);
 
         var source = new ActivitySource("test.activity", "1.0.0");
diff --git a/test/Serilog.Sinks.OpenTelemetry.Tests/PrimitiveConversionsTests.cs b/test/Serilog.Sinks.OpenTelemetry.Tests/PrimitiveConversionsTests.cs
@@ -64,8 +64,10 @@ public void TestToSeverityNumber()
             {LogEventLevel.Fatal, SeverityNumber.Fatal},
         };
 
-        foreach (var (level, severity) in data)
+        foreach (var kvp in data)
         {
+            var severity = kvp.Value;
+            var level = kvp.Key;
             Assert.Equal(severity, PrimitiveConversions.ToSeverityNumber(level));
         }
     }
@@ -266,8 +268,10 @@ public void TestOnlyHexDigits()
             ["\"123\""] = "123",
         };
 
-        foreach (var (input, expected) in tests)
+        foreach (var kvp in tests)
         {
+            var input = kvp.Key;
+            var expected = kvp.Value;
             Assert.Equal(expected, PrimitiveConversions.OnlyHexDigits(input));
         }
     }
diff --git a/test/Serilog.Sinks.OpenTelemetry.Tests/RequiredResourceAttributeTests.cs b/test/Serilog.Sinks.OpenTelemetry.Tests/RequiredResourceAttributeTests.cs
@@ -11,21 +11,18 @@ public class RequiredResourceAttributeTests
     public void ServiceNameIsPreservedWhenPresent()
     {
         var supplied = Some.String();
-        var ra = new Dictionary<string, object>
-        {
-            ["service.name"] = supplied
-        };
+        var ra = new Dictionary<string, object> { ["service.name"] = supplied };
 
         var actual = RequiredResourceAttributes.AddDefaults(ra);
-        
+
         Assert.Equal(supplied, actual["service.name"]);
     }
 
     [Fact]
     public void MissingServiceNameDefaultsToExecutableName()
     {
         var actual = RequiredResourceAttributes.AddDefaults(new Dictionary<string, object>());
-        
+
         Assert.StartsWith("unknown_service:", (string)actual["service.name"]);
     }
 
@@ -36,6 +33,13 @@ public void MissingTelemetrySdkGroupDefaultsToKnownValues()
         Assert.Equal("serilog", actual["telemetry.sdk.name"]);
         Assert.Equal("dotnet", actual["telemetry.sdk.language"]);
         // First character of the version is always expected to be numeric.
-        Assert.True(int.TryParse(((string)actual["telemetry.sdk.version"])[..1], NumberStyles.Integer, CultureInfo.InvariantCulture, out _));
+        Assert.True(
+            int.TryParse(
+                ((string)actual["telemetry.sdk.version"])[0].ToString(),
+                NumberStyles.Integer,
+                CultureInfo.InvariantCulture,
+                out _
+            )
+        );
     }
 }
diff --git a/test/Serilog.Sinks.OpenTelemetry.Tests/Serilog.Sinks.OpenTelemetry.Tests.csproj b/test/Serilog.Sinks.OpenTelemetry.Tests/Serilog.Sinks.OpenTelemetry.Tests.csproj
@@ -1,9 +1,20 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFrameworks Condition=" '$(OS)' == 'Windows_NT'">net471;net462</TargetFrameworks>
+    <!-- Policy is to trim TFM-specific builds to `netstandard2.0`, `net6.0`,
+        all active LTS versions, and optionally the latest RTM version, when releasing new
+        major Serilog versions. -->
+    <TargetFrameworks>$(TargetFrameworks);net8.0;net6.0</TargetFrameworks>
     <GenerateRuntimeConfigurationFiles>true</GenerateRuntimeConfigurationFiles>
     <IsPackable>False</IsPackable>
+    <NoWarn>$(NoWarn);NU1701</NoWarn>
+    <LangVersion>12</LangVersion>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(TargetFramework)' == 'net471' OR '$(TargetFramework)' == 'net462'">
+    <DebugType>full</DebugType>
+    <Optimize>false</Optimize>
   </PropertyGroup>
 
   <ItemGroup>

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"sdk": {`
`3`		`- "version": "8.0.204",`
	`3`	`+ "version": "8.0.401",`
`4`	`4`	`"allowPrerelease": false,`
`5`	`5`	`"rollForward": "latestFeature"`
`6`	`6`	`}`