From a2c9bfaa2aa0896f220ec039e4770e33c7527106 Mon Sep 17 00:00:00 2001 From: Claudiu Guiman Date: Thu, 6 Jun 2024 17:41:37 -0400 Subject: [PATCH] Enable parameter capturing in no suspend mode. (#6778) * Enable parameter capturing in no suspend mode. * Change the suspension mode in the configureApp callback * Update ScenarioRunner.SingleTarget to support starting the app before the tool and add a test to verify the feature doesn't work if the startup hook is manually configured. * docs update --- documentation/api/parameters.md | 5 +- .../ParameterCapturingTests.cs | 90 ++++++++++++++----- .../Runners/ScenarioRunner.cs | 76 +++++++++++----- .../Commands/CollectCommandHandler.cs | 6 +- 4 files changed, 129 insertions(+), 48 deletions(-) diff --git a/documentation/api/parameters.md b/documentation/api/parameters.md index d20f5431334..93891024716 100644 --- a/documentation/api/parameters.md +++ b/documentation/api/parameters.md @@ -128,11 +128,10 @@ Content-Type: application/x-ndjson ## Additional Requirements -- The target application must use ASP.NET Core. - The target application cannot have [Hot Reload](https://learn.microsoft.com/visualstudio/debugger/hot-reload) enabled. -- `dotnet-monitor` must be set to `Listen` mode, and the target application must start suspended. See [diagnostic port configuration](../configuration/diagnostic-port-configuration.md) for information on how to do this. +- `dotnet-monitor` must be set to `Listen` mode. See [diagnostic port configuration](../configuration/diagnostic-port-configuration.md) for information on how to do this. +- If the target application is using .NET 7 then the dotnet-monitor startup hook must be manually configured and the target application must start suspended. In .NET 8+ this is not a requirement. - This feature relies on a [ICorProfilerCallback](https://docs.microsoft.com/dotnet/framework/unmanaged-api/profiling/icorprofilercallback-interface) implementation. If the target application is already using an `ICorProfiler` that isn't notify-only, this feature will not be available. -- If a target application is using .NET 7 then the `dotnet-monitor` startup hook must be configured. This is automatically done in .NET 8+. ## Additional Notes diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/ParameterCapturingTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/ParameterCapturingTests.cs index e155b0788d4..55da1356767 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/ParameterCapturingTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/ParameterCapturingTests.cs @@ -51,7 +51,12 @@ public ParameterCapturingTests(ITestOutputHelper outputHelper, ServiceProviderFi [MemberData(nameof(ProfilerHelper.GetArchitecture), MemberType = typeof(ProfilerHelper))] public async Task UnresolvableMethodsFailsOperation(Architecture targetArchitecture) { - await RunTestCaseCore(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, targetArchitecture, async (appRunner, apiClient) => + await RunTestCaseCore( + TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, + targetArchitecture, + shouldSuspendTargetApp: true, + enableStartupHook: true, + async (appRunner, apiClient) => { int processId = await appRunner.ProcessIdTask; @@ -78,40 +83,52 @@ await RunTestCaseCore(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp [Theory] [MemberData(nameof(ProfilerHelper.GetArchitecture), MemberType = typeof(ProfilerHelper))] public Task CapturesParametersInNonAspNetApps(Architecture targetArchitecture) => - CapturesParametersCore(TestAppScenarios.ParameterCapturing.SubScenarios.NonAspNetApp, targetArchitecture, CapturedParameterFormat.JsonSequence); + CapturesParametersCore(TestAppScenarios.ParameterCapturing.SubScenarios.NonAspNetApp, targetArchitecture, CapturedParameterFormat.JsonSequence, shouldSuspendTargetApp: true, enableStartupHook: true); [Theory] [MemberData(nameof(ProfilerHelper.GetArchitecture), MemberType = typeof(ProfilerHelper))] public Task CapturesParametersAndOutputJsonSequence(Architecture targetArchitecture) => - CapturesParametersCore(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, targetArchitecture, CapturedParameterFormat.JsonSequence); + CapturesParametersCore(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, targetArchitecture, CapturedParameterFormat.JsonSequence, shouldSuspendTargetApp: true, enableStartupHook: true); [Theory] [MemberData(nameof(ProfilerHelper.GetArchitecture), MemberType = typeof(ProfilerHelper))] public Task CapturesParametersAndOutputNewlineDelimitedJson(Architecture targetArchitecture) => - CapturesParametersCore(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, targetArchitecture, CapturedParameterFormat.NewlineDelimitedJson); + CapturesParametersCore(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, targetArchitecture, CapturedParameterFormat.NewlineDelimitedJson, shouldSuspendTargetApp: true, enableStartupHook: true); -#else // NET7_0_OR_GREATER +#if NET8_0_OR_GREATER [Theory] [MemberData(nameof(ProfilerHelper.GetArchitecture), MemberType = typeof(ProfilerHelper))] - public async Task Net6AppFailsOperation(Architecture targetArchitecture) - { - await RunTestCaseCore(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, targetArchitecture, async (appRunner, apiClient) => - { - int processId = await appRunner.ProcessIdTask; + public Task CapturesParametersNoSuspend(Architecture targetArchitecture) => + CapturesParametersCore(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, targetArchitecture, CapturedParameterFormat.JsonSequence, shouldSuspendTargetApp: false, enableStartupHook: false); - CaptureParametersConfiguration config = GetValidConfiguration(); +#endif // NET8_0_OR_GREATER - ValidationProblemDetailsException validationException = await Assert.ThrowsAsync(() => apiClient.CaptureParametersAsync(processId, Timeout.InfiniteTimeSpan, config)); - Assert.Equal(HttpStatusCode.BadRequest, validationException.StatusCode); + [Theory] + [MemberData(nameof(ProfilerHelper.GetArchitecture), MemberType = typeof(ProfilerHelper))] + public Task AppWithStartupHookFailsInNoSuspend(Architecture targetArchitecture) => + ValidateBadRequestFailure(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, targetArchitecture, shouldSuspendTargetApp: false, enableStartupHook: true); + +#else // NET7_0_OR_GREATER + [Theory] + [MemberData(nameof(ProfilerHelper.GetArchitecture), MemberType = typeof(ProfilerHelper))] + public Task Net6AppFailsOperation(Architecture targetArchitecture) => + ValidateBadRequestFailure(TestAppScenarios.ParameterCapturing.SubScenarios.AspNetApp, targetArchitecture, shouldSuspendTargetApp: true, enableStartupHook: true); - await appRunner.SendCommandAsync(TestAppScenarios.ParameterCapturing.Commands.Continue); - }); - } #endif // NET7_0_OR_GREATER - private async Task CapturesParametersCore(string subScenarioName,Architecture targetArchitecture, CapturedParameterFormat format) + private async Task CapturesParametersCore( + string subScenarioName, + Architecture targetArchitecture, + CapturedParameterFormat format, + bool shouldSuspendTargetApp, + bool enableStartupHook) { - await RunTestCaseCore(subScenarioName, targetArchitecture, async (appRunner, apiClient) => + await RunTestCaseCore( + subScenarioName, + targetArchitecture, + shouldSuspendTargetApp: shouldSuspendTargetApp, + enableStartupHook: enableStartupHook, + async (appRunner, apiClient) => { int processId = await appRunner.ProcessIdTask; @@ -143,7 +160,36 @@ await RunTestCaseCore(subScenarioName, targetArchitecture, async (appRunner, api }); } - private async Task RunTestCaseCore(string subScenarioName, Architecture targetArchitecture, Func appValidate) + private async Task ValidateBadRequestFailure( + string subScenarioName, + Architecture targetArchitecture, + bool shouldSuspendTargetApp, + bool enableStartupHook) + { + await RunTestCaseCore( + subScenarioName, + targetArchitecture, + shouldSuspendTargetApp: shouldSuspendTargetApp, + enableStartupHook: enableStartupHook, + async (appRunner, apiClient) => + { + int processId = await appRunner.ProcessIdTask; + + CaptureParametersConfiguration config = GetValidConfiguration(); + + ValidationProblemDetailsException validationException = await Assert.ThrowsAsync(() => apiClient.CaptureParametersAsync(processId, Timeout.InfiniteTimeSpan, config)); + Assert.Equal(HttpStatusCode.BadRequest, validationException.StatusCode); + + await appRunner.SendCommandAsync(TestAppScenarios.ParameterCapturing.Commands.Continue); + }); + } + + private async Task RunTestCaseCore( + string subScenarioName, + Architecture targetArchitecture, + bool shouldSuspendTargetApp, + bool enableStartupHook, + Func appValidate) { await ScenarioRunner.SingleTarget( _outputHelper, @@ -153,8 +199,9 @@ await ScenarioRunner.SingleTarget( appValidate: appValidate, configureApp: runner => { - runner.EnableMonitorStartupHook = true; + runner.EnableMonitorStartupHook = enableStartupHook; runner.Architecture = targetArchitecture; + runner.DiagnosticPortSuspend = shouldSuspendTargetApp; }, configureTool: (toolRunner) => { @@ -166,7 +213,8 @@ await ScenarioRunner.SingleTarget( toolRunner.WriteKeyPerValueConfiguration(new RootOptions().AddFileSystemEgress(FileProviderName, _tempDirectory.FullName)); }, profilerLogLevel: LogLevel.Trace, - subScenarioName: subScenarioName); + subScenarioName: subScenarioName, + startAppBeforeTool: !shouldSuspendTargetApp); } private static CaptureParametersConfiguration GetValidConfiguration() 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 341e3ad526a..ce60896f26b 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/ScenarioRunner.cs @@ -28,7 +28,8 @@ public static async Task SingleTarget( Action configureTool = null, bool disableHttpEgress = false, LogLevel profilerLogLevel = LogLevel.Error, - string subScenarioName = null) + string subScenarioName = null, + bool startAppBeforeTool = false) { DiagnosticPortHelper.Generate( mode, @@ -43,37 +44,66 @@ public static async Task SingleTarget( configureTool?.Invoke(toolRunner); - await toolRunner.StartAsync(); - - using HttpClient httpClient = await toolRunner.CreateHttpClientDefaultAddressAsync(httpClientFactory); - ApiClient apiClient = new(outputHelper, httpClient); - - await using AppRunner appRunner = new(outputHelper, Assembly.GetExecutingAssembly()); - if (profilerLogLevel != LogLevel.None) + if (!startAppBeforeTool) { - appRunner.ProfilerLogLevel = profilerLogLevel.ToString("G"); + await toolRunner.StartAsync(); } - appRunner.ConnectionMode = appConnectionMode; - appRunner.DiagnosticPortPath = diagnosticPortPath; - appRunner.ScenarioName = scenarioName; - appRunner.SubScenarioName = subScenarioName; - configureApp?.Invoke(appRunner); +#nullable enable + HttpClient? httpClient = null; + ApiClient? apiClient = null; - await appRunner.ExecuteAsync(async () => + try { - // Wait for the process to be discovered. - int processId = await appRunner.ProcessIdTask; - _ = await apiClient.GetProcessWithRetryAsync(outputHelper, pid: processId); + if (!startAppBeforeTool) + { + httpClient = await toolRunner.CreateHttpClientDefaultAddressAsync(httpClientFactory); + apiClient = new(outputHelper, httpClient); + } + + await using AppRunner appRunner = new(outputHelper, Assembly.GetExecutingAssembly()); + if (profilerLogLevel != LogLevel.None) + { + appRunner.ProfilerLogLevel = profilerLogLevel.ToString("G"); + } + appRunner.ConnectionMode = appConnectionMode; + appRunner.DiagnosticPortPath = diagnosticPortPath; + appRunner.ScenarioName = scenarioName; + appRunner.SubScenarioName = subScenarioName; + + configureApp?.Invoke(appRunner); + + await appRunner.ExecuteAsync(async () => + { + if (startAppBeforeTool) + { + await toolRunner.StartAsync(); + httpClient = await toolRunner.CreateHttpClientDefaultAddressAsync(httpClientFactory); + apiClient = new(outputHelper, httpClient); + } - await appValidate(appRunner, apiClient); - }); - Assert.Equal(0, appRunner.ExitCode); + // Wait for the process to be discovered. + int processId = await appRunner.ProcessIdTask; + _ = await apiClient.GetProcessWithRetryAsync(outputHelper, pid: processId); - if (null != postAppValidate) + Assert.NotNull(apiClient); + + await appValidate(appRunner, apiClient); + }); + Assert.Equal(0, appRunner.ExitCode); + + Assert.NotNull(apiClient); + + if (null != postAppValidate) + { + await postAppValidate(apiClient, await appRunner.ProcessIdTask); + } + } + finally { - await postAppValidate(apiClient, await appRunner.ProcessIdTask); + httpClient?.Dispose(); } +#nullable disable } } } diff --git a/src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs b/src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs index b14fd48fa18..20134f09fab 100644 --- a/src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs +++ b/src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs @@ -131,11 +131,15 @@ private static IHostBuilder Configure(this IHostBuilder builder, StartupAuthenti services.AddSingleton(); services.ConfigureCollectionRules(); services.ConfigureLibrarySharing(); + /* + * ConfigureInProcessFeatures needs to be called before ConfigureProfiler + * because the profiler needs to have access to environment variables set by in process features. + */ + services.ConfigureInProcessFeatures(context.Configuration); services.ConfigureProfiler(); services.ConfigureStartupHook(); services.ConfigureExceptions(); services.ConfigureStartupLoggers(authConfigurator); - services.ConfigureInProcessFeatures(context.Configuration); services.AddSingleton(); services.AddSingleton(); services.AddSingleton();