Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wire up DiagnosticsHandler #456

Merged
merged 22 commits into from
Nov 12, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public sealed class ProxyHttpClientOptions

public int? MaxConnectionsPerServer { get; set; }

public bool? PropagateActivityContext { get; set; }

// TODO: Add this property once we have migrated to SDK version that supports it.
//public bool? EnableMultipleHttp2Connections { get; set; }

Expand All @@ -28,7 +30,8 @@ internal ProxyHttpClientOptions DeepClone()
DangerousAcceptAnyServerCertificate = DangerousAcceptAnyServerCertificate,
// TODO: Clone certificate?
ClientCertificate = ClientCertificate,
MaxConnectionsPerServer = MaxConnectionsPerServer
MaxConnectionsPerServer = MaxConnectionsPerServer,
PropagateActivityContext = PropagateActivityContext,
};
}

Expand All @@ -47,7 +50,8 @@ internal static bool Equals(ProxyHttpClientOptions options1, ProxyHttpClientOpti
return options1.SslProtocols == options2.SslProtocols
&& Equals(options1.ClientCertificate, options2.ClientCertificate)
&& options1.DangerousAcceptAnyServerCertificate == options2.DangerousAcceptAnyServerCertificate
&& options1.MaxConnectionsPerServer == options2.MaxConnectionsPerServer;
&& options1.MaxConnectionsPerServer == options2.MaxConnectionsPerServer
&& options1.PropagateActivityContext == options2.PropagateActivityContext;
}

private static bool Equals(X509Certificate2 certificate1, X509Certificate2 certificate2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,8 @@ private ProxyHttpClientOptions Convert(ProxyHttpClientData data)
SslProtocols = sslProtocols,
DangerousAcceptAnyServerCertificate = data.DangerousAcceptAnyServerCertificate,
ClientCertificate = clientCertificate,
MaxConnectionsPerServer = data.MaxConnectionsPerServer
MaxConnectionsPerServer = data.MaxConnectionsPerServer,
PropagateActivityContext = data.PropagateActivityContext
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public sealed class ProxyHttpClientData

public int? MaxConnectionsPerServer { get; set; }

public bool? PropagateActivityContext { get; set; }

// TODO: Add this property once we have migrated to SDK version that supports it.
//public bool? EnableMultipleHttp2Connections { get; set; }
}
Expand Down
3 changes: 2 additions & 1 deletion src/ReverseProxy/Service/Management/ProxyConfigManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,8 @@ private ClusterProxyHttpClientOptions ConvertProxyHttpClientOptions(ProxyHttpCli
httpClientOptions.SslProtocols,
httpClientOptions.DangerousAcceptAnyServerCertificate,
httpClientOptions.ClientCertificate,
httpClientOptions.MaxConnectionsPerServer);
httpClientOptions.MaxConnectionsPerServer,
httpClientOptions.PropagateActivityContext);
}

public void Dispose()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Net.Http;
using System.Security.Cryptography.X509Certificates;
using Microsoft.Extensions.Logging;
using Microsoft.ReverseProxy.Telemetry;

namespace Microsoft.ReverseProxy.Service.Proxy.Infrastructure
{
Expand Down Expand Up @@ -65,6 +66,12 @@ public HttpMessageInvoker CreateClient(ProxyHttpClientContext context)
}

Log.ProxyClientCreated(_logger, context.ClusterId);

if (newClientOptions.PropagateActivityContext.GetValueOrDefault(true))
{
return new HttpMessageInvoker(new ActivityPropagationHandler(handler), disposeHandler: true);
}

return new HttpMessageInvoker(handler, disposeHandler: true);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ public ClusterProxyHttpClientOptions(
SslProtocols? sslProtocols,
bool acceptAnyServerCertificate,
X509Certificate2 clientCertificate,
int? maxConnectionsPerServer)
int? maxConnectionsPerServer,
bool? propagateActivityContext)
{
SslProtocols = sslProtocols;
DangerousAcceptAnyServerCertificate = acceptAnyServerCertificate;
ClientCertificate = clientCertificate;
MaxConnectionsPerServer = maxConnectionsPerServer;
PropagateActivityContext = propagateActivityContext;
}

public SslProtocols? SslProtocols { get; }
Expand All @@ -30,6 +32,8 @@ public ClusterProxyHttpClientOptions(

public int? MaxConnectionsPerServer { get; }

public bool? PropagateActivityContext { get; }

// TODO: Add this property once we have migrated to SDK version that supports it.
//public bool? EnableMultipleHttp2Connections { get; }

Expand All @@ -43,7 +47,8 @@ public bool Equals(ClusterProxyHttpClientOptions other)
return SslProtocols == other.SslProtocols &&
DangerousAcceptAnyServerCertificate == other.DangerousAcceptAnyServerCertificate &&
EqualityComparer<X509Certificate2>.Default.Equals(ClientCertificate, other.ClientCertificate) &&
MaxConnectionsPerServer == other.MaxConnectionsPerServer;
MaxConnectionsPerServer == other.MaxConnectionsPerServer &&
PropagateActivityContext == other.PropagateActivityContext;
}

public override int GetHashCode()
Expand Down
96 changes: 96 additions & 0 deletions src/ReverseProxy/Telemetry/ActivityPropagationHandler.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.ReverseProxy.Telemetry
{
/// <summary>
/// ActivityPropagationHandler propagates the current Activity to the downstream service
/// </summary>
internal sealed class ActivityPropagationHandler : DelegatingHandler
jmezach marked this conversation as resolved.
Show resolved Hide resolved
{
private const string RequestIdHeaderName = "Request-Id";
private const string CorrelationContextHeaderName = "Correlation-Context";

private const string TraceParentHeaderName = "traceparent";
private const string TraceStateHeaderName = "tracestate";

/// <summary>
/// ActivityPropagationHandler constructor
/// </summary>
/// <param name="innerHandler">Inner handler: Windows or Unix implementation of HttpMessageHandler.
/// Note that ActivityPropagationHandler is the latest in the pipeline </param>
jmezach marked this conversation as resolved.
Show resolved Hide resolved
public ActivityPropagationHandler(HttpMessageHandler innerHandler) : base(innerHandler)
{
}

protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
CancellationToken cancellationToken)
{
// This handler is conditionally inserted by the ProxyHttpClientFactory based on the configuration
// If inserted it will insert the necessary headers to propagate the current activity context to
// the downstream service, if there is a current activity

if (request == null)
{
throw new ArgumentNullException(nameof(request));
}

// If we are on at all, we propagate current activity information
var currentActivity = Activity.Current;
if (currentActivity != null)
{
InjectHeaders(currentActivity, request);
}

return base.SendAsync(request, cancellationToken);
}

#region private
jmezach marked this conversation as resolved.
Show resolved Hide resolved

private void InjectHeaders(Activity currentActivity, HttpRequestMessage request)
{
if (currentActivity.IdFormat == ActivityIdFormat.W3C)
{
request.Headers.Remove(TraceParentHeaderName);
jmezach marked this conversation as resolved.
Show resolved Hide resolved

request.Headers.TryAddWithoutValidation(TraceParentHeaderName, currentActivity.Id);
if (currentActivity.TraceStateString != null)
{
request.Headers.TryAddWithoutValidation(TraceStateHeaderName, currentActivity.TraceStateString);
}
}
else
{
request.Headers.Remove(RequestIdHeaderName);
request.Headers.TryAddWithoutValidation(RequestIdHeaderName, currentActivity.Id);
}

// we expect baggage to be empty or contain a few items
using (var e = currentActivity.Baggage.GetEnumerator())
{
if (e.MoveNext())
{
var baggage = new List<string>();
do
{
var item = e.Current;
baggage.Add(new NameValueHeaderValue(Uri.EscapeDataString(item.Key), Uri.EscapeDataString(item.Value)).ToString());
}
while (e.MoveNext());
request.Headers.TryAddWithoutValidation(CorrelationContextHeaderName, baggage);
}
}
}

#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ public class ConfigurationConfigProviderTests
SslProtocols = new List<SslProtocols> { SslProtocols.Tls11, SslProtocols.Tls12 },
MaxConnectionsPerServer = 10,
DangerousAcceptAnyServerCertificate = true,
ClientCertificate = new CertificateConfigData { Path = "mycert.pfx", Password = "myPassword1234" }
ClientCertificate = new CertificateConfigData { Path = "mycert.pfx", Password = "myPassword1234" },
PropagateActivityContext = true,
},
Metadata = new Dictionary<string, string> { { "cluster1-K1", "cluster1-V1" }, { "cluster1-K2", "cluster1-V2" } }
}
Expand Down Expand Up @@ -197,7 +198,8 @@ public class ConfigurationConfigProviderTests
""Location"": null,
""AllowInvalid"": null
},
""MaxConnectionsPerServer"": 10
""MaxConnectionsPerServer"": 10,
""PropagateActivityContext"": true,
},
""Destinations"": {
""destinationA"": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.ReverseProxy.Common.Tests;
using Microsoft.ReverseProxy.RuntimeModel;
using Microsoft.ReverseProxy.Service.Proxy.Infrastructure;
using Microsoft.ReverseProxy.Telemetry;
using Microsoft.ReverseProxy.Utilities.Tests;
using Xunit;

Expand Down Expand Up @@ -44,7 +45,7 @@ public void CreateClient_Works()
public void CreateClient_ApplySslProtocols_Success()
{
var factory = new ProxyHttpClientFactory(Mock<ILogger<ProxyHttpClientFactory>>().Object);
var options = new ClusterProxyHttpClientOptions(SslProtocols.Tls12 | SslProtocols.Tls13, default, default, default);
var options = new ClusterProxyHttpClientOptions(SslProtocols.Tls12 | SslProtocols.Tls13, default, default, default, default);
var client = factory.CreateClient(new ProxyHttpClientContext { NewOptions = options });

var handler = GetHandler(client);
Expand All @@ -58,7 +59,7 @@ public void CreateClient_ApplySslProtocols_Success()
public void CreateClient_ApplyDangerousAcceptAnyServerCertificate_Success()
{
var factory = new ProxyHttpClientFactory(Mock<ILogger<ProxyHttpClientFactory>>().Object);
var options = new ClusterProxyHttpClientOptions(default, true, default, default);
var options = new ClusterProxyHttpClientOptions(default, true, default, default, default);
var client = factory.CreateClient(new ProxyHttpClientContext { NewOptions = options });

var handler = GetHandler(client);
Expand All @@ -74,7 +75,7 @@ public void CreateClient_ApplyClientCertificate_Success()
{
var factory = new ProxyHttpClientFactory(Mock<ILogger<ProxyHttpClientFactory>>().Object);
var certificate = TestResources.GetTestCertificate();
var options = new ClusterProxyHttpClientOptions(default, default, certificate, default);
var options = new ClusterProxyHttpClientOptions(default, default, certificate, default, default);
var client = factory.CreateClient(new ProxyHttpClientContext { NewOptions = options });

var handler = GetHandler(client);
Expand All @@ -89,7 +90,7 @@ public void CreateClient_ApplyClientCertificate_Success()
public void CreateClient_ApplyMaxConnectionsPerServer_Success()
{
var factory = new ProxyHttpClientFactory(Mock<ILogger<ProxyHttpClientFactory>>().Object);
var options = new ClusterProxyHttpClientOptions(default, default, default, 22);
var options = new ClusterProxyHttpClientOptions(default, default, default, 22, default);
var client = factory.CreateClient(new ProxyHttpClientContext { NewOptions = options });

var handler = GetHandler(client);
Expand All @@ -99,14 +100,28 @@ public void CreateClient_ApplyMaxConnectionsPerServer_Success()
VerifyDefaultValues(handler, "MaxConnectionsPerServer");
}

[Fact]
public void CreateClient_ApplyPropagateActivityContext_Success()
{
var factory = new ProxyHttpClientFactory(Mock<ILogger<ProxyHttpClientFactory>>().Object);
var options = new ClusterProxyHttpClientOptions(default, default, default, default, true);
var client = factory.CreateClient(new ProxyHttpClientContext { NewOptions = options });

var handler = GetHandler(client);
jmezach marked this conversation as resolved.
Show resolved Hide resolved

Assert.NotNull(handler);
// TODO Assert.Equal(true, handler);
VerifyDefaultValues(handler, "MaxConnectionsPerServer");
}

[Fact]
public void CreateClient_OldClientExistsNoConfigChange_ReturnsOldInstance()
{
var factory = new ProxyHttpClientFactory(Mock<ILogger<ProxyHttpClientFactory>>().Object);
var oldClient = new HttpMessageInvoker(new SocketsHttpHandler());
var clientCertificate = TestResources.GetTestCertificate();
var oldOptions = new ClusterProxyHttpClientOptions(SslProtocols.Tls11 | SslProtocols.Tls12, true, clientCertificate, 10);
var newOptions = new ClusterProxyHttpClientOptions(SslProtocols.Tls11 | SslProtocols.Tls12, true, clientCertificate, 10);
var oldOptions = new ClusterProxyHttpClientOptions(SslProtocols.Tls11 | SslProtocols.Tls12, true, clientCertificate, 10, true);
var newOptions = new ClusterProxyHttpClientOptions(SslProtocols.Tls11 | SslProtocols.Tls12, true, clientCertificate, 10, true);
var oldMetadata = new Dictionary<string, string> { { "key1", "value1" }, { "key2", "value2" } };
var newMetadata = new Dictionary<string, string> { { "key1", "value1" }, { "key2", "value2" } };
var context = new ProxyHttpClientContext { ClusterId = "cluster1", OldOptions = oldOptions, OldMetadata = oldMetadata, OldClient = oldClient, NewOptions = newOptions, NewMetadata = newMetadata };
Expand Down Expand Up @@ -137,41 +152,51 @@ public static IEnumerable<object[]> GetChangedHttpClientOptions()
return new[]
{
new object[] {
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11 | SslProtocols.Tls12, true, clientCertificate, null)
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null, false),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11 | SslProtocols.Tls12, true, clientCertificate, null, false)
},
new object[] {
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, false, clientCertificate, null)
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null, false),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, false, clientCertificate, null, false)
},
new object[] {
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, null)
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null, false),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, null, false)
},
new object[] {
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, null),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null)
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, null, false),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null, false)
},
new object[] {
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, null),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, 10)
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, null, false),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, 10, false)
},
new object[] {
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, 10),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null)
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, 10, false),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, null, false)
},
new object[] {
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, 10),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, 20)
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, 10, false),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, clientCertificate, 20, false)
},
new object[] {
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, 10, true),
new ClusterProxyHttpClientOptions(SslProtocols.Tls11, true, null, 10, false)
}
};
}

public static SocketsHttpHandler GetHandler(HttpMessageInvoker client)
{
var handlerFieldInfo = typeof(HttpMessageInvoker).GetFields(BindingFlags.Instance | BindingFlags.NonPublic).Single(f => f.Name == "_handler");
var result = (SocketsHttpHandler)handlerFieldInfo.GetValue(client);
return result;
var handler = handlerFieldInfo.GetValue(client);

if (handler is ActivityPropagationHandler diagnosticsHandler)
{
return (SocketsHttpHandler)diagnosticsHandler.InnerHandler;
}

return (SocketsHttpHandler)handler;
}

private void VerifyDefaultValues(SocketsHttpHandler actualHandler, params string[] skippedExtractors)
Expand Down