KEMBAR78
Add Restore Instrumentation by brianrob · Pull Request #5650 · NuGet/NuGet.Client · GitHub
Skip to content

Add Restore Instrumentation #5650

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 29, 2024
Merged

Add Restore Instrumentation #5650

merged 3 commits into from
Feb 29, 2024

Conversation

@brianrob
Copy link
Contributor

@brianrob brianrob commented Feb 28, 2024 •
edited by jeffkl
Loading

Adds restore instrumentation via NuGetEventSource to enable performance analysis and comparisons on particular areas of restore.

Bug

Fixes: NuGet/Home#13274

Regression? Last working version:

Description

Adds ETW events for measuring performance during restore

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception - Not needed
    • OR
    • N/A

  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@brianrob brianrob requested a review from a team as a code owner February 28, 2024 00:52
@dotnet-policy-service dotnet-policy-service bot added the Community PRs created by someone not in the NuGet team label Feb 28, 2024
nkolev92
nkolev92 reviewed Feb 28, 2024
View reviewed changes
Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

I think there's a better name for 1 of the events.
The rest look great.

@jeffkl jeffkl self-assigned this Feb 28, 2024
jeffkl
jeffkl reviewed Feb 28, 2024
View reviewed changes
nkolev92
nkolev92 previously approved these changes Feb 28, 2024
View reviewed changes
jeffkl
jeffkl previously approved these changes Feb 28, 2024
View reviewed changes
Copy link
Contributor

@jeffkl jeffkl left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. When you have a chance, can you paste a screenshot showing some events and what they look like? I want everyone to be able visualize what this looks like in PerfView

@brianrob
Copy link
Contributor Author

Absolutely. Here's a screenshot of the events view in PerfView displaying these events:

image

@nkolev92
Copy link
Member

It seems like dotnet format is complaining about a whitespace change.

Not immediately obvious to me, but this should be easily reproducible: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=9160077&view=logs&j=05c93c9e-1e6d-54c9-bea4-701773852b98&t=ace55480-569f-557a-9ed3-d2ed255eda7f.

@brianrob brianrob dismissed stale reviews from jeffkl and nkolev92 via f3128f5 February 28, 2024 23:10
@brianrob
Copy link
Contributor Author

Thanks @nkolev92. I think I fixed it.

@jeffkl jeffkl enabled auto-merge (squash) February 29, 2024 16:15
@jeffkl jeffkl merged commit f9b48fa into NuGet:dev Feb 29, 2024
@brianrob brianrob deleted the dev-brianrob-perf-instrumentation branch February 29, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nkolev92 nkolev92 nkolev92 approved these changes

@jeffkl jeffkl jeffkl approved these changes

Assignees

@jeffkl jeffkl

Labels

Community PRs created by someone not in the NuGet team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add event tracing to restore to make it possible to measure performance

3 participants

@brianrob @nkolev92 @jeffkl