Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom dump path for helix #2525

Merged
4 commits merged into from
Aug 19, 2020
Merged

Custom dump path for helix #2525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3033c7f
Add VSTEST_DUMP_PATH env var
nohwnd Aug 19, 2020
691e1e9
Add VSTEST_DUMP_PATH variable for shortcircuting dump location and up…
nohwnd Aug 19, 2020
bb4540a
Fix typo
nohwnd Aug 19, 2020
b306502
Add context
nohwnd Aug 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionAttachmentManager.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop

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
Open in desktop
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>
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
<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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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