Skip to content

Commit

Permalink
Custom dump path for helix (#2525)
Browse files Browse the repository at this point in the history 
* Add VSTEST_DUMP_PATH env var

* Add VSTEST_DUMP_PATH variable for shortcircuting dump location and upload

Use NetCore client to dump net5.0

* Fix typo

* Add context
  • Loading branch information
@nohwnd
nohwnd authored Aug 19, 2020
1 parent b59b50b commit 193ed30
Show file tree
Hide file tree
Showing 19 changed files with 104 additions and 86 deletions.
5 changes: 4 additions & 1 deletion src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionAttachmentManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ public List<AttachmentSet> GetAttachments(DataCollectionContext dataCollectionCo
{
try
{
Task.WhenAll(this.attachmentTasks[dataCollectionContext].ToArray()).Wait();
if (this.attachmentTasks.TryGetValue(dataCollectionContext, out var tasks))
{
Task.WhenAll(tasks.ToArray()).Wait();
}
}
catch (Exception ex)
{
Expand Down
131 changes: 75 additions & 56 deletions src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public class BlameCollector : DataCollector, ITestExecutionEnvironmentSpecifier
private bool dumpWasCollectedByHangDumper;
private string targetFramework;
private List<KeyValuePair<string, string>> environmentVariables = new List<KeyValuePair<string, string>>();
private bool uploadDumpFiles;
private string tempDirectory;

/// <summary>
/// Initializes a new instance of the <see cref="BlameCollector"/> class.
Expand Down Expand Up @@ -147,10 +149,7 @@ public override void Initialize(
this.environmentVariables.Add(new KeyValuePair<string, string>("COMPlus_DbgMiniDumpType", this.processFullDumpEnabled ? "2" : "1"));
}

var guid = Guid.NewGuid().ToString();

var dumpDirectory = Path.Combine(Path.GetTempPath(), guid);
Directory.CreateDirectory(dumpDirectory);
var dumpDirectory = this.GetDumpDirectory();
var dumpPath = Path.Combine(dumpDirectory, $"%e_%p_%t_crashdump.dmp");
this.environmentVariables.Add(new KeyValuePair<string, string>("COMPlus_DbgMiniDumpName", dumpPath));
}
Expand Down Expand Up @@ -206,7 +205,8 @@ private void CollectDumpAndAbortTesthost()

try
{
this.processDumpUtility.StartHangBasedProcessDump(this.testHostProcessId, this.GetTempDirectory(), this.processFullDumpEnabled, this.targetFramework);
var dumpDirectory = this.GetDumpDirectory();
this.processDumpUtility.StartHangBasedProcessDump(this.testHostProcessId, dumpDirectory, this.processFullDumpEnabled, this.targetFramework);
}
catch (Exception ex)
{
Expand All @@ -220,35 +220,42 @@ private void CollectDumpAndAbortTesthost()
this.processDumpUtility.DetachFromTargetProcess(this.testHostProcessId);
}

try
if (this.uploadDumpFiles)
{
var dumpFiles = this.processDumpUtility.GetDumpFiles();
foreach (var dumpFile in dumpFiles)
try
{
try
var dumpFiles = this.processDumpUtility.GetDumpFiles();
foreach (var dumpFile in dumpFiles)
{
if (!string.IsNullOrEmpty(dumpFile))
try
{
this.dumpWasCollectedByHangDumper = true;
var fileTransferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true, this.fileHelper);
this.dataCollectionSink.SendFileAsync(fileTransferInformation);
if (!string.IsNullOrEmpty(dumpFile))
{
this.dumpWasCollectedByHangDumper = true;
var fileTransferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true, this.fileHelper);
this.dataCollectionSink.SendFileAsync(fileTransferInformation);
}
}
catch (Exception ex)
{
// Eat up any exception here and log it but proceed with killing the test host process.
EqtTrace.Error(ex);
}
}
catch (Exception ex)
{
// Eat up any exception here and log it but proceed with killing the test host process.
EqtTrace.Error(ex);
}

if (!dumpFiles.Any())
{
EqtTrace.Error("BlameCollector.CollectDumpAndAbortTesthost: blame:CollectDumpOnHang was enabled but dump file was not generated.");
if (!dumpFiles.Any())
{
EqtTrace.Error("BlameCollector.CollectDumpAndAbortTesthost: blame:CollectDumpOnHang was enabled but dump file was not generated.");
}
}
}
catch (Exception ex)
{
ConsoleOutput.Instance.Error(true, $"Blame: Collecting hang dump failed with error {ex}.");
}
}
catch (Exception ex)
else
{
ConsoleOutput.Instance.Error(true, $"Blame: Collecting hang dump failed with error {ex}.");
EqtTrace.Info("BlameCollector.CollectDumpAndAbortTesthost: Custom path to dump directory was provided via VSTEST_DUMP_PATH. Skipping attachment upload, the caller is responsible for collecting and uploading the dumps themselves.");
}

try
Expand Down Expand Up @@ -440,30 +447,37 @@ private void SessionEndedHandler(object sender, SessionEndEventArgs args)
// we won't dump the killed process again and that would just show a warning on the command line
if ((this.testStartCount > this.testEndCount || this.collectDumpAlways) && !this.dumpWasCollectedByHangDumper)
{
try
if (this.uploadDumpFiles)
{
var dumpFiles = this.processDumpUtility.GetDumpFiles();
foreach (var dumpFile in dumpFiles)
try
{
if (!string.IsNullOrEmpty(dumpFile))
var dumpFiles = this.processDumpUtility.GetDumpFiles();
foreach (var dumpFile in dumpFiles)
{
try
{
var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
this.dataCollectionSink.SendFileAsync(fileTranferInformation);
}
catch (FileNotFoundException ex)
if (!string.IsNullOrEmpty(dumpFile))
{
EqtTrace.Warning(ex.ToString());
this.logger.LogWarning(args.Context, ex.ToString());
try
{
var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
this.dataCollectionSink.SendFileAsync(fileTranferInformation);
}
catch (FileNotFoundException ex)
{
EqtTrace.Warning(ex.ToString());
this.logger.LogWarning(args.Context, ex.ToString());
}
}
}
}
catch (FileNotFoundException ex)
{
EqtTrace.Warning(ex.ToString());
this.logger.LogWarning(args.Context, ex.ToString());
}
}
catch (FileNotFoundException ex)
else
{
EqtTrace.Warning(ex.ToString());
this.logger.LogWarning(args.Context, ex.ToString());
EqtTrace.Info("BlameCollector.CollectDumpAndAbortTesthost: Custom path to dump directory was provided via VSTEST_DUMP_PATH. Skipping attachment upload, the caller is responsible for collecting and uploading the dumps themselves.");
}
}
}
Expand Down Expand Up @@ -497,7 +511,8 @@ private void TestHostLaunchedHandler(object sender, TestHostLaunchedEventArgs ar

try
{
this.processDumpUtility.StartTriggerBasedProcessDump(args.TestHostProcessId, this.GetTempDirectory(), this.processFullDumpEnabled, this.targetFramework);
var dumpDirectory = this.GetDumpDirectory();
this.processDumpUtility.StartTriggerBasedProcessDump(args.TestHostProcessId, dumpDirectory, this.processFullDumpEnabled, this.targetFramework);
}
catch (TestPlatformException e)
{
Expand Down Expand Up @@ -552,26 +567,30 @@ private void DeregisterEvents()

private string GetTempDirectory()
{
string tempPath = null;
var netDumperPath = this.environmentVariables.SingleOrDefault(p => p.Key == "COMPlus_DbgMiniDumpName").Value;

try
{
if (!string.IsNullOrWhiteSpace(netDumperPath))
{
tempPath = Path.GetDirectoryName(netDumperPath);
}
}
catch (ArgumentException)
if (string.IsNullOrWhiteSpace(this.tempDirectory))
{
// the path was not correct do nothing
this.tempDirectory = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
Directory.CreateDirectory(this.tempDirectory);
return this.tempDirectory;
}

var tmp = !string.IsNullOrWhiteSpace(tempPath) ? tempPath : Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());

Directory.CreateDirectory(tmp);
return this.tempDirectory;
}

return tmp;
private string GetDumpDirectory()
{
// Using a custom dump path for scenarios where we want to upload the
// dump files ourselves, such as when running in Helix.
// This will save into the directory specified via VSTEST_DUMP_PATH, and
// skip uploading dumps via attachments.
var dumpDirectoryOverride = Environment.GetEnvironmentVariable("VSTEST_DUMP_PATH");
var dumpDirectoryOverrideHasValue = !string.IsNullOrWhiteSpace(dumpDirectoryOverride);
this.uploadDumpFiles = !dumpDirectoryOverrideHasValue;

var dumpDirectory = dumpDirectoryOverrideHasValue ? dumpDirectoryOverride : this.GetTempDirectory();
Directory.CreateDirectory(dumpDirectory);
var dumpPath = Path.Combine(Path.GetFullPath(dumpDirectory));
return dumpPath;
}
}
}
20 changes: 8 additions & 12 deletions src/Microsoft.TestPlatform.Extensions.BlameDataCollector/CrashDumperFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,14 @@ public ICrashDumper Create(string targetFramework)

