diff --git a/.vscode/launch.json b/.vscode/launch.json index e7af7be00..0b877dbe5 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -10,8 +10,8 @@ "request": "launch", "preLaunchTask": "build", // If you have changed target frameworks, make sure to update the program path. - "program": "${workspaceFolder}/src/Microsoft.ComponentDetection/bin/Debug/netcoreapp3.1/Microsoft.ComponentDetection.dll", - "args": ["scan", "--Debug", "--Verbosity", "Verbose", "--Output", "${workspaceFolder}/scan-output", "--SourceDirectory", "${workspaceFolder}"], + "program": "${workspaceFolder}/src/Microsoft.ComponentDetection/bin/Debug/net8.0/Microsoft.ComponentDetection.dll", + "args": ["scan", "--Debug", "--LogLevel", "Verbose", "--Output", "${workspaceFolder}/scan-output", "--SourceDirectory", "${workspaceFolder}", "--DebugTelemetry"], "cwd": "${workspaceFolder}/src/Microsoft.ComponentDetection", // For more information about the 'console' field, see https://aka.ms/VSCode-CS-LaunchJson-Console "console": "internalConsole", diff --git a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/BaseDetectionTelemetryRecord.cs b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/BaseDetectionTelemetryRecord.cs index be3402694..da489a632 100644 --- a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/BaseDetectionTelemetryRecord.cs +++ b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/BaseDetectionTelemetryRecord.cs @@ -1,19 +1,29 @@ namespace Microsoft.ComponentDetection.Common.Telemetry.Records; using System; +using System.Collections.Generic; using System.Diagnostics; +using System.IO; +using System.Text.Json.Serialization; using Microsoft.ComponentDetection.Common.Telemetry.Attributes; public abstract class BaseDetectionTelemetryRecord : IDetectionTelemetryRecord { + internal const int MaxNonDiagnosticLines = 10; + private readonly Stopwatch stopwatch = new Stopwatch(); private bool disposedValue; protected BaseDetectionTelemetryRecord() => this.stopwatch.Start(); + internal static bool DiagnosticEnabled { get; set; } = string.Equals(Environment.GetEnvironmentVariable("AGENT_DIAGNOSTIC"), "True", StringComparison.OrdinalIgnoreCase) || string.Equals(Environment.GetEnvironmentVariable("SYSTEM_DEBUG"), "True", StringComparison.OrdinalIgnoreCase); + public abstract string RecordName { get; } + [JsonIgnore] + public virtual bool IsDiagnostic { get; } + [Metric] public TimeSpan? ExecutionTime { get; protected set; } @@ -26,6 +36,31 @@ public void StopExecutionTimer() } } + protected static string? TruncateToMaxLines(string? text) + { + if (string.IsNullOrEmpty(text)) + { + return text; + } + + var lines = new List(); + using (var reader = new StringReader(text)) + { + string? line; + while ((line = reader.ReadLine()) != null) + { + if (lines.Count >= MaxNonDiagnosticLines) + { + return string.Join(Environment.NewLine, lines); + } + + lines.Add(line); + } + } + + return text; + } + public void Dispose() { this.Dispose(true); @@ -39,7 +74,10 @@ protected virtual void Dispose(bool disposing) if (disposing) { this.StopExecutionTimer(); - TelemetryRelay.Instance.PostTelemetryRecord(this); + if (!this.IsDiagnostic || DiagnosticEnabled) + { + TelemetryRelay.Instance.PostTelemetryRecord(this); + } } this.disposedValue = true; diff --git a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/CommandLineInvocationTelemetryRecord.cs b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/CommandLineInvocationTelemetryRecord.cs index c5730d20c..47adc6c52 100644 --- a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/CommandLineInvocationTelemetryRecord.cs +++ b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/CommandLineInvocationTelemetryRecord.cs @@ -20,7 +20,8 @@ internal class CommandLineInvocationTelemetryRecord : BaseDetectionTelemetryReco internal void Track(CommandLineExecutionResult result, string path, string parameters) { this.ExitCode = result.ExitCode; - this.StandardError = result.StdErr?.RemoveSensitiveInformation(); + var sanitizedError = result.StdErr?.RemoveSensitiveInformation(); + this.StandardError = DiagnosticEnabled ? sanitizedError : TruncateToMaxLines(sanitizedError); this.TrackCommon(path, parameters); } diff --git a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DependencyGraphTranslationRecord.cs b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DependencyGraphTranslationRecord.cs index 782dd3022..168f08973 100644 --- a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DependencyGraphTranslationRecord.cs +++ b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DependencyGraphTranslationRecord.cs @@ -4,6 +4,8 @@ namespace Microsoft.ComponentDetection.Common.Telemetry.Records; internal class DependencyGraphTranslationRecord : BaseDetectionTelemetryRecord { + public override bool IsDiagnostic => true; + public override string RecordName => "DependencyGraphTranslationRecord"; public string? DetectorId { get; set; } diff --git a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DetectedComponentScopeRecord.cs b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DetectedComponentScopeRecord.cs index 4ec418c07..8e78f5b54 100644 --- a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DetectedComponentScopeRecord.cs +++ b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DetectedComponentScopeRecord.cs @@ -4,6 +4,8 @@ namespace Microsoft.ComponentDetection.Common.Telemetry.Records; internal class DetectedComponentScopeRecord : BaseDetectionTelemetryRecord { + public override bool IsDiagnostic => true; + public override string RecordName => "ComponentScopeRecord"; public int? MavenProvidedScopeCount { get; set; } = 0; diff --git a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DetectorExecutionTelemetryRecord.cs b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DetectorExecutionTelemetryRecord.cs index d2e1f47b4..45e815f03 100644 --- a/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DetectorExecutionTelemetryRecord.cs +++ b/src/Microsoft.ComponentDetection.Common/Telemetry/Records/DetectorExecutionTelemetryRecord.cs @@ -2,6 +2,8 @@ namespace Microsoft.ComponentDetection.Common.Telemetry.Records; internal class DetectorExecutionTelemetryRecord : BaseDetectionTelemetryRecord { + private string? experimentalInformation; + public override string RecordName => "DetectorExecution"; public string? DetectorId { get; set; } @@ -14,7 +16,11 @@ internal class DetectorExecutionTelemetryRecord : BaseDetectionTelemetryRecord public bool IsExperimental { get; set; } - public string? ExperimentalInformation { get; set; } + public string? ExperimentalInformation + { + get => this.experimentalInformation; + set => this.experimentalInformation = DiagnosticEnabled ? value : TruncateToMaxLines(value); + } public string? AdditionalTelemetryDetails { get; set; } } diff --git a/test/Microsoft.ComponentDetection.Common.Tests/BaseDetectionTelemetryRecordTests.cs b/test/Microsoft.ComponentDetection.Common.Tests/BaseDetectionTelemetryRecordTests.cs index d5635ad80..51187ef16 100644 --- a/test/Microsoft.ComponentDetection.Common.Tests/BaseDetectionTelemetryRecordTests.cs +++ b/test/Microsoft.ComponentDetection.Common.Tests/BaseDetectionTelemetryRecordTests.cs @@ -7,9 +7,13 @@ namespace Microsoft.ComponentDetection.Common.Tests; using System.Net; using System.Reflection; using System.Runtime.Serialization; +using System.Text.Json.Serialization; using AwesomeAssertions; +using Microsoft.ComponentDetection.Common.Telemetry; using Microsoft.ComponentDetection.Common.Telemetry.Records; +using Microsoft.ComponentDetection.Contracts; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; [TestClass] [TestCategory("Governance/All")] @@ -17,6 +21,9 @@ namespace Microsoft.ComponentDetection.Common.Tests; public class BaseDetectionTelemetryRecordTests { private Type[] recordTypes; + private bool originalDiagnosticEnabled; + private Mock telemetryServiceMock; + private List publishedRecords; [TestInitialize] public void Initialize() @@ -26,6 +33,23 @@ public void Initialize() .Where(type => typeof(BaseDetectionTelemetryRecord).IsAssignableFrom(type)) .Where(type => !type.IsAbstract) .ToArray(); + + this.originalDiagnosticEnabled = BaseDetectionTelemetryRecord.DiagnosticEnabled; + + this.publishedRecords = []; + this.telemetryServiceMock = new Mock(); + this.telemetryServiceMock + .Setup(x => x.PostRecord(It.IsAny())) + .Callback(this.publishedRecords.Add); + + TelemetryRelay.Instance.Init([this.telemetryServiceMock.Object]); + } + + [TestCleanup] + public void Cleanup() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = this.originalDiagnosticEnabled; + TelemetryRelay.Instance.Init([]); } [TestMethod] @@ -66,6 +90,11 @@ public void SerializableProperties() { foreach (var property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance)) { + if (property.GetCustomAttribute() != null) + { + continue; + } + if (!serializableTypes.Contains(property.PropertyType)) { Attribute.GetCustomAttribute(property.PropertyType, typeof(DataContractAttribute)).Should().NotBeNull( @@ -75,4 +104,175 @@ public void SerializableProperties() } } } + + [TestMethod] + public void IsDiagnostic_DefaultsToFalse() + { + new CommandLineInvocationTelemetryRecord().IsDiagnostic.Should().BeFalse(); + new DetectorExecutionTelemetryRecord().IsDiagnostic.Should().BeFalse(); + } + + [TestMethod] + public void IsDiagnostic_HasJsonIgnoreAttribute() + { + var property = typeof(BaseDetectionTelemetryRecord).GetProperty(nameof(BaseDetectionTelemetryRecord.IsDiagnostic)); + + property.GetCustomAttribute().Should().NotBeNull( + "IsDiagnostic should not be serialized into telemetry output"); + } + + [TestMethod] + public void NonDiagnosticRecord_IsPosted_WhenDiagnosticsDisabled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = false; + + using (new NonDiagnosticTestRecord()) + { + } + + this.publishedRecords.Should().ContainSingle("regular records are always published regardless of DiagnosticEnabled"); + } + + [TestMethod] + public void NonDiagnosticRecord_IsPosted_WhenDiagnosticsEnabled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = true; + + using (new NonDiagnosticTestRecord()) + { + } + + this.publishedRecords.Should().ContainSingle("regular records are always published regardless of DiagnosticEnabled"); + } + + [TestMethod] + public void DiagnosticRecord_NotPosted_WhenDiagnosticsDisabled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = false; + + using (new DiagnosticTestRecord()) + { + } + + this.publishedRecords.Should().BeEmpty("diagnostic records should not be published when DiagnosticEnabled=false"); + } + + [TestMethod] + public void DiagnosticRecord_IsPosted_WhenDiagnosticsEnabled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = true; + + using (new DiagnosticTestRecord()) + { + } + + this.publishedRecords.Should().ContainSingle("diagnostic records should be published when DiagnosticEnabled=true"); + } + + [TestMethod] + public void CommandLineInvocation_StandardError_TruncatedTo10Lines_WhenDiagnosticsDisabled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = false; + + var record = new CommandLineInvocationTelemetryRecord(); + record.Track(new CommandLineExecutionResult { ExitCode = 1, StdErr = this.BuildLines(50) }, "/cmd", string.Empty); + + record.StandardError.Split(Environment.NewLine).Should().HaveCount(10); + record.StandardError.Should().Contain("Line 1"); + record.StandardError.Should().Contain("Line 10"); + record.StandardError.Should().NotContain("Line 11"); + } + + [TestMethod] + public void CommandLineInvocation_StandardError_NotTruncated_WhenDiagnosticsEnabled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = true; + + var record = new CommandLineInvocationTelemetryRecord(); + record.Track(new CommandLineExecutionResult { ExitCode = 1, StdErr = this.BuildLines(50) }, "/cmd", string.Empty); + + record.StandardError.Split(Environment.NewLine).Should().HaveCount(50); + } + + [TestMethod] + public void CommandLineInvocation_StandardError_Unchanged_WhenUnder10Lines() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = false; + + var fiveLines = this.BuildLines(5); + var record = new CommandLineInvocationTelemetryRecord(); + record.Track(new CommandLineExecutionResult { ExitCode = 1, StdErr = fiveLines }, "/cmd", string.Empty); + + record.StandardError.Should().Be(fiveLines); + } + + [TestMethod] + public void CommandLineInvocation_StandardError_NullHandled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = false; + + var record = new CommandLineInvocationTelemetryRecord(); + record.Track(new CommandLineExecutionResult { ExitCode = 0, StdErr = null }, "/cmd", string.Empty); + + record.StandardError.Should().BeNull(); + } + + [TestMethod] + public void DetectorExecution_ExperimentalInformation_TruncatedTo10Lines_WhenDiagnosticsDisabled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = false; + + var record = new DetectorExecutionTelemetryRecord { ExperimentalInformation = this.BuildLines(50) }; + + record.ExperimentalInformation.Split(Environment.NewLine).Should().HaveCount(10); + record.ExperimentalInformation.Should().Contain("Line 1"); + record.ExperimentalInformation.Should().Contain("Line 10"); + record.ExperimentalInformation.Should().NotContain("Line 11"); + } + + [TestMethod] + public void DetectorExecution_ExperimentalInformation_NotTruncated_WhenDiagnosticsEnabled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = true; + + var record = new DetectorExecutionTelemetryRecord { ExperimentalInformation = this.BuildLines(50) }; + + record.ExperimentalInformation.Split(Environment.NewLine).Should().HaveCount(50); + } + + [TestMethod] + public void DetectorExecution_ExperimentalInformation_Unchanged_WhenUnder10Lines() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = false; + + var fiveLines = this.BuildLines(5); + var record = new DetectorExecutionTelemetryRecord { ExperimentalInformation = fiveLines }; + + record.ExperimentalInformation.Should().Be(fiveLines); + } + + [TestMethod] + public void DetectorExecution_ExperimentalInformation_NullHandled() + { + BaseDetectionTelemetryRecord.DiagnosticEnabled = false; + + var record = new DetectorExecutionTelemetryRecord { ExperimentalInformation = null }; + + record.ExperimentalInformation.Should().BeNull(); + } + + private string BuildLines(int count) => + string.Join(Environment.NewLine, Enumerable.Range(1, count).Select(i => $"Line {i}")); + + private sealed class NonDiagnosticTestRecord : BaseDetectionTelemetryRecord + { + public override string RecordName => "NonDiagnosticTestRecord"; + } + + private sealed class DiagnosticTestRecord : BaseDetectionTelemetryRecord + { + public override string RecordName => "DiagnosticTestRecord"; + + public override bool IsDiagnostic => true; + } }