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

[TEST] http metrics sandbox #88530

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
ec16299
initial implementation of request metrics
antonfirsov Jun 9, 2023
2bacea0
simplify error handling code and add comments
antonfirsov Jun 9, 2023
74597a4
typo
antonfirsov Jun 9, 2023
12af084
fix Http.Unit.Tests build failure
antonfirsov Jun 9, 2023
528f3b1
wip
antonfirsov Jun 22, 2023
0d97a7d
Merge branch 'main' into MetricsHandler-02
antonfirsov Jun 22, 2023
4fcfee6
callback-based enrichment API with reftype context
antonfirsov Jun 22, 2023
bd461c8
Protect against re-reentrancy
antonfirsov Jun 22, 2023
3c80a0b
use shared counters with the shared meter
antonfirsov Jun 22, 2023
7406c46
do not cache instruments already cached by Meter
antonfirsov Jun 23, 2023
9da9485
use [ThreadStatic] instead of ThreadLocal
antonfirsov Jun 23, 2023
9251264
Merge branch 'main' into MetricsHandler-05
antonfirsov Jun 27, 2023
2a90c07
sync with main
antonfirsov Jun 27, 2023
869bd34
review feedback
antonfirsov Jun 27, 2023
1d43c3e
use FrozenDictionary for caching status codes
antonfirsov Jun 27, 2023
f6ea058
Merge branch 'main' into MetricsHandler-05
antonfirsov Jul 5, 2023
d5e0ca2
Update API signature
antonfirsov Jul 5, 2023
df36921
update implementation and tests to use IMeterFactory
antonfirsov Jul 5, 2023
8ba8772
wip
antonfirsov Jul 7, 2023
ed22f0e
Merge branch 'main' into MetricsHandler-05
antonfirsov Jul 7, 2023
bed511d
fix merge issues
antonfirsov Jul 7, 2023
c932f9c
simplify tags storage
antonfirsov Jul 7, 2023
1af67ff
do not record errors after "headers finished"
antonfirsov Jul 7, 2023
29148d7
oops
antonfirsov Jul 7, 2023
66bd0dd
Simplify HttpClientHandler code
antonfirsov Jul 8, 2023
d8f1638
consolidate request null checks into HttpClientHandler
antonfirsov Jul 8, 2023
b0081cc
HttpMetricsTest -> MetricsTest
antonfirsov Jul 8, 2023
3f988a1
refactor enrichment
antonfirsov Jul 8, 2023
07a954e
basic HttpMetricsEnrichmentContext caching with ConcurrentQueue
antonfirsov Jul 8, 2023
373466c
limit cache capacity
antonfirsov Jul 9, 2023
92ec574
re-enable HTTP/3.0 metrics tests
antonfirsov Jul 9, 2023
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
15 changes: 15 additions & 0 deletions src/libraries/System.Net.Http/ref/System.Net.Http.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ public HttpClientHandler() { }
public long MaxRequestContentBufferSize { get { throw null; } set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
public int MaxResponseHeadersLength { get { throw null; } set { } }
[System.CLSCompliantAttribute(false)]
public System.Diagnostics.Metrics.IMeterFactory? MeterFactory { get { throw null; } set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
public bool PreAuthenticate { get { throw null; } set { } }
public System.Collections.Generic.IDictionary<string, object?> Properties { get { throw null; } }
Expand Down Expand Up @@ -397,6 +399,8 @@ public SocketsHttpHandler() { }
public int MaxConnectionsPerServer { get { throw null; } set { } }
public int MaxResponseDrainSize { get { throw null; } set { } }
public int MaxResponseHeadersLength { get { throw null; } set { } }
[System.CLSCompliantAttribute(false)]
public System.Diagnostics.Metrics.IMeterFactory? MeterFactory { get { throw null; } set { } }
public System.TimeSpan PooledConnectionIdleTimeout { get { throw null; } set { } }
public System.TimeSpan PooledConnectionLifetime { get { throw null; } set { } }
public bool PreAuthenticate { get { throw null; } set { } }
Expand Down Expand Up @@ -901,3 +905,14 @@ public WarningHeaderValue(int code, string agent, string text, System.DateTimeOf
public static bool TryParse([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] string? input, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Net.Http.Headers.WarningHeaderValue? parsedValue) { throw null; }
}
}
namespace System.Net.Http.Metrics
{
public sealed class HttpMetricsEnrichmentContext
{
public System.Net.Http.HttpRequestMessage Request { get { throw null; } }
public System.Net.Http.HttpResponseMessage? Response { get { throw null; } }
public System.Exception? Exception { get { throw null; } }
public void AddCustomTag(string name, object? value) { throw null; }
public static void AddCallback(System.Net.Http.HttpRequestMessage request, System.Action<System.Net.Http.Metrics.HttpMetricsEnrichmentContext> callback) { throw null; }
}
}
7 changes: 5 additions & 2 deletions src/libraries/System.Net.Http/src/System.Net.Http.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@
<Compile Include="System\Net\Http\Headers\UriHeaderParser.cs" />
<Compile Include="System\Net\Http\Headers\ViaHeaderValue.cs" />
<Compile Include="System\Net\Http\Headers\WarningHeaderValue.cs" />
<Compile Include="System\Net\Http\Metrics\HttpMetricsEnrichmentContext.cs" />
<Compile Include="System\Net\Http\Metrics\MetricsHandler.cs" />
<Compile Include="System\Net\Http\SocketsHttpHandler\SocketsHttpPlaintextStreamFilterContext.cs" />
<Compile Include="$(CommonPath)DisableRuntimeMarshalling.cs"
Link="Common\DisableRuntimeMarshalling.cs" />
Expand Down Expand Up @@ -227,7 +229,7 @@
Link="Common\System\Net\DebugSafeHandleZeroOrMinusOneIsInvalid.cs" />
<Compile Include="$(CommonPath)System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs"
Link="Common\System\Threading\Tasks\TaskCompletionSourceWithCancellation.cs" />
<!-- Header support -->
<!-- Header support -->
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\IHttpStreamHeadersHandler.cs">
<Link>Common\System\Net\Http\aspnetcore\IHttpStreamHeadersHandler.cs</Link>
</Compile>
Expand Down Expand Up @@ -434,6 +436,7 @@
<Reference Include="Microsoft.Win32.Primitives" />
<Reference Include="System.Collections" />
<Reference Include="System.Collections.Concurrent" />
<Reference Include="System.Collections.Immutable" />
<Reference Include="System.Collections.NonGeneric" />
<Reference Include="System.Console" Condition="'$(Configuration)' == 'Debug'" />
<Reference Include="System.Diagnostics.DiagnosticSource" />
Expand Down Expand Up @@ -471,4 +474,4 @@
<ItemGroup>
<None Include="Resources\SR.resx" />
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,6 @@ private static HttpResponseMessage ConvertResponse(HttpRequestMessage request, W

protected internal override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(request);
bool? allowAutoRedirect = _isAllowAutoRedirectTouched ? AllowAutoRedirect : null;
#if FEATURE_WASM_THREADS
return JSHost.CurrentOrMainJSSynchronizationContext.Send(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading.Tasks;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics;
using System.Diagnostics.Metrics;

namespace System.Net.Http
{
Expand Down Expand Up @@ -91,6 +92,13 @@ public int MaxResponseDrainSize
set => throw new PlatformNotSupportedException();
}

[CLSCompliant(false)]
public IMeterFactory? MeterFactory
{
get => throw new PlatformNotSupportedException();
set => throw new PlatformNotSupportedException();
}

public TimeSpan ResponseDrainTimeout
{
get => throw new PlatformNotSupportedException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ internal override ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage re
{
if (IsEnabled())
{
ArgumentNullException.ThrowIfNull(request);
return SendAsyncCore(request, async, cancellationToken);
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.Metrics;
using System.Globalization;
using System.Net.Http.Metrics;
using System.Net.Security;
using System.Reflection;
using System.Runtime.ExceptionServices;
Expand All @@ -21,37 +23,28 @@ namespace System.Net.Http
public partial class HttpClientHandler : HttpMessageHandler
{
private readonly SocketsHttpHandler? _socketHandler;
private readonly DiagnosticsHandler? _diagnosticsHandler;

private readonly HttpMessageHandler? _nativeHandler;
private MetricsHandler? _metricsHandler;

private static readonly ConcurrentDictionary<string, MethodInfo?> s_cachedMethods =
new ConcurrentDictionary<string, MethodInfo?>();

private IMeterFactory? _meterFactory;
private ClientCertificateOption _clientCertificateOptions;

private volatile bool _disposed;

public HttpClientHandler()
{
HttpMessageHandler handler;

if (IsNativeHandlerEnabled)
{
_nativeHandler = CreateNativeHandler();
handler = _nativeHandler;
}
else
{
_socketHandler = new SocketsHttpHandler();
handler = _socketHandler;
ClientCertificateOptions = ClientCertificateOption.Manual;
}

if (DiagnosticsHandler.IsGloballyEnabled())
{
_diagnosticsHandler = new DiagnosticsHandler(handler, DistributedContextPropagator.Current);
}
}

protected override void Dispose(bool disposing)
Expand All @@ -73,6 +66,17 @@ protected override void Dispose(bool disposing)
base.Dispose(disposing);
}

[CLSCompliant(false)]
public IMeterFactory? MeterFactory
{
get => _meterFactory;
set
{
CheckDisposedOrStarted();
_meterFactory = value;
}
}

[UnsupportedOSPlatform("browser")]
public bool UseCookies
{
Expand Down Expand Up @@ -713,19 +717,9 @@ protected internal override HttpResponseMessage Send(HttpRequestMessage request,
protected internal override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
CancellationToken cancellationToken)
{
if (DiagnosticsHandler.IsGloballyEnabled() && _diagnosticsHandler != null)
{
return _diagnosticsHandler!.SendAsync(request, cancellationToken);
}

if (IsNativeHandlerEnabled)
{
return _nativeHandler!.SendAsync(request, cancellationToken);
}
else
{
return _socketHandler!.SendAsync(request, cancellationToken);
}
ArgumentNullException.ThrowIfNull(request);
MetricsHandler handler = _metricsHandler ?? SetupHandlerChain();
return handler.SendAsync(request, cancellationToken);
}

// lazy-load the validator func so it can be trimmed by the ILLinker if it isn't used.
Expand All @@ -741,6 +735,32 @@ protected internal override Task<HttpResponseMessage> SendAsync(HttpRequestMessa
}
}

private MetricsHandler SetupHandlerChain()
{
HttpMessageHandler handler = IsNativeHandlerEnabled ? _nativeHandler! : _socketHandler!;
if (DiagnosticsHandler.IsGloballyEnabled())
{
handler = new DiagnosticsHandler(handler, DistributedContextPropagator.Current);
}
MetricsHandler metricsHandler = new MetricsHandler(handler, _meterFactory);

// Ensure a single handler is used for all requests.
if (Interlocked.CompareExchange(ref _metricsHandler, metricsHandler, null) != null)
{
handler.Dispose();
}
return _metricsHandler;
}

private void CheckDisposedOrStarted()
{
ObjectDisposedException.ThrowIf(_disposed, this);
if (_metricsHandler != null)
{
throw new InvalidOperationException(SR.net_http_operation_started);
}
}

private void ThrowForModifiedManagedSslOptionsIfStarted()
{
// Hack to trigger an InvalidOperationException if a property that's stored on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
using System.Diagnostics;
using System.Diagnostics.Metrics;
#if TARGET_BROWSER
using System.Diagnostics;
using System.Net.Http.Metrics;
using HttpHandlerType = System.Net.Http.BrowserHttpHandler;
#else
using HttpHandlerType = System.Net.Http.SocketsHttpHandler;
Expand All @@ -23,7 +25,33 @@ public partial class HttpClientHandler : HttpMessageHandler
private readonly HttpHandlerType _underlyingHandler;

#if TARGET_BROWSER
private HttpMessageHandler Handler { get; }
private IMeterFactory? _meterFactory;
private MetricsHandler? _metricsHandler;

private MetricsHandler Handler
{
get
{
if (_metricsHandler != null)
{
return _metricsHandler;
}

HttpMessageHandler handler = _underlyingHandler;
if (DiagnosticsHandler.IsGloballyEnabled())
{
handler = new DiagnosticsHandler(handler, DistributedContextPropagator.Current);
}
MetricsHandler metricsHandler = new MetricsHandler(handler, _meterFactory);

// Ensure a single handler is used for all requests.
if (Interlocked.CompareExchange(ref _metricsHandler, metricsHandler, null) != null)
{
metricsHandler.Dispose();
}
return _metricsHandler;
}
}
#else
private HttpHandlerType Handler => _underlyingHandler;
#endif
Expand All @@ -34,14 +62,6 @@ public HttpClientHandler()
{
_underlyingHandler = new HttpHandlerType();

#if TARGET_BROWSER
Handler = _underlyingHandler;
if (DiagnosticsHandler.IsGloballyEnabled())
{
Handler = new DiagnosticsHandler(Handler, DistributedContextPropagator.Current);
}
#endif

ClientCertificateOptions = ClientCertificateOption.Manual;
}

Expand All @@ -60,6 +80,27 @@ protected override void Dispose(bool disposing)
public virtual bool SupportsProxy => HttpHandlerType.SupportsProxy;
public virtual bool SupportsRedirectConfiguration => HttpHandlerType.SupportsRedirectConfiguration;

[CLSCompliant(false)]

public IMeterFactory? MeterFactory
{
#if TARGET_BROWSER
get => _meterFactory;
set
{
ObjectDisposedException.ThrowIf(_disposed, this);
if (_metricsHandler != null)
{
throw new InvalidOperationException(SR.net_http_operation_started);
}
_meterFactory = value;
}
#else
get => _underlyingHandler.MeterFactory;
set => _underlyingHandler.MeterFactory = value;
#endif
}

[UnsupportedOSPlatform("browser")]
public bool UseCookies
{
Expand Down Expand Up @@ -288,6 +329,8 @@ public SslProtocols SslProtocols

public IDictionary<string, object?> Properties => _underlyingHandler.Properties;



//
// Attributes are commented out due to https://github.com/dotnet/arcade/issues/7585
// API compat will fail until this is fixed
Expand All @@ -296,11 +339,17 @@ public SslProtocols SslProtocols
[UnsupportedOSPlatform("browser")]
//[UnsupportedOSPlatform("ios")]
//[UnsupportedOSPlatform("tvos")]
protected internal override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) =>
Handler.Send(request, cancellationToken);
protected internal override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(request);
return Handler.Send(request, cancellationToken);
}

protected internal override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) =>
Handler.SendAsync(request, cancellationToken);
protected internal override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(request);
return Handler.SendAsync(request, cancellationToken);
}

// lazy-load the validator func so it can be trimmed by the ILLinker if it isn't used.
private static Func<HttpRequestMessage, X509Certificate2?, X509Chain?, SslPolicyErrors, bool>? s_dangerousAcceptAnyServerCertificateValidator;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;
using System.Net.Http.Headers;
using System.Runtime.Versioning;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -124,7 +122,6 @@ protected virtual void Dispose(bool disposing)
if (disposing && !_disposed)
{
_disposed = true;

if (_disposeHandler)
{
_handler.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class HttpRequestMessage : IDisposable
private Version _version;
private HttpVersionPolicy _versionPolicy;
private HttpContent? _content;
private HttpRequestOptions? _options;
internal HttpRequestOptions? _options;

public Version Version
{
Expand Down
Loading