Skip to content
This repository was archived by the owner on Jul 30, 2024. It is now read-only.

Commit 66d68d1

Browse files
cristinamanumcristinamanum
authored
FailValidationWithMixedSymbols (#656)
* Fix mixed(failing and passing) symbols validation. * Feedback - add better telemetry.
1 parent a9ea47d commit 66d68d1

4 files changed

Lines changed: 114 additions & 74 deletions

File tree

‎src/Validation.Symbols/ITelemetryService.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,21 @@ public interface ITelemetryService : ISubscriptionProcessorTelemetryService
3232
IDisposable TrackSymbolValidationDurationEvent(string packageId, string packageNormalizedVersion, int symbolCount);
3333

3434
/// <summary>
35-
/// Tracks the status of the validation.
35+
/// Tracks the status of the validation per assembly.
3636
/// </summary>
3737
/// <param name="packageId">The package id.</param>
3838
/// <param name="packageNormalizedVersion">The package normalized version.</param>
3939
/// <param name="validationStatus">The validation result.</param>
4040
/// <param name="issue">Information about the issue id failed or empty if passed..</param>
41-
void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue);
41+
/// <param name="assemblyName">The assembly name.</param>
42+
void TrackSymbolsAssemblyValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue, string assemblyName);
43+
44+
/// <summary>
45+
/// Tracks the status of the validation per package.
46+
/// </summary>
47+
/// <param name="packageId">The package id.</param>
48+
/// <param name="packageNormalizedVersion">The package normalized version.</param>
49+
/// <param name="validationStatus">The validation result.</param>
50+
void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus);
4251
}
4352
}

‎src/Validation.Symbols/SymbolsValidatorService.cs‎

Lines changed: 78 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ public class SymbolsValidatorService : ISymbolsValidatorService
2121
{
2222
private static TimeSpan _cleanWorkingDirectoryTimeSpan = TimeSpan.FromSeconds(20);
2323
private static readonly string[] PEExtensionsPatterns = new string[] { "*.dll", "*.exe" };
24-
private static readonly string SymbolExtensionPattern = "*.pdb";
2524
private static readonly string[] PEExtensions = new string[] { ".dll", ".exe" };
2625
private static readonly string[] SymbolExtension = new string[] { ".pdb" };
2726

@@ -62,9 +61,14 @@ public async Task<IValidationResult> ValidateSymbolsAsync(SymbolsValidatorMessag
6261

6362
using (_telemetryService.TrackSymbolValidationDurationEvent(message.PackageId, message.PackageNormalizedVersion, pdbs.Count))
6463
{
65-
if (!SymbolsHaveMatchingPEFiles(pdbs, pes))
64+
List<string> orphanSymbolFiles;
65+
if (!SymbolsHaveMatchingPEFiles(pdbs, pes, out orphanSymbolFiles))
6666
{
67-
_telemetryService.TrackSymbolsValidationResultEvent(message.PackageId, message.PackageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound));
67+
orphanSymbolFiles.ForEach((symbol) =>
68+
{
69+
_telemetryService.TrackSymbolsAssemblyValidationResultEvent(message.PackageId, message.PackageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound), assemblyName: symbol);
70+
});
71+
_telemetryService.TrackSymbolsValidationResultEvent(message.PackageId, message.PackageNormalizedVersion, ValidationStatus.Failed);
6872
return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound);
6973
}
7074
var targetDirectory = Settings.GetWorkingDirectory();
@@ -151,79 +155,85 @@ public virtual IValidationResult ValidateSymbolMatching(string targetDirectory,
151155
{
152156
foreach (string peFile in Directory.GetFiles(targetDirectory, extension, SearchOption.AllDirectories))
153157
{
154-
using (var peStream = File.OpenRead(peFile))
155-
using (var peReader = new PEReader(peStream))
158+
IValidationResult validationResult;
159+
if (!IsChecksumMatch(peFile, packageId, packageNormalizedVersion, out validationResult))
156160
{
157-
// This checks if portable PDB is associated with the PE file and opens it for reading.
158-
// It also validates that it matches the PE file.
159-
// It does not validate that the checksum matches, so we need to do that in the following block.
160-
if (peReader.TryOpenAssociatedPortablePdb(peFile, File.OpenRead, out var pdbReaderProvider, out var pdbPath) &&
161-
// No need to validate embedded PDB (pdbPath == null for embedded)
162-
pdbPath != null)
163-
{
164-
// Get all checksum entries. There can be more than one. At least one must match the PDB.
165-
var checksumRecords = peReader.ReadDebugDirectory().Where(entry => entry.Type == DebugDirectoryEntryType.PdbChecksum)
166-
.Select(e => peReader.ReadPdbChecksumDebugDirectoryData(e))
167-
.ToArray();
161+
_telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed);
162+
return validationResult;
163+
}
164+
}
165+
}
166+
_telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Succeeded);
167+
return ValidationResult.Succeeded;
168+
}
168169

