Merge pull request #163 from nblumhardt/header-parsing

Parse `OTEL_` header and resource attribute vars according to spec

Author: nblumhardt

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);
+    }
+}

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;
         }
     }
 }

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());
+    }
+}

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