SBOM Task Outputs Directly to Console Instead of Using MSBuild Logging APIs

[JonDouglas]

When using the SBOM task in an MSBuild project, the task outputs messages directly to the console rather than utilizing MSBuild's Logging APIs. This behavior results in cluttered and unstructured console output, which is not consistent with standard MSBuild practices. Proper logging through MSBuild's APIs would allow for better categorization and filtering of messages.

Steps to Reproduce:

1. Create a .NET project and include the SBOM task.
2. Run the command dotnet pack -bl to build the project with a binary log.
3. Observe the console output during the build process.

Expected behavior:

Sbom task should output messages through MSBuild logging APIs & categorized appropriately.

We can utilize TaskLoggingHelper:

public class SbomTask : Microsoft.Build.Utilities.Task
{
    public override bool Execute()
    {
        // Example of logging an informational message
        Log.LogMessage(MessageImportance.High, "Finding components...");
        // Example of logging a warning
        Log.LogWarning("No instructions received to scan docker images.");
        // Example of logging an error
        Log.LogError("An error occurred during SBOM generation.");

        // Rest of the task implementation

        return !Log.HasLoggedErrors;
    }
}

[KalleOlaviNiemitalo]

For MSBuild on .NET Framework (MSBuild.exe /target:Pack or Visual Studio), Microsoft.Sbom.Targets implements the GenerateSbom task by starting Microsoft.Sbom.Tool.exe as a child process (but this doesn't currently work; https://github.com/microsoft/sbom-tool/issues/719). In this case, the task would need to read the output of the child process and forward it to the MSBuild logging API. So the .NET Framework case will need a separate implementation and separate testing. I suppose the minimal implementation would be similar to how the Exec task of MSBuild does it. I don't remember whether the diagnostic output of Microsoft.Sbom.Tool.exe already matches the MSBuild and Visual Studio format for diagnostic messages; if not, that could be implemented as a separate improvement, perhaps controlled by a command-line option.

[baronfel]

IMO this one is a blocker for including the targets in the .NET SDK - the output is not guaranteed at all so this Target will create spew all over the stdout. I think that making a bridge between Microsoft.Extensions.Logging.ILogger/Serilog logging and MSBuild's Logging infrastructure should be at least workable - though as @KalleOlaviNiemitalo the Warning/Error format would need checking.

I tried making such a bridge (we had to do similar for Containers ) but I think more would be needed because of the use of Serilog here.

[sfoslund]

@baronfel @KalleOlaviNiemitalo I assumed that you get MSBuild logging for free when using ToolTasks, is that not the case? I have fixed the issue that was blocking the .NET framework case and in my testing its logging output visually appears to be coming via the MSBuild APIs but I'm not entirely sure how to tell.

As for the non-.NET framework case, it seems like some additional investigation is required here to figure out how best to implement this. My first thought was to add an option to AddSbomTool (which is called in the task set up) to allow users to request a MSBuild logger instead of our normal serilog logger, but I haven't had time to look into this further and determine if that would actually work. If you think making a bridge between serilog and the MSbuild logger would work/ be simpler I'd love to hear any more information you have

[baronfel]

@sfoslund ToolTask does do the logging - the case I was talking about was the modern .NET case. I think your proposed path should be ok - you know the logging infra for your tool better than I do :) - and I don't have any specific requirements there aside from "don't write directly to stdout".

[sfoslund]

Sounds good, I have a branch where I seem to have things working when testing e2e but some tests are failing due to a missing MSBuild assembly and the task logging before being initialized. Unfortunately I am being pulled off this work so I likely won't be able to look further into this any time in the near future, but we will be happy to accept contributions if anyone wants to work off of these changes and prep a PR.

[baronfel]

I have a PR that fixes the failing tests (at least on my machine) here against @sfoslund's branch. If that looks good we can open the full PR to address this issue.