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

[AOT] suppressed trimming warnings in HTTP Instrumentation package #4770

Merged
merged 11 commits into from Aug 16, 2023
2 changes: 1 addition & 1 deletion build/test-aot-compatibility.ps1
Expand Up @@ -29,7 +29,7 @@ if ($LastExitCode -ne 0)
popd

Write-Host "Actual warning count is:", $actualWarningCount
$expectedWarningCount = 26
$expectedWarningCount = 20

$testPassed = 0
if ($actualWarningCount -ne $expectedWarningCount)
Expand Down
Expand Up @@ -15,6 +15,9 @@
// </copyright>

using System.Diagnostics;
#if NET6_0_OR_GREATER
using System.Diagnostics.CodeAnalysis;
#endif
#if NETFRAMEWORK
using System.Net.Http;
#endif
Expand All @@ -40,10 +43,10 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler
private const string OnStopEvent = "System.Net.Http.HttpRequestOut.Stop";
private const string OnUnhandledExceptionEvent = "System.Net.Http.Exception";

private readonly PropertyFetcher<HttpRequestMessage> startRequestFetcher = new("Request");
private readonly PropertyFetcher<HttpResponseMessage> stopResponseFetcher = new("Response");
private readonly PropertyFetcher<Exception> stopExceptionFetcher = new("Exception");
private readonly PropertyFetcher<TaskStatus> stopRequestStatusFetcher = new("RequestTaskStatus");
private static readonly PropertyFetcher<HttpRequestMessage> StartRequestFetcher = new("Request");
private static readonly PropertyFetcher<HttpResponseMessage> StopResponseFetcher = new("Response");
private static readonly PropertyFetcher<Exception> StopExceptionFetcher = new("Exception");
private static readonly PropertyFetcher<TaskStatus> StopRequestStatusFetcher = new("RequestTaskStatus");
private readonly HttpClientInstrumentationOptions options;
private readonly bool emitOldAttributes;
private readonly bool emitNewAttributes;
Expand Down Expand Up @@ -112,7 +115,7 @@ public void OnStartActivity(Activity activity, object payload)
return;
}

if (!this.startRequestFetcher.TryFetch(payload, out HttpRequestMessage request) || request == null)
if (!TryFetchRequest(payload, out HttpRequestMessage request))
{
HttpInstrumentationEventSource.Log.NullPayload(nameof(HttpHandlerDiagnosticListener), nameof(this.OnStartActivity));
return;
Expand Down Expand Up @@ -211,15 +214,28 @@ public void OnStartActivity(Activity activity, object payload)
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}
}

// The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved.
// see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325
#if NET6_0_OR_GREATER
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The event source guarantees that top-level properties are preserved")]
#endif
static bool TryFetchRequest(object payload, out HttpRequestMessage request)
{
if (!StartRequestFetcher.TryFetch(payload, out request) || request == null)
{
return false;
}

return true;
}
}

public void OnStopActivity(Activity activity, object payload)
{
if (activity.IsAllDataRequested)
{
// https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs
// requestTaskStatus is not null
_ = this.stopRequestStatusFetcher.TryFetch(payload, out var requestTaskStatus);
var requestTaskStatus = GetRequestStatus(payload);

ActivityStatusCode currentStatusCode = activity.Status;
if (requestTaskStatus != TaskStatus.RanToCompletion)
Expand All @@ -241,7 +257,7 @@ public void OnStopActivity(Activity activity, object payload)
}
}

if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
if (TryFetchResponse(payload, out HttpResponseMessage response))
{
if (this.emitOldAttributes)
{
Expand All @@ -267,14 +283,43 @@ public void OnStopActivity(Activity activity, object payload)
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}
}

// The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved.
// see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325
#if NET6_0_OR_GREATER
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The event source guarantees that top-level properties are preserved")]
#endif
static TaskStatus GetRequestStatus(object payload)
{
// requestTaskStatus (type is TaskStatus) is a non-nullable enum so we don't need to have a null check here.
// See: https://github.com/dotnet/runtime/blob/79c021d65c280020246d1035b0e87ae36f2d36a9/src/libraries/System.Net.Http/src/HttpDiagnosticsGuide.md?plain=1#L69
_ = StopRequestStatusFetcher.TryFetch(payload, out var requestTaskStatus);

return requestTaskStatus;
}
}

