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

Add a unit test to validate FX EventSource Events that are trimmer suppressed #49248

Open
LakshanF opened this issue Mar 5, 2021 · 5 comments
Assignees
Labels
area-System.Diagnostics.Tracing linkable-framework Issues associated with delivering a linker friendly framework
Milestone

Comments

@LakshanF
Copy link
Member

LakshanF commented Mar 5, 2021

Description

EventSource's WriteEvent(int,params object?[]) and Write<T> methods have been annotated with the RequiresUnreferencedCode to prevent developers who derive EventSource to get into trimming problems due to serialization issue. Developers need to manually look at the event method arguments and make them safe (either via additional dependecy attributes or if the arguments are primitive)

As part of making the change to add the RequiresUnreferencedCode, we manually looked at all the library eventsource and ensured that they are safe as described above.

We need to add a test (likely a reflection based one) to ensure that these methods continue to be safe

Configuration

.NET 6.0

Regression?

No

Other information

Take a look at similar tests @eerhardt added for other areas in the library

@LakshanF LakshanF self-assigned this Mar 5, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Diagnostics.Tracing untriaged New issue has not been triaged by the area owner labels Mar 5, 2021
@ghost
Copy link

ghost commented Mar 5, 2021

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

EventSource's WriteEvent(int,params object?[]) and Write methods have been annotated with the RequiresUnreferencedCode to prevent developers who derive EventSource to get into trimming problems due to serialization issue. Developers need to manually look at the event method arguments and make them safe (either via additional dependecy attributes or if the arguments are primitive)

As part of making the change to add the RequiresUnreferencedCode, we manually looked at all the library eventsource and ensured that they are safe as described above.

We need to add a test (likely a reflection based one) to ensure that these methods continue to be safe

Configuration

.NET 6.0

Regression?

No

Other information

Take a look at similar tests @eerhardt added for other areas in the library

Author: LakshanF
Assignees: LakshanF
Labels:

area-System.Diagnostics.Tracing, untriaged

Milestone: -

@LakshanF LakshanF added the linkable-framework Issues associated with delivering a linker friendly framework label Mar 5, 2021
@ghost
Copy link

ghost commented Mar 5, 2021

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @tannergooding, @sbomer
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

EventSource's WriteEvent(int,params object?[]) and Write methods have been annotated with the RequiresUnreferencedCode to prevent developers who derive EventSource to get into trimming problems due to serialization issue. Developers need to manually look at the event method arguments and make them safe (either via additional dependecy attributes or if the arguments are primitive)

As part of making the change to add the RequiresUnreferencedCode, we manually looked at all the library eventsource and ensured that they are safe as described above.

We need to add a test (likely a reflection based one) to ensure that these methods continue to be safe

Configuration

.NET 6.0

Regression?

No

Other information

Take a look at similar tests @eerhardt added for other areas in the library

Author: LakshanF
Assignees: LakshanF
Labels:

area-System.Diagnostics.Tracing, linkable-framework, untriaged

Milestone: -

@tommcdon tommcdon removed the untriaged New issue has not been triaged by the area owner label Mar 9, 2021
@tommcdon tommcdon added this to the 6.0.0 milestone Mar 12, 2021
@LakshanF LakshanF modified the milestones: 6.0.0, 7.0.0 Jul 21, 2021
@LakshanF
Copy link
Member Author

Cannot think of an easy way to add a generic test and moving this issue the next version. The best practice for any new Framework EventSource should have the following flavor (and avoid the need for a generic testing approach) but need to get consensus and evangelize this;

@LakshanF
Copy link
Member Author

Added a manifest trimming test, #56463. And there is a good smoke EventSource test in the same directory that should cover the basic scenarios testing for .NET6

@tommcdon tommcdon modified the milestones: 7.0.0, Future Jul 7, 2022
@tommcdon
Copy link
Member

tommcdon commented Jul 7, 2022

Moving older issues to the Future milestone as it is unlikely we will have time to address this in the .NET 7 timeframe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Diagnostics.Tracing linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

No branches or pull requests

2 participants