-
Notifications
You must be signed in to change notification settings - Fork 142
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Remap errors to warnings if logged inside ComponentDetection (#554)
- Loading branch information
Showing
6 changed files
with
276 additions
and
17 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
62 changes: 62 additions & 0 deletions
62
...soft.Sbom.Extensions.DependencyInjection/RemapComponentDetectionErrorsToWarningsLogger.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // 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 Serilog; | ||
| using Serilog.Events; | ||
|
|
||
| /// <summary> | ||
| /// A class to remap Errors logged from ComponentDetection assemblies to Warnings. | ||
| /// </summary> | ||
| public class RemapComponentDetectionErrorsToWarningsLogger : ILogger | ||
| { | ||
| private readonly ILogger logger; | ||
| private readonly Func<string?> stackTraceProvider; | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="RemapComponentDetectionErrorsToWarningsLogger"/> class. | ||
| /// Production constructor. | ||
| /// </summary> | ||
| public RemapComponentDetectionErrorsToWarningsLogger(ILogger logger) | ||
| : this(logger, () => Environment.StackTrace) | ||
| { | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="RemapComponentDetectionErrorsToWarningsLogger"/> class. | ||
| /// Testable constructor. | ||
| /// </summary> | ||
| /// <exception cref="ArgumentNullException"></exception> | ||
| internal RemapComponentDetectionErrorsToWarningsLogger(ILogger logger, Func<string?> stackTraceProvider) | ||
| { | ||
| this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); | ||
| this.stackTraceProvider = stackTraceProvider ?? throw new ArgumentNullException(nameof(stackTraceProvider)); | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| /// <remarks>If ComponentDetection is on the stack, remap and log Errors to Warnings.</remarks> | ||
| public void Write(LogEvent logEvent) | ||
| { | ||
| // For performance reasons, bypass StackTrace work for non-errors. | ||
| if (logEvent.Level == LogEventLevel.Error) | ||
| { | ||
| var stackTrace = stackTraceProvider(); | ||
| if (stackTrace is not null && stackTrace.Contains("Microsoft.ComponentDetection.")) | ||
| { | ||
| var warningLogEvent = new LogEvent( | ||
| logEvent.Timestamp, | ||
| LogEventLevel.Warning, | ||
| logEvent.Exception, | ||
| logEvent.MessageTemplate, | ||
| logEvent.Properties.Select(kvp => new LogEventProperty(kvp.Key, kvp.Value))); | ||
|
|
||
| logger.Write(warningLogEvent); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| logger.Write(logEvent); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
22 changes: 22 additions & 0 deletions
22
...ions.DependencyInjection.Tests/Microsoft.Sbom.Extensions.DependencyInjection.Tests.csproj
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <IsPackable>false</IsPackable> | ||
| <SignAssembly>True</SignAssembly> | ||
| <RootNamespace>Microsoft.Sbom.DependencyInjection.Tests</RootNamespace> | ||
| <AssemblyOriginatorKeyFile>$(StrongNameSigningKeyFilePath)</AssemblyOriginatorKeyFile> | ||
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'"> | ||
| <DefineConstants>TRACE</DefineConstants> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Serilog.Sinks.Console" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\..\src\Microsoft.Sbom.Extensions.DependencyInjection\Microsoft.Sbom.Extensions.DependencyInjection.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> |
162 changes: 162 additions & 0 deletions
162
...xtensions.DependencyInjection.Tests/RemapComponentDetectionErrorsToWarningsLoggerTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| // Copyright (c) Microsoft. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
| using Moq; | ||
| using Serilog; | ||
| using Serilog.Events; | ||
| using Serilog.Parsing; | ||
|
|
||
| namespace Microsoft.Sbom.Extensions.DependencyInjection.Tests; | ||
|
|
||
| [TestClass] | ||
| public class RemapComponentDetectionErrorsToWarningsLoggerTests | ||
| { | ||
| private const string TestPropertyKey = "TestPropertyKey"; | ||
|
|
||
| private Mock<ILogger> loggerMock; | ||
| private Func<string> stackTraceProvider; | ||
| private ILogger testSubject; | ||
| private int testStackTraceCount; | ||
| private string? testStackTraceReturnValue; | ||
|
|
||
| // We need a concrete implementation of MessageTemplateToken | ||
| internal class TestMessageTemplateToken : MessageTemplateToken | ||
| { | ||
| public override int Length => throw new NotImplementedException(); | ||
|
|
||
| public override void Render(IReadOnlyDictionary<string, LogEventPropertyValue> properties, TextWriter output, IFormatProvider formatProvider = null) => throw new NotImplementedException(); | ||
| } | ||
|
|
||
| // We need a concrete implementation of LogEventPropertyValue | ||
| internal class TestLogEventPropertyValue : LogEventPropertyValue | ||
| { | ||
| public override void Render(TextWriter output, string format = null, IFormatProvider formatProvider = null) => throw new NotImplementedException(); | ||
| } | ||
|
|
||
| [TestInitialize] | ||
| public void TestInit() | ||
| { | ||
| loggerMock = new Mock<ILogger>(MockBehavior.Strict); | ||
| stackTraceProvider = TestGetStackTrace; | ||
| testStackTraceCount = 0; | ||
| testStackTraceReturnValue = null; | ||
| testSubject = new RemapComponentDetectionErrorsToWarningsLogger(loggerMock.Object, stackTraceProvider); | ||
| } | ||
|
|
||
| [TestCleanup] | ||
| public void TestCleanup() | ||
| { | ||
| loggerMock.VerifyAll(); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Constructor_LoggerIsNull_ThrowsNullArgumentException() | ||
| { | ||
| Assert.ThrowsException<ArgumentNullException>(() => new RemapComponentDetectionErrorsToWarningsLogger(null, stackTraceProvider)); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Constructor_StackTraceProviderIsNull_ThrowsNullArgumentException() | ||
| { | ||
| Assert.ThrowsException<ArgumentNullException>(() => new RemapComponentDetectionErrorsToWarningsLogger(loggerMock.Object, null)); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Write_LogEventLevelIsNotError_DoesNotCallStackTraceProvider_LogsEventAsSpecified() | ||
| { | ||
| var nonErrorLevels = Enum.GetValues(typeof(LogEventLevel)).Cast<LogEventLevel>().Where((x) => x != LogEventLevel.Error); | ||
| foreach (var level in nonErrorLevels) | ||
| { | ||
| var logEvent = GetLogEvent(level); | ||
| loggerMock.Setup(x => x.Write(logEvent)).Verifiable(); | ||
|
|
||
| testSubject.Write(logEvent); | ||
|
|
||
| Assert.AreEqual(0, testStackTraceCount); | ||
|
|
||
| // The VerifyAll and Reset calls are needed here because we need to confirm them for _each iteration_ of the loop. | ||
| loggerMock.VerifyAll(); | ||
| loggerMock.Reset(); | ||
| } | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Write_LogEventLevelIsError_StackTraceReturnsNull_LogsEventAsSpecified() | ||
| { | ||
| var logEvent = GetLogEvent(LogEventLevel.Error); | ||
| loggerMock.Setup(x => x.Write(logEvent)).Verifiable(); | ||
|
|
||
| testSubject.Write(logEvent); | ||
|
|
||
| Assert.AreEqual(1, testStackTraceCount); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Write_LogEventLevelIsError_UsesDefaultStackTraceProvider_LogsEventAsSpecified() | ||
| { | ||
| var logEvent = GetLogEvent(LogEventLevel.Error); | ||
| loggerMock.Setup(x => x.Write(logEvent)).Verifiable(); | ||
|
|
||
| // Use the production constructor here to force use of the default StackTraceProvider | ||
| new RemapComponentDetectionErrorsToWarningsLogger(loggerMock.Object).Write(logEvent); | ||
|
|
||
| Assert.AreEqual(0, testStackTraceCount); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Write_LogEventLevelIsError_StackTraceDoesNotContainComponentDetection_LogsEventAsSpecified() | ||
| { | ||
| var logEvent = GetLogEvent(LogEventLevel.Error); | ||
| loggerMock.Setup(x => x.Write(logEvent)).Verifiable(); | ||
| testStackTraceReturnValue = "at Microsoft.Sbom.Foo.Bar"; | ||
|
|
||
| testSubject.Write(logEvent); | ||
|
|
||
| Assert.AreEqual(1, testStackTraceCount); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Write_LogEventLevelIsError_StackTraceContainsComponentDetection_LogsEventAsWarning() | ||
| { | ||
| LogEvent actualEvent = null; | ||
| var logEvent = GetLogEvent(LogEventLevel.Error); | ||
| loggerMock.Setup(x => x.Write(It.IsAny<LogEvent>())).Callback<LogEvent>((l) => actualEvent = l).Verifiable(); | ||
| testStackTraceReturnValue = "at Microsoft.ComponentDetection.Foo.Bar"; | ||
|
|
||
| testSubject.Write(logEvent); | ||
|
|
||
| Assert.AreEqual(1, testStackTraceCount); | ||
|
|
||
| // Ensure that the LogEvent was correctly copied. Only the Level should be different | ||
| Assert.AreEqual(LogEventLevel.Warning, actualEvent.Level); | ||
| Assert.AreEqual(logEvent.Timestamp, actualEvent.Timestamp); | ||
| Assert.AreSame(logEvent.MessageTemplate, actualEvent.MessageTemplate); | ||
| Assert.AreEqual(logEvent.Properties.Count, actualEvent.Properties.Count); | ||
| Assert.AreEqual(1, actualEvent.Properties.Count); | ||
| Assert.IsInstanceOfType(logEvent.Properties[TestPropertyKey], typeof(TestLogEventPropertyValue)); | ||
| Assert.AreSame(logEvent.Properties[TestPropertyKey], actualEvent.Properties[TestPropertyKey]); | ||
| } | ||
|
|
||
| // A helper function to return a LogEvent with the specified LogEventLevel | ||
| private LogEvent GetLogEvent(LogEventLevel logEventLevel) | ||
| { | ||
| return new LogEvent( | ||
| DateTimeOffset.Now, | ||
| logEventLevel, | ||
| null, | ||
| new MessageTemplate(new List<MessageTemplateToken> { new TestMessageTemplateToken() }), | ||
| new LogEventProperty[1] { new LogEventProperty(TestPropertyKey, new TestLogEventPropertyValue()) }); | ||
| } | ||
|
|
||
| // A helper function to allow us to easily test the StackTraceProvider functionality | ||
| private string TestGetStackTrace() | ||
| { | ||
| testStackTraceCount++; | ||
| return testStackTraceReturnValue; | ||
| } | ||
| } |