VSadov/Satori
1
0
Fork 0
mirror of https://github.com/VSadov/Satori.git synced 2025-06-09 09:34:49 +09:00
Code Activity

Swap MetricsHandler and DiagnosticsHandler (#104455)

Browse source 
To support Exemplars, http.request.duration must be recorded before stopping the HTTP request Activity.
This commit is contained in:
Anton Firszov 2024-07-08 22:17:08 +02:00 • committed by GitHub
parent e4d9e266b7
commit 8b9ea5e180
Signed by: github
GPG key ID: B5690EEEBB952194
5 changed files with 83 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.InvokeNativeHandler.cs
View file

@ -176,7 +176,7 @@ namespace System.Net.Http
try
{
return method!.Invoke(_nativeHandler, parameters)!;
return method!.Invoke(_nativeUnderlyingHandler, parameters)!;
}
catch (TargetInvocationException e)
{

21
src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs
View file

@ -22,9 +22,9 @@ namespace System.Net.Http
{
private static readonly ConcurrentDictionary<string, MethodInfo?> s_cachedMethods = new();
private readonly HttpMessageHandler? _nativeHandler;
private readonly HttpMessageHandler? _nativeUnderlyingHandler;
private IMeterFactory? _nativeMeterFactory;
private MetricsHandler? _nativeMetricsHandler;
private HttpMessageHandler? _nativeFirstHandler; // DiagnosticsHandler or MetricsHandler, depending on global configuration.
private readonly SocketsHttpHandler? _socketHandler;
@ -38,23 +38,24 @@ namespace System.Net.Http
{
if (IsNativeHandlerEnabled)
{
if (_nativeMetricsHandler is null)
if (_nativeFirstHandler is null)
{
// We only setup these handlers for the native handler. SocketsHttpHandler already does this internally.
HttpMessageHandler handler = _nativeHandler!;
HttpMessageHandler handler = _nativeUnderlyingHandler!;
// MetricsHandler should be descendant of DiagnosticsHandler in the handler chain to make sure the 'http.request.duration'
// metric is recorded before stopping the request Activity. This is needed to make sure that our telemetry supports Exemplars.
handler = new MetricsHandler(handler, _nativeMeterFactory, out _);
if (DiagnosticsHandler.IsGloballyEnabled())
{
handler = new DiagnosticsHandler(handler, DistributedContextPropagator.Current);
}
MetricsHandler metricsHandler = new MetricsHandler(handler, _nativeMeterFactory, out _);
// Ensure a single handler is used for all requests.
Interlocked.CompareExchange(ref _nativeMetricsHandler, metricsHandler, null);
Interlocked.CompareExchange(ref _nativeFirstHandler, handler, null);
}
return _nativeMetricsHandler;
return _nativeFirstHandler;
}
else
{
@ -67,7 +68,7 @@ namespace System.Net.Http
{
if (IsNativeHandlerEnabled)
{
_nativeHandler = CreateNativeHandler();
_nativeUnderlyingHandler = CreateNativeHandler();
}
else
{
@ -115,7 +116,7 @@ namespace System.Net.Http
if (IsNativeHandlerEnabled)
{
if (_nativeMetricsHandler is not null)
if (_nativeFirstHandler is not null)
{
throw new InvalidOperationException(SR.net_http_operation_started);
}

22
src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs
View file

@ -26,30 +26,34 @@ namespace System.Net.Http
#if TARGET_BROWSER
private IMeterFactory? _meterFactory;
private MetricsHandler? _metricsHandler;
private HttpMessageHandler? _firstHandler; // DiagnosticsHandler or MetricsHandler, depending on global configuration.
private MetricsHandler Handler
private HttpMessageHandler Handler
{
get
{
if (_metricsHandler != null)
if (_firstHandler != null)
{
return _metricsHandler;
return _firstHandler;
}
HttpMessageHandler handler = _underlyingHandler;
// MetricsHandler should be descendant of DiagnosticsHandler in the handler chain to make sure the 'http.request.duration'
// metric is recorded before stopping the request Activity. This is needed to make sure that our telemetry supports Exemplars.
handler = new MetricsHandler(handler, _meterFactory, out _);
if (DiagnosticsHandler.IsGloballyEnabled())
{
handler = new DiagnosticsHandler(handler, DistributedContextPropagator.Current);
}
MetricsHandler metricsHandler = new MetricsHandler(handler, _meterFactory, out _);
// Ensure a single handler is used for all requests.
if (Interlocked.CompareExchange(ref _metricsHandler, metricsHandler, null) != null)
if (Interlocked.CompareExchange(ref _firstHandler, handler, null) != null)
{
metricsHandler.Dispose();
handler.Dispose();
}
return _metricsHandler;
return _firstHandler;
}
}
#else
@ -95,7 +99,7 @@ namespace System.Net.Http
set
{
ObjectDisposedException.ThrowIf(_disposed, this);
if (_metricsHandler != null)
if (_firstHandler != null)
{
throw new InvalidOperationException(SR.net_http_operation_started);
}

9
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs
View file

@ -529,16 +529,17 @@ namespace System.Net.Http
handler = new HttpAuthenticatedConnectionHandler(poolManager);
}
// MetricsHandler should be descendant of DiagnosticsHandler in the handler chain to make sure the 'http.request.duration'
// metric is recorded before stopping the request Activity. This is needed to make sure that our telemetry supports Exemplars.
handler = new MetricsHandler(handler, settings._meterFactory, out Meter meter);
settings._metrics = new SocketsHttpHandlerMetrics(meter);
// DiagnosticsHandler is inserted before RedirectHandler so that trace propagation is done on redirects as well
if (DiagnosticsHandler.IsGloballyEnabled() && settings._activityHeadersPropagator is DistributedContextPropagator propagator)
{
handler = new DiagnosticsHandler(handler, propagator, settings._allowAutoRedirect);
}
handler = new MetricsHandler(handler, settings._meterFactory, out Meter meter);
settings._metrics = new SocketsHttpHandlerMetrics(meter);
if (settings._allowAutoRedirect)
{
// Just as with WinHttpHandler, for security reasons, we do not support authentication on redirects

55
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs
View file

@ -160,6 +160,8 @@ namespace System.Net.Http.Functional.Tests
private readonly ConcurrentQueue<Measurement<T>> _values = new();
private Meter? _meter;
public Action? MeasurementRecorded;
public InstrumentRecorder(string instrumentName)
{
_meterListener.InstrumentPublished = (instrument, listener) =>
@ -187,7 +189,12 @@ namespace System.Net.Http.Functional.Tests
_meterListener.Start();
}
private void OnMeasurementRecorded(Instrument instrument, T measurement, ReadOnlySpan<KeyValuePair<string, object?>> tags, object? state) => _values.Enqueue(new Measurement<T>(measurement, tags));
private void OnMeasurementRecorded(Instrument instrument, T measurement, ReadOnlySpan<KeyValuePair<string, object?>> tags, object? state)
{
_values.Enqueue(new Measurement<T>(measurement, tags));
MeasurementRecorded?.Invoke();
}
public IReadOnlyList<Measurement<T>> GetMeasurements() => _values.ToArray();
public void Dispose() => _meterListener.Dispose();
}
@ -334,6 +341,51 @@ namespace System.Net.Http.Functional.Tests
});
}
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public async Task RequestDuration_HttpTracingEnabled_RecordedWhileRequestActivityRunning()
{
await RemoteExecutor.Invoke(static testClass =>
{
HttpMetricsTest test = (HttpMetricsTest)Activator.CreateInstance(Type.GetType(testClass), (ITestOutputHelper)null);
return test.LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
{
using HttpMessageInvoker client = test.CreateHttpMessageInvoker();
using InstrumentRecorder<double> recorder = test.SetupInstrumentRecorder<double>(InstrumentNames.RequestDuration);
Activity? activity = null;
bool stopped = false;
ActivitySource.AddActivityListener(new ActivityListener
{
ShouldListenTo = s => s.Name is "System.Net.Http",
Sample = (ref ActivityCreationOptions<ActivityContext> _) => ActivitySamplingResult.AllData,
ActivityStarted = created => activity = created,
ActivityStopped = _ => stopped = true
});
recorder.MeasurementRecorded = () =>
{
Assert.NotNull(activity);
Assert.False(stopped);
Assert.Same(activity, Activity.Current);
};
using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = test.UseVersion };
using HttpResponseMessage response = await test.SendAsync(client, request);
Assert.NotNull(activity);
Measurement<double> m = Assert.Single(recorder.GetMeasurements());
VerifyRequestDuration(m, uri, test.UseVersion, 200, "GET");
}, async server =>
{
await server.AcceptConnectionSendResponseAndCloseAsync();
});
}, GetType().FullName).DisposeAsync();
}
[Fact]
public Task RequestDuration_CustomTags_Recorded()
{
@ -363,7 +415,6 @@ namespace System.Net.Http.Functional.Tests
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[InlineData("System.Net.Http.HttpRequestOut.Start")]
[InlineData("System.Net.Http.Request")]
[InlineData("System.Net.Http.HttpRequestOut.Stop")]
public async Task RequestDuration_CustomTags_DiagnosticListener_Recorded(string eventName)
{
await RemoteExecutor.Invoke(static async (testClassName, eventNameInner) =>