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

Use source generator to assist EventSource initialization for SPCL EventSources #49659

Open
sywhang opened this issue Mar 15, 2021 · 3 comments
Labels
area-System.Diagnostics.Tracing enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@sywhang
Copy link
Contributor

sywhang commented Mar 15, 2021

EventSource suffers from reflection-heavy ops upon initialization to generate manifest for the event data where it uses reflection to look up every single event-annotated method + their attributes defined to generate the manifest data.

This issue tracks the work to assist the generation of these manifest data for EventSources in System.Private.CoreLib to use source generator. Once this is successful, we can look into expanding the set of EventSources in the BCL to use the generator.

Related issues:
#49592 #45466

cc @tommcdon @josalem

@sywhang sywhang added enhancement Product code improvement that does NOT require public API changes/additions area-System.Diagnostics.Tracing labels Mar 15, 2021
@sywhang sywhang added this to the 6.0.0 milestone Mar 15, 2021
@sywhang sywhang self-assigned this Mar 15, 2021
@ghost
Copy link

ghost commented Mar 15, 2021

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

Issue Details

EventSource suffers from reflection-heavy ops upon initialization to generate manifest for the event data where it uses reflection to look up every single event-annotated method + their attributes defined to generate the manifest data.

This issue tracks the work to assist the generation of these manifest data for EventSources in System.Private.CoreLib to use source generator. Once this is successful, we can look into expanding the set of EventSources in the BCL to use the generator.

Related issues:
#49592 #45466

cc @tommcdon @josalem

Author: sywhang
Assignees: sywhang
Labels:

area-System.Diagnostics.Tracing, enhancement

Milestone: 6.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Mar 15, 2021
@sywhang sywhang removed the untriaged New issue has not been triaged by the area owner label Mar 15, 2021
@benaadams
Copy link
Member

Once this is successful, we can look into expanding the set of EventSources in the BCL to use the generator.

For scope on other EventSources (from #43390 (comment))

For a standard ASP.NET Core site + ApplicationInsights (e.g. https://themesof.net/) it creates 27 EventSources on start-up.

With all the ones not in System.Private.CoreLib.dll doing reflection to get the Guid to use for the constructor (as that overload is not available outside SPCL):

image

@sywhang
Copy link
Contributor Author

sywhang commented Mar 16, 2021

Thanks for the additional info @benaadams and all the great feedback you've given us so far in the tracing space and the initial source generator work you did that I'm building on top of : )

I'm hopeful that we'll be able to get this into System.Private.CoreLib and BCL in .NET 6. I'm going to try my best to push this up the stack as much as time allows, but it may not be possible to push it up the stack before the .NET 6 deadline if other work takes over in the meantime. If that happens, I'll still push this through the next release version of .NET.

Just for context, the main challenge around the source generator work so far has been around figuring out the versioning + customer adoption story (i.e. can we ship it with Roslyn analyzers to assist developers to move towards source-generator friendly EventSources, should this ship as its own NuGet package or with the SDK, etc.), trying to come up with a reasonable public API that wouldn't break existing code written around EventSources (unfortunately self-describing EventSources are not exactly source-generator friendly with some methods allowing anonymous objects to be logged) as well as defining the set of APIs that would allow the minimal set of changes for one to move over to source-generated EventSource.

Many of these can go unanswered for BCL/System.Private.CoreLib EventSources, but to push it up the stack, some of these questions will need to be answered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Diagnostics.Tracing enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants