From 3f42e99d8e3695c1a19db6a36446a8b433795dea Mon Sep 17 00:00:00 2001 From: Marek Aldorf Date: Mon, 29 Jul 2024 17:30:46 +0200 Subject: [PATCH 1/4] Adding ConsoleLogger to a compiler data logger outputs --- ReleaseHistory.md | 1 + src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ReleaseHistory.md b/ReleaseHistory.md index fb7b47bb..3f6122aa 100644 --- a/ReleaseHistory.md +++ b/ReleaseHistory.md @@ -16,6 +16,7 @@ - NEW => new feature ## UNRELEASED +* BUG: Fork telemetry to log always to Console and AppInsights in the same time when Error occur. [1002](https://github.com/microsoft/binskim/pull/1002) ## **v4.3.0** * DEP: Update `msdia140.dll` from 14.36.32532.0 to 14.40.33810.0. This update fixes the `System.AccessViolationException: Attempted to read or write protected memory` exception that occurs when reading certain PDB files. [996](https://github.com/microsoft/binskim/pull/996) diff --git a/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs b/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs index 725bfeb9..e460f9ec 100644 --- a/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs +++ b/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs @@ -48,6 +48,8 @@ private bool IsValidScanTarget(string file) public override BinaryAnalyzerContext InitializeGlobalContextFromOptions(AnalyzeOptions options, ref BinaryAnalyzerContext context) { + base.InitializeGlobalContextFromOptions(options, ref context); + if (this.Telemetry?.TelemetryClient != null) { var aggregatingLogger = new AggregatingLogger(); @@ -55,6 +57,7 @@ public override BinaryAnalyzerContext InitializeGlobalContextFromOptions(Analyze var ruleTelemetryLogger = new RuleTelemetryLogger(this.Telemetry.TelemetryClient); ruleTelemetryLogger.AnalysisStarted(); + aggregatingLogger.Loggers.Add(context.Logger); aggregatingLogger.Loggers.Add(ruleTelemetryLogger); context.Logger = aggregatingLogger; } @@ -65,7 +68,7 @@ public override BinaryAnalyzerContext InitializeGlobalContextFromOptions(Analyze ? options.MaxFileSizeInKilobytes.Value : long.MaxValue; - base.InitializeGlobalContextFromOptions(options, ref context); + // Update context object based on command-line parameters. context.SymbolPath = options.SymbolsPath ?? context.SymbolPath; From d8bc8cd48369220d9b66940935862d2d938f0654 Mon Sep 17 00:00:00 2001 From: Marek Aldorf Date: Tue, 6 Aug 2024 12:36:29 +0200 Subject: [PATCH 2/4] Add comment to clarify logger aggregation --- src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs b/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs index e460f9ec..6b429185 100644 --- a/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs +++ b/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs @@ -57,6 +57,7 @@ public override BinaryAnalyzerContext InitializeGlobalContextFromOptions(Analyze var ruleTelemetryLogger = new RuleTelemetryLogger(this.Telemetry.TelemetryClient); ruleTelemetryLogger.AnalysisStarted(); + // Combine rule telemetry with any other loggers that may be present. aggregatingLogger.Loggers.Add(context.Logger); aggregatingLogger.Loggers.Add(ruleTelemetryLogger); context.Logger = aggregatingLogger; From da3bd0992b7d3f7c4f0a05cb9e7f30ffb4405790 Mon Sep 17 00:00:00 2001 From: Marek Aldorf Date: Wed, 7 Aug 2024 13:28:54 +0200 Subject: [PATCH 3/4] Adding type check to avoid empty IAnalysisLogger in the first plase of context.Logger.Loggers --- src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs b/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs index 6b429185..99edb664 100644 --- a/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs +++ b/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs @@ -8,6 +8,8 @@ using System.Linq; using System.Reflection; +using CommandLine; + using Microsoft.CodeAnalysis.BinaryParsers; using Microsoft.CodeAnalysis.IL.Rules; using Microsoft.CodeAnalysis.IL.Sdk; @@ -52,13 +54,21 @@ public override BinaryAnalyzerContext InitializeGlobalContextFromOptions(Analyze if (this.Telemetry?.TelemetryClient != null) { + // Create an aggregating logger that will combine all loggers into a single logger. var aggregatingLogger = new AggregatingLogger(); + if (context.Logger is AggregatingLogger) + { + aggregatingLogger = context.Logger as AggregatingLogger; + } + else + { + aggregatingLogger.Loggers.Add(context.Logger); + } var ruleTelemetryLogger = new RuleTelemetryLogger(this.Telemetry.TelemetryClient); ruleTelemetryLogger.AnalysisStarted(); // Combine rule telemetry with any other loggers that may be present. - aggregatingLogger.Loggers.Add(context.Logger); aggregatingLogger.Loggers.Add(ruleTelemetryLogger); context.Logger = aggregatingLogger; } From a8a4d830c8fa759b844a0b1ac8faf86a0053323d Mon Sep 17 00:00:00 2001 From: Marek Aldorf Date: Fri, 9 Aug 2024 16:28:59 +0200 Subject: [PATCH 4/4] Add telemetry-related unit tests and using directives 1. Verify ConsoleLogger when telemetry is disabled. 2. Verify ConsoleLogger and RuleTelemetryLogger when telemetry is enabled. 3. Verify RuleTelemetryLogger when quiet option and telemetry are enabled. 4. Verify no loggers when both quiet option and telemetry are disabled. --- .../MultithreadedAnalyzeCommandTests.cs | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/src/Test.UnitTests.BinSkim.Driver/MultithreadedAnalyzeCommandTests.cs b/src/Test.UnitTests.BinSkim.Driver/MultithreadedAnalyzeCommandTests.cs index 46cb35b7..2fecd28b 100644 --- a/src/Test.UnitTests.BinSkim.Driver/MultithreadedAnalyzeCommandTests.cs +++ b/src/Test.UnitTests.BinSkim.Driver/MultithreadedAnalyzeCommandTests.cs @@ -2,11 +2,16 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Generic; using System.Text; using FluentAssertions; +using Microsoft.ApplicationInsights; +using Microsoft.ApplicationInsights.Extensibility; using Microsoft.CodeAnalysis.IL; +using Microsoft.CodeAnalysis.IL.Sdk; +using Microsoft.CodeAnalysis.Sarif.Writers; using Xunit; @@ -150,5 +155,89 @@ private static string[] Shuffle(string[] data) return data; } + + [Fact] + public void MultithreadedAnalyzeCommand_InitializeGlobalContextFromOptions_TelemetryNotSpecifiedHasOneConsoleLogger() + { + var options = new AnalyzeOptions(); + options.TargetFileSpecifiers = new List { "test.dll" }; + options.DisableTelemetry = true; + options.OutputFilePath = "test/path/"; + var context = new BinaryAnalyzerContext(); + + var command = new MultithreadedAnalyzeCommand(); + command.InitializeGlobalContextFromOptions(options, ref context); + + Assert.IsType(context.Logger); + + var aggregatingLogger = (Sarif.Driver.AggregatingLogger)context.Logger; + Assert.Contains(aggregatingLogger.Loggers, l => l is ConsoleLogger); + Assert.Equal(1, aggregatingLogger.Loggers.Count); + } + + [Fact] + public void MultithreadedAnalyzeCommand_InitializeGlobalContextFromOptions_TelemetrySpecifiedHasTwoLoggers() + { + var telemetryConfiguration = TelemetryConfiguration.CreateDefault(); + var telemetryClient = new TelemetryClient(telemetryConfiguration); + var telemetry = new Telemetry(telemetryConfiguration); + + var options = new AnalyzeOptions(); + options.TargetFileSpecifiers = new List { "test.dll" }; + options.OutputFilePath = "test/path/"; + var context = new BinaryAnalyzerContext(); + + var command = new MultithreadedAnalyzeCommand(telemetry); + command.InitializeGlobalContextFromOptions(options, ref context); + + Assert.IsType(context.Logger); + + var aggregatingLogger = (Sarif.Driver.AggregatingLogger)context.Logger; + Assert.Contains(aggregatingLogger.Loggers, l => l is ConsoleLogger); + Assert.Contains(aggregatingLogger.Loggers, l => l is RuleTelemetryLogger); + Assert.Equal(2, aggregatingLogger.Loggers.Count); + } + + [Fact] + public void MultithreadedAnalyzeCommand_InitializeGlobalContextFromOptions_QuietOptionSetHasOneRuleTelemetryLogger() + { + var telemetryConfiguration = TelemetryConfiguration.CreateDefault(); + var telemetryClient = new TelemetryClient(telemetryConfiguration); + var telemetry = new Telemetry(telemetryConfiguration); + + var options = new AnalyzeOptions(); + options.TargetFileSpecifiers = new List { "test.dll" }; + options.OutputFilePath = "test/path/"; + options.Quiet = true; + var context = new BinaryAnalyzerContext(); + + var command = new MultithreadedAnalyzeCommand(telemetry); + command.InitializeGlobalContextFromOptions(options, ref context); + + Assert.IsType(context.Logger); + + var aggregatingLogger = (Sarif.Driver.AggregatingLogger)context.Logger; + Assert.Contains(aggregatingLogger.Loggers, l => l is RuleTelemetryLogger); + Assert.Equal(1, aggregatingLogger.Loggers.Count); + } + + [Fact] + public void MultithreadedAnalyzeCommand_InitializeGlobalContextFromOptions_QuietOptionSetAndNoTelemetryHasOneConsoleLogger() + { + var options = new AnalyzeOptions(); + options.TargetFileSpecifiers = new List { "test.dll" }; + options.DisableTelemetry = true; + options.Quiet = true; + options.OutputFilePath = "test/path/"; + var context = new BinaryAnalyzerContext(); + + var command = new MultithreadedAnalyzeCommand(); + command.InitializeGlobalContextFromOptions(options, ref context); + + Assert.IsType(context.Logger); + + var aggregatingLogger = (Sarif.Driver.AggregatingLogger)context.Logger; + Assert.Equal(0, aggregatingLogger.Loggers.Count); + } } }