diff --git a/src/Validation.Symbols/ITelemetryService.cs b/src/Validation.Symbols/ITelemetryService.cs index a9ed22b82..1b86ebf52 100644 --- a/src/Validation.Symbols/ITelemetryService.cs +++ b/src/Validation.Symbols/ITelemetryService.cs @@ -32,12 +32,21 @@ public interface ITelemetryService : ISubscriptionProcessorTelemetryService IDisposable TrackSymbolValidationDurationEvent(string packageId, string packageNormalizedVersion, int symbolCount); /// - /// Tracks the status of the validation. + /// Tracks the status of the validation per assembly. /// /// The package id. /// The package normalized version. /// The validation result. /// Information about the issue id failed or empty if passed.. - void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue); + /// The assembly name. + void TrackSymbolsAssemblyValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue, string assemblyName); + + /// + /// Tracks the status of the validation per package. + /// + /// The package id. + /// The package normalized version. + /// The validation result. + void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus); } } diff --git a/src/Validation.Symbols/SymbolsValidatorService.cs b/src/Validation.Symbols/SymbolsValidatorService.cs index b5c81ccbc..10ea953b7 100644 --- a/src/Validation.Symbols/SymbolsValidatorService.cs +++ b/src/Validation.Symbols/SymbolsValidatorService.cs @@ -21,7 +21,6 @@ public class SymbolsValidatorService : ISymbolsValidatorService { private static TimeSpan _cleanWorkingDirectoryTimeSpan = TimeSpan.FromSeconds(20); private static readonly string[] PEExtensionsPatterns = new string[] { "*.dll", "*.exe" }; - private static readonly string SymbolExtensionPattern = "*.pdb"; private static readonly string[] PEExtensions = new string[] { ".dll", ".exe" }; private static readonly string[] SymbolExtension = new string[] { ".pdb" }; @@ -62,9 +61,14 @@ public async Task ValidateSymbolsAsync(SymbolsValidatorMessag using (_telemetryService.TrackSymbolValidationDurationEvent(message.PackageId, message.PackageNormalizedVersion, pdbs.Count)) { - if (!SymbolsHaveMatchingPEFiles(pdbs, pes)) + List orphanSymbolFiles; + if (!SymbolsHaveMatchingPEFiles(pdbs, pes, out orphanSymbolFiles)) { - _telemetryService.TrackSymbolsValidationResultEvent(message.PackageId, message.PackageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound)); + orphanSymbolFiles.ForEach((symbol) => + { + _telemetryService.TrackSymbolsAssemblyValidationResultEvent(message.PackageId, message.PackageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound), assemblyName: symbol); + }); + _telemetryService.TrackSymbolsValidationResultEvent(message.PackageId, message.PackageNormalizedVersion, ValidationStatus.Failed); return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound); } var targetDirectory = Settings.GetWorkingDirectory(); @@ -151,79 +155,85 @@ public virtual IValidationResult ValidateSymbolMatching(string targetDirectory, { foreach (string peFile in Directory.GetFiles(targetDirectory, extension, SearchOption.AllDirectories)) { - using (var peStream = File.OpenRead(peFile)) - using (var peReader = new PEReader(peStream)) + IValidationResult validationResult; + if (!IsChecksumMatch(peFile, packageId, packageNormalizedVersion, out validationResult)) { - // This checks if portable PDB is associated with the PE file and opens it for reading. - // It also validates that it matches the PE file. - // It does not validate that the checksum matches, so we need to do that in the following block. - if (peReader.TryOpenAssociatedPortablePdb(peFile, File.OpenRead, out var pdbReaderProvider, out var pdbPath) && - // No need to validate embedded PDB (pdbPath == null for embedded) - pdbPath != null) - { - // Get all checksum entries. There can be more than one. At least one must match the PDB. - var checksumRecords = peReader.ReadDebugDirectory().Where(entry => entry.Type == DebugDirectoryEntryType.PdbChecksum) - .Select(e => peReader.ReadPdbChecksumDebugDirectoryData(e)) - .ToArray(); + _telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed); + return validationResult; + } + } + } + _telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Succeeded); + return ValidationResult.Succeeded; + } - if (checksumRecords.Length == 0) - { - _telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch)); - return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch); - } + private bool IsChecksumMatch(string peFilePath, string packageId, string packageNormalizedVersion, out IValidationResult validationResult) + { + validationResult = ValidationResult.Succeeded; + using (var peStream = File.OpenRead(peFilePath)) + using (var peReader = new PEReader(peStream)) + { + // This checks if portable PDB is associated with the PE file and opens it for reading. + // It also validates that it matches the PE file. + // It does not validate that the checksum matches, so we need to do that in the following block. + if (peReader.TryOpenAssociatedPortablePdb(peFilePath, File.OpenRead, out var pdbReaderProvider, out var pdbPath) && + // No need to validate embedded PDB (pdbPath == null for embedded) + pdbPath != null) + { + // Get all checksum entries. There can be more than one. At least one must match the PDB. + var checksumRecords = peReader.ReadDebugDirectory().Where(entry => entry.Type == DebugDirectoryEntryType.PdbChecksum) + .Select(e => peReader.ReadPdbChecksumDebugDirectoryData(e)) + .ToArray(); - var pdbBytes = File.ReadAllBytes(pdbPath); - var hashes = new Dictionary(); + if (checksumRecords.Length == 0) + { + _telemetryService.TrackSymbolsAssemblyValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch), assemblyName: Path.GetFileName(peFilePath)); + validationResult = ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch); + return false; + } - using (pdbReaderProvider) - { - var pdbReader = pdbReaderProvider.GetMetadataReader(); - int idOffset = pdbReader.DebugMetadataHeader.IdStartOffset; + var pdbBytes = File.ReadAllBytes(pdbPath); + var hashes = new Dictionary(); + + using (pdbReaderProvider) + { + var pdbReader = pdbReaderProvider.GetMetadataReader(); + int idOffset = pdbReader.DebugMetadataHeader.IdStartOffset; - foreach (var checksumRecord in checksumRecords) + foreach (var checksumRecord in checksumRecords) + { + if (!hashes.TryGetValue(checksumRecord.AlgorithmName, out var hash)) + { + HashAlgorithmName han = new HashAlgorithmName(checksumRecord.AlgorithmName); + using (var hashAlg = IncrementalHash.CreateHash(han)) { - if (!hashes.TryGetValue(checksumRecord.AlgorithmName, out var hash)) - { - HashAlgorithmName han = new HashAlgorithmName(checksumRecord.AlgorithmName); - using (var hashAlg = IncrementalHash.CreateHash(han)) - { - hashAlg.AppendData(pdbBytes, 0, idOffset); - hashAlg.AppendData(new byte[20]); - int offset = idOffset + 20; - int count = pdbBytes.Length - offset; - hashAlg.AppendData(pdbBytes, offset, count); - hash = hashAlg.GetHashAndReset(); - } - hashes.Add(checksumRecord.AlgorithmName, hash); - } - if (checksumRecord.Checksum.ToArray().SequenceEqual(hash)) - { - // found the right checksum - _telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Succeeded, ""); - return ValidationResult.Succeeded; - } + hashAlg.AppendData(pdbBytes, 0, idOffset); + hashAlg.AppendData(new byte[20]); + int offset = idOffset + 20; + int count = pdbBytes.Length - offset; + hashAlg.AppendData(pdbBytes, offset, count); + hash = hashAlg.GetHashAndReset(); } - - // Not found any checksum record that matches the PDB. - _telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch)); - return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch); + hashes.Add(checksumRecord.AlgorithmName, hash); + } + if (checksumRecord.Checksum.ToArray().SequenceEqual(hash)) + { + // found the right checksum + _telemetryService.TrackSymbolsAssemblyValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Succeeded, issue:"", assemblyName:Path.GetFileName(peFilePath)); + return true; } } + + // Not found any checksum record that matches the PDB. + _telemetryService.TrackSymbolsAssemblyValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch), assemblyName: Path.GetFileName(peFilePath)); + validationResult = ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch); + return false; } - _telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound)); - return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound); } } - // If did not return there were not any PE files to validate. In this case return error to not proceeed with an ingestion. - _logger.LogError("{ValidatorName}: There were not any dll or exe files found locally." + - "This could indicate an issue in the execution or the package was not correct created. PackageId {PackageId} PackageNormalizedVersion {PackageNormalizedVersion}. " + - "SymbolCount: {SymbolCount}", - ValidatorName.SymbolsValidator, - packageId, - packageNormalizedVersion, - Directory.GetFiles(targetDirectory, SymbolExtensionPattern, SearchOption.AllDirectories)); - _telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound)); - return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound); + _telemetryService.TrackSymbolsAssemblyValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound), assemblyName: Path.GetFileName(peFilePath)); + validationResult = ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound); + return false; } /// @@ -232,7 +242,7 @@ public virtual IValidationResult ValidateSymbolMatching(string targetDirectory, /// Symbol list extracted from the compressed folder. /// The list of PE files extracted from the compressed folder. /// - public static bool SymbolsHaveMatchingPEFiles(IEnumerable symbols, IEnumerable PEs) + public static bool SymbolsHaveMatchingPEFiles(IEnumerable symbols, IEnumerable PEs, out List orphanSymbolFiles) { if(symbols == null) { @@ -244,7 +254,9 @@ public static bool SymbolsHaveMatchingPEFiles(IEnumerable symbols, IEnum } var symbolsWithoutExtension = ZipArchiveService.RemoveExtension(symbols); var PEsWithoutExtensions = ZipArchiveService.RemoveExtension(PEs); - return !symbolsWithoutExtension.Except(PEsWithoutExtensions, StringComparer.OrdinalIgnoreCase).Any(); + orphanSymbolFiles = symbolsWithoutExtension.Except(PEsWithoutExtensions, StringComparer.OrdinalIgnoreCase).ToList(); + + return !orphanSymbolFiles.Any(); } } } diff --git a/src/Validation.Symbols/TelemetryService.cs b/src/Validation.Symbols/TelemetryService.cs index 93473d461..e99c17b9a 100644 --- a/src/Validation.Symbols/TelemetryService.cs +++ b/src/Validation.Symbols/TelemetryService.cs @@ -17,6 +17,7 @@ public class TelemetryService : ITelemetryService private const string MessageDeliveryLag = Prefix + "MessageDeliveryLag"; private const string MessageEnqueueLag = Prefix + "MessageEnqueueLag"; private const string SymbolValidationResult = Prefix + "SymbolValidationResult"; + private const string SymbolAssemblyValidationResult = Prefix + "SymbolAssemblyValidationResult"; private const string PackageId = "PackageId"; private const string PackageNormalizedVersion = "PackageNormalizedVersion"; @@ -24,6 +25,7 @@ public class TelemetryService : ITelemetryService private const string SymbolCount = "SymbolCount"; private const string ValidationResult = "ValidationResult"; private const string Issue = "Issue"; + private const string AssemblyName = "AssemblyName"; private readonly ITelemetryClient _telemetryClient; @@ -68,16 +70,30 @@ public IDisposable TrackSymbolValidationDurationEvent(string packageId, string p }); } - public void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue) + public void TrackSymbolsAssemblyValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue, string assemblyName) { _telemetryClient.TrackMetric( - SymbolValidationResult, + SymbolAssemblyValidationResult, 1, new Dictionary { { ValidationResult, validationStatus.ToString() }, { Issue, issue }, { PackageId, packageId }, + { PackageNormalizedVersion, packageNormalizedVersion }, + { AssemblyName, assemblyName } + }); + } + + public void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus) + { + _telemetryClient.TrackMetric( + SymbolValidationResult, + 1, + new Dictionary + { + { ValidationResult, validationStatus.ToString() }, + { PackageId, packageId }, { PackageNormalizedVersion, packageNormalizedVersion } }); } diff --git a/tests/Validation.Symbols.Tests/SymbolsValidatorServiceTests.cs b/tests/Validation.Symbols.Tests/SymbolsValidatorServiceTests.cs index 4bb07f459..d345bf92d 100644 --- a/tests/Validation.Symbols.Tests/SymbolsValidatorServiceTests.cs +++ b/tests/Validation.Symbols.Tests/SymbolsValidatorServiceTests.cs @@ -128,7 +128,8 @@ public void SymbolsHaveMatchingPEFilesReturnsTrueWhenPdbsMatchPEFiles() string[] pes = new string[] { @"lib\math\foo.dll", @"lib\math\bar.exe", @"lib\math2\bar2.exe" }; // Act - var result = SymbolsValidatorService.SymbolsHaveMatchingPEFiles(symbols, pes); + List orphanSymbols; + var result = SymbolsValidatorService.SymbolsHaveMatchingPEFiles(symbols, pes, out orphanSymbols); // Assert Assert.True(result); @@ -142,7 +143,8 @@ public void SymbolsHaveMatchingPEFilesReturnsFalseWhenPdbsDoNotMatchPEFiles() string[] pes = new string[] { @"lib\math\foo.dll", @"lib\math\bar.exe", @"lib\math2\bar2.exe" }; // Act - var result = SymbolsValidatorService.SymbolsHaveMatchingPEFiles(symbols, pes); + List orphanSymbols; + var result = SymbolsValidatorService.SymbolsHaveMatchingPEFiles(symbols, pes, out orphanSymbols); // Assert Assert.False(result); @@ -156,8 +158,9 @@ public void SymbolsHaveMatchingNullCheck() string[] pes = new string[] { @"lib\math\foo.dll", @"lib\math\bar.exe", @"lib\math2\bar2.exe" }; // Act + Assert - Assert.Throws(()=>SymbolsValidatorService.SymbolsHaveMatchingPEFiles(null, pes)); - Assert.Throws(() => SymbolsValidatorService.SymbolsHaveMatchingPEFiles(symbols, null)); + List orphanSymbols; + Assert.Throws(()=>SymbolsValidatorService.SymbolsHaveMatchingPEFiles(null, pes, out orphanSymbols)); + Assert.Throws(() => SymbolsValidatorService.SymbolsHaveMatchingPEFiles(symbols, null, out orphanSymbols)); } }