From b054923bde00ebf9c80d8e96e2b4bb1d0f8bac77 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 14:12:16 -0700 Subject: [PATCH 01/45] Initial refactor --- .../Controllers/DiagController.Metrics.cs | 2 - .../Controllers/DiagController.cs | 37 ++++++---- .../Models/EgressOperationStatus.cs | 3 + .../Operation/EgressOperation.cs | 19 +++-- .../Operation/EgressOperationStore.cs | 2 + .../Operation/IEgressOperation.cs | 2 + .../RequestLimitAttribute.cs | 14 ---- .../dotnet-monitor/RequestLimitMiddleware.cs | 71 ------------------- src/Tools/dotnet-monitor/Startup.cs | 3 - 9 files changed, 41 insertions(+), 112 deletions(-) delete mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitAttribute.cs delete mode 100644 src/Tools/dotnet-monitor/RequestLimitMiddleware.cs diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs index 7137822e1b4..9e8bfb9b4e3 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs @@ -25,7 +25,6 @@ partial class DiagController [ProducesWithProblemDetails(ContentTypes.ApplicationJsonSequence)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = Utilities.ArtifactType_Metrics)] [EgressValidation] public Task CaptureMetrics( [FromQuery] @@ -75,7 +74,6 @@ public Task CaptureMetrics( [ProducesWithProblemDetails(ContentTypes.ApplicationJsonSequence)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = Utilities.ArtifactType_Metrics)] [EgressValidation] public Task CaptureMetricsCustom( [FromBody][Required] diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 313faa8e0f7..83d53a2f524 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -207,7 +207,6 @@ public Task>> GetProcessEnvironment( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(FileResult), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = Utilities.ArtifactType_Dump)] [EgressValidation] public Task CaptureDump( [FromQuery] @@ -266,7 +265,6 @@ public Task CaptureDump( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(FileResult), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = Utilities.ArtifactType_GCDump)] [EgressValidation] public Task CaptureGcDump( [FromQuery] @@ -325,7 +323,6 @@ public Task CaptureGcDump( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(FileResult), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = Utilities.ArtifactType_Trace)] [EgressValidation] public Task CaptureTrace( [FromQuery] @@ -369,7 +366,6 @@ public Task CaptureTrace( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(FileResult), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = Utilities.ArtifactType_Trace)] [EgressValidation] public Task CaptureTraceCustom( [FromBody][Required] @@ -420,7 +416,6 @@ public Task CaptureTraceCustom( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(string), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = Utilities.ArtifactType_Logs)] [EgressValidation] public Task CaptureLogs( [FromQuery] @@ -476,7 +471,6 @@ public Task CaptureLogs( [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(string), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = Utilities.ArtifactType_Logs)] [EgressValidation] public Task CaptureLogsCustom( [FromBody] @@ -593,7 +587,6 @@ public Task> GetCollectionRuleDe [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status429TooManyRequests)] [ProducesResponseType(typeof(string), StatusCodes.Status200OK)] [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] - [RequestLimit(LimitKey = Utilities.ArtifactType_Stacks)] [EgressValidation] public async Task CaptureStacks( [FromQuery] @@ -725,7 +718,7 @@ private Task StartLogs( return null; } - private Task Result( + private async Task Result( string artifactType, string providerName, Func action, @@ -738,15 +731,24 @@ private Task Result( if (string.IsNullOrEmpty(providerName)) { - return Task.FromResult(new OutputStreamResult( + HttpContext.Response.Headers["href"] = await RegisterOperation(egressOperation: null, limitKey: artifactType); + var foo = new EgressOperation( + action, + providerName, + fileName, + processInfo, + contentType, + scope); + + return new OutputStreamResult( action, contentType, asAttachment ? fileName : null, - scope)); + scope); } else { - return SendToEgress(new EgressOperation( + return await SendToEgress(new EgressOperation( action, providerName, fileName, @@ -757,16 +759,21 @@ private Task Result( } } - private async Task SendToEgress(EgressOperation egressStreamResult, string limitKey) + + private async Task RegisterOperation(EgressOperation egressOperation, string limitKey) { // Will throw TooManyRequestsException if there are too many concurrent operations. - Guid operationId = await _operationsStore.AddOperation(egressStreamResult, limitKey); - string newUrl = this.Url.Action( + Guid operationId = await _operationsStore.AddOperation(egressOperation, limitKey); + return this.Url.Action( action: nameof(OperationsController.GetOperationStatus), controller: OperationsController.ControllerName, new { operationId = operationId }, protocol: this.HttpContext.Request.Scheme, this.HttpContext.Request.Host.ToString()); + } - return Accepted(newUrl); + private async Task SendToEgress(EgressOperation egressOperation, string limitKey) + { + string operationUrl = await RegisterOperation(egressOperation, limitKey); + return Accepted(operationUrl); } private Task InvokeForProcess(Func func, ProcessKey? processKey, string artifactType = null) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs index 54edfca8e24..fee6e4730d6 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs @@ -23,6 +23,9 @@ public class OperationSummary [JsonPropertyName("process")] public OperationProcessInfo Process { get; set; } + + [JsonPropertyName("egressProviderName")] + public string EgressProviderName { get; set; } } /// diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs index 756a9ab3d71..a3e29f99dc6 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs @@ -14,23 +14,23 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi internal class EgressOperation : IEgressOperation { private readonly Func> _egress; - private readonly string _egressProvider; private readonly KeyValueLogScope _scope; public EgressProcessInfo ProcessInfo { get; private set; } + public string EgressProviderName { get; private set; } public EgressOperation(Func> action, string endpointName, string artifactName, IProcessInfo processInfo, string contentType, KeyValueLogScope scope, CollectionRuleMetadata collectionRuleMetadata = null) { _egress = (service, token) => service.EgressAsync(endpointName, action, artifactName, contentType, processInfo.EndpointInfo, collectionRuleMetadata, token); - _egressProvider = endpointName; _scope = scope; + EgressProviderName = endpointName; ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); } public EgressOperation(Func action, string endpointName, string artifactName, IProcessInfo processInfo, string contentType, KeyValueLogScope scope, CollectionRuleMetadata collectionRuleMetadata = null) { _egress = (service, token) => service.EgressAsync(endpointName, action, artifactName, contentType, processInfo.EndpointInfo, collectionRuleMetadata, token); - _egressProvider = endpointName; + EgressProviderName = endpointName; _scope = scope; ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); @@ -40,14 +40,14 @@ public EgressOperation(Func action, string endp public EgressOperation(Func action, string endpointName, string artifactName, IEndpointInfo source, string contentType, KeyValueLogScope scope, CollectionRuleMetadata collectionRuleMetadata) { _egress = (service, token) => service.EgressAsync(endpointName, action, artifactName, contentType, source, collectionRuleMetadata, token); - _egressProvider = endpointName; + EgressProviderName = endpointName; _scope = scope; } public EgressOperation(Func> action, string endpointName, string artifactName, IEndpointInfo source, string contentType, KeyValueLogScope scope, CollectionRuleMetadata collectionRuleMetadata) { _egress = (service, token) => service.EgressAsync(endpointName, action, artifactName, contentType, source, collectionRuleMetadata, token); - _egressProvider = endpointName; + EgressProviderName = endpointName; _scope = scope; } @@ -76,12 +76,17 @@ public async Task> ExecuteAsync(IServiceProvider s return ExecutionResult.Succeeded(egressResult); }, logger, token); } - + public void Validate(IServiceProvider serviceProvider) { + if (string.IsNullOrEmpty(EgressProviderName)) + { + return; + } + serviceProvider .GetRequiredService() - .ValidateProvider(_egressProvider); + .ValidateProvider(EgressProviderName); } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs index 4408ab7f569..6b01c329f7d 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs @@ -164,6 +164,7 @@ public void CompleteOperation(Guid operationId, ExecutionResult re OperationId = kvp.Key, CreatedDateTime = kvp.Value.CreatedDateTime, Status = kvp.Value.State, + EgressProviderName = kvp.Value.EgressRequest.EgressOperation.EgressProviderName, Process = processInfo != null ? new Models.OperationProcessInfo { @@ -191,6 +192,7 @@ public Models.OperationStatus GetOperationStatus(Guid operationId) OperationId = entry.EgressRequest.OperationId, Status = entry.State, CreatedDateTime = entry.CreatedDateTime, + EgressProviderName = entry.EgressRequest.EgressOperation.EgressProviderName, Process = processInfo != null ? new Models.OperationProcessInfo { diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs index 057fec61971..4ba00a8f88b 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs @@ -10,6 +10,8 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi { internal interface IEgressOperation { + public string EgressProviderName { get; } + public EgressProcessInfo ProcessInfo { get; } Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitAttribute.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitAttribute.cs deleted file mode 100644 index a6347efed62..00000000000 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitAttribute.cs +++ /dev/null @@ -1,14 +0,0 @@ -// 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; - -namespace Microsoft.Diagnostics.Monitoring.WebApi -{ - [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class)] - internal sealed class RequestLimitAttribute : Attribute - { - public string LimitKey { get; set; } - } -} diff --git a/src/Tools/dotnet-monitor/RequestLimitMiddleware.cs b/src/Tools/dotnet-monitor/RequestLimitMiddleware.cs deleted file mode 100644 index c65d8e667dd..00000000000 --- a/src/Tools/dotnet-monitor/RequestLimitMiddleware.cs +++ /dev/null @@ -1,71 +0,0 @@ -// 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.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; -using Microsoft.Diagnostics.Monitoring.WebApi; -using System; -using System.Text.Json; -using System.Threading.Tasks; - -namespace Microsoft.Diagnostics.Tools.Monitor -{ - /// - /// Limits the amount of requests that can be sent to the server. - /// - The current rate limits are based on concurrent requests to the server from any source on a per endpoint basis. - /// - Note we do not use Microsoft.AspNetCore.ConcurrencyLimiter because it works over the whole application instead of per endpoint. - /// - In the future, we may want to switch to https://github.com/dotnet/aspnetcore/issues/29933 - /// TODO For asp.net 2.1, this would be implemented as an ActionFilter. For 3.1+, we use an endpoints + middleware - /// - internal sealed class RequestLimitMiddleware - { - private readonly RequestDelegate _next; - - private readonly RequestLimitTracker _limitTracker; - private const string EgressQuery = "egressprovider"; - - public RequestLimitMiddleware(RequestDelegate next, RequestLimitTracker requestLimitTracker) - { - _next = next; - _limitTracker = requestLimitTracker; - } - - public async Task Invoke(HttpContext context) - { - var endpoint = context.GetEndpoint(); - - RequestLimitAttribute requestLimit = endpoint?.Metadata.GetMetadata(); - IDisposable incrementor = null; - - try - { - //Operations and middleware both share the same increment limits, but - //we don't want the middleware to increment the limit if the operation is doing it as well. - if ((requestLimit != null) && !context.Request.Query.ContainsKey(EgressQuery)) - { - incrementor = _limitTracker.Increment(requestLimit.LimitKey, out bool allowOperation); - if (!allowOperation) - { - - //We should report the same kind of error from Middleware and the Mvc layer. - context.Response.StatusCode = StatusCodes.Status429TooManyRequests; - context.Response.ContentType = ContentTypes.ApplicationProblemJson; - await context.Response.WriteAsync(JsonSerializer.Serialize(new ProblemDetails - { - Status = StatusCodes.Status429TooManyRequests, - Detail = Microsoft.Diagnostics.Monitoring.WebApi.Strings.ErrorMessage_TooManyRequests - }), context.RequestAborted); - return; - } - } - - await _next(context); - } - finally - { - incrementor?.Dispose(); - } - } - } -} diff --git a/src/Tools/dotnet-monitor/Startup.cs b/src/Tools/dotnet-monitor/Startup.cs index f91b19437ed..a45bc836cd0 100644 --- a/src/Tools/dotnet-monitor/Startup.cs +++ b/src/Tools/dotnet-monitor/Startup.cs @@ -95,9 +95,6 @@ public static void Configure(IApplicationBuilder app, IWebHostEnvironment env, I // https://github.com/dotnet/aspnetcore/issues/36960 //app.UseResponseCompression(); - //Note this must be after UseRouting but before UseEndpoints - app.UseMiddleware(); - app.UseEndpoints(builder => { builder.MapControllers(); From ad8b7c1aafb06535b244811a7ad92c6359000c64 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 14:23:51 -0700 Subject: [PATCH 02/45] Shim egress operations --- .../Controllers/DiagController.cs | 15 ++++------ .../Operation/EgressOperation.cs | 19 ------------ .../Operation/EgressProcessInfo.cs | 22 ++++++++++++++ .../Operation/HttpResponseEgressOperation.cs | 30 +++++++++++++++++++ 4 files changed, 57 insertions(+), 29 deletions(-) create mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressProcessInfo.cs create mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 83d53a2f524..6577f159398 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -731,14 +731,9 @@ private async Task Result( if (string.IsNullOrEmpty(providerName)) { - HttpContext.Response.Headers["href"] = await RegisterOperation(egressOperation: null, limitKey: artifactType); - var foo = new EgressOperation( - action, - providerName, - fileName, - processInfo, - contentType, - scope); + HttpContext.Response.Headers["href"] = await RegisterOperation( + new HttpResponseEgressOperation(processInfo), + limitKey: artifactType); return new OutputStreamResult( action, @@ -760,7 +755,7 @@ private async Task Result( } - private async Task RegisterOperation(EgressOperation egressOperation, string limitKey) + private async Task RegisterOperation(IEgressOperation egressOperation, string limitKey) { // Will throw TooManyRequestsException if there are too many concurrent operations. Guid operationId = await _operationsStore.AddOperation(egressOperation, limitKey); @@ -770,7 +765,7 @@ private async Task RegisterOperation(EgressOperation egressOperation, st protocol: this.HttpContext.Request.Scheme, this.HttpContext.Request.Host.ToString()); } - private async Task SendToEgress(EgressOperation egressOperation, string limitKey) + private async Task SendToEgress(IEgressOperation egressOperation, string limitKey) { string operationUrl = await RegisterOperation(egressOperation, limitKey); return Accepted(operationUrl); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs index a3e29f99dc6..5468aab8f5c 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs @@ -79,28 +79,9 @@ public async Task> ExecuteAsync(IServiceProvider s public void Validate(IServiceProvider serviceProvider) { - if (string.IsNullOrEmpty(EgressProviderName)) - { - return; - } - serviceProvider .GetRequiredService() .ValidateProvider(EgressProviderName); } } - - internal class EgressProcessInfo - { - public string ProcessName { get; } - public int ProcessId { get; } - public Guid RuntimeInstanceCookie { get; } - - public EgressProcessInfo(string processName, int processId, Guid runtimeInstanceCookie) - { - this.ProcessName = processName; - this.ProcessId = processId; - this.RuntimeInstanceCookie = runtimeInstanceCookie; - } - } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressProcessInfo.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressProcessInfo.cs new file mode 100644 index 00000000000..049f1f1f305 --- /dev/null +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressProcessInfo.cs @@ -0,0 +1,22 @@ +// 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; + +namespace Microsoft.Diagnostics.Monitoring.WebApi +{ + internal class EgressProcessInfo + { + public string ProcessName { get; } + public int ProcessId { get; } + public Guid RuntimeInstanceCookie { get; } + + public EgressProcessInfo(string processName, int processId, Guid runtimeInstanceCookie) + { + this.ProcessName = processName; + this.ProcessId = processId; + this.RuntimeInstanceCookie = runtimeInstanceCookie; + } + } +} diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs new file mode 100644 index 00000000000..cc3ec35c6a6 --- /dev/null +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -0,0 +1,30 @@ +// 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.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Diagnostics.Monitoring.WebApi +{ + internal class HttpResponseEgressOperation : IEgressOperation + { + public EgressProcessInfo ProcessInfo { get; private set; } + public string EgressProviderName { get { return null; } } + + public HttpResponseEgressOperation(IProcessInfo processInfo) + { + ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); + } + + public Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token) + { + return Task.FromResult(ExecutionResult.Empty()); + } + + public void Validate(IServiceProvider serviceProvider) + { + } + } +} From 42f7db802dc98f5e7df2fb2c2fa9cb24dd322cfe Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 14:42:03 -0700 Subject: [PATCH 03/45] Unify egress operations --- .../Operation/EgressOperation.cs | 2 +- .../Operation/EgressOperationService.cs | 12 +++++-- .../Operation/EgressOperationStore.cs | 34 +++++++++++-------- .../Operation/EgressRequest.cs | 2 +- .../Operation/HttpResponseEgressOperation.cs | 13 ------- .../Operation/IEgressActionableOperation.cs | 17 ++++++++++ .../Operation/IEgressOperation.cs | 8 ----- 7 files changed, 48 insertions(+), 40 deletions(-) create mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressActionableOperation.cs diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs index 5468aab8f5c..c9c4c61bc87 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs @@ -11,7 +11,7 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi { - internal class EgressOperation : IEgressOperation + internal class EgressOperation : IEgressActionableOperation { private readonly Func> _egress; private readonly KeyValueLogScope _scope; diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs index 5c59eaaf5f3..c23a4398a51 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Hosting; using System; +using System.Diagnostics; using System.Threading; using System.Threading.Tasks; @@ -29,13 +30,18 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) while (!stoppingToken.IsCancellationRequested) { EgressRequest egressRequest = await _queue.DequeueAsync(stoppingToken); + if (egressRequest.EgressOperation is not IEgressActionableOperation egressOperation) + { + Debug.Fail($"Unexpected non-actionable egress operation in the queue."); + continue; + } //Note we do not await these tasks, but we do limit how many can be executed at the same time - _ = Task.Run(() => ExecuteEgressOperation(egressRequest, stoppingToken), stoppingToken); + _ = Task.Run(() => ExecuteEgressOperation(egressRequest, egressOperation, stoppingToken), stoppingToken); } } - private async Task ExecuteEgressOperation(EgressRequest egressRequest, CancellationToken stoppingToken) + private async Task ExecuteEgressOperation(EgressRequest egressRequest, IEgressActionableOperation egressOperation, CancellationToken stoppingToken) { //We have two stopping tokens, one per item that can be triggered via Delete //and if we are stopping the service @@ -47,7 +53,7 @@ private async Task ExecuteEgressOperation(EgressRequest egressRequest, Cancellat try { - var result = await egressRequest.EgressOperation.ExecuteAsync(_serviceProvider, token); + var result = await egressOperation.ExecuteAsync(_serviceProvider, token); //It is possible that this operation never completes, due to infinite duration operations. _operationsStore.CompleteOperation(egressRequest.OperationId, result); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs index 6b01c329f7d..6e5f6ce0b9b 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs @@ -39,23 +39,12 @@ public EgressOperationStore( _serviceProvider = serviceProvider; } - public async Task AddOperation(IEgressOperation egressOperation, string limitKey) + public async Task AddOperation(IEgressActionableOperation egressOperation, string limitKey) { egressOperation.Validate(_serviceProvider); + Guid operationId = await AddOperation((IEgressOperation)egressOperation, limitKey); - Guid operationId = Guid.NewGuid(); - - IDisposable limitTracker = _requestLimits.Increment(limitKey, out bool allowOperation); - //We increment the limit here, and decrement it once the operation is cancelled or completed. - //We do this here so that we can provide immediate errors if the user queues up too many operations. - - if (!allowOperation) - { - limitTracker.Dispose(); - throw new TooManyRequestsException(); - } - - var request = new EgressRequest(operationId, egressOperation, limitTracker); + var request = new EgressRequest(operationId, egressOperation, null); lock (_requests) { //Add operation object to central table. @@ -74,6 +63,23 @@ public async Task AddOperation(IEgressOperation egressOperation, string li return operationId; } + public Task AddOperation(IEgressOperation egressOperation, string limitKey) + { + Guid operationId = Guid.NewGuid(); + + IDisposable limitTracker = _requestLimits.Increment(limitKey, out bool allowOperation); + //We increment the limit here, and decrement it once the operation is cancelled or completed. + //We do this here so that we can provide immediate errors if the user queues up too many operations. + + if (!allowOperation) + { + limitTracker.Dispose(); + throw new TooManyRequestsException(); + } + + return Task.FromResult(operationId); + } + public void CancelOperation(Guid operationId) { lock (_requests) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs index bb914d8ccfd..a562951e788 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs @@ -16,7 +16,7 @@ internal sealed class EgressRequest : IDisposable private bool _disposed; private IDisposable _limitTracker; - public EgressRequest(Guid operationId, IEgressOperation egressOperation, IDisposable limitTracker) + public EgressRequest(Guid operationId, IEgressActionableOperation egressOperation, IDisposable limitTracker) { OperationId = operationId; EgressOperation = egressOperation; diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index cc3ec35c6a6..aa06796270e 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -2,10 +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 System; -using System.Threading; -using System.Threading.Tasks; - namespace Microsoft.Diagnostics.Monitoring.WebApi { internal class HttpResponseEgressOperation : IEgressOperation @@ -17,14 +13,5 @@ public HttpResponseEgressOperation(IProcessInfo processInfo) { ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); } - - public Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token) - { - return Task.FromResult(ExecutionResult.Empty()); - } - - public void Validate(IServiceProvider serviceProvider) - { - } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressActionableOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressActionableOperation.cs new file mode 100644 index 00000000000..209a930f046 --- /dev/null +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressActionableOperation.cs @@ -0,0 +1,17 @@ +// 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.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Diagnostics.Monitoring.WebApi +{ + internal interface IEgressActionableOperation : IEgressOperation + { + Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token); + + void Validate(IServiceProvider serviceProvider); + } +} diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs index 4ba00a8f88b..7d19290b38f 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs @@ -2,10 +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 System; -using System.Threading; -using System.Threading.Tasks; - namespace Microsoft.Diagnostics.Monitoring.WebApi { internal interface IEgressOperation @@ -13,9 +9,5 @@ internal interface IEgressOperation public string EgressProviderName { get; } public EgressProcessInfo ProcessInfo { get; } - - Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token); - - void Validate(IServiceProvider serviceProvider); } } From 8ba2c1b332d5a8d7de8293699766db961b8db1b7 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 14:53:31 -0700 Subject: [PATCH 04/45] Support intake of non-actionable operations --- .../Operation/EgressOperationStore.cs | 40 ++++++++++--------- .../Operation/EgressRequest.cs | 2 +- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs index 6e5f6ce0b9b..6d63c2c2165 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs @@ -42,28 +42,19 @@ public EgressOperationStore( public async Task AddOperation(IEgressActionableOperation egressOperation, string limitKey) { egressOperation.Validate(_serviceProvider); - Guid operationId = await AddOperation((IEgressOperation)egressOperation, limitKey); - - var request = new EgressRequest(operationId, egressOperation, null); - lock (_requests) - { - //Add operation object to central table. - _requests.Add(operationId, - new EgressEntry - { - State = Models.OperationState.Running, - EgressRequest = request, - OperationId = operationId - }); - } - - //Kick off work to attempt egress - await _taskQueue.EnqueueAsync(request); + (Guid operationId, EgressRequest egressRequest) = AddOperationCore(egressOperation, limitKey); + await _taskQueue.EnqueueAsync(egressRequest); return operationId; } public Task AddOperation(IEgressOperation egressOperation, string limitKey) + { + (Guid operationId, _) = AddOperationCore(egressOperation, limitKey); + return Task.FromResult(operationId); + } + + private (Guid operationId, EgressRequest egressRequest) AddOperationCore(IEgressOperation egressOperation, string limitKey) { Guid operationId = Guid.NewGuid(); @@ -77,7 +68,20 @@ public Task AddOperation(IEgressOperation egressOperation, string limitKey throw new TooManyRequestsException(); } - return Task.FromResult(operationId); + var request = new EgressRequest(operationId, egressOperation, limitTracker); + lock (_requests) + { + //Add operation object to central table. + _requests.Add(operationId, + new EgressEntry + { + State = Models.OperationState.Running, + EgressRequest = request, + OperationId = operationId + }); + } + + return (operationId, request); } public void CancelOperation(Guid operationId) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs index a562951e788..bb914d8ccfd 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs @@ -16,7 +16,7 @@ internal sealed class EgressRequest : IDisposable private bool _disposed; private IDisposable _limitTracker; - public EgressRequest(Guid operationId, IEgressActionableOperation egressOperation, IDisposable limitTracker) + public EgressRequest(Guid operationId, IEgressOperation egressOperation, IDisposable limitTracker) { OperationId = operationId; EgressOperation = egressOperation; From b211782a5b3603847e6fd59bbaeb9d21c7e60638 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 14:54:29 -0700 Subject: [PATCH 05/45] Remove extra interpolation --- .../Operation/EgressOperationService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs index c23a4398a51..465099a3d90 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs @@ -32,7 +32,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) EgressRequest egressRequest = await _queue.DequeueAsync(stoppingToken); if (egressRequest.EgressOperation is not IEgressActionableOperation egressOperation) { - Debug.Fail($"Unexpected non-actionable egress operation in the queue."); + Debug.Fail("Unexpected non-actionable egress operation in the queue."); continue; } From eafdeba0445843db54e8749378f4d6d01ac8a097 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 15:07:51 -0700 Subject: [PATCH 06/45] Support graceful stops --- .../Controllers/DiagController.cs | 27 ++++++++++++------- .../Operation/EgressOperationStore.cs | 13 ++++----- .../Operation/EgressRequest.cs | 5 +++- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 6577f159398..983005097e4 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -228,6 +228,7 @@ public Task CaptureDump( if (string.IsNullOrEmpty(egressProvider)) { + await RegisterHttpResponseAsOperation(processInfo, Utilities.ArtifactType_Dump); Stream dumpStream = await _dumpService.DumpAsync(processInfo.EndpointInfo, type, HttpContext.RequestAborted); _logger.WrittenToHttpStream(); @@ -725,16 +726,14 @@ private async Task Result( string fileName, string contentType, IProcessInfo processInfo, - bool asAttachment = true) + bool asAttachment = true, + TaskCompletionSource requestGracefulStopCompletionSource = null) { KeyValueLogScope scope = Utilities.CreateArtifactScope(artifactType, processInfo.EndpointInfo); if (string.IsNullOrEmpty(providerName)) { - HttpContext.Response.Headers["href"] = await RegisterOperation( - new HttpResponseEgressOperation(processInfo), - limitKey: artifactType); - + await RegisterHttpResponseAsOperation(processInfo, artifactType, requestGracefulStopCompletionSource); return new OutputStreamResult( action, contentType, @@ -750,24 +749,32 @@ private async Task Result( processInfo, contentType, scope), - limitKey: artifactType); + limitKey: artifactType, + requestGracefulStopCompletionSource); } } + private async Task RegisterHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, TaskCompletionSource requestGracefulStopCompletionSource = null) + { + HttpContext.Response.Headers["href"] = await RegisterOperation( + new HttpResponseEgressOperation(processInfo), + limitKey: artifactType, + requestGracefulStopCompletionSource); + } - private async Task RegisterOperation(IEgressOperation egressOperation, string limitKey) + private async Task RegisterOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource) { // Will throw TooManyRequestsException if there are too many concurrent operations. - Guid operationId = await _operationsStore.AddOperation(egressOperation, limitKey); + Guid operationId = await _operationsStore.AddOperation(egressOperation, limitKey, requestGracefulStopCompletionSource); return this.Url.Action( action: nameof(OperationsController.GetOperationStatus), controller: OperationsController.ControllerName, new { operationId = operationId }, protocol: this.HttpContext.Request.Scheme, this.HttpContext.Request.Host.ToString()); } - private async Task SendToEgress(IEgressOperation egressOperation, string limitKey) + private async Task SendToEgress(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource = null) { - string operationUrl = await RegisterOperation(egressOperation, limitKey); + string operationUrl = await RegisterOperation(egressOperation, limitKey, requestGracefulStopCompletionSource); return Accepted(operationUrl); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs index 6d63c2c2165..299fc92f225 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs @@ -39,22 +39,23 @@ public EgressOperationStore( _serviceProvider = serviceProvider; } - public async Task AddOperation(IEgressActionableOperation egressOperation, string limitKey) + public async Task AddOperation(IEgressActionableOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource = null) { egressOperation.Validate(_serviceProvider); - (Guid operationId, EgressRequest egressRequest) = AddOperationCore(egressOperation, limitKey); + (Guid operationId, EgressRequest egressRequest) = AddOperationCore(egressOperation, limitKey, requestGracefulStopCompletionSource); await _taskQueue.EnqueueAsync(egressRequest); return operationId; } - public Task AddOperation(IEgressOperation egressOperation, string limitKey) + public Task AddOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource = null) { - (Guid operationId, _) = AddOperationCore(egressOperation, limitKey); + // JSFIX: Link cancellation tokens + (Guid operationId, _) = AddOperationCore(egressOperation, limitKey, requestGracefulStopCompletionSource); return Task.FromResult(operationId); } - private (Guid operationId, EgressRequest egressRequest) AddOperationCore(IEgressOperation egressOperation, string limitKey) + private (Guid operationId, EgressRequest egressRequest) AddOperationCore(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource) { Guid operationId = Guid.NewGuid(); @@ -68,7 +69,7 @@ public Task AddOperation(IEgressOperation egressOperation, string limitKey throw new TooManyRequestsException(); } - var request = new EgressRequest(operationId, egressOperation, limitTracker); + var request = new EgressRequest(operationId, egressOperation, limitTracker, requestGracefulStopCompletionSource); lock (_requests) { //Add operation object to central table. diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs index bb914d8ccfd..0555da223dd 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs @@ -4,6 +4,7 @@ using System; using System.Threading; +using System.Threading.Tasks; namespace Microsoft.Diagnostics.Monitoring.WebApi { @@ -16,14 +17,16 @@ internal sealed class EgressRequest : IDisposable private bool _disposed; private IDisposable _limitTracker; - public EgressRequest(Guid operationId, IEgressOperation egressOperation, IDisposable limitTracker) + public EgressRequest(Guid operationId, IEgressOperation egressOperation, IDisposable limitTracker, TaskCompletionSource requestGracefulStopCompletionSource) { OperationId = operationId; EgressOperation = egressOperation; + RequestGracefulStopCompletionSource = requestGracefulStopCompletionSource; _limitTracker = limitTracker; } public CancellationTokenSource CancellationTokenSource { get; } = new(); + public TaskCompletionSource RequestGracefulStopCompletionSource { get; } public Guid OperationId { get; } From 9d6741650a95a709444fa52c4616c2315eb7b72e Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 15:08:52 -0700 Subject: [PATCH 07/45] Track dump operations --- .../Controllers/DiagController.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 983005097e4..61b902a29e9 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -220,6 +220,8 @@ public Task CaptureDump( [FromQuery] string egressProvider = null) { + const string artifactType = Utilities.ArtifactType_Dump; + ProcessKey? processKey = Utilities.GetProcessKey(pid, uid, name); return InvokeForProcess(async processInfo => @@ -228,7 +230,7 @@ public Task CaptureDump( if (string.IsNullOrEmpty(egressProvider)) { - await RegisterHttpResponseAsOperation(processInfo, Utilities.ArtifactType_Dump); + await RegisterHttpResponseAsOperation(processInfo, artifactType); Stream dumpStream = await _dumpService.DumpAsync(processInfo.EndpointInfo, type, HttpContext.RequestAborted); _logger.WrittenToHttpStream(); @@ -238,7 +240,7 @@ public Task CaptureDump( } else { - KeyValueLogScope scope = Utilities.CreateArtifactScope(Utilities.ArtifactType_Dump, processInfo.EndpointInfo); + KeyValueLogScope scope = Utilities.CreateArtifactScope(artifactType, processInfo.EndpointInfo); return await SendToEgress(new EgressOperation( token => _dumpService.DumpAsync(processInfo.EndpointInfo, type, token), @@ -246,9 +248,9 @@ public Task CaptureDump( dumpFileName, processInfo, ContentTypes.ApplicationOctetStream, - scope), limitKey: Utilities.ArtifactType_Dump); + scope), limitKey: artifactType); } - }, processKey, Utilities.ArtifactType_Dump); + }, processKey, artifactType); } /// From 5f19392694dda70dc42ed91fe58f652ffd4e8bfa Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 15:41:34 -0700 Subject: [PATCH 08/45] Add stopping state --- .../Controllers/DiagController.cs | 2 +- .../Models/EgressOperationStatus.cs | 3 +- .../Operation/EgressOperation.cs | 2 +- .../Operation/EgressOperationService.cs | 12 ++---- .../Operation/EgressOperationStore.cs | 41 +++++++++---------- .../Operation/HttpResponseEgressOperation.cs | 32 ++++++++++++++- .../Operation/IEgressActionableOperation.cs | 17 -------- .../Operation/IEgressOperation.cs | 8 ++++ 8 files changed, 66 insertions(+), 51 deletions(-) delete mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressActionableOperation.cs diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 61b902a29e9..463101798b4 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -759,7 +759,7 @@ private async Task Result( private async Task RegisterHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, TaskCompletionSource requestGracefulStopCompletionSource = null) { HttpContext.Response.Headers["href"] = await RegisterOperation( - new HttpResponseEgressOperation(processInfo), + new HttpResponseEgressOperation(HttpContext, processInfo), limitKey: artifactType, requestGracefulStopCompletionSource); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs index fee6e4730d6..c1f3d7598be 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs @@ -65,7 +65,8 @@ public enum OperationState Running, Succeeded, Failed, - Cancelled + Cancelled, + Stopping } public class OperationError diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs index c9c4c61bc87..5468aab8f5c 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs @@ -11,7 +11,7 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi { - internal class EgressOperation : IEgressActionableOperation + internal class EgressOperation : IEgressOperation { private readonly Func> _egress; private readonly KeyValueLogScope _scope; diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs index 465099a3d90..5c59eaaf5f3 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs @@ -4,7 +4,6 @@ using Microsoft.Extensions.Hosting; using System; -using System.Diagnostics; using System.Threading; using System.Threading.Tasks; @@ -30,18 +29,13 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) while (!stoppingToken.IsCancellationRequested) { EgressRequest egressRequest = await _queue.DequeueAsync(stoppingToken); - if (egressRequest.EgressOperation is not IEgressActionableOperation egressOperation) - { - Debug.Fail("Unexpected non-actionable egress operation in the queue."); - continue; - } //Note we do not await these tasks, but we do limit how many can be executed at the same time - _ = Task.Run(() => ExecuteEgressOperation(egressRequest, egressOperation, stoppingToken), stoppingToken); + _ = Task.Run(() => ExecuteEgressOperation(egressRequest, stoppingToken), stoppingToken); } } - private async Task ExecuteEgressOperation(EgressRequest egressRequest, IEgressActionableOperation egressOperation, CancellationToken stoppingToken) + private async Task ExecuteEgressOperation(EgressRequest egressRequest, CancellationToken stoppingToken) { //We have two stopping tokens, one per item that can be triggered via Delete //and if we are stopping the service @@ -53,7 +47,7 @@ private async Task ExecuteEgressOperation(EgressRequest egressRequest, IEgressAc try { - var result = await egressOperation.ExecuteAsync(_serviceProvider, token); + var result = await egressRequest.EgressOperation.ExecuteAsync(_serviceProvider, token); //It is possible that this operation never completes, due to infinite duration operations. _operationsStore.CompleteOperation(egressRequest.OperationId, result); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs index 299fc92f225..c558b13c0e6 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs @@ -39,24 +39,10 @@ public EgressOperationStore( _serviceProvider = serviceProvider; } - public async Task AddOperation(IEgressActionableOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource = null) + public async Task AddOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource = null) { egressOperation.Validate(_serviceProvider); - (Guid operationId, EgressRequest egressRequest) = AddOperationCore(egressOperation, limitKey, requestGracefulStopCompletionSource); - await _taskQueue.EnqueueAsync(egressRequest); - return operationId; - } - - public Task AddOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource = null) - { - // JSFIX: Link cancellation tokens - (Guid operationId, _) = AddOperationCore(egressOperation, limitKey, requestGracefulStopCompletionSource); - return Task.FromResult(operationId); - } - - private (Guid operationId, EgressRequest egressRequest) AddOperationCore(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource) - { Guid operationId = Guid.NewGuid(); IDisposable limitTracker = _requestLimits.Increment(limitKey, out bool allowOperation); @@ -81,11 +67,12 @@ public Task AddOperation(IEgressOperation egressOperation, string limitKey OperationId = operationId }); } + await _taskQueue.EnqueueAsync(request); - return (operationId, request); + return operationId; } - public void CancelOperation(Guid operationId) + public void CancelOperation(Guid operationId, bool gracefulStop = false) { lock (_requests) { @@ -94,13 +81,23 @@ public void CancelOperation(Guid operationId) throw new InvalidOperationException(Strings.ErrorMessage_OperationNotFound); } - if (entry.State != Models.OperationState.Running) + if (entry.State != Models.OperationState.Running && + entry.State != Models.OperationState.Stopping) { throw new InvalidOperationException(Strings.ErrorMessage_OperationNotRunning); } - entry.State = Models.OperationState.Cancelled; - entry.EgressRequest.CancellationTokenSource.Cancel(); + if (gracefulStop && entry.EgressRequest.RequestGracefulStopCompletionSource != null) + { + entry.EgressRequest.RequestGracefulStopCompletionSource.TrySetResult(null); + entry.State = Models.OperationState.Stopping; + } + else + { + entry.EgressRequest.CancellationTokenSource.Cancel(); + entry.State = Models.OperationState.Cancelled; + } + entry.EgressRequest.Dispose(); } } @@ -113,7 +110,9 @@ public void CompleteOperation(Guid operationId, ExecutionResult re { throw new InvalidOperationException(Strings.ErrorMessage_OperationNotFound); } - if (entry.State != Models.OperationState.Running) + + if (entry.State != Models.OperationState.Running && + entry.State != Models.OperationState.Stopping) { throw new InvalidOperationException(Strings.ErrorMessage_OperationNotRunning); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index aa06796270e..d3b5c970a12 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -2,16 +2,46 @@ // 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.Http; +using System; +using System.Net; +using System.Threading; +using System.Threading.Tasks; + namespace Microsoft.Diagnostics.Monitoring.WebApi { internal class HttpResponseEgressOperation : IEgressOperation { + private readonly HttpContext _httpContext; + private readonly TaskCompletionSource _responseFinishedCompletionSource = new(); + public EgressProcessInfo ProcessInfo { get; private set; } public string EgressProviderName { get { return null; } } - public HttpResponseEgressOperation(IProcessInfo processInfo) + public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo) { + _httpContext = context; + // JSFIX: investigate if the user closes the stream + _httpContext.Response.OnCompleted((_) => + { + _responseFinishedCompletionSource.TrySetResult(_httpContext.Response.StatusCode); + return Task.CompletedTask; + }, null); + ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); } + + public async Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token) + { + int statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(token); + return statusCode == (int)HttpStatusCode.OK + ? ExecutionResult.Empty() + : ExecutionResult.Failed(new Exception($"HTTP request failed with status code: ${statusCode}")); + } + + public void Validate(IServiceProvider serviceProvider) + { + // noop + } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressActionableOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressActionableOperation.cs deleted file mode 100644 index 209a930f046..00000000000 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressActionableOperation.cs +++ /dev/null @@ -1,17 +0,0 @@ -// 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.Threading; -using System.Threading.Tasks; - -namespace Microsoft.Diagnostics.Monitoring.WebApi -{ - internal interface IEgressActionableOperation : IEgressOperation - { - Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token); - - void Validate(IServiceProvider serviceProvider); - } -} diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs index 7d19290b38f..4ba00a8f88b 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs @@ -2,6 +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 System; +using System.Threading; +using System.Threading.Tasks; + namespace Microsoft.Diagnostics.Monitoring.WebApi { internal interface IEgressOperation @@ -9,5 +13,9 @@ internal interface IEgressOperation public string EgressProviderName { get; } public EgressProcessInfo ProcessInfo { get; } + + Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token); + + void Validate(IServiceProvider serviceProvider); } } From 7219aa59f61447ea6a6f36bd27f71b231647867a Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 15:58:48 -0700 Subject: [PATCH 09/45] Support gracefully stopping a trace --- .../Controllers/DiagController.cs | 31 ++++++++++++------- .../Controllers/OperationsController.cs | 9 ++++-- .../Operation/EgressOperationStore.cs | 9 +++--- .../Operation/EgressRequest.cs | 6 ++-- .../Operation/HttpResponseEgressOperation.cs | 5 +++ .../Utilities/TraceUtilities.cs | 18 +++++++++-- .../Actions/CollectTraceAction.cs | 2 +- 7 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 463101798b4..3c81a84af67 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -633,6 +633,7 @@ private Task StartTrace( string egressProvider) { string fileName = TraceUtilities.GenerateTraceFileName(processInfo.EndpointInfo); + TaskCompletionSource requestStopCompletionSource = new(); return Result( Utilities.ArtifactType_Trace, @@ -646,7 +647,14 @@ private Task StartTrace( { operationRegistration = _operationTrackerService.Register(processInfo.EndpointInfo); } - await TraceUtilities.CaptureTraceAsync(null, processInfo.EndpointInfo, configuration, duration, outputStream, token); + await TraceUtilities.CaptureTraceAsync( + startCompletionSource: null, + processInfo.EndpointInfo, + configuration, + duration, + outputStream, + requestStopCompletionSource, + token); } finally { @@ -655,7 +663,8 @@ private Task StartTrace( }, fileName, ContentTypes.ApplicationOctetStream, - processInfo); + processInfo, + requestStopCompletionSource: requestStopCompletionSource); } private Task StartLogs( @@ -729,13 +738,13 @@ private async Task Result( string contentType, IProcessInfo processInfo, bool asAttachment = true, - TaskCompletionSource requestGracefulStopCompletionSource = null) + TaskCompletionSource requestStopCompletionSource = null) { KeyValueLogScope scope = Utilities.CreateArtifactScope(artifactType, processInfo.EndpointInfo); if (string.IsNullOrEmpty(providerName)) { - await RegisterHttpResponseAsOperation(processInfo, artifactType, requestGracefulStopCompletionSource); + await RegisterHttpResponseAsOperation(processInfo, artifactType, requestStopCompletionSource); return new OutputStreamResult( action, contentType, @@ -752,31 +761,31 @@ private async Task Result( contentType, scope), limitKey: artifactType, - requestGracefulStopCompletionSource); + requestStopCompletionSource); } } - private async Task RegisterHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, TaskCompletionSource requestGracefulStopCompletionSource = null) + private async Task RegisterHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, TaskCompletionSource requestStopCompletionSource = null) { HttpContext.Response.Headers["href"] = await RegisterOperation( new HttpResponseEgressOperation(HttpContext, processInfo), limitKey: artifactType, - requestGracefulStopCompletionSource); + requestStopCompletionSource); } - private async Task RegisterOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource) + private async Task RegisterOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestStopCompletionSource) { // Will throw TooManyRequestsException if there are too many concurrent operations. - Guid operationId = await _operationsStore.AddOperation(egressOperation, limitKey, requestGracefulStopCompletionSource); + Guid operationId = await _operationsStore.AddOperation(egressOperation, limitKey, requestStopCompletionSource); return this.Url.Action( action: nameof(OperationsController.GetOperationStatus), controller: OperationsController.ControllerName, new { operationId = operationId }, protocol: this.HttpContext.Request.Scheme, this.HttpContext.Request.Host.ToString()); } - private async Task SendToEgress(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource = null) + private async Task SendToEgress(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestStopCompletionSource = null) { - string operationUrl = await RegisterOperation(egressOperation, limitKey, requestGracefulStopCompletionSource); + string operationUrl = await RegisterOperation(egressOperation, limitKey, requestStopCompletionSource); return Accepted(operationUrl); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs index 7eb26f5680b..1f9592cac1d 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs @@ -72,13 +72,18 @@ public IActionResult GetOperationStatus(Guid operationId) [HttpDelete("{operationId}")] [ProducesWithProblemDetails(ContentTypes.ApplicationJson)] [ProducesResponseType(typeof(void), StatusCodes.Status200OK)] - public IActionResult CancelOperation(Guid operationId) + public IActionResult CancelOperation( + Guid operationId, + [FromQuery] + bool? gracefulStop = null) { return this.InvokeService(() => { + // JSFIX: Should we break out stop into its own method? + //Note that if the operation is not found, it will throw an InvalidOperationException and //return an error code. - _operationsStore.CancelOperation(operationId); + _operationsStore.CancelOperation(operationId, gracefulStop.Value); return Ok(); }, _logger); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs index c558b13c0e6..48096f1cecb 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs @@ -39,7 +39,7 @@ public EgressOperationStore( _serviceProvider = serviceProvider; } - public async Task AddOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestGracefulStopCompletionSource = null) + public async Task AddOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestStopCompletionSource = null) { egressOperation.Validate(_serviceProvider); @@ -55,7 +55,7 @@ public async Task AddOperation(IEgressOperation egressOperation, string li throw new TooManyRequestsException(); } - var request = new EgressRequest(operationId, egressOperation, limitTracker, requestGracefulStopCompletionSource); + var request = new EgressRequest(operationId, egressOperation, limitTracker, requestStopCompletionSource); lock (_requests) { //Add operation object to central table. @@ -87,9 +87,10 @@ public void CancelOperation(Guid operationId, bool gracefulStop = false) throw new InvalidOperationException(Strings.ErrorMessage_OperationNotRunning); } - if (gracefulStop && entry.EgressRequest.RequestGracefulStopCompletionSource != null) + // JSFIX: What should be the fallback behavior for a graceful stop that can't happen? + if (gracefulStop && entry.EgressRequest.RequestStopCompletionSource != null) { - entry.EgressRequest.RequestGracefulStopCompletionSource.TrySetResult(null); + entry.EgressRequest.RequestStopCompletionSource.TrySetResult(null); entry.State = Models.OperationState.Stopping; } else diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs index 0555da223dd..bae4c941c30 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs @@ -17,16 +17,16 @@ internal sealed class EgressRequest : IDisposable private bool _disposed; private IDisposable _limitTracker; - public EgressRequest(Guid operationId, IEgressOperation egressOperation, IDisposable limitTracker, TaskCompletionSource requestGracefulStopCompletionSource) + public EgressRequest(Guid operationId, IEgressOperation egressOperation, IDisposable limitTracker, TaskCompletionSource requestStopCompletionSource) { OperationId = operationId; EgressOperation = egressOperation; - RequestGracefulStopCompletionSource = requestGracefulStopCompletionSource; + RequestStopCompletionSource = requestStopCompletionSource; _limitTracker = limitTracker; } public CancellationTokenSource CancellationTokenSource { get; } = new(); - public TaskCompletionSource RequestGracefulStopCompletionSource { get; } + public TaskCompletionSource RequestStopCompletionSource { get; } public Guid OperationId { get; } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index d3b5c970a12..3e737b15a0f 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -33,7 +33,12 @@ public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo public async Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token) { + // todo: link cancellation token int statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(token); + if (token.IsCancellationRequested) + { + // Signal to the request + } return statusCode == (int)HttpStatusCode.OK ? ExecutionResult.Empty() : ExecutionResult.Failed(new Exception($"HTTP request failed with status code: ${statusCode}")); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs index 050b6856186..a82cfd6c567 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs @@ -77,7 +77,7 @@ public static MonitoringSourceConfiguration GetTraceConfiguration(Models.EventPi bufferSizeInMB: bufferSizeInMB); } - public static async Task CaptureTraceAsync(TaskCompletionSource startCompletionSource, IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan duration, Stream outputStream, CancellationToken token) + public static async Task CaptureTraceAsync(TaskCompletionSource startCompletionSource, IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan duration, Stream outputStream, TaskCompletionSource requestStopCompletionSource, CancellationToken token) { Func streamAvailable = async (Stream eventStream, CancellationToken token) => { @@ -97,7 +97,21 @@ public static async Task CaptureTraceAsync(TaskCompletionSource startCom Duration = duration, }, streamAvailable); - await pipeProcessor.RunAsync(token); + Task pipelineRunTask = pipeProcessor.RunAsync(token); + if (requestStopCompletionSource == null) + { + await pipelineRunTask; + } + else + { + await Task.WhenAny(pipelineRunTask, requestStopCompletionSource.Task).Unwrap(); + + if (requestStopCompletionSource.Task.IsCompleted) + { + await pipeProcessor.StopAsync(token); + await pipelineRunTask; + } + } } public static async Task CaptureTraceUntilEventAsync(TaskCompletionSource startCompletionSource, IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan timeout, Stream outputStream, string providerName, string eventName, IDictionary payloadFilter, ILogger logger, CancellationToken token) diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs index b3d4dc8dd2e..ed3eb491b2e 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs @@ -104,7 +104,7 @@ protected override async Task ExecuteCoreAsync( } else { - await TraceUtilities.CaptureTraceAsync(startCompletionSource, EndpointInfo, configuration, duration, outputStream, token); + await TraceUtilities.CaptureTraceAsync(startCompletionSource, EndpointInfo, configuration, duration, outputStream, requestStopCompletionSource: null, token); } }, egressProvider, From 3e7fbde2aa132edcb4af1095fa3cf111a63e381e Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 16:14:42 -0700 Subject: [PATCH 10/45] Mirror cancellation state --- .../Operation/EgressOperationService.cs | 14 +++++++++++++- .../Operation/HttpResponseEgressOperation.cs | 13 +++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs index 5c59eaaf5f3..ef645b0d448 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs @@ -52,8 +52,20 @@ private async Task ExecuteEgressOperation(EgressRequest egressRequest, Cancellat //It is possible that this operation never completes, due to infinite duration operations. _operationsStore.CompleteOperation(egressRequest.OperationId, result); } + catch (OperationCanceledException) + { + try + { + // Try to mirror the state in the operations store + _operationsStore.CancelOperation(egressRequest.OperationId); + } + catch (Exception) + { + + } + } //This is unexpected, but an unhandled exception should still fail the operation. - catch (Exception e) when (!(e is OperationCanceledException)) + catch (Exception e) { _operationsStore.CompleteOperation(egressRequest.OperationId, ExecutionResult.Failed(e)); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index 3e737b15a0f..c04d61eb5f8 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -21,7 +21,6 @@ internal class HttpResponseEgressOperation : IEgressOperation public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo) { _httpContext = context; - // JSFIX: investigate if the user closes the stream _httpContext.Response.OnCompleted((_) => { _responseFinishedCompletionSource.TrySetResult(_httpContext.Response.StatusCode); @@ -33,12 +32,14 @@ public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo public async Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token) { - // todo: link cancellation token + using IDisposable registration = _httpContext.RequestAborted.Register( + () => _responseFinishedCompletionSource.TrySetCanceled(_httpContext.RequestAborted)); + + // If the http request is aborted, it will cause an OperationCanceledException here. + // When this occurs, the operation service will mirror the cancelled state into the + // operation store. int statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(token); - if (token.IsCancellationRequested) - { - // Signal to the request - } + return statusCode == (int)HttpStatusCode.OK ? ExecutionResult.Empty() : ExecutionResult.Failed(new Exception($"HTTP request failed with status code: ${statusCode}")); From 83771f4545529b02a46ea496273229c501eddea8 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 25 Oct 2022 16:23:32 -0700 Subject: [PATCH 11/45] Handle when the operation service is starved of cpu --- .../Operation/HttpResponseEgressOperation.cs | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index c04d61eb5f8..40ff83ad7ff 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -32,13 +32,30 @@ public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo public async Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token) { - using IDisposable registration = _httpContext.RequestAborted.Register( - () => _responseFinishedCompletionSource.TrySetCanceled(_httpContext.RequestAborted)); + int statusCode; + try + { + using IDisposable registration = _httpContext.RequestAborted.Register( + () => _responseFinishedCompletionSource.TrySetCanceled(_httpContext.RequestAborted)); - // If the http request is aborted, it will cause an OperationCanceledException here. - // When this occurs, the operation service will mirror the cancelled state into the - // operation store. - int statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(token); + // If the http request is aborted, it will cause an OperationCanceledException here. + // When this occurs, the operation service will mirror the cancelled state into the + // operation store. + statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(token); + } + catch (ObjectDisposedException) + { + // If the http request is disposed by the time the operation service has a chance to get here + // then either the response must have gracefully completed or it was aborted. + if (_responseFinishedCompletionSource.Task.IsCompleted) + { + statusCode = await _responseFinishedCompletionSource.Task; + } + else + { + throw new OperationCanceledException("The HTTP request was aborted before the operation could be completed."); + } + } return statusCode == (int)HttpStatusCode.OK ? ExecutionResult.Empty() From d97b1b043145b9a3ae445902779dab51bb91cd71 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Wed, 26 Oct 2022 14:39:00 -0700 Subject: [PATCH 12/45] Simplify cancellation --- .../Operation/HttpResponseEgressOperation.cs | 27 +++---------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index 40ff83ad7ff..f94e1188338 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -27,35 +27,16 @@ public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo return Task.CompletedTask; }, null); + ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); } public async Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token) { - int statusCode; - try - { - using IDisposable registration = _httpContext.RequestAborted.Register( - () => _responseFinishedCompletionSource.TrySetCanceled(_httpContext.RequestAborted)); + using CancellationTokenSource cancellationTokenSourc = CancellationTokenSource.CreateLinkedTokenSource(token, _httpContext.RequestAborted); + using IDisposable registration = token.Register(_httpContext.Abort); - // If the http request is aborted, it will cause an OperationCanceledException here. - // When this occurs, the operation service will mirror the cancelled state into the - // operation store. - statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(token); - } - catch (ObjectDisposedException) - { - // If the http request is disposed by the time the operation service has a chance to get here - // then either the response must have gracefully completed or it was aborted. - if (_responseFinishedCompletionSource.Task.IsCompleted) - { - statusCode = await _responseFinishedCompletionSource.Task; - } - else - { - throw new OperationCanceledException("The HTTP request was aborted before the operation could be completed."); - } - } + int statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(cancellationTokenSourc.Token); return statusCode == (int)HttpStatusCode.OK ? ExecutionResult.Empty() From e39eb3e2e4767dbddbd8da4012044583f12410a5 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Wed, 26 Oct 2022 14:41:16 -0700 Subject: [PATCH 13/45] Remove extra state --- .../Operation/HttpResponseEgressOperation.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index f94e1188338..f1c49bf8e02 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -21,11 +21,11 @@ internal class HttpResponseEgressOperation : IEgressOperation public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo) { _httpContext = context; - _httpContext.Response.OnCompleted((_) => + _httpContext.Response.OnCompleted(() => { _responseFinishedCompletionSource.TrySetResult(_httpContext.Response.StatusCode); return Task.CompletedTask; - }, null); + }); ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); From 76564f9da865975841fcf286b620faa707bf15af Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Wed, 26 Oct 2022 14:54:03 -0700 Subject: [PATCH 14/45] Fix egress cancel endpoint --- .../Controllers/OperationsController.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs index 1f9592cac1d..ee0aa06bdd1 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs @@ -75,7 +75,7 @@ public IActionResult GetOperationStatus(Guid operationId) public IActionResult CancelOperation( Guid operationId, [FromQuery] - bool? gracefulStop = null) + bool gracefulStop = false) { return this.InvokeService(() => { @@ -83,7 +83,7 @@ public IActionResult CancelOperation( //Note that if the operation is not found, it will throw an InvalidOperationException and //return an error code. - _operationsStore.CancelOperation(operationId, gracefulStop.Value); + _operationsStore.CancelOperation(operationId, gracefulStop); return Ok(); }, _logger); } From 80408236728a76df48efae80baad5ca265952adb Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Wed, 26 Oct 2022 15:10:14 -0700 Subject: [PATCH 15/45] Report stoppable status --- .../Controllers/OperationsController.cs | 11 +++-- .../Models/EgressOperationStatus.cs | 3 ++ .../Operation/EgressOperationStore.cs | 44 ++++++++++++++----- .../Strings.Designer.cs | 9 ++++ .../Strings.resx | 3 ++ 5 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs index ee0aa06bdd1..c205a8a29f9 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs @@ -79,11 +79,16 @@ public IActionResult CancelOperation( { return this.InvokeService(() => { - // JSFIX: Should we break out stop into its own method? - //Note that if the operation is not found, it will throw an InvalidOperationException and //return an error code. - _operationsStore.CancelOperation(operationId, gracefulStop); + if (gracefulStop) + { + _operationsStore.StopOperation(operationId); + } + else + { + _operationsStore.CancelOperation(operationId); + } return Ok(); }, _logger); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs index c1f3d7598be..7490ae760ee 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Models/EgressOperationStatus.cs @@ -26,6 +26,9 @@ public class OperationSummary [JsonPropertyName("egressProviderName")] public string EgressProviderName { get; set; } + + [JsonPropertyName("isStoppable")] + public bool IsStoppable { get; set; } } /// diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs index 48096f1cecb..024cbfbc090 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs @@ -14,6 +14,14 @@ internal sealed class EgressOperationStore { private sealed class EgressEntry { + public bool IsStoppable + { + get + { + return State == Models.OperationState.Running && EgressRequest.RequestStopCompletionSource != null; + } + } + public ExecutionResult ExecutionResult { get; set; } public Models.OperationState State { get; set; } @@ -72,7 +80,7 @@ public async Task AddOperation(IEgressOperation egressOperation, string li return operationId; } - public void CancelOperation(Guid operationId, bool gracefulStop = false) + public void StopOperation(Guid operationId) { lock (_requests) { @@ -81,24 +89,38 @@ public void CancelOperation(Guid operationId, bool gracefulStop = false) throw new InvalidOperationException(Strings.ErrorMessage_OperationNotFound); } - if (entry.State != Models.OperationState.Running && - entry.State != Models.OperationState.Stopping) + if (entry.State != Models.OperationState.Running) { throw new InvalidOperationException(Strings.ErrorMessage_OperationNotRunning); } - // JSFIX: What should be the fallback behavior for a graceful stop that can't happen? - if (gracefulStop && entry.EgressRequest.RequestStopCompletionSource != null) + if (entry.EgressRequest.RequestStopCompletionSource == null) { - entry.EgressRequest.RequestStopCompletionSource.TrySetResult(null); - entry.State = Models.OperationState.Stopping; + throw new InvalidOperationException(Strings.ErrorMessage_OperationDoesNotSupportBeingStopped); } - else + + entry.EgressRequest.RequestStopCompletionSource.TrySetResult(null); + entry.State = Models.OperationState.Stopping; + } + } + + public void CancelOperation(Guid operationId) + { + lock (_requests) + { + if (!_requests.TryGetValue(operationId, out EgressEntry entry)) { - entry.EgressRequest.CancellationTokenSource.Cancel(); - entry.State = Models.OperationState.Cancelled; + throw new InvalidOperationException(Strings.ErrorMessage_OperationNotFound); + } + + if (entry.State != Models.OperationState.Running && + entry.State != Models.OperationState.Stopping) + { + throw new InvalidOperationException(Strings.ErrorMessage_OperationNotRunning); } + entry.State = Models.OperationState.Cancelled; + entry.EgressRequest.CancellationTokenSource.Cancel(); entry.EgressRequest.Dispose(); } } @@ -176,6 +198,7 @@ public void CompleteOperation(Guid operationId, ExecutionResult re CreatedDateTime = kvp.Value.CreatedDateTime, Status = kvp.Value.State, EgressProviderName = kvp.Value.EgressRequest.EgressOperation.EgressProviderName, + IsStoppable = kvp.Value.IsStoppable, Process = processInfo != null ? new Models.OperationProcessInfo { @@ -204,6 +227,7 @@ public Models.OperationStatus GetOperationStatus(Guid operationId) Status = entry.State, CreatedDateTime = entry.CreatedDateTime, EgressProviderName = entry.EgressRequest.EgressOperation.EgressProviderName, + IsStoppable = entry.IsStoppable, Process = processInfo != null ? new Models.OperationProcessInfo { diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs index c2344e16bea..7c39869fb80 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs @@ -141,6 +141,15 @@ internal static string ErrorMessage_NoTargetProcess { } } + /// + /// Looks up a localized string similar to Operation does not support being stopped.. + /// + internal static string ErrorMessage_OperationDoesNotSupportBeingStopped { + get { + return ResourceManager.GetString("ErrorMessage_OperationDoesNotSupportBeingStopped", resourceCulture); + } + } + /// /// Looks up a localized string similar to Operation not found.. /// diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx index dd4f32fda9a..59c0ebdae5c 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx @@ -151,6 +151,9 @@ Unable to discover a target process. Gets a string similar to "Unable to discover a target process.". + + Operation does not support being stopped. + Operation not found. From 24e29b6d77f77c2c29fa7b02317a719ddbd02b0f Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Wed, 26 Oct 2022 15:39:55 -0700 Subject: [PATCH 16/45] code cleanup --- .../Operation/HttpResponseEgressOperation.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index f1c49bf8e02..55dab5fdf61 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -27,7 +27,6 @@ public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo return Task.CompletedTask; }); - ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); } From df6ef91fc1ecbeef0acde5ea7f6dd5f16b52b367 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Thu, 27 Oct 2022 10:04:54 -0700 Subject: [PATCH 17/45] Add initial tests --- .../Controllers/OperationsController.cs | 5 ++- .../Operation/EgressOperationService.cs | 3 +- .../Operation/HttpResponseEgressOperation.cs | 4 +- .../EgressTests.cs | 36 ++++++++++++++- .../HttpApi/ApiClient.cs | 45 +++++++++++++++++++ .../HttpApi/ApiClientExtensions.cs | 6 +++ 6 files changed, 94 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs index c205a8a29f9..f27e79ec493 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs @@ -72,6 +72,7 @@ public IActionResult GetOperationStatus(Guid operationId) [HttpDelete("{operationId}")] [ProducesWithProblemDetails(ContentTypes.ApplicationJson)] [ProducesResponseType(typeof(void), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(void), StatusCodes.Status202Accepted)] public IActionResult CancelOperation( Guid operationId, [FromQuery] @@ -84,12 +85,14 @@ public IActionResult CancelOperation( if (gracefulStop) { _operationsStore.StopOperation(operationId); + // Stop operations are not instant, they are instead queued and can take an indeterminate amount of time. + return Accepted(); } else { _operationsStore.CancelOperation(operationId); + return Ok(); } - return Ok(); }, _logger); } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs index ef645b0d448..3d5e4666913 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs @@ -56,7 +56,8 @@ private async Task ExecuteEgressOperation(EgressRequest egressRequest, Cancellat { try { - // Try to mirror the state in the operations store + // Mirror the state in the operations store incase the operation was cancelled via another means besides + // the operations API. _operationsStore.CancelOperation(egressRequest.OperationId); } catch (Exception) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index 55dab5fdf61..f9a96d1a016 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -32,10 +32,10 @@ public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo public async Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token) { - using CancellationTokenSource cancellationTokenSourc = CancellationTokenSource.CreateLinkedTokenSource(token, _httpContext.RequestAborted); + using CancellationTokenSource cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(token, _httpContext.RequestAborted); using IDisposable registration = token.Register(_httpContext.Abort); - int statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(cancellationTokenSourc.Token); + int statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(cancellationTokenSource.Token); return statusCode == (int)HttpStatusCode.OK ? ExecutionResult.Empty() diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs index d065f63b68a..e1fe723a629 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs @@ -21,7 +21,6 @@ using System.Linq; using System.Net; using System.Net.Http; -using System.Runtime.InteropServices; using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; @@ -110,6 +109,41 @@ await ScenarioRunner.SingleTarget( }); } + [Fact] + public async Task EgressStopTest() + { + await ScenarioRunner.SingleTarget( + _outputHelper, + _httpClientFactory, + DiagnosticPortConnectionMode.Connect, + TestAppScenarios.AsyncWait.Name, + appValidate: async (appRunner, apiClient) => + { + int processId = await appRunner.ProcessIdTask; + + OperationResponse response = await apiClient.EgressTraceAsync(processId, durationSeconds: -1, FileProviderName); + Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); + + OperationStatusResponse operationResult = await apiClient.GetOperationStatus(response.OperationUri); + Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); + Assert.Equal(OperationState.Running, operationResult.OperationStatus.Status); + + HttpStatusCode deleteStatus = await apiClient.StopEgressOperation(response.OperationUri); + Assert.Equal(HttpStatusCode.Accepted, deleteStatus); + + operationResult = await apiClient.GetOperationStatus(response.OperationUri); + Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); + Assert.True(operationResult.OperationStatus.Status == OperationState.Stopping || + operationResult.OperationStatus.Status == OperationState.Succeeded); + + await appRunner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); + }, + configureTool: (toolRunner) => + { + toolRunner.WriteKeyPerValueConfiguration(new RootOptions().AddFileSystemEgress(FileProviderName, _tempDirectory.FullName)); + }); + } + // https://github.com/dotnet/dotnet-monitor/issues/1285 [ConditionalFact(nameof(IsNotCore31OnOSX))] public async Task EgressListTest() 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 c10e292225f..763a5ddb793 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Diagnostics.Monitoring.Options; using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Extensions.Logging; @@ -585,6 +586,28 @@ public async Task EgressTraceAsync(int processId, int duratio throw await CreateUnexpectedStatusCodeExceptionAsync(response).ConfigureAwait(false); } + public async Task CaptureTraceAsync(int processId, int durationSeconds, CancellationToken token) + { + string uri = FormattableString.Invariant($"/trace?pid={processId}&durationSeconds={durationSeconds}"); + using HttpRequestMessage request = new(HttpMethod.Get, uri); + using HttpResponseMessage response = await SendAndLogAsync(request, HttpCompletionOption.ResponseContentRead, token).ConfigureAwait(false); + + switch (response.StatusCode) + { + case HttpStatusCode.Accepted: + return new OperationResponse(response.StatusCode, response.Headers.Location); + case HttpStatusCode.BadRequest: + case HttpStatusCode.TooManyRequests: + ValidateContentType(response, ContentTypes.ApplicationProblemJson); + throw await CreateValidationProblemDetailsExceptionAsync(response).ConfigureAwait(false); + case HttpStatusCode.Unauthorized: + ThrowIfNotSuccess(response); + break; + } + + throw await CreateUnexpectedStatusCodeExceptionAsync(response).ConfigureAwait(false); + } + public async Task> GetOperations(CancellationToken token) { using HttpRequestMessage request = new(HttpMethod.Get, "/operations"); @@ -628,6 +651,28 @@ public async Task GetOperationStatus(Uri operation, Can throw await CreateUnexpectedStatusCodeExceptionAsync(response).ConfigureAwait(false); } + public async Task StopEgressOperation(Uri operation, CancellationToken token) + { + string operationUri = QueryHelpers.AddQueryString(operation.ToString(), "gracefulStop", "true"); + + using HttpRequestMessage request = new(HttpMethod.Delete, operationUri); + using HttpResponseMessage response = await SendAndLogAsync(request, HttpCompletionOption.ResponseContentRead, token).ConfigureAwait(false); + + switch (response.StatusCode) + { + case HttpStatusCode.Accepted: + return response.StatusCode; + case HttpStatusCode.BadRequest: + ValidateContentType(response, ContentTypes.ApplicationProblemJson); + throw await CreateValidationProblemDetailsExceptionAsync(response).ConfigureAwait(false); + case HttpStatusCode.Unauthorized: + ThrowIfNotSuccess(response); + break; + } + + throw await CreateUnexpectedStatusCodeExceptionAsync(response).ConfigureAwait(false); + } + public async Task CancelEgressOperation(Uri operation, CancellationToken token) { using HttpRequestMessage request = new(HttpMethod.Delete, operation.ToString()); 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 a353491ffb1..f0fc6b4c6d6 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs @@ -423,6 +423,12 @@ public static async Task> GetOperations(this ApiClient cl return await client.GetOperations(timeoutSource.Token).ConfigureAwait(false); } + public static async Task StopEgressOperation(this ApiClient client, Uri operation) + { + using CancellationTokenSource timeoutSource = new(TestTimeouts.HttpApi); + return await client.StopEgressOperation(operation, timeoutSource.Token).ConfigureAwait(false); + } + public static async Task CancelEgressOperation(this ApiClient client, Uri operation) { using CancellationTokenSource timeoutSource = new(TestTimeouts.HttpApi); From aa00608debe95f840ebe30bd24f7002752ad53c3 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Thu, 27 Oct 2022 10:19:08 -0700 Subject: [PATCH 18/45] Check new fields --- .../EgressTests.cs | 9 +++++---- .../HttpApi/ApiClientExtensions.cs | 8 ++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs index e1fe723a629..f94128abb07 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs @@ -131,10 +131,9 @@ await ScenarioRunner.SingleTarget( HttpStatusCode deleteStatus = await apiClient.StopEgressOperation(response.OperationUri); Assert.Equal(HttpStatusCode.Accepted, deleteStatus); - operationResult = await apiClient.GetOperationStatus(response.OperationUri); - Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); - Assert.True(operationResult.OperationStatus.Status == OperationState.Stopping || - operationResult.OperationStatus.Status == OperationState.Succeeded); + OperationStatusResponse pollOperationResult = await apiClient.PollOperationToCompletion(response.OperationUri); + Assert.Equal(HttpStatusCode.Created, pollOperationResult.StatusCode); + Assert.Equal(OperationState.Succeeded, pollOperationResult.OperationStatus.Status); await appRunner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }, @@ -374,6 +373,8 @@ private static void ValidateOperation(OperationStatus expected, OperationSummary Assert.Equal(expected.OperationId, summary.OperationId); Assert.Equal(expected.Status, summary.Status); Assert.Equal(expected.CreatedDateTime, summary.CreatedDateTime); + Assert.Equal(expected.EgressProviderName, summary.EgressProviderName); + Assert.Equal(expected.IsStoppable, summary.IsStoppable); } public void Dispose() 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 f0fc6b4c6d6..2b2f70422e2 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs @@ -450,10 +450,14 @@ public static async Task PollOperationToCompletion(this { OperationStatusResponse operationResult = await apiClient.GetOperationStatus(operationUrl).ConfigureAwait(false); Assert.True(operationResult.StatusCode == HttpStatusCode.OK || operationResult.StatusCode == HttpStatusCode.Created); - Assert.True(operationResult.OperationStatus.Status == OperationState.Running || operationResult.OperationStatus.Status == OperationState.Succeeded); + Assert.True( + operationResult.OperationStatus.Status == OperationState.Running || + operationResult.OperationStatus.Status == OperationState.Succeeded || + operationResult.OperationStatus.Status == OperationState.Stopping); using CancellationTokenSource cancellationTokenSource = new(timeout); - while (operationResult.OperationStatus.Status == OperationState.Running) + while (operationResult.OperationStatus.Status == OperationState.Running || + operationResult.OperationStatus.Status == OperationState.Stopping) { cancellationTokenSource.Token.ThrowIfCancellationRequested(); await Task.Delay(TimeSpan.FromSeconds(1), cancellationTokenSource.Token).ConfigureAwait(false); From 4f2ba09278cf750ff0f4883069cf86b4333f822b Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Thu, 27 Oct 2022 10:25:54 -0700 Subject: [PATCH 19/45] Expand test coverage --- .../EgressTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs index f94128abb07..bf63d2b0fdc 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs @@ -100,6 +100,7 @@ await ScenarioRunner.SingleTarget( operationResult = await apiClient.GetOperationStatus(response.OperationUri); Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); Assert.Equal(OperationState.Cancelled, operationResult.OperationStatus.Status); + Assert.False(operationResult.OperationStatus.IsStoppable); await appRunner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }, @@ -134,6 +135,7 @@ await ScenarioRunner.SingleTarget( OperationStatusResponse pollOperationResult = await apiClient.PollOperationToCompletion(response.OperationUri); Assert.Equal(HttpStatusCode.Created, pollOperationResult.StatusCode); Assert.Equal(OperationState.Succeeded, pollOperationResult.OperationStatus.Status); + Assert.False(pollOperationResult.OperationStatus.IsStoppable); await appRunner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }, @@ -166,10 +168,14 @@ await ScenarioRunner.SingleTarget( OperationStatusResponse status1 = await apiClient.GetOperationStatus(response1.OperationUri); OperationSummary summary1 = result.First(os => os.OperationId == status1.OperationStatus.OperationId); ValidateOperation(status1.OperationStatus, summary1); + Assert.True(summary1.IsStoppable); + Assert.Equal(FileProviderName, summary1.EgressProviderName); OperationStatusResponse status2 = await apiClient.GetOperationStatus(response2.OperationUri); OperationSummary summary2 = result.First(os => os.OperationId == status2.OperationStatus.OperationId); ValidateOperation(status2.OperationStatus, summary2); + Assert.False(summary2.IsStoppable); + Assert.Equal(FileProviderName, summary2.EgressProviderName); await appRunner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }, From 32c015a4721df877b664045e02bad6f8a5287c46 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 1 Nov 2022 15:31:01 -0700 Subject: [PATCH 20/45] Add test case --- .../Controllers/DiagController.cs | 2 +- .../Controllers/OperationsController.cs | 2 +- .../Operation/EgressOperationService.cs | 2 + .../CollectTraceTests.cs | 3 +- .../EgressTests.cs | 118 ++++++++++++++++++ .../HttpApi/ApiClient.cs | 39 +++++- .../HttpApi/ApiClientExtensions.cs | 6 + .../HttpApi/ResponseStreamHolder.cs | 8 +- 8 files changed, 171 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 3c81a84af67..2619ced879a 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -767,7 +767,7 @@ private async Task Result( private async Task RegisterHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, TaskCompletionSource requestStopCompletionSource = null) { - HttpContext.Response.Headers["href"] = await RegisterOperation( + HttpContext.Response.Headers["Location"] = await RegisterOperation( new HttpResponseEgressOperation(HttpContext, processInfo), limitKey: artifactType, requestStopCompletionSource); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs index f27e79ec493..7ea53039bc2 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs @@ -84,8 +84,8 @@ public IActionResult CancelOperation( //return an error code. if (gracefulStop) { - _operationsStore.StopOperation(operationId); // Stop operations are not instant, they are instead queued and can take an indeterminate amount of time. + _operationsStore.StopOperation(operationId); return Accepted(); } else diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs index 3d5e4666913..37f4e4a7591 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs @@ -64,6 +64,8 @@ private async Task ExecuteEgressOperation(EgressRequest egressRequest, Cancellat { } + + throw; } //This is unexpected, but an unhandled exception should still fail the operation. catch (Exception e) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs index 46590a293fe..2dffe7b790b 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs @@ -89,6 +89,8 @@ private static string ConstructQualifiedEventName(string eventName, TraceEventOp ? eventName : FormattableString.Invariant($"{eventName}/{opcode}"); } +#endif // NET5_0_OR_GREATER + private async Task StopOnEventTestCore(bool expectStoppingEvent, TraceEventOpcode opcode = TestAppScenarios.TraceEvents.UniqueEventOpcode, bool collectRundown = true, IDictionary payloadFilter = null, TimeSpan? duration = null) { @@ -185,6 +187,5 @@ await ScenarioRunner.SingleTarget(_outputHelper, return (didSeeStoppingEvent, didSeeRundownEvents); }); } -#endif // NET5_0_OR_GREATER } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs index bf63d2b0fdc..a4382666ade 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs @@ -13,6 +13,8 @@ using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Diagnostics.Tools.Monitor; +using Microsoft.Diagnostics.Tracing; +using Microsoft.Diagnostics.Tracing.Parsers.Clr; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using System; @@ -265,6 +267,122 @@ await ScenarioRunner.SingleTarget( }); } + [Fact] + public async Task HttpEgressCancelTest() + { + await ScenarioRunner.SingleTarget( + _outputHelper, + _httpClientFactory, + DiagnosticPortConnectionMode.Connect, + TestAppScenarios.AsyncWait.Name, + appValidate: async (appRunner, apiClient) => + { + int processId = await appRunner.ProcessIdTask; + + using ResponseStreamHolder responseBox = await apiClient.HttpEgressTraceAsync(processId, durationSeconds: -1); + + Uri operationUri = responseBox.Response.Headers.Location; + Assert.NotNull(operationUri); + + // Start consuming the stream + Task drainResponseTask = responseBox.Stream.CopyToAsync(Stream.Null); + + // Make sure the operation exists + OperationStatusResponse operationResult = await apiClient.GetOperationStatus(operationUri); + Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); + Assert.True(operationResult.OperationStatus.Status == OperationState.Running); + + // Cancel it. + + HttpStatusCode deleteStatus = await apiClient.CancelEgressOperation(operationUri); + Assert.Equal(HttpStatusCode.OK, deleteStatus); + + operationResult = await apiClient.GetOperationStatus(operationUri); + Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); + Assert.Equal(OperationState.Cancelled, operationResult.OperationStatus.Status); + + await Assert.ThrowsAsync(async () => await drainResponseTask); + + await appRunner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); + }, + configureTool: (toolRunner) => + { + toolRunner.WriteKeyPerValueConfiguration(new RootOptions().AddFileSystemEgress(FileProviderName, _tempDirectory.FullName)); + }); + } + + [Fact] + public async Task HttpEgressStopTest() + { + using TemporaryDirectory tempDir = new(_outputHelper); + + await ScenarioRunner.SingleTarget( + _outputHelper, + _httpClientFactory, + DiagnosticPortConnectionMode.Connect, + TestAppScenarios.AsyncWait.Name, + appValidate: async (appRunner, apiClient) => + { + int processId = await appRunner.ProcessIdTask; + + using ResponseStreamHolder responseBox = await apiClient.HttpEgressTraceAsync(processId, durationSeconds: -1); + + Uri operationUri = responseBox.Response.Headers.Location; + Assert.NotNull(operationUri); + + // Start saving the stream + string traceFile = Path.Combine(tempDir.FullName, "test.nettrace"); + using FileStream traceFileWriter = File.OpenWrite(traceFile); + + using MemoryStream dstStream = new(); + Task drainResponseTask = responseBox.Stream.CopyToAsync(traceFileWriter); + + // Make sure the operation exists + OperationStatusResponse operationResult = await apiClient.GetOperationStatus(operationUri); + Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); + Assert.True(operationResult.OperationStatus.Status == OperationState.Running); + + // Stop it. + HttpStatusCode deleteStatus = await apiClient.StopEgressOperation(operationUri); + Assert.Equal(HttpStatusCode.Accepted, deleteStatus); + + await drainResponseTask; + await traceFileWriter.DisposeAsync(); + + operationResult = await apiClient.GetOperationStatus(operationUri); + Assert.Equal(HttpStatusCode.Created, operationResult.StatusCode); + Assert.Equal(OperationState.Succeeded, operationResult.OperationStatus.Status); + + // Verify the resulting trace has rundown information + Assert.True(await DoesNettraceFileHaveRundown(traceFile)); + + await appRunner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); + }, + configureTool: (toolRunner) => + { + toolRunner.WriteKeyPerValueConfiguration(new RootOptions().AddFileSystemEgress(FileProviderName, _tempDirectory.FullName)); + }); + } + + private static Task DoesNettraceFileHaveRundown(string filePath) + { + return Task.Run(() => + { + using FileStream fs = File.OpenRead(filePath); + using EventPipeEventSource eventSource = new(fs); + + bool didSeeRundownEvents = false; + ClrRundownTraceEventParser rundown = new(eventSource); + rundown.RuntimeStart += (data) => + { + didSeeRundownEvents = true; + }; + + eventSource.Process(); + return didSeeRundownEvents; + }); + } + /// /// Tests that turning off HTTP egress results in an error for dumps and logs (gcdumps and traces are currently not tested) /// 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 763a5ddb793..4cab25b7ee9 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs @@ -564,6 +564,34 @@ public async Task ApiCall(string routeAndQuery, Cancellatio return response; } + public async Task HttpEgressTraceAsync(int processId, int durationSeconds, CancellationToken token) + { + string uri = FormattableString.Invariant($"/trace?pid={processId}&durationSeconds={durationSeconds}"); + using HttpRequestMessage request = new(HttpMethod.Get, uri); + + using DisposableBox responseBox = new( + await SendAndLogAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + token).ConfigureAwait(false)); + + + switch (responseBox.Value.StatusCode) + { + case HttpStatusCode.OK: + return await ResponseStreamHolder.CreateAsync(responseBox).ConfigureAwait(false); + case HttpStatusCode.BadRequest: + case HttpStatusCode.TooManyRequests: + ValidateContentType(responseBox.Value, ContentTypes.ApplicationProblemJson); + throw await CreateValidationProblemDetailsExceptionAsync(responseBox.Value).ConfigureAwait(false); + case HttpStatusCode.Unauthorized: + ThrowIfNotSuccess(responseBox.Value); + break; + } + + throw await CreateUnexpectedStatusCodeExceptionAsync(responseBox.Value).ConfigureAwait(false); + } + public async Task EgressTraceAsync(int processId, int durationSeconds, string egressProvider, CancellationToken token) { string uri = FormattableString.Invariant($"/trace?pid={processId}&egressProvider={egressProvider}&durationSeconds={durationSeconds}"); @@ -718,8 +746,15 @@ private async Task SendAndLogAsync(HttpRequestMessage reque _outputHelper.WriteLine("-> {0}", request.ToString()); } - _outputHelper.WriteLine("<- {0}", response.ToString()); - _outputHelper.WriteLine($"Request duration: {sw.ElapsedMilliseconds} ms"); + if (completionOption == HttpCompletionOption.ResponseContentRead) + { + _outputHelper.WriteLine("<- {0}", response.ToString()); + _outputHelper.WriteLine($"Request duration: {sw.ElapsedMilliseconds} ms"); + } + else + { + _outputHelper.WriteLine($"Request ACK duration: {sw.ElapsedMilliseconds} ms"); + } return response; } 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 2b2f70422e2..f3c1b200bbe 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs @@ -411,6 +411,12 @@ public static async Task EgressTraceAsync(this ApiClient clie return await client.EgressTraceAsync(processId, durationSeconds, egressProvider, timeoutSource.Token).ConfigureAwait(false); } + public static async Task HttpEgressTraceAsync(this ApiClient client, int processId, int durationSeconds) + { + using CancellationTokenSource timeoutSource = new(TestTimeouts.HttpApi); + return await client.HttpEgressTraceAsync(processId, durationSeconds, timeoutSource.Token).ConfigureAwait(false); + } + public static async Task GetOperationStatus(this ApiClient client, Uri operation) { using CancellationTokenSource timeoutSource = new(TestTimeouts.HttpApi); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ResponseStreamHolder.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ResponseStreamHolder.cs index ab6a1334947..52ba369868e 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ResponseStreamHolder.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ResponseStreamHolder.cs @@ -15,25 +15,25 @@ namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.HttpApi /// internal class ResponseStreamHolder : IDisposable { - private readonly HttpResponseMessage _response; + public HttpResponseMessage Response { get; } public Stream Stream { get; private set; } private ResponseStreamHolder(HttpResponseMessage response) { - _response = response ?? throw new ArgumentNullException(nameof(response)); + Response = response ?? throw new ArgumentNullException(nameof(response)); } public void Dispose() { // The response disposes the stream when disposed. - _response.Dispose(); + Response.Dispose(); } public static async Task CreateAsync(DisposableBox responseBox) { using DisposableBox holderBox = new(new(responseBox.Release())); - holderBox.Value.Stream = await holderBox.Value._response.Content.ReadAsStreamAsync().ConfigureAwait(false); + holderBox.Value.Stream = await holderBox.Value.Response.Content.ReadAsStreamAsync().ConfigureAwait(false); return holderBox.Release(); } } From 932af32d62a2a0b65518c8c7ead2eb35c4a36bd1 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 1 Nov 2022 15:32:58 -0700 Subject: [PATCH 21/45] Add comment --- .../Controllers/DiagController.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 2619ced879a..1afaad915f8 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -767,6 +767,8 @@ private async Task Result( private async Task RegisterHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, TaskCompletionSource requestStopCompletionSource = null) { + // While not strictly a Location redirect, use the same header as + // externally egressed operations for consistency. HttpContext.Response.Headers["Location"] = await RegisterOperation( new HttpResponseEgressOperation(HttpContext, processInfo), limitKey: artifactType, From 5cb63457361809979290e54fee538d641582e43d Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 1 Nov 2022 16:10:55 -0700 Subject: [PATCH 22/45] Update comments --- .../EgressTests.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs index a4382666ade..98484615ec2 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs @@ -292,8 +292,7 @@ await ScenarioRunner.SingleTarget( Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); Assert.True(operationResult.OperationStatus.Status == OperationState.Running); - // Cancel it. - + // Cancel the trace operation HttpStatusCode deleteStatus = await apiClient.CancelEgressOperation(operationUri); Assert.Equal(HttpStatusCode.OK, deleteStatus); @@ -342,7 +341,7 @@ await ScenarioRunner.SingleTarget( Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); Assert.True(operationResult.OperationStatus.Status == OperationState.Running); - // Stop it. + // Stop the trace operation HttpStatusCode deleteStatus = await apiClient.StopEgressOperation(operationUri); Assert.Equal(HttpStatusCode.Accepted, deleteStatus); From 720059b11d0085a6fcbc4c65d5c6964434c53e6b Mon Sep 17 00:00:00 2001 From: Joe Schmitt Date: Wed, 2 Nov 2022 16:11:38 +0000 Subject: [PATCH 23/45] Update openapi.json --- documentation/openapi.json | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/documentation/openapi.json b/documentation/openapi.json index fa32bedb132..6d2929ff808 100644 --- a/documentation/openapi.json +++ b/documentation/openapi.json @@ -1207,6 +1207,14 @@ "type": "string", "format": "uuid" } + }, + { + "name": "gracefulStop", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } } ], "responses": { @@ -1215,6 +1223,9 @@ }, "200": { "description": "Success" + }, + "202": { + "description": "Accepted" } } } @@ -1496,7 +1507,8 @@ "Running", "Succeeded", "Failed", - "Cancelled" + "Cancelled", + "Stopping" ], "type": "string" }, @@ -1517,6 +1529,13 @@ "process": { "$ref": "#/components/schemas/OperationProcessInfo" }, + "egressProviderName": { + "type": "string", + "nullable": true + }, + "isStoppable": { + "type": "boolean" + }, "resourceLocation": { "type": "string", "nullable": true @@ -1544,6 +1563,13 @@ }, "process": { "$ref": "#/components/schemas/OperationProcessInfo" + }, + "egressProviderName": { + "type": "string", + "nullable": true + }, + "isStoppable": { + "type": "boolean" } }, "additionalProperties": false, From 8b71b7db7542180593b0830cdab0daeb02c092d1 Mon Sep 17 00:00:00 2001 From: Joe Schmitt Date: Wed, 2 Nov 2022 16:27:56 +0000 Subject: [PATCH 24/45] Initial doc changes --- documentation/api/operations-delete.md | 4 +- documentation/api/operations-stop.md | 58 ++++++++++++++++++++++++++ documentation/api/operations.md | 1 + documentation/api/trace-get.md | 7 +++- 4 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 documentation/api/operations-stop.md diff --git a/documentation/api/operations-delete.md b/documentation/api/operations-delete.md index 50cddb21105..525f7962a0c 100644 --- a/documentation/api/operations-delete.md +++ b/documentation/api/operations-delete.md @@ -3,7 +3,8 @@ # Operations - Delete -Cancel a running operation. Only valid against operations in the `Running` state. Transitions the operation to `Cancelled` state. +Cancel a running operation. Only valid against operations in the `Running` or `Stopping` state. Transitions the operation to `Cancelled` state. Cancelling an operation may result in an incomplete or unreadable artifact. To stop an operation early while still producing a valid artifact using [Stop Operation](operations-stop.md). + ## HTTP Route @@ -45,6 +46,7 @@ Authorization: Bearer fffffffffffffffffffffffffffffffffffffffffff= ```http HTTP/1.1 200 OK +``` ## Supported Runtimes diff --git a/documentation/api/operations-stop.md b/documentation/api/operations-stop.md new file mode 100644 index 00000000000..8d68a8668b9 --- /dev/null +++ b/documentation/api/operations-stop.md @@ -0,0 +1,58 @@ + +### Was this documentation helpful? [Share feedback](https://www.research.net/r/DGDQWXH?src=documentation%2Fapi%2Foperations-stop) + +# Operations - Stop + +Gracefully stops a running operation. Only valid against operations with the `isStoppable` property set to `true`, not all operations support being gracefully stopped. Transitions the operation to `Succeeded` or `Failed` state depending on if the operation was successful. + +Stopping an operation may not happen immediately such as in the case of traces where stopping may collect rundown information. An operation in the `Stopping` state can still be cancelled using [Delete Operation](operations-delete.md). + +## HTTP Route + +```http +DELETE /operations/{operationId}?gracefulStop=true HTTP/1.1 +``` + +## Host Address + +The default host address for these routes is `https://localhost:52323`. This route is only available on the addresses configured via the `--urls` command line parameter and the `DOTNETMONITOR_URLS` environment variable. + +## Authentication + +Authentication is enforced for this route. See [Authentication](./../authentication.md) for further information. + +Allowed schemes: +- `Bearer` +- `Negotiate` (Windows only, running as unelevated) + +## Responses + +| Name | Type | Description | Content Type | +|---|---|---|---| +| 202 Accepted | | The operation was successfully queued to stop. | `application/json` | +| 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | +| 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | + +## Examples + +### Sample Request + +```http +DELETE /operations/67f07e40-5cca-4709-9062-26302c484f18?gracefulStop=true HTTP/1.1 +Host: localhost:52323 +Authorization: Bearer fffffffffffffffffffffffffffffffffffffffffff= +``` + +### Sample Response + +```http +HTTP/1.1 202 OK +``` + +## Supported Runtimes + +| Operating System | Runtime Version | +|---|---| +| Windows | .NET Core 3.1, .NET 5+ | +| Linux | .NET Core 3.1, .NET 5+ | +| MacOS | .NET Core 3.1, .NET 5+ | diff --git a/documentation/api/operations.md b/documentation/api/operations.md index 1241fb49662..834f234b370 100644 --- a/documentation/api/operations.md +++ b/documentation/api/operations.md @@ -10,3 +10,4 @@ Operations are used to track long running operations in dotnet-monitor, specific | [List Operations](operations-list.md) | Lists all the operations and their current state. | | [Get Operation](operations-get.md) | Get detailed information about an operation. | | [Delete Operation](operations-delete.md) | Cancels a running operation. | +| [Stop Operation](operations-stop.md) | Gracefully stops a running operation. | diff --git a/documentation/api/trace-get.md b/documentation/api/trace-get.md index 40c68261e9c..7a94db9667e 100644 --- a/documentation/api/trace-get.md +++ b/documentation/api/trace-get.md @@ -44,12 +44,14 @@ Allowed schemes: | Name | Type | Description | Content Type | |---|---|---|---| -| 200 OK | stream | A trace of the process. | `application/octet-stream` | -| 202 Accepted | | When an egress provider is specified, the Location header containers the URI of the operation for querying the egress status. | | +| 200 OK | stream | A trace of the process when no egress provider is specified. | `application/octet-stream` | +| 202 Accepted | | When an egress provider is specified. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many trace requests at this time. Try to request a trace at a later time. | `application/problem+json` | +Regardless if an egress provider is specified (response codes 200 or 202), if a trace is successfully started, the Location header containers the URI of the operation. This can be used to query the status of the operation, immediately cancel it, or request a graceful stop to the trace which will include rundown information. + > **NOTE:** After the expiration of the trace duration, completing the request may take a long time (up to several minutes) for large applications. The runtime needs to send over the type cache for all managed code that was captured in the trace, known as rundown events. Thus, the length of time of the request may take significantly longer than the requested duration. ## Examples @@ -78,6 +80,7 @@ The 1 minute trace with http request handling and metric information, chunk enco HTTP/1.1 200 OK Content-Type: application/octet-stream Transfer-Encoding: chunked +Location: localhost:52323/operation/627efe4f-602e-451b-a5a4-27eefaba2464 ``` ## Supported Runtimes From 2aa86d315e08653fc52732fc3431928573e94c08 Mon Sep 17 00:00:00 2001 From: Joe Schmitt Date: Wed, 2 Nov 2022 16:56:55 +0000 Subject: [PATCH 25/45] Update docs --- documentation/api/definitions.md | 11 ++++++++++- documentation/api/dump.md | 7 +++++-- documentation/api/gcdump.md | 7 +++++-- documentation/api/livemetrics-custom.md | 5 ++++- documentation/api/livemetrics-get.md | 5 ++++- documentation/api/logs-custom.md | 5 ++++- documentation/api/logs-get.md | 5 ++++- documentation/api/operations-get.md | 2 ++ documentation/api/operations-list.md | 25 ++++++++++++++++++++++--- documentation/api/operations-stop.md | 5 +++++ documentation/api/stacks.md | 6 +++++- documentation/api/trace-custom.md | 7 +++++-- documentation/api/trace-get.md | 6 +++--- 13 files changed, 78 insertions(+), 18 deletions(-) diff --git a/documentation/api/definitions.md b/documentation/api/definitions.md index 913a4072660..62b4e818d0a 100644 --- a/documentation/api/definitions.md +++ b/documentation/api/definitions.md @@ -267,6 +267,7 @@ Status of the egress operation. |---|---| | `Running` | Operation has been started. This is the initial state. | | `Cancelled` | The operation was cancelled by the user. | +| `Stopping` | The operation is in the process of stopping at the request of the user. | | `Succeeded` | Egress operation has been successful. Querying the operation will return the location of the egressed artifact. | | `Failed` | Egress operation failed. Querying the operation will return detailed error information. | @@ -281,6 +282,8 @@ Detailed information about an operation. | `operationId` | guid | Unique identifier for the operation. | | `createdDateTime` | datetime string | UTC DateTime string of when the operation was created. | | `status` | [OperationState](#operationstate) | The current status of operation. | +| `egressProviderName` | string | The name of the egress provider that the artifact is being sent to. This will be null if the artifact is being sent directly back to the user from an HTTP request. | +| `isStoppable` | bool | Whether this operation can be gracefully stopped using [Stop Operation](operations-stop.md). Not all operations support being stopped. | ### Example @@ -290,7 +293,9 @@ Detailed information about an operation. "error": null, "operationId": "67f07e40-5cca-4709-9062-26302c484f18", "createdDateTime": "2021-07-21T06:21:15.315861Z", - "status": "Succeeded" + "status": "Succeeded", + "egressProviderName": "monitorBlob", + "isStoppable": false, } ``` @@ -303,6 +308,8 @@ Summary state of an operation. | `operationId` | guid | Unique identifier for the operation. | | `createdDateTime` | datetime string | UTC DateTime string of when the operation was created. | | `status` | [OperationState](#operationstate) | The current status of operation. | +| `egressProviderName` | string | The name of the egress provider that the artifact is being sent to. This will be null if the artifact is being sent directly back to the user from an HTTP request. | +| `isStoppable` | bool | Whether this operation can be gracefully stopped using [Stop Operation](operations-stop.md). Not all operations support being stopped. | | `process` | [OperationProcessInfo](#operationprocessinfo) | (6.3+) The process on which the operation is performed. | ### Example @@ -312,6 +319,8 @@ Summary state of an operation. "operationId": "67f07e40-5cca-4709-9062-26302c484f18", "createdDateTime": "2021-07-21T06:21:15.315861Z", "status": "Succeeded", + "egressProviderName": null, + "isStoppable": false, "process": { "pid": 21632, "uid": "cd4da319-fa9e-4987-ac4e-e57b2aac248b", diff --git a/documentation/api/dump.md b/documentation/api/dump.md index ee6724c1292..001bb407824 100644 --- a/documentation/api/dump.md +++ b/documentation/api/dump.md @@ -45,12 +45,14 @@ Allowed schemes: | Name | Type | Description | Content Type | |---|---|---|---| -| 200 OK | stream | A managed dump of the process. | `application/octet-stream` | -| 202 Accepted | | When an egress provider is specified, the Location header containers the URI of the operation for querying the egress status. | | +| 200 OK | stream | A managed dump of the process when no egress provider is specified. | `application/octet-stream` | +| 202 Accepted | | When an egress provider is specified, the artifact has begun being collected. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many dump requests at this time. Try to request a dump at a later time. | | +> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. + ## Examples ### Sample Request @@ -77,6 +79,7 @@ The managed dump containing all memory of the process, chunk encoded, is returne HTTP/1.1 200 OK Content-Type: application/octet-stream Transfer-Encoding: chunked +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 ``` ## Supported Runtimes diff --git a/documentation/api/gcdump.md b/documentation/api/gcdump.md index 963ca7a2b1f..371657b97c6 100644 --- a/documentation/api/gcdump.md +++ b/documentation/api/gcdump.md @@ -48,12 +48,14 @@ Allowed schemes: | Name | Type | Description | Content Type | |---|---|---|---| -| 200 OK | stream | A GC dump of the process. | `application/octet-stream` | -| 202 Accepted | | When an egress provider is specified, the Location header containers the URI of the operation for querying the egress status. | | +| 200 OK | stream | A GC dump of the process when no egress provider is specified. | `application/octet-stream` | +| 202 Accepted | | When an egress provider is specified, the artifact has begun being collected. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many GC dump requests at this time. Try to request a GC dump at a later time. | `application/problem+json` | +> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. + ## Examples ### Sample Request @@ -80,6 +82,7 @@ The GC dump, chunk encoded, is returned as the response body. HTTP/1.1 200 OK Content-Type: application/octet-stream Transfer-Encoding: chunked +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 ``` ## Supported Runtimes diff --git a/documentation/api/livemetrics-custom.md b/documentation/api/livemetrics-custom.md index ad404d9b48c..fca0590c887 100644 --- a/documentation/api/livemetrics-custom.md +++ b/documentation/api/livemetrics-custom.md @@ -50,11 +50,13 @@ The expected content type is `application/json`. | Name | Type | Description | Content Type | |---|---|---|---| | 200 OK | [Metric](./definitions.md#metric) | The metrics from the process formatted as json sequence. Each JSON object is a [metrics object](./definitions.md#metric)| `application/json-seq` | -| 202 Accepted | | When an egress provider is specified, the Location header containers the URI of the operation for querying the egress status. | | +| 202 Accepted | | When an egress provider is specified, the artifact has begun being collected. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many requests at this time. Try to request metrics at a later time. | `application/problem+json` | +> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. + ## Examples ### Sample Request @@ -83,6 +85,7 @@ Authorization: Bearer fffffffffffffffffffffffffffffffffffffffffff= ```http HTTP/1.1 200 OK Content-Type: application/json-seq +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 { "timestamp": "2021-08-31T16:58:39.7514031+00:00", diff --git a/documentation/api/livemetrics-get.md b/documentation/api/livemetrics-get.md index b7712403a9c..79380456878 100644 --- a/documentation/api/livemetrics-get.md +++ b/documentation/api/livemetrics-get.md @@ -46,11 +46,13 @@ Allowed schemes: | Name | Type | Description | Content Type | |---|---|---|---| | 200 OK | [Metric](./definitions.md#metric) | The metrics from the process formatted as json sequence. | `application/json-seq` | -| 202 Accepted | | When an egress provider is specified, the Location header containers the URI of the operation for querying the egress status. | | +| 202 Accepted | | When an egress provider is specified, the artifact has begun being collected. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many requests at this time. Try to request metrics at a later time. | `application/problem+json` | +> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. + ## Examples ### Sample Request @@ -66,6 +68,7 @@ Authorization: Bearer fffffffffffffffffffffffffffffffffffffffffff= ```http HTTP/1.1 200 OK Content-Type: application/json-seq +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 { "timestamp": "2021-08-31T16:58:39.7514031+00:00", diff --git a/documentation/api/logs-custom.md b/documentation/api/logs-custom.md index 654b400c2de..1c57dcbb05d 100644 --- a/documentation/api/logs-custom.md +++ b/documentation/api/logs-custom.md @@ -53,11 +53,13 @@ The expected content type is `application/json`. |---|---|---|---| | 200 OK | | The logs from the process formatted as [newline delimited JSON](https://github.com/ndjson/ndjson-spec). Each JSON object is a [LogEntry](definitions.md#logentry) | `application/x-ndjson` | | 200 OK | | The logs from the process formatted as plain text, similar to the output of the JSON console formatter. | `text/plain` | -| 202 Accepted | | When an egress provider is specified, the Location header containers the URI of the operation for querying the egress status. | | +| 202 Accepted | | When an egress provider is specified, the artifact has begun being collected. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many logs requests at this time. Try to request logs at a later time. | `application/problem+json` | +> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. + ## Examples ### Sample Request @@ -98,6 +100,7 @@ The log statements logged at the Information level or higher for 1 minute is ret ```http HTTP/1.1 200 OK Content-Type: text/plain +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 2021-05-13 18:06:41Z info: Microsoft.AspNetCore.Hosting.Diagnostics[1] => RequestId:0HM8M726ENU3K:0000002B, RequestPath:/, SpanId:|4791a4a7-433aa59a9e362743., TraceId:4791a4a7-433aa59a9e362743, ParentId: diff --git a/documentation/api/logs-get.md b/documentation/api/logs-get.md index ed525f85875..5661eb6fb0b 100644 --- a/documentation/api/logs-get.md +++ b/documentation/api/logs-get.md @@ -48,11 +48,13 @@ Allowed schemes: |---|---|---|---| | 200 OK | | The logs from the process formatted as [newline delimited JSON](https://github.com/ndjson/ndjson-spec). Each JSON object is a [LogEntry](definitions.md#logentry) | `application/x-ndjson` | | 200 OK | | The logs from the process formatted as plain text, similar to the output of the JSON console formatter. | `text/plain` | -| 202 Accepted | | When an egress provider is specified, the Location header containers the URI of the operation for querying the egress status. | | +| 202 Accepted | | When an egress provider is specified,. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many logs requests at this time. Try to request logs at a later time. | `application/problem+json` | +> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. + ## Examples ### Sample Request @@ -78,6 +80,7 @@ The log statements logged at the Information level or higher for 1 minute is ret ```http HTTP/1.1 200 OK Content-Type: text/plain +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 info: Agent.RequestProcessor[3][ProcessRequest] Processing request 353f398a-dc74-4adc-b107-ec35edd09968. diff --git a/documentation/api/operations-get.md b/documentation/api/operations-get.md index 6f29c433829..203490accee 100644 --- a/documentation/api/operations-get.md +++ b/documentation/api/operations-get.md @@ -53,6 +53,8 @@ Content-Type: application/json "operationId": "67f07e40-5cca-4709-9062-26302c484f18", "createdDateTime": "2021-07-21T06:21:15.315861Z", "status": "Succeeded", + "egressProviderName": "monitorBlob", + "isStoppable": true, "process": { "pid":1, diff --git a/documentation/api/operations-list.md b/documentation/api/operations-list.md index aa5bc67cdcc..2dbde05537f 100644 --- a/documentation/api/operations-list.md +++ b/documentation/api/operations-list.md @@ -65,7 +65,22 @@ Content-Type: application/json { "operationId": "67f07e40-5cca-4709-9062-26302c484f18", "createdDateTime": "2021-07-21T06:21:15.315861Z", - "status": "Succeeded", + "status": "Succeeded", + "egressProviderName": "monitorBlob", + "isStoppable": false, + "process": + { + "pid":1, + "uid":"95b0202a-4ed3-44a6-98f1-767d270ec783", + "name":"dotnet-monitor-demo" + } + }, + { + "operationId": "06ac07e2-f7cd-45ad-80c6-e38160bc5881", + "createdDateTime": "2021-07-21T20:22:15.315861Z", + "status": "Stopping", + "egressProviderName": null, + "isStoppable": false, "process": { "pid":1, @@ -76,7 +91,9 @@ Content-Type: application/json { "operationId": "26e74e52-0a16-4e84-84bb-27f904bfaf85", "createdDateTime": "2021-07-21T23:30:22.3058272Z", - "status": "Failed", + "status": "Failed", + "egressProviderName": "monitorBlob", + "isStoppable": false, "process": { "pid":11782, @@ -105,7 +122,9 @@ Content-Type: application/json { "operationId": "67f07e40-5cca-4709-9062-26302c484f18", "createdDateTime": "2021-07-21T06:21:15.315861Z", - "status": "Succeeded", + "status": "Succeeded", + "egressProviderName": "monitorBlob", + "isStoppable": false, "process": { "pid":1, diff --git a/documentation/api/operations-stop.md b/documentation/api/operations-stop.md index 8d68a8668b9..1db07df310e 100644 --- a/documentation/api/operations-stop.md +++ b/documentation/api/operations-stop.md @@ -7,6 +7,11 @@ Gracefully stops a running operation. Only valid against operations with the `is Stopping an operation may not happen immediately such as in the case of traces where stopping may collect rundown information. An operation in the `Stopping` state can still be cancelled using [Delete Operation](operations-delete.md). +## Supported Artifacts + +The following API endpoints have support for their operations being gracefully stopped when they are in the `Running` state. +- [`/trace`](./api/trace.md) + ## HTTP Route ```http diff --git a/documentation/api/stacks.md b/documentation/api/stacks.md index 862f7056588..5fcd13e1737 100644 --- a/documentation/api/stacks.md +++ b/documentation/api/stacks.md @@ -46,11 +46,13 @@ Allowed schemes: |---|---|---|---| | 200 OK | [CallStackResult](definitions.md#experimental-callstackresult-70) | Callstacks for all managed threads in the process. | `application/json` | | 200 OK | text | Text representation of callstacks in the process. | `text/plain` | -| 202 Accepted | | When an egress provider is specified, the Location header containers the URI of the operation for querying the egress status. | | +| 202 Accepted | | When an egress provider is specified, the artifact has begun being collected. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many stack requests at this time. Try to request a stack at a later time. | `application/problem+json` | +> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. + ## Examples ### Sample Request @@ -67,6 +69,7 @@ Accept: application/json ```http HTTP/1.1 200 OK Content-Type: application/json +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 { "threadId": 30860, @@ -103,6 +106,7 @@ Accept: text/plain ```http HTTP/1.1 200 OK Content-Type: text/plain +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 Thread: (0x68C0) System.Private.CoreLib.dll!System.Threading.Monitor.Wait diff --git a/documentation/api/trace-custom.md b/documentation/api/trace-custom.md index 302215323e4..d1e915044c5 100644 --- a/documentation/api/trace-custom.md +++ b/documentation/api/trace-custom.md @@ -49,12 +49,14 @@ The expected content type is `application/json`. | Name | Type | Description | Content Type | |---|---|---|---| -| 200 OK | stream | A trace of the process. | `application/octet-stream` | -| 202 Accepted | | When an egress provider is specified, the Location header containers the URI of the operation for querying the egress status. | | +| 200 OK | stream | A trace of the process when no egress provider is specified. | `application/octet-stream` | +| 202 Accepted | | When an egress provider is specified, the artifact has begun being collected. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many trace requests at this time. Try to request a trace at a later time. | `application/problem+json` | +> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. + > **NOTE:** After the expiration of the trace duration, completing the request may take a long time (up to several minutes) for large applications if `EventProvidersConfiguration.RequestRundown` is set to `true`. The runtime needs to send over the type cache for all managed code that was captured in the trace, known as rundown events. Thus, the length of time of the request may take significantly longer than the requested duration. ## Examples @@ -109,6 +111,7 @@ The 1 minute trace with CPU information, chunk encoded, is returned as the respo HTTP/1.1 200 OK Content-Type: application/octet-stream Transfer-Encoding: chunked +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 ``` ## Supported Runtimes diff --git a/documentation/api/trace-get.md b/documentation/api/trace-get.md index 7a94db9667e..50ae61d5ddd 100644 --- a/documentation/api/trace-get.md +++ b/documentation/api/trace-get.md @@ -45,12 +45,12 @@ Allowed schemes: | Name | Type | Description | Content Type | |---|---|---|---| | 200 OK | stream | A trace of the process when no egress provider is specified. | `application/octet-stream` | -| 202 Accepted | | When an egress provider is specified. | | +| 202 Accepted | | When an egress provider is specified, the artifact has begun being collected. | | | 400 Bad Request | [ValidationProblemDetails](definitions.md#validationproblemdetails) | An error occurred due to invalid input. The response body describes the specific problem(s). | `application/problem+json` | | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many trace requests at this time. Try to request a trace at a later time. | `application/problem+json` | -Regardless if an egress provider is specified (response codes 200 or 202), if a trace is successfully started, the Location header containers the URI of the operation. This can be used to query the status of the operation, immediately cancel it, or request a graceful stop to the trace which will include rundown information. +> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. > **NOTE:** After the expiration of the trace duration, completing the request may take a long time (up to several minutes) for large applications. The runtime needs to send over the type cache for all managed code that was captured in the trace, known as rundown events. Thus, the length of time of the request may take significantly longer than the requested duration. @@ -80,7 +80,7 @@ The 1 minute trace with http request handling and metric information, chunk enco HTTP/1.1 200 OK Content-Type: application/octet-stream Transfer-Encoding: chunked -Location: localhost:52323/operation/627efe4f-602e-451b-a5a4-27eefaba2464 +Location: localhost:52323/operations/67f07e40-5cca-4709-9062-26302c484f18 ``` ## Supported Runtimes From af340ef8042792129f07aaf28d98fa1cdc69bf9f Mon Sep 17 00:00:00 2001 From: Joe Schmitt Date: Wed, 2 Nov 2022 16:58:24 +0000 Subject: [PATCH 26/45] Update docs --- documentation/api/dump.md | 2 +- documentation/api/gcdump.md | 2 +- documentation/api/livemetrics-custom.md | 2 +- documentation/api/livemetrics-get.md | 2 +- documentation/api/logs-custom.md | 2 +- documentation/api/logs-get.md | 2 +- documentation/api/stacks.md | 2 +- documentation/api/trace-custom.md | 2 +- documentation/api/trace-get.md | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/documentation/api/dump.md b/documentation/api/dump.md index 001bb407824..ec2271bff85 100644 --- a/documentation/api/dump.md +++ b/documentation/api/dump.md @@ -51,7 +51,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many dump requests at this time. Try to request a dump at a later time. | | -> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/gcdump.md b/documentation/api/gcdump.md index 371657b97c6..6c3f0456a9f 100644 --- a/documentation/api/gcdump.md +++ b/documentation/api/gcdump.md @@ -54,7 +54,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many GC dump requests at this time. Try to request a GC dump at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/livemetrics-custom.md b/documentation/api/livemetrics-custom.md index fca0590c887..cd819d82f70 100644 --- a/documentation/api/livemetrics-custom.md +++ b/documentation/api/livemetrics-custom.md @@ -55,7 +55,7 @@ The expected content type is `application/json`. | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many requests at this time. Try to request metrics at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/livemetrics-get.md b/documentation/api/livemetrics-get.md index 79380456878..c510553f0c0 100644 --- a/documentation/api/livemetrics-get.md +++ b/documentation/api/livemetrics-get.md @@ -51,7 +51,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many requests at this time. Try to request metrics at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/logs-custom.md b/documentation/api/logs-custom.md index 1c57dcbb05d..77e8616b783 100644 --- a/documentation/api/logs-custom.md +++ b/documentation/api/logs-custom.md @@ -58,7 +58,7 @@ The expected content type is `application/json`. | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many logs requests at this time. Try to request logs at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/logs-get.md b/documentation/api/logs-get.md index 5661eb6fb0b..cf12ffd8448 100644 --- a/documentation/api/logs-get.md +++ b/documentation/api/logs-get.md @@ -53,7 +53,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many logs requests at this time. Try to request logs at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/stacks.md b/documentation/api/stacks.md index 5fcd13e1737..363f4fc4083 100644 --- a/documentation/api/stacks.md +++ b/documentation/api/stacks.md @@ -51,7 +51,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many stack requests at this time. Try to request a stack at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/trace-custom.md b/documentation/api/trace-custom.md index d1e915044c5..f329767dcf4 100644 --- a/documentation/api/trace-custom.md +++ b/documentation/api/trace-custom.md @@ -55,7 +55,7 @@ The expected content type is `application/json`. | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many trace requests at this time. Try to request a trace at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. > **NOTE:** After the expiration of the trace duration, completing the request may take a long time (up to several minutes) for large applications if `EventProvidersConfiguration.RequestRundown` is set to `true`. The runtime needs to send over the type cache for all managed code that was captured in the trace, known as rundown events. Thus, the length of time of the request may take significantly longer than the requested duration. diff --git a/documentation/api/trace-get.md b/documentation/api/trace-get.md index 50ae61d5ddd..d9682d1dc60 100644 --- a/documentation/api/trace-get.md +++ b/documentation/api/trace-get.md @@ -50,7 +50,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many trace requests at this time. Try to request a trace at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified, if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. > **NOTE:** After the expiration of the trace duration, completing the request may take a long time (up to several minutes) for large applications. The runtime needs to send over the type cache for all managed code that was captured in the trace, known as rundown events. Thus, the length of time of the request may take significantly longer than the requested duration. From c06f5f9a4471f32dd77cdf0facdeff0c7f256f72 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Wed, 2 Nov 2022 10:05:58 -0700 Subject: [PATCH 27/45] Code cleanup --- .../Controllers/DiagController.cs | 9 ++++----- .../Operation/EgressOperationService.cs | 7 ++++--- .../HttpApi/ApiClient.cs | 1 - 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 1afaad915f8..8d163df4728 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -230,7 +230,7 @@ public Task CaptureDump( if (string.IsNullOrEmpty(egressProvider)) { - await RegisterHttpResponseAsOperation(processInfo, artifactType); + await RegisterCurrentHttpResponseAsOperation(processInfo, artifactType); Stream dumpStream = await _dumpService.DumpAsync(processInfo.EndpointInfo, type, HttpContext.RequestAborted); _logger.WrittenToHttpStream(); @@ -744,7 +744,7 @@ private async Task Result( if (string.IsNullOrEmpty(providerName)) { - await RegisterHttpResponseAsOperation(processInfo, artifactType, requestStopCompletionSource); + await RegisterCurrentHttpResponseAsOperation(processInfo, artifactType, requestStopCompletionSource); return new OutputStreamResult( action, contentType, @@ -765,10 +765,9 @@ private async Task Result( } } - private async Task RegisterHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, TaskCompletionSource requestStopCompletionSource = null) + private async Task RegisterCurrentHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, TaskCompletionSource requestStopCompletionSource = null) { - // While not strictly a Location redirect, use the same header as - // externally egressed operations for consistency. + // While not strictly a Location redirect, use the same header as externally egressed operations for consistency. HttpContext.Response.Headers["Location"] = await RegisterOperation( new HttpResponseEgressOperation(HttpContext, processInfo), limitKey: artifactType, diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs index 37f4e4a7591..73cd48d5609 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationService.cs @@ -47,7 +47,7 @@ private async Task ExecuteEgressOperation(EgressRequest egressRequest, Cancellat try { - var result = await egressRequest.EgressOperation.ExecuteAsync(_serviceProvider, token); + ExecutionResult result = await egressRequest.EgressOperation.ExecuteAsync(_serviceProvider, token); //It is possible that this operation never completes, due to infinite duration operations. _operationsStore.CompleteOperation(egressRequest.OperationId, result); @@ -60,14 +60,15 @@ private async Task ExecuteEgressOperation(EgressRequest egressRequest, Cancellat // the operations API. _operationsStore.CancelOperation(egressRequest.OperationId); } - catch (Exception) + // Expected if the state already reflects the cancellation. + catch (InvalidOperationException) { } throw; } - //This is unexpected, but an unhandled exception should still fail the operation. + // This is unexpected, but an unhandled exception should still fail the operation. catch (Exception e) { _operationsStore.CompleteOperation(egressRequest.OperationId, ExecutionResult.Failed(e)); 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 4cab25b7ee9..39aca33b5a6 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs @@ -575,7 +575,6 @@ await SendAndLogAsync( HttpCompletionOption.ResponseHeadersRead, token).ConfigureAwait(false)); - switch (responseBox.Value.StatusCode) { case HttpStatusCode.OK: From db53f7ed7ea6784d01b5e85492ce89f4198f588a Mon Sep 17 00:00:00 2001 From: Joe Schmitt Date: Wed, 2 Nov 2022 10:46:11 -0700 Subject: [PATCH 28/45] Update documentation/api/operations-stop.md --- documentation/api/operations-stop.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/api/operations-stop.md b/documentation/api/operations-stop.md index 1db07df310e..549b3fb6ecf 100644 --- a/documentation/api/operations-stop.md +++ b/documentation/api/operations-stop.md @@ -10,7 +10,7 @@ Stopping an operation may not happen immediately such as in the case of traces w ## Supported Artifacts The following API endpoints have support for their operations being gracefully stopped when they are in the `Running` state. -- [`/trace`](./api/trace.md) +- [`/trace`](./trace.md) ## HTTP Route From 13908509585b468a90ea628dbec2d3c0b002132b Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Wed, 2 Nov 2022 10:47:26 -0700 Subject: [PATCH 29/45] Address PR feedback --- .../Operation/HttpResponseEgressOperation.cs | 3 ++- .../CollectTraceTests.cs | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index f9a96d1a016..fbc2f8a7d89 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -37,7 +37,8 @@ public async Task> ExecuteAsync(IServiceProvider s int statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(cancellationTokenSource.Token); - return statusCode == (int)HttpStatusCode.OK + + return statusCode >= (int)HttpStatusCode.OK && statusCode < (int)HttpStatusCode.Ambiguous ? ExecutionResult.Empty() : ExecutionResult.Failed(new Exception($"HTTP request failed with status code: ${statusCode}")); } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs index 2dffe7b790b..46590a293fe 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs @@ -89,8 +89,6 @@ private static string ConstructQualifiedEventName(string eventName, TraceEventOp ? eventName : FormattableString.Invariant($"{eventName}/{opcode}"); } -#endif // NET5_0_OR_GREATER - private async Task StopOnEventTestCore(bool expectStoppingEvent, TraceEventOpcode opcode = TestAppScenarios.TraceEvents.UniqueEventOpcode, bool collectRundown = true, IDictionary payloadFilter = null, TimeSpan? duration = null) { @@ -187,5 +185,6 @@ await ScenarioRunner.SingleTarget(_outputHelper, return (didSeeStoppingEvent, didSeeRundownEvents); }); } +#endif // NET5_0_OR_GREATER } } From 5bb6ce1242c02e609661a4a553a954601dbac4f6 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Thu, 3 Nov 2022 09:11:05 -0700 Subject: [PATCH 30/45] Address PR feedback --- documentation/api/dump.md | 2 +- documentation/api/gcdump.md | 2 +- documentation/api/livemetrics-custom.md | 2 +- documentation/api/livemetrics-get.md | 2 +- documentation/api/logs-custom.md | 2 +- documentation/api/logs-get.md | 2 +- documentation/api/operations-delete.md | 2 +- documentation/api/stacks.md | 2 +- documentation/api/trace-custom.md | 2 +- documentation/api/trace-get.md | 2 +- .../Operation/HttpResponseEgressOperation.cs | 1 - .../HttpApi/ApiClient.cs | 11 ++--------- 12 files changed, 12 insertions(+), 20 deletions(-) diff --git a/documentation/api/dump.md b/documentation/api/dump.md index ec2271bff85..88df9721d4a 100644 --- a/documentation/api/dump.md +++ b/documentation/api/dump.md @@ -51,7 +51,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many dump requests at this time. Try to request a dump at a later time. | | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/gcdump.md b/documentation/api/gcdump.md index 6c3f0456a9f..476d5b6c4e5 100644 --- a/documentation/api/gcdump.md +++ b/documentation/api/gcdump.md @@ -54,7 +54,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many GC dump requests at this time. Try to request a GC dump at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/livemetrics-custom.md b/documentation/api/livemetrics-custom.md index cd819d82f70..633605b10e7 100644 --- a/documentation/api/livemetrics-custom.md +++ b/documentation/api/livemetrics-custom.md @@ -55,7 +55,7 @@ The expected content type is `application/json`. | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many requests at this time. Try to request metrics at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/livemetrics-get.md b/documentation/api/livemetrics-get.md index c510553f0c0..c687643b7c7 100644 --- a/documentation/api/livemetrics-get.md +++ b/documentation/api/livemetrics-get.md @@ -51,7 +51,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many requests at this time. Try to request metrics at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/logs-custom.md b/documentation/api/logs-custom.md index 77e8616b783..053c5ce2b28 100644 --- a/documentation/api/logs-custom.md +++ b/documentation/api/logs-custom.md @@ -58,7 +58,7 @@ The expected content type is `application/json`. | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many logs requests at this time. Try to request logs at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/logs-get.md b/documentation/api/logs-get.md index cf12ffd8448..82f89bd1330 100644 --- a/documentation/api/logs-get.md +++ b/documentation/api/logs-get.md @@ -53,7 +53,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many logs requests at this time. Try to request logs at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/operations-delete.md b/documentation/api/operations-delete.md index 525f7962a0c..637f639d712 100644 --- a/documentation/api/operations-delete.md +++ b/documentation/api/operations-delete.md @@ -3,7 +3,7 @@ # Operations - Delete -Cancel a running operation. Only valid against operations in the `Running` or `Stopping` state. Transitions the operation to `Cancelled` state. Cancelling an operation may result in an incomplete or unreadable artifact. To stop an operation early while still producing a valid artifact using [Stop Operation](operations-stop.md). +Cancel a running operation. Only valid against operations in the `Running` or `Stopping` state. Transitions the operation to `Cancelled` state. Cancelling an operation may result in an incomplete or unreadable artifact. To stop an operation early while still producing a valid artifact, use the [Stop Operation](operations-stop.md). ## HTTP Route diff --git a/documentation/api/stacks.md b/documentation/api/stacks.md index 363f4fc4083..5f0db7b4211 100644 --- a/documentation/api/stacks.md +++ b/documentation/api/stacks.md @@ -51,7 +51,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many stack requests at this time. Try to request a stack at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/trace-custom.md b/documentation/api/trace-custom.md index f329767dcf4..125830ca88b 100644 --- a/documentation/api/trace-custom.md +++ b/documentation/api/trace-custom.md @@ -55,7 +55,7 @@ The expected content type is `application/json`. | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many trace requests at this time. Try to request a trace at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. > **NOTE:** After the expiration of the trace duration, completing the request may take a long time (up to several minutes) for large applications if `EventProvidersConfiguration.RequestRundown` is set to `true`. The runtime needs to send over the type cache for all managed code that was captured in the trace, known as rundown events. Thus, the length of time of the request may take significantly longer than the requested duration. diff --git a/documentation/api/trace-get.md b/documentation/api/trace-get.md index d9682d1dc60..04568dce052 100644 --- a/documentation/api/trace-get.md +++ b/documentation/api/trace-get.md @@ -50,7 +50,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many trace requests at this time. Try to request a trace at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header containers the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. > **NOTE:** After the expiration of the trace duration, completing the request may take a long time (up to several minutes) for large applications. The runtime needs to send over the type cache for all managed code that was captured in the trace, known as rundown events. Thus, the length of time of the request may take significantly longer than the requested duration. diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index fbc2f8a7d89..2df53e21234 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -37,7 +37,6 @@ public async Task> ExecuteAsync(IServiceProvider s int statusCode = await _responseFinishedCompletionSource.Task.WaitAsync(cancellationTokenSource.Token); - return statusCode >= (int)HttpStatusCode.OK && statusCode < (int)HttpStatusCode.Ambiguous ? ExecutionResult.Empty() : ExecutionResult.Failed(new Exception($"HTTP request failed with status code: ${statusCode}")); 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 39aca33b5a6..e5443c0eb69 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs @@ -745,15 +745,8 @@ private async Task SendAndLogAsync(HttpRequestMessage reque _outputHelper.WriteLine("-> {0}", request.ToString()); } - if (completionOption == HttpCompletionOption.ResponseContentRead) - { - _outputHelper.WriteLine("<- {0}", response.ToString()); - _outputHelper.WriteLine($"Request duration: {sw.ElapsedMilliseconds} ms"); - } - else - { - _outputHelper.WriteLine($"Request ACK duration: {sw.ElapsedMilliseconds} ms"); - } + _outputHelper.WriteLine("<- {0}", response.ToString()); + _outputHelper.WriteLine($"Request duration: {sw.ElapsedMilliseconds} ms"); return response; } From cfde64aa4fa356e4d027a57d3eae11da63026590 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Thu, 3 Nov 2022 09:14:44 -0700 Subject: [PATCH 31/45] Trigger PR update --- documentation/api/operations-delete.md | 1 - 1 file changed, 1 deletion(-) diff --git a/documentation/api/operations-delete.md b/documentation/api/operations-delete.md index 637f639d712..49c1f4558f6 100644 --- a/documentation/api/operations-delete.md +++ b/documentation/api/operations-delete.md @@ -5,7 +5,6 @@ Cancel a running operation. Only valid against operations in the `Running` or `Stopping` state. Transitions the operation to `Cancelled` state. Cancelling an operation may result in an incomplete or unreadable artifact. To stop an operation early while still producing a valid artifact, use the [Stop Operation](operations-stop.md). - ## HTTP Route ```http From 08e8ffd12f34d5a46b8d7fa58405751afe380b54 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Thu, 3 Nov 2022 09:16:55 -0700 Subject: [PATCH 32/45] Remove old test code --- .../HttpApi/ApiClient.cs | 22 ------------------- 1 file changed, 22 deletions(-) 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 e5443c0eb69..cf0aa543c85 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs @@ -613,28 +613,6 @@ public async Task EgressTraceAsync(int processId, int duratio throw await CreateUnexpectedStatusCodeExceptionAsync(response).ConfigureAwait(false); } - public async Task CaptureTraceAsync(int processId, int durationSeconds, CancellationToken token) - { - string uri = FormattableString.Invariant($"/trace?pid={processId}&durationSeconds={durationSeconds}"); - using HttpRequestMessage request = new(HttpMethod.Get, uri); - using HttpResponseMessage response = await SendAndLogAsync(request, HttpCompletionOption.ResponseContentRead, token).ConfigureAwait(false); - - switch (response.StatusCode) - { - case HttpStatusCode.Accepted: - return new OperationResponse(response.StatusCode, response.Headers.Location); - case HttpStatusCode.BadRequest: - case HttpStatusCode.TooManyRequests: - ValidateContentType(response, ContentTypes.ApplicationProblemJson); - throw await CreateValidationProblemDetailsExceptionAsync(response).ConfigureAwait(false); - case HttpStatusCode.Unauthorized: - ThrowIfNotSuccess(response); - break; - } - - throw await CreateUnexpectedStatusCodeExceptionAsync(response).ConfigureAwait(false); - } - public async Task> GetOperations(CancellationToken token) { using HttpRequestMessage request = new(HttpMethod.Get, "/operations"); From 53837b4fbb5428fd3479d8113dc25c96e840c573 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Mon, 7 Nov 2022 09:13:14 -0800 Subject: [PATCH 33/45] Stash --- documentation/api/operations-stop.md | 4 +- .../Controllers/OperationsController.cs | 4 +- .../ITraceOperationFactory.cs | 23 ++++++ .../Operation/HttpResponseEgressOperation.cs | 2 +- .../HttpApi/ApiClient.cs | 2 +- .../Trace/AbstractTraceOperation.cs | 41 ++++++++++ .../dotnet-monitor/Trace/TraceOperation.cs | 37 +++++++++ .../Trace/TraceOperationFactory.cs | 33 ++++++++ .../Trace/TraceUntilEventOperation.cs | 76 +++++++++++++++++++ 9 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs create mode 100644 src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs create mode 100644 src/Tools/dotnet-monitor/Trace/TraceOperation.cs create mode 100644 src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs create mode 100644 src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs diff --git a/documentation/api/operations-stop.md b/documentation/api/operations-stop.md index 549b3fb6ecf..082ad39d20e 100644 --- a/documentation/api/operations-stop.md +++ b/documentation/api/operations-stop.md @@ -15,7 +15,7 @@ The following API endpoints have support for their operations being gracefully s ## HTTP Route ```http -DELETE /operations/{operationId}?gracefulStop=true HTTP/1.1 +DELETE /operations/{operationId}?stop=true HTTP/1.1 ``` ## Host Address @@ -43,7 +43,7 @@ Allowed schemes: ### Sample Request ```http -DELETE /operations/67f07e40-5cca-4709-9062-26302c484f18?gracefulStop=true HTTP/1.1 +DELETE /operations/67f07e40-5cca-4709-9062-26302c484f18?stop=true HTTP/1.1 Host: localhost:52323 Authorization: Bearer fffffffffffffffffffffffffffffffffffffffffff= ``` diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs index 7ea53039bc2..752442435b7 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs @@ -76,13 +76,13 @@ public IActionResult GetOperationStatus(Guid operationId) public IActionResult CancelOperation( Guid operationId, [FromQuery] - bool gracefulStop = false) + bool stop = false) { return this.InvokeService(() => { //Note that if the operation is not found, it will throw an InvalidOperationException and //return an error code. - if (gracefulStop) + if (stop) { // Stop operations are not instant, they are instead queued and can take an indeterminate amount of time. _operationsStore.StopOperation(operationId); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs new file mode 100644 index 00000000000..a6dac46de14 --- /dev/null +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs @@ -0,0 +1,23 @@ +// 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.EventPipe; +using Microsoft.Diagnostics.Monitoring.Options; + +namespace Microsoft.Diagnostics.Monitoring.WebApi +{ + /// + /// Factory for creating operations that produce trace artifacts. + /// + internal interface ITraceOperationFactory + { + /// + /// Creates an operation that produces a trace artifact. + /// + IArtifactOperation Create( + IEndpointInfo endpointInfo, + EventTracePipelineSettings settings, + LogFormat format); + } +} diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index 2df53e21234..28fa716f5f9 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -10,7 +10,7 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi { - internal class HttpResponseEgressOperation : IEgressOperation + internal sealed class HttpResponseEgressOperation : IEgressOperation { private readonly HttpContext _httpContext; private readonly TaskCompletionSource _responseFinishedCompletionSource = new(); 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 cf0aa543c85..edf757c69ac 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs @@ -658,7 +658,7 @@ public async Task GetOperationStatus(Uri operation, Can public async Task StopEgressOperation(Uri operation, CancellationToken token) { - string operationUri = QueryHelpers.AddQueryString(operation.ToString(), "gracefulStop", "true"); + string operationUri = QueryHelpers.AddQueryString(operation.ToString(), "stop", "true"); using HttpRequestMessage request = new(HttpMethod.Delete, operationUri); using HttpResponseMessage response = await SendAndLogAsync(request, HttpCompletionOption.ResponseContentRead, token).ConfigureAwait(false); diff --git a/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs b/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs new file mode 100644 index 00000000000..afdbb5644b4 --- /dev/null +++ b/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs @@ -0,0 +1,41 @@ +// 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.EventPipe; +using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Extensions.Logging; +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Utils = Microsoft.Diagnostics.Monitoring.WebApi.Utilities; + +namespace Microsoft.Diagnostics.Tools.Monitor +{ + internal abstract class AbstractTraceOperation : IArtifactOperation + { + // Buffer size matches FileStreamResult + protected const int DefaultBufferSize = 0x10000; + + protected readonly ILogger _logger; + protected readonly IEndpointInfo _endpointInfo; + protected readonly EventTracePipelineSettings _settings; + + public AbstractTraceOperation(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, ILogger logger) + { + _logger = logger; + _endpointInfo = endpointInfo; + _settings = settings; + } + + public abstract Task ExecuteAsync(Stream outputStream, TaskCompletionSource startCompletionSource, CancellationToken token); + + public string GenerateFileName() + { + return FormattableString.Invariant($"{Utils.GetFileNameTimeStampUtcNow()}_{_endpointInfo.ProcessId}.nettrace"); + } + + public string ContentType => ContentTypes.ApplicationOctetStream; + } +} diff --git a/src/Tools/dotnet-monitor/Trace/TraceOperation.cs b/src/Tools/dotnet-monitor/Trace/TraceOperation.cs new file mode 100644 index 00000000000..446c89d95b7 --- /dev/null +++ b/src/Tools/dotnet-monitor/Trace/TraceOperation.cs @@ -0,0 +1,37 @@ +// 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.EventPipe; +using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Diagnostics.NETCore.Client; +using Microsoft.Extensions.Logging; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Utils = Microsoft.Diagnostics.Monitoring.WebApi.Utilities; + +namespace Microsoft.Diagnostics.Tools.Monitor +{ + internal sealed class TraceOperation : AbstractTraceOperation + { + public TraceOperation(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, ILogger logger) + : base(endpointInfo, settings, logger) { } + + public override async Task ExecuteAsync(Stream outputStream, TaskCompletionSource startCompletionSource, CancellationToken token) + { + _logger.StartCollectArtifact(Utils.ArtifactType_Trace); + + DiagnosticsClient client = new(_endpointInfo.Endpoint); + + await using EventTracePipeline pipeProcessor = new EventTracePipeline(client, _settings, + async (eventStream, token) => + { + startCompletionSource.TrySetResult(null); + await eventStream.CopyToAsync(outputStream, DefaultBufferSize, token); + }); + + await pipeProcessor.RunAsync(token); + } + } +} diff --git a/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs b/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs new file mode 100644 index 00000000000..4a8f2f4d1f2 --- /dev/null +++ b/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs @@ -0,0 +1,33 @@ +// 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.EventPipe; +using Microsoft.Diagnostics.Monitoring.Options; +using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Diagnostics.Tools.Monitor +{ + internal sealed class TraceOperationFactory : ITraceOperationFactory + { + private readonly ILogger _logger; + + public TraceOperationFactory(ILogger logger) + { + _logger = logger; + } + + public IArtifactOperation Create(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, LogFormat format) + { + if (true) + { + return new TraceUntilEventOperation(endpointInfo, settings, format, _logger); + } + else + { + return new TraceOperation(endpointInfo, settings, format, _logger); + } + } + } +} diff --git a/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs b/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs new file mode 100644 index 00000000000..1ec51a78d74 --- /dev/null +++ b/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs @@ -0,0 +1,76 @@ +// 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.EventPipe; +using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Diagnostics.NETCore.Client; +using Microsoft.Extensions.Logging; +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Utils = Microsoft.Diagnostics.Monitoring.WebApi.Utilities; + +namespace Microsoft.Diagnostics.Tools.Monitor +{ + internal sealed class TraceUntilEventOperation : AbstractTraceOperation + { + private readonly string _providerName; + private readonly string _eventName; + private readonly IDictionary _payloadFilter; + + public TraceUntilEventOperation(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, string providerName, string eventName, IDictionary payloadFilter, ILogger logger) + : base(endpointInfo, settings, logger) + { + _providerName = providerName; + _eventName = eventName; + _payloadFilter = payloadFilter; + } + + public override async Task ExecuteAsync(Stream outputStream, TaskCompletionSource startCompletionSource, CancellationToken token) + { + _logger.StartCollectArtifact(Utils.ArtifactType_Trace); + + DiagnosticsClient client = new(_endpointInfo.Endpoint); + TaskCompletionSource stoppingEventHitSource = new(TaskCreationOptions.RunContinuationsAsynchronously); + + using IDisposable registration = token.Register( + () => stoppingEventHitSource.TrySetCanceled(token)); + + await using EventTracePipeline pipeProcessor = new(client, _settings, + async (eventStream, token) => + { + startCompletionSource?.TrySetResult(null); + + await using EventMonitoringPassthroughStream eventMonitoringStream = new( + _providerName, + _eventName, + _payloadFilter, + onEvent: (traceEvent) => + { + _logger.StoppingTraceEventHit(traceEvent); + stoppingEventHitSource.TrySetResult(null); + }, + onPayloadFilterMismatch: _logger.StoppingTraceEventPayloadFilterMismatch, + eventStream, + outputStream, + DefaultBufferSize, + callOnEventOnlyOnce: true, + leaveDestinationStreamOpen: true /* We do not have ownership of the outputStream */); + + await eventMonitoringStream.ProcessAsync(token); + }); + + Task pipelineRunTask = pipeProcessor.RunAsync(token); + await Task.WhenAny(pipelineRunTask, stoppingEventHitSource.Task).Unwrap(); + + if (stoppingEventHitSource.Task.IsCompleted) + { + await pipeProcessor.StopAsync(token); + await pipelineRunTask; + } + } + } +} From 62ddca21a3068c560772faa9326688ef28a14187 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Mon, 7 Nov 2022 14:24:10 -0800 Subject: [PATCH 34/45] Stash --- .../Controllers/DiagController.cs | 100 ++++++++---------- .../Controllers/OperationsController.cs | 6 +- .../ITraceOperationFactory.cs | 7 +- .../LoggingExtensions.cs | 11 ++ .../Operation/EgressOperation.cs | 15 +++ .../Operation/EgressOperationStore.cs | 22 ++-- .../Operation/EgressRequest.cs | 5 +- .../Operation/HttpResponseEgressOperation.cs | 27 +++++ .../Operation/IEgressOperation.cs | 4 + .../Strings.Designer.cs | 9 ++ .../Strings.resx | 6 ++ .../Utilities/TraceUtilities.cs | 93 ---------------- .../TestHostHelper.cs | 1 + .../Actions/CollectTraceAction.cs | 27 ++--- .../Commands/CollectCommandHandler.cs | 1 + .../PipelineArtifactOperation.cs | 7 +- .../Trace/AbstractTraceOperation.cs | 18 +--- .../dotnet-monitor/Trace/TraceOperation.cs | 21 ++-- .../Trace/TraceOperationFactory.cs | 21 ++-- .../Trace/TraceStoppingEvent.cs | 22 ++++ .../Trace/TraceUntilEventOperation.cs | 64 ++++++----- 21 files changed, 242 insertions(+), 245 deletions(-) create mode 100644 src/Tools/dotnet-monitor/Trace/TraceStoppingEvent.cs diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 5e25a3035bf..576f8e984a7 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -52,6 +52,7 @@ public partial class DiagController : ControllerBase private readonly ICollectionRuleService _collectionRuleService; private readonly ProfilerChannel _profilerChannel; private readonly ILogsOperationFactory _logsOperationFactory; + private readonly ITraceOperationFactory _traceOperationFactory; public DiagController(ILogger logger, IServiceProvider serviceProvider) @@ -67,6 +68,8 @@ public DiagController(ILogger logger, _collectionRuleService = serviceProvider.GetRequiredService(); _profilerChannel = serviceProvider.GetRequiredService(); _logsOperationFactory = serviceProvider.GetRequiredService(); + _traceOperationFactory = serviceProvider.GetRequiredService(); + } /// @@ -632,45 +635,28 @@ private Task StartTrace( TimeSpan duration, string egressProvider) { - string fileName = TraceUtilities.GenerateTraceFileName(processInfo.EndpointInfo); - TaskCompletionSource requestStopCompletionSource = new(); + IArtifactOperation traceOperation = _traceOperationFactory.Create( + processInfo.EndpointInfo, + configuration, + duration); + + if (_diagnosticPortOptions.Value.ConnectionMode == DiagnosticPortConnectionMode.Listen) + { + IDisposable operationRegistration = _operationTrackerService.Register(processInfo.EndpointInfo); + HttpContext.Response.RegisterForDispose(operationRegistration); + } return Result( Utilities.ArtifactType_Trace, egressProvider, - async (outputStream, token) => - { - IDisposable operationRegistration = null; - try - { - if (_diagnosticPortOptions.Value.ConnectionMode == DiagnosticPortConnectionMode.Listen) - { - operationRegistration = _operationTrackerService.Register(processInfo.EndpointInfo); - } - await TraceUtilities.CaptureTraceAsync( - startCompletionSource: null, - processInfo.EndpointInfo, - configuration, - duration, - outputStream, - requestStopCompletionSource, - token); - } - finally - { - operationRegistration?.Dispose(); - } - }, - fileName, - ContentTypes.ApplicationOctetStream, - processInfo, - requestStopCompletionSource: requestStopCompletionSource); + traceOperation, + processInfo); } private Task StartLogs( - IProcessInfo processInfo, - EventLogsPipelineSettings settings, - string egressProvider) + IProcessInfo processInfo, + EventLogsPipelineSettings settings, + string egressProvider) { LogFormat? format = ComputeLogFormat(Request.GetTypedHeaders().Accept); if (null == format) @@ -728,7 +714,7 @@ private Task StartLogs( return null; } - private Task Result( + private async Task Result( string artifactType, string providerName, IArtifactOperation operation, @@ -739,14 +725,15 @@ private Task Result( if (string.IsNullOrEmpty(providerName)) { - return Task.FromResult(new OutputStreamResult( + await RegisterCurrentHttpResponseAsOperation(processInfo, artifactType, operation); + return new OutputStreamResult( operation, asAttachment ? operation.GenerateFileName() : null, - scope)); + scope); } else { - return SendToEgress(new EgressOperation( + return await SendToEgress(new EgressOperation( operation, providerName, processInfo, @@ -756,20 +743,19 @@ private Task Result( } private async Task Result( - string artifactType, - string providerName, - Func action, - string fileName, - string contentType, - IProcessInfo processInfo, - bool asAttachment = true, - TaskCompletionSource requestStopCompletionSource = null) + string artifactType, + string providerName, + Func action, + string fileName, + string contentType, + IProcessInfo processInfo, + bool asAttachment = true) { KeyValueLogScope scope = Utilities.CreateArtifactScope(artifactType, processInfo.EndpointInfo); if (string.IsNullOrEmpty(providerName)) { - await RegisterCurrentHttpResponseAsOperation(processInfo, artifactType, requestStopCompletionSource); + await RegisterCurrentHttpResponseAsOperation(processInfo, artifactType); return new OutputStreamResult( action, contentType, @@ -785,33 +771,39 @@ private async Task Result( processInfo, contentType, scope), - limitKey: artifactType, - requestStopCompletionSource); + limitKey: artifactType); } } - private async Task SendToEgress(EgressOperation egressStreamResult, string limitKey) + private async Task RegisterCurrentHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, IArtifactOperation operation) + { + // While not strictly a Location redirect, use the same header as externally egressed operations for consistency. + HttpContext.Response.Headers["Location"] = await RegisterOperation( + new HttpResponseEgressOperation(HttpContext, processInfo, operation), + limitKey: artifactType); + } + + private async Task RegisterCurrentHttpResponseAsOperation(IProcessInfo processInfo, string artifactType) { // While not strictly a Location redirect, use the same header as externally egressed operations for consistency. HttpContext.Response.Headers["Location"] = await RegisterOperation( new HttpResponseEgressOperation(HttpContext, processInfo), - limitKey: artifactType, - requestStopCompletionSource); + limitKey: artifactType); } - private async Task RegisterOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestStopCompletionSource) + private async Task RegisterOperation(IEgressOperation egressOperation, string limitKey) { // Will throw TooManyRequestsException if there are too many concurrent operations. - Guid operationId = await _operationsStore.AddOperation(egressOperation, limitKey, requestStopCompletionSource); + Guid operationId = await _operationsStore.AddOperation(egressOperation, limitKey); return this.Url.Action( action: nameof(OperationsController.GetOperationStatus), controller: OperationsController.ControllerName, new { operationId = operationId }, protocol: this.HttpContext.Request.Scheme, this.HttpContext.Request.Host.ToString()); } - private async Task SendToEgress(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestStopCompletionSource = null) + private async Task SendToEgress(IEgressOperation egressOperation, string limitKey) { - string operationUrl = await RegisterOperation(egressOperation, limitKey, requestStopCompletionSource); + string operationUrl = await RegisterOperation(egressOperation, limitKey); return Accepted(operationUrl); } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs index 752442435b7..1c420885038 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/OperationsController.cs @@ -84,8 +84,12 @@ public IActionResult CancelOperation( //return an error code. if (stop) { + // If stopping an operation fails, it's undefined behavior. + // Leave the operation in the "Stopping" state and it'll either complete on its own + // or the user will cancel it. + _operationsStore.StopOperation(operationId, (ex) => _logger.StopOperationFailed(operationId, ex)); + // Stop operations are not instant, they are instead queued and can take an indeterminate amount of time. - _operationsStore.StopOperation(operationId); return Accepted(); } else diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs index a6dac46de14..9212a790744 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs @@ -3,7 +3,7 @@ // See the LICENSE file in the project root for more information. using Microsoft.Diagnostics.Monitoring.EventPipe; -using Microsoft.Diagnostics.Monitoring.Options; +using System; namespace Microsoft.Diagnostics.Monitoring.WebApi { @@ -17,7 +17,8 @@ internal interface ITraceOperationFactory /// IArtifactOperation Create( IEndpointInfo endpointInfo, - EventTracePipelineSettings settings, - LogFormat format); + MonitoringSourceConfiguration configuration, + TimeSpan duration, + object stoppingEvent = null); } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/LoggingExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/LoggingExtensions.cs index a4d2c05a244..d4e936fe6fd 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/LoggingExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/LoggingExtensions.cs @@ -70,6 +70,12 @@ internal static class LoggingExtensions logLevel: LogLevel.Debug, formatString: Strings.LogFormatString_DiagnosticRequestFailed); + private static readonly Action _stopOperationFailed = + LoggerMessage.Define( + eventId: new EventId(11, "StopOperationFailed"), + logLevel: LogLevel.Warning, + formatString: Strings.LogFormatString_StopOperationFailed); + public static void RequestFailed(this ILogger logger, Exception ex) { _requestFailed(logger, ex); @@ -119,5 +125,10 @@ public static void DiagnosticRequestFailed(this ILogger logger, int processId, E { _diagnosticRequestFailed(logger, processId, ex); } + + public static void StopOperationFailed(this ILogger logger, Guid operationId, Exception ex) + { + _stopOperationFailed(logger, operationId, ex); + } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs index 7bda9dd00a7..93542e90d29 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperation.cs @@ -17,6 +17,10 @@ internal class EgressOperation : IEgressOperation private readonly KeyValueLogScope _scope; public EgressProcessInfo ProcessInfo { get; private set; } public string EgressProviderName { get; private set; } + public bool IsStoppable { get { return _operation?.IsStoppable ?? false; } } + + private readonly IArtifactOperation _operation; + public EgressOperation(Func> action, string endpointName, string artifactName, IProcessInfo processInfo, string contentType, KeyValueLogScope scope, CollectionRuleMetadata collectionRuleMetadata = null) { @@ -39,6 +43,7 @@ public EgressOperation(Func action, string endp public EgressOperation(IArtifactOperation operation, string endpointName, IProcessInfo processInfo, KeyValueLogScope scope, CollectionRuleMetadata collectionRuleMetadata = null) : this(operation.ExecuteAsync, endpointName, operation.GenerateFileName(), processInfo, operation.ContentType, scope, collectionRuleMetadata) { + _operation = operation; } // The below constructors don't need EgressProcessInfo as their callers don't store to the operations table. @@ -88,5 +93,15 @@ public void Validate(IServiceProvider serviceProvider) .GetRequiredService() .ValidateProvider(EgressProviderName); } + + public Task StopAsync(CancellationToken token) + { + if (_operation == null) + { + throw new InvalidOperationException(); + } + + return _operation.StopAsync(token); + } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs index 024cbfbc090..53d40bcf61c 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs @@ -4,8 +4,10 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.Linq; +using System.Threading; using System.Threading.Tasks; namespace Microsoft.Diagnostics.Monitoring.WebApi @@ -18,7 +20,7 @@ public bool IsStoppable { get { - return State == Models.OperationState.Running && EgressRequest.RequestStopCompletionSource != null; + return State == Models.OperationState.Running && EgressRequest.EgressOperation.IsStoppable; } } @@ -47,7 +49,7 @@ public EgressOperationStore( _serviceProvider = serviceProvider; } - public async Task AddOperation(IEgressOperation egressOperation, string limitKey, TaskCompletionSource requestStopCompletionSource = null) + public async Task AddOperation(IEgressOperation egressOperation, string limitKey) { egressOperation.Validate(_serviceProvider); @@ -63,7 +65,7 @@ public async Task AddOperation(IEgressOperation egressOperation, string li throw new TooManyRequestsException(); } - var request = new EgressRequest(operationId, egressOperation, limitTracker, requestStopCompletionSource); + var request = new EgressRequest(operationId, egressOperation, limitTracker); lock (_requests) { //Add operation object to central table. @@ -80,7 +82,7 @@ public async Task AddOperation(IEgressOperation egressOperation, string li return operationId; } - public void StopOperation(Guid operationId) + public void StopOperation(Guid operationId, Action onStopException, CancellationToken stopCancellatonToken = default) { lock (_requests) { @@ -94,13 +96,21 @@ public void StopOperation(Guid operationId) throw new InvalidOperationException(Strings.ErrorMessage_OperationNotRunning); } - if (entry.EgressRequest.RequestStopCompletionSource == null) + if (!entry.EgressRequest.EgressOperation.IsStoppable) { throw new InvalidOperationException(Strings.ErrorMessage_OperationDoesNotSupportBeingStopped); } - entry.EgressRequest.RequestStopCompletionSource.TrySetResult(null); entry.State = Models.OperationState.Stopping; + _ = Task.Run(() => entry.EgressRequest.EgressOperation.StopAsync(stopCancellatonToken), stopCancellatonToken) + .ContinueWith(task => + { + Debug.Assert(task.Exception != null); + onStopException(task.Exception); + }, + stopCancellatonToken, + TaskContinuationOptions.OnlyOnFaulted, + TaskScheduler.Default); } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs index bae4c941c30..bb914d8ccfd 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressRequest.cs @@ -4,7 +4,6 @@ using System; using System.Threading; -using System.Threading.Tasks; namespace Microsoft.Diagnostics.Monitoring.WebApi { @@ -17,16 +16,14 @@ internal sealed class EgressRequest : IDisposable private bool _disposed; private IDisposable _limitTracker; - public EgressRequest(Guid operationId, IEgressOperation egressOperation, IDisposable limitTracker, TaskCompletionSource requestStopCompletionSource) + public EgressRequest(Guid operationId, IEgressOperation egressOperation, IDisposable limitTracker) { OperationId = operationId; EgressOperation = egressOperation; - RequestStopCompletionSource = requestStopCompletionSource; _limitTracker = limitTracker; } public CancellationTokenSource CancellationTokenSource { get; } = new(); - public TaskCompletionSource RequestStopCompletionSource { get; } public Guid OperationId { get; } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index 28fa716f5f9..8b51d15376c 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -17,6 +17,23 @@ internal sealed class HttpResponseEgressOperation : IEgressOperation public EgressProcessInfo ProcessInfo { get; private set; } public string EgressProviderName { get { return null; } } + public bool IsStoppable { get { return _operation?.IsStoppable ?? false; } } + + private readonly IArtifactOperation _operation; + + public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo, IArtifactOperation operation) + { + _httpContext = context; + _httpContext.Response.OnCompleted(() => + { + _responseFinishedCompletionSource.TrySetResult(_httpContext.Response.StatusCode); + return Task.CompletedTask; + }); + + _operation = operation; + + ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); + } public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo) { @@ -46,5 +63,15 @@ public void Validate(IServiceProvider serviceProvider) { // noop } + + public Task StopAsync(CancellationToken token) + { + if (_operation == null) + { + throw new InvalidOperationException(); + } + + return _operation.StopAsync(token); + } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs index 4ba00a8f88b..2ad9e935482 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/IEgressOperation.cs @@ -10,12 +10,16 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi { internal interface IEgressOperation { + public bool IsStoppable { get; } + public string EgressProviderName { get; } public EgressProcessInfo ProcessInfo { get; } Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token); + Task StopAsync(CancellationToken token); + void Validate(IServiceProvider serviceProvider); } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs index 104919c0e99..027c1cc34b6 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs @@ -303,6 +303,15 @@ internal static string LogFormatString_ResolvedTargetProcess { } } + /// + /// Looks up a localized string similar to Failed to stop the '{operationId}' operation.. + /// + internal static string LogFormatString_StopOperationFailed { + get { + return ResourceManager.GetString("LogFormatString_StopOperationFailed", resourceCulture); + } + } + /// /// Looks up a localized string similar to Hit stopping trace event '{providerName}/{eventName}'. /// diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx index 0f462a53bdf..9843b3f059c 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx @@ -231,6 +231,12 @@ Resolved target process. Gets the format string that is printed in the 3:ResolvedTargetProcess event. 0 Format Parameters + + + Failed to stop the '{operationId}' operation. + Gets the format string that is printed in the 11:StopOperationFailed event. +1 Format Parameter: +1. operationId: The id of the operation that failed to stop. Hit stopping trace event '{providerName}/{eventName}' diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs index a82cfd6c567..69e7294d0c4 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs @@ -8,23 +8,12 @@ using Microsoft.Extensions.Logging; using System; using System.Collections.Generic; -using System.IO; using System.Linq; -using System.Threading; -using System.Threading.Tasks; namespace Microsoft.Diagnostics.Monitoring.WebApi { internal static class TraceUtilities { - // Buffer size matches FileStreamResult - private const int DefaultBufferSize = 0x10000; - - public static string GenerateTraceFileName(IEndpointInfo endpointInfo) - { - return FormattableString.Invariant($"{Utilities.GetFileNameTimeStampUtcNow()}_{endpointInfo.ProcessId}.nettrace"); - } - public static MonitoringSourceConfiguration GetTraceConfiguration(Models.TraceProfile profile, float metricsIntervalSeconds) { var configurations = new List(); @@ -76,87 +65,5 @@ public static MonitoringSourceConfiguration GetTraceConfiguration(Models.EventPi requestRundown: requestRundown, bufferSizeInMB: bufferSizeInMB); } - - public static async Task CaptureTraceAsync(TaskCompletionSource startCompletionSource, IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan duration, Stream outputStream, TaskCompletionSource requestStopCompletionSource, CancellationToken token) - { - Func streamAvailable = async (Stream eventStream, CancellationToken token) => - { - if (null != startCompletionSource) - { - startCompletionSource.TrySetResult(null); - } - //CONSIDER Should we allow client to change the buffer size? - await eventStream.CopyToAsync(outputStream, DefaultBufferSize, token); - }; - - var client = new DiagnosticsClient(endpointInfo.Endpoint); - - await using EventTracePipeline pipeProcessor = new EventTracePipeline(client, new EventTracePipelineSettings - { - Configuration = configuration, - Duration = duration, - }, streamAvailable); - - Task pipelineRunTask = pipeProcessor.RunAsync(token); - if (requestStopCompletionSource == null) - { - await pipelineRunTask; - } - else - { - await Task.WhenAny(pipelineRunTask, requestStopCompletionSource.Task).Unwrap(); - - if (requestStopCompletionSource.Task.IsCompleted) - { - await pipeProcessor.StopAsync(token); - await pipelineRunTask; - } - } - } - - public static async Task CaptureTraceUntilEventAsync(TaskCompletionSource startCompletionSource, IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan timeout, Stream outputStream, string providerName, string eventName, IDictionary payloadFilter, ILogger logger, CancellationToken token) - { - DiagnosticsClient client = new(endpointInfo.Endpoint); - TaskCompletionSource stoppingEventHitSource = new(TaskCreationOptions.RunContinuationsAsynchronously); - - using IDisposable registration = token.Register( - () => stoppingEventHitSource.TrySetCanceled(token)); - - await using EventTracePipeline pipeProcessor = new(client, new EventTracePipelineSettings - { - Configuration = configuration, - Duration = timeout, - }, - async (eventStream, token) => - { - startCompletionSource?.TrySetResult(null); - await using EventMonitoringPassthroughStream eventMonitoringStream = new( - providerName, - eventName, - payloadFilter, - onEvent: (traceEvent) => - { - logger.StoppingTraceEventHit(traceEvent); - stoppingEventHitSource.TrySetResult(null); - }, - onPayloadFilterMismatch: logger.StoppingTraceEventPayloadFilterMismatch, - eventStream, - outputStream, - DefaultBufferSize, - callOnEventOnlyOnce: true, - leaveDestinationStreamOpen: true /* We do not have ownership of the outputStream */); - - await eventMonitoringStream.ProcessAsync(token); - }); - - Task pipelineRunTask = pipeProcessor.RunAsync(token); - await Task.WhenAny(pipelineRunTask, stoppingEventHitSource.Task).Unwrap(); - - if (stoppingEventHitSource.Task.IsCompleted) - { - await pipeProcessor.StopAsync(token); - await pipelineRunTask; - } - } } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs index 546920f0a8a..a3b917e0917 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs @@ -109,6 +109,7 @@ public static IHost CreateHost( services.ConfigureInProcessFeatures(context.Configuration); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); servicesCallback?.Invoke(services); }) .Build(); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs index ed3eb491b2e..2bd750724c6 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs @@ -8,7 +8,6 @@ using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Exceptions; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using System; using System.Collections.Generic; @@ -86,31 +85,25 @@ protected override async Task ExecuteCoreAsync( stoppingEvent = Options.StoppingEvent; } - string fileName = TraceUtilities.GenerateTraceFileName(EndpointInfo); - KeyValueLogScope scope = Utils.CreateArtifactScope(Utils.ArtifactType_Trace, EndpointInfo); + ITraceOperationFactory operationFactory = _serviceProvider.GetRequiredService(); + IArtifactOperation operation = operationFactory.Create( + EndpointInfo, + configuration, + duration, + stoppingEvent); + EgressOperation egressOperation = new EgressOperation( async (outputStream, token) => { using IDisposable operationRegistration = _operationTrackerService.Register(EndpointInfo); - if (null != stoppingEvent) - { - ILogger logger = _serviceProvider - .GetRequiredService() - .CreateLogger(); - - await TraceUtilities.CaptureTraceUntilEventAsync(startCompletionSource, EndpointInfo, configuration, duration, outputStream, stoppingEvent.ProviderName, stoppingEvent.EventName, stoppingEvent.PayloadFilter, logger, token); - } - else - { - await TraceUtilities.CaptureTraceAsync(startCompletionSource, EndpointInfo, configuration, duration, outputStream, requestStopCompletionSource: null, token); - } + await operation.ExecuteAsync(outputStream, startCompletionSource, token); }, egressProvider, - fileName, + operation.GenerateFileName(), EndpointInfo, - ContentTypes.ApplicationOctetStream, + operation.ContentType, scope, collectionRuleMetadata); diff --git a/src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs b/src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs index 0d6b8b2b9ec..0eaf42d58db 100644 --- a/src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs +++ b/src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs @@ -168,6 +168,7 @@ private static IHostBuilder Configure(this IHostBuilder builder, AuthConfigurati services.ConfigureInProcessFeatures(context.Configuration); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); }) .ConfigureContainer((HostBuilderContext context, IServiceCollection services) => { diff --git a/src/Tools/dotnet-monitor/PipelineArtifactOperation.cs b/src/Tools/dotnet-monitor/PipelineArtifactOperation.cs index 0efe1f3e6af..615f44e7f31 100644 --- a/src/Tools/dotnet-monitor/PipelineArtifactOperation.cs +++ b/src/Tools/dotnet-monitor/PipelineArtifactOperation.cs @@ -17,15 +17,14 @@ internal abstract class PipelineArtifactOperation : where T : Pipeline { private readonly string _artifactType; - private readonly ILogger _logger; private Func _stopFunc; protected PipelineArtifactOperation(ILogger logger, string artifactType, IEndpointInfo endpointInfo, bool isStoppable = true) { _artifactType = artifactType; - _logger = logger; + Logger = logger; EndpointInfo = endpointInfo; IsStoppable = isStoppable; } @@ -38,7 +37,7 @@ public async Task ExecuteAsync(Stream outputStream, TaskCompletionSource Task runTask = await StartPipelineAsync(pipeline, token); - _logger.StartCollectArtifact(_artifactType); + Logger.StartCollectArtifact(_artifactType); // Signal that the logs operation has started startCompletionSource?.TrySetResult(null); @@ -78,5 +77,7 @@ public async Task StopAsync(CancellationToken token) protected abstract Task StartPipelineAsync(T pipeline, CancellationToken token); protected IEndpointInfo EndpointInfo { get; } + + protected ILogger Logger { get; } } } diff --git a/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs b/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs index afdbb5644b4..3888ff88d38 100644 --- a/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs +++ b/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs @@ -6,36 +6,28 @@ using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Extensions.Logging; using System; -using System.IO; -using System.Threading; -using System.Threading.Tasks; using Utils = Microsoft.Diagnostics.Monitoring.WebApi.Utilities; namespace Microsoft.Diagnostics.Tools.Monitor { - internal abstract class AbstractTraceOperation : IArtifactOperation + internal abstract class AbstractTraceOperation : PipelineArtifactOperation { // Buffer size matches FileStreamResult protected const int DefaultBufferSize = 0x10000; - protected readonly ILogger _logger; - protected readonly IEndpointInfo _endpointInfo; protected readonly EventTracePipelineSettings _settings; public AbstractTraceOperation(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, ILogger logger) + : base(logger, Utils.ArtifactType_Trace, endpointInfo, isStoppable: true) { - _logger = logger; - _endpointInfo = endpointInfo; _settings = settings; } - public abstract Task ExecuteAsync(Stream outputStream, TaskCompletionSource startCompletionSource, CancellationToken token); - - public string GenerateFileName() + public override string GenerateFileName() { - return FormattableString.Invariant($"{Utils.GetFileNameTimeStampUtcNow()}_{_endpointInfo.ProcessId}.nettrace"); + return FormattableString.Invariant($"{Utils.GetFileNameTimeStampUtcNow()}_{EndpointInfo.ProcessId}.nettrace"); } - public string ContentType => ContentTypes.ApplicationOctetStream; + public override string ContentType => ContentTypes.ApplicationOctetStream; } } diff --git a/src/Tools/dotnet-monitor/Trace/TraceOperation.cs b/src/Tools/dotnet-monitor/Trace/TraceOperation.cs index 446c89d95b7..e57c9c9dcff 100644 --- a/src/Tools/dotnet-monitor/Trace/TraceOperation.cs +++ b/src/Tools/dotnet-monitor/Trace/TraceOperation.cs @@ -9,29 +9,32 @@ using System.IO; using System.Threading; using System.Threading.Tasks; -using Utils = Microsoft.Diagnostics.Monitoring.WebApi.Utilities; namespace Microsoft.Diagnostics.Tools.Monitor { internal sealed class TraceOperation : AbstractTraceOperation { + private readonly TaskCompletionSource _eventStreamAvailableCompletionSource = new(); + public TraceOperation(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, ILogger logger) : base(endpointInfo, settings, logger) { } - public override async Task ExecuteAsync(Stream outputStream, TaskCompletionSource startCompletionSource, CancellationToken token) + protected override EventTracePipeline CreatePipeline(Stream outputStream) { - _logger.StartCollectArtifact(Utils.ArtifactType_Trace); - - DiagnosticsClient client = new(_endpointInfo.Endpoint); - - await using EventTracePipeline pipeProcessor = new EventTracePipeline(client, _settings, + DiagnosticsClient client = new(EndpointInfo.Endpoint); + return new EventTracePipeline(client, _settings, async (eventStream, token) => { - startCompletionSource.TrySetResult(null); + _eventStreamAvailableCompletionSource.TrySetResult(null); await eventStream.CopyToAsync(outputStream, DefaultBufferSize, token); }); + } - await pipeProcessor.RunAsync(token); + protected override async Task StartPipelineAsync(EventTracePipeline pipeline, CancellationToken token) + { + Task pipelineRunTask = pipeline.RunAsync(token); + await _eventStreamAvailableCompletionSource.Task; + return pipelineRunTask; } } } diff --git a/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs b/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs index 4a8f2f4d1f2..3c594282bc5 100644 --- a/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs +++ b/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs @@ -3,9 +3,10 @@ // See the LICENSE file in the project root for more information. using Microsoft.Diagnostics.Monitoring.EventPipe; -using Microsoft.Diagnostics.Monitoring.Options; using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; using Microsoft.Extensions.Logging; +using System; namespace Microsoft.Diagnostics.Tools.Monitor { @@ -18,16 +19,18 @@ public TraceOperationFactory(ILogger logger) _logger = logger; } - public IArtifactOperation Create(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, LogFormat format) + public IArtifactOperation Create(IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan duration, object stoppingEvent = null) { - if (true) + EventTracePipelineSettings settings = new() { - return new TraceUntilEventOperation(endpointInfo, settings, format, _logger); - } - else - { - return new TraceOperation(endpointInfo, settings, format, _logger); - } + Configuration = configuration, + Duration = duration + }; + + // JSFIX: cast + return stoppingEvent != null + ? new TraceUntilEventOperation(endpointInfo, settings, (TraceEventFilter)stoppingEvent, _logger) + : new TraceOperation(endpointInfo, settings, _logger); } } } diff --git a/src/Tools/dotnet-monitor/Trace/TraceStoppingEvent.cs b/src/Tools/dotnet-monitor/Trace/TraceStoppingEvent.cs new file mode 100644 index 00000000000..4e4a4b61204 --- /dev/null +++ b/src/Tools/dotnet-monitor/Trace/TraceStoppingEvent.cs @@ -0,0 +1,22 @@ +// 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.Collections.Generic; + +namespace Microsoft.Diagnostics.Tools.Monitor +{ + internal readonly struct TraceStoppingEvent + { + public TraceStoppingEvent(string providerName, string eventName, IDictionary payloadFilter) + { + ProviderName = providerName; + EventName = eventName; + PayloadFilter = payloadFilter; + } + + public string ProviderName { get; } + public string EventName { get; } + public IDictionary PayloadFilter { get; } + } +} diff --git a/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs b/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs index 1ec51a78d74..e7cbf2c261a 100644 --- a/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs +++ b/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs @@ -5,55 +5,45 @@ using Microsoft.Diagnostics.Monitoring.EventPipe; using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.NETCore.Client; +using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; using Microsoft.Extensions.Logging; -using System; -using System.Collections.Generic; using System.IO; using System.Threading; using System.Threading.Tasks; -using Utils = Microsoft.Diagnostics.Monitoring.WebApi.Utilities; namespace Microsoft.Diagnostics.Tools.Monitor { internal sealed class TraceUntilEventOperation : AbstractTraceOperation { - private readonly string _providerName; - private readonly string _eventName; - private readonly IDictionary _payloadFilter; + private readonly TraceEventFilter _stoppingEvent; - public TraceUntilEventOperation(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, string providerName, string eventName, IDictionary payloadFilter, ILogger logger) + private readonly TaskCompletionSource _eventStreamAvailableCompletionSource = new(); + private readonly TaskCompletionSource _stoppingEventHitSource = new(); + + public TraceUntilEventOperation(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, TraceEventFilter stoppingEvent, ILogger logger) : base(endpointInfo, settings, logger) { - _providerName = providerName; - _eventName = eventName; - _payloadFilter = payloadFilter; + _stoppingEvent = stoppingEvent; } - public override async Task ExecuteAsync(Stream outputStream, TaskCompletionSource startCompletionSource, CancellationToken token) + protected override EventTracePipeline CreatePipeline(Stream outputStream) { - _logger.StartCollectArtifact(Utils.ArtifactType_Trace); - - DiagnosticsClient client = new(_endpointInfo.Endpoint); - TaskCompletionSource stoppingEventHitSource = new(TaskCreationOptions.RunContinuationsAsynchronously); - - using IDisposable registration = token.Register( - () => stoppingEventHitSource.TrySetCanceled(token)); - - await using EventTracePipeline pipeProcessor = new(client, _settings, + DiagnosticsClient client = new(EndpointInfo.Endpoint); + return new EventTracePipeline(client, _settings, async (eventStream, token) => { - startCompletionSource?.TrySetResult(null); + _eventStreamAvailableCompletionSource?.TrySetResult(null); await using EventMonitoringPassthroughStream eventMonitoringStream = new( - _providerName, - _eventName, - _payloadFilter, + _stoppingEvent.ProviderName, + _stoppingEvent.EventName, + _stoppingEvent.PayloadFilter, onEvent: (traceEvent) => { - _logger.StoppingTraceEventHit(traceEvent); - stoppingEventHitSource.TrySetResult(null); + Logger.StoppingTraceEventHit(traceEvent); + _stoppingEventHitSource.TrySetResult(null); }, - onPayloadFilterMismatch: _logger.StoppingTraceEventPayloadFilterMismatch, + onPayloadFilterMismatch: Logger.StoppingTraceEventPayloadFilterMismatch, eventStream, outputStream, DefaultBufferSize, @@ -62,15 +52,23 @@ public override async Task ExecuteAsync(Stream outputStream, TaskCompletionSourc await eventMonitoringStream.ProcessAsync(token); }); + } - Task pipelineRunTask = pipeProcessor.RunAsync(token); - await Task.WhenAny(pipelineRunTask, stoppingEventHitSource.Task).Unwrap(); + protected override async Task StartPipelineAsync(EventTracePipeline pipeline, CancellationToken token) + { + Task pipelineRunTask = pipeline.RunAsync(token); + await _eventStreamAvailableCompletionSource.Task; - if (stoppingEventHitSource.Task.IsCompleted) + return Task.Run(async () => { - await pipeProcessor.StopAsync(token); - await pipelineRunTask; - } + await Task.WhenAny(pipelineRunTask, _stoppingEventHitSource.Task).Unwrap(); + + if (_stoppingEventHitSource.Task.IsCompleted) + { + await pipeline.StopAsync(token); + await pipelineRunTask; + } + }, token); } } } From 88e9d4ca9f5cf7cf4e3ca4cc3d09859be5164d92 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Mon, 7 Nov 2022 14:35:19 -0800 Subject: [PATCH 35/45] Remove remaining JSFIX --- .../Controllers/DiagController.cs | 10 +------- .../ITraceOperationFactory.cs | 13 +++++++++- .../Operation/HttpResponseEgressOperation.cs | 14 +---------- .../Actions/CollectTraceAction.cs | 23 ++++++++++++++---- .../Trace/TraceOperationFactory.cs | 20 +++++++++++----- .../Trace/TraceUntilEventOperation.cs | 24 +++++++++++++------ 6 files changed, 63 insertions(+), 41 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 576f8e984a7..8c9368dfc8d 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -775,7 +775,7 @@ private async Task Result( } } - private async Task RegisterCurrentHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, IArtifactOperation operation) + private async Task RegisterCurrentHttpResponseAsOperation(IProcessInfo processInfo, string artifactType, IArtifactOperation operation = null) { // While not strictly a Location redirect, use the same header as externally egressed operations for consistency. HttpContext.Response.Headers["Location"] = await RegisterOperation( @@ -783,14 +783,6 @@ private async Task RegisterCurrentHttpResponseAsOperation(IProcessInfo processIn limitKey: artifactType); } - private async Task RegisterCurrentHttpResponseAsOperation(IProcessInfo processInfo, string artifactType) - { - // While not strictly a Location redirect, use the same header as externally egressed operations for consistency. - HttpContext.Response.Headers["Location"] = await RegisterOperation( - new HttpResponseEgressOperation(HttpContext, processInfo), - limitKey: artifactType); - } - private async Task RegisterOperation(IEgressOperation egressOperation, string limitKey) { // Will throw TooManyRequestsException if there are too many concurrent operations. diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs index 9212a790744..b82e6c04a99 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/ITraceOperationFactory.cs @@ -4,6 +4,7 @@ using Microsoft.Diagnostics.Monitoring.EventPipe; using System; +using System.Collections.Generic; namespace Microsoft.Diagnostics.Monitoring.WebApi { @@ -15,10 +16,20 @@ internal interface ITraceOperationFactory /// /// Creates an operation that produces a trace artifact. /// + IArtifactOperation Create( + IEndpointInfo endpointInfo, + MonitoringSourceConfiguration configuration, + TimeSpan duration); + + /// + /// Creates an operation that produces a trace artifact and supports a stopping event. + /// IArtifactOperation Create( IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan duration, - object stoppingEvent = null); + string providerName, + string eventName, + IDictionary payloadFilter); } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs index 8b51d15376c..62716ce2db2 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/HttpResponseEgressOperation.cs @@ -21,7 +21,7 @@ internal sealed class HttpResponseEgressOperation : IEgressOperation private readonly IArtifactOperation _operation; - public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo, IArtifactOperation operation) + public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo, IArtifactOperation operation = null) { _httpContext = context; _httpContext.Response.OnCompleted(() => @@ -35,18 +35,6 @@ public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); } - public HttpResponseEgressOperation(HttpContext context, IProcessInfo processInfo) - { - _httpContext = context; - _httpContext.Response.OnCompleted(() => - { - _responseFinishedCompletionSource.TrySetResult(_httpContext.Response.StatusCode); - return Task.CompletedTask; - }); - - ProcessInfo = new EgressProcessInfo(processInfo.ProcessName, processInfo.EndpointInfo.ProcessId, processInfo.EndpointInfo.RuntimeInstanceCookie); - } - public async Task> ExecuteAsync(IServiceProvider serviceProvider, CancellationToken token) { using CancellationTokenSource cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(token, _httpContext.RequestAborted); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs index 2bd750724c6..4bbe48ebd36 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs @@ -88,11 +88,24 @@ protected override async Task ExecuteCoreAsync( KeyValueLogScope scope = Utils.CreateArtifactScope(Utils.ArtifactType_Trace, EndpointInfo); ITraceOperationFactory operationFactory = _serviceProvider.GetRequiredService(); - IArtifactOperation operation = operationFactory.Create( - EndpointInfo, - configuration, - duration, - stoppingEvent); + IArtifactOperation operation; + if (stoppingEvent == null) + { + operation = operationFactory.Create( + EndpointInfo, + configuration, + duration); + } + else + { + operation = operationFactory.Create( + EndpointInfo, + configuration, + duration, + stoppingEvent.ProviderName, + stoppingEvent.EventName, + stoppingEvent.PayloadFilter); + } EgressOperation egressOperation = new EgressOperation( async (outputStream, token) => diff --git a/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs b/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs index 3c594282bc5..4e3c6d6b457 100644 --- a/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs +++ b/src/Tools/dotnet-monitor/Trace/TraceOperationFactory.cs @@ -4,9 +4,9 @@ using Microsoft.Diagnostics.Monitoring.EventPipe; using Microsoft.Diagnostics.Monitoring.WebApi; -using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; using Microsoft.Extensions.Logging; using System; +using System.Collections.Generic; namespace Microsoft.Diagnostics.Tools.Monitor { @@ -19,7 +19,7 @@ public TraceOperationFactory(ILogger logger) _logger = logger; } - public IArtifactOperation Create(IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan duration, object stoppingEvent = null) + public IArtifactOperation Create(IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan duration) { EventTracePipelineSettings settings = new() { @@ -27,10 +27,18 @@ public IArtifactOperation Create(IEndpointInfo endpointInfo, MonitoringSourceCon Duration = duration }; - // JSFIX: cast - return stoppingEvent != null - ? new TraceUntilEventOperation(endpointInfo, settings, (TraceEventFilter)stoppingEvent, _logger) - : new TraceOperation(endpointInfo, settings, _logger); + return new TraceOperation(endpointInfo, settings, _logger); + } + + public IArtifactOperation Create(IEndpointInfo endpointInfo, MonitoringSourceConfiguration configuration, TimeSpan duration, string providerName, string eventName, IDictionary payloadFilter) + { + EventTracePipelineSettings settings = new() + { + Configuration = configuration, + Duration = duration + }; + + return new TraceUntilEventOperation(endpointInfo, settings, providerName, eventName, payloadFilter, _logger); } } } diff --git a/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs b/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs index e7cbf2c261a..5b65727c851 100644 --- a/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs +++ b/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs @@ -5,8 +5,8 @@ using Microsoft.Diagnostics.Monitoring.EventPipe; using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.NETCore.Client; -using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; using Microsoft.Extensions.Logging; +using System.Collections.Generic; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -15,15 +15,25 @@ namespace Microsoft.Diagnostics.Tools.Monitor { internal sealed class TraceUntilEventOperation : AbstractTraceOperation { - private readonly TraceEventFilter _stoppingEvent; + private readonly string _providerName; + private readonly string _eventName; + private readonly IDictionary _payloadFilter; private readonly TaskCompletionSource _eventStreamAvailableCompletionSource = new(); private readonly TaskCompletionSource _stoppingEventHitSource = new(); - public TraceUntilEventOperation(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, TraceEventFilter stoppingEvent, ILogger logger) + public TraceUntilEventOperation( + IEndpointInfo endpointInfo, + EventTracePipelineSettings settings, + string providerName, + string eventName, + IDictionary payloadFilter, + ILogger logger) : base(endpointInfo, settings, logger) { - _stoppingEvent = stoppingEvent; + _providerName = providerName; + _eventName = eventName; + _payloadFilter = payloadFilter; } protected override EventTracePipeline CreatePipeline(Stream outputStream) @@ -35,9 +45,9 @@ protected override EventTracePipeline CreatePipeline(Stream outputStream) _eventStreamAvailableCompletionSource?.TrySetResult(null); await using EventMonitoringPassthroughStream eventMonitoringStream = new( - _stoppingEvent.ProviderName, - _stoppingEvent.EventName, - _stoppingEvent.PayloadFilter, + _providerName, + _eventName, + _payloadFilter, onEvent: (traceEvent) => { Logger.StoppingTraceEventHit(traceEvent); From 93f44c33d7879962da0134d4e0a20f4444ff19b9 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Mon, 7 Nov 2022 14:41:20 -0800 Subject: [PATCH 36/45] Clean leftover code --- documentation/api/operations-stop.md | 5 ----- documentation/openapi.json | 2 +- .../Trace/TraceStoppingEvent.cs | 22 ------------------- 3 files changed, 1 insertion(+), 28 deletions(-) delete mode 100644 src/Tools/dotnet-monitor/Trace/TraceStoppingEvent.cs diff --git a/documentation/api/operations-stop.md b/documentation/api/operations-stop.md index 082ad39d20e..05caeb78045 100644 --- a/documentation/api/operations-stop.md +++ b/documentation/api/operations-stop.md @@ -7,11 +7,6 @@ Gracefully stops a running operation. Only valid against operations with the `is Stopping an operation may not happen immediately such as in the case of traces where stopping may collect rundown information. An operation in the `Stopping` state can still be cancelled using [Delete Operation](operations-delete.md). -## Supported Artifacts - -The following API endpoints have support for their operations being gracefully stopped when they are in the `Running` state. -- [`/trace`](./trace.md) - ## HTTP Route ```http diff --git a/documentation/openapi.json b/documentation/openapi.json index 6d2929ff808..53b5b96b0e5 100644 --- a/documentation/openapi.json +++ b/documentation/openapi.json @@ -1209,7 +1209,7 @@ } }, { - "name": "gracefulStop", + "name": "stop", "in": "query", "schema": { "type": "boolean", diff --git a/src/Tools/dotnet-monitor/Trace/TraceStoppingEvent.cs b/src/Tools/dotnet-monitor/Trace/TraceStoppingEvent.cs deleted file mode 100644 index 4e4a4b61204..00000000000 --- a/src/Tools/dotnet-monitor/Trace/TraceStoppingEvent.cs +++ /dev/null @@ -1,22 +0,0 @@ -// 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.Collections.Generic; - -namespace Microsoft.Diagnostics.Tools.Monitor -{ - internal readonly struct TraceStoppingEvent - { - public TraceStoppingEvent(string providerName, string eventName, IDictionary payloadFilter) - { - ProviderName = providerName; - EventName = eventName; - PayloadFilter = payloadFilter; - } - - public string ProviderName { get; } - public string EventName { get; } - public IDictionary PayloadFilter { get; } - } -} From f0cdb29d40b0e7687e35adc1c8834cf06c523eb3 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Mon, 7 Nov 2022 14:44:43 -0800 Subject: [PATCH 37/45] Fix merge fomatting issues --- .../Controllers/DiagController.cs | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 8c9368dfc8d..3406f99e826 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -69,7 +69,6 @@ public DiagController(ILogger logger, _profilerChannel = serviceProvider.GetRequiredService(); _logsOperationFactory = serviceProvider.GetRequiredService(); _traceOperationFactory = serviceProvider.GetRequiredService(); - } /// @@ -654,9 +653,9 @@ private Task StartTrace( } private Task StartLogs( - IProcessInfo processInfo, - EventLogsPipelineSettings settings, - string egressProvider) + IProcessInfo processInfo, + EventLogsPipelineSettings settings, + string egressProvider) { LogFormat? format = ComputeLogFormat(Request.GetTypedHeaders().Accept); if (null == format) @@ -743,13 +742,13 @@ private async Task Result( } private async Task Result( - string artifactType, - string providerName, - Func action, - string fileName, - string contentType, - IProcessInfo processInfo, - bool asAttachment = true) + string artifactType, + string providerName, + Func action, + string fileName, + string contentType, + IProcessInfo processInfo, + bool asAttachment = true) { KeyValueLogScope scope = Utilities.CreateArtifactScope(artifactType, processInfo.EndpointInfo); From e321ec0f61ad7c48cc94813ee1e945f298a818dc Mon Sep 17 00:00:00 2001 From: Joe Schmitt Date: Mon, 7 Nov 2022 15:33:42 -0800 Subject: [PATCH 38/45] Apply suggestions from code review Co-authored-by: Justin Anderson --- src/Tools/dotnet-monitor/Trace/TraceOperation.cs | 4 ++-- src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Tools/dotnet-monitor/Trace/TraceOperation.cs b/src/Tools/dotnet-monitor/Trace/TraceOperation.cs index e57c9c9dcff..153ea27bdf7 100644 --- a/src/Tools/dotnet-monitor/Trace/TraceOperation.cs +++ b/src/Tools/dotnet-monitor/Trace/TraceOperation.cs @@ -14,7 +14,7 @@ namespace Microsoft.Diagnostics.Tools.Monitor { internal sealed class TraceOperation : AbstractTraceOperation { - private readonly TaskCompletionSource _eventStreamAvailableCompletionSource = new(); + private readonly TaskCompletionSource _eventStreamAvailableCompletionSource = new(TaskCreationOptions.RunContinuationsAsynchronously); public TraceOperation(IEndpointInfo endpointInfo, EventTracePipelineSettings settings, ILogger logger) : base(endpointInfo, settings, logger) { } @@ -33,7 +33,7 @@ protected override EventTracePipeline CreatePipeline(Stream outputStream) protected override async Task StartPipelineAsync(EventTracePipeline pipeline, CancellationToken token) { Task pipelineRunTask = pipeline.RunAsync(token); - await _eventStreamAvailableCompletionSource.Task; + await _eventStreamAvailableCompletionSource.Task.WaitAsync(token); return pipelineRunTask; } } diff --git a/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs b/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs index 5b65727c851..57b4189f406 100644 --- a/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs +++ b/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs @@ -19,8 +19,8 @@ internal sealed class TraceUntilEventOperation : AbstractTraceOperation private readonly string _eventName; private readonly IDictionary _payloadFilter; - private readonly TaskCompletionSource _eventStreamAvailableCompletionSource = new(); - private readonly TaskCompletionSource _stoppingEventHitSource = new(); + private readonly TaskCompletionSource _eventStreamAvailableCompletionSource = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly TaskCompletionSource _stoppingEventHitSource = new(TaskCreationOptions.RunContinuationsAsynchronously); public TraceUntilEventOperation( IEndpointInfo endpointInfo, @@ -67,7 +67,7 @@ protected override EventTracePipeline CreatePipeline(Stream outputStream) protected override async Task StartPipelineAsync(EventTracePipeline pipeline, CancellationToken token) { Task pipelineRunTask = pipeline.RunAsync(token); - await _eventStreamAvailableCompletionSource.Task; + await _eventStreamAvailableCompletionSource.Task.WaitAsync(token); return Task.Run(async () => { From 890c7cb01c87e6a79a9b10e58f66860f47d3d5ba Mon Sep 17 00:00:00 2001 From: Joe Schmitt Date: Mon, 7 Nov 2022 15:33:56 -0800 Subject: [PATCH 39/45] Update src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs Co-authored-by: Justin Anderson --- src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs b/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs index 3888ff88d38..6f75ff5654b 100644 --- a/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs +++ b/src/Tools/dotnet-monitor/Trace/AbstractTraceOperation.cs @@ -18,7 +18,7 @@ internal abstract class AbstractTraceOperation : PipelineArtifactOperation Date: Tue, 8 Nov 2022 09:35:29 -0800 Subject: [PATCH 40/45] Use egress cancellation token --- .../Operation/EgressOperationStore.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs index 53d40bcf61c..a516a18e3fe 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Operation/EgressOperationStore.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Globalization; using System.Linq; using System.Threading; @@ -82,7 +81,7 @@ public async Task AddOperation(IEgressOperation egressOperation, string li return operationId; } - public void StopOperation(Guid operationId, Action onStopException, CancellationToken stopCancellatonToken = default) + public void StopOperation(Guid operationId, Action onStopException) { lock (_requests) { @@ -102,13 +101,11 @@ public void StopOperation(Guid operationId, Action onStopException, C } entry.State = Models.OperationState.Stopping; - _ = Task.Run(() => entry.EgressRequest.EgressOperation.StopAsync(stopCancellatonToken), stopCancellatonToken) - .ContinueWith(task => - { - Debug.Assert(task.Exception != null); - onStopException(task.Exception); - }, - stopCancellatonToken, + + CancellationToken token = entry.EgressRequest.CancellationTokenSource.Token; + _ = Task.Run(() => entry.EgressRequest.EgressOperation.StopAsync(token), token) + .ContinueWith(task => onStopException(task.Exception), + token, TaskContinuationOptions.OnlyOnFaulted, TaskScheduler.Default); } From 07f6ede3f2fc9bbebd43bdac97c8d272ebd953a4 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 8 Nov 2022 11:38:29 -0800 Subject: [PATCH 41/45] Adress PR feedback --- .../TraceTestUtilities.cs | 13 ++++++-- .../EgressTests.cs | 32 ++++--------------- .../CollectTraceActionTests.cs | 4 +-- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs index 533962327cd..a214ce5147a 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using Microsoft.Diagnostics.Tracing; +using Microsoft.Diagnostics.Tracing.Parsers.Clr; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -12,25 +13,33 @@ namespace Microsoft.Diagnostics.Monitoring.TestCommon { public static class TraceTestUtilities { - public static async Task ValidateTrace(Stream traceStream) + public static async Task ValidateTrace(Stream traceStream, bool expectRundown) { using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(CommonTestTimeouts.ValidateTraceTimeout); using var eventSource = new EventPipeEventSource(traceStream); // Dispose event source when cancelled. - using var _ = cancellationTokenSource.Token.Register(() => eventSource.Dispose()); + using var _ = cancellationTokenSource.Token.Register(eventSource.Dispose); bool foundTraceObject = false; + bool foundRundown = false; eventSource.Dynamic.All += (TraceEvent obj) => { foundTraceObject = true; }; + var rundown = new ClrRundownTraceEventParser(eventSource); + rundown.RuntimeStart += (data) => + { + foundRundown = true; + }; + await Task.Run(() => Assert.True(eventSource.Process()), cancellationTokenSource.Token); Assert.True(foundTraceObject); + Assert.Equal(expectRundown, foundRundown); } } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs index 98484615ec2..3151120b1c9 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/EgressTests.cs @@ -13,8 +13,6 @@ using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.Monitoring.WebApi.Models; using Microsoft.Diagnostics.Tools.Monitor; -using Microsoft.Diagnostics.Tracing; -using Microsoft.Diagnostics.Tracing.Parsers.Clr; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using System; @@ -23,6 +21,7 @@ using System.Linq; using System.Net; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; @@ -130,6 +129,7 @@ await ScenarioRunner.SingleTarget( OperationStatusResponse operationResult = await apiClient.GetOperationStatus(response.OperationUri); Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); Assert.Equal(OperationState.Running, operationResult.OperationStatus.Status); + Assert.True(operationResult.OperationStatus.IsStoppable); HttpStatusCode deleteStatus = await apiClient.StopEgressOperation(response.OperationUri); Assert.Equal(HttpStatusCode.Accepted, deleteStatus); @@ -300,7 +300,7 @@ await ScenarioRunner.SingleTarget( Assert.Equal(HttpStatusCode.OK, operationResult.StatusCode); Assert.Equal(OperationState.Cancelled, operationResult.OperationStatus.Status); - await Assert.ThrowsAsync(async () => await drainResponseTask); + await Assert.ThrowsAsync(() => drainResponseTask); await appRunner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }, @@ -333,7 +333,6 @@ await ScenarioRunner.SingleTarget( string traceFile = Path.Combine(tempDir.FullName, "test.nettrace"); using FileStream traceFileWriter = File.OpenWrite(traceFile); - using MemoryStream dstStream = new(); Task drainResponseTask = responseBox.Stream.CopyToAsync(traceFileWriter); // Make sure the operation exists @@ -345,7 +344,8 @@ await ScenarioRunner.SingleTarget( HttpStatusCode deleteStatus = await apiClient.StopEgressOperation(operationUri); Assert.Equal(HttpStatusCode.Accepted, deleteStatus); - await drainResponseTask; + using CancellationTokenSource timeoutCancellation = new(CommonTestTimeouts.TraceTimeout); + await drainResponseTask.WaitAsync(timeoutCancellation.Token); await traceFileWriter.DisposeAsync(); operationResult = await apiClient.GetOperationStatus(operationUri); @@ -353,7 +353,8 @@ await ScenarioRunner.SingleTarget( Assert.Equal(OperationState.Succeeded, operationResult.OperationStatus.Status); // Verify the resulting trace has rundown information - Assert.True(await DoesNettraceFileHaveRundown(traceFile)); + using FileStream traceStream = File.OpenRead(traceFile); + await TraceTestUtilities.ValidateTrace(traceStream, expectRundown: true); await appRunner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }, @@ -363,25 +364,6 @@ await ScenarioRunner.SingleTarget( }); } - private static Task DoesNettraceFileHaveRundown(string filePath) - { - return Task.Run(() => - { - using FileStream fs = File.OpenRead(filePath); - using EventPipeEventSource eventSource = new(fs); - - bool didSeeRundownEvents = false; - ClrRundownTraceEventParser rundown = new(eventSource); - rundown.RuntimeStart += (data) => - { - didSeeRundownEvents = true; - }; - - eventSource.Process(); - return didSeeRundownEvents; - }); - } - /// /// Tests that turning off HTTP egress results in an error for dumps and logs (gcdumps and traces are currently not tested) /// diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs index 49d33789a26..dec05520f64 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs @@ -10,13 +10,11 @@ using Microsoft.Diagnostics.Tools.Monitor.CollectionRules; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Actions; using Microsoft.Diagnostics.Tools.Monitor.CollectionRules.Options.Actions; -using Microsoft.Diagnostics.Tracing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using System; using System.Collections.Generic; using System.IO; -using System.Threading; using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; @@ -112,7 +110,7 @@ await runner.ExecuteAsync(async () => using FileStream traceStream = new(egressPath, FileMode.Open, FileAccess.Read); Assert.NotNull(traceStream); - await TraceTestUtilities.ValidateTrace(traceStream); + await TraceTestUtilities.ValidateTrace(traceStream, expectRundown: true); await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }); From 52c07be6ba8d9840ded7ae3a960e5186a3d9be3a Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 8 Nov 2022 12:02:09 -0800 Subject: [PATCH 42/45] Version docs --- documentation/api/definitions.md | 8 ++++---- documentation/api/dump.md | 2 +- documentation/api/gcdump.md | 2 +- documentation/api/livemetrics-custom.md | 2 +- documentation/api/livemetrics-get.md | 2 +- documentation/api/logs-custom.md | 2 +- documentation/api/logs-get.md | 2 +- documentation/api/operations-stop.md | 2 +- documentation/api/operations.md | 2 +- documentation/api/stacks.md | 2 +- documentation/api/trace-custom.md | 2 +- 11 files changed, 14 insertions(+), 14 deletions(-) diff --git a/documentation/api/definitions.md b/documentation/api/definitions.md index 62b4e818d0a..c136ab6e74e 100644 --- a/documentation/api/definitions.md +++ b/documentation/api/definitions.md @@ -282,8 +282,8 @@ Detailed information about an operation. | `operationId` | guid | Unique identifier for the operation. | | `createdDateTime` | datetime string | UTC DateTime string of when the operation was created. | | `status` | [OperationState](#operationstate) | The current status of operation. | -| `egressProviderName` | string | The name of the egress provider that the artifact is being sent to. This will be null if the artifact is being sent directly back to the user from an HTTP request. | -| `isStoppable` | bool | Whether this operation can be gracefully stopped using [Stop Operation](operations-stop.md). Not all operations support being stopped. | +| `egressProviderName` | string | (8.0+) The name of the egress provider that the artifact is being sent to. This will be null if the artifact is being sent directly back to the user from an HTTP request. | +| `isStoppable` | bool | (8.0+) Whether this operation can be gracefully stopped using [Stop Operation](operations-stop.md). Not all operations support being stopped. | ### Example @@ -308,8 +308,8 @@ Summary state of an operation. | `operationId` | guid | Unique identifier for the operation. | | `createdDateTime` | datetime string | UTC DateTime string of when the operation was created. | | `status` | [OperationState](#operationstate) | The current status of operation. | -| `egressProviderName` | string | The name of the egress provider that the artifact is being sent to. This will be null if the artifact is being sent directly back to the user from an HTTP request. | -| `isStoppable` | bool | Whether this operation can be gracefully stopped using [Stop Operation](operations-stop.md). Not all operations support being stopped. | +| `egressProviderName` | string | (8.0+) The name of the egress provider that the artifact is being sent to. This will be null if the artifact is being sent directly back to the user from an HTTP request. | +| `isStoppable` | bool | (8.0+) Whether this operation can be gracefully stopped using [Stop Operation](operations-stop.md). Not all operations support being stopped. | | `process` | [OperationProcessInfo](#operationprocessinfo) | (6.3+) The process on which the operation is performed. | ### Example diff --git a/documentation/api/dump.md b/documentation/api/dump.md index 88df9721d4a..df28864fde1 100644 --- a/documentation/api/dump.md +++ b/documentation/api/dump.md @@ -51,7 +51,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many dump requests at this time. Try to request a dump at a later time. | | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE: (8.0+)** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/gcdump.md b/documentation/api/gcdump.md index 476d5b6c4e5..a6d63677160 100644 --- a/documentation/api/gcdump.md +++ b/documentation/api/gcdump.md @@ -54,7 +54,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many GC dump requests at this time. Try to request a GC dump at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE: (8.0+)** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/livemetrics-custom.md b/documentation/api/livemetrics-custom.md index 633605b10e7..4f9f3aaca11 100644 --- a/documentation/api/livemetrics-custom.md +++ b/documentation/api/livemetrics-custom.md @@ -55,7 +55,7 @@ The expected content type is `application/json`. | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many requests at this time. Try to request metrics at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE: (8.0+)** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/livemetrics-get.md b/documentation/api/livemetrics-get.md index c687643b7c7..bedb17ab2ba 100644 --- a/documentation/api/livemetrics-get.md +++ b/documentation/api/livemetrics-get.md @@ -51,7 +51,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many requests at this time. Try to request metrics at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE: (8.0+)** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/logs-custom.md b/documentation/api/logs-custom.md index 053c5ce2b28..fb92e10923b 100644 --- a/documentation/api/logs-custom.md +++ b/documentation/api/logs-custom.md @@ -58,7 +58,7 @@ The expected content type is `application/json`. | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many logs requests at this time. Try to request logs at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE: (8.0+)** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/logs-get.md b/documentation/api/logs-get.md index 82f89bd1330..01b9dd97199 100644 --- a/documentation/api/logs-get.md +++ b/documentation/api/logs-get.md @@ -53,7 +53,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many logs requests at this time. Try to request logs at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE: (8.0+)** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/operations-stop.md b/documentation/api/operations-stop.md index 05caeb78045..6deaec64e34 100644 --- a/documentation/api/operations-stop.md +++ b/documentation/api/operations-stop.md @@ -1,7 +1,7 @@ ### Was this documentation helpful? [Share feedback](https://www.research.net/r/DGDQWXH?src=documentation%2Fapi%2Foperations-stop) -# Operations - Stop +# Operations - Stop (8.0+) Gracefully stops a running operation. Only valid against operations with the `isStoppable` property set to `true`, not all operations support being gracefully stopped. Transitions the operation to `Succeeded` or `Failed` state depending on if the operation was successful. diff --git a/documentation/api/operations.md b/documentation/api/operations.md index 834f234b370..6851da5eac0 100644 --- a/documentation/api/operations.md +++ b/documentation/api/operations.md @@ -10,4 +10,4 @@ Operations are used to track long running operations in dotnet-monitor, specific | [List Operations](operations-list.md) | Lists all the operations and their current state. | | [Get Operation](operations-get.md) | Get detailed information about an operation. | | [Delete Operation](operations-delete.md) | Cancels a running operation. | -| [Stop Operation](operations-stop.md) | Gracefully stops a running operation. | +| [Stop Operation](operations-stop.md) (8.0+) | Gracefully stops a running operation. | diff --git a/documentation/api/stacks.md b/documentation/api/stacks.md index 5f0db7b4211..27bf84e753d 100644 --- a/documentation/api/stacks.md +++ b/documentation/api/stacks.md @@ -51,7 +51,7 @@ Allowed schemes: | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many stack requests at this time. Try to request a stack at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE: (8.0+)** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. ## Examples diff --git a/documentation/api/trace-custom.md b/documentation/api/trace-custom.md index 125830ca88b..48cfa9e2c67 100644 --- a/documentation/api/trace-custom.md +++ b/documentation/api/trace-custom.md @@ -55,7 +55,7 @@ The expected content type is `application/json`. | 401 Unauthorized | | Authentication is required to complete the request. See [Authentication](./../authentication.md) for further information. | | | 429 Too Many Requests | | There are too many trace requests at this time. Try to request a trace at a later time. | `application/problem+json` | -> **NOTE:** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. +> **NOTE: (8.0+)** Regardless if an egress provider is specified if the request was successful (response codes 200 or 202), the Location header contains the URI of the operation. This can be used to query the status of the operation or change its state. > **NOTE:** After the expiration of the trace duration, completing the request may take a long time (up to several minutes) for large applications if `EventProvidersConfiguration.RequestRundown` is set to `true`. The runtime needs to send over the type cache for all managed code that was captured in the trace, known as rundown events. Thus, the length of time of the request may take significantly longer than the requested duration. From 9e4eab8e8331910bcd8fa41f9db508a9d3ebe9b8 Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 8 Nov 2022 12:06:23 -0800 Subject: [PATCH 43/45] Fix tests --- .../TraceTestUtilities.cs | 19 +++++++++++++------ .../CollectTraceActionTests.cs | 4 ++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs index a214ce5147a..2c63f732c62 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs @@ -13,7 +13,7 @@ namespace Microsoft.Diagnostics.Monitoring.TestCommon { public static class TraceTestUtilities { - public static async Task ValidateTrace(Stream traceStream, bool expectRundown) + public static async Task ValidateTrace(Stream traceStream, bool? expectRundown = null) { using CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(CommonTestTimeouts.ValidateTraceTimeout); @@ -30,16 +30,23 @@ public static async Task ValidateTrace(Stream traceStream, bool expectRundown) foundTraceObject = true; }; - var rundown = new ClrRundownTraceEventParser(eventSource); - rundown.RuntimeStart += (data) => + if (expectRundown.HasValue) { - foundRundown = true; - }; + var rundown = new ClrRundownTraceEventParser(eventSource); + rundown.RuntimeStart += (data) => + { + foundRundown = true; + }; + } await Task.Run(() => Assert.True(eventSource.Process()), cancellationTokenSource.Token); Assert.True(foundTraceObject); - Assert.Equal(expectRundown, foundRundown); + + if (expectRundown.HasValue) + { + Assert.Equal(expectRundown, foundRundown); + } } } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs index dec05520f64..517b648c2ec 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs @@ -83,7 +83,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => }); } - private async Task PerformTrace(IHost host, TargetFrameworkMoniker tfm) + private async Task PerformTrace(IHost host, TargetFrameworkMoniker tfm, bool expectRundown = true) { CollectTraceOptions options = ActionTestsHelper.GetActionOptions(host, DefaultRuleName); @@ -110,7 +110,7 @@ await runner.ExecuteAsync(async () => using FileStream traceStream = new(egressPath, FileMode.Open, FileAccess.Read); Assert.NotNull(traceStream); - await TraceTestUtilities.ValidateTrace(traceStream, expectRundown: true); + await TraceTestUtilities.ValidateTrace(traceStream); await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue); }); From 9d353e6bbe7c8feb50070c42d3d1aae0d87a8d1f Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 8 Nov 2022 12:07:27 -0800 Subject: [PATCH 44/45] Code cleanup --- .../CollectTraceActionTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs index 517b648c2ec..c9a540e8998 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs @@ -83,7 +83,7 @@ await TestHostHelper.CreateCollectionRulesHost(_outputHelper, rootOptions => }); } - private async Task PerformTrace(IHost host, TargetFrameworkMoniker tfm, bool expectRundown = true) + private async Task PerformTrace(IHost host, TargetFrameworkMoniker tfm) { CollectTraceOptions options = ActionTestsHelper.GetActionOptions(host, DefaultRuleName); From ca51f9cedec4eed2acc03c5264a018ab5eb77fee Mon Sep 17 00:00:00 2001 From: Joe Schmitt <1146681+schmittjoseph@users.noreply.github.com> Date: Tue, 8 Nov 2022 12:09:54 -0800 Subject: [PATCH 45/45] Cleanup --- .../TraceTestUtilities.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs index 2c63f732c62..0c088a1ff01 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs @@ -45,7 +45,7 @@ public static async Task ValidateTrace(Stream traceStream, bool? expectRundown = if (expectRundown.HasValue) { - Assert.Equal(expectRundown, foundRundown); + Assert.Equal(expectRundown.Value, foundRundown); } } }