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

Remove "Checking compatibility..." log messages from RestoreTask #10383

Closed
KirillOsenkov opened this issue Dec 16, 2020 · 6 comments · Fixed by NuGet/NuGet.Client#5362
Closed
Assignees
Labels
Area:Logging Category:Quality Week Issues that should be considered for quality week Functionality:Restore Priority:2 Issues for the current backlog. Type:DCR Design Change Request
Milestone

Comments

@KirillOsenkov
Copy link

Currently RestoreTask logs a lot of messages that start with Checking compatibility:

image

The log output is full of these. I'm questioning the value of having these in the logs since they don't provide much useful information and generate a ton of noise.

I suggest that we just stop logging this particular message.

@KirillOsenkov
Copy link
Author

An average size binlog contains 89193 messages like this:
image

Another medium-size binlog I have has:
image

It significantly contributes to the bloat in the binlog, makes investigations slower and adds noise (because the search matches for all these assembly names are full of these useless results)

@KirillOsenkov
Copy link
Author

I guess in general the logging approach in this task follows the philosophy of "What am I currently doing?" instead of answering "Here's what you need to know".

image

When reading logs messages like Resolving conflicts for .NETFramework,Version=v4.7.2...
are of questionable value:

  1. They don't say which project is being restored, this is inferred from the context
  2. Which target framework is a useful piece of information, but it should be printed once at the beginning of restore for the project, and not duplicated over and over again (searching for this string returns too many junk results)
  3. The message in and of itself is not helpful. I only need to know if something goes wrong. If this is normal operation, I don't need to know.

@KirillOsenkov
Copy link
Author

I've highlighted the messages that are worth keeping, because they are useful:
image

Note how the rest is pretty much redundant and just slows down the builds and increases log sizes and causes noise when reading and searching through logs.

@KirillOsenkov
Copy link
Author

Again, highlighted the useful stuff:
image

KirillOsenkov added a commit to KirillOsenkov/MSBuildStructuredLog that referenced this issue Dec 16, 2020
KirillOsenkov added a commit to KirillOsenkov/MSBuildStructuredLog that referenced this issue Dec 31, 2020
@zivkan zivkan self-assigned this Apr 1, 2021
@zivkan zivkan added this to the Sprint 2021-04 milestone Apr 1, 2021
@zivkan zivkan added the Type:DCR Design Change Request label Apr 1, 2021
@zkat zkat modified the milestones: Sprint 2021-04, Sprint 2021-05 Apr 12, 2021
@zkat zkat assigned aortiz-msft and unassigned zivkan Apr 12, 2021
@zkat
Copy link
Contributor

zkat commented Apr 12, 2021

@zivkan we've moved this to customer sprint, and it sounds like Arturo is gonna try and take care of it himself :)

@KirillOsenkov
Copy link
Author

I'd be happy to help or code review if needed.

@nkolev92 nkolev92 added the Priority:2 Issues for the current backlog. label Dec 6, 2021
@nkolev92 nkolev92 added Category:Quality Week Issues that should be considered for quality week and removed Category:Customer Sprint labels Aug 29, 2022
@nkolev92 nkolev92 assigned Nigusu-Allehu and unassigned aortiz-msft Aug 8, 2023
@zivkan zivkan added this to the 6.8 milestone Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Logging Category:Quality Week Issues that should be considered for quality week Functionality:Restore Priority:2 Issues for the current backlog. Type:DCR Design Change Request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants