Add crash report support to the CrashDump extension #8191
+581
−44
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 |
|---|---|---|
|
|
@@ -11,6 +11,14 @@ namespace Microsoft.Testing.Extensions.Diagnostics; | |
| internal sealed class CrashDumpCommandLineProvider : ICommandLineOptionsProvider | ||
| { | ||
| private static readonly string[] DumpTypeOptions = ["Mini", "Heap", "Triage", "Full"]; | ||
| private static readonly IReadOnlyCollection<CommandLineOption> CachedCommandLineOptions = | ||
| [ | ||
| new(CrashDumpCommandLineOptions.CrashDumpOptionName, CrashDumpResources.CrashDumpOptionDescription, ArgumentArity.Zero, false), | ||
| new(CrashDumpCommandLineOptions.CrashReportOptionName, CrashDumpResources.CrashReportOptionDescription, ArgumentArity.Zero, false), | ||
| new(CrashDumpCommandLineOptions.CrashReportOnlyOptionName, CrashDumpResources.CrashReportOnlyOptionDescription, ArgumentArity.Zero, false), | ||
| new(CrashDumpCommandLineOptions.CrashDumpFileNameOptionName, CrashDumpResources.CrashDumpFileNameOptionDescription, ArgumentArity.ExactlyOne, false), | ||
| new(CrashDumpCommandLineOptions.CrashDumpTypeOptionName, CrashDumpResources.CrashDumpTypeOptionDescription, ArgumentArity.ExactlyOne, false) | ||
| ]; | ||
|
|
||
| public string Uid => nameof(CrashDumpCommandLineProvider); | ||
|
|
||
|
|
@@ -22,13 +30,7 @@ internal sealed class CrashDumpCommandLineProvider : ICommandLineOptionsProvider | |
|
|
||
| public Task<bool> IsEnabledAsync() => Task.FromResult(true); | ||
|
|
||
| public IReadOnlyCollection<CommandLineOption> GetCommandLineOptions() => CachedCommandLineOptions; | ||
|
|
||
| public Task<ValidationResult> ValidateOptionArgumentsAsync(CommandLineOption commandOption, string[] arguments) | ||
| { | ||
|
|
@@ -45,5 +47,12 @@ public Task<ValidationResult> ValidateOptionArgumentsAsync(CommandLineOption com | |
| } | ||
|
|
||
| public Task<ValidationResult> ValidateCommandLineOptionsAsync(ICommandLineOptions commandLineOptions) | ||
| => commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName) && | ||
| !commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName) | ||
| ? ValidationResult.InvalidTask(CrashDumpResources.CrashReportRequiresCrashDumpErrorMessage) | ||
| : commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOnlyOptionName) && | ||
| (commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName) || | ||
| commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName)) | ||
| ? ValidationResult.InvalidTask(CrashDumpResources.CrashReportOnlyCannotBeCombinedErrorMessage) | ||
| : ValidationResult.ValidTask; | ||
|
Member
There was a problem hiding this comment. Done in 64d9b6f (and the prior d68feac). Beyond the requested refactor: the entire validation method is now a no-op because the CLI surface was simplified to a single --crash-report flag (see #8191 (comment) / the rename discussion in PR comments). The two mutually-exclusive rules and their error messages no longer exist. |
||
| } | ||
This file contains hidden or 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 |
|---|---|---|
|
|
@@ -18,6 +18,8 @@ internal sealed class CrashDumpEnvironmentVariableProvider : ITestHostEnvironmen | |
| private const string MiniDumpNameVariable = "DbgMiniDumpName"; | ||
| private const string CreateDumpDiagnosticsVariable = "CreateDumpDiagnostics"; | ||
| private const string CreateDumpVerboseDiagnosticsVariable = "CreateDumpVerboseDiagnostics"; | ||
| private const string EnableCrashReportVariable = "EnableCrashReport"; | ||
| private const string EnableCrashReportOnlyVariable = "EnableCrashReportOnly"; | ||
| private const string EnableMiniDumpValue = "1"; | ||
|
|
||
| private static readonly string[] Prefixes = ["DOTNET_", "COMPlus_"]; | ||
|
|
@@ -54,17 +56,48 @@ public CrashDumpEnvironmentVariableProvider( | |
|
|
||
| /// <inheritdoc /> | ||
| public Task<bool> IsEnabledAsync() | ||
| => Task.FromResult( | ||
| (_commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName) || | ||
| _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName) || | ||
| _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOnlyOptionName)) && | ||
| _crashDumpGeneratorConfiguration.Enable); | ||
|
|
||
| public Task UpdateAsync(IEnvironmentVariables environmentVariables) | ||
| { | ||
| bool crashDumpEnabled = _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName); | ||
| bool crashReportEnabled = _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName); | ||
| bool crashReportOnlyEnabled = _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOnlyOptionName); | ||
|
|
||
| if (crashDumpEnabled || crashReportOnlyEnabled) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| environmentVariables.SetVariable(new($"{prefix}{EnableMiniDumpVariable}", EnableMiniDumpValue, false, true)); | ||
| } | ||
|
Comment on lines
+69
to
+74
|
||
| } | ||
|
Member
|
||
|
|
||
| foreach (string prefix in Prefixes) | ||
| { | ||
| environmentVariables.SetVariable(new($"{prefix}{CreateDumpDiagnosticsVariable}", EnableMiniDumpValue, false, true)); | ||
| environmentVariables.SetVariable(new($"{prefix}{CreateDumpVerboseDiagnosticsVariable}", EnableMiniDumpValue, false, true)); | ||
| } | ||
|
|
||
| if (crashReportEnabled) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| environmentVariables.SetVariable(new($"{prefix}{EnableCrashReportVariable}", EnableMiniDumpValue, false, true)); | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. Done in d68feac. Renamed EnableMiniDumpValue to the neutral EnabledValue since it's reused by all three runtime variables. |
||
|
|
||
| if (crashReportOnlyEnabled) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| environmentVariables.SetVariable(new($"{prefix}{EnableCrashReportOnlyVariable}", EnableMiniDumpValue, false, true)); | ||
| } | ||
| } | ||
|
Comment on lines
+83
to
+92
Member
There was a problem hiding this comment. Done in e8b4818. Renamed |
||
|
|
||
| string miniDumpTypeValue = "4"; | ||
|
|
||
| if (_commandLineOptions.TryGetOptionArgumentList(CrashDumpCommandLineOptions.CrashDumpTypeOptionName, out string[]? dumpTypeString)) | ||
|
|
@@ -133,12 +166,16 @@ public Task<ValidationResult> ValidateTestHostEnvironmentVariablesAsync(IReadOnl | |
| return ValidationResult.InvalidTask(CrashDumpResources.CrashDumpNotSupportedInNonNetCoreErrorMessage); | ||
| #else | ||
| StringBuilder errors = new(); | ||
| if (_commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName) || | ||
| _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOnlyOptionName)) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| if (!environmentVariables.TryGetVariable($"{prefix}{EnableMiniDumpVariable}", out OwnedEnvironmentVariable? enableMiniDump) | ||
| || enableMiniDump.Value != EnableMiniDumpValue) | ||
| { | ||
| AddError(errors, $"{prefix}{EnableMiniDumpVariable}", EnableMiniDumpValue, enableMiniDump?.Value); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -160,6 +197,30 @@ public Task<ValidationResult> ValidateTestHostEnvironmentVariablesAsync(IReadOnl | |
| } | ||
| } | ||
|
|
||
| if (_commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName)) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| if (!environmentVariables.TryGetVariable($"{prefix}{EnableCrashReportVariable}", out OwnedEnvironmentVariable? enableCrashReport) | ||
| || enableCrashReport.Value != EnableMiniDumpValue) | ||
| { | ||
| AddError(errors, $"{prefix}{EnableCrashReportVariable}", EnableMiniDumpValue, enableCrashReport?.Value); | ||
|
Member
There was a problem hiding this comment. Done in d68feac. Extracted a ValidateBothPrefixes(string variableName, string expectedValue) local function and replaced all three duplicated blocks with calls to it. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| if (_commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOnlyOptionName)) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| if (!environmentVariables.TryGetVariable($"{prefix}{EnableCrashReportOnlyVariable}", out OwnedEnvironmentVariable? enableCrashReportOnly) | ||
| || enableCrashReportOnly.Value != EnableMiniDumpValue) | ||
| { | ||
| AddError(errors, $"{prefix}{EnableCrashReportOnlyVariable}", EnableMiniDumpValue, enableCrashReportOnly?.Value); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| foreach (string prefix in Prefixes) | ||
| { | ||
| if (!environmentVariables.TryGetVariable($"{prefix}{MiniDumpTypeVariable}", out OwnedEnvironmentVariable? miniDumpType)) | ||
|
|
||
This file contains hidden or 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 |
|---|---|---|
|
|
@@ -46,8 +46,11 @@ public CrashDumpProcessLifetimeHandler( | |
| public Type[] DataTypesProduced => [typeof(FileArtifact)]; | ||
|
|
||
| public Task<bool> IsEnabledAsync() | ||
| => Task.FromResult( | ||
| (_commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName) || | ||
| _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName) || | ||
| _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOnlyOptionName)) && | ||
| _netCoreCrashDumpGeneratorConfiguration.Enable); | ||
|
|
||
| public Task BeforeTestHostProcessStartAsync(CancellationToken _) => Task.CompletedTask; | ||
|
|
||
|
|
@@ -63,22 +66,51 @@ public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testH | |
| } | ||
|
|
||
| ApplicationStateGuard.Ensure(_netCoreCrashDumpGeneratorConfiguration.DumpFileNamePattern is not null); | ||
| bool generateDump = _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName); | ||
| bool generateCrashReport = _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName) || | ||
| _commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOnlyOptionName); | ||
|
|
||
| string processCrashedMessage = generateDump && generateCrashReport | ||
| ? string.Format(CultureInfo.InvariantCulture, CrashDumpResources.CrashDumpProcessCrashedDumpAndReportFileCreated, testHostProcessInformation.PID) | ||
| : generateCrashReport | ||
| ? string.Format(CultureInfo.InvariantCulture, CrashDumpResources.CrashDumpProcessCrashedReportFileCreated, testHostProcessInformation.PID) | ||
| : string.Format(CultureInfo.InvariantCulture, CrashDumpResources.CrashDumpProcessCrashedDumpFileCreated, testHostProcessInformation.PID); | ||
| await _outputDisplay.DisplayAsync(this, new ErrorMessageOutputDeviceData(processCrashedMessage), cancellationToken).ConfigureAwait(false); | ||
|
|
||
| // TODO: Crash dump supports more placeholders that we don't handle here. | ||
| // See "Dump name formatting" in: | ||
| // https://github.com/dotnet/runtime/blob/82742628310076fff22d7e7ee216a74384352056/docs/design/coreclr/botr/xplat-minidump-generation.md | ||
| string expectedDumpFile = _netCoreCrashDumpGeneratorConfiguration.DumpFileNamePattern.Replace("%p", testHostProcessInformation.PID.ToString(CultureInfo.InvariantCulture)); | ||
| if (generateDump) | ||
| { | ||
| if (File.Exists(expectedDumpFile)) | ||
| { | ||
| await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(expectedDumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); | ||
| } | ||
| else | ||
| { | ||
| await _outputDisplay.DisplayAsync(this, new ErrorMessageOutputDeviceData(string.Format(CultureInfo.InvariantCulture, CrashDumpResources.CannotFindExpectedCrashDumpFile, expectedDumpFile)), cancellationToken).ConfigureAwait(false); | ||
| foreach (string dumpFile in Directory.GetFiles(Path.GetDirectoryName(expectedDumpFile)!, "*.dmp")) | ||
| { | ||
| await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (generateCrashReport) | ||
| { | ||
| string expectedCrashReportFile = $"{expectedDumpFile}.crashreport.json"; | ||
|
Member
There was a problem hiding this comment. Done in d68feac. Extracted private const string CrashReportFileExtension = |
||
| if (File.Exists(expectedCrashReportFile)) | ||
| { | ||
| await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(expectedCrashReportFile), CrashDumpResources.CrashReportArtifactDisplayName, CrashDumpResources.CrashReportArtifactDescription)).ConfigureAwait(false); | ||
| } | ||
| else | ||
| { | ||
| await _outputDisplay.DisplayAsync(this, new ErrorMessageOutputDeviceData(string.Format(CultureInfo.InvariantCulture, CrashDumpResources.CannotFindExpectedCrashReportFile, expectedCrashReportFile)), cancellationToken).ConfigureAwait(false); | ||
| foreach (string crashReportFile in Directory.GetFiles(Path.GetDirectoryName(expectedCrashReportFile)!, "*.crashreport.json")) | ||
| { | ||
| await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(crashReportFile), CrashDumpResources.CrashReportArtifactDisplayName, CrashDumpResources.CrashReportArtifactDescription)).ConfigureAwait(false); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or 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 hidden or 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
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in e8b4818. Refactored into explicit if-statements (one per invalid combination) so future mutually-exclusive crash options are easy to add.