Skip to content

Simplify Http.Diagnostics #6174

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Simplify Http.Diagnostics #6174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 11 additions & 12 deletions ...braries/Microsoft.Extensions.Http.Diagnostics/Http/DownstreamDependencyMetadataManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,31 +100,30 @@ private static void AddRouteToTrie(RequestMetadata routeMetadata, Dictionary<str
var trieCurrent = routeMetadataTrieRoot;
trieCurrent.Parent = trieCurrent;

ReadOnlySpan<char> requestRouteAsSpan = routeMetadata.RequestRoute.AsSpan();

if (requestRouteAsSpan.Length > 0)
var route = routeMetadata.RequestRoute;
if (!string.IsNullOrEmpty(route))
{
if (requestRouteAsSpan[0] != '/')
if (route[0] != '/')
{
requestRouteAsSpan = $"/{routeMetadata.RequestRoute}".AsSpan();
route = $"/{routeMetadata.RequestRoute}";
}
else if (requestRouteAsSpan.StartsWith("//".AsSpan(), StringComparison.OrdinalIgnoreCase))
else if (route.StartsWith("//", StringComparison.Ordinal))
{
requestRouteAsSpan = requestRouteAsSpan.Slice(1);
route = route.Substring(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this now possibly increasing allocation? Same below?

I'm not clear on why this is an improvement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really since previously requestRouteAsSpan.ToString() was called on the span always, even if it was initialized from a string or even a constant.

}

if (requestRouteAsSpan.Length > 1 && requestRouteAsSpan[requestRouteAsSpan.Length - 1] == '/')
if (route.Length > 1 && route[route.Length - 1] == '/')
{
requestRouteAsSpan = requestRouteAsSpan.Slice(0, requestRouteAsSpan.Length - 1);
route = route.Substring(0, route.Length - 1);
}

route = _routeRegex.Replace(route, "*").ToUpperInvariant();
}
else
{
requestRouteAsSpan = "/".AsSpan();
route = "/";
}

var route = _routeRegex.Replace(requestRouteAsSpan.ToString(), "*");
route = route.ToUpperInvariant();
for (int i = 0; i < route.Length; i++)
{
char ch = route[i];
Expand Down
4 changes: 2 additions & 2 deletions ...ies/Microsoft.Extensions.Http.Diagnostics/Latency/HttpClientLatencyTelemetryExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public static IServiceCollection AddHttpClientLatencyTelemetry(this IServiceColl

_ = services.RegisterCheckpointNames(HttpCheckpoints.Checkpoints);
_ = services.AddOptions<HttpClientLatencyTelemetryOptions>();
_ = services.AddActivatedSingleton<HttpRequestLatencyListener>();
_ = services.AddActivatedSingleton<HttpClientLatencyContext>();
_ = services.AddSingleton<HttpRequestLatencyListener>();
_ = services.AddSingleton<HttpClientLatencyContext>();
_ = services.AddTransient<HttpLatencyTelemetryHandler>();
_ = services.AddHttpClientLogEnricher<HttpClientLatencyLogEnricher>();

Expand Down
5 changes: 0 additions & 5 deletions ...raries/Microsoft.Extensions.Http.Diagnostics/Latency/Internal/HttpClientLatencyContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,4 @@ public void Set(ILatencyContext context)
{
_latencyContext.Value = context;
}

public void Unset()
{
_latencyContext.Value = null;
}
}
18 changes: 5 additions & 13 deletions ...ies/Microsoft.Extensions.Http.Diagnostics/Latency/Internal/HttpLatencyTelemetryHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using Microsoft.Extensions.Diagnostics.Latency;
using Microsoft.Extensions.Http.Diagnostics;
using Microsoft.Extensions.Options;
using Microsoft.Shared.Diagnostics;

namespace Microsoft.Extensions.Http.Latency.Internal;

Expand All @@ -24,17 +23,14 @@ internal sealed class HttpLatencyTelemetryHandler : DelegatingHandler
private readonly string _applicationName;

public HttpLatencyTelemetryHandler(HttpRequestLatencyListener latencyListener, ILatencyContextTokenIssuer tokenIssuer, ILatencyContextProvider latencyContextProvider,
IOptions<HttpClientLatencyTelemetryOptions> options, IOptions<ApplicationMetadata> appMetdata)
IOptions<HttpClientLatencyTelemetryOptions> options, IOptions<ApplicationMetadata> appMetadata)
{
var appMetadata = Throw.IfMemberNull(appMetdata, appMetdata.Value);
var telemetryOptions = Throw.IfMemberNull(options, options.Value);

_latencyListener = latencyListener;
_latencyContextProvider = latencyContextProvider;
_handlerStart = tokenIssuer.GetCheckpointToken(HttpCheckpoints.HandlerRequestStart);
_applicationName = appMetdata.Value.ApplicationName;
_applicationName = appMetadata.Value.ApplicationName;

if (telemetryOptions.EnableDetailedLatencyBreakdown)
if (options.Value.EnableDetailedLatencyBreakdown)
{
_latencyListener.Enable();
}
Expand All @@ -46,12 +42,8 @@ protected async override Task<HttpResponseMessage> SendAsync(HttpRequestMessage
context.AddCheckpoint(_handlerStart);
_latencyListener.LatencyContext.Set(context);

request.Headers.Add(TelemetryConstants.ClientApplicationNameHeader, _applicationName);

var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);

_latencyListener.LatencyContext.Unset();
_ = request.Headers.TryAddWithoutValidation(TelemetryConstants.ClientApplicationNameHeader, _applicationName);

return response;
return await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
}
}
81 changes: 21 additions & 60 deletions ...ries/Microsoft.Extensions.Http.Diagnostics/Latency/Internal/HttpRequestLatencyListener.cs
View file
Open in desktop
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;
using System.Collections.Concurrent;
using System.Collections.Frozen;
using System.Collections.Generic;
using System.Diagnostics.Tracing;
Expand All @@ -17,57 +15,43 @@ internal sealed class HttpRequestLatencyListener : EventListener
private const string HttpProviderName = "System.Net.Http";
private const string NameResolutionProivderName = "System.Net.NameResolution";

