added RoslynLogger to project system tool.#44
added RoslynLogger to project system tool.#44panopticoncentral merged 2 commits intodotnet:masterfrom
Conversation
29a71dc to
cf97e04
Compare
|
@panopticoncentral can you take a look? @dotnet/roslyn-ide |
cf97e04 to
fcffa51
Compare
fcffa51 to
b49cbba
Compare
|
ping @panopticoncentral ? |
|
Can you explain a little more what you're trying to do here? When would someone record this log? And how would it be used? |
|
I am logging Roslyn activities around live/build errors and error list. the log will let us know what caused live or build analysis to happen, what errors actually reached error list and what actually shown to users. what live/build deduplication happened. what options (full solution analysis, semantic errors off and etc) were on when analysis happened and etc. ... people having these problems (https://devdiv.visualstudio.com/DevDiv/_workitems/edit/364961) can create this log while they repro the issue, and give the log to us. it is nice to be in this tool since "semantic error off" can happen due to "design time build error", and we can ask users to use same tool to gather that information as well. ... E2E scenario is as follow
|
|
I don't think the build log window is the right place for this information to go. I'd prefer it to be something like an output window logger since that's where other loggers like this are directed. If you're concerned about performance if we leave it on all the time, then we can add an options page for the tool (was going to to do that sooner or later anyway) to turn the logging on/off. |
|
that's one idea too. tagging @jinujoseph @Pilchie I was thinking making this tool the one tool we give to users to gather any kind of logs about our team |
|
I agree that the extension is a good place to add helpful tools, I'm just saying that this logger doesn't fit with this particular window from a UI design perspective... |
|
one good thing about it being in this tool is that we can ask all related log at one place since the issue I am tracking can also happen due to design time build issues. otherwise, we need to explain to users, you go here to enable roslyn logger and then go there to grap logs and then install this tool to get design time build errors and etc. much simpler if we can just say install this tool, run it, repro, save all logs and send them to us. |
|
@panopticoncentral "this logger doesn't fit with this particular window from a UI design perspective..." if you let me know, how you want me to change UI, I can go and do it. |
I was thinking making this tool the one tool we give to users to gather any kind of logs about our teamI second that idea , it should be less direction for customers , we are open to idea of how to extend/skip it from the UI. |
| DesignTimeBuild = 0x2, | ||
| Evaluation = 0x4 | ||
| Evaluation = 0x4, | ||
| Live = 0x8, |
There was a problem hiding this comment.
Live [](start = 8, length = 4)
This should Roslyn since that's whats in the log.
| } | ||
|
|
||
| public void Close(string logPath) | ||
| public void PreserveLogfile(string logPath) |
There was a problem hiding this comment.
PreserveLogfile [](start = 20, length = 15)
Since copying the file is no longer contained within this class (since the Roslyn log doesn't use it), we should push this copy up into the callers and remove this method. Then there will just be SetLogPath.
| <value>Evaluation</value> | ||
| </data> | ||
| <data name="FilterBuildLives" xml:space="preserve"> | ||
| <value>Live</value> |
There was a problem hiding this comment.
Live [](start = 11, length = 4)
Roslyn
| <data name="ProjectTypeHeaderLabel" xml:space="preserve"> | ||
| <value>Type</value> | ||
| </data> | ||
| <data name="RoslynLoggerProjectName_SolutionWide" xml:space="preserve"> |
There was a problem hiding this comment.
RoslynLoggerProjectName_SolutionWide [](start = 14, length = 36)
Nice name instead of generated one?
| NotifyChange(); | ||
| } | ||
|
|
||
| public bool SupportRoslynLog => _roslynLogger.Supported; |
There was a problem hiding this comment.
SupportRoslynLog [](start = 20, length = 16)
Put with other properties. Should be "SupportsRoslynLogging"
|
If we address these issues, it's fine for the moment. |
|
@panopticoncentral addressed your feedback. |
|
@panopticoncentral thank you. I dont have access to the repo though. |
|
Thanks @panopticoncentral for review and suggestions |
|
@panopticoncentral thank you!. will this automatically updated on https://marketplace.visualstudio.com/items?itemName=VisualStudioProductTeam.ProjectSystemTools ? we would like to start to give this out to customer to capture logs. |
|
I have to do a build and push out an update, I will try and do that today or Monday. |
this adds "Live" build type.
and add this entry when logs are collected.
and create Roslyn activity logs
this require this change to work
dotnet/roslyn#24551
this code dynamically check whether VS has the above change, if it doesn't, it won't add "Live"