if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
EqtTrace.Info($"CrashDumperFactory: This is Windows, returning ProcDumpCrashDumper that uses ProcDump utility.");
return new ProcDumpCrashDumper();

// enable this once crashdump can trigger itself on exceptions that originate from task, then we can avoid using procdump
// if (!string.IsNullOrWhiteSpace(targetFramework) && !targetFramework.Contains("v5.0"))
// {
// EqtTrace.Info($"CrashDumperFactory: This is Windows on {targetFramework} which is not net5.0 or newer, returning ProcDumpCrashDumper that uses ProcDump utility.");
// return new ProcDumpCrashDumper();
// }

// EqtTrace.Info($"CrashDumperFactory: This is Windows on {targetFramework}, returning the .NETClient dumper which uses env variables to collect crashdumps of testhost and any child process.");
// return new NetClientCrashDumper();
if (!isNet50OrNewer)
{
EqtTrace.Info($"CrashDumperFactory: This is Windows on {targetFramework} which is not net5.0 or newer, returning ProcDumpCrashDumper that uses ProcDump utility.");
return new ProcDumpCrashDumper();
}

EqtTrace.Info($"CrashDumperFactory: This is Windows on {targetFramework}, returning the .NETClient dumper which uses env variables to collect crashdumps of testhost and any child process.");
return new NetClientCrashDumper();
}

if (isNet50OrNewer)
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/Resources.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/Resources.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@
<value>The specified inactivity time of {0} minute/s has elapsed. Collecting a dump and killing the test host process.</value>
</data>
<data name="NotGeneratingSequenceFile" xml:space="preserve">
<value>All tests finished running, Sequence file will not be generated.</value>
<comment>"Sequence" is the name of the file.</comment>
<value>All tests finished running, Sequence file will not be generated</value>
<comment>"Sequence" is the name of the file. No . at the end, because this is a blame message and the . will be added automatically.</comment>
</data>
<data name="ProcDumpCouldNotStart" xml:space="preserve">
<value>Could not start process dump: {0}</value>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.cs.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.de.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.es.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.fr.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.it.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ja.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ko.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pl.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pt-BR.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ru.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.tr.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
<note></note>
</trans-unit>
<trans-unit id="NotGeneratingSequenceFile">
<source>All tests finished running, Sequence file will not be generated.</source>
<source>All tests finished running, Sequence file will not be generated</source>
<target state="new">All tests finished running, Sequence file will not be generated.</target>
<note>"Sequence" is the name of the file.</note>
</trans-unit>
Expand Down
Loading

0 comments on commit 193ed30

Please sign in to comment.