FileDownloader logs downloads without using available structured logging features

[PassivePicasso]

Squirrel version(s) 2.0.1

Description FileDownloader logs its attempts to download updates without using structured logging. This is setup using simple string concatenation rather than structured logging for the URL property. This limits the functionality for some environments which expect structured logging to be used.

Steps to recreate

Supply an implementation of IFullLogger that supports structured logging to a back end logging system.

var funcLogManager = new FuncLogManager((Type type) => new SerilogFullLogger(LauncherLog.ForContext(type)));
SquirrelLocator.CurrentMutable.RegisterConstant(funcLogManager, typeof(ILogManager));

Expected behavior Logs downloads using structured logging, splitting the url into a property referenced by the message template

Actual behavior Logs downloads using string concatenation

Additional information I'd open a PR for this issue but I don't know how to setup my development environment for this project. I could supply the change but would be unable to test as a result. If I'm missing documentation on how to setup a development environment for this project please direct me to it and I'll be happy to get setup and submit a PR for this change.

The offending line of code is here: https://github.com/Squirrel/Squirrel.Windows/blob/develop/src/Squirrel/FileDownloader.cs#L40

this.Log().Info("Downloading file: " + (failedUrl ?? url));

this could be modified to be

this.Log().Info("Downloading file: {0}",  (failedUrl ?? url));

[anaisbetts]

I don't think that Squirrel will support structured logging, sorry

[PassivePicasso]

It already supports it in a number of places?

This is a stand out case where it doesn't utilize it.

[PassivePicasso]

Most of the logging being done is passing logging using templates and properties, even in the update system. This is the only case I'm currently aware of where it doesn't

[PassivePicasso]

It looks like there are only 36 locations where the simple single string Info log call is made. Of those, 18 need to be modified to ensure that Squirrel as a whole uses structured logging.

I'm happy to put in the work here to bring everything in line with the structured logging provided by the Splat library I however cannot seem to get the project to compile and the requirements for getting Squirrel.Windows to build isn't clear. While most of the project seems to be setup correctly, I get errors about a few missing header files.

I hope you'll reconsidered closing this issue without discussion.

Edit: there are some more cases for Warn, and Exception logging calls, the Exception Logging calls look like they would be stuck to be this way. However, the logging calls could be easily updated.

None of the work to update this is complicated, most are some simple edits and will result in Squirrel.Windows having a much more robust logging design.

[caesay]

@PassivePicasso I would be happy to take on a PR in my fork for properly handling structured logging, if it's of any interest to you. Things have advanced/improved quite far in the linked fork compared to the state of things here at the moment. It will build out of the box on VS2022 and you're welcome to join our Discord if you'd like to discuss or have questions / any issues compiling.

[PassivePicasso]

@caesay I'll definitely take a look. At a glance, migrating to your fork would be a lot of work for me due to my gitlab build pipelines but the improvements are compelling other than the size increase of the update.exe and its inclusion in every update.

Regardless of that, I'll probably look at building the PR up in my spare time, which I don't have a lot of these days, since I can't justify putting this work in for something that creates a larger project.