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 AspNetCore and GrpcNetClient Instrumentation packages #4795

Merged
merged 8 commits into from
Aug 22, 2023
Merged
2 changes: 1 addition & 1 deletion build/test-aot-compatibility.ps1
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 = 19
$expectedWarningCount = 16

$testPassed = 0
if ($actualWarningCount -ne $expectedWarningCount)
Expand Down
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.Reflection;
#if !NETSTANDARD2_0
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -56,12 +59,13 @@ internal class HttpInListener : ListenerHandler
private const string UnknownHostName = "UNKNOWN-HOST";

private static readonly Func<HttpRequest, string, IEnumerable<string>> HttpRequestHeaderValuesGetter = (request, name) => request.Headers[name];
private static readonly PropertyFetcher<Exception> StopExceptionFetcher = new("Exception");
Yun-Ting marked this conversation as resolved.
Show resolved Hide resolved

#if !NET6_0_OR_GREATER
private readonly PropertyFetcher<object> beforeActionActionDescriptorFetcher = new("actionDescriptor");
private readonly PropertyFetcher<object> beforeActionAttributeRouteInfoFetcher = new("AttributeRouteInfo");
private readonly PropertyFetcher<string> beforeActionTemplateFetcher = new("Template");
#endif
private readonly PropertyFetcher<Exception> stopExceptionFetcher = new("Exception");
private readonly AspNetCoreInstrumentationOptions options;
private readonly bool emitOldAttributes;
private readonly bool emitNewAttributes;
Expand Down Expand Up @@ -404,7 +408,7 @@ public void OnException(Activity activity, object payload)
if (activity.IsAllDataRequested)
{
// We need to use reflection here as the payload type is not a defined public type.
if (!this.stopExceptionFetcher.TryFetch(payload, out Exception exc) || exc == null)
if (!TryFetchException(payload, out Exception exc))
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnException), activity.OperationName);
return;
Expand All @@ -426,6 +430,15 @@ public void OnException(Activity activity, object payload)
AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInListener), nameof(this.OnException), activity.OperationName, ex);
}
}

// See https://github.com/dotnet/aspnetcore/blob/690d78279e940d267669f825aa6627b0d731f64c/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L252
// and https://github.com/dotnet/aspnetcore/blob/690d78279e940d267669f825aa6627b0d731f64c/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs#L174
// this makes sure that top-level properties on the payload object are always preserved.
#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)
=> StopExceptionFetcher.TryFetch(payload, out exc) && exc != null;
}

private static string GetUri(HttpRequest request)
Expand Down
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.Reflection;
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Instrumentation.Http;
Expand All @@ -33,11 +36,12 @@ internal sealed class GrpcClientDiagnosticListener : ListenerHandler
private const string OnStartEvent = "Grpc.Net.Client.GrpcOut.Start";
private const string OnStopEvent = "Grpc.Net.Client.GrpcOut.Stop";

private static readonly PropertyFetcher<HttpRequestMessage> StartRequestFetcher = new("Request");
private static readonly PropertyFetcher<HttpResponseMessage> StopResponseFetcher = new("Response");

private readonly GrpcClientInstrumentationOptions options;
private readonly bool emitOldAttributes;
private readonly bool emitNewAttributes;
private readonly PropertyFetcher<HttpRequestMessage> startRequestFetcher = new("Request");
private readonly PropertyFetcher<HttpResponseMessage> stopRequestFetcher = new("Response");

public GrpcClientDiagnosticListener(GrpcClientInstrumentationOptions options)
: base("Grpc.Net.Client")
Expand Down Expand Up @@ -86,7 +90,7 @@ public void OnStartActivity(Activity activity, object payload)
}

// Ensure context propagation irrespective of sampling decision
if (!this.startRequestFetcher.TryFetch(payload, out HttpRequestMessage request) || request == null)
if (!TryFetchRequest(payload, out HttpRequestMessage request))
{
GrpcInstrumentationEventSource.Log.NullPayload(nameof(GrpcClientDiagnosticListener), nameof(this.OnStartActivity));
return;
Expand Down Expand Up @@ -180,6 +184,14 @@ public void OnStartActivity(Activity activity, object payload)
GrpcInstrumentationEventSource.Log.EnrichmentException(ex);
}
}

// See https://github.com/grpc/grpc-dotnet/blob/ff1a07b90c498f259e6d9f4a50cdad7c89ecd3c0/src/Grpc.Net.Client/Internal/GrpcCall.cs#L1180-L1183
// this makes sure that top-level properties on the payload object are always preserved.
#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)
=> StartRequestFetcher.TryFetch(payload, out request) && request != null;
}

public void OnStopActivity(Activity activity, object payload)
Expand All @@ -201,7 +213,7 @@ public void OnStopActivity(Activity activity, object payload)
// Remove the grpc.status_code tag added by the gRPC .NET library
activity.SetTag(GrpcTagHelper.GrpcStatusCodeTagName, null);

if (this.stopRequestFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
if (TryFetchResponse(payload, out HttpResponseMessage response))
{
try
{
Expand All @@ -213,5 +225,13 @@ public void OnStopActivity(Activity activity, object payload)
}
}
}

// See https://github.com/grpc/grpc-dotnet/blob/ff1a07b90c498f259e6d9f4a50cdad7c89ecd3c0/src/Grpc.Net.Client/Internal/GrpcCall.cs#L1180-L1183
// this makes sure that top-level properties on the payload object are always preserved.
#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;
}
}