Remove ActivityPropagationHandler and add header filtering

docs/docfx/articles/config-files.md

@@ -191,7 +191,6 @@ For additional fields see [ClusterConfig](xref:Yarp.ReverseProxy.Configuration.C
           "SSLProtocols" : "Tls13",
           "DangerousAcceptAnyServerCertificate" : false,
           "MaxConnectionsPerServer" : 1024,
-          "ActivityContextHeaders" : "None", // Or "Baggage", "CorrelationContext", "BaggageAndCorrelationContext"
           "EnableMultipleHttp2Connections" : true,
           "RequestHeaderEncoding" : "Latin1" // How to interpret non ASCII characters in header values
         },

samples/ReverseProxy.Config.Sample/appsettings.json

@@ -122,7 +122,6 @@
           },
 */
           "MaxConnectionsPerServer": 1024, // Destination server can further limit this number
-          "ActivityContextHeaders": "None", // Or "Baggage", "CorrelationContext", "BaggageAndCorrelationContext"
           "EnableMultipleHttp2Connections": true,
           "RequestHeaderEncoding": "Latin1" // How to interpret non ASCII characters in header values
         },

src/ReverseProxy/Configuration/ConfigProvider/ConfigurationConfigProvider.cs

@@ -348,7 +348,6 @@ private static RouteQueryParameter CreateRouteQueryParameter(IConfigurationSecti
                 EnableMultipleHttp2Connections = section.ReadBool(nameof(HttpClientConfig.EnableMultipleHttp2Connections)),
                 RequestHeaderEncoding = section[nameof(HttpClientConfig.RequestHeaderEncoding)],
 #endif
-                ActivityContextHeaders = section.ReadEnum<ActivityContextHeaders>(nameof(HttpClientConfig.ActivityContextHeaders)),
                 WebProxy = webProxy
             };
         }

src/ReverseProxy/Configuration/HttpClientConfig.cs

@@ -32,11 +32,6 @@ public sealed record HttpClientConfig
         /// </summary>
         public int? MaxConnectionsPerServer { get; init; }
 
-        /// <summary>
-        /// Specifies the activity correlation headers for outgoing requests.
-        /// </summary>
-        public ActivityContextHeaders? ActivityContextHeaders { get; init; }
-
         /// <summary>
         /// Optional web proxy used when communicating with the destination server. 
         /// </summary>
@@ -71,7 +66,6 @@ public bool Equals(HttpClientConfig? other)
                    // Comparing by reference is fine here since Encoding.GetEncoding returns the same instance for each encoding.
                    && RequestHeaderEncoding == other.RequestHeaderEncoding
 #endif
-                   && ActivityContextHeaders == other.ActivityContextHeaders
                    && WebProxy == other.WebProxy;
         }
 
@@ -84,7 +78,6 @@ public override int GetHashCode()
                 EnableMultipleHttp2Connections,
                 RequestHeaderEncoding,
 #endif
-                ActivityContextHeaders,
                 WebProxy);
         }
     }

src/ReverseProxy/Forwarder/ForwarderHttpClientFactory.cs

@@ -124,16 +124,9 @@ protected virtual void ConfigureHandler(ForwarderHttpClientContext context, Sock
 
         /// <summary>
         /// Adds any wrapping middleware around the <see cref="HttpMessageHandler"/>.
-        /// The base implementation conditionally includes the <see cref="ActivityPropagationHandler"/>.
         /// </summary>
         protected virtual HttpMessageHandler WrapHandler(ForwarderHttpClientContext context, HttpMessageHandler handler)
         {
-            var activityContextHeaders = context.NewConfig.ActivityContextHeaders.GetValueOrDefault(ActivityContextHeaders.CorrelationContext);
-            if (activityContextHeaders != ActivityContextHeaders.None)
-            {
-                handler = new ActivityPropagationHandler(activityContextHeaders, handler);
-            }
-
             return handler;
         }
 

src/ReverseProxy/Forwarder/RequestUtilities.cs

@@ -59,7 +59,7 @@ internal static bool ShouldSkipResponseHeader(string headerName)
             return _headersToExclude.Contains(headerName);
         }
 
