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/8.0] Remove public provider from rundown session #92048

Merged
merged 10 commits into from
Sep 18, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 14, 2023

Backport of #91383 to release/8.0

/cc @davmason

Customer Impact

When we created EventPipe we mimicked the rundown logic from Perfview, which enabled the public Microsoft-Windows-DOTNETRuntime provider during rundown to support pre v4 runtimes. This has the unintended side effect of introducing unwanted events in to traces - e.g. if a user configured a trace without GC events we would have GC events emitted during rundown. This has the potential to confuse some analysis tools.

Testing

A CI run with a change that would failfast if we saw a non-rundown provider being fired on a rundown thread, plus manual verification with the dotnet-* tools.

Risk

Low - we could not identify any non rundown events that are emitted or any tools that expect those events to be emitted during rundown.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

@carlossanlop
Copy link
Member

@davmason please fill out the template, get a code owner sign-off, and add the servicing-consider label when ready to get reviewed & approved by @jeffschwMSFT.

@davmason davmason requested a review from a team September 14, 2023 22:24
@davmason davmason self-assigned this Sep 14, 2023
@davmason davmason requested a review from lateralusX September 14, 2023 22:25
@davmason davmason added this to the 8.0.0 milestone Sep 14, 2023
@carlossanlop carlossanlop added the Servicing-consider Issue for next servicing release review label Sep 15, 2023
@carlossanlop
Copy link
Member

@jeffschwMSFT can we get your autograph?

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. please get a code review. once ready this can be merged

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

@lateralusX can you please sign-off the backport too? Or alternatively @lambdageek @noahfalk @brianrob @MichalStrehovsky who were added as reviewers in the main PR.

Copy link
Member

@lateralusX lateralusX left a comment

Choose a reason for hiding this comment

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

LGTM!

@carlossanlop carlossanlop merged commit bcdff13 into release/8.0 Sep 18, 2023
@carlossanlop carlossanlop deleted the backport/pr-91383-to-release/8.0 branch September 18, 2023 17:09
@radical radical mentioned this pull request Sep 26, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 18, 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