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

[Release/6.0] Don't emit manifest for NativeRuntimeEventSource (#78213) #79008

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

davmason
Copy link
Member

Port #78213 to 6.0

Customer Impact

When starting an ETW session on a machine with lots of .net processes every 6.0 and newer process will emit a very large manifest event for NativeRuntimeEventSource (~50kb per process). This causes an event storm and will overflow buffers leading to dropped events. See #77014 for a customer impacted.

Historically we have never emitted the manifest for the Microsoft-Windows-DotNETRuntime* providers, consumers need to hardcode each event, or use a library such as TraceEvent that parses it for you. In 6.0 we added the ability for managed BCL code to emit events from the Microsoft-Windows-DotNETRuntime provider by adding events to NativeRuntimeEventSource, previously it would not have any events declared and only existed to forward events to in process EventListeners. Making NativeRuntimeEventSource have events unintentionally caused us to start emitting the manifest.

Regression?

Yes

Testing

Manual testing that the manifest is not emitted with this change

Risk

Low

@davmason davmason added this to the 6.0.x milestone Nov 30, 2022
@davmason davmason requested review from jeffschwMSFT and a team November 30, 2022 00:01
@davmason davmason self-assigned this Nov 30, 2022
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved. we will take for consideration in 6.0.x

@carlossanlop carlossanlop added the Servicing-consider Issue for next servicing release review label Jan 4, 2023
@carlossanlop
Copy link
Member

approved. we will take for consideration in 6.0.x

@jeffschwMSFT based on your comment, I added the servicing-consider label.

@jeffschwMSFT
Copy link
Member

this was approved in person before the holidays, it was an oversight not to add the approval tag

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 5, 2023
@carlossanlop
Copy link
Member

Approved by Tactics (6.0.14).
Signed off by area owners.
No OOB changes needed.
CI failures are known/unrelated/already fixed: #79098
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 4810b47 into dotnet:release/6.0 Jan 5, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tracing-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants