Set the baggage even when the trace id is not successfully extracted (#55341)

* Set the baggage even when the trace id is not successfully extracted

* Make comment more explicit.

Co-authored-by: Noah Falk <[email protected]>

---------

Co-authored-by: joegoldman2 <[email protected]>
Co-authored-by: Andrew Casey <[email protected]>
Co-authored-by: Noah Falk <[email protected]>

Author: 3 people

src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs

@@ -430,28 +430,33 @@ private void RecordRequestStartMetrics(HttpContext httpContext)
             }
         }
 
+        // The trace id was successfully extracted, so we can set the trace state
+        // https://www.w3.org/TR/trace-context/#tracestate-header
         if (!string.IsNullOrEmpty(requestId))
         {
             if (!string.IsNullOrEmpty(traceState))
             {
                 activity.TraceStateString = traceState;
             }
-            var baggage = _propagator.ExtractBaggage(headers, static (object? carrier, string fieldName, out string? fieldValue, out IEnumerable<string>? fieldValues) =>
-            {
-                fieldValues = default;
-                var headers = (IHeaderDictionary)carrier!;
-                fieldValue = headers[fieldName];
-            });
+        }
 
-            // AddBaggage adds items at the beginning  of the list, so we need to add them in reverse to keep the same order as the client
-            // By contract, the propagator has already reversed the order of items so we need not reverse it again
-            // Order could be important if baggage has two items with the same key (that is allowed by the contract)
-            if (baggage is not null)
+        // Baggage can be used regardless of whether a distributed trace id was present on the inbound request.
+        // https://www.w3.org/TR/baggage/#abstract
+        var baggage = _propagator.ExtractBaggage(headers, static (object? carrier, string fieldName, out string? fieldValue, out IEnumerable<string>? fieldValues) =>
+        {
+            fieldValues = default;
+            var headers = (IHeaderDictionary)carrier!;
+            fieldValue = headers[fieldName];
+        });
+
+        // AddBaggage adds items at the beginning  of the list, so we need to add them in reverse to keep the same order as the client
+        // By contract, the propagator has already reversed the order of items so we need not reverse it again
+        // Order could be important if baggage has two items with the same key (that is allowed by the contract)
+        if (baggage is not null)
+        {
+            foreach (var baggageItem in baggage)
             {
-                foreach (var baggageItem in baggage)
-                {
-                    activity.AddBaggage(baggageItem.Key, baggageItem.Value);
-                }
+                activity.AddBaggage(baggageItem.Key, baggageItem.Value);
             }
         }
 

src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs

@@ -504,10 +504,10 @@ public void ActivityParentIdAndBaggageReadFromHeaders()
         features.Set<IHttpRequestFeature>(new HttpRequestFeature()
         {
             Headers = new HeaderDictionary()
-                {
-                    {"Request-Id", "ParentId1"},
-                    {"baggage", "Key1=value1, Key2=value2"}
-                }
+            {
+                {"Request-Id", "ParentId1"},
+                {"baggage", "Key1=value1, Key2=value2"}
+            }
         });
         hostingApplication.CreateContext(features);
         Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", Activity.Current.OperationName);
@@ -517,7 +517,7 @@ public void ActivityParentIdAndBaggageReadFromHeaders()
     }
 
     [Fact]
-    public void ActivityBaggageReadFromLegacyHeaders()
+    public void BaggageReadFromHeadersWithoutRequestId()
     {
         var diagnosticListener = new DiagnosticListener("DummySource");
         var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);
@@ -535,10 +535,39 @@ public void ActivityBaggageReadFromLegacyHeaders()
         features.Set<IHttpRequestFeature>(new HttpRequestFeature()
         {
             Headers = new HeaderDictionary()
+            {
+                {"baggage", "Key1=value1, Key2=value2"}
+            }
+        });
+        hostingApplication.CreateContext(features);
+        Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", Activity.Current.OperationName);
+        Assert.Contains(Activity.Current.Baggage, pair => pair.Key == "Key1" && pair.Value == "value1");
+        Assert.Contains(Activity.Current.Baggage, pair => pair.Key == "Key2" && pair.Value == "value2");
+    }
+
+    [Fact]
+    public void ActivityBaggageReadFromLegacyHeaders()
+    {
+        var diagnosticListener = new DiagnosticListener("DummySource");
+        var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);
+
+        diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair => { }),
+            s =>
+            {
+                if (s.StartsWith("Microsoft.AspNetCore.Hosting.HttpRequestIn", StringComparison.Ordinal))
                 {
-                    {"Request-Id", "ParentId1"},
-                    {"Correlation-Context", "Key1=value1, Key2=value2"}
+                    return true;
                 }
+                return false;
+            });
+
+        features.Set<IHttpRequestFeature>(new HttpRequestFeature()
+        {
+            Headers = new HeaderDictionary()
+            {
+                {"Request-Id", "ParentId1"},
+                {"Correlation-Context", "Key1=value1, Key2=value2"}
+            }
         });
         hostingApplication.CreateContext(features);
         Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", Activity.Current.OperationName);
