Collect all Setup Logs in single file and add Configuration diagnostic logs #201
Conversation
florelis
requested review from
dhoehna,
EricJohnson327,
AmelBawa-msft and
bbonaby
| @@ -69,14 +73,36 @@ public async Task ApplyConfigurationAsync() | |||
| throw new InvalidOperationException(); | |||
| } | |||
|
|
|||
| _logger.ReportInfo("Starting to apply configuration set"); | |||
| Log.Logger?.ReportInfo(Log.Component.Configuration, "Starting to apply configuration set"); | |||
| _result = await _processor.ApplySetAsync(_configSet, ApplyConfigurationSetFlags.None); | |||
There was a problem hiding this comment.
Would this crash if you called ApplySetAsync at the top of the function then awaited the task later in the method? A lot of this code uses member variables and I don't know if calling functions out of order would throw something off.
There was a problem hiding this comment.
Yeah, it probably would. Changed to have this return an object with the result, instead of relying on accessing the other properties after the await. The only remaining dependency regarding order is that the Open method must be awaited before calling the Apply one.
| { | ||
| switch (diagnosticInformation.Level) | ||
| { | ||
| case DiagnosticLevel.Verbose: |
There was a problem hiding this comment.
Should this have a default just in case the enum is updated and the change wasn't reflected here?
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| _processor = new ConfigurationProcessor(factory); | ||
| factory.MinimumLevel = DiagnosticLevel.Verbose; |
There was a problem hiding this comment.
Duplicate factory.MinimumLevel = DiagnosticLevel.Verbose;
Thanks for adding the Diagnostics logs here 🥳
There was a problem hiding this comment.
Good catch. It should have been Processor.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Summary of the pull request
This changes the Setup Flow logs to create a single file, instead of multiple for each component - except that the elevated process still logs to its own file, but it is now in the same folder.
It also standardizes the component strings used for prefixing the log messages, and wires the Configuration diagnostic messages to write to the same log files.
References and relevant issues
Detailed description of the pull request / Additional comments
Validation steps performed
PR checklist