-        private static readonly HashSet<string> _headersToExclude = new(17, StringComparer.OrdinalIgnoreCase)
+        private static readonly HashSet<string> _headersToExclude = new(22, StringComparer.OrdinalIgnoreCase)
         {
             HeaderNames.Connection,
             HeaderNames.TransferEncoding,
@@ -75,14 +75,22 @@ internal static bool ShouldSkipResponseHeader(string headerName)
             "ALPN",
             "Close",
             "HTTP2-Settings",
-            "Upgrade-Insecure-Requests",
+            HeaderNames.UpgradeInsecureRequests,
             HeaderNames.TE,
 #if NET
             HeaderNames.AltSvc,
 #else
             "Alt-Svc",
 #endif
 
+#if NET6_0_OR_GREATER
+            // Distributed context headers
+            HeaderNames.TraceParent,
+            HeaderNames.RequestId,
+            HeaderNames.TraceState,
+            HeaderNames.Baggage,
+            HeaderNames.CorrelationContext,
+#endif
         };
 
         // Headers marked as HttpHeaderType.Content in

src/ServiceFabric/ServiceDiscovery/Util/LabelsParser.cs

@@ -268,7 +268,6 @@ internal static ClusterConfig BuildCluster(Uri serviceName, Dictionary<string, s
 
             var versionLabel = GetLabel<string>(labels, "YARP.Backend.HttpRequest.Version", null);
 
-            var activityContextHeadersLabel = GetLabel<string>(labels, "YARP.Backend.HttpClient.ActivityContextHeaders", null);
             var sslProtocolsLabel = GetLabel<string>(labels, "YARP.Backend.HttpClient.SslProtocols", null);
 
 #if NET
@@ -328,7 +327,6 @@ internal static ClusterConfig BuildCluster(Uri serviceName, Dictionary<string, s
                 {
                     DangerousAcceptAnyServerCertificate = GetLabel<bool?>(labels, "YARP.Backend.HttpClient.DangerousAcceptAnyServerCertificate", null),
                     MaxConnectionsPerServer = GetLabel<int?>(labels, "YARP.Backend.HttpClient.MaxConnectionsPerServer", null),
-                    ActivityContextHeaders = !string.IsNullOrEmpty(activityContextHeadersLabel) ? Enum.Parse<ActivityContextHeaders>(activityContextHeadersLabel) : null,
                     SslProtocols = !string.IsNullOrEmpty(sslProtocolsLabel) ? Enum.Parse<SslProtocols>(sslProtocolsLabel) : null,
 #if NET
                     EnableMultipleHttp2Connections = GetLabel<bool?>(labels, "YARP.Backend.HttpClient.EnableMultipleHttp2Connections", null),

test/ReverseProxy.FunctionalTests/Common/TestEnvironment.cs

@@ -15,6 +15,8 @@
 using Microsoft.Extensions.Hosting;
 using Yarp.Tests.Common;
 using Yarp.ReverseProxy.Configuration;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Configuration;
 
 namespace Yarp.ReverseProxy.Common
 {
@@ -142,6 +144,18 @@ private static IHost CreateHost(HttpProtocols protocols, bool useHttps, Encoding
             Action<IServiceCollection> configureServices, Action<IApplicationBuilder> configureApp)
         {
             return new HostBuilder()
+                .ConfigureAppConfiguration(config =>
+                {
+                    config.AddInMemoryCollection(new Dictionary<string, string>()
+                    {
+                        { "Logging:LogLevel:Microsoft.AspNetCore.Hosting.Diagnostics", "Information" }
+                    });
+                })
+                .ConfigureLogging((hostingContext, loggingBuilder) =>
+                {
+                    loggingBuilder.AddConfiguration(hostingContext.Configuration.GetSection("Logging"));
+                    loggingBuilder.AddEventSourceLogger();
+                })
                 .ConfigureWebHost(webHostBuilder =>
                 {
                     webHostBuilder

test/ReverseProxy.FunctionalTests/DistributedTracingTests.cs

@@ -0,0 +1,127 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System.Diagnostics;
+using System.Linq;
+using System.Net.Http;
+using System.Text;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Http;
+using Xunit;
+using Yarp.ReverseProxy.Common;
+
+namespace Yarp.ReverseProxy
+{
+    public class DistributedTracingTests
+    {
+        // These constants depend on the default behavior of DiagnosticsHandler in 5.0
+        // and the DistributedContextPropagator used in 6.0
+        private const string Baggage = "Correlation-Context";
+        private const string TraceParent = "traceparent";
+        private const string TraceState = "tracestate";
+        private const string RequestId = "Request-Id";
+
+        [Theory]
+        [InlineData(ActivityIdFormat.W3C)]
+        [InlineData(ActivityIdFormat.Hierarchical)]
+        public async Task DistributedTracing_Works(ActivityIdFormat idFormat)
+        {
+            var proxyHeaders = new HeaderDictionary();
+            var downstreamHeaders = new HeaderDictionary();
+
+            var test = new TestEnvironment(
+                async context =>
+                {
+                    foreach (var header in context.Request.Headers)
+                    {
+                        downstreamHeaders.Add(header);
+                    }
+                    await context.Response.Body.WriteAsync(Encoding.UTF8.GetBytes("Hello"));
+                },
+                proxyBuilder => { },
+                proxyApp =>
+                {
+                    proxyApp.Use(next => context =>
+                    {
+                        foreach (var header in context.Request.Headers)
+                        {
+                            proxyHeaders.Add(header);
+                        }
+                        return next(context);
+                    });
+                });
+
+            var clientActivity = new Activity("Foo");
+            clientActivity.SetIdFormat(idFormat);
+            clientActivity.TraceStateString = "Bar";
+            clientActivity.AddBaggage("One", "1");
+            clientActivity.AddBaggage("Two", "2");
+            clientActivity.Start();
+
+            await test.Invoke(async uri =>
+            {
+                using var client = new HttpClient();
+                Assert.Equal("Hello", await client.GetStringAsync(uri));
+            });
+
+            Assert.NotEmpty(proxyHeaders);
+            Assert.NotEmpty(downstreamHeaders);
+
+            ValidateActivities(idFormat, clientActivity, proxyHeaders, downstreamHeaders);
+        }
+
+        private static void ValidateActivities(ActivityIdFormat idFormat, Activity client, HeaderDictionary proxy, HeaderDictionary downstream)
+        {
+            var baggage = string.Join(", ", client.Baggage.Select(pair => $"{pair.Key}={pair.Value}"));
+            Assert.Equal(baggage, proxy[Baggage]);
+            Assert.Equal(baggage, downstream[Baggage]);
+
+            if (idFormat == ActivityIdFormat.W3C)
+            {
+#if NET
+                Assert.True(ActivityContext.TryParse(proxy[TraceParent], proxy[TraceState], out var proxyContext));
+                Assert.True(ActivityContext.TryParse(downstream[TraceParent], downstream[TraceState], out var downstreamContext));
+                Assert.Equal(client.TraceStateString, proxyContext.TraceState);
+                Assert.Equal(client.TraceStateString, downstreamContext.TraceState);
+                var proxyTraceId = proxyContext.TraceId.ToHexString();
+                var proxySpanId = proxyContext.SpanId.ToHexString();
+                var downstreamTraceId = downstreamContext.TraceId.ToHexString();
+                var downstreamSpanId = downstreamContext.SpanId.ToHexString();
+#else
+                // 3.1 does not have ActivityContext
+                Assert.Equal(client.TraceStateString, proxy[TraceState]);
+                Assert.Equal(client.TraceStateString, downstream[TraceState]);
+                var proxyTraceId = proxy[TraceParent].ToString().Split('-')[1];
+                var proxySpanId = proxy[TraceParent].ToString().Split('-')[2];
+                var downstreamTraceId = downstream[TraceParent].ToString().Split('-')[1];
+                var downstreamSpanId = downstream[TraceParent].ToString().Split('-')[2];
+#endif
+
+                Assert.Equal(client.TraceId.ToHexString(), proxyTraceId);
+                Assert.Equal(client.TraceId.ToHexString(), downstreamTraceId);
+
+#if NET6_0_OR_GREATER
+                Assert.NotEqual(proxySpanId, downstreamSpanId);
+#else
+                // Before 6.0, YARP is just pass-through as far as distributed tracing is concerned
+                Assert.Equal(proxySpanId, downstreamSpanId);
+#endif
+            }
+            else
+            {
+                var proxyId = proxy[RequestId].ToString();
+                var downstreamId = downstream[RequestId].ToString();
+
+                Assert.StartsWith(client.Id, proxyId);
+                Assert.StartsWith(proxyId, downstreamId);
+
+#if NET6_0_OR_GREATER
+                Assert.NotEqual(proxyId, downstreamId);
+#else
+                // Before 6.0, YARP is just pass-through as far as distributed tracing is concerned
+                Assert.Equal(proxyId, downstreamId);
+#endif
+            }
+        }
+    }
+}