@@ -565,11 +594,11 @@ public void ActivityBaggagePrefersW3CBaggageHeaderName()
         features.Set<IHttpRequestFeature>(new HttpRequestFeature()
         {
             Headers = new HeaderDictionary()
-                {
-                    {"Request-Id", "ParentId1"},
-                    {"Correlation-Context", "Key1=value1, Key2=value2"},
-                    {"baggage", "Key1=value3, Key2=value4"}
-                }
+            {
+                {"Request-Id", "ParentId1"},
+                {"Correlation-Context", "Key1=value1, Key2=value2"},
+                {"baggage", "Key1=value3, Key2=value4"}
+            }
         });
         hostingApplication.CreateContext(features);
         Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", Activity.Current.OperationName);
@@ -596,20 +625,20 @@ public void ActivityBaggagePreservesItemsOrder()
         features.Set<IHttpRequestFeature>(new HttpRequestFeature()
         {
             Headers = new HeaderDictionary()
-                {
-                    {"Request-Id", "ParentId1"},
-                    {"baggage", "Key1=value1, Key2=value2, Key1=value3"} // duplicated keys allowed by the contract
-                }
+            {
+                {"Request-Id", "ParentId1"},
+                {"baggage", "Key1=value1, Key2=value2, Key1=value3"} // duplicated keys allowed by the contract
+            }
         });
         hostingApplication.CreateContext(features);
         Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", Activity.Current.OperationName);
 
         var expectedBaggage = new[]
         {
-                KeyValuePair.Create("Key1","value1"),
-                KeyValuePair.Create("Key2","value2"),
-                KeyValuePair.Create("Key1","value3")
-            };
+            KeyValuePair.Create("Key1","value1"),
+            KeyValuePair.Create("Key2","value2"),
+            KeyValuePair.Create("Key1","value3")
+        };
 
         Assert.Equal(expectedBaggage, Activity.Current.Baggage.ToArray());
     }
@@ -633,10 +662,10 @@ public void ActivityBaggageValuesAreUrlDecodedFromHeaders()
         features.Set<IHttpRequestFeature>(new HttpRequestFeature()
         {
             Headers = new HeaderDictionary()
-                {
-                    {"Request-Id", "ParentId1"},
-                    {"baggage", "Key1=value1%2F1"}
-                }
+            {
+                {"Request-Id", "ParentId1"},
+                {"baggage", "Key1=value1%2F1"}
+            }
         });
         hostingApplication.CreateContext(features);
         Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", Activity.Current.OperationName);
@@ -662,11 +691,11 @@ public void ActivityTraceParentAndTraceStateFromHeaders()
         features.Set<IHttpRequestFeature>(new HttpRequestFeature()
         {
             Headers = new HeaderDictionary()
-                {
-                    {"traceparent", "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01"},
-                    {"tracestate", "TraceState1"},
-                    {"baggage", "Key1=value1, Key2=value2"}
-                }
+            {
+                {"traceparent", "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01"},
+                {"tracestate", "TraceState1"},
+                {"baggage", "Key1=value1, Key2=value2"}
+            }
         });
         hostingApplication.CreateContext(features);
         Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", Activity.Current.OperationName);
@@ -704,9 +733,9 @@ public void SamplersReceiveCorrectParentAndTraceIds()
         features.Set<IHttpRequestFeature>(new HttpRequestFeature()
         {
             Headers = new HeaderDictionary()
-                {
-                    {"traceparent", "00-35aae61e3e99044eb5ea5007f2cd159b-40a8bd87c078cb4c-00"},
-                }
+            {
+                {"traceparent", "00-35aae61e3e99044eb5ea5007f2cd159b-40a8bd87c078cb4c-00"},
+            }
         });
 
         hostingApplication.CreateContext(features);
@@ -772,11 +801,11 @@ public void ActivityListenersAreCalled()
         features.Set<IHttpRequestFeature>(new HttpRequestFeature()
         {
             Headers = new HeaderDictionary()
-                {
-                    {"traceparent", "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01"},
-                    {"tracestate", "TraceState1"},
-                    {"baggage", "Key1=value1, Key2=value2"}
-                }
+            {
+                {"traceparent", "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01"},
+                {"tracestate", "TraceState1"},
+                {"baggage", "Key1=value1, Key2=value2"}
+            }
         });
 
         hostingApplication.CreateContext(features);