Updated SwqlStudio.csproj to use Serilog for logging #275
Conversation
|
This PR is just a request for comment. Switching up the logging like this makes it easier to swap out logging approaches in the future, and it would allow us to remove the SolarWinds.Logging abstraction, but it also means a larger MSI installer because of the many transitive dependencies involved. If this approach makes sense, we'd want to remove SolarWinds.Logging and log4net uses throughout the solution before merging anything. If not, we should just close this PR. |
| @@ -17,7 +18,7 @@ namespace SwqlStudio | |||
| UseSynchronizationContext = false)] | |||
| internal partial class MainForm : Form, IApplicationService | |||
| { | |||
| private static readonly SolarWinds.Logging.Log log = new SolarWinds.Logging.Log(); | |||
| private static readonly ILogger<MainForm> log = Program.LoggerFactory.CreateLogger<MainForm>(); | |||
There was a problem hiding this comment.
The absence of true dependency injection makes these initializations a bit uglier than they would otherwise be.
|
This seems reasonable to me. The extra DLLs are a mild nuisance, but not a big deal. |
OK, let me do a little work and turn this into a real PR, then. |
Updated SwqlStudio.csproj to eliminate the SolarWinds.Logging dependency (and a transitive dependency on log4net. It is configured instead to use Microsoft.Extensions.Logging and Serilog for logging. The log file path is no longer configurable via the App.config file in this approach. Updated the installer to add the new dependencies, including transitive dependencies. The SolarWinds.Logging and log4net assemblies are still deployed since SolarWinds.InformationService.Contract still depends on them. This is not ready for merging.
Removed SolarWinds.Logging references from SwisPowerShell. It was only referenced in the CustomCertificateValidator class for a single log entry, and logging was never configured.
Updated SolarWinds.InformationService.Contract to use Microsoft.Extensions.Logging instead of SolarWinds.Logging. Updated classes in SwqlStudio and SwisPowerShell accordingly. Note that SwisPowerShell uses null loggers after this change.
Removed SolarWinds.Logging.csproj from the solution.
Updated the installer, removing SolarWinds.Logging.dll and log4net.dll and adding the new dependencies.
danjagnow
force-pushed
the
feature/logging
branch
from
f730992
to
0b3117f
Compare
Updated SwqlStudio.csproj to eliminate the SolarWinds.Logging dependency (and a transitive dependency on log4net. It is configured instead to use Microsoft.Extensions.Logging and Serilog for logging. The log file path is no longer configurable via the App.config file in this approach. Updated the installer to add the new dependencies, including transitive dependencies. The SolarWinds.Logging and log4net assemblies are still deployed since SolarWinds.InformationService.Contract still depends on them. This is not ready for merging.