Tyrrrz / CliFx

Class-first framework for building command-line interfaces
MIT License
1.5k stars 61 forks source link

How to add logging to services #44

Closed sommmen closed 4 years ago

sommmen commented 4 years ago

Hi!

I like this library, it looks clean and well written. I only find the progress reporting a bit lacking because i'd like to add what im doing or processing when i report progress (e.g. 50% removing file: c:/somefolder/somefile.txt).

but to the core of my question;

I have SomeService which is injected to MyCmd, now i'd like to do some logging from out of my service. In asp.net core i inject a logger factory for this. However i'm uncertain how to go about this when using this fx. Any thoughts?

edit; playing around with trying to get IConsole from CliApplication into the container somehow.

Tyrrrz commented 4 years ago

Hi!

If you include Microsoft.Extensions.Logging you should be able to do the same thing. If you're also using Microsoft.Extensions.DependencyInjection as your container, you should be able to call AddLogging() on your service collection. Alternatively, you should be able to register all the required services manually.

Here's a StackOverflow answer explaining how to configure Microsoft.Extensions.Logging outside of ASP.NET Core (in this case with AutoFac as dependency container), it should be applicable to CliFx as well.

Here's another article.

sommmen commented 4 years ago

Hi!

If you include Microsoft.Extensions.Logging you should be able to do the same thing. If you're also using Microsoft.Extensions.DependencyInjection as your container, you should be able to call AddLogging() on your service collection. Alternatively, you should be able to register all the required services manually.

Here's a StackOverflow answer explaining how to configure Microsoft.Extensions.Logging outside of ASP.NET Core (in this case with AutoFac as dependency container), it should be applicable to CliFx as well.

Here's another article.

Hello and thanks for investing the time to answer me!

Yes - I have been using microsoft's solution before, although CliFx uses IConsole which it passes to the command; public async ValueTask ExecuteAsync(IConsole console)

Now if i use AddLogging() with the consoleLogger of microsoft then I feel like i might be intefering with this, consider for example writing to the console from several threads. I however have not done any testing on my end, i was just hoping to get some 'best practices' if anyone had them.

I thought about passing my own IConsole (the default SystemConsole the default builder makes) to CliApplication and then injecting this into a custom logger factory and then adding that to the service collection or something along those lines. Does that sound practical?

Also - not related to this question at all but 2 random thoughts that come into my mind;

Tyrrrz commented 4 years ago

If you use .AddConsole(), it's not in itself a catastrophe. It will bypass IConsole which means if you replace the SystemConsole with VirtualConsole in your tests, the logs will still go to the system console and not the virtual console. If you're okay with that, you can leave like that and then just inject a no-op logger or something in your tests.

The IConsole interface is there so that you can easily test your command's outputs without the need to allocate an actual terminal. The default implementation, SystemConsole, simply routes to the System.Console static class. Except the GetCancellationToken() method which is a bit more intricate than that.

However, if you want your logs to go to IConsole, it looks like you will have to inject a custom instance of Microsoft.Extensions.Logging.Console.IConsole. Yes, this is a different IConsole, this one is used by Microsoft.Extensions.Logging as an abstraction for their logging stuff. It has three methods so it should be easy:

public class CliFxLoggingConsole : Microsoft.Extensions.Logging.Console.IConsole
{
      private readonly IConsole _console; // CliFx's console
      public CliFxLoggingConsole(IConsole console) => _console = console;

      public void Write(string message, ConsoleColor? background, ConsoleColor? foreground)
      {
            if (background != null) _console.BackgroundColor = background.Value;
            if (foreground != null) _console.ForegroundColor = foreground.Value;
            _console.Output.Write(message);
            _console.ResetColor();
      }

      public void WriteLine(string message, ConsoleColor? background, ConsoleColor? foreground)
      {
            if (background != null) _console.BackgroundColor = background.Value;
            if (foreground != null) _console.ForegroundColor = foreground.Value;
            _console.Output.WriteLine(message);
            _console.ResetColor();
      }

     public void Flush()
     {
          // nothing to do
     }
}
Tyrrrz commented 4 years ago

Perhaps a cool feature would be structured logging like microsoft.extensions.logging can do

I'm still trying to figure out what's the best way to provide a good experience in regards to logging, so I'm collecting feedback.

This library seems well organized, and i'd love to learn from the architecture so perhaps one day when you have the time you could write a technical detailed blog or wiki post, i'd be reading it for sure!

Thanks! I will write one in the future.

sommmen commented 4 years ago

Welp, I think i know enough now and i will close this issue, thanks for your effort.

Tyrrrz commented 4 years ago

You're welcome :)