private readonly ConcurrentDictionary<string, EventSource?> _eventSources = new()
{
[SocketProviderName] = null,
[HttpProviderName] = null,
[NameResolutionProivderName] = null
};
private readonly FrozenDictionary<string, FrozenDictionary<string, CheckpointToken>> _eventToTokenMap;

internal HttpClientLatencyContext LatencyContext { get; }

private readonly EventToCheckpointToken _eventToCheckpointToken;

private int _enabled;

internal bool Enabled => _enabled == 1;

public HttpRequestLatencyListener(HttpClientLatencyContext latencyContext, ILatencyContextTokenIssuer tokenIssuer)
{
LatencyContext = latencyContext;
_eventToCheckpointToken = new(tokenIssuer);
_eventToTokenMap = EventToCheckpointToken.Build(tokenIssuer);
}

public void Enable()
{
if (Interlocked.CompareExchange(ref _enabled, 1, 0) == 0)
{
foreach (var eventSource in _eventSources)
{
if (eventSource.Value != null)
{
EnableEventSource(eventSource.Value);
}
}
// process already existing listeners once again
EventSourceCreated += (_, args) => OnEventSourceCreated(args.EventSource!);
}
}

internal void OnEventWritten(string eventSourceName, string? eventName)
{
// If event of interest, add a checkpoint for it.
CheckpointToken? token = _eventToCheckpointToken.GetCheckpointToken(eventSourceName, eventName);
if (token.HasValue)
if (eventName != null && _eventToTokenMap[eventSourceName].TryGetValue(eventName, out var token))
{
LatencyContext.Get()?.AddCheckpoint(token.Value);
LatencyContext.Get()?.AddCheckpoint(token);
}
}

internal void OnEventSourceCreated(string eventSourceName, EventSource eventSource)
{
if (_eventSources.ContainsKey(eventSourceName))
if (Enabled && _eventToTokenMap.ContainsKey(eventSourceName))
{
_eventSources[eventSourceName] = eventSource;
EnableEventSource(eventSource);
EnableEvents(eventSource, EventLevel.Informational);
}
}

Expand All @@ -81,15 +65,7 @@ protected override void OnEventWritten(EventWrittenEventArgs eventData)
OnEventWritten(eventData.EventSource.Name, eventData.EventName);
}

private void EnableEventSource(EventSource eventSource)
{
if (Enabled && !eventSource.IsEnabled())
{
EnableEvents(eventSource, EventLevel.Informational);
}
}

private sealed class EventToCheckpointToken
private static class EventToCheckpointToken
{
private static readonly Dictionary<string, string> _socketMap = new()
{
Expand Down Expand Up @@ -117,47 +93,32 @@ private sealed class EventToCheckpointToken
{ "ResponseContentStop", HttpCheckpoints.ResponseContentEnd }
};

private readonly FrozenDictionary<string, FrozenDictionary<string, CheckpointToken>> _eventToTokenMap;

public EventToCheckpointToken(ILatencyContextTokenIssuer tokenIssuer)
public static FrozenDictionary<string, FrozenDictionary<string, CheckpointToken>> Build(ILatencyContextTokenIssuer tokenIssuer)
{
Dictionary<string, CheckpointToken> socket = [];
foreach (string key in _socketMap.Keys)
foreach (var kv in _socketMap)
{
socket[key] = tokenIssuer.GetCheckpointToken(_socketMap[key]);
socket[kv.Key] = tokenIssuer.GetCheckpointToken(kv.Value);
}

Dictionary<string, CheckpointToken> nameResolution = [];
foreach (string key in _nameResolutionMap.Keys)
foreach (var kv in _nameResolutionMap)
{
nameResolution[key] = tokenIssuer.GetCheckpointToken(_nameResolutionMap[key]);
nameResolution[kv.Key] = tokenIssuer.GetCheckpointToken(kv.Value);
}

Dictionary<string, CheckpointToken> http = [];
foreach (string key in _httpMap.Keys)
foreach (var kv in _httpMap)
{
http[key] = tokenIssuer.GetCheckpointToken(_httpMap[key]);
http[kv.Key] = tokenIssuer.GetCheckpointToken(kv.Value);
}

_eventToTokenMap = new Dictionary<string, FrozenDictionary<string, CheckpointToken>>
return new Dictionary<string, FrozenDictionary<string, CheckpointToken>>
{
{ SocketProviderName, socket.ToFrozenDictionary(StringComparer.Ordinal) },
{ NameResolutionProivderName, nameResolution.ToFrozenDictionary(StringComparer.Ordinal) },
{ HttpProviderName, http.ToFrozenDictionary(StringComparer.Ordinal) }
}.ToFrozenDictionary(StringComparer.Ordinal);
}

public CheckpointToken? GetCheckpointToken(string eventSourceName, string? eventName)
{
if (eventName != null && _eventToTokenMap.TryGetValue(eventSourceName, out var events))
{
if (events.TryGetValue(eventName, out var token))
{
return token;
}
}

return null;
{ SocketProviderName, socket.ToFrozenDictionary() },
{ NameResolutionProivderName, nameResolution.ToFrozenDictionary() },
{ HttpProviderName, http.ToFrozenDictionary() }
}.ToFrozenDictionary();
}
}
}
8 changes: 4 additions & 4 deletions src/Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpClientLogger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,22 +109,22 @@ internal HttpClientLogger(
}
}

