Skip to content

Commit

Permalink
[AOT] suppressed trimming warnings in HTTP Instrumentation package (#…
Browse files Browse the repository at this point in the history 
…4770)

Co-authored-by: Eric Erhardt <[email protected]>
Co-authored-by: Reiley Yang <[email protected]>
  • Loading branch information
@Yun-Ting @eerhardt @reyang
3 people committed Aug 16, 2023
1 parent 49031ba commit 5784b45
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 16 deletions.
2 changes: 1 addition & 1 deletion build/test-aot-compatibility.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ if ($LastExitCode -ne 0)
popd

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

$testPassed = 0
if ($actualWarningCount -ne $expectedWarningCount)
Expand Down
80 changes: 70 additions & 10 deletions src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
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;
}
}
}
29 changes: 24 additions & 5 deletions ...OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
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<HttpRequestMessage> StopRequestFetcher = new("Request");
private static readonly PropertyFetcher<HttpResponseMessage> StopResponseFetcher = new("Response");
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;
}
}

0 comments on commit 5784b45

Please sign in to comment.