Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use MSBuild Logging APIs for Task output instead of StdOut #933

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add MSBuild logger
  • Loading branch information
sfoslund authored and baronfel committed Mar 1, 2025
commit 6f3cfc0319cbd9f15d24dfa92dc2ea89a23f093d
47 changes: 47 additions & 0 deletions src/Microsoft.Sbom.Extensions.DependencyInjection/MSBuildLogger.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.Sbom.Extensions.DependencyInjection;

using System;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
using Serilog;
using Serilog.Events;
using ILogger = Serilog.ILogger;

/// <summary>
/// A class to remap Errors logged from ComponentDetection assemblies to Warnings.
/// </summary>
public class MSBuildLogger : ILogger
{
private readonly TaskLoggingHelper loggingHelper;

public MSBuildLogger(TaskLoggingHelper loggingHelperToWrap)
{
loggingHelper = loggingHelperToWrap;
}

public void Write(LogEvent logEvent)
{
var logLevel = logEvent.Level;
switch (logLevel)
{
case LogEventLevel.Debug:
loggingHelper.LogMessage(MessageImportance.Low, logEvent.RenderMessage());
break;
case LogEventLevel.Information:
loggingHelper.LogMessage(MessageImportance.High, logEvent.RenderMessage());
break;
case LogEventLevel.Warning:
loggingHelper.LogWarning(logEvent.RenderMessage());
break;
case LogEventLevel.Error:
case LogEventLevel.Fatal:
loggingHelper.LogError(logEvent.RenderMessage());
break;
default:
break;
}
}
}
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@

<ItemGroup>
<PackageReference Include="AutoMapper.Extensions.Microsoft.DependencyInjection" />
<PackageReference Include="Microsoft.Build.Utilities.Core" />
<PackageReference Include="Microsoft.Extensions.Hosting" />
<PackageReference Include="Microsoft.Extensions.Http" />
<PackageReference Include="Scrutor" />
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Concurrent;
using Microsoft.Build.Utilities;
using Microsoft.ComponentDetection.Orchestrator;
using Microsoft.ComponentDetection.Orchestrator.Extensions;
using Microsoft.Extensions.DependencyInjection;
@@ -59,15 +60,15 @@ public static IServiceCollection AddSbomConfiguration(this IServiceCollection se
return services;
}