public async ValueTask LogRequestStopAsync(
public ValueTask LogRequestStopAsync(
object? context,
HttpRequestMessage request,
HttpResponseMessage response,
TimeSpan elapsed,
CancellationToken cancellationToken = default)
=> await LogResponseAsync(context, request, response, null, elapsed, cancellationToken).ConfigureAwait(false);
=> LogResponseAsync(context, request, response, null, elapsed, cancellationToken);

public async ValueTask LogRequestFailedAsync(
public ValueTask LogRequestFailedAsync(
object? context,
HttpRequestMessage request,
HttpResponseMessage? response,
Exception exception,
TimeSpan elapsed,
CancellationToken cancellationToken = default)
=> await LogResponseAsync(context, request, response, exception, elapsed, cancellationToken).ConfigureAwait(false);
=> LogResponseAsync(context, request, response, exception, elapsed, cancellationToken);

public object? LogRequestStart(HttpRequestMessage request)
=> throw new NotSupportedException(SyncLoggingExceptionMessage);
Expand Down
37 changes: 7 additions & 30 deletions ...Libraries/Microsoft.Extensions.Http.Diagnostics/Logging/Internal/HttpRequestBodyReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,14 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Buffers;
using System.Collections.Frozen;
using System.IO;
using System.Net.Http;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
#if NETCOREAPP3_1_OR_GREATER
using Microsoft.Extensions.ObjectPool;
#endif
using Microsoft.Shared.Diagnostics;
#if NETCOREAPP3_1_OR_GREATER
using Microsoft.Shared.Pools;
#else
using System.Buffers;
#endif

namespace Microsoft.Extensions.Http.Logging.Internal;

Expand All @@ -27,9 +20,6 @@ internal sealed class HttpRequestBodyReader
/// </summary>
internal readonly TimeSpan RequestReadTimeout;

#if NETCOREAPP3_1_OR_GREATER
private static readonly ObjectPool<BufferWriter<byte>> _bufferWriterPool = BufferWriterPool.SharedBufferWriterPool;
#endif
private readonly FrozenSet<string> _readableRequestContentTypes;
private readonly int _requestReadLimit;

Expand Down Expand Up @@ -93,33 +83,20 @@ private static async ValueTask<string> ReadFromStreamAsync(HttpRequestMessage re
#endif

var readLimit = Math.Min(readSizeLimit, (int)streamToReadFrom.Length);
#if NETCOREAPP3_1_OR_GREATER
var bufferWriter = _bufferWriterPool.Get();
try
{
var memory = bufferWriter.GetMemory(readLimit).Slice(0, readLimit);
var charsWritten = await streamToReadFrom.ReadAsync(memory, cancellationToken).ConfigureAwait(false);

return Encoding.UTF8.GetString(memory[..charsWritten].Span);
}
finally
{
_bufferWriterPool.Return(bufferWriter);
streamToReadFrom.Seek(0, SeekOrigin.Begin);
}

#else
var buffer = ArrayPool<byte>.Shared.Rent(readLimit);
try
{
_ = await streamToReadFrom.ReadAsync(buffer, 0, buffer.Length, cancellationToken).ConfigureAwait(false);
return Encoding.UTF8.GetString(buffer.AsSpan(0, readLimit).ToArray());
#if NET
var read = await streamToReadFrom.ReadAsync(buffer.AsMemory(0, readLimit), cancellationToken).ConfigureAwait(false);
#else
var read = await streamToReadFrom.ReadAsync(buffer, 0, readLimit, cancellationToken).ConfigureAwait(false);
#endif
return Encoding.UTF8.GetString(buffer, 0, read);
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
streamToReadFrom.Seek(0, SeekOrigin.Begin);
}
#endif
}
}
Loading
Loading