// The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved.
// see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325
#if NET6_0_OR_GREATER
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The event source guarantees that top-level properties are preserved")]
#endif
static bool TryFetchResponse(object payload, out HttpResponseMessage response)
{
if (StopResponseFetcher.TryFetch(payload, out response) && response != null)
{
return true;
}

return false;
}
}

public void OnException(Activity activity, object payload)
{
if (activity.IsAllDataRequested)
{
if (!this.stopExceptionFetcher.TryFetch(payload, out Exception exc) || exc == null)
if (!TryFetchException(payload, out Exception exc))
{
HttpInstrumentationEventSource.Log.NullPayload(nameof(HttpHandlerDiagnosticListener), nameof(this.OnException));
return;
Expand All @@ -299,5 +344,20 @@ public void OnException(Activity activity, object payload)
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}
}

// The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved.
// see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325
#if NET6_0_OR_GREATER
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The event source guarantees that top-level properties are preserved")]
#endif
static bool TryFetchException(object payload, out Exception exc)
{
if (!StopExceptionFetcher.TryFetch(payload, out exc) || exc == null)
{
return false;
}

return true;
}
}
}
Expand Up @@ -15,6 +15,9 @@
// </copyright>

using System.Diagnostics;
#if NET6_0_OR_GREATER
using System.Diagnostics.CodeAnalysis;
#endif
using System.Diagnostics.Metrics;
#if NETFRAMEWORK
using System.Net.Http;
Expand All @@ -28,8 +31,8 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler
{
internal const string OnStopEvent = "System.Net.Http.HttpRequestOut.Stop";

private readonly PropertyFetcher<HttpResponseMessage> stopResponseFetcher = new("Response");
private readonly PropertyFetcher<HttpRequestMessage> stopRequestFetcher = new("Request");
private static readonly PropertyFetcher<HttpResponseMessage> StopResponseFetcher = new("Response");
private static readonly PropertyFetcher<HttpRequestMessage> StopRequestFetcher = new("Request");
Yun-Ting marked this conversation as resolved.
Show resolved Hide resolved
private readonly Histogram<double> httpClientDuration;
private readonly HttpClientMetricInstrumentationOptions options;
private readonly bool emitOldAttributes;
Expand All @@ -56,7 +59,7 @@ public override void OnEventWritten(string name, object payload)
}

var activity = Activity.Current;
if (this.stopRequestFetcher.TryFetch(payload, out HttpRequestMessage request) && request != null)
if (TryFetchRequest(payload, out HttpRequestMessage request))
{
TagList tags = default;

Expand All @@ -73,7 +76,7 @@ public override void OnEventWritten(string name, object payload)
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port));
}

if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
if (TryFetchResponse(payload, out HttpResponseMessage response))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)));
}
Expand All @@ -91,7 +94,7 @@ public override void OnEventWritten(string name, object payload)
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeServerPort, request.RequestUri.Port));
}

if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
if (TryFetchResponse(payload, out HttpResponseMessage response))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)));
}
Expand All @@ -103,5 +106,21 @@ public override void OnEventWritten(string name, object payload)
this.httpClientDuration.Record(activity.Duration.TotalMilliseconds, tags);
}
}

// The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved.
// see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325
#if NET6_0_OR_GREATER
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The event source guarantees that top-level properties are preserved")]
#endif
static bool TryFetchRequest(object payload, out HttpRequestMessage request) =>
StopRequestFetcher.TryFetch(payload, out request) && request != null;

// The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved.
// see https://github.com/dotnet/runtime/blob/f9246538e3d49b90b0e9128d7b1defef57cd6911/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L325
#if NET6_0_OR_GREATER
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The event source guarantees that top-level properties are preserved")]
#endif
static bool TryFetchResponse(object payload, out HttpResponseMessage response) =>
StopResponseFetcher.TryFetch(payload, out response) && response != null;
}
}