public static IServiceCollection AddSbomTool(this IServiceCollection services, LogEventLevel logLevel = LogEventLevel.Information)
public static IServiceCollection AddSbomTool(this IServiceCollection services, LogEventLevel logLevel = LogEventLevel.Information, TaskLoggingHelper? taskLoggingHelper = null)
{
services
.AddSingleton<IConfiguration, Configuration>()
.AddTransient(_ => FileSystemUtilsProvider.CreateInstance(CreateLogger(logLevel)))
.AddTransient(_ => FileSystemUtilsProvider.CreateInstance(CreateLogger(logLevel, taskLoggingHelper)))
.AddTransient(x =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that this will create a breaking change for API users, as they will now have to inject a logger similarly to how we are now doing it in Program.cs, is that right? This is okay with me as our next release will be a breaking change anyways, but tagging @DaveTryon for visibility

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, good call - I didn't realize that the dependency injection project was public API surface area. Let me noodle on how that could be fixed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a commit that minimizes the API surface area changes, and adds a ton of xmldoc that instructs users on the new requirement of registering their own ILogger. That won't stop someone that just updates and then runs tests from failing, but maybe that will help mitigate somewhat.

{
logLevel = x.GetService<InputConfiguration>()?.Verbosity?.Value ?? logLevel;
return Log.Logger = CreateLogger(logLevel);
return Log.Logger = CreateLogger(logLevel, taskLoggingHelper);
})
.AddTransient<IWorkflow<SbomParserBasedValidationWorkflow>, SbomParserBasedValidationWorkflow>()
.AddTransient<IWorkflow<SbomGenerationWorkflow>, SbomGenerationWorkflow>()
@@ -200,26 +201,33 @@ public static IServiceCollection ConfigureLoggingProviders(this IServiceCollecti
return services;
}

private static ILogger CreateLogger(LogEventLevel logLevel)
private static ILogger CreateLogger(LogEventLevel logLevel, TaskLoggingHelper? taskLoggingHelper = null)
{
return new RemapComponentDetectionErrorsToWarningsLogger(
new LoggerConfiguration()
.MinimumLevel.ControlledBy(new LoggingLevelSwitch { MinimumLevel = logLevel })
.Filter.ByExcluding(Matching.FromSource("System.Net.Http.HttpClient"))
.Enrich.With<LoggingEnricher>()
.Enrich.FromLogContext()
.WriteTo.Map(
LoggingEnricher.LogFilePathPropertyName,
(logFilePath, wt) => wt.Async(x => x.File($"{logFilePath}")),
1) // sinkMapCountLimit
.WriteTo.Map<bool>(
LoggingEnricher.PrintStderrPropertyName,
(printLogsToStderr, wt) => wt.Logger(lc => lc
.WriteTo.Console(outputTemplate: Constants.LoggerTemplate, standardErrorFromLevel: printLogsToStderr ? LogEventLevel.Debug : null)
if (taskLoggingHelper == null)
{
return new RemapComponentDetectionErrorsToWarningsLogger(
new LoggerConfiguration()
.MinimumLevel.ControlledBy(new LoggingLevelSwitch { MinimumLevel = logLevel })
.Filter.ByExcluding(Matching.FromSource("System.Net.Http.HttpClient"))
.Enrich.With<LoggingEnricher>()
.Enrich.FromLogContext()
.WriteTo.Map(
LoggingEnricher.LogFilePathPropertyName,
(logFilePath, wt) => wt.Async(x => x.File($"{logFilePath}")),
1) // sinkMapCountLimit
.WriteTo.Map<bool>(
LoggingEnricher.PrintStderrPropertyName,
(printLogsToStderr, wt) => wt.Logger(lc => lc
.WriteTo.Console(outputTemplate: Constants.LoggerTemplate, standardErrorFromLevel: printLogsToStderr ? LogEventLevel.Debug : null)

// Don't write the detection times table from DetectorProcessingService to the console, only the log file
.Filter.ByExcluding(Matching.WithProperty<string>("DetectionTimeLine", x => !string.IsNullOrEmpty(x)))),
1) // sinkMapCountLimit
.CreateLogger());
// Don't write the detection times table from DetectorProcessingService to the console, only the log file
.Filter.ByExcluding(Matching.WithProperty<string>("DetectionTimeLine", x => !string.IsNullOrEmpty(x)))),
1) // sinkMapCountLimit
.CreateLogger());
}
else
{
return new MSBuildLogger(taskLoggingHelper);
}
}
}
8 changes: 5 additions & 3 deletions src/Microsoft.Sbom.Targets/GenerateSbomTask.cs
Original file line number Diff line number Diff line change
@@ -19,8 +19,9 @@ namespace Microsoft.Sbom.Targets;
using Microsoft.Sbom.Contracts.Interfaces;
using Microsoft.Sbom.Extensions;
using Microsoft.Sbom.Extensions.DependencyInjection;
using Serilog.Events;
using SPDX22 = Microsoft.Sbom.Parsers.Spdx22SbomParser;
using SPDX30= Microsoft.Sbom.Parsers.Spdx30SbomParser;
using SPDX30 = Microsoft.Sbom.Parsers.Spdx30SbomParser;

/// <summary>
/// MSBuild task for generating SBOMs from build output.
@@ -34,10 +35,11 @@ public partial class GenerateSbom : Task
/// </summary>
public GenerateSbom()
{
var taskLoggingHelper = new TaskLoggingHelper(this);
var host = Host.CreateDefaultBuilder()
.ConfigureServices((host, services) =>
services
.AddSbomTool()
.AddSbomTool(LogEventLevel.Information, taskLoggingHelper)
/* Manually adding some dependencies since `AddSbomTool()` does not add them when
* running the MSBuild Task from another project.
*/
@@ -118,7 +120,7 @@ private IList<SbomSpecification> ValidateAndAssignSpecifications()
{
if (!string.IsNullOrWhiteSpace(this.ManifestInfo))
{
return [SbomSpecification.Parse(this.ManifestInfo)];
return [SbomSpecification.Parse(this.ManifestInfo)];
}

return null;
1 change: 1 addition & 0 deletions src/Microsoft.Sbom.Tool/Microsoft.Sbom.Tool.csproj
Original file line number Diff line number Diff line change
@@ -24,5 +24,6 @@
<ItemGroup>
<PackageReference Include="AutoMapper.Extensions.Microsoft.DependencyInjection" />
<PackageReference Include="Microsoft.Extensions.Hosting" />
<PackageReference Include="Microsoft.Build.Utilities.Core" />
</ItemGroup>
</Project>