169-
if (checksumRecords.Length == 0)
170-
{
171-
_telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch));
172-
return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch);
173-
}
170+
private bool IsChecksumMatch(string peFilePath, string packageId, string packageNormalizedVersion, out IValidationResult validationResult)
171+
{
172+
validationResult = ValidationResult.Succeeded;
173+
using (var peStream = File.OpenRead(peFilePath))
174+
using (var peReader = new PEReader(peStream))
175+
{
176+
// This checks if portable PDB is associated with the PE file and opens it for reading.
177+
// It also validates that it matches the PE file.
178+
// It does not validate that the checksum matches, so we need to do that in the following block.
179+
if (peReader.TryOpenAssociatedPortablePdb(peFilePath, File.OpenRead, out var pdbReaderProvider, out var pdbPath) &&
180+
// No need to validate embedded PDB (pdbPath == null for embedded)
181+
pdbPath != null)
182+
{
183+
// Get all checksum entries. There can be more than one. At least one must match the PDB.
184+
var checksumRecords = peReader.ReadDebugDirectory().Where(entry => entry.Type == DebugDirectoryEntryType.PdbChecksum)
185+
.Select(e => peReader.ReadPdbChecksumDebugDirectoryData(e))
186+
.ToArray();
174187

175-
var pdbBytes = File.ReadAllBytes(pdbPath);
176-
var hashes = new Dictionary<string, byte[]>();
188+
if (checksumRecords.Length == 0)
189+
{
190+
_telemetryService.TrackSymbolsAssemblyValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch), assemblyName: Path.GetFileName(peFilePath));
191+
validationResult = ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch);
192+
return false;
193+
}
177194

178-
using (pdbReaderProvider)
179-
{
180-
var pdbReader = pdbReaderProvider.GetMetadataReader();
181-
int idOffset = pdbReader.DebugMetadataHeader.IdStartOffset;
195+
var pdbBytes = File.ReadAllBytes(pdbPath);
196+
var hashes = new Dictionary<string, byte[]>();
197+
198+
using (pdbReaderProvider)
199+
{
200+
var pdbReader = pdbReaderProvider.GetMetadataReader();
201+
int idOffset = pdbReader.DebugMetadataHeader.IdStartOffset;
182202

183-
foreach (var checksumRecord in checksumRecords)
203+
foreach (var checksumRecord in checksumRecords)
204+
{
205+
if (!hashes.TryGetValue(checksumRecord.AlgorithmName, out var hash))
206+
{
207+
HashAlgorithmName han = new HashAlgorithmName(checksumRecord.AlgorithmName);
208+
using (var hashAlg = IncrementalHash.CreateHash(han))
184209
{
185-
if (!hashes.TryGetValue(checksumRecord.AlgorithmName, out var hash))
186-
{
187-
HashAlgorithmName han = new HashAlgorithmName(checksumRecord.AlgorithmName);
188-
using (var hashAlg = IncrementalHash.CreateHash(han))
189-
{
190-
hashAlg.AppendData(pdbBytes, 0, idOffset);
191-
hashAlg.AppendData(new byte[20]);
192-
int offset = idOffset + 20;
193-
int count = pdbBytes.Length - offset;
194-
hashAlg.AppendData(pdbBytes, offset, count);
195-
hash = hashAlg.GetHashAndReset();
196-
}
197-
hashes.Add(checksumRecord.AlgorithmName, hash);
198-
}
199-
if (checksumRecord.Checksum.ToArray().SequenceEqual(hash))
200-
{
201-
// found the right checksum
202-
_telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Succeeded, "");
203-
return ValidationResult.Succeeded;
204-
}
210+
hashAlg.AppendData(pdbBytes, 0, idOffset);
211+
hashAlg.AppendData(new byte[20]);
212+
int offset = idOffset + 20;
213+
int count = pdbBytes.Length - offset;
214+
hashAlg.AppendData(pdbBytes, offset, count);
215+
hash = hashAlg.GetHashAndReset();
205216
}
206-
207-
// Not found any checksum record that matches the PDB.
208-
_telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch));
209-
return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch);
217+
hashes.Add(checksumRecord.AlgorithmName, hash);
218+
}
219+
if (checksumRecord.Checksum.ToArray().SequenceEqual(hash))
220+
{
221+
// found the right checksum
222+
_telemetryService.TrackSymbolsAssemblyValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Succeeded, issue:"", assemblyName:Path.GetFileName(peFilePath));
223+
return true;
210224
}
211225
}
226+
227+
// Not found any checksum record that matches the PDB.
228+
_telemetryService.TrackSymbolsAssemblyValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch), assemblyName: Path.GetFileName(peFilePath));
229+
validationResult = ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_ChecksumDoesNotMatch);
230+
return false;
212231
}
213-
_telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound));
214-
return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound);
215232
}
216233
}
217-
// If did not return there were not any PE files to validate. In this case return error to not proceeed with an ingestion.
218-
_logger.LogError("{ValidatorName}: There were not any dll or exe files found locally." +
219-
"This could indicate an issue in the execution or the package was not correct created. PackageId {PackageId} PackageNormalizedVersion {PackageNormalizedVersion}. " +
220-
"SymbolCount: {SymbolCount}",
221-
ValidatorName.SymbolsValidator,
222-
packageId,
223-
packageNormalizedVersion,
224-
Directory.GetFiles(targetDirectory, SymbolExtensionPattern, SearchOption.AllDirectories));
225-
_telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound));
226-
return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound);
234+
_telemetryService.TrackSymbolsAssemblyValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound), assemblyName: Path.GetFileName(peFilePath));
235+
validationResult = ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound);
236+
return false;
227237
}
228238

229239
/// <summary>
@@ -232,7 +242,7 @@ public virtual IValidationResult ValidateSymbolMatching(string targetDirectory,
232242
/// <param name="symbols">Symbol list extracted from the compressed folder.</param>
233243
/// <param name="PEs">The list of PE files extracted from the compressed folder.</param>
234244
/// <returns></returns>
235-
public static bool SymbolsHaveMatchingPEFiles(IEnumerable<string> symbols, IEnumerable<string> PEs)
245+
public static bool SymbolsHaveMatchingPEFiles(IEnumerable<string> symbols, IEnumerable<string> PEs, out List<string> orphanSymbolFiles)
236246
{
237247
if(symbols == null)
238248
{
@@ -244,7 +254,9 @@ public static bool SymbolsHaveMatchingPEFiles(IEnumerable<string> symbols, IEnum
244254
}
245255
var symbolsWithoutExtension = ZipArchiveService.RemoveExtension(symbols);
246256
var PEsWithoutExtensions = ZipArchiveService.RemoveExtension(PEs);
247-
return !symbolsWithoutExtension.Except(PEsWithoutExtensions, StringComparer.OrdinalIgnoreCase).Any();
257+
orphanSymbolFiles = symbolsWithoutExtension.Except(PEsWithoutExtensions, StringComparer.OrdinalIgnoreCase).ToList();
258+
259+
return !orphanSymbolFiles.Any();
248260
}
249261
}
250262
}

‎src/Validation.Symbols/TelemetryService.cs‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,15 @@ public class TelemetryService : ITelemetryService
1717
private const string MessageDeliveryLag = Prefix + "MessageDeliveryLag";
1818
private const string MessageEnqueueLag = Prefix + "MessageEnqueueLag";
1919
private const string SymbolValidationResult = Prefix + "SymbolValidationResult";
20+
private const string SymbolAssemblyValidationResult = Prefix + "SymbolAssemblyValidationResult";
2021

2122
private const string PackageId = "PackageId";
2223
private const string PackageNormalizedVersion = "PackageNormalizedVersion";
2324
private const string MessageType = "MessageType";
2425
private const string SymbolCount = "SymbolCount";
2526
private const string ValidationResult = "ValidationResult";
2627
private const string Issue = "Issue";
28+
private const string AssemblyName = "AssemblyName";
2729

2830
private readonly ITelemetryClient _telemetryClient;
2931

@@ -68,16 +70,30 @@ public IDisposable TrackSymbolValidationDurationEvent(string packageId, string p
6870
});
6971
}
7072

71-
public void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue)
73+
public void TrackSymbolsAssemblyValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue, string assemblyName)
7274
{
7375
_telemetryClient.TrackMetric(
74-
SymbolValidationResult,
76+
SymbolAssemblyValidationResult,
7577
1,
7678
new Dictionary<string, string>
7779
{
7880
{ ValidationResult, validationStatus.ToString() },
7981
{ Issue, issue },
8082
{ PackageId, packageId },
83+
{ PackageNormalizedVersion, packageNormalizedVersion },
84+
{ AssemblyName, assemblyName }
85+
});
86+
}
87+
88+
public void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus)
89+
{
90+
_telemetryClient.TrackMetric(
91+
SymbolValidationResult,
92+
1,
93+
new Dictionary<string, string>
94+
{
95+
{ ValidationResult, validationStatus.ToString() },
96+
{ PackageId, packageId },
8197
{ PackageNormalizedVersion, packageNormalizedVersion }
8298
});
8399
}

0 commit comments

Comments
 (0)