From 1fd04e3f1735a36804136d3c666f081d537c3201 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Mon, 23 Aug 2021 13:33:54 -0700 Subject: [PATCH 01/22] Currently a complete mess; the execute action for completing a dump may be close to functional, but don't yet have tests to confirm that. --- .../HttpApi/ApiClient.cs | 2 +- .../Runners/ScenarioRunner.cs | 2 +- .../CollectDumpActionTests.cs | 65 +++++++++++++ .../EndpointInfoSourceTests.cs | 2 +- ...agnostics.Monitoring.Tool.UnitTests.csproj | 2 + .../Actions/CollectDumpAction.cs | 94 ++++++++++++++++++- 6 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs index f401f492407..330078d6cd8 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs @@ -79,7 +79,7 @@ public Task GetProcessAsync(int? pid, Guid? uid, string name, Cance return GetProcessAsync(GetProcessQuery(pid: pid, uid: uid, name: name), token); } - private async Task GetProcessAsync(string processQuery, CancellationToken token) + public async Task GetProcessAsync(string processQuery, CancellationToken token) { using HttpRequestMessage request = new(HttpMethod.Get, $"/process?" + processQuery); request.Headers.Add(HeaderNames.Accept, ContentTypes.ApplicationJson); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs index 51ee104ad7f..eff6c77d4ef 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs @@ -16,7 +16,7 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Runners { - internal static class ScenarioRunner + public static class ScenarioRunner { public static async Task SingleTarget( ITestOutputHelper outputHelper, diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs new file mode 100644 index 00000000000..eb648800554 --- /dev/null +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -0,0 +1,65 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; +using System.Threading.Tasks; +using Xunit; +using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Actions; +using System.Threading; +using System; +using System.IO; +using System.Globalization; +using Xunit.Abstractions; +using System.Net.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Fixtures; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests +{ + public sealed class CollectDumpActionTests + { + private const int TokenTimeoutMs = 60000; + //private const int DelayMs = 1000; + + private readonly IHttpClientFactory _httpClientFactory; + private readonly ITestOutputHelper _outputHelper; + private readonly IServiceProvider _serviceProvider; + // Not sure how to hook into Services for Singleton executor and logger, so currently just making one for the tests + private ILogger _logger = new Logger(new LoggerFactory()); + + public CollectDumpActionTests(ITestOutputHelper outputHelper, ServiceProviderFixture serviceProviderFixture) + { + _httpClientFactory = serviceProviderFixture.ServiceProvider.GetService(); + _outputHelper = outputHelper; + _serviceProvider = serviceProviderFixture.ServiceProvider; + } + + [Fact] + public async Task CollectDumpAction_NoEgressProvider() + { + CollectDumpAction action = new(_logger, _serviceProvider); + + CollectDumpOptions options = new(); + + options.Egress = null; + + using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); + + IEndpointInfo endpointInfo; // Need to figure out how to actually get the IEndpointInfo + + CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); + + string egressPath = result.OutputValues["EgressPath"]; + + if (!File.Exists(egressPath)) + { + throw new FileNotFoundException(string.Format(CultureInfo.InvariantCulture, Tools.Monitor.Strings.ErrorMessage_FileNotFound, egressPath)); + } + + //ValidateActionResult(result, "0"); + } + } +} diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs index ff08632e164..677de7db98c 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs @@ -243,7 +243,7 @@ private static void VerifyConnection(AppRunner runner, IEndpointInfo endpointInf Assert.NotNull(endpointInfo.Endpoint); } - private sealed class TestServerEndpointInfoSource : ServerEndpointInfoSource + internal sealed class TestServerEndpointInfoSource : ServerEndpointInfoSource { private readonly ITestOutputHelper _outputHelper; private readonly List>> _addedEndpointInfoSources = new(); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj index fd22da0c0c4..2519422e723 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj @@ -12,6 +12,8 @@ + + diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 5b9074e81e5..248a6e3ac84 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -2,20 +2,108 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using Microsoft.AspNetCore.Mvc; using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Diagnostics.Monitoring.WebApi.Controllers; +using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using System; +using System.Collections.Generic; +using System.IO; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; namespace Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Actions { - internal sealed class CollectDumpAction : + internal sealed class CollectDumpAction : ControllerBase, ICollectionRuleAction { - public Task ExecuteAsync(CollectDumpOptions options, IEndpointInfo endpointInfo, CancellationToken token) + public const string ArtifactType_Dump = "dump"; + + private readonly ILogger _logger; + private readonly IDiagnosticServices _diagnosticServices; + private readonly EgressOperationStore _operationsStore; + + public CollectDumpAction(ILogger logger, + IServiceProvider serviceProvider) + { + _logger = logger; + _diagnosticServices = serviceProvider.GetRequiredService(); + //_diagnosticPortOptions = serviceProvider.GetService>(); + _operationsStore = serviceProvider.GetRequiredService(); + } + + public async Task ExecuteAsync(CollectDumpOptions options, IEndpointInfo endpointInfo, CancellationToken token) + { + DumpType dumpType = options.Type.GetValueOrDefault(CollectDumpOptionsDefaults.Type); + string egressProvider = options.Egress; // Since egress provider shouldn't change during execution, do we need to check this? + + int pid = endpointInfo.ProcessId; + + ProcessKey? processKey = new ProcessKey(pid); + + IProcessInfo processInfo = await _diagnosticServices.GetProcessAsync(processKey, token); + + string dumpFileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? + FormattableString.Invariant($"dump_{GetFileNameTimeStampUtcNow()}.dmp") : + FormattableString.Invariant($"core_{GetFileNameTimeStampUtcNow()}"); + + string dumpFilePath = ""; + + if (string.IsNullOrEmpty(egressProvider)) + { + Stream dumpStream = await _diagnosticServices.GetDump(processInfo, dumpType, token); + + dumpFilePath = ((FileStream)dumpStream).Name; + + _logger.WrittenToHttpStream(); + //Compression is done automatically by the response + //Chunking is done because the result has no content-length + FileStreamResult FileStreamResult = File(dumpStream, ContentTypes.ApplicationOctetStream, dumpFileName); + } + else + { + KeyValueLogScope scope = new KeyValueLogScope(); + scope.AddArtifactType(ArtifactType_Dump); + scope.AddEndpointInfo(processInfo.EndpointInfo); + + dumpFilePath = await SendToEgress(new EgressOperation( + token => _diagnosticServices.GetDump(processInfo, dumpType, token), + egressProvider, + dumpFileName, + processInfo.EndpointInfo, + ContentTypes.ApplicationOctetStream, + scope), limitKey: ArtifactType_Dump); + } + + return new CollectionRuleActionResult() + { + OutputValues = new Dictionary(StringComparer.Ordinal) + { + { "EgressPath", dumpFilePath } + } + }; + } + + private static string GetFileNameTimeStampUtcNow() { - throw new NotImplementedException("TODO: Implement action"); + return DateTime.UtcNow.ToString("yyyyMMdd_HHmmss"); + } + + private async Task SendToEgress(EgressOperation egressStreamResult, string limitKey) + { + // Will throw TooManyRequestsException if there are too many concurrent operations. + Guid operationId = await _operationsStore.AddOperation(egressStreamResult, limitKey); + string newUrl = this.Url.Action( + action: nameof(OperationsController.GetOperationStatus), + controller: OperationsController.ControllerName, new { operationId = operationId }, + protocol: this.HttpContext.Request.Scheme, this.HttpContext.Request.Host.ToString()); + + return newUrl; + //return Accepted(newUrl); } } } From 0d7c109677728d449152b5bd50c065d543eea430 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Tue, 24 Aug 2021 09:20:16 -0700 Subject: [PATCH 02/22] In the process of trying to get tests up and running; mostly just figuring out what needs to be pulled from existing tests and how to combine it to make a test that can collect a dump using ExecuteAsync. --- .../Runners/ScenarioRunner.cs | 2 +- .../CollectDumpActionTests.cs | 107 +++++++++++++++++- ...agnostics.Monitoring.Tool.UnitTests.csproj | 1 - .../Actions/CollectDumpAction.cs | 6 +- 4 files changed, 107 insertions(+), 9 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs index eff6c77d4ef..51ee104ad7f 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs @@ -16,7 +16,7 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Runners { - public static class ScenarioRunner + internal static class ScenarioRunner { public static async Task SingleTarget( ITestOutputHelper outputHelper, diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index eb648800554..c468172b851 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -16,15 +16,24 @@ using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Fixtures; using Microsoft.Extensions.Logging; +using Microsoft.Diagnostics.Monitoring.TestCommon; +using System.Collections.Generic; +using static Microsoft.Diagnostics.Monitoring.Tool.UnitTests.EndpointInfoSourceTests; +using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; +using Microsoft.Diagnostics.Tools.Monitor; +using System.Reflection; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { public sealed class CollectDumpActionTests { - private const int TokenTimeoutMs = 60000; + private static readonly TimeSpan GetEndpointInfoTimeout = TimeSpan.FromSeconds(10); + + private const int TokenTimeoutMs = 60000; // Arbitrarily set to 1 minute -> potentially needs to be bigger...? //private const int DelayMs = 1000; - private readonly IHttpClientFactory _httpClientFactory; + + //private readonly IHttpClientFactory _httpClientFactory; private readonly ITestOutputHelper _outputHelper; private readonly IServiceProvider _serviceProvider; // Not sure how to hook into Services for Singleton executor and logger, so currently just making one for the tests @@ -32,7 +41,7 @@ public sealed class CollectDumpActionTests public CollectDumpActionTests(ITestOutputHelper outputHelper, ServiceProviderFixture serviceProviderFixture) { - _httpClientFactory = serviceProviderFixture.ServiceProvider.GetService(); + //_httpClientFactory = serviceProviderFixture.ServiceProvider.GetService(); _outputHelper = outputHelper; _serviceProvider = serviceProviderFixture.ServiceProvider; } @@ -48,7 +57,53 @@ public async Task CollectDumpAction_NoEgressProvider() using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); - IEndpointInfo endpointInfo; // Need to figure out how to actually get the IEndpointInfo + + /////////////////// + + await using var source = CreateServerSource(out string transportName); + source.Start(); + + var endpointInfos = await GetEndpointInfoAsync(source); + Assert.Empty(endpointInfos); + + AppRunner runner = CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60 + + Task newEndpointInfoTask = source.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); + + await runner.ExecuteAsync(async () => + { + await newEndpointInfoTask; + + endpointInfos = await GetEndpointInfoAsync(source); + + var endpointInfo = Assert.Single(endpointInfos); + Assert.NotNull(endpointInfo.CommandLine); + Assert.NotNull(endpointInfo.OperatingSystem); + Assert.NotNull(endpointInfo.ProcessArchitecture); + VerifyConnection(runner, endpointInfo); + + CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); + + string egressPath = result.OutputValues["EgressPath"]; + + if (!File.Exists(egressPath)) + { + throw new FileNotFoundException(string.Format(CultureInfo.InvariantCulture, Tools.Monitor.Strings.ErrorMessage_FileNotFound, egressPath)); + } + + await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); + }); + + await Task.Delay(TimeSpan.FromSeconds(1)); + + endpointInfos = await GetEndpointInfoAsync(source); + + Assert.Empty(endpointInfos); + + + //////////////// + /// + /* CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); @@ -60,6 +115,50 @@ public async Task CollectDumpAction_NoEgressProvider() } //ValidateActionResult(result, "0"); + */ + } + + + public static IEnumerable GetTfmsSupportingPortListener() + { + yield return new object[] { TargetFrameworkMoniker.Net50 }; + yield return new object[] { TargetFrameworkMoniker.Net60 }; + } + + private TestServerEndpointInfoSource CreateServerSource(out string transportName) + { + DiagnosticPortHelper.Generate(DiagnosticPortConnectionMode.Listen, out _, out transportName); + _outputHelper.WriteLine("Starting server endpoint info source at '" + transportName + "'."); + return new TestServerEndpointInfoSource(transportName, _outputHelper); + } + + private AppRunner CreateAppRunner(string transportName, TargetFrameworkMoniker tfm, int appId = 1) + { + AppRunner appRunner = new(_outputHelper, Assembly.GetExecutingAssembly(), appId, tfm); + appRunner.ConnectionMode = DiagnosticPortConnectionMode.Connect; + appRunner.DiagnosticPortPath = transportName; + appRunner.ScenarioName = TestAppScenarios.AsyncWait.Name; + return appRunner; + } + + private async Task> GetEndpointInfoAsync(ServerEndpointInfoSource source) + { + _outputHelper.WriteLine("Getting endpoint infos."); + using CancellationTokenSource cancellationSource = new(GetEndpointInfoTimeout); + return await source.GetEndpointInfoAsync(cancellationSource.Token); } + + /// + /// Verifies basic information on the connection and that it matches the target process from the runner. + /// + private static void VerifyConnection(AppRunner runner, IEndpointInfo endpointInfo) + { + Assert.NotNull(runner); + Assert.NotNull(endpointInfo); + Assert.Equal(runner.ProcessId, endpointInfo.ProcessId); + Assert.NotEqual(Guid.Empty, endpointInfo.RuntimeInstanceCookie); + Assert.NotNull(endpointInfo.Endpoint); + } + } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj index 2519422e723..f1c0d541b9f 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj @@ -12,7 +12,6 @@ - diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 248a6e3ac84..4f42dfbb48f 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -59,10 +59,10 @@ public async Task ExecuteAsync(CollectDumpOptions op dumpFilePath = ((FileStream)dumpStream).Name; - _logger.WrittenToHttpStream(); + _logger.WrittenToHttpStream(); // Do we need to do logging for trigger-related dumps? //Compression is done automatically by the response //Chunking is done because the result has no content-length - FileStreamResult FileStreamResult = File(dumpStream, ContentTypes.ApplicationOctetStream, dumpFileName); + FileStreamResult FileStreamResult = File(dumpStream, ContentTypes.ApplicationOctetStream, dumpFileName); // How and where do we use FileStreamResult? Do we need to call an execute method on the instance, or can we call File without keeping the value? } else { @@ -102,7 +102,7 @@ private async Task SendToEgress(EgressOperation egressStreamResult, stri controller: OperationsController.ControllerName, new { operationId = operationId }, protocol: this.HttpContext.Request.Scheme, this.HttpContext.Request.Host.ToString()); - return newUrl; + return newUrl; // Switched to returning the URL so that we can include it in the CollectionRuleActionResult //return Accepted(newUrl); } } From ff5cc4d5c92173032191a6ebf01db36ddaf35f06 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Thu, 26 Aug 2021 10:02:46 -0700 Subject: [PATCH 03/22] Dump Tests now successfully executes and outputs a file. Doesn't have the .dmp suffix and not entirely sure whether it's working correctly, but exciting to see it function. Code is currently a mess and needs to be refined and trimmed. --- .../CollectDumpActionTests.cs | 18 ++++++++++++++++-- .../Actions/CollectDumpAction.cs | 2 +- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index 8cd1b833a62..c1473bd7afa 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -23,6 +23,7 @@ using Microsoft.Diagnostics.Tools.Monitor; using System.Reflection; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Configuration; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { @@ -46,7 +47,14 @@ public CollectDumpActionTests(ITestOutputHelper outputHelper) internal void SetUpHost() { IHost host = new HostBuilder() - .ConfigureServices(services => + .ConfigureAppConfiguration((IConfigurationBuilder builder) => + { + builder.AddInMemoryCollection(new Dictionary + { + {ConfigurationPath.Combine(ConfigurationKeys.Storage, nameof(StorageOptions.DumpTempFolder)), StorageOptionsDefaults.DumpTempFolder } + }); + }) + .ConfigureServices((HostBuilderContext context, IServiceCollection services) => { services.AddSingleton(); services.AddSingleton(); @@ -59,12 +67,17 @@ internal void SetUpHost() services.AddSingleton(); services.ConfigureCollectionRules(); services.ConfigureEgress(); + services.ConfigureOperationStore(); + services.ConfigureMetrics(context.Configuration); + services.ConfigureStorage(context.Configuration); + services.ConfigureDefaultProcess(context.Configuration); + }) .Build(); _serviceProvider = host.Services; _logger = host.Services.GetRequiredService().CreateLogger(); - _outputHelper = host.Services.GetService(); + //_outputHelper = host.Services.GetService(); } @@ -76,6 +89,7 @@ public async Task CollectDumpAction_NoEgressProvider() CollectDumpOptions options = new(); options.Egress = null; + //options.Type = WebApi.Models.DumpType.Full; using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 4f42dfbb48f..5ebb9795031 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -39,7 +39,7 @@ public CollectDumpAction(ILogger logger, public async Task ExecuteAsync(CollectDumpOptions options, IEndpointInfo endpointInfo, CancellationToken token) { DumpType dumpType = options.Type.GetValueOrDefault(CollectDumpOptionsDefaults.Type); - string egressProvider = options.Egress; // Since egress provider shouldn't change during execution, do we need to check this? + string egressProvider = options.Egress; // Do we need to check this? int pid = endpointInfo.ProcessId; From ffc3f1b536844af3b80447e76cc60efe3750de9e Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Thu, 26 Aug 2021 16:00:51 -0700 Subject: [PATCH 04/22] Discovered that the file I was returning was the temporary one, and that the permanent one is not being written. Need to investigate how that actually happens. --- .../CollectDumpActionTests.cs | 112 +++++++++++++----- .../Actions/CollectDumpAction.cs | 4 +- 2 files changed, 88 insertions(+), 28 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index c1473bd7afa..fda66e68cd9 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -41,10 +41,9 @@ public sealed class CollectDumpActionTests public CollectDumpActionTests(ITestOutputHelper outputHelper) { _outputHelper = outputHelper; - SetUpHost(); } - internal void SetUpHost() + private void SetUpHost(string egressProvider) { IHost host = new HostBuilder() .ConfigureAppConfiguration((IConfigurationBuilder builder) => @@ -58,19 +57,19 @@ internal void SetUpHost() { services.AddSingleton(); services.AddSingleton(); - services.AddHostedService(); - services.AddSingleton(); - services.AddSingleton(); - services.AddHostedService(); services.AddSingleton(); - services.ConfigureCollectionRules(); - services.ConfigureEgress(); - services.ConfigureOperationStore(); - services.ConfigureMetrics(context.Configuration); services.ConfigureStorage(context.Configuration); - services.ConfigureDefaultProcess(context.Configuration); + + + //services.AddHostedService(); + //services.AddHostedService(); + //services.ConfigureCollectionRules(); + //services.ConfigureEgress(); + //services.ConfigureOperationStore(); + //services.ConfigureMetrics(context.Configuration); + //services.ConfigureDefaultProcess(context.Configuration); }) .Build(); @@ -78,12 +77,36 @@ internal void SetUpHost() _serviceProvider = host.Services; _logger = host.Services.GetRequiredService().CreateLogger(); //_outputHelper = host.Services.GetService(); + + + + + + + /* + const string ExpectedEgressProvider = "TmpEgressProvider"; + + return ValidateSuccess( + rootOptions => + { + rootOptions.CreateCollectionRule(DefaultRuleName) + .SetStartupTrigger() + .AddCollectDumpAction(ExpectedDumpType, ExpectedEgressProvider); + rootOptions.AddFileSystemEgress(ExpectedEgressProvider, "/tmp"); + }, + ruleOptions => + { + ruleOptions.VerifyCollectDumpAction(0, ExpectedDumpType, ExpectedEgressProvider); + }); + */ } [Fact] public async Task CollectDumpAction_NoEgressProvider() { + SetUpHost(null); + CollectDumpAction action = new(_logger, _serviceProvider); CollectDumpOptions options = new(); @@ -135,31 +158,66 @@ await runner.ExecuteAsync(async () => endpointInfos = await GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); + } + /* + [Fact] + public async Task CollectDumpAction_FileEgressProvider() + { + SetUpHost(); - //////////////// - /// - /* + CollectDumpAction action = new(_logger, _serviceProvider); + + CollectDumpOptions options = new(); + + options.Egress = null; + //options.Type = WebApi.Models.DumpType.Full; - CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); + using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); - string egressPath = result.OutputValues["EgressPath"]; + /////////////////// - if (!File.Exists(egressPath)) + ServerEndpointInfoCallback callback = new(_outputHelper); + await using var source = CreateServerSource(out string transportName, callback); + source.Start(); + + var endpointInfos = await GetEndpointInfoAsync(source); + Assert.Empty(endpointInfos); + + AppRunner runner = CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60 + + Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); + + await runner.ExecuteAsync(async () => { - throw new FileNotFoundException(string.Format(CultureInfo.InvariantCulture, Tools.Monitor.Strings.ErrorMessage_FileNotFound, egressPath)); - } + await newEndpointInfoTask; - //ValidateActionResult(result, "0"); - */ - } + endpointInfos = await GetEndpointInfoAsync(source); + var endpointInfo = Assert.Single(endpointInfos); + Assert.NotNull(endpointInfo.CommandLine); + Assert.NotNull(endpointInfo.OperatingSystem); + Assert.NotNull(endpointInfo.ProcessArchitecture); + VerifyConnection(runner, endpointInfo); - public static IEnumerable GetTfmsSupportingPortListener() - { - yield return new object[] { TargetFrameworkMoniker.Net50 }; - yield return new object[] { TargetFrameworkMoniker.Net60 }; - } + CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); + + string egressPath = result.OutputValues["EgressPath"]; + + if (!File.Exists(egressPath)) + { + throw new FileNotFoundException(string.Format(CultureInfo.InvariantCulture, Tools.Monitor.Strings.ErrorMessage_FileNotFound, egressPath)); + } + + await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); + }); + + await Task.Delay(TimeSpan.FromSeconds(1)); + + endpointInfos = await GetEndpointInfoAsync(source); + + Assert.Empty(endpointInfos); + }*/ private ServerEndpointInfoSource CreateServerSource(out string transportName, ServerEndpointInfoCallback callback = null) { diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 5ebb9795031..5e951974991 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -57,12 +57,14 @@ public async Task ExecuteAsync(CollectDumpOptions op { Stream dumpStream = await _diagnosticServices.GetDump(processInfo, dumpType, token); - dumpFilePath = ((FileStream)dumpStream).Name; + //dumpFilePath = ((FileStream)dumpStream).Name; // I believe this gives us the temporary file (and then it gets deleted) _logger.WrittenToHttpStream(); // Do we need to do logging for trigger-related dumps? //Compression is done automatically by the response //Chunking is done because the result has no content-length FileStreamResult FileStreamResult = File(dumpStream, ContentTypes.ApplicationOctetStream, dumpFileName); // How and where do we use FileStreamResult? Do we need to call an execute method on the instance, or can we call File without keeping the value? + + dumpFilePath = FileStreamResult.FileDownloadName; // I believe this gives us the actual dump_....dmp name } else { From 346af2514e949b6c73f0d753ab27470111298932 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Fri, 27 Aug 2021 13:14:14 -0700 Subject: [PATCH 05/22] Realized that we aren't supporting the null scenario for egress; intermediary check in --- .../CollectDumpActionTests.cs | 9 ++++++--- .../CollectionRules/Actions/CollectDumpAction.cs | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index fda66e68cd9..2440ab223bd 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -30,6 +30,7 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests public sealed class CollectDumpActionTests { private static readonly TimeSpan GetEndpointInfoTimeout = TimeSpan.FromSeconds(10); + //private const string FileProviderName = "files"; private const int TokenTimeoutMs = 60000; // Arbitrarily set to 1 minute -> potentially needs to be bigger...? //private const int DelayMs = 1000; @@ -37,10 +38,12 @@ public sealed class CollectDumpActionTests private IServiceProvider _serviceProvider; private ILogger _logger; private ITestOutputHelper _outputHelper; + private readonly DirectoryInfo _tempEgressPath; public CollectDumpActionTests(ITestOutputHelper outputHelper) { _outputHelper = outputHelper; + _tempEgressPath = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "Egress", Guid.NewGuid().ToString())); } private void SetUpHost(string egressProvider) @@ -103,15 +106,15 @@ private void SetUpHost(string egressProvider) [Fact] - public async Task CollectDumpAction_NoEgressProvider() + public async Task CollectDumpAction_FileEgressProvider() { - SetUpHost(null); + SetUpHost(null); // Can potentially remove this param (or need to figure out what it will do for us) CollectDumpAction action = new(_logger, _serviceProvider); CollectDumpOptions options = new(); - options.Egress = null; + options.Egress = _tempEgressPath.ToString(); // Pay attention to this //options.Type = WebApi.Models.DumpType.Full; using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 5e951974991..1c4119fc5a7 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -39,7 +39,7 @@ public CollectDumpAction(ILogger logger, public async Task ExecuteAsync(CollectDumpOptions options, IEndpointInfo endpointInfo, CancellationToken token) { DumpType dumpType = options.Type.GetValueOrDefault(CollectDumpOptionsDefaults.Type); - string egressProvider = options.Egress; // Do we need to check this? + string egressProvider = options.Egress; // I believe we should be doing a check for a non-null value (or maybe a valid one)? int pid = endpointInfo.ProcessId; From f24cc13d4d0f6a0379788dada99ec6dadeb870e4 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Tue, 31 Aug 2021 08:12:49 -0700 Subject: [PATCH 06/22] Not currently in a functioning state (for tests); need to ask Justin about how to best integrate with existing infrastructure. --- .../CollectDumpActionTests.cs | 96 ++++++++++++++++++- .../TestHostHelper.cs | 16 +++- .../Actions/CollectDumpAction.cs | 11 +-- 3 files changed, 108 insertions(+), 15 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index 2440ab223bd..eabc5752175 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -24,6 +24,9 @@ using System.Reflection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Configuration; +using Microsoft.Diagnostics.Monitoring.TestCommon.Options; +using Microsoft.Diagnostics.Monitoring.WebApi.Models; +using Microsoft.Diagnostics.Monitoring.Tool.UnitTests.Runners; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { @@ -35,10 +38,14 @@ public sealed class CollectDumpActionTests private const int TokenTimeoutMs = 60000; // Arbitrarily set to 1 minute -> potentially needs to be bigger...? //private const int DelayMs = 1000; + private const string DefaultRuleName = "Default"; + private IServiceProvider _serviceProvider; private ILogger _logger; private ITestOutputHelper _outputHelper; private readonly DirectoryInfo _tempEgressPath; + private IHttpClientFactory _httpClientFactory; + public CollectDumpActionTests(ITestOutputHelper outputHelper) { @@ -79,6 +86,9 @@ private void SetUpHost(string egressProvider) _serviceProvider = host.Services; _logger = host.Services.GetRequiredService().CreateLogger(); + + _httpClientFactory = host.Services.GetService(); + //_outputHelper = host.Services.GetService(); @@ -104,11 +114,87 @@ private void SetUpHost(string egressProvider) */ } - - [Fact] - public async Task CollectDumpAction_FileEgressProvider() + [InlineData(DiagnosticPortConnectionMode.Connect)] +#if NET5_0_OR_GREATER + [InlineData(DiagnosticPortConnectionMode.Listen)] +#endif + [Theory] + public async Task CollectDumpAction_FileEgressProvider(DiagnosticPortConnectionMode mode) { - SetUpHost(null); // Can potentially remove this param (or need to figure out what it will do for us) + SetUpHost(null); + + const string ExpectedEgressProvider = "TmpEgressProvider"; + const DumpType ExpectedDumpType = DumpType.Mini; + + await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => + { + rootOptions.CreateCollectionRule(DefaultRuleName) + .SetStartupTrigger() + .AddCollectDumpAction(ExpectedDumpType, ExpectedEgressProvider); + + rootOptions.AddFileSystemEgress(ExpectedEgressProvider, "/tmp"); + }, async host => + { + _serviceProvider = host.Services; + _logger = host.Services.GetRequiredService().CreateLogger(); + + CollectDumpAction action = new(_logger, _serviceProvider); + + CollectDumpOptions options = new(); + + options.Egress = ExpectedEgressProvider; // Pay attention to this + options.Type = ExpectedDumpType; + + using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); + + /////////////////// + + ServerEndpointInfoCallback callback = new(_outputHelper); + await using var source = CreateServerSource(out string transportName, callback); + source.Start(); + + var endpointInfos = await GetEndpointInfoAsync(source); + Assert.Empty(endpointInfos); + + AppRunner runner = CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60 + + Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); + + await runner.ExecuteAsync(async () => + { + await newEndpointInfoTask; + + endpointInfos = await GetEndpointInfoAsync(source); + + var endpointInfo = Assert.Single(endpointInfos); + Assert.NotNull(endpointInfo.CommandLine); + Assert.NotNull(endpointInfo.OperatingSystem); + Assert.NotNull(endpointInfo.ProcessArchitecture); + VerifyConnection(runner, endpointInfo); + + CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); + + string egressPath = result.OutputValues["EgressPath"]; + + if (!File.Exists(egressPath)) + { + throw new FileNotFoundException(string.Format(CultureInfo.InvariantCulture, Tools.Monitor.Strings.ErrorMessage_FileNotFound, egressPath)); + } + + await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); + }); + + await Task.Delay(TimeSpan.FromSeconds(1)); + + endpointInfos = await GetEndpointInfoAsync(source); + + Assert.Empty(endpointInfos); + }); + }); + + + /* + //SetUpHost(null); // Can potentially remove this param (or need to figure out what it will do for us) CollectDumpAction action = new(_logger, _serviceProvider); @@ -161,6 +247,8 @@ await runner.ExecuteAsync(async () => endpointInfos = await GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); + + */ } /* diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs index f43c50c66e9..368a8b6cf1c 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs @@ -2,8 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.Tools.Monitor; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using System; using System.Collections.Generic; @@ -67,11 +69,23 @@ public static IHost CreateHost( outputHelper.WriteLine("End Configuration"); builder.AddInMemoryCollection(configurationValues); + + builder.AddInMemoryCollection(new Dictionary + { + {ConfigurationPath.Combine(ConfigurationKeys.Storage, nameof(StorageOptions.DumpTempFolder)), StorageOptionsDefaults.DumpTempFolder } + }); }) - .ConfigureServices(services => + .ConfigureServices((HostBuilderContext context, IServiceCollection services) => { services.ConfigureCollectionRules(); services.ConfigureEgress(); + + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.ConfigureStorage(context.Configuration); }) .Build(); } diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 1c4119fc5a7..5db41efaea3 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -55,16 +55,7 @@ public async Task ExecuteAsync(CollectDumpOptions op if (string.IsNullOrEmpty(egressProvider)) { - Stream dumpStream = await _diagnosticServices.GetDump(processInfo, dumpType, token); - - //dumpFilePath = ((FileStream)dumpStream).Name; // I believe this gives us the temporary file (and then it gets deleted) - - _logger.WrittenToHttpStream(); // Do we need to do logging for trigger-related dumps? - //Compression is done automatically by the response - //Chunking is done because the result has no content-length - FileStreamResult FileStreamResult = File(dumpStream, ContentTypes.ApplicationOctetStream, dumpFileName); // How and where do we use FileStreamResult? Do we need to call an execute method on the instance, or can we call File without keeping the value? - - dumpFilePath = FileStreamResult.FileDownloadName; // I believe this gives us the actual dump_....dmp name + throw new ArgumentException("No Egress Provider was supplied."); } else { From 880924409f1e9007d7c13709caf1ed708064d471 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Wed, 1 Sep 2021 14:59:20 -0700 Subject: [PATCH 07/22] Removed a lot of commented out code that was no longer relevant and was hurting readability in tests. No functional changes. --- .../CollectDumpActionTests.cs | 134 +----------------- 1 file changed, 3 insertions(+), 131 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index eabc5752175..35a416fef7b 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -26,7 +26,6 @@ using Microsoft.Extensions.Configuration; using Microsoft.Diagnostics.Monitoring.TestCommon.Options; using Microsoft.Diagnostics.Monitoring.WebApi.Models; -using Microsoft.Diagnostics.Monitoring.Tool.UnitTests.Runners; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { @@ -91,11 +90,6 @@ private void SetUpHost(string egressProvider) //_outputHelper = host.Services.GetService(); - - - - - /* const string ExpectedEgressProvider = "TmpEgressProvider"; @@ -114,14 +108,10 @@ private void SetUpHost(string egressProvider) */ } - [InlineData(DiagnosticPortConnectionMode.Connect)] -#if NET5_0_OR_GREATER - [InlineData(DiagnosticPortConnectionMode.Listen)] -#endif - [Theory] - public async Task CollectDumpAction_FileEgressProvider(DiagnosticPortConnectionMode mode) + [Fact] + public async Task CollectDumpAction_FileEgressProvider() { - SetUpHost(null); + // SetUpHost(null); const string ExpectedEgressProvider = "TmpEgressProvider"; const DumpType ExpectedDumpType = DumpType.Mini; @@ -190,126 +180,8 @@ await runner.ExecuteAsync(async () => Assert.Empty(endpointInfos); }); - }); - - - /* - //SetUpHost(null); // Can potentially remove this param (or need to figure out what it will do for us) - - CollectDumpAction action = new(_logger, _serviceProvider); - - CollectDumpOptions options = new(); - - options.Egress = _tempEgressPath.ToString(); // Pay attention to this - //options.Type = WebApi.Models.DumpType.Full; - - using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); - - /////////////////// - - ServerEndpointInfoCallback callback = new(_outputHelper); - await using var source = CreateServerSource(out string transportName, callback); - source.Start(); - - var endpointInfos = await GetEndpointInfoAsync(source); - Assert.Empty(endpointInfos); - - AppRunner runner = CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60 - - Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); - - await runner.ExecuteAsync(async () => - { - await newEndpointInfoTask; - - endpointInfos = await GetEndpointInfoAsync(source); - - var endpointInfo = Assert.Single(endpointInfos); - Assert.NotNull(endpointInfo.CommandLine); - Assert.NotNull(endpointInfo.OperatingSystem); - Assert.NotNull(endpointInfo.ProcessArchitecture); - VerifyConnection(runner, endpointInfo); - - CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); - - string egressPath = result.OutputValues["EgressPath"]; - - if (!File.Exists(egressPath)) - { - throw new FileNotFoundException(string.Format(CultureInfo.InvariantCulture, Tools.Monitor.Strings.ErrorMessage_FileNotFound, egressPath)); - } - - await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); - }); - - await Task.Delay(TimeSpan.FromSeconds(1)); - - endpointInfos = await GetEndpointInfoAsync(source); - - Assert.Empty(endpointInfos); - - */ } - /* - [Fact] - public async Task CollectDumpAction_FileEgressProvider() - { - SetUpHost(); - - CollectDumpAction action = new(_logger, _serviceProvider); - - CollectDumpOptions options = new(); - - options.Egress = null; - //options.Type = WebApi.Models.DumpType.Full; - - using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); - - /////////////////// - - ServerEndpointInfoCallback callback = new(_outputHelper); - await using var source = CreateServerSource(out string transportName, callback); - source.Start(); - - var endpointInfos = await GetEndpointInfoAsync(source); - Assert.Empty(endpointInfos); - - AppRunner runner = CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60 - - Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); - - await runner.ExecuteAsync(async () => - { - await newEndpointInfoTask; - - endpointInfos = await GetEndpointInfoAsync(source); - - var endpointInfo = Assert.Single(endpointInfos); - Assert.NotNull(endpointInfo.CommandLine); - Assert.NotNull(endpointInfo.OperatingSystem); - Assert.NotNull(endpointInfo.ProcessArchitecture); - VerifyConnection(runner, endpointInfo); - - CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); - - string egressPath = result.OutputValues["EgressPath"]; - - if (!File.Exists(egressPath)) - { - throw new FileNotFoundException(string.Format(CultureInfo.InvariantCulture, Tools.Monitor.Strings.ErrorMessage_FileNotFound, egressPath)); - } - - await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); - }); - - await Task.Delay(TimeSpan.FromSeconds(1)); - - endpointInfos = await GetEndpointInfoAsync(source); - - Assert.Empty(endpointInfos); - }*/ - private ServerEndpointInfoSource CreateServerSource(out string transportName, ServerEndpointInfoCallback callback = null) { DiagnosticPortHelper.Generate(DiagnosticPortConnectionMode.Listen, out _, out transportName); From 77c4619229d2af0438d90e5695d67d85185bdc15 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Thu, 2 Sep 2021 10:25:31 -0700 Subject: [PATCH 08/22] File egress test is now passing and file is being egressed to the specified directory. Still need to do refactoring to better share code, and probably another test for azure blob egress. --- .../DumpTests.cs | 2 +- .../TestTimeouts.cs | 2 +- .../CollectDumpActionTests.cs | 175 +++++++++++------- .../Actions/CollectDumpAction.cs | 22 ++- 4 files changed, 124 insertions(+), 77 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs index cfad516dd64..4c77426dbce 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs @@ -144,7 +144,7 @@ public Task DumpTest(DiagnosticPortConnectionMode mode, DumpType type) }); } - private class MinidumpHeader : TStruct + public class MinidumpHeader : TStruct { public uint Signature = 0; public uint Version = 0; diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/TestTimeouts.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/TestTimeouts.cs index 35403cacebb..87e4aa4a2f6 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/TestTimeouts.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/TestTimeouts.cs @@ -6,7 +6,7 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests { - internal static class TestTimeouts + public static class TestTimeouts { /// /// Default timeout for HTTP API calls diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index 35a416fef7b..1f11dbe8438 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -26,6 +26,12 @@ using Microsoft.Extensions.Configuration; using Microsoft.Diagnostics.Monitoring.TestCommon.Options; using Microsoft.Diagnostics.Monitoring.WebApi.Models; +using Microsoft.FileFormats; +using System.Runtime.InteropServices; +using Microsoft.FileFormats.ELF; +using Microsoft.FileFormats.MachO; +using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests; +using static Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.DumpTests; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { @@ -35,94 +41,36 @@ public sealed class CollectDumpActionTests //private const string FileProviderName = "files"; private const int TokenTimeoutMs = 60000; // Arbitrarily set to 1 minute -> potentially needs to be bigger...? - //private const int DelayMs = 1000; + //private const int DelayMs = 1000; + private const string EnableElfDumpOnMacOS = "COMPlus_DbgEnableElfDumpOnMacOS"; private const string DefaultRuleName = "Default"; + private const string TempEgressDirectory = "/tmp"; private IServiceProvider _serviceProvider; private ILogger _logger; private ITestOutputHelper _outputHelper; - private readonly DirectoryInfo _tempEgressPath; - private IHttpClientFactory _httpClientFactory; - public CollectDumpActionTests(ITestOutputHelper outputHelper) { _outputHelper = outputHelper; - _tempEgressPath = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "Egress", Guid.NewGuid().ToString())); - } - - private void SetUpHost(string egressProvider) - { - IHost host = new HostBuilder() - .ConfigureAppConfiguration((IConfigurationBuilder builder) => - { - builder.AddInMemoryCollection(new Dictionary - { - {ConfigurationPath.Combine(ConfigurationKeys.Storage, nameof(StorageOptions.DumpTempFolder)), StorageOptionsDefaults.DumpTempFolder } - }); - }) - .ConfigureServices((HostBuilderContext context, IServiceCollection services) => - { - services.AddSingleton(); - services.AddSingleton(); - services.AddSingleton(); - services.AddSingleton(); - services.AddSingleton(); - services.ConfigureStorage(context.Configuration); - - - //services.AddHostedService(); - //services.AddHostedService(); - //services.ConfigureCollectionRules(); - //services.ConfigureEgress(); - //services.ConfigureOperationStore(); - //services.ConfigureMetrics(context.Configuration); - //services.ConfigureDefaultProcess(context.Configuration); - - }) - .Build(); - - _serviceProvider = host.Services; - _logger = host.Services.GetRequiredService().CreateLogger(); - - _httpClientFactory = host.Services.GetService(); - - //_outputHelper = host.Services.GetService(); - - /* - const string ExpectedEgressProvider = "TmpEgressProvider"; - - return ValidateSuccess( - rootOptions => - { - rootOptions.CreateCollectionRule(DefaultRuleName) - .SetStartupTrigger() - .AddCollectDumpAction(ExpectedDumpType, ExpectedEgressProvider); - rootOptions.AddFileSystemEgress(ExpectedEgressProvider, "/tmp"); - }, - ruleOptions => - { - ruleOptions.VerifyCollectDumpAction(0, ExpectedDumpType, ExpectedEgressProvider); - }); - */ } [Fact] public async Task CollectDumpAction_FileEgressProvider() { - // SetUpHost(null); - const string ExpectedEgressProvider = "TmpEgressProvider"; const DumpType ExpectedDumpType = DumpType.Mini; + string uniqueEgressDirectory = TempEgressDirectory + Guid.NewGuid(); + await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => { rootOptions.CreateCollectionRule(DefaultRuleName) .SetStartupTrigger() .AddCollectDumpAction(ExpectedDumpType, ExpectedEgressProvider); - rootOptions.AddFileSystemEgress(ExpectedEgressProvider, "/tmp"); + rootOptions.AddFileSystemEgress(ExpectedEgressProvider, uniqueEgressDirectory); }, async host => { _serviceProvider = host.Services; @@ -132,13 +80,11 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => CollectDumpOptions options = new(); - options.Egress = ExpectedEgressProvider; // Pay attention to this + options.Egress = ExpectedEgressProvider; options.Type = ExpectedDumpType; using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); - /////////////////// - ServerEndpointInfoCallback callback = new(_outputHelper); await using var source = CreateServerSource(out string transportName, callback); source.Start(); @@ -170,6 +116,84 @@ await runner.ExecuteAsync(async () => { throw new FileNotFoundException(string.Format(CultureInfo.InvariantCulture, Tools.Monitor.Strings.ErrorMessage_FileNotFound, egressPath)); } + else + { + using (StreamReader reader = new StreamReader(egressPath, true)) + { + Stream dumpStream = reader.BaseStream; + + Assert.NotNull(dumpStream); + + byte[] headerBuffer = new byte[64]; + + // Read enough to deserialize the header. + int read; + int total = 0; + using CancellationTokenSource cancellation = new(TestTimeouts.DumpTimeout); + while (total < headerBuffer.Length && 0 != (read = await dumpStream.ReadAsync(headerBuffer, total, headerBuffer.Length - total, cancellation.Token))) + { + total += read; + } + Assert.Equal(headerBuffer.Length, total); + + // Read header and validate + using MemoryStream headerStream = new(headerBuffer); + + StreamAddressSpace dumpAddressSpace = new(headerStream); + Reader dumpReader = new(dumpAddressSpace); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + MinidumpHeader header = dumpReader.Read(0); + // Validate Signature + Assert.True(header.IsSignatureValid.Check()); + } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + ELFHeaderIdent ident = dumpReader.Read(0); + Assert.True(ident.IsIdentMagicValid.Check()); + Assert.True(ident.IsClassValid.Check()); + Assert.True(ident.IsDataValid.Check()); + + LayoutManager layoutManager = new(); + layoutManager.AddELFTypes( + isBigEndian: ident.Data == ELFData.BigEndian, + is64Bit: ident.Class == ELFClass.Class64); + Reader headerReader = new(dumpAddressSpace, layoutManager); + + ELFHeader header = headerReader.Read(0); + // Validate Signature + Assert.True(header.IsIdentMagicValid.Check()); + // Validate ELF file is a core dump + Assert.Equal(ELFHeaderType.Core, header.Type); + } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + if (runner.Environment.ContainsKey(EnableElfDumpOnMacOS)) + { + ELFHeader header = dumpReader.Read(0); + // Validate Signature + Assert.True(header.IsIdentMagicValid.Check()); + // Validate ELF file is a core dump + Assert.Equal(ELFHeaderType.Core, header.Type); + } + else + { + MachHeader header = dumpReader.Read(0); + // Validate Signature + Assert.True(header.IsMagicValid.Check()); + // Validate MachO file is a core dump + Assert.True(header.IsFileTypeValid.Check()); + Assert.Equal(MachHeaderFileType.Core, header.FileType); + } + } + else + { + throw new NotImplementedException("Dump header check not implemented for this OS platform."); + } + } + + } await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }); @@ -179,6 +203,22 @@ await runner.ExecuteAsync(async () => endpointInfos = await GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); + + try + { + DirectoryInfo outputDirectory = Directory.CreateDirectory(uniqueEgressDirectory); + + try + { + outputDirectory?.Delete(recursive: true); + } + catch + { + } + } + catch + { + } }); } @@ -222,6 +262,5 @@ private static void VerifyConnection(AppRunner runner, IEndpointInfo endpointInf Assert.NotEqual(Guid.Empty, endpointInfo.RuntimeInstanceCookie); Assert.NotNull(endpointInfo.Endpoint); } - } } diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 5db41efaea3..c7b1f12b82e 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -26,10 +26,12 @@ internal sealed class CollectDumpAction : ControllerBase, private readonly ILogger _logger; private readonly IDiagnosticServices _diagnosticServices; private readonly EgressOperationStore _operationsStore; + private readonly IServiceProvider _serviceProvider; public CollectDumpAction(ILogger logger, IServiceProvider serviceProvider) { + _serviceProvider = serviceProvider; _logger = logger; _diagnosticServices = serviceProvider.GetRequiredService(); //_diagnosticPortOptions = serviceProvider.GetService>(); @@ -39,7 +41,7 @@ public CollectDumpAction(ILogger logger, public async Task ExecuteAsync(CollectDumpOptions options, IEndpointInfo endpointInfo, CancellationToken token) { DumpType dumpType = options.Type.GetValueOrDefault(CollectDumpOptionsDefaults.Type); - string egressProvider = options.Egress; // I believe we should be doing a check for a non-null value (or maybe a valid one)? + string egressProvider = options.Egress; int pid = endpointInfo.ProcessId; @@ -47,6 +49,7 @@ public async Task ExecuteAsync(CollectDumpOptions op IProcessInfo processInfo = await _diagnosticServices.GetProcessAsync(processKey, token); + // Move into utility method in WebApi string dumpFileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? FormattableString.Invariant($"dump_{GetFileNameTimeStampUtcNow()}.dmp") : FormattableString.Invariant($"core_{GetFileNameTimeStampUtcNow()}"); @@ -59,6 +62,7 @@ public async Task ExecuteAsync(CollectDumpOptions op } else { + // Move into utility method in WebApi KeyValueLogScope scope = new KeyValueLogScope(); scope.AddArtifactType(ArtifactType_Dump); scope.AddEndpointInfo(processInfo.EndpointInfo); @@ -69,7 +73,7 @@ public async Task ExecuteAsync(CollectDumpOptions op dumpFileName, processInfo.EndpointInfo, ContentTypes.ApplicationOctetStream, - scope), limitKey: ArtifactType_Dump); + scope), token); } return new CollectionRuleActionResult() @@ -86,17 +90,21 @@ private static string GetFileNameTimeStampUtcNow() return DateTime.UtcNow.ToString("yyyyMMdd_HHmmss"); } - private async Task SendToEgress(EgressOperation egressStreamResult, string limitKey) + // Look into IEgressService + private async Task SendToEgress(EgressOperation egressStreamResult, CancellationToken token) { + var result = await egressStreamResult.ExecuteAsync(_serviceProvider, token); + + return result.Result.Value; // Not sure what this is, but is a string + + /* // Will throw TooManyRequestsException if there are too many concurrent operations. Guid operationId = await _operationsStore.AddOperation(egressStreamResult, limitKey); - string newUrl = this.Url.Action( - action: nameof(OperationsController.GetOperationStatus), - controller: OperationsController.ControllerName, new { operationId = operationId }, - protocol: this.HttpContext.Request.Scheme, this.HttpContext.Request.Host.ToString()); + string newUrl = egressStreamResult. return newUrl; // Switched to returning the URL so that we can include it in the CollectionRuleActionResult //return Accepted(newUrl); + */ } } } From 77813561d322d00b2f60acc40ce96c567519a8d4 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Thu, 2 Sep 2021 13:52:08 -0700 Subject: [PATCH 09/22] Reorganization and some code clean-up. --- .../Controllers/DiagController.cs | 20 ++- .../DumpTests.cs | 159 ++++++++--------- .../IDumpTestInterface.cs | 118 +++++++++++++ .../CollectDumpActionTests.cs | 167 ++---------------- .../EndpointInfoSourceTests.cs | 8 +- .../Actions/CollectDumpAction.cs | 50 +----- 6 files changed, 241 insertions(+), 281 deletions(-) create mode 100644 src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 8193b4ce4b3..20a34447cc0 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -211,9 +211,7 @@ public Task CaptureDump( return InvokeForProcess(async processInfo => { - string dumpFileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? - FormattableString.Invariant($"dump_{GetFileNameTimeStampUtcNow()}.dmp") : - FormattableString.Invariant($"core_{GetFileNameTimeStampUtcNow()}"); + string dumpFileName = GenerateDumpFileName(); if (string.IsNullOrEmpty(egressProvider)) { @@ -584,6 +582,22 @@ private static string GetDotnetMonitorVersion() } } + internal static string GenerateDumpFileName() + { + return RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? + FormattableString.Invariant($"dump_{GetFileNameTimeStampUtcNow()}.dmp") : + FormattableString.Invariant($"core_{GetFileNameTimeStampUtcNow()}"); + } + + internal static KeyValueLogScope GetDumpScope(IProcessInfo processInfo) + { + KeyValueLogScope scope = new KeyValueLogScope(); + scope.AddArtifactType(ArtifactType_Dump); + scope.AddEndpointInfo(processInfo.EndpointInfo); + + return scope; + } + public DiagnosticPortConnectionMode GetDiagnosticPortMode() { return _diagnosticPortOptions.Value.ConnectionMode.GetValueOrDefault(DiagnosticPortOptionsDefaults.ConnectionMode); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs index 4c77426dbce..636ee82d43e 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs @@ -25,10 +25,8 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests { [Collection(DefaultCollectionFixture.Name)] - public class DumpTests + public class DumpTests : IDumpTestInterface { - private const string EnableElfDumpOnMacOS = "COMPlus_DbgEnableElfDumpOnMacOS"; - private readonly IHttpClientFactory _httpClientFactory; private readonly ITestOutputHelper _outputHelper; @@ -64,73 +62,7 @@ public Task DumpTest(DiagnosticPortConnectionMode mode, DumpType type) using ResponseStreamHolder holder = await client.CaptureDumpAsync(runner.ProcessId, type); Assert.NotNull(holder); - byte[] headerBuffer = new byte[64]; - - // Read enough to deserialize the header. - int read; - int total = 0; - using CancellationTokenSource cancellation = new(TestTimeouts.DumpTimeout); - while (total < headerBuffer.Length && 0 != (read = await holder.Stream.ReadAsync(headerBuffer, total, headerBuffer.Length - total, cancellation.Token))) - { - total += read; - } - Assert.Equal(headerBuffer.Length, total); - - // Read header and validate - using MemoryStream headerStream = new(headerBuffer); - - StreamAddressSpace dumpAddressSpace = new(headerStream); - Reader dumpReader = new(dumpAddressSpace); - - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - MinidumpHeader header = dumpReader.Read(0); - // Validate Signature - Assert.True(header.IsSignatureValid.Check()); - } - else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) - { - ELFHeaderIdent ident = dumpReader.Read(0); - Assert.True(ident.IsIdentMagicValid.Check()); - Assert.True(ident.IsClassValid.Check()); - Assert.True(ident.IsDataValid.Check()); - - LayoutManager layoutManager = new(); - layoutManager.AddELFTypes( - isBigEndian: ident.Data == ELFData.BigEndian, - is64Bit: ident.Class == ELFClass.Class64); - Reader headerReader = new(dumpAddressSpace, layoutManager); - - ELFHeader header = headerReader.Read(0); - // Validate Signature - Assert.True(header.IsIdentMagicValid.Check()); - // Validate ELF file is a core dump - Assert.Equal(ELFHeaderType.Core, header.Type); - } - else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) - { - if (runner.Environment.ContainsKey(EnableElfDumpOnMacOS)) - { - ELFHeader header = dumpReader.Read(0); - // Validate Signature - Assert.True(header.IsIdentMagicValid.Check()); - // Validate ELF file is a core dump - Assert.Equal(ELFHeaderType.Core, header.Type); - } - else - { - MachHeader header = dumpReader.Read(0); - // Validate Signature - Assert.True(header.IsMagicValid.Check()); - // Validate MachO file is a core dump - Assert.True(header.IsFileTypeValid.Check()); - Assert.Equal(MachHeaderFileType.Core, header.FileType); - } - } - else - { - throw new NotImplementedException("Dump header check not implemented for this OS platform."); - } + await IDumpTestInterface.ValidateDump(runner, holder.Stream); await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }, @@ -139,23 +71,86 @@ public Task DumpTest(DiagnosticPortConnectionMode mode, DumpType type) // MachO not supported on .NET 5, only ELF: https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/xplat-minidump-generation.md#os-x if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && DotNetHost.RuntimeVersion.Major == 5) { - runner.Environment.Add(EnableElfDumpOnMacOS, "1"); + runner.Environment.Add(IDumpTestInterface.EnableElfDumpOnMacOS, "1"); } }); } - public class MinidumpHeader : TStruct + /* + public static async Task ValidateDump(AppRunner runner, Stream dumpStream) { - public uint Signature = 0; - public uint Version = 0; - public uint NumberOfStreams = 0; - public uint StreamDirectoryRva = 0; - public uint CheckSum = 0; - public uint TimeDateStamp = 0; - public ulong Flags = 0; - - // 50,4D,44,4D = PMDM - public ValidationRule IsSignatureValid => new ValidationRule("Invalid Signature", () => Signature == 0x504D444DU); + Assert.NotNull(dumpStream); + + byte[] headerBuffer = new byte[64]; + + // Read enough to deserialize the header. + int read; + int total = 0; + using CancellationTokenSource cancellation = new(TestTimeouts.DumpTimeout); + while (total < headerBuffer.Length && 0 != (read = await dumpStream.ReadAsync(headerBuffer, total, headerBuffer.Length - total, cancellation.Token))) + { + total += read; + } + Assert.Equal(headerBuffer.Length, total); + + // Read header and validate + using MemoryStream headerStream = new(headerBuffer); + + StreamAddressSpace dumpAddressSpace = new(headerStream); + Reader dumpReader = new(dumpAddressSpace); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + MinidumpHeader header = dumpReader.Read(0); + // Validate Signature + Assert.True(header.IsSignatureValid.Check()); + } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + ELFHeaderIdent ident = dumpReader.Read(0); + Assert.True(ident.IsIdentMagicValid.Check()); + Assert.True(ident.IsClassValid.Check()); + Assert.True(ident.IsDataValid.Check()); + + LayoutManager layoutManager = new(); + layoutManager.AddELFTypes( + isBigEndian: ident.Data == ELFData.BigEndian, + is64Bit: ident.Class == ELFClass.Class64); + Reader headerReader = new(dumpAddressSpace, layoutManager); + + ELFHeader header = headerReader.Read(0); + // Validate Signature + Assert.True(header.IsIdentMagicValid.Check()); + // Validate ELF file is a core dump + Assert.Equal(ELFHeaderType.Core, header.Type); + } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + if (runner.Environment.ContainsKey(EnableElfDumpOnMacOS)) + { + ELFHeader header = dumpReader.Read(0); + // Validate Signature + Assert.True(header.IsIdentMagicValid.Check()); + // Validate ELF file is a core dump + Assert.Equal(ELFHeaderType.Core, header.Type); + } + else + { + MachHeader header = dumpReader.Read(0); + // Validate Signature + Assert.True(header.IsMagicValid.Check()); + // Validate MachO file is a core dump + Assert.True(header.IsFileTypeValid.Check()); + Assert.Equal(MachHeaderFileType.Core, header.FileType); + } + } + else + { + throw new NotImplementedException("Dump header check not implemented for this OS platform."); + } } + */ + + } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs new file mode 100644 index 00000000000..5d25b862868 --- /dev/null +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs @@ -0,0 +1,118 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.Diagnostics.Monitoring.TestCommon; +using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; +using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Fixtures; +using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.HttpApi; +using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Runners; +using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Diagnostics.Monitoring.WebApi.Models; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.FileFormats; +using Microsoft.FileFormats.ELF; +using Microsoft.FileFormats.MachO; +using System; +using System.IO; +using System.Net.Http; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests +{ + public interface IDumpTestInterface + { + internal const string EnableElfDumpOnMacOS = "COMPlus_DbgEnableElfDumpOnMacOS"; + + public static async Task ValidateDump(AppRunner runner, Stream dumpStream) + { + Assert.NotNull(dumpStream); + + byte[] headerBuffer = new byte[64]; + + // Read enough to deserialize the header. + int read; + int total = 0; + using CancellationTokenSource cancellation = new(TestTimeouts.DumpTimeout); + while (total < headerBuffer.Length && 0 != (read = await dumpStream.ReadAsync(headerBuffer, total, headerBuffer.Length - total, cancellation.Token))) + { + total += read; + } + Assert.Equal(headerBuffer.Length, total); + + // Read header and validate + using MemoryStream headerStream = new(headerBuffer); + + StreamAddressSpace dumpAddressSpace = new(headerStream); + Reader dumpReader = new(dumpAddressSpace); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + MinidumpHeader header = dumpReader.Read(0); + // Validate Signature + Assert.True(header.IsSignatureValid.Check()); + } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + ELFHeaderIdent ident = dumpReader.Read(0); + Assert.True(ident.IsIdentMagicValid.Check()); + Assert.True(ident.IsClassValid.Check()); + Assert.True(ident.IsDataValid.Check()); + + LayoutManager layoutManager = new(); + layoutManager.AddELFTypes( + isBigEndian: ident.Data == ELFData.BigEndian, + is64Bit: ident.Class == ELFClass.Class64); + Reader headerReader = new(dumpAddressSpace, layoutManager); + + ELFHeader header = headerReader.Read(0); + // Validate Signature + Assert.True(header.IsIdentMagicValid.Check()); + // Validate ELF file is a core dump + Assert.Equal(ELFHeaderType.Core, header.Type); + } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + if (runner.Environment.ContainsKey(EnableElfDumpOnMacOS)) + { + ELFHeader header = dumpReader.Read(0); + // Validate Signature + Assert.True(header.IsIdentMagicValid.Check()); + // Validate ELF file is a core dump + Assert.Equal(ELFHeaderType.Core, header.Type); + } + else + { + MachHeader header = dumpReader.Read(0); + // Validate Signature + Assert.True(header.IsMagicValid.Check()); + // Validate MachO file is a core dump + Assert.True(header.IsFileTypeValid.Check()); + Assert.Equal(MachHeaderFileType.Core, header.FileType); + } + } + else + { + throw new NotImplementedException("Dump header check not implemented for this OS platform."); + } + } + + public class MinidumpHeader : TStruct + { + public uint Signature = 0; + public uint Version = 0; + public uint NumberOfStreams = 0; + public uint StreamDirectoryRva = 0; + public uint CheckSum = 0; + public uint TimeDateStamp = 0; + public ulong Flags = 0; + + // 50,4D,44,4D = PMDM + public ValidationRule IsSignatureValid => new ValidationRule("Invalid Signature", () => Signature == 0x504D444DU); + } + } +} diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index 1f11dbe8438..eebf9526a79 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -11,44 +11,25 @@ using System.IO; using System.Globalization; using Xunit.Abstractions; -using System.Net.Http; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Diagnostics.Monitoring.WebApi; -using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Fixtures; -using Microsoft.Extensions.Logging; using Microsoft.Diagnostics.Monitoring.TestCommon; using System.Collections.Generic; using static Microsoft.Diagnostics.Monitoring.Tool.UnitTests.EndpointInfoSourceTests; using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; using Microsoft.Diagnostics.Tools.Monitor; using System.Reflection; -using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Configuration; using Microsoft.Diagnostics.Monitoring.TestCommon.Options; using Microsoft.Diagnostics.Monitoring.WebApi.Models; -using Microsoft.FileFormats; -using System.Runtime.InteropServices; -using Microsoft.FileFormats.ELF; -using Microsoft.FileFormats.MachO; using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests; -using static Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.DumpTests; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { public sealed class CollectDumpActionTests { - private static readonly TimeSpan GetEndpointInfoTimeout = TimeSpan.FromSeconds(10); - //private const string FileProviderName = "files"; - - private const int TokenTimeoutMs = 60000; // Arbitrarily set to 1 minute -> potentially needs to be bigger...? - //private const int DelayMs = 1000; - - private const string EnableElfDumpOnMacOS = "COMPlus_DbgEnableElfDumpOnMacOS"; private const string DefaultRuleName = "Default"; private const string TempEgressDirectory = "/tmp"; private IServiceProvider _serviceProvider; - private ILogger _logger; private ITestOutputHelper _outputHelper; public CollectDumpActionTests(ITestOutputHelper outputHelper) @@ -56,11 +37,15 @@ public CollectDumpActionTests(ITestOutputHelper outputHelper) _outputHelper = outputHelper; } - [Fact] - public async Task CollectDumpAction_FileEgressProvider() + [Theory] + [InlineData(DumpType.Full)] + [InlineData(DumpType.Mini)] + [InlineData(DumpType.Triage)] + [InlineData(DumpType.WithHeap)] + public async Task CollectDumpAction_FileEgressProvider(DumpType dumpType) { const string ExpectedEgressProvider = "TmpEgressProvider"; - const DumpType ExpectedDumpType = DumpType.Mini; + DumpType ExpectedDumpType = dumpType; string uniqueEgressDirectory = TempEgressDirectory + Guid.NewGuid(); @@ -74,25 +59,26 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => }, async host => { _serviceProvider = host.Services; - _logger = host.Services.GetRequiredService().CreateLogger(); - CollectDumpAction action = new(_logger, _serviceProvider); + CollectDumpAction action = new(_serviceProvider); CollectDumpOptions options = new(); options.Egress = ExpectedEgressProvider; options.Type = ExpectedDumpType; - using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TokenTimeoutMs); + using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TestTimeouts.DumpTimeout); + + EndpointInfoSourceTests endpointInfoSourceTests = new(_outputHelper); ServerEndpointInfoCallback callback = new(_outputHelper); - await using var source = CreateServerSource(out string transportName, callback); + await using var source = endpointInfoSourceTests.CreateServerSource(out string transportName, callback); source.Start(); - var endpointInfos = await GetEndpointInfoAsync(source); + var endpointInfos = await endpointInfoSourceTests.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); - AppRunner runner = CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60 + AppRunner runner = endpointInfoSourceTests.CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60; should we test against multiple versions? Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); @@ -100,7 +86,7 @@ await runner.ExecuteAsync(async () => { await newEndpointInfoTask; - endpointInfos = await GetEndpointInfoAsync(source); + endpointInfos = await endpointInfoSourceTests.GetEndpointInfoAsync(source); var endpointInfo = Assert.Single(endpointInfos); Assert.NotNull(endpointInfo.CommandLine); @@ -121,78 +107,10 @@ await runner.ExecuteAsync(async () => using (StreamReader reader = new StreamReader(egressPath, true)) { Stream dumpStream = reader.BaseStream; - Assert.NotNull(dumpStream); - byte[] headerBuffer = new byte[64]; - - // Read enough to deserialize the header. - int read; - int total = 0; - using CancellationTokenSource cancellation = new(TestTimeouts.DumpTimeout); - while (total < headerBuffer.Length && 0 != (read = await dumpStream.ReadAsync(headerBuffer, total, headerBuffer.Length - total, cancellation.Token))) - { - total += read; - } - Assert.Equal(headerBuffer.Length, total); - - // Read header and validate - using MemoryStream headerStream = new(headerBuffer); - - StreamAddressSpace dumpAddressSpace = new(headerStream); - Reader dumpReader = new(dumpAddressSpace); - - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - MinidumpHeader header = dumpReader.Read(0); - // Validate Signature - Assert.True(header.IsSignatureValid.Check()); - } - else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) - { - ELFHeaderIdent ident = dumpReader.Read(0); - Assert.True(ident.IsIdentMagicValid.Check()); - Assert.True(ident.IsClassValid.Check()); - Assert.True(ident.IsDataValid.Check()); - - LayoutManager layoutManager = new(); - layoutManager.AddELFTypes( - isBigEndian: ident.Data == ELFData.BigEndian, - is64Bit: ident.Class == ELFClass.Class64); - Reader headerReader = new(dumpAddressSpace, layoutManager); - - ELFHeader header = headerReader.Read(0); - // Validate Signature - Assert.True(header.IsIdentMagicValid.Check()); - // Validate ELF file is a core dump - Assert.Equal(ELFHeaderType.Core, header.Type); - } - else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) - { - if (runner.Environment.ContainsKey(EnableElfDumpOnMacOS)) - { - ELFHeader header = dumpReader.Read(0); - // Validate Signature - Assert.True(header.IsIdentMagicValid.Check()); - // Validate ELF file is a core dump - Assert.Equal(ELFHeaderType.Core, header.Type); - } - else - { - MachHeader header = dumpReader.Read(0); - // Validate Signature - Assert.True(header.IsMagicValid.Check()); - // Validate MachO file is a core dump - Assert.True(header.IsFileTypeValid.Check()); - Assert.Equal(MachHeaderFileType.Core, header.FileType); - } - } - else - { - throw new NotImplementedException("Dump header check not implemented for this OS platform."); - } + await IDumpTestInterface.ValidateDump(runner, dumpStream); } - } await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); @@ -200,67 +118,20 @@ await runner.ExecuteAsync(async () => await Task.Delay(TimeSpan.FromSeconds(1)); - endpointInfos = await GetEndpointInfoAsync(source); + endpointInfos = await endpointInfoSourceTests.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); try { - DirectoryInfo outputDirectory = Directory.CreateDirectory(uniqueEgressDirectory); + DirectoryInfo outputDirectory = Directory.CreateDirectory(uniqueEgressDirectory); // Do we have a better way of getting the current directory (to delete it) - try - { - outputDirectory?.Delete(recursive: true); - } - catch - { - } + outputDirectory?.Delete(recursive: true); } catch { } }); } - - private ServerEndpointInfoSource CreateServerSource(out string transportName, ServerEndpointInfoCallback callback = null) - { - DiagnosticPortHelper.Generate(DiagnosticPortConnectionMode.Listen, out _, out transportName); - _outputHelper.WriteLine("Starting server endpoint info source at '" + transportName + "'."); - - List callbacks = new(); - if (null != callback) - { - callbacks.Add(callback); - } - return new ServerEndpointInfoSource(transportName, callbacks); - } - - private AppRunner CreateAppRunner(string transportName, TargetFrameworkMoniker tfm, int appId = 1) - { - AppRunner appRunner = new(_outputHelper, Assembly.GetExecutingAssembly(), appId, tfm); - appRunner.ConnectionMode = DiagnosticPortConnectionMode.Connect; - appRunner.DiagnosticPortPath = transportName; - appRunner.ScenarioName = TestAppScenarios.AsyncWait.Name; - return appRunner; - } - - private async Task> GetEndpointInfoAsync(ServerEndpointInfoSource source) - { - _outputHelper.WriteLine("Getting endpoint infos."); - using CancellationTokenSource cancellationSource = new(GetEndpointInfoTimeout); - return await source.GetEndpointInfoAsync(cancellationSource.Token); - } - - /// - /// Verifies basic information on the connection and that it matches the target process from the runner. - /// - private static void VerifyConnection(AppRunner runner, IEndpointInfo endpointInfo) - { - Assert.NotNull(runner); - Assert.NotNull(endpointInfo); - Assert.Equal(runner.ProcessId, endpointInfo.ProcessId); - Assert.NotEqual(Guid.Empty, endpointInfo.RuntimeInstanceCookie); - Assert.NotNull(endpointInfo.Endpoint); - } } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs index c25663977a4..c260817a74c 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs @@ -210,7 +210,7 @@ public static IEnumerable GetTfmsSupportingPortListener() yield return new object[] { TargetFrameworkMoniker.Net60 }; } - private ServerEndpointInfoSource CreateServerSource(out string transportName, ServerEndpointInfoCallback callback = null) + internal ServerEndpointInfoSource CreateServerSource(out string transportName, ServerEndpointInfoCallback callback = null) { DiagnosticPortHelper.Generate(DiagnosticPortConnectionMode.Listen, out _, out transportName); _outputHelper.WriteLine("Starting server endpoint info source at '" + transportName + "'."); @@ -223,7 +223,7 @@ private ServerEndpointInfoSource CreateServerSource(out string transportName, Se return new ServerEndpointInfoSource(transportName, callbacks); } - private AppRunner CreateAppRunner(string transportName, TargetFrameworkMoniker tfm, int appId = 1) + internal AppRunner CreateAppRunner(string transportName, TargetFrameworkMoniker tfm, int appId = 1) { AppRunner appRunner = new(_outputHelper, Assembly.GetExecutingAssembly(), appId, tfm); appRunner.ConnectionMode = DiagnosticPortConnectionMode.Connect; @@ -232,7 +232,7 @@ private AppRunner CreateAppRunner(string transportName, TargetFrameworkMoniker t return appRunner; } - private async Task> GetEndpointInfoAsync(ServerEndpointInfoSource source) + internal async Task> GetEndpointInfoAsync(ServerEndpointInfoSource source) { _outputHelper.WriteLine("Getting endpoint infos."); using CancellationTokenSource cancellationSource = new(GetEndpointInfoTimeout); @@ -242,7 +242,7 @@ private async Task> GetEndpointInfoAsync(ServerEndpoi /// /// Verifies basic information on the connection and that it matches the target process from the runner. /// - private static void VerifyConnection(AppRunner runner, IEndpointInfo endpointInfo) + internal static void VerifyConnection(AppRunner runner, IEndpointInfo endpointInfo) { Assert.NotNull(runner); Assert.NotNull(endpointInfo); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index c7b1f12b82e..3316338ab1a 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -2,40 +2,27 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Microsoft.AspNetCore.Mvc; using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.Monitoring.WebApi.Controllers; using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using System; using System.Collections.Generic; -using System.IO; -using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; namespace Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Actions { - internal sealed class CollectDumpAction : ControllerBase, - ICollectionRuleAction + internal sealed class CollectDumpAction : ICollectionRuleAction { - public const string ArtifactType_Dump = "dump"; - - private readonly ILogger _logger; private readonly IDiagnosticServices _diagnosticServices; - private readonly EgressOperationStore _operationsStore; private readonly IServiceProvider _serviceProvider; - public CollectDumpAction(ILogger logger, - IServiceProvider serviceProvider) + public CollectDumpAction(IServiceProvider serviceProvider) { _serviceProvider = serviceProvider; - _logger = logger; _diagnosticServices = serviceProvider.GetRequiredService(); - //_diagnosticPortOptions = serviceProvider.GetService>(); - _operationsStore = serviceProvider.GetRequiredService(); } public async Task ExecuteAsync(CollectDumpOptions options, IEndpointInfo endpointInfo, CancellationToken token) @@ -43,16 +30,9 @@ public async Task ExecuteAsync(CollectDumpOptions op DumpType dumpType = options.Type.GetValueOrDefault(CollectDumpOptionsDefaults.Type); string egressProvider = options.Egress; - int pid = endpointInfo.ProcessId; - - ProcessKey? processKey = new ProcessKey(pid); + IProcessInfo processInfo = await _diagnosticServices.GetProcessAsync(new ProcessKey(endpointInfo.ProcessId), token); - IProcessInfo processInfo = await _diagnosticServices.GetProcessAsync(processKey, token); - - // Move into utility method in WebApi - string dumpFileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? - FormattableString.Invariant($"dump_{GetFileNameTimeStampUtcNow()}.dmp") : - FormattableString.Invariant($"core_{GetFileNameTimeStampUtcNow()}"); + string dumpFileName = DiagController.GenerateDumpFileName(); string dumpFilePath = ""; @@ -62,10 +42,7 @@ public async Task ExecuteAsync(CollectDumpOptions op } else { - // Move into utility method in WebApi - KeyValueLogScope scope = new KeyValueLogScope(); - scope.AddArtifactType(ArtifactType_Dump); - scope.AddEndpointInfo(processInfo.EndpointInfo); + KeyValueLogScope scope = DiagController.GetDumpScope(processInfo); dumpFilePath = await SendToEgress(new EgressOperation( token => _diagnosticServices.GetDump(processInfo, dumpType, token), @@ -85,26 +62,11 @@ public async Task ExecuteAsync(CollectDumpOptions op }; } - private static string GetFileNameTimeStampUtcNow() - { - return DateTime.UtcNow.ToString("yyyyMMdd_HHmmss"); - } - - // Look into IEgressService private async Task SendToEgress(EgressOperation egressStreamResult, CancellationToken token) { var result = await egressStreamResult.ExecuteAsync(_serviceProvider, token); - return result.Result.Value; // Not sure what this is, but is a string - - /* - // Will throw TooManyRequestsException if there are too many concurrent operations. - Guid operationId = await _operationsStore.AddOperation(egressStreamResult, limitKey); - string newUrl = egressStreamResult. - - return newUrl; // Switched to returning the URL so that we can include it in the CollectionRuleActionResult - //return Accepted(newUrl); - */ + return result.Result.Value; } } } From 47f4568d40e5d734a846c874c7970e034f2c608e Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Thu, 2 Sep 2021 15:36:28 -0700 Subject: [PATCH 10/22] Some more clean-up and commenting; decided not to add test for invalid egress provider due to built-in validation that is not used when running unit tests. Should enter PR soon. --- .../DumpTests.cs | 6 ------ .../IDumpTestInterface.cs | 9 --------- .../CollectDumpActionTests.cs | 18 ++++++++++-------- .../Actions/CollectDumpAction.cs | 1 + 4 files changed, 11 insertions(+), 23 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs index 636ee82d43e..3e61bffb1e9 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs @@ -10,14 +10,8 @@ using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Extensions.DependencyInjection; -using Microsoft.FileFormats; -using Microsoft.FileFormats.ELF; -using Microsoft.FileFormats.MachO; -using System; -using System.IO; using System.Net.Http; using System.Runtime.InteropServices; -using System.Threading; using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs index 5d25b862868..37fc059bf10 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs @@ -2,25 +2,16 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Microsoft.Diagnostics.Monitoring.TestCommon; using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; -using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Fixtures; -using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.HttpApi; -using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Runners; -using Microsoft.Diagnostics.Monitoring.WebApi; -using Microsoft.Diagnostics.Monitoring.WebApi.Models; -using Microsoft.Extensions.DependencyInjection; using Microsoft.FileFormats; using Microsoft.FileFormats.ELF; using Microsoft.FileFormats.MachO; using System; using System.IO; -using System.Net.Http; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Xunit; -using Xunit.Abstractions; namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests { diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index eebf9526a79..030234a17b5 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -11,13 +11,9 @@ using System.IO; using System.Globalization; using Xunit.Abstractions; -using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.Monitoring.TestCommon; -using System.Collections.Generic; using static Microsoft.Diagnostics.Monitoring.Tool.UnitTests.EndpointInfoSourceTests; using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; -using Microsoft.Diagnostics.Tools.Monitor; -using System.Reflection; using Microsoft.Diagnostics.Monitoring.TestCommon.Options; using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests; @@ -42,10 +38,11 @@ public CollectDumpActionTests(ITestOutputHelper outputHelper) [InlineData(DumpType.Mini)] [InlineData(DumpType.Triage)] [InlineData(DumpType.WithHeap)] - public async Task CollectDumpAction_FileEgressProvider(DumpType dumpType) + [InlineData(null)] + public async Task CollectDumpAction_Success(DumpType? dumpType) { const string ExpectedEgressProvider = "TmpEgressProvider"; - DumpType ExpectedDumpType = dumpType; + DumpType ExpectedDumpType = (dumpType != null) ? dumpType.Value : CollectDumpOptionsDefaults.Type; string uniqueEgressDirectory = TempEgressDirectory + Guid.NewGuid(); @@ -65,7 +62,12 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => CollectDumpOptions options = new(); options.Egress = ExpectedEgressProvider; - options.Type = ExpectedDumpType; + + // This is for the scenario where no DumpType is specified + if (dumpType != null) + { + options.Type = ExpectedDumpType; + } using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TestTimeouts.DumpTimeout); @@ -78,7 +80,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => var endpointInfos = await endpointInfoSourceTests.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); - AppRunner runner = endpointInfoSourceTests.CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60; should we test against multiple versions? + AppRunner runner = endpointInfoSourceTests.CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60; should we test against other frameworks? Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 3316338ab1a..42f5229f6d6 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -36,6 +36,7 @@ public async Task ExecuteAsync(CollectDumpOptions op string dumpFilePath = ""; + // Given our options validation, I believe this is probably redundant...should I remove it? if (string.IsNullOrEmpty(egressProvider)) { throw new ArgumentException("No Egress Provider was supplied."); From f67ff518af4680f2849c8e3ecabb50b7c88a7fe9 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Thu, 2 Sep 2021 16:15:13 -0700 Subject: [PATCH 11/22] Temp changes that should go in before PR; not passing tests though for unknown reasons. --- .../DumpTests.cs | 77 ------------------- .../HttpApi/ApiClient.cs | 2 +- .../CollectDumpActionTests.cs | 7 +- .../Actions/CollectDumpAction.cs | 23 ++++-- 4 files changed, 19 insertions(+), 90 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs index 3e61bffb1e9..9bef2fd04b9 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs @@ -69,82 +69,5 @@ public Task DumpTest(DiagnosticPortConnectionMode mode, DumpType type) } }); } - - /* - public static async Task ValidateDump(AppRunner runner, Stream dumpStream) - { - Assert.NotNull(dumpStream); - - byte[] headerBuffer = new byte[64]; - - // Read enough to deserialize the header. - int read; - int total = 0; - using CancellationTokenSource cancellation = new(TestTimeouts.DumpTimeout); - while (total < headerBuffer.Length && 0 != (read = await dumpStream.ReadAsync(headerBuffer, total, headerBuffer.Length - total, cancellation.Token))) - { - total += read; - } - Assert.Equal(headerBuffer.Length, total); - - // Read header and validate - using MemoryStream headerStream = new(headerBuffer); - - StreamAddressSpace dumpAddressSpace = new(headerStream); - Reader dumpReader = new(dumpAddressSpace); - - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - MinidumpHeader header = dumpReader.Read(0); - // Validate Signature - Assert.True(header.IsSignatureValid.Check()); - } - else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) - { - ELFHeaderIdent ident = dumpReader.Read(0); - Assert.True(ident.IsIdentMagicValid.Check()); - Assert.True(ident.IsClassValid.Check()); - Assert.True(ident.IsDataValid.Check()); - - LayoutManager layoutManager = new(); - layoutManager.AddELFTypes( - isBigEndian: ident.Data == ELFData.BigEndian, - is64Bit: ident.Class == ELFClass.Class64); - Reader headerReader = new(dumpAddressSpace, layoutManager); - - ELFHeader header = headerReader.Read(0); - // Validate Signature - Assert.True(header.IsIdentMagicValid.Check()); - // Validate ELF file is a core dump - Assert.Equal(ELFHeaderType.Core, header.Type); - } - else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) - { - if (runner.Environment.ContainsKey(EnableElfDumpOnMacOS)) - { - ELFHeader header = dumpReader.Read(0); - // Validate Signature - Assert.True(header.IsIdentMagicValid.Check()); - // Validate ELF file is a core dump - Assert.Equal(ELFHeaderType.Core, header.Type); - } - else - { - MachHeader header = dumpReader.Read(0); - // Validate Signature - Assert.True(header.IsMagicValid.Check()); - // Validate MachO file is a core dump - Assert.True(header.IsFileTypeValid.Check()); - Assert.Equal(MachHeaderFileType.Core, header.FileType); - } - } - else - { - throw new NotImplementedException("Dump header check not implemented for this OS platform."); - } - } - */ - - } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs index 330078d6cd8..f401f492407 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs @@ -79,7 +79,7 @@ public Task GetProcessAsync(int? pid, Guid? uid, string name, Cance return GetProcessAsync(GetProcessQuery(pid: pid, uid: uid, name: name), token); } - public async Task GetProcessAsync(string processQuery, CancellationToken token) + private async Task GetProcessAsync(string processQuery, CancellationToken token) { using HttpRequestMessage request = new(HttpMethod.Get, $"/process?" + processQuery); request.Headers.Add(HeaderNames.Accept, ContentTypes.ApplicationJson); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index 030234a17b5..09fc79e3e49 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -24,8 +24,8 @@ public sealed class CollectDumpActionTests { private const string DefaultRuleName = "Default"; private const string TempEgressDirectory = "/tmp"; + private const string ExpectedEgressProvider = "TmpEgressProvider"; - private IServiceProvider _serviceProvider; private ITestOutputHelper _outputHelper; public CollectDumpActionTests(ITestOutputHelper outputHelper) @@ -41,7 +41,6 @@ public CollectDumpActionTests(ITestOutputHelper outputHelper) [InlineData(null)] public async Task CollectDumpAction_Success(DumpType? dumpType) { - const string ExpectedEgressProvider = "TmpEgressProvider"; DumpType ExpectedDumpType = (dumpType != null) ? dumpType.Value : CollectDumpOptionsDefaults.Type; string uniqueEgressDirectory = TempEgressDirectory + Guid.NewGuid(); @@ -55,9 +54,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => rootOptions.AddFileSystemEgress(ExpectedEgressProvider, uniqueEgressDirectory); }, async host => { - _serviceProvider = host.Services; - - CollectDumpAction action = new(_serviceProvider); + CollectDumpAction action = new(host.Services); CollectDumpOptions options = new(); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 42f5229f6d6..53c31afc451 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -5,6 +5,7 @@ using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.Monitoring.WebApi.Controllers; using Microsoft.Diagnostics.Monitoring.WebApi.Models; +using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Exceptions; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; using Microsoft.Extensions.DependencyInjection; using System; @@ -39,19 +40,27 @@ public async Task ExecuteAsync(CollectDumpOptions op // Given our options validation, I believe this is probably redundant...should I remove it? if (string.IsNullOrEmpty(egressProvider)) { + // Also, I would move this to Strings.resx if we do keep it, but I decided to wait for feedback before doing that. throw new ArgumentException("No Egress Provider was supplied."); } else { KeyValueLogScope scope = DiagController.GetDumpScope(processInfo); - dumpFilePath = await SendToEgress(new EgressOperation( - token => _diagnosticServices.GetDump(processInfo, dumpType, token), - egressProvider, - dumpFileName, - processInfo.EndpointInfo, - ContentTypes.ApplicationOctetStream, - scope), token); + try + { + dumpFilePath = await SendToEgress(new EgressOperation( + token => _diagnosticServices.GetDump(processInfo, dumpType, token), + egressProvider, + dumpFileName, + processInfo.EndpointInfo, + ContentTypes.ApplicationOctetStream, + scope), token); + } + catch (Exception ex) + { + throw new CollectionRuleActionException(ex); + } } return new CollectionRuleActionResult() From 279c0e86f6e8b3df7e4e704eed57c3c8d1b45456 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Fri, 3 Sep 2021 08:04:58 -0700 Subject: [PATCH 12/22] Small tweaks; test command is succeeding (may have been an issue in VS) --- .../CollectDumpActionTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index 09fc79e3e49..c4a8e0063fc 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -22,7 +22,7 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { public sealed class CollectDumpActionTests { - private const string DefaultRuleName = "Default"; + //private const string DefaultRuleName = "Default"; private const string TempEgressDirectory = "/tmp"; private const string ExpectedEgressProvider = "TmpEgressProvider"; @@ -47,9 +47,9 @@ public async Task CollectDumpAction_Success(DumpType? dumpType) await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => { - rootOptions.CreateCollectionRule(DefaultRuleName) - .SetStartupTrigger() - .AddCollectDumpAction(ExpectedDumpType, ExpectedEgressProvider); + //rootOptions.CreateCollectionRule(DefaultRuleName) + //.SetStartupTrigger(); + //.AddCollectDumpAction(ExpectedDumpType, ExpectedEgressProvider); rootOptions.AddFileSystemEgress(ExpectedEgressProvider, uniqueEgressDirectory); }, async host => From 3e90a0069adc29e4f9b01182506665e1bdc1aa82 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Fri, 3 Sep 2021 13:10:26 -0700 Subject: [PATCH 13/22] Removed an extraneous comment. --- .../CollectDumpActionTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index b434eccce74..4a2d6058044 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -22,7 +22,6 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { public sealed class CollectDumpActionTests { - //private const string DefaultRuleName = "Default"; private const string TempEgressDirectory = "/tmp"; private const string ExpectedEgressProvider = "TmpEgressProvider"; From 2e9231304ade508a22806550c9b21e8d01ddfc31 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Tue, 7 Sep 2021 14:57:55 -0700 Subject: [PATCH 14/22] Significant refactoring for Wiktor and Justin; overall functionality should be consistent with the initial commit. --- .../Controllers/DiagController.cs | 34 +-- .../DiagnosticServices.cs | 51 ----- .../DumpService.cs | 87 ++++++++ .../IDiagnosticServices.cs | 2 +- .../IDumpService.cs | 15 ++ .../Utilities.cs | 24 +++ .../CommonTestTimeouts.cs | 8 + .../DumpTestUtilities.cs} | 13 +- ...t.Diagnostics.Monitoring.TestCommon.csproj | 7 + .../DumpTests.cs | 6 +- .../HttpApi/ApiClientExtensions.cs | 3 +- .../TestTimeouts.cs | 10 +- .../CollectDumpActionTests.cs | 21 +- .../EndpointInfoSourceTests.cs | 195 ++---------------- .../EndpointUtilities.cs | 189 +++++++++++++++++ ...agnostics.Monitoring.Tool.UnitTests.csproj | 1 - .../TestHostHelper.cs | 11 +- .../Actions/CollectDumpAction.cs | 33 ++- .../ConfigurationBuilderExtensions.cs | 10 + .../DiagnosticsMonitorCommandHandler.cs | 11 +- 20 files changed, 411 insertions(+), 320 deletions(-) create mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs create mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/IDumpService.cs create mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs rename src/Tests/{Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs => Microsoft.Diagnostics.Monitoring.TestCommon/DumpTestUtilities.cs} (88%) create mode 100644 src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 20a34447cc0..1b210af9184 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -52,6 +52,7 @@ public class DiagController : ControllerBase private readonly IDiagnosticServices _diagnosticServices; private readonly IOptions _diagnosticPortOptions; private readonly EgressOperationStore _operationsStore; + private readonly IDumpService _dumpService; public DiagController(ILogger logger, IServiceProvider serviceProvider) @@ -60,6 +61,7 @@ public DiagController(ILogger logger, _diagnosticServices = serviceProvider.GetRequiredService(); _diagnosticPortOptions = serviceProvider.GetService>(); _operationsStore = serviceProvider.GetRequiredService(); + _dumpService = serviceProvider.GetRequiredService(); } /// @@ -211,11 +213,11 @@ public Task CaptureDump( return InvokeForProcess(async processInfo => { - string dumpFileName = GenerateDumpFileName(); + string dumpFileName = Utilities.GenerateDumpFileName(); if (string.IsNullOrEmpty(egressProvider)) { - Stream dumpStream = await _diagnosticServices.GetDump(processInfo, type, HttpContext.RequestAborted); + Stream dumpStream = await _dumpService.DumpAsync(processInfo.EndpointInfo, type, HttpContext.RequestAborted); _logger.WrittenToHttpStream(); //Compression is done automatically by the response @@ -224,12 +226,10 @@ public Task CaptureDump( } else { - KeyValueLogScope scope = new KeyValueLogScope(); - scope.AddArtifactType(ArtifactType_Dump); - scope.AddEndpointInfo(processInfo.EndpointInfo); + KeyValueLogScope scope = GetDumpScope(processInfo.EndpointInfo); return await SendToEgress(new EgressOperation( - token => _diagnosticServices.GetDump(processInfo, type, token), + token => _dumpService.DumpAsync(processInfo.EndpointInfo, type, token), egressProvider, dumpFileName, processInfo.EndpointInfo, @@ -270,7 +270,7 @@ public Task CaptureGcDump( return InvokeForProcess(processInfo => { - string fileName = FormattableString.Invariant($"{GetFileNameTimeStampUtcNow()}_{processInfo.EndpointInfo.ProcessId}.gcdump"); + string fileName = FormattableString.Invariant($"{Utilities.GetFileNameTimeStampUtcNow()}_{processInfo.EndpointInfo.ProcessId}.gcdump"); Func> action = async (token) => { var graph = new Graphs.MemoryGraph(50_000); @@ -582,18 +582,11 @@ private static string GetDotnetMonitorVersion() } } - internal static string GenerateDumpFileName() - { - return RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? - FormattableString.Invariant($"dump_{GetFileNameTimeStampUtcNow()}.dmp") : - FormattableString.Invariant($"core_{GetFileNameTimeStampUtcNow()}"); - } - - internal static KeyValueLogScope GetDumpScope(IProcessInfo processInfo) + internal static KeyValueLogScope GetDumpScope(IEndpointInfo endpointInfo) { KeyValueLogScope scope = new KeyValueLogScope(); scope.AddArtifactType(ArtifactType_Dump); - scope.AddEndpointInfo(processInfo.EndpointInfo); + scope.AddEndpointInfo(endpointInfo); return scope; } @@ -614,7 +607,7 @@ private Task StartTrace( TimeSpan duration, string egressProvider) { - string fileName = FormattableString.Invariant($"{GetFileNameTimeStampUtcNow()}_{processInfo.EndpointInfo.ProcessId}.nettrace"); + string fileName = FormattableString.Invariant($"{Utilities.GetFileNameTimeStampUtcNow()}_{processInfo.EndpointInfo.ProcessId}.nettrace"); Func action = async (outputStream, token) => { @@ -656,7 +649,7 @@ private Task StartLogs( return Task.FromResult(this.NotAcceptable()); } - string fileName = FormattableString.Invariant($"{GetFileNameTimeStampUtcNow()}_{processInfo.EndpointInfo.ProcessId}.txt"); + string fileName = FormattableString.Invariant($"{Utilities.GetFileNameTimeStampUtcNow()}_{processInfo.EndpointInfo.ProcessId}.txt"); string contentType = ContentTypes.TextEventStream; if (format == LogFormat.EventStream) @@ -706,11 +699,6 @@ private static TimeSpan ConvertSecondsToTimeSpan(int durationSeconds) return (pid == null && uid == null && name == null) ? null : new ProcessKey(pid, uid, name); } - private static string GetFileNameTimeStampUtcNow() - { - return DateTime.UtcNow.ToString("yyyyMMdd_HHmmss"); - } - private static LogFormat ComputeLogFormat(IList acceptedHeaders) { if (acceptedHeaders == null) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/DiagnosticServices.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/DiagnosticServices.cs index 0e0950fdb2c..1d2546ed5e8 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/DiagnosticServices.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/DiagnosticServices.cs @@ -4,8 +4,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; -using System.Globalization; using System.IO; using System.Linq; using System.Runtime.InteropServices; @@ -29,15 +27,12 @@ internal sealed class DiagnosticServices : IDiagnosticServices private readonly IEndpointInfoSourceInternal _endpointInfoSource; private readonly CancellationTokenSource _tokenSource = new CancellationTokenSource(); - private readonly IOptionsMonitor _storageOptions; private readonly IOptionsMonitor _defaultProcessOptions; public DiagnosticServices(IEndpointInfoSource endpointInfoSource, - IOptionsMonitor storageOptions, IOptionsMonitor defaultProcessMonitor) { _endpointInfoSource = (IEndpointInfoSourceInternal)endpointInfoSource; - _storageOptions = storageOptions; _defaultProcessOptions = defaultProcessMonitor; } @@ -81,51 +76,6 @@ public async Task> GetProcessesAsync(DiagProcessFilter return processes.ToArray(); } - public async Task GetDump(IProcessInfo pi, Models.DumpType mode, CancellationToken token) - { - string dumpFilePath = Path.Combine(_storageOptions.CurrentValue.DumpTempFolder, FormattableString.Invariant($"{Guid.NewGuid()}_{pi.EndpointInfo.ProcessId}")); - NETCore.Client.DumpType dumpType = MapDumpType(mode); - - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - // Get the process - Process process = Process.GetProcessById(pi.EndpointInfo.ProcessId); - await Dumper.CollectDumpAsync(process, dumpFilePath, dumpType); - } - else - { - await Task.Run(() => - { - var client = new DiagnosticsClient(pi.EndpointInfo.Endpoint); - client.WriteDump(dumpType, dumpFilePath); - }); - } - - return new AutoDeleteFileStream(dumpFilePath); - } - - private static DumpType MapDumpType(Models.DumpType dumpType) - { - switch (dumpType) - { - case Models.DumpType.Full: - return DumpType.Full; - case Models.DumpType.WithHeap: - return DumpType.WithHeap; - case Models.DumpType.Triage: - return DumpType.Triage; - case Models.DumpType.Mini: - return DumpType.Normal; - default: - throw new ArgumentException( - string.Format( - CultureInfo.InvariantCulture, - Strings.ErrorMessage_UnexpectedType, - nameof(DumpType), - dumpType), - nameof(dumpType)); - } - } public Task GetProcessAsync(ProcessKey? processKey, CancellationToken token) { @@ -182,7 +132,6 @@ public AutoDeleteFileStream(string path) : base(path, FileMode.Open, FileAccess. public override bool CanSeek => false; } - private sealed class ProcessInfo : IProcessInfo { // String returned for a process field when its value could not be retrieved. This is the same diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs new file mode 100644 index 00000000000..651766ab9e4 --- /dev/null +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs @@ -0,0 +1,87 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.Diagnostics.NETCore.Client; +using Microsoft.Extensions.Options; +using System; +using System.Diagnostics; +using System.Globalization; +using System.IO; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Diagnostics.Monitoring.WebApi +{ + internal class DumpService : IDumpService + { + private readonly IOptionsMonitor _storageOptions; + + public DumpService(IOptionsMonitor storageOptions) + { + _storageOptions = storageOptions; + } + + public async Task DumpAsync(IEndpointInfo endpointInfo, Models.DumpType mode, CancellationToken token) + { + string dumpFilePath = Path.Combine(_storageOptions.CurrentValue.DumpTempFolder, FormattableString.Invariant($"{Guid.NewGuid()}_{endpointInfo.ProcessId}")); + NETCore.Client.DumpType dumpType = MapDumpType(mode); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + // Get the process + Process process = Process.GetProcessById(endpointInfo.ProcessId); + await Dumper.CollectDumpAsync(process, dumpFilePath, dumpType); + } + else + { + await Task.Run(() => + { + var client = new DiagnosticsClient(endpointInfo.Endpoint); + client.WriteDump(dumpType, dumpFilePath); + }); + } + + return new AutoDeleteFileStream(dumpFilePath); + } + + /// + /// We want to make sure we destroy files we finish streaming. + /// We want to make sure that we stream out files since we compress on the fly; the size cannot be known upfront. + /// CONSIDER The above implies knowledge of how the file is used by the rest api. + /// + private sealed class AutoDeleteFileStream : FileStream + { + public AutoDeleteFileStream(string path) : base(path, FileMode.Open, FileAccess.ReadWrite, FileShare.ReadWrite | FileShare.Delete, + bufferSize: 4096, FileOptions.DeleteOnClose) + { + } + + public override bool CanSeek => false; + } + + private static DumpType MapDumpType(Models.DumpType dumpType) + { + switch (dumpType) + { + case Models.DumpType.Full: + return DumpType.Full; + case Models.DumpType.WithHeap: + return DumpType.WithHeap; + case Models.DumpType.Triage: + return DumpType.Triage; + case Models.DumpType.Mini: + return DumpType.Normal; + default: + throw new ArgumentException( + string.Format( + CultureInfo.InvariantCulture, + Strings.ErrorMessage_UnexpectedType, + nameof(DumpType), + dumpType), + nameof(dumpType)); + } + } + } +} diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/IDiagnosticServices.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/IDiagnosticServices.cs index ccde596bd00..8b4a2940600 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/IDiagnosticServices.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/IDiagnosticServices.cs @@ -31,7 +31,7 @@ internal interface IDiagnosticServices : IDisposable /// Task GetProcessAsync(ProcessKey? processKey, CancellationToken token); - Task GetDump(IProcessInfo pi, Models.DumpType mode, CancellationToken token); + //Task GetDump(IProcessInfo pi, Models.DumpType mode, CancellationToken token); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/IDumpService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/IDumpService.cs new file mode 100644 index 00000000000..c914b1c1690 --- /dev/null +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/IDumpService.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Diagnostics.Monitoring.WebApi +{ + internal interface IDumpService + { + Task DumpAsync(IEndpointInfo endpointInfo, Models.DumpType mode, CancellationToken token); + } +} diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs new file mode 100644 index 00000000000..1b8eb6a0cf6 --- /dev/null +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.InteropServices; + +namespace Microsoft.Diagnostics.Monitoring.WebApi +{ + public class Utilities + { + internal static string GenerateDumpFileName() + { + return RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? + FormattableString.Invariant($"dump_{GetFileNameTimeStampUtcNow()}.dmp") : + FormattableString.Invariant($"core_{GetFileNameTimeStampUtcNow()}"); + } + + internal static string GetFileNameTimeStampUtcNow() + { + return DateTime.UtcNow.ToString("yyyyMMdd_HHmmss"); + } + } +} diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/CommonTestTimeouts.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/CommonTestTimeouts.cs index fd269a050a7..d2203a0dc36 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/CommonTestTimeouts.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/CommonTestTimeouts.cs @@ -22,5 +22,13 @@ public static class CommonTestTimeouts /// Default timeout for waiting for an executable to exit. /// public static readonly TimeSpan WaitForExit = TimeSpan.FromSeconds(15); + + /// + /// Default timeout for dump collection. + /// + /// + /// Dumps (especially full dumps) can be quite large and take a significant amount of time to transfer. + /// + public static readonly TimeSpan DumpTimeout = TimeSpan.FromMinutes(3); } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/DumpTestUtilities.cs similarity index 88% rename from src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs rename to src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/DumpTestUtilities.cs index 37fc059bf10..345a09b1683 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/IDumpTestInterface.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/DumpTestUtilities.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; using Microsoft.FileFormats; using Microsoft.FileFormats.ELF; using Microsoft.FileFormats.MachO; @@ -13,13 +12,13 @@ using System.Threading.Tasks; using Xunit; -namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests +namespace Microsoft.Diagnostics.Monitoring.TestCommon { - public interface IDumpTestInterface + public static class DumpTestUtilities { - internal const string EnableElfDumpOnMacOS = "COMPlus_DbgEnableElfDumpOnMacOS"; + public const string EnableElfDumpOnMacOS = "COMPlus_DbgEnableElfDumpOnMacOS"; - public static async Task ValidateDump(AppRunner runner, Stream dumpStream) + public static async Task ValidateDump(bool expectElfDump, Stream dumpStream) { Assert.NotNull(dumpStream); @@ -28,7 +27,7 @@ public static async Task ValidateDump(AppRunner runner, Stream dumpStream) // Read enough to deserialize the header. int read; int total = 0; - using CancellationTokenSource cancellation = new(TestTimeouts.DumpTimeout); + using CancellationTokenSource cancellation = new(CommonTestTimeouts.DumpTimeout); while (total < headerBuffer.Length && 0 != (read = await dumpStream.ReadAsync(headerBuffer, total, headerBuffer.Length - total, cancellation.Token))) { total += read; @@ -68,7 +67,7 @@ public static async Task ValidateDump(AppRunner runner, Stream dumpStream) } else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { - if (runner.Environment.ContainsKey(EnableElfDumpOnMacOS)) + if (expectElfDump) { ELFHeader header = dumpReader.Read(0); // Validate Signature diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj index de52c842696..57a61e20ea8 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj @@ -11,6 +11,13 @@ + + + + + + + diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs index 35a3fc6d8f8..55bdfacb088 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/DumpTests.cs @@ -19,7 +19,7 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests { [Collection(DefaultCollectionFixture.Name)] - public class DumpTests : IDumpTestInterface + public class DumpTests { private readonly IHttpClientFactory _httpClientFactory; private readonly ITestOutputHelper _outputHelper; @@ -58,7 +58,7 @@ public Task DumpTest(DiagnosticPortConnectionMode mode, DumpType type) using ResponseStreamHolder holder = await client.CaptureDumpAsync(processId, type); Assert.NotNull(holder); - await IDumpTestInterface.ValidateDump(runner, holder.Stream); + await DumpTestUtilities.ValidateDump(runner.Environment.ContainsKey(DumpTestUtilities.EnableElfDumpOnMacOS), holder.Stream); await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }, @@ -67,7 +67,7 @@ public Task DumpTest(DiagnosticPortConnectionMode mode, DumpType type) // MachO not supported on .NET 5, only ELF: https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/xplat-minidump-generation.md#os-x if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && DotNetHost.RuntimeVersion.Major == 5) { - runner.Environment.Add(IDumpTestInterface.EnableElfDumpOnMacOS, "1"); + runner.Environment.Add(DumpTestUtilities.EnableElfDumpOnMacOS, "1"); } }); } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs index 7774f5b086d..d7802ba4875 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using Microsoft.Diagnostics.Monitoring.TestCommon; using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Extensions.Logging; @@ -206,7 +207,7 @@ public static async Task> GetProcessEnvironmentAsync( /// public static Task CaptureDumpAsync(this ApiClient client, int pid, DumpType dumpType) { - return client.CaptureDumpAsync(pid, dumpType, TestTimeouts.DumpTimeout); + return client.CaptureDumpAsync(pid, dumpType, CommonTestTimeouts.DumpTimeout); } /// diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/TestTimeouts.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/TestTimeouts.cs index 87e4aa4a2f6..98bbc6f65eb 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/TestTimeouts.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/TestTimeouts.cs @@ -6,7 +6,7 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests { - public static class TestTimeouts + internal static class TestTimeouts { /// /// Default timeout for HTTP API calls @@ -18,14 +18,6 @@ public static class TestTimeouts /// public static readonly TimeSpan LogsDuration = TimeSpan.FromSeconds(10); - /// - /// Default timeout for dump collection. - /// - /// - /// Dumps (especially full dumps) can be quite large and take a significant amount of time to transfer. - /// - public static readonly TimeSpan DumpTimeout = TimeSpan.FromMinutes(3); - /// /// Timeout for polling a long running operation to completion. /// This may need to be adjusted for individual calls that are longer than 30 seconds. diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index 4a2d6058044..6076adaf567 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -12,11 +12,10 @@ using System.Globalization; using Xunit.Abstractions; using Microsoft.Diagnostics.Monitoring.TestCommon; -using static Microsoft.Diagnostics.Monitoring.Tool.UnitTests.EndpointInfoSourceTests; using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; using Microsoft.Diagnostics.Monitoring.TestCommon.Options; using Microsoft.Diagnostics.Monitoring.WebApi.Models; -using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests; +using static Microsoft.Diagnostics.Monitoring.Tool.UnitTests.EndpointUtilities; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { @@ -26,10 +25,12 @@ public sealed class CollectDumpActionTests private const string ExpectedEgressProvider = "TmpEgressProvider"; private ITestOutputHelper _outputHelper; + private readonly EndpointUtilities _endpointUtilities; public CollectDumpActionTests(ITestOutputHelper outputHelper) { _outputHelper = outputHelper; + _endpointUtilities = new(_outputHelper); } [Theory] @@ -61,16 +62,14 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => options.Type = ExpectedDumpType; } - EndpointInfoSourceTests endpointInfoSourceTests = new(_outputHelper); - ServerEndpointInfoCallback callback = new(_outputHelper); - await using var source = endpointInfoSourceTests.CreateServerSource(out string transportName, callback); + await using var source = _endpointUtilities.CreateServerSource(out string transportName, callback); source.Start(); - var endpointInfos = await endpointInfoSourceTests.GetEndpointInfoAsync(source); + var endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); - AppRunner runner = endpointInfoSourceTests.CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60; should we test against other frameworks? + AppRunner runner = _endpointUtilities.CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60; should we test against other frameworks? using CancellationTokenSource callbackCancellation = new(CommonTestTimeouts.StartProcess); Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, callbackCancellation.Token); @@ -79,7 +78,7 @@ await runner.ExecuteAsync(async () => { await newEndpointInfoTask; - endpointInfos = await endpointInfoSourceTests.GetEndpointInfoAsync(source); + endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); var endpointInfo = Assert.Single(endpointInfos); Assert.NotNull(endpointInfo.CommandLine); @@ -87,7 +86,7 @@ await runner.ExecuteAsync(async () => Assert.NotNull(endpointInfo.ProcessArchitecture); await VerifyConnectionAsync(runner, endpointInfo); - using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(TestTimeouts.DumpTimeout); + using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(CommonTestTimeouts.DumpTimeout); CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); string egressPath = result.OutputValues["EgressPath"]; @@ -103,7 +102,7 @@ await runner.ExecuteAsync(async () => Stream dumpStream = reader.BaseStream; Assert.NotNull(dumpStream); - await IDumpTestInterface.ValidateDump(runner, dumpStream); + await DumpTestUtilities.ValidateDump(runner.Environment.ContainsKey(DumpTestUtilities.EnableElfDumpOnMacOS), dumpStream); } } @@ -112,7 +111,7 @@ await runner.ExecuteAsync(async () => await Task.Delay(TimeSpan.FromSeconds(1)); - endpointInfos = await endpointInfoSourceTests.GetEndpointInfoAsync(source); + endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs index 3aa95f595ec..2e30b2cd446 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs @@ -5,15 +5,14 @@ using Microsoft.Diagnostics.Monitoring.TestCommon; using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; using Microsoft.Diagnostics.Monitoring.WebApi; -using Microsoft.Diagnostics.Tools.Monitor; using System; using System.Collections.Generic; using System.Linq; -using System.Reflection; using System.Threading; using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; +using static Microsoft.Diagnostics.Monitoring.Tool.UnitTests.EndpointUtilities; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { @@ -21,13 +20,13 @@ public class EndpointInfoSourceTests { private static readonly TimeSpan DefaultNegativeVerificationTimeout = TimeSpan.FromSeconds(2); - private static readonly TimeSpan GetEndpointInfoTimeout = TimeSpan.FromSeconds(10); - private readonly ITestOutputHelper _outputHelper; + private readonly EndpointUtilities _endpointUtilities; public EndpointInfoSourceTests(ITestOutputHelper outputHelper) { _outputHelper = outputHelper; + _endpointUtilities = new EndpointUtilities(_outputHelper); } /// @@ -37,7 +36,7 @@ public EndpointInfoSourceTests(ITestOutputHelper outputHelper) [Fact] public async Task ServerSourceNoStartTest() { - await using var source = CreateServerSource(out string transportName); + await using var source = _endpointUtilities.CreateServerSource(out string transportName); // Intentionally do not call Start using CancellationTokenSource cancellation = new(DefaultNegativeVerificationTimeout); @@ -52,10 +51,10 @@ await Assert.ThrowsAsync( [Fact] public async Task ServerSourceNoConnectionsTest() { - await using var source = CreateServerSource(out _); + await using var source = _endpointUtilities.CreateServerSource(out _); source.Start(); - var endpointInfos = await GetEndpointInfoAsync(source); + var endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); } @@ -66,7 +65,7 @@ public async Task ServerSourceNoConnectionsTest() [Fact] public async Task ServerSourceThrowsWhenDisposedTest() { - var source = CreateServerSource(out _); + var source = _endpointUtilities.CreateServerSource(out _); source.Start(); await source.DisposeAsync(); @@ -91,7 +90,7 @@ await Assert.ThrowsAsync( [Fact] public async Task ServerSourceThrowsWhenMultipleStartTest() { - await using var source = CreateServerSource(out _); + await using var source = _endpointUtilities.CreateServerSource(out _); source.Start(); Assert.Throws( @@ -110,13 +109,13 @@ public async Task ServerSourceThrowsWhenMultipleStartTest() public async Task ServerSourceAddRemoveSingleConnectionTest(TargetFrameworkMoniker appTfm) { ServerEndpointInfoCallback callback = new(_outputHelper); - await using var source = CreateServerSource(out string transportName, callback); + await using var source = _endpointUtilities.CreateServerSource(out string transportName, callback); source.Start(); - var endpointInfos = await GetEndpointInfoAsync(source); + var endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); - AppRunner runner = CreateAppRunner(transportName, appTfm); + AppRunner runner = _endpointUtilities.CreateAppRunner(transportName, appTfm); using CancellationTokenSource cancellation = new(CommonTestTimeouts.StartProcess); Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, cancellation.Token); @@ -125,7 +124,7 @@ await runner.ExecuteAsync(async () => { await newEndpointInfoTask; - endpointInfos = await GetEndpointInfoAsync(source); + endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); var endpointInfo = Assert.Single(endpointInfos); Assert.NotNull(endpointInfo.CommandLine); @@ -138,7 +137,7 @@ await runner.ExecuteAsync(async () => await Task.Delay(TimeSpan.FromSeconds(1)); - endpointInfos = await GetEndpointInfoAsync(source); + endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); } @@ -152,10 +151,10 @@ await runner.ExecuteAsync(async () => public async Task ServerSourceAddRemoveMultipleConnectionTest(TargetFrameworkMoniker appTfm) { ServerEndpointInfoCallback callback = new(_outputHelper); - await using var source = CreateServerSource(out string transportName, callback); + await using var source = _endpointUtilities.CreateServerSource(out string transportName, callback); source.Start(); - var endpointInfos = await GetEndpointInfoAsync(source); + var endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); const int appCount = 5; @@ -166,7 +165,7 @@ public async Task ServerSourceAddRemoveMultipleConnectionTest(TargetFrameworkMon using CancellationTokenSource cancellation = new(CommonTestTimeouts.StartProcess); for (int i = 0; i < appCount; i++) { - runners[i] = CreateAppRunner(transportName, appTfm, appId: i + 1); + runners[i] = _endpointUtilities.CreateAppRunner(transportName, appTfm, appId: i + 1); newEndpointInfoTasks[i] = callback.WaitForNewEndpointInfoAsync(runners[i], cancellation.Token); } @@ -176,7 +175,7 @@ await runners.ExecuteAsync(async () => await Task.WhenAll(newEndpointInfoTasks); _outputHelper.WriteLine("Received all new endpoint info notifications."); - endpointInfos = await GetEndpointInfoAsync(source); + endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); Assert.Equal(appCount, endpointInfos.Count()); @@ -203,7 +202,7 @@ await runners.ExecuteAsync(async () => await Task.Delay(TimeSpan.FromSeconds(1)); - endpointInfos = await GetEndpointInfoAsync(source); + endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); Assert.Empty(endpointInfos); } @@ -213,163 +212,5 @@ public static IEnumerable GetTfmsSupportingPortListener() yield return new object[] { TargetFrameworkMoniker.Net50 }; yield return new object[] { TargetFrameworkMoniker.Net60 }; } - - internal ServerEndpointInfoSource CreateServerSource(out string transportName, ServerEndpointInfoCallback callback = null) - { - DiagnosticPortHelper.Generate(DiagnosticPortConnectionMode.Listen, out _, out transportName); - _outputHelper.WriteLine("Starting server endpoint info source at '" + transportName + "'."); - - List callbacks = new(); - if (null != callback) - { - callbacks.Add(callback); - } - return new ServerEndpointInfoSource(transportName, callbacks); - } - - internal AppRunner CreateAppRunner(string transportName, TargetFrameworkMoniker tfm, int appId = 1) - { - AppRunner appRunner = new(_outputHelper, Assembly.GetExecutingAssembly(), appId, tfm); - appRunner.ConnectionMode = DiagnosticPortConnectionMode.Connect; - appRunner.DiagnosticPortPath = transportName; - appRunner.ScenarioName = TestAppScenarios.AsyncWait.Name; - return appRunner; - } - - internal async Task> GetEndpointInfoAsync(ServerEndpointInfoSource source) - { - _outputHelper.WriteLine("Getting endpoint infos."); - using CancellationTokenSource cancellationSource = new(GetEndpointInfoTimeout); - return await source.GetEndpointInfoAsync(cancellationSource.Token); - } - - /// - /// Verifies basic information on the connection and that it matches the target process from the runner. - /// - internal static async Task VerifyConnectionAsync(AppRunner runner, IEndpointInfo endpointInfo) - { - Assert.NotNull(runner); - Assert.NotNull(endpointInfo); - Assert.Equal(await runner.ProcessIdTask, endpointInfo.ProcessId); - Assert.NotEqual(Guid.Empty, endpointInfo.RuntimeInstanceCookie); - Assert.NotNull(endpointInfo.Endpoint); - } - - internal sealed class ServerEndpointInfoCallback : IEndpointInfoSourceCallbacks - { - private readonly ITestOutputHelper _outputHelper; - /// - /// Use to protect the completion list from mutation while processing - /// callbacks from it. The processing is done in an async method with async - /// calls, which are not allowed in a lock, thus use SemaphoreSlim. - /// - private readonly SemaphoreSlim _completionEntriesSemaphore = new(1); - private readonly List _completionEntries = new(); - - public ServerEndpointInfoCallback(ITestOutputHelper outputHelper) - { - _outputHelper = outputHelper; - } - - public async Task WaitForNewEndpointInfoAsync(AppRunner runner, CancellationToken token) - { - CompletionEntry entry = new(runner); - using var _ = token.Register(() => entry.CompletionSource.TrySetCanceled(token)); - - await _completionEntriesSemaphore.WaitAsync(token); - try - { - _completionEntries.Add(entry); - _outputHelper.WriteLine($"[Wait] Register App{runner.AppId}"); - } - finally - { - _completionEntriesSemaphore.Release(); - } - - _outputHelper.WriteLine($"[Wait] Wait for App{runner.AppId} notification"); - IEndpointInfo endpointInfo = await entry.CompletionSource.Task; - _outputHelper.WriteLine($"[Wait] Received App{runner.AppId} notification"); - - return endpointInfo; - } - - public Task OnBeforeResumeAsync(IEndpointInfo endpointInfo, CancellationToken token) - { - return Task.CompletedTask; - } - - public async Task OnAddedEndpointInfoAsync(IEndpointInfo info, CancellationToken token) - { - _outputHelper.WriteLine($"[Source] Added: {ToOutputString(info)}"); - - await _completionEntriesSemaphore.WaitAsync(token); - try - { - _outputHelper.WriteLine($"[Source] Start notifications for process {info.ProcessId}"); - - // Create a mapping of the process ID tasks to the completion entries - IDictionary, CompletionEntry> map = new Dictionary, CompletionEntry>(_completionEntries.Count); - foreach (CompletionEntry entry in _completionEntries) - { - map.Add(entry.Runner.ProcessIdTask.WithCancellation(token), entry); - } - - while (map.Count > 0) - { - // Wait for any of the process ID tasks to complete. - Task completedTask = await Task.WhenAny(map.Keys); - - map.Remove(completedTask, out CompletionEntry entry); - - _outputHelper.WriteLine($"[Source] Checking App{entry.Runner.AppId}"); - - if (completedTask.IsCompletedSuccessfully) - { - // If the process ID matches the one that was reported via the callback, - // then signal its completion source. - if (info.ProcessId == completedTask.Result) - { - _outputHelper.WriteLine($"[Source] Notifying App{entry.Runner.AppId}"); - entry.CompletionSource.TrySetResult(info); - - _completionEntries.Remove(entry); - - break; - } - } - } - - _outputHelper.WriteLine($"[Source] Finished notifications for process {info.ProcessId}"); - } - finally - { - _completionEntriesSemaphore.Release(); - } - } - - public void OnRemovedEndpointInfo(IEndpointInfo info) - { - _outputHelper.WriteLine($"[Source] Removed: {ToOutputString(info)}"); - } - - private static string ToOutputString(IEndpointInfo info) - { - return FormattableString.Invariant($"PID={info.ProcessId}, Cookie={info.RuntimeInstanceCookie}"); - } - - private sealed class CompletionEntry - { - public CompletionEntry(AppRunner runner) - { - Runner = runner; - CompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - } - - public AppRunner Runner { get; } - - public TaskCompletionSource CompletionSource { get; } - } - } } } \ No newline at end of file diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs new file mode 100644 index 00000000000..0fbe125f854 --- /dev/null +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs @@ -0,0 +1,189 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.Diagnostics.Monitoring.TestCommon; +using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; +using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Diagnostics.Tools.Monitor; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests +{ + internal class EndpointUtilities + { + private readonly ITestOutputHelper _outputHelper; + + private static readonly TimeSpan GetEndpointInfoTimeout = TimeSpan.FromSeconds(10); + + public EndpointUtilities(ITestOutputHelper outputHelper) + { + _outputHelper = outputHelper; + } + + public ServerEndpointInfoSource CreateServerSource(out string transportName, ServerEndpointInfoCallback callback = null) + { + DiagnosticPortHelper.Generate(DiagnosticPortConnectionMode.Listen, out _, out transportName); + _outputHelper.WriteLine("Starting server endpoint info source at '" + transportName + "'."); + + List callbacks = new(); + if (null != callback) + { + callbacks.Add(callback); + } + return new ServerEndpointInfoSource(transportName, callbacks); + } + + public AppRunner CreateAppRunner(string transportName, TargetFrameworkMoniker tfm, int appId = 1) + { + AppRunner appRunner = new(_outputHelper, Assembly.GetExecutingAssembly(), appId, tfm); + appRunner.ConnectionMode = DiagnosticPortConnectionMode.Connect; + appRunner.DiagnosticPortPath = transportName; + appRunner.ScenarioName = TestAppScenarios.AsyncWait.Name; + return appRunner; + } + + public async Task> GetEndpointInfoAsync(ServerEndpointInfoSource source) + { + _outputHelper.WriteLine("Getting endpoint infos."); + using CancellationTokenSource cancellationSource = new(GetEndpointInfoTimeout); + return await source.GetEndpointInfoAsync(cancellationSource.Token); + } + + /// + /// Verifies basic information on the connection and that it matches the target process from the runner. + /// + public static async Task VerifyConnectionAsync(AppRunner runner, IEndpointInfo endpointInfo) + { + Assert.NotNull(runner); + Assert.NotNull(endpointInfo); + Assert.Equal(await runner.ProcessIdTask, endpointInfo.ProcessId); + Assert.NotEqual(Guid.Empty, endpointInfo.RuntimeInstanceCookie); + Assert.NotNull(endpointInfo.Endpoint); + } + + public sealed class ServerEndpointInfoCallback : IEndpointInfoSourceCallbacks + { + private readonly ITestOutputHelper _outputHelper; + /// + /// Use to protect the completion list from mutation while processing + /// callbacks from it. The processing is done in an async method with async + /// calls, which are not allowed in a lock, thus use SemaphoreSlim. + /// + private readonly SemaphoreSlim _completionEntriesSemaphore = new(1); + private readonly List _completionEntries = new(); + + public ServerEndpointInfoCallback(ITestOutputHelper outputHelper) + { + _outputHelper = outputHelper; + } + + public async Task WaitForNewEndpointInfoAsync(AppRunner runner, CancellationToken token) + { + CompletionEntry entry = new(runner); + using var _ = token.Register(() => entry.CompletionSource.TrySetCanceled(token)); + + await _completionEntriesSemaphore.WaitAsync(token); + try + { + _completionEntries.Add(entry); + _outputHelper.WriteLine($"[Wait] Register App{runner.AppId}"); + } + finally + { + _completionEntriesSemaphore.Release(); + } + + _outputHelper.WriteLine($"[Wait] Wait for App{runner.AppId} notification"); + IEndpointInfo endpointInfo = await entry.CompletionSource.Task; + _outputHelper.WriteLine($"[Wait] Received App{runner.AppId} notification"); + + return endpointInfo; + } + + public Task OnBeforeResumeAsync(IEndpointInfo endpointInfo, CancellationToken token) + { + return Task.CompletedTask; + } + + public async Task OnAddedEndpointInfoAsync(IEndpointInfo info, CancellationToken token) + { + _outputHelper.WriteLine($"[Source] Added: {ToOutputString(info)}"); + + await _completionEntriesSemaphore.WaitAsync(token); + try + { + _outputHelper.WriteLine($"[Source] Start notifications for process {info.ProcessId}"); + + // Create a mapping of the process ID tasks to the completion entries + IDictionary, CompletionEntry> map = new Dictionary, CompletionEntry>(_completionEntries.Count); + foreach (CompletionEntry entry in _completionEntries) + { + map.Add(entry.Runner.ProcessIdTask.WithCancellation(token), entry); + } + + while (map.Count > 0) + { + // Wait for any of the process ID tasks to complete. + Task completedTask = await Task.WhenAny(map.Keys); + + map.Remove(completedTask, out CompletionEntry entry); + + _outputHelper.WriteLine($"[Source] Checking App{entry.Runner.AppId}"); + + if (completedTask.IsCompletedSuccessfully) + { + // If the process ID matches the one that was reported via the callback, + // then signal its completion source. + if (info.ProcessId == completedTask.Result) + { + _outputHelper.WriteLine($"[Source] Notifying App{entry.Runner.AppId}"); + entry.CompletionSource.TrySetResult(info); + + _completionEntries.Remove(entry); + + break; + } + } + } + + _outputHelper.WriteLine($"[Source] Finished notifications for process {info.ProcessId}"); + } + finally + { + _completionEntriesSemaphore.Release(); + } + } + + public void OnRemovedEndpointInfo(IEndpointInfo info) + { + _outputHelper.WriteLine($"[Source] Removed: {ToOutputString(info)}"); + } + + private static string ToOutputString(IEndpointInfo info) + { + return FormattableString.Invariant($"PID={info.ProcessId}, Cookie={info.RuntimeInstanceCookie}"); + } + + private sealed class CompletionEntry + { + public CompletionEntry(AppRunner runner) + { + Runner = runner; + CompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + } + + public AppRunner Runner { get; } + + public TaskCompletionSource CompletionSource { get; } + } + } + } +} diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj index f1c0d541b9f..fd22da0c0c4 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests.csproj @@ -12,7 +12,6 @@ - diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs index 368a8b6cf1c..aecfcd06397 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs @@ -70,21 +70,14 @@ public static IHost CreateHost( builder.AddInMemoryCollection(configurationValues); - builder.AddInMemoryCollection(new Dictionary - { - {ConfigurationPath.Combine(ConfigurationKeys.Storage, nameof(StorageOptions.DumpTempFolder)), StorageOptionsDefaults.DumpTempFolder } - }); + builder.ConfigureStorageDefaults(); }) .ConfigureServices((HostBuilderContext context, IServiceCollection services) => { services.ConfigureCollectionRules(); services.ConfigureEgress(); - services.AddSingleton(); - services.AddSingleton(); - services.AddSingleton(); - services.AddSingleton(); - services.AddSingleton(); + services.AddSingleton(); services.ConfigureStorage(context.Configuration); }) .Build(); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 53c31afc451..ee46b89c319 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -8,6 +8,7 @@ using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Exceptions; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using System; using System.Collections.Generic; using System.Threading; @@ -17,13 +18,13 @@ namespace Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Actions { internal sealed class CollectDumpAction : ICollectionRuleAction { - private readonly IDiagnosticServices _diagnosticServices; + private readonly IDumpService _dumpService; private readonly IServiceProvider _serviceProvider; public CollectDumpAction(IServiceProvider serviceProvider) { _serviceProvider = serviceProvider; - _diagnosticServices = serviceProvider.GetRequiredService(); + _dumpService = serviceProvider.GetRequiredService(); } public async Task ExecuteAsync(CollectDumpOptions options, IEndpointInfo endpointInfo, CancellationToken token) @@ -31,11 +32,9 @@ public async Task ExecuteAsync(CollectDumpOptions op DumpType dumpType = options.Type.GetValueOrDefault(CollectDumpOptionsDefaults.Type); string egressProvider = options.Egress; - IProcessInfo processInfo = await _diagnosticServices.GetProcessAsync(new ProcessKey(endpointInfo.ProcessId), token); + string dumpFileName = Monitoring.WebApi.Utilities.GenerateDumpFileName(); - string dumpFileName = DiagController.GenerateDumpFileName(); - - string dumpFilePath = ""; + string dumpFilePath = string.Empty; // Given our options validation, I believe this is probably redundant...should I remove it? if (string.IsNullOrEmpty(egressProvider)) @@ -45,17 +44,22 @@ public async Task ExecuteAsync(CollectDumpOptions op } else { - KeyValueLogScope scope = DiagController.GetDumpScope(processInfo); + KeyValueLogScope scope = DiagController.GetDumpScope(endpointInfo); try { - dumpFilePath = await SendToEgress(new EgressOperation( - token => _diagnosticServices.GetDump(processInfo, dumpType, token), + EgressOperation egressOperation = new EgressOperation( + token => _dumpService.DumpAsync(endpointInfo, dumpType, token), egressProvider, dumpFileName, - processInfo.EndpointInfo, + endpointInfo, ContentTypes.ApplicationOctetStream, - scope), token); + scope); + + ExecutionResult result = await egressOperation.ExecuteAsync(_serviceProvider, token); + + dumpFilePath = result.Result.Value; + } catch (Exception ex) { @@ -71,12 +75,5 @@ public async Task ExecuteAsync(CollectDumpOptions op } }; } - - private async Task SendToEgress(EgressOperation egressStreamResult, CancellationToken token) - { - var result = await egressStreamResult.ExecuteAsync(_serviceProvider, token); - - return result.Result.Value; - } } } diff --git a/src/Tools/dotnet-monitor/ConfigurationBuilderExtensions.cs b/src/Tools/dotnet-monitor/ConfigurationBuilderExtensions.cs index 5aedc801952..0e390224ea5 100644 --- a/src/Tools/dotnet-monitor/ConfigurationBuilderExtensions.cs +++ b/src/Tools/dotnet-monitor/ConfigurationBuilderExtensions.cs @@ -2,11 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.KeyPerFile; using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Primitives; using System; +using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Reflection; @@ -31,6 +33,14 @@ public static IConfigurationBuilder AddKeyPerFileWithChangeTokenSupport(this ICo }); } + public static IConfigurationBuilder ConfigureStorageDefaults(this IConfigurationBuilder builder) + { + return builder.AddInMemoryCollection(new Dictionary + { + {ConfigurationPath.Combine(ConfigurationKeys.Storage, nameof(StorageOptions.DumpTempFolder)), StorageOptionsDefaults.DumpTempFolder } + }); + } + private class KeyPerFileConfigurationSourceWithChangeTokenSupport : KeyPerFileConfigurationSource, IConfigurationSource diff --git a/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs b/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs index 303deff9402..3f6b5390448 100644 --- a/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs +++ b/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs @@ -141,7 +141,6 @@ public static IHostBuilder CreateHostBuilder(IConsole console, string[] urls, st //Note these are in precedence order. ConfigureEndpointInfoSource(builder, diagnosticPort); ConfigureMetricsEndpoint(builder, metrics, metricUrls); - ConfigureStorageDefaults(builder); builder.AddCommandLine(new[] { "--urls", ConfigurationHelper.JoinValue(urls) }); @@ -166,6 +165,7 @@ public static IHostBuilder CreateHostBuilder(IConsole console, string[] urls, st builder.AddKeyPerFileWithChangeTokenSupport(path, optional: true, reloadOnChange: true); builder.AddEnvironmentVariables(ConfigPrefix); + builder.ConfigureStorageDefaults(); if (authenticationOptions.KeyAuthenticationMode == KeyAuthenticationMode.TemporaryKey) { @@ -239,6 +239,7 @@ public static IHostBuilder CreateHostBuilder(IConsole console, string[] urls, st services.AddSingleton(); services.AddHostedService(); services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.ConfigureOperationStore(); services.ConfigureEgress(); @@ -306,14 +307,6 @@ private static void ConfigureTempApiHashKey(IConfigurationBuilder builder, AuthC } } - private static void ConfigureStorageDefaults(IConfigurationBuilder builder) - { - builder.AddInMemoryCollection(new Dictionary - { - {ConfigurationPath.Combine(ConfigurationKeys.Storage, nameof(StorageOptions.DumpTempFolder)), StorageOptionsDefaults.DumpTempFolder } - }); - } - private static void ConfigureMetricsEndpoint(IConfigurationBuilder builder, bool enableMetrics, string[] metricEndpoints) { builder.AddInMemoryCollection(new Dictionary From 29e1cadb21f4641dd3a89ef6ff3e2f424cd88bbb Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Tue, 7 Sep 2021 15:40:12 -0700 Subject: [PATCH 15/22] Resolving an error that resulted from GitHub's automatic merge. --- .../Controllers/DiagController.Metrics.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs index be435b66345..bba7125344d 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs @@ -145,6 +145,6 @@ public Task CaptureMetricsCustom( } private static string GetMetricFilename(IProcessInfo processInfo) => - FormattableString.Invariant($"{GetFileNameTimeStampUtcNow()}_{processInfo.EndpointInfo.ProcessId}.metrics.json"); + FormattableString.Invariant($"{Utilities.GetFileNameTimeStampUtcNow()}_{processInfo.EndpointInfo.ProcessId}.metrics.json"); } } From bdc4847738c57c2d3afc48db0b393c2ab921570d Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Wed, 8 Sep 2021 10:10:44 -0700 Subject: [PATCH 16/22] This appears to have resolved the merge conflict issues (mostly surrounding EndpointInfoSourceCallback and EndpointUtilities). Local tests are passing. --- .../CollectDumpActionTests.cs | 5 +- .../EndpointInfoSourceTests.cs | 6 +- .../EndpointUtilities.cs | 119 +----------------- 3 files changed, 6 insertions(+), 124 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index 6076adaf567..cdeed627a08 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -62,7 +62,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => options.Type = ExpectedDumpType; } - ServerEndpointInfoCallback callback = new(_outputHelper); + EndpointInfoSourceCallback callback = new(_outputHelper); await using var source = _endpointUtilities.CreateServerSource(out string transportName, callback); source.Start(); @@ -71,8 +71,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => AppRunner runner = _endpointUtilities.CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60; should we test against other frameworks? - using CancellationTokenSource callbackCancellation = new(CommonTestTimeouts.StartProcess); - Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, callbackCancellation.Token); + Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); await runner.ExecuteAsync(async () => { diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs index 1142612e9b7..8d0f97ce5d9 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs @@ -108,7 +108,7 @@ public async Task ServerSourceThrowsWhenMultipleStartTest() [MemberData(nameof(GetTfmsSupportingPortListener))] public async Task ServerSourceAddRemoveSingleConnectionTest(TargetFrameworkMoniker appTfm) { - ServerEndpointInfoCallback callback = new(_outputHelper); + EndpointInfoSourceCallback callback = new(_outputHelper); await using var source = _endpointUtilities.CreateServerSource(out string transportName, callback); source.Start(); @@ -149,7 +149,7 @@ await runner.ExecuteAsync(async () => [MemberData(nameof(GetTfmsSupportingPortListener))] public async Task ServerSourceAddRemoveMultipleConnectionTest(TargetFrameworkMoniker appTfm) { - ServerEndpointInfoCallback callback = new(_outputHelper); + EndpointInfoSourceCallback callback = new(_outputHelper); await using var source = _endpointUtilities.CreateServerSource(out string transportName, callback); source.Start(); @@ -164,7 +164,7 @@ public async Task ServerSourceAddRemoveMultipleConnectionTest(TargetFrameworkMon for (int i = 0; i < appCount; i++) { runners[i] = _endpointUtilities.CreateAppRunner(transportName, appTfm, appId: i + 1); - newEndpointInfoTasks[i] = callback.WaitForNewEndpointInfoAsync(runners[i], cancellation.Token); + newEndpointInfoTasks[i] = callback.WaitForNewEndpointInfoAsync(runners[i], CommonTestTimeouts.StartProcess); } await runners.ExecuteAsync(async () => diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs index 0fbe125f854..c22e5cac555 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs @@ -28,7 +28,7 @@ public EndpointUtilities(ITestOutputHelper outputHelper) _outputHelper = outputHelper; } - public ServerEndpointInfoSource CreateServerSource(out string transportName, ServerEndpointInfoCallback callback = null) + public ServerEndpointInfoSource CreateServerSource(out string transportName, EndpointInfoSourceCallback callback = null) { DiagnosticPortHelper.Generate(DiagnosticPortConnectionMode.Listen, out _, out transportName); _outputHelper.WriteLine("Starting server endpoint info source at '" + transportName + "'."); @@ -68,122 +68,5 @@ public static async Task VerifyConnectionAsync(AppRunner runner, IEndpointInfo e Assert.NotEqual(Guid.Empty, endpointInfo.RuntimeInstanceCookie); Assert.NotNull(endpointInfo.Endpoint); } - - public sealed class ServerEndpointInfoCallback : IEndpointInfoSourceCallbacks - { - private readonly ITestOutputHelper _outputHelper; - /// - /// Use to protect the completion list from mutation while processing - /// callbacks from it. The processing is done in an async method with async - /// calls, which are not allowed in a lock, thus use SemaphoreSlim. - /// - private readonly SemaphoreSlim _completionEntriesSemaphore = new(1); - private readonly List _completionEntries = new(); - - public ServerEndpointInfoCallback(ITestOutputHelper outputHelper) - { - _outputHelper = outputHelper; - } - - public async Task WaitForNewEndpointInfoAsync(AppRunner runner, CancellationToken token) - { - CompletionEntry entry = new(runner); - using var _ = token.Register(() => entry.CompletionSource.TrySetCanceled(token)); - - await _completionEntriesSemaphore.WaitAsync(token); - try - { - _completionEntries.Add(entry); - _outputHelper.WriteLine($"[Wait] Register App{runner.AppId}"); - } - finally - { - _completionEntriesSemaphore.Release(); - } - - _outputHelper.WriteLine($"[Wait] Wait for App{runner.AppId} notification"); - IEndpointInfo endpointInfo = await entry.CompletionSource.Task; - _outputHelper.WriteLine($"[Wait] Received App{runner.AppId} notification"); - - return endpointInfo; - } - - public Task OnBeforeResumeAsync(IEndpointInfo endpointInfo, CancellationToken token) - { - return Task.CompletedTask; - } - - public async Task OnAddedEndpointInfoAsync(IEndpointInfo info, CancellationToken token) - { - _outputHelper.WriteLine($"[Source] Added: {ToOutputString(info)}"); - - await _completionEntriesSemaphore.WaitAsync(token); - try - { - _outputHelper.WriteLine($"[Source] Start notifications for process {info.ProcessId}"); - - // Create a mapping of the process ID tasks to the completion entries - IDictionary, CompletionEntry> map = new Dictionary, CompletionEntry>(_completionEntries.Count); - foreach (CompletionEntry entry in _completionEntries) - { - map.Add(entry.Runner.ProcessIdTask.WithCancellation(token), entry); - } - - while (map.Count > 0) - { - // Wait for any of the process ID tasks to complete. - Task completedTask = await Task.WhenAny(map.Keys); - - map.Remove(completedTask, out CompletionEntry entry); - - _outputHelper.WriteLine($"[Source] Checking App{entry.Runner.AppId}"); - - if (completedTask.IsCompletedSuccessfully) - { - // If the process ID matches the one that was reported via the callback, - // then signal its completion source. - if (info.ProcessId == completedTask.Result) - { - _outputHelper.WriteLine($"[Source] Notifying App{entry.Runner.AppId}"); - entry.CompletionSource.TrySetResult(info); - - _completionEntries.Remove(entry); - - break; - } - } - } - - _outputHelper.WriteLine($"[Source] Finished notifications for process {info.ProcessId}"); - } - finally - { - _completionEntriesSemaphore.Release(); - } - } - - public void OnRemovedEndpointInfo(IEndpointInfo info) - { - _outputHelper.WriteLine($"[Source] Removed: {ToOutputString(info)}"); - } - - private static string ToOutputString(IEndpointInfo info) - { - return FormattableString.Invariant($"PID={info.ProcessId}, Cookie={info.RuntimeInstanceCookie}"); - } - - private sealed class CompletionEntry - { - public CompletionEntry(AppRunner runner) - { - Runner = runner; - CompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - } - - public AppRunner Runner { get; } - - public TaskCompletionSource CompletionSource { get; } - } - } } } From 527fb5833068236e97e04c9070a4afa0da5a1ded Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Wed, 8 Sep 2021 14:21:26 -0700 Subject: [PATCH 17/22] Slight refactoring to get a shared method out of DiagController and into the Utilities class. --- .../Controllers/DiagController.Metrics.cs | 12 ++--- .../Controllers/DiagController.cs | 53 +++++++------------ .../RequestThrottling/RequestLimitTracker.cs | 10 ++-- .../Utilities.cs | 15 ++++++ .../Actions/CollectDumpAction.cs | 7 ++- 5 files changed, 47 insertions(+), 50 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs index bba7125344d..8ff19f864c2 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs @@ -34,7 +34,7 @@ partial class DiagController [ProducesWithProblemDetails(ContentTypes.ApplicationJsonSequence)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = ArtifactType_Metrics)] + [RequestLimit(LimitKey = Utilities.ArtifactType_Metrics)] [EgressValidation] public Task CaptureMetrics( [FromQuery] @@ -72,13 +72,13 @@ public Task CaptureMetrics( await eventCounterPipeline.RunAsync(token); }; - return await Result(ArtifactType_Metrics, + return await Result(Utilities.ArtifactType_Metrics, egressProvider, action, fileName, ContentTypes.ApplicationJsonSequence, processInfo.EndpointInfo); - }, processKey, ArtifactType_Metrics); + }, processKey, Utilities.ArtifactType_Metrics); } /// @@ -95,7 +95,7 @@ public Task CaptureMetrics( [ProducesWithProblemDetails(ContentTypes.ApplicationJsonSequence)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = ArtifactType_Metrics)] + [RequestLimit(LimitKey = Utilities.ArtifactType_Metrics)] [EgressValidation] public Task CaptureMetricsCustom( [FromBody][Required] @@ -135,13 +135,13 @@ public Task CaptureMetricsCustom( await eventCounterPipeline.RunAsync(token); }; - return await Result(ArtifactType_Metrics, + return await Result(Utilities.ArtifactType_Metrics, egressProvider, action, fileName, ContentTypes.ApplicationJsonSequence, processInfo.EndpointInfo); - }, processKey, ArtifactType_Metrics); + }, processKey, Utilities.ArtifactType_Metrics); } private static string GetMetricFilename(IProcessInfo processInfo) => diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 9ce9eafe0cf..c5cc6c0d741 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -38,12 +38,6 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi.Controllers [ProducesResponseType(typeof(void), StatusCodes.Status401Unauthorized)] public partial class DiagController : ControllerBase { - public const string ArtifactType_Dump = "dump"; - public const string ArtifactType_GCDump = "gcdump"; - public const string ArtifactType_Logs = "logs"; - public const string ArtifactType_Trace = "trace"; - public const string ArtifactType_Metrics = "collectmetrics"; - private const Models.TraceProfile DefaultTraceProfiles = Models.TraceProfile.Cpu | Models.TraceProfile.Http | Models.TraceProfile.Metrics; private static readonly MediaTypeHeaderValue NdJsonHeader = new MediaTypeHeaderValue(ContentTypes.ApplicationNdJson); private static readonly MediaTypeHeaderValue JsonSequenceHeader = new MediaTypeHeaderValue(ContentTypes.ApplicationJsonSequence); @@ -196,7 +190,7 @@ public Task>> GetProcessEnvironment( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(FileResult), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = ArtifactType_Dump)] + [RequestLimit(LimitKey = Utilities.ArtifactType_Dump)] [EgressValidation] public Task CaptureDump( [FromQuery] @@ -227,7 +221,7 @@ public Task CaptureDump( } else { - KeyValueLogScope scope = GetDumpScope(processInfo.EndpointInfo); + KeyValueLogScope scope = Utilities.GetScope(Utilities.ArtifactType_Dump, processInfo.EndpointInfo); return await SendToEgress(new EgressOperation( token => _dumpService.DumpAsync(processInfo.EndpointInfo, type, token), @@ -235,9 +229,9 @@ public Task CaptureDump( dumpFileName, processInfo.EndpointInfo, ContentTypes.ApplicationOctetStream, - scope), limitKey: ArtifactType_Dump); + scope), limitKey: Utilities.ArtifactType_Dump); } - }, processKey, ArtifactType_Dump); + }, processKey, Utilities.ArtifactType_Dump); } /// @@ -255,7 +249,7 @@ public Task CaptureDump( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(FileResult), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = ArtifactType_GCDump)] + [RequestLimit(LimitKey = Utilities.ArtifactType_GCDump)] [EgressValidation] public Task CaptureGcDump( [FromQuery] @@ -293,13 +287,13 @@ public Task CaptureGcDump( }; return Result( - ArtifactType_GCDump, + Utilities.ArtifactType_GCDump, egressProvider, ConvertFastSerializeAction(action), fileName, ContentTypes.ApplicationOctetStream, processInfo.EndpointInfo); - }, processKey, ArtifactType_GCDump); + }, processKey, Utilities.ArtifactType_GCDump); } /// @@ -319,7 +313,7 @@ public Task CaptureGcDump( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(FileResult), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = ArtifactType_Trace)] + [RequestLimit(LimitKey = Utilities.ArtifactType_Trace)] [EgressValidation] public Task CaptureTrace( [FromQuery] @@ -368,7 +362,7 @@ public Task CaptureTrace( var aggregateConfiguration = new AggregateSourceConfiguration(configurations.ToArray()); return StartTrace(processInfo, aggregateConfiguration, duration, egressProvider); - }, processKey, ArtifactType_Trace); + }, processKey, Utilities.ArtifactType_Trace); } /// @@ -387,7 +381,7 @@ public Task CaptureTrace( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(FileResult), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = ArtifactType_Trace)] + [RequestLimit(LimitKey = Utilities.ArtifactType_Trace)] [EgressValidation] public Task CaptureTraceCustom( [FromBody][Required] @@ -432,7 +426,7 @@ public Task CaptureTraceCustom( bufferSizeInMB: configuration.BufferSizeInMB); return StartTrace(processInfo, traceConfiguration, duration, egressProvider); - }, processKey, ArtifactType_Trace); + }, processKey, Utilities.ArtifactType_Trace); } /// @@ -449,7 +443,7 @@ public Task CaptureTraceCustom( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(string), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = ArtifactType_Logs)] + [RequestLimit(LimitKey = Utilities.ArtifactType_Logs)] [EgressValidation] public Task CaptureLogs( [FromQuery] @@ -488,7 +482,7 @@ public Task CaptureLogs( } return StartLogs(processInfo, settings, egressProvider); - }, processKey, ArtifactType_Logs); + }, processKey, Utilities.ArtifactType_Logs); } /// @@ -505,7 +499,7 @@ public Task CaptureLogs( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(string), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = ArtifactType_Logs)] + [RequestLimit(LimitKey = Utilities.ArtifactType_Logs)] [EgressValidation] public Task CaptureLogsCustom( [FromBody] @@ -536,7 +530,7 @@ public Task CaptureLogsCustom( }; return StartLogs(processInfo, settings, egressProvider); - }, processKey, ArtifactType_Logs); + }, processKey, Utilities.ArtifactType_Logs); } /// @@ -583,15 +577,6 @@ private static string GetDotnetMonitorVersion() } } - internal static KeyValueLogScope GetDumpScope(IEndpointInfo endpointInfo) - { - KeyValueLogScope scope = new KeyValueLogScope(); - scope.AddArtifactType(ArtifactType_Dump); - scope.AddEndpointInfo(endpointInfo); - - return scope; - } - public DiagnosticPortConnectionMode GetDiagnosticPortMode() { return _diagnosticPortOptions.Value.ConnectionMode.GetValueOrDefault(DiagnosticPortOptionsDefaults.ConnectionMode); @@ -631,7 +616,7 @@ private Task StartTrace( }; return Result( - ArtifactType_Trace, + Utilities.ArtifactType_Trace, egressProvider, action, fileName, @@ -679,7 +664,7 @@ private Task StartLogs( }; return Result( - ArtifactType_Logs, + Utilities.ArtifactType_Logs, egressProvider, action, fileName, @@ -743,9 +728,7 @@ private Task Result( IEndpointInfo endpointInfo, bool asAttachment = true) { - KeyValueLogScope scope = new KeyValueLogScope(); - scope.AddArtifactType(artifactType); - scope.AddEndpointInfo(endpointInfo); + KeyValueLogScope scope = Utilities.GetScope(artifactType, endpointInfo); if (string.IsNullOrEmpty(providerName)) { diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitTracker.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitTracker.cs index 34eb54f7c60..8b715c70dbe 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitTracker.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitTracker.cs @@ -28,11 +28,11 @@ public RequestLimitTracker(ILogger logger) { //CONSIDER Should we have configuration for these? - _requestLimitTable.Add(Controllers.DiagController.ArtifactType_Dump, 1); - _requestLimitTable.Add(Controllers.DiagController.ArtifactType_GCDump, 1); - _requestLimitTable.Add(Controllers.DiagController.ArtifactType_Logs, 3); - _requestLimitTable.Add(Controllers.DiagController.ArtifactType_Trace, 3); - _requestLimitTable.Add(Controllers.DiagController.ArtifactType_Metrics, 3); + _requestLimitTable.Add(Utilities.ArtifactType_Dump, 1); + _requestLimitTable.Add(Utilities.ArtifactType_GCDump, 1); + _requestLimitTable.Add(Utilities.ArtifactType_Logs, 3); + _requestLimitTable.Add(Utilities.ArtifactType_Trace, 3); + _requestLimitTable.Add(Utilities.ArtifactType_Metrics, 3); _logger = logger; } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs index 1b8eb6a0cf6..dbc3ff8bfb9 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs @@ -9,6 +9,12 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi { public class Utilities { + public const string ArtifactType_Dump = "dump"; + public const string ArtifactType_GCDump = "gcdump"; + public const string ArtifactType_Logs = "logs"; + public const string ArtifactType_Trace = "trace"; + public const string ArtifactType_Metrics = "collectmetrics"; + internal static string GenerateDumpFileName() { return RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? @@ -20,5 +26,14 @@ internal static string GetFileNameTimeStampUtcNow() { return DateTime.UtcNow.ToString("yyyyMMdd_HHmmss"); } + + internal static KeyValueLogScope GetScope(string artifactType, IEndpointInfo endpointInfo) + { + KeyValueLogScope scope = new KeyValueLogScope(); + scope.AddArtifactType(artifactType); + scope.AddEndpointInfo(endpointInfo); + + return scope; + } } } diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index ee46b89c319..e1a6d21a4e6 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -3,16 +3,15 @@ // See the LICENSE file in the project root for more information. using Microsoft.Diagnostics.Monitoring.WebApi; -using Microsoft.Diagnostics.Monitoring.WebApi.Controllers; using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Exceptions; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; using System; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using Utils = Microsoft.Diagnostics.Monitoring.WebApi.Utilities; namespace Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Actions { @@ -32,7 +31,7 @@ public async Task ExecuteAsync(CollectDumpOptions op DumpType dumpType = options.Type.GetValueOrDefault(CollectDumpOptionsDefaults.Type); string egressProvider = options.Egress; - string dumpFileName = Monitoring.WebApi.Utilities.GenerateDumpFileName(); + string dumpFileName = Utils.GenerateDumpFileName(); string dumpFilePath = string.Empty; @@ -44,7 +43,7 @@ public async Task ExecuteAsync(CollectDumpOptions op } else { - KeyValueLogScope scope = DiagController.GetDumpScope(endpointInfo); + KeyValueLogScope scope = Utils.GetScope(Utils.ArtifactType_Dump, endpointInfo); try { From da9d016d338b0cf61feeea3b9b182df8ab07e445 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Thu, 9 Sep 2021 13:35:30 -0700 Subject: [PATCH 18/22] Still experimenting to get tests to pass. Issue appears to be with FileNotFound exceptions, so made some changes that bring this code to more closely resemble what's being done in the Egress tests. Tests still pass locally. --- .../CollectDumpActionTests.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index cdeed627a08..b7ac21a8a69 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -43,11 +43,11 @@ public async Task CollectDumpAction_Success(DumpType? dumpType) { DumpType ExpectedDumpType = (dumpType != null) ? dumpType.Value : CollectDumpOptionsDefaults.Type; - string uniqueEgressDirectory = TempEgressDirectory + Guid.NewGuid(); + DirectoryInfo uniqueEgressDirectory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), TempEgressDirectory, Guid.NewGuid().ToString())); await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => { - rootOptions.AddFileSystemEgress(ExpectedEgressProvider, uniqueEgressDirectory); + rootOptions.AddFileSystemEgress(ExpectedEgressProvider, uniqueEgressDirectory.FullName); }, async host => { CollectDumpAction action = new(host.Services); @@ -116,9 +116,7 @@ await runner.ExecuteAsync(async () => try { - DirectoryInfo outputDirectory = Directory.CreateDirectory(uniqueEgressDirectory); // Do we have a better way of getting the current directory (to delete it) - - outputDirectory?.Delete(recursive: true); + uniqueEgressDirectory?.Delete(recursive: true); } catch { From 6ff42bf7a62e951d6c423b969d399bc446b3368f Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Fri, 10 Sep 2021 13:13:58 -0700 Subject: [PATCH 19/22] Changes for Justin and Wiktor --- .../DumpService.cs | 20 ++-- .../IDiagnosticServices.cs | 3 - .../Utilities.cs | 8 +- ...t.Diagnostics.Monitoring.TestCommon.csproj | 8 +- .../ActionListExecutorTests.cs | 2 +- .../CollectDumpActionTests.cs | 99 ++++++++----------- .../CollectionRuleOptionsTests.cs | 4 +- .../CollectionRuleOptionsExtensions.cs | 2 +- .../DiagnosticsMonitorCommandHandler.cs | 2 +- 9 files changed, 62 insertions(+), 86 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs index 651766ab9e4..cbc61baa735 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs @@ -26,22 +26,22 @@ public DumpService(IOptionsMonitor storageOptions) public async Task DumpAsync(IEndpointInfo endpointInfo, Models.DumpType mode, CancellationToken token) { string dumpFilePath = Path.Combine(_storageOptions.CurrentValue.DumpTempFolder, FormattableString.Invariant($"{Guid.NewGuid()}_{endpointInfo.ProcessId}")); - NETCore.Client.DumpType dumpType = MapDumpType(mode); + DumpType dumpType = MapDumpType(mode); if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { // Get the process Process process = Process.GetProcessById(endpointInfo.ProcessId); await Dumper.CollectDumpAsync(process, dumpFilePath, dumpType); - } - else - { - await Task.Run(() => - { - var client = new DiagnosticsClient(endpointInfo.Endpoint); - client.WriteDump(dumpType, dumpFilePath); - }); - } + } + else + { + await Task.Run(async () => + { + var client = new DiagnosticsClient(endpointInfo.Endpoint); + await client.WriteDumpAsync(dumpType, dumpFilePath, logDumpGeneration: false, token); + }); + } return new AutoDeleteFileStream(dumpFilePath); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/IDiagnosticServices.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/IDiagnosticServices.cs index 8b4a2940600..9f3b87e51db 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/IDiagnosticServices.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/IDiagnosticServices.cs @@ -30,11 +30,8 @@ internal interface IDiagnosticServices : IDisposable /// situations allow a different process object. /// Task GetProcessAsync(ProcessKey? processKey, CancellationToken token); - - //Task GetDump(IProcessInfo pi, Models.DumpType mode, CancellationToken token); } - internal interface IProcessInfo { IEndpointInfo EndpointInfo { get; } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs index dbc3ff8bfb9..c24124060e9 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities.cs @@ -7,7 +7,7 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi { - public class Utilities + internal static class Utilities { public const string ArtifactType_Dump = "dump"; public const string ArtifactType_GCDump = "gcdump"; @@ -15,19 +15,19 @@ public class Utilities public const string ArtifactType_Trace = "trace"; public const string ArtifactType_Metrics = "collectmetrics"; - internal static string GenerateDumpFileName() + public static string GenerateDumpFileName() { return RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? FormattableString.Invariant($"dump_{GetFileNameTimeStampUtcNow()}.dmp") : FormattableString.Invariant($"core_{GetFileNameTimeStampUtcNow()}"); } - internal static string GetFileNameTimeStampUtcNow() + public static string GetFileNameTimeStampUtcNow() { return DateTime.UtcNow.ToString("yyyyMMdd_HHmmss"); } - internal static KeyValueLogScope GetScope(string artifactType, IEndpointInfo endpointInfo) + public static KeyValueLogScope GetScope(string artifactType, IEndpointInfo endpointInfo) { KeyValueLogScope scope = new KeyValueLogScope(); scope.AddArtifactType(artifactType); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj index 693032fe012..5dfb1bb8139 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj @@ -14,15 +14,9 @@ + - - - - - - - diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/ActionListExecutorTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/ActionListExecutorTests.cs index 0726634714e..4b783675940 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/ActionListExecutorTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/ActionListExecutorTests.cs @@ -19,7 +19,7 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests public sealed class ActionListExecutorTests { private const int TokenTimeoutMs = 10000; - + private readonly ITestOutputHelper _outputHelper; private const string DefaultRuleName = "Default"; diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index b7ac21a8a69..be897c8790f 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -16,6 +16,11 @@ using Microsoft.Diagnostics.Monitoring.TestCommon.Options; using Microsoft.Diagnostics.Monitoring.WebApi.Models; using static Microsoft.Diagnostics.Monitoring.Tool.UnitTests.EndpointUtilities; +using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Diagnostics.Tools.Monitor.CollectionRules; +using Microsoft.Extensions.Options; +using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { @@ -23,6 +28,7 @@ public sealed class CollectDumpActionTests { private const string TempEgressDirectory = "/tmp"; private const string ExpectedEgressProvider = "TmpEgressProvider"; + private const string DefaultRuleName = "Default"; private ITestOutputHelper _outputHelper; private readonly EndpointUtilities _endpointUtilities; @@ -41,79 +47,58 @@ public CollectDumpActionTests(ITestOutputHelper outputHelper) [InlineData(null)] public async Task CollectDumpAction_Success(DumpType? dumpType) { - DumpType ExpectedDumpType = (dumpType != null) ? dumpType.Value : CollectDumpOptionsDefaults.Type; + DirectoryInfo uniqueEgressDirectory = null; - DirectoryInfo uniqueEgressDirectory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), TempEgressDirectory, Guid.NewGuid().ToString())); + try { + DumpType? ExpectedDumpType = dumpType; - await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => - { - rootOptions.AddFileSystemEgress(ExpectedEgressProvider, uniqueEgressDirectory.FullName); - }, async host => - { - CollectDumpAction action = new(host.Services); - - CollectDumpOptions options = new(); - - options.Egress = ExpectedEgressProvider; + uniqueEgressDirectory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), TempEgressDirectory, Guid.NewGuid().ToString())); - // This is for the scenario where no DumpType is specified - if (dumpType != null) + await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => { - options.Type = ExpectedDumpType; - } - - EndpointInfoSourceCallback callback = new(_outputHelper); - await using var source = _endpointUtilities.CreateServerSource(out string transportName, callback); - source.Start(); - - var endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); - Assert.Empty(endpointInfos); + rootOptions.AddFileSystemEgress(ExpectedEgressProvider, uniqueEgressDirectory.FullName); - AppRunner runner = _endpointUtilities.CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60; should we test against other frameworks? - - Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); - - await runner.ExecuteAsync(async () => + rootOptions.CreateCollectionRule(DefaultRuleName) + .AddCollectDumpAction(ExpectedEgressProvider, ExpectedDumpType) + .SetStartupTrigger(); + }, async host => { - await newEndpointInfoTask; + IOptionsMonitor ruleOptionsMonitor = host.Services.GetService>(); + CollectDumpOptions options = (CollectDumpOptions)ruleOptionsMonitor.Get(DefaultRuleName).Actions[0].Settings; - endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); + ICollectionRuleActionProxy action; + host.Services.GetService().TryCreateAction(KnownCollectionRuleActions.CollectDump, out action); - var endpointInfo = Assert.Single(endpointInfos); - Assert.NotNull(endpointInfo.CommandLine); - Assert.NotNull(endpointInfo.OperatingSystem); - Assert.NotNull(endpointInfo.ProcessArchitecture); - await VerifyConnectionAsync(runner, endpointInfo); + EndpointInfoSourceCallback callback = new(_outputHelper); + await using var source = _endpointUtilities.CreateServerSource(out string transportName, callback); + source.Start(); - using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(CommonTestTimeouts.DumpTimeout); - CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); + AppRunner runner = _endpointUtilities.CreateAppRunner(transportName, TargetFrameworkMoniker.Net60); // Arbitrarily chose Net60; should we test against other frameworks? - string egressPath = result.OutputValues["EgressPath"]; + Task newEndpointInfoTask = callback.WaitForNewEndpointInfoAsync(runner, CommonTestTimeouts.StartProcess); - if (!File.Exists(egressPath)) + await runner.ExecuteAsync(async () => { - throw new FileNotFoundException(string.Format(CultureInfo.InvariantCulture, Tools.Monitor.Strings.ErrorMessage_FileNotFound, egressPath)); - } - else - { - using (StreamReader reader = new StreamReader(egressPath, true)) - { - Stream dumpStream = reader.BaseStream; - Assert.NotNull(dumpStream); + IEndpointInfo endpointInfo = await newEndpointInfoTask; - await DumpTestUtilities.ValidateDump(runner.Environment.ContainsKey(DumpTestUtilities.EnableElfDumpOnMacOS), dumpStream); - } - } + using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(CommonTestTimeouts.DumpTimeout); + CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); - await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); - }); + Assert.NotNull(result.OutputValues); + Assert.True(result.OutputValues.TryGetValue("EgressPath", out string egressPath)); + Assert.True(File.Exists(egressPath)); - await Task.Delay(TimeSpan.FromSeconds(1)); + using FileStream dumpStream = new(egressPath, FileMode.Open, FileAccess.Read); + Assert.NotNull(dumpStream); - endpointInfos = await _endpointUtilities.GetEndpointInfoAsync(source); - - Assert.Empty(endpointInfos); + await DumpTestUtilities.ValidateDump(runner.Environment.ContainsKey(DumpTestUtilities.EnableElfDumpOnMacOS), dumpStream); + await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); + }); + }); + } + finally + { try { uniqueEgressDirectory?.Delete(recursive: true); @@ -121,7 +106,7 @@ await runner.ExecuteAsync(async () => catch { } - }); + } } } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectionRuleOptionsTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectionRuleOptionsTests.cs index c66ff038820..823e3f0e846 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectionRuleOptionsTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectionRuleOptionsTests.cs @@ -261,7 +261,7 @@ public Task CollectionRuleOptions_CollectDumpAction_RoundTrip() { rootOptions.CreateCollectionRule(DefaultRuleName) .SetStartupTrigger() - .AddCollectDumpAction(ExpectedDumpType, ExpectedEgressProvider); + .AddCollectDumpAction(ExpectedEgressProvider, ExpectedDumpType); rootOptions.AddFileSystemEgress(ExpectedEgressProvider, "/tmp"); }, ruleOptions => @@ -278,7 +278,7 @@ public Task CollectionRuleOptions_CollectDumpAction_PropertyValidation() { rootOptions.CreateCollectionRule(DefaultRuleName) .SetStartupTrigger() - .AddCollectDumpAction((DumpType)20, UnknownEgressName); + .AddCollectDumpAction(UnknownEgressName, (DumpType)20); }, ex => { diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Options/CollectionRuleOptionsExtensions.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Options/CollectionRuleOptionsExtensions.cs index f708d394b03..f4f2eb32023 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Options/CollectionRuleOptionsExtensions.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/Options/CollectionRuleOptionsExtensions.cs @@ -33,7 +33,7 @@ public static CollectionRuleOptions AddAction(this CollectionRuleOptions options return options; } - public static CollectionRuleOptions AddCollectDumpAction(this CollectionRuleOptions options, DumpType type, string egress) + public static CollectionRuleOptions AddCollectDumpAction(this CollectionRuleOptions options, string egress, DumpType? type = null) { options.AddAction(KnownCollectionRuleActions.CollectDump, out CollectionRuleActionOptions actionOptions); diff --git a/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs b/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs index 3f6b5390448..6871a764c49 100644 --- a/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs +++ b/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs @@ -141,6 +141,7 @@ public static IHostBuilder CreateHostBuilder(IConsole console, string[] urls, st //Note these are in precedence order. ConfigureEndpointInfoSource(builder, diagnosticPort); ConfigureMetricsEndpoint(builder, metrics, metricUrls); + builder.ConfigureStorageDefaults(); builder.AddCommandLine(new[] { "--urls", ConfigurationHelper.JoinValue(urls) }); @@ -165,7 +166,6 @@ public static IHostBuilder CreateHostBuilder(IConsole console, string[] urls, st builder.AddKeyPerFileWithChangeTokenSupport(path, optional: true, reloadOnChange: true); builder.AddEnvironmentVariables(ConfigPrefix); - builder.ConfigureStorageDefaults(); if (authenticationOptions.KeyAuthenticationMode == KeyAuthenticationMode.TemporaryKey) { From f707c789acc91f0105301d9f2aea8dfd13dd1e55 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Fri, 10 Sep 2021 15:23:11 -0700 Subject: [PATCH 20/22] More tweaks for Justin. --- .../DumpService.cs | 16 ++--- ...t.Diagnostics.Monitoring.TestCommon.csproj | 4 +- .../CollectDumpActionTests.cs | 32 +++++----- .../EndpointInfoSourceTests.cs | 5 +- .../Actions/CollectDumpAction.cs | 62 ++++++++++--------- 5 files changed, 61 insertions(+), 58 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs index cbc61baa735..a9e921c6962 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs @@ -20,11 +20,16 @@ internal class DumpService : IDumpService public DumpService(IOptionsMonitor storageOptions) { - _storageOptions = storageOptions; + _storageOptions = storageOptions ?? throw new ArgumentNullException(nameof(StorageOptions)); } public async Task DumpAsync(IEndpointInfo endpointInfo, Models.DumpType mode, CancellationToken token) { + if (endpointInfo == null) + { + throw new ArgumentNullException(nameof(endpointInfo)); + } + string dumpFilePath = Path.Combine(_storageOptions.CurrentValue.DumpTempFolder, FormattableString.Invariant($"{Guid.NewGuid()}_{endpointInfo.ProcessId}")); DumpType dumpType = MapDumpType(mode); @@ -32,15 +37,12 @@ public async Task DumpAsync(IEndpointInfo endpointInfo, Models.DumpType { // Get the process Process process = Process.GetProcessById(endpointInfo.ProcessId); - await Dumper.CollectDumpAsync(process, dumpFilePath, dumpType); + await Dumper.CollectDumpAsync(process, dumpFilePath, dumpType); } else { - await Task.Run(async () => - { - var client = new DiagnosticsClient(endpointInfo.Endpoint); - await client.WriteDumpAsync(dumpType, dumpFilePath, logDumpGeneration: false, token); - }); + var client = new DiagnosticsClient(endpointInfo.Endpoint); + await client.WriteDumpAsync(dumpType, dumpFilePath, logDumpGeneration: false, token); } return new AutoDeleteFileStream(dumpFilePath); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj index 5dfb1bb8139..651cba748f1 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Microsoft.Diagnostics.Monitoring.TestCommon.csproj @@ -11,10 +11,10 @@ - - + + diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index be897c8790f..68e007c8ada 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -2,25 +2,23 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; -using System.Threading.Tasks; -using Xunit; -using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Actions; -using System.Threading; -using System; -using System.IO; -using System.Globalization; -using Xunit.Abstractions; using Microsoft.Diagnostics.Monitoring.TestCommon; using Microsoft.Diagnostics.Monitoring.TestCommon.Runners; using Microsoft.Diagnostics.Monitoring.TestCommon.Options; -using Microsoft.Diagnostics.Monitoring.WebApi.Models; -using static Microsoft.Diagnostics.Monitoring.Tool.UnitTests.EndpointUtilities; using Microsoft.Diagnostics.Monitoring.WebApi; -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules; -using Microsoft.Extensions.Options; +using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Actions; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options; +using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { @@ -50,8 +48,6 @@ public async Task CollectDumpAction_Success(DumpType? dumpType) DirectoryInfo uniqueEgressDirectory = null; try { - DumpType? ExpectedDumpType = dumpType; - uniqueEgressDirectory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), TempEgressDirectory, Guid.NewGuid().ToString())); await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => @@ -59,7 +55,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => rootOptions.AddFileSystemEgress(ExpectedEgressProvider, uniqueEgressDirectory.FullName); rootOptions.CreateCollectionRule(DefaultRuleName) - .AddCollectDumpAction(ExpectedEgressProvider, ExpectedDumpType) + .AddCollectDumpAction(ExpectedEgressProvider, dumpType) .SetStartupTrigger(); }, async host => { @@ -67,7 +63,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => CollectDumpOptions options = (CollectDumpOptions)ruleOptionsMonitor.Get(DefaultRuleName).Actions[0].Settings; ICollectionRuleActionProxy action; - host.Services.GetService().TryCreateAction(KnownCollectionRuleActions.CollectDump, out action); + Assert.True(host.Services.GetService().TryCreateAction(KnownCollectionRuleActions.CollectDump, out action)); EndpointInfoSourceCallback callback = new(_outputHelper); await using var source = _endpointUtilities.CreateServerSource(out string transportName, callback); @@ -85,7 +81,7 @@ await runner.ExecuteAsync(async () => CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); Assert.NotNull(result.OutputValues); - Assert.True(result.OutputValues.TryGetValue("EgressPath", out string egressPath)); + Assert.True(result.OutputValues.TryGetValue(CollectDumpAction.egressPath, out string egressPath)); Assert.True(File.Exists(egressPath)); using FileStream dumpStream = new(egressPath, FileMode.Open, FileAccess.Read); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs index 8d0f97ce5d9..ca01bef905e 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs @@ -12,7 +12,6 @@ using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; -using static Microsoft.Diagnostics.Monitoring.Tool.UnitTests.EndpointUtilities; namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests { @@ -129,7 +128,7 @@ await runner.ExecuteAsync(async () => Assert.NotNull(endpointInfo.CommandLine); Assert.NotNull(endpointInfo.OperatingSystem); Assert.NotNull(endpointInfo.ProcessArchitecture); - await VerifyConnectionAsync(runner, endpointInfo); + await EndpointUtilities.VerifyConnectionAsync(runner, endpointInfo); await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }); @@ -187,7 +186,7 @@ await runners.ExecuteAsync(async () => Assert.NotNull(endpointInfo.OperatingSystem); Assert.NotNull(endpointInfo.ProcessArchitecture); - await VerifyConnectionAsync(runners[i], endpointInfo); + await EndpointUtilities.VerifyConnectionAsync(runners[i], endpointInfo); await runners[i].SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); } diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index e1a6d21a4e6..376732e199d 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -9,6 +9,7 @@ using Microsoft.Extensions.DependencyInjection; using System; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.Threading; using System.Threading.Tasks; using Utils = Microsoft.Diagnostics.Monitoring.WebApi.Utilities; @@ -20,14 +21,26 @@ internal sealed class CollectDumpAction : ICollectionRuleAction(); } public async Task ExecuteAsync(CollectDumpOptions options, IEndpointInfo endpointInfo, CancellationToken token) { + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + if (endpointInfo == null) + { + throw new ArgumentNullException(nameof(endpointInfo)); + } + DumpType dumpType = options.Type.GetValueOrDefault(CollectDumpOptionsDefaults.Type); string egressProvider = options.Egress; @@ -35,42 +48,35 @@ public async Task ExecuteAsync(CollectDumpOptions op string dumpFilePath = string.Empty; - // Given our options validation, I believe this is probably redundant...should I remove it? - if (string.IsNullOrEmpty(egressProvider)) - { - // Also, I would move this to Strings.resx if we do keep it, but I decided to wait for feedback before doing that. - throw new ArgumentException("No Egress Provider was supplied."); - } - else - { - KeyValueLogScope scope = Utils.GetScope(Utils.ArtifactType_Dump, endpointInfo); + ValidationContext context = new(options, _serviceProvider, items: null); + Validator.ValidateObject(options, context, validateAllProperties: true); - try - { - EgressOperation egressOperation = new EgressOperation( - token => _dumpService.DumpAsync(endpointInfo, dumpType, token), - egressProvider, - dumpFileName, - endpointInfo, - ContentTypes.ApplicationOctetStream, - scope); + KeyValueLogScope scope = Utils.GetScope(Utils.ArtifactType_Dump, endpointInfo); - ExecutionResult result = await egressOperation.ExecuteAsync(_serviceProvider, token); + try + { + EgressOperation egressOperation = new EgressOperation( + token => _dumpService.DumpAsync(endpointInfo, dumpType, token), + egressProvider, + dumpFileName, + endpointInfo, + ContentTypes.ApplicationOctetStream, + scope); - dumpFilePath = result.Result.Value; - - } - catch (Exception ex) - { - throw new CollectionRuleActionException(ex); - } + ExecutionResult result = await egressOperation.ExecuteAsync(_serviceProvider, token); + + dumpFilePath = result.Result.Value; + } + catch (Exception ex) + { + throw new CollectionRuleActionException(ex); } return new CollectionRuleActionResult() { OutputValues = new Dictionary(StringComparer.Ordinal) { - { "EgressPath", dumpFilePath } + { egressPath, dumpFilePath } } }; } From 19ccda917a2938445ffc6fa3328cdb16068dce3e Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Mon, 13 Sep 2021 08:56:17 -0700 Subject: [PATCH 21/22] Formatting tweaks and removing some unused using statements. --- .../Controllers/DiagController.cs | 3 --- .../CollectDumpActionTests.cs | 5 +++-- .../EndpointUtilities.cs | 1 - 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index c5cc6c0d741..fa502735a9b 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -16,12 +16,9 @@ using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; -using System.Diagnostics; using System.IO; using System.Linq; using System.Reflection; -using System.Net; -using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index 68e007c8ada..a45a6effdc3 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -47,7 +47,8 @@ public async Task CollectDumpAction_Success(DumpType? dumpType) { DirectoryInfo uniqueEgressDirectory = null; - try { + try + { uniqueEgressDirectory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), TempEgressDirectory, Guid.NewGuid().ToString())); await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => @@ -105,4 +106,4 @@ await runner.ExecuteAsync(async () => } } } -} +} \ No newline at end of file diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs index c22e5cac555..6ee2a8a184c 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs @@ -8,7 +8,6 @@ using Microsoft.Diagnostics.Tools.Monitor; using System; using System.Collections.Generic; -using System.Linq; using System.Reflection; using System.Threading; using System.Threading.Tasks; From 6ffdb5b467047edb290293741a2e9d5b9b3789c0 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Mon, 13 Sep 2021 13:55:36 -0700 Subject: [PATCH 22/22] Rename for Justin. --- .../CollectDumpActionTests.cs | 2 +- .../CollectionRules/Actions/CollectDumpAction.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs index a45a6effdc3..22679076298 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectDumpActionTests.cs @@ -82,7 +82,7 @@ await runner.ExecuteAsync(async () => CollectionRuleActionResult result = await action.ExecuteAsync(options, endpointInfo, cancellationTokenSource.Token); Assert.NotNull(result.OutputValues); - Assert.True(result.OutputValues.TryGetValue(CollectDumpAction.egressPath, out string egressPath)); + Assert.True(result.OutputValues.TryGetValue(CollectDumpAction.EgressPathOutputValueName, out string egressPath)); Assert.True(File.Exists(egressPath)); using FileStream dumpStream = new(egressPath, FileMode.Open, FileAccess.Read); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs index 376732e199d..7ad29571431 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectDumpAction.cs @@ -21,7 +21,7 @@ internal sealed class CollectDumpAction : ICollectionRuleAction ExecuteAsync(CollectDumpOptions op { OutputValues = new Dictionary(StringComparer.Ordinal) { - { egressPath, dumpFilePath } + { EgressPathOutputValueName, dumpFilePath } } }; }