dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.24k stars 1.35k forks source link

[Proposal] Add adapter for `TaskLoggingHelper` and `ILogger` #8061

Open premun opened 2 years ago

premun commented 2 years ago

Context

A common setup that we have is that we have some business logic in some library. We want to then call into this logic from different entrypoints:

The usual way to approach this is to use Microsoft.Extensions.DependencyInjection to build the classes and Microsoft.Extensions.Logging.ILogger for generic logging. Then, in various environments, we inject the right logger:

The MSBuild environment uses the Microsoft.Build.Utilities.Task.Log member (TaskLoggingHelper class) but it is not possible to plug this well together with the ILogger interface that is quite native to .NET these days.

Goal

Ideally, similarly to the other options, we would have:

The adapter should ideally adhere to verbosity rules and similar setting.

Example

Something like this comes to mind:

public class CustomTask : Task
{
    private readonly IServiceProvider _serviceProvider;

    public VirtualMonoRepo_Initialize()
    {
        _serviceProvider = CreateServiceProvider();
    }

    public override bool Execute()
    {
        var myCustomLogic = _serviceProvider.GetRequiredService<IBusinessLogic>();
        return myCustomLogic.DoSomething();
    }

    private IServiceProvider CreateServiceProvider() =>
          new ServiceCollection()
                .AddBusinessLogic()  // this registers my app's classes into DI
                .AddTaskLogging(Log) // this registers current task's logger as ILogger
                .BuildServiceProvider();
}
KalleOlaviNiemitalo commented 2 years ago

Would the logger expect the state object to implement IReadOnlyCollection<KeyValuePair<string, object>>, and then recognise keys such as "Subcategory", "File", "LineNumber"?

premun commented 2 years ago

Honestly, I didn't think this 100% through as I don't know the specifics of the TaskLoggingHelper so I'd imagine this could a subject to further specification. I only know, I've tried a similar setup like this and had a bit hacky quick way to achieve this rather than a proper solution. I might clean up the code and share it here later if I have time.

premun commented 2 years ago

@matkoch I was wondering whether you have dealt with this situation?

KalleOlaviNiemitalo commented 2 years ago

I have implemented something related: logging interfaces and classes that can output to a SARIF 2.1.0 file or to MSBuild-compatible Console.Error. I don't think Microsoft.Extensions.Logging.ILogger would have been a good fit for that; it seems more suitable for logging in which the logger provider does not understand the semantics of the data structures and just needs to serialize them somewhere for later processing.

premun commented 2 years ago

I just found another .NET team that has implemented roughly the same as what I need/had in mind: https://github.com/dotnet/templating/blob/fedcb3c6e2eeafd13822f10ec4f0b8d0576c7af5/tools/Microsoft.TemplateEngine.Authoring.Tasks/Utilities/MSBuildLogger.cs

baronfel commented 1 year ago

When this is tackled, this blog is a great resource for how we should tackle the 'BeginScope' implementation.