-
Notifications
You must be signed in to change notification settings - Fork 802
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
Conversation
1be02e2
to
4994f52
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4795 +/- ##
==========================================
+ Coverage 85.57% 85.62% +0.04%
==========================================
Files 284 284
Lines 11683 11686 +3
==========================================
+ Hits 9998 10006 +8
+ Misses 1685 1680 -5
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
This is part of the PR description - and it's incorrect - it's probably a copy/paste from the previous PR which did this for System.Net.Http. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me with a non-blocking naming question.
Towards #3429
cc: @vitek-karas @eerhardt
Changes
Suppressed trimming warnings IL2026 in AspNetCore and GrpcNetClient Instrumentation packages because the AOT-annotation DynamicallyAccessedMembers
https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.dynamicallyaccessedmembersattribute?view=net-7.0
ensures that top-level properties on the payload object are always preserved.
For OpenTelemetry.Instrumentation.AspNetCore, 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
For OpenTelemetry.GrpcNetClient.Instrumentation, see
https://github.com/grpc/grpc-dotnet/blob/ff1a07b90c498f259e6d9f4a50cdad7c89ecd3c0/src/Grpc.Net.Client/Internal/GrpcCall.cs#L1180-L1183
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial changes