From 3dcc3447be26c746f7d919e894d76f9c0a4440d0 Mon Sep 17 00:00:00 2001 From: Wiktor Kopec Date: Wed, 3 Nov 2021 23:08:17 -0700 Subject: [PATCH 1/2] Do not attempt to prune services during tracing --- .../Controllers/DiagController.cs | 19 ++++++- .../DumpService.cs | 40 ++------------- .../IDumpService.cs | 4 -- .../OperationTrackerService.cs | 50 +++++++++++++++++++ .../EndpointInfoSourceTests.cs | 26 +++++----- .../EndpointUtilities.cs | 7 +-- .../TestHostHelper.cs | 3 +- .../Actions/CollectTraceAction.cs | 15 +++++- .../DiagnosticsMonitorCommandHandler.cs | 1 + .../DumpServiceEndpointInfoSourceCallback.cs | 9 ++-- .../EndpointInfo/ServerEndpointInfoSource.cs | 8 +-- 11 files changed, 115 insertions(+), 67 deletions(-) create mode 100644 src/Microsoft.Diagnostics.Monitoring.WebApi/OperationTrackerService.cs diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 52e9114d09e..c02d2d085e8 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -46,6 +46,7 @@ public partial class DiagController : ControllerBase private readonly IOptionsMonitor _counterOptions; private readonly EgressOperationStore _operationsStore; private readonly IDumpService _dumpService; + private readonly OperationTrackerService _operationTrackerService; public DiagController(ILogger logger, IServiceProvider serviceProvider) @@ -56,6 +57,7 @@ public DiagController(ILogger logger, _operationsStore = serviceProvider.GetRequiredService(); _dumpService = serviceProvider.GetRequiredService(); _counterOptions = serviceProvider.GetRequiredService>(); + _operationTrackerService = serviceProvider.GetRequiredService(); } /// @@ -537,7 +539,22 @@ private Task StartTrace( return Result( Utilities.ArtifactType_Trace, egressProvider, - (outputStream, token) => TraceUtilities.CaptureTraceAsync(null, processInfo.EndpointInfo, configuration, duration, outputStream, token), + async (outputStream, token) => + { + IDisposable operationRegistration = null; + try + { + if (_diagnosticPortOptions.Value.ConnectionMode == DiagnosticPortConnectionMode.Listen) + { + operationRegistration = _operationTrackerService.Register(processInfo.EndpointInfo); + } + await TraceUtilities.CaptureTraceAsync(null, processInfo.EndpointInfo, configuration, duration, outputStream, token); + } + finally + { + operationRegistration?.Dispose(); + } + }, fileName, ContentTypes.ApplicationOctetStream, processInfo.EndpointInfo); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs index 311a4ecd345..3a53963dcb0 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/DumpService.cs @@ -17,16 +17,18 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi { internal class DumpService : IDumpService { - private readonly ConcurrentDictionary _operations = new(); + private readonly OperationTrackerService _operationTrackerService; private readonly IOptionsMonitor _portOptions; private readonly IOptionsMonitor _storageOptions; public DumpService( IOptionsMonitor storageOptions, - IOptionsMonitor portOptions) + IOptionsMonitor portOptions, + OperationTrackerService operationTrackerService) { _portOptions = portOptions ?? throw new ArgumentNullException(nameof(portOptions)); _storageOptions = storageOptions ?? throw new ArgumentNullException(nameof(StorageOptions)); + _operationTrackerService = operationTrackerService ?? throw new ArgumentNullException(nameof(operationTrackerService)); } public async Task DumpAsync(IEndpointInfo endpointInfo, Models.DumpType mode, CancellationToken token) @@ -50,7 +52,7 @@ public async Task DumpAsync(IEndpointInfo endpointInfo, Models.DumpType // a dump operation causing the runtime to temporarily be unresponsive. Long term, this // concept should be folded into RequestLimitTracker with registered endpoint information // and allowing to query the status of an endpoint for a given artifact. - operationRegistration = GetOperationsTracker(endpointInfo).Register(); + operationRegistration = _operationTrackerService.Register(endpointInfo); } try @@ -75,25 +77,6 @@ public async Task DumpAsync(IEndpointInfo endpointInfo, Models.DumpType return new AutoDeleteFileStream(dumpFilePath); } - public bool IsExecutingOperation(IEndpointInfo endpointInfo) - { - if (IsListenMode) - { - return GetOperationsTracker(endpointInfo).IsExecutingOperation; - } - return false; - } - - public void EndpointRemoved(IEndpointInfo endpointInfo) - { - _operations.TryRemove(endpointInfo, out _); - } - - private OperationsTracker GetOperationsTracker(IEndpointInfo endpointInfo) - { - return _operations.GetOrAdd(endpointInfo, _ => new OperationsTracker()); - } - private bool IsListenMode => _portOptions.CurrentValue.GetConnectionMode() == DiagnosticPortConnectionMode.Listen; /// @@ -134,19 +117,6 @@ private static DumpType MapDumpType(Models.DumpType dumpType) } } - private sealed class OperationsTracker : IDisposable - { - private int _count = 0; - public IDisposable Register() - { - Interlocked.Increment(ref _count); - return this; - } - - public bool IsExecutingOperation => 0 != Interlocked.CompareExchange(ref _count, 0, 0); - - public void Dispose() => Interlocked.Decrement(ref _count); - } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/IDumpService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/IDumpService.cs index 8bf57c1d029..c914b1c1690 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/IDumpService.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/IDumpService.cs @@ -11,9 +11,5 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi internal interface IDumpService { Task DumpAsync(IEndpointInfo endpointInfo, Models.DumpType mode, CancellationToken token); - - bool IsExecutingOperation(IEndpointInfo endpointInfo); - - void EndpointRemoved(IEndpointInfo endpointInfo); } } diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/OperationTrackerService.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/OperationTrackerService.cs new file mode 100644 index 00000000000..714dadb292d --- /dev/null +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/OperationTrackerService.cs @@ -0,0 +1,50 @@ +// 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.Collections.Concurrent; +using System.Threading; + +namespace Microsoft.Diagnostics.Monitoring.WebApi +{ + internal sealed class OperationTrackerService + { + private readonly ConcurrentDictionary _operations = new(); + + private sealed class OperationsTracker : IDisposable + { + private int _count = 0; + + public IDisposable Register() + { + Interlocked.Increment(ref _count); + return this; + } + + public bool IsExecutingOperation => 0 != Interlocked.CompareExchange(ref _count, 0, 0); + + public void Dispose() => Interlocked.Decrement(ref _count); + } + + public bool IsExecutingOperation(IEndpointInfo endpointInfo) + { + return GetOperationsTracker(endpointInfo).IsExecutingOperation; + } + + public IDisposable Register(IEndpointInfo endpointInfo) + { + return GetOperationsTracker(endpointInfo).Register(); + } + + public void EndpointRemoved(IEndpointInfo endpointInfo) + { + _operations.TryRemove(endpointInfo, out _); + } + + private OperationsTracker GetOperationsTracker(IEndpointInfo endpointInfo) + { + return _operations.GetOrAdd(endpointInfo, _ => new OperationsTracker()); + } + } +} diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs index 1930ac422be..763adfc90d2 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs @@ -161,8 +161,9 @@ await runners.ExecuteAsync(async () => public async Task ServerSourceNoPruneDuringDumpTest(TargetFrameworkMoniker appTfm) { EndpointInfoSourceCallback callback = new(_outputHelper); - MockDumpService dumpService = new(); - await using ServerSourceHolder sourceHolder = await _endpointUtilities.StartServerAsync(callback, dumpService); + var operatonTrackerService = new OperationTrackerService(); + MockDumpService dumpService = new(operatonTrackerService); + await using ServerSourceHolder sourceHolder = await _endpointUtilities.StartServerAsync(callback, dumpService, operatonTrackerService); AppRunner runner = _endpointUtilities.CreateAppRunner(sourceHolder.TransportName, appTfm); @@ -226,30 +227,27 @@ private sealed class MockDumpService : IDumpService private readonly TaskCompletionSource _dumpCompletionSource = new(TaskCreationOptions.RunContinuationsAsynchronously); - private int _operationCount = 0; + private readonly OperationTrackerService _operationTrackerService; + + public MockDumpService(OperationTrackerService operationTrackerService) + { + _operationTrackerService = operationTrackerService; + } public async Task DumpAsync(IEndpointInfo endpointInfo, DumpType mode, CancellationToken token) { - Interlocked.Increment(ref _operationCount); + IDisposable operationRegistration = null; try { + operationRegistration = _operationTrackerService.Register(endpointInfo); return await _dumpCompletionSource.Task; } finally { - Interlocked.Decrement(ref _operationCount); + operationRegistration.Dispose(); } } - public void EndpointRemoved(IEndpointInfo endpointInfo) - { - } - - public bool IsExecutingOperation(IEndpointInfo endpointInfo) - { - return 0 != Interlocked.CompareExchange(ref _operationCount, 0, 0); - } - public void CompleteOperation() { _dumpCompletionSource.SetResult(new MemoryStream()); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs index b0afb1fd7f4..3cc6344c7f3 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs @@ -28,7 +28,8 @@ public EndpointUtilities(ITestOutputHelper outputHelper) _outputHelper = outputHelper; } - public async Task StartServerAsync(EndpointInfoSourceCallback sourceCallback = null, IDumpService dumpService = null) + public async Task StartServerAsync(EndpointInfoSourceCallback sourceCallback = null, IDumpService dumpService = null, + OperationTrackerService operationTrackerService = null) { DiagnosticPortHelper.Generate(DiagnosticPortConnectionMode.Listen, out _, out string transportName); _outputHelper.WriteLine("Starting server endpoint info source at '" + transportName + "'."); @@ -39,7 +40,7 @@ public async Task StartServerAsync(EndpointInfoSourceCallbac callbacks.Add(sourceCallback); if (null != dumpService) { - callbacks.Add(new DumpServiceEndpointInfoSourceCallback(dumpService)); + callbacks.Add(new DumpServiceEndpointInfoSourceCallback(operationTrackerService)); } } @@ -50,7 +51,7 @@ public async Task StartServerAsync(EndpointInfoSourceCallbac EndpointName = transportName }); - ServerEndpointInfoSource source = new(portOptions, callbacks, dumpService); + ServerEndpointInfoSource source = new(portOptions, callbacks, operationTrackerService); await source.StartAsync(CancellationToken.None); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs index ee991752f2f..60148a135de 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs @@ -86,9 +86,10 @@ public static IHost CreateHost( .ConfigureServices((HostBuilderContext context, IServiceCollection services) => { services.ConfigureGlobalCounter(context.Configuration); + services.AddSingleton(); services.ConfigureCollectionRules(); services.ConfigureEgress(); - +; services.AddSingleton(); services.ConfigureStorage(context.Configuration); servicesCallback?.Invoke(services); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs index 9d34ad5d086..2829ebcc466 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs @@ -45,12 +45,14 @@ private sealed class CollectTraceAction : { private readonly IServiceProvider _serviceProvider; private readonly IOptionsMonitor _counterOptions; + private readonly OperationTrackerService _operationTrackerService; public CollectTraceAction(IServiceProvider serviceProvider, IEndpointInfo endpointInfo, CollectTraceOptions options) : base(endpointInfo, options) { _serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider)); _counterOptions = _serviceProvider.GetRequiredService>(); + _operationTrackerService = _serviceProvider.GetRequiredService(); } protected override async Task ExecuteCoreAsync( @@ -83,7 +85,18 @@ protected override async Task ExecuteCoreAsync( KeyValueLogScope scope = Utils.CreateArtifactScope(Utils.ArtifactType_Trace, EndpointInfo); EgressOperation egressOperation = new EgressOperation( - (outputStream, token) => TraceUtilities.CaptureTraceAsync(startCompletionSource, EndpointInfo, configuration, duration, outputStream, token), + async (outputStream, token) => + { + IDisposable operationRegistration = _operationTrackerService.Register(EndpointInfo); + try + { + await TraceUtilities.CaptureTraceAsync(startCompletionSource, EndpointInfo, configuration, duration, outputStream, token); + } + finally + { + operationRegistration.Dispose(); + } + }, egressProvider, fileName, EndpointInfo, diff --git a/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs b/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs index c87c9783516..7213aedd0aa 100644 --- a/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs +++ b/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs @@ -243,6 +243,7 @@ public static IHostBuilder CreateHostBuilder(IConsole console, string[] urls, st services.Configure(context.Configuration.GetSection(ConfigurationKeys.DiagnosticPort)); services.AddSingleton, DiagnosticPortValidateOptions>(); + services.AddSingleton(); services.ConfigureGlobalCounter(context.Configuration); diff --git a/src/Tools/dotnet-monitor/DumpServiceEndpointInfoSourceCallback.cs b/src/Tools/dotnet-monitor/DumpServiceEndpointInfoSourceCallback.cs index 232b5275094..a6f0c33a0e6 100644 --- a/src/Tools/dotnet-monitor/DumpServiceEndpointInfoSourceCallback.cs +++ b/src/Tools/dotnet-monitor/DumpServiceEndpointInfoSourceCallback.cs @@ -9,13 +9,14 @@ namespace Microsoft.Diagnostics.Tools.Monitor { + //TODO Rename this internal sealed class DumpServiceEndpointInfoSourceCallback : IEndpointInfoSourceCallbacks { - private readonly IDumpService _dumpService; + private readonly OperationTrackerService _operationTrackerService; - public DumpServiceEndpointInfoSourceCallback(IDumpService dumpService) + public DumpServiceEndpointInfoSourceCallback(OperationTrackerService operationTrackerService) { - _dumpService = dumpService ?? throw new ArgumentNullException(nameof(dumpService)); + _operationTrackerService = operationTrackerService ?? throw new ArgumentNullException(nameof(operationTrackerService)); } public Task OnAddedEndpointInfoAsync(IEndpointInfo endpointInfo, CancellationToken cancellationToken) @@ -30,7 +31,7 @@ public Task OnBeforeResumeAsync(IEndpointInfo endpointInfo, CancellationToken ca public Task OnRemovedEndpointInfoAsync(IEndpointInfo endpointInfo, CancellationToken cancellationToken) { - _dumpService.EndpointRemoved(endpointInfo); + _operationTrackerService.EndpointRemoved(endpointInfo); return Task.CompletedTask; } diff --git a/src/Tools/dotnet-monitor/EndpointInfo/ServerEndpointInfoSource.cs b/src/Tools/dotnet-monitor/EndpointInfo/ServerEndpointInfoSource.cs index 5790811b69d..5e777b682bc 100644 --- a/src/Tools/dotnet-monitor/EndpointInfo/ServerEndpointInfoSource.cs +++ b/src/Tools/dotnet-monitor/EndpointInfo/ServerEndpointInfoSource.cs @@ -42,7 +42,7 @@ internal sealed class ServerEndpointInfoSource : BackgroundService, IEndpointInf private readonly IEnumerable _callbacks; private readonly DiagnosticPortOptions _portOptions; - private readonly IDumpService _dumpService; + private readonly OperationTrackerService _operationTrackerService; /// /// Constructs a that aggregates diagnostic endpoints @@ -51,10 +51,10 @@ internal sealed class ServerEndpointInfoSource : BackgroundService, IEndpointInf public ServerEndpointInfoSource( IOptions portOptions, IEnumerable callbacks = null, - IDumpService dumpService = null) + OperationTrackerService operationTrackerService = null) { _callbacks = callbacks ?? Enumerable.Empty(); - _dumpService = dumpService; + _operationTrackerService = operationTrackerService; _portOptions = portOptions.Value; BoundedChannelOptions channelOptions = new(PendingRemovalChannelCapacity) @@ -253,7 +253,7 @@ private async Task CheckEndpointAsync(EndpointInfo info, CancellationToken // If a dump operation is in progress, the runtime is likely to not respond to // diagnostic requests. Do not check for responsiveness while the dump operation // is in progress. - if (_dumpService?.IsExecutingOperation(info) == true) + if (_operationTrackerService?.IsExecutingOperation(info) == true) { return true; } From c8dd77a5d19aaccdfd391ba2cdd9988047fb6ff4 Mon Sep 17 00:00:00 2001 From: Wiktor Kopec Date: Thu, 4 Nov 2021 10:17:43 -0700 Subject: [PATCH 2/2] PR Feedback + Log message for timeout --- .../EndpointInfoSourceTests.cs | 8 ++++---- .../EndpointUtilities.cs | 2 +- .../TestHostHelper.cs | 2 +- .../CollectionRules/Actions/CollectTraceAction.cs | 11 ++--------- .../DiagnosticsMonitorCommandHandler.cs | 2 +- .../EndpointInfo/ServerEndpointInfoSource.cs | 7 ++++++- src/Tools/dotnet-monitor/LoggingEventIds.cs | 1 + src/Tools/dotnet-monitor/LoggingExtensions.cs | 11 +++++++++++ ...rationTrackerServiceEndpointInfoSourceCallback.cs} | 5 ++--- src/Tools/dotnet-monitor/Strings.Designer.cs | 9 +++++++++ src/Tools/dotnet-monitor/Strings.resx | 3 +++ 11 files changed, 41 insertions(+), 20 deletions(-) rename src/Tools/dotnet-monitor/{DumpServiceEndpointInfoSourceCallback.cs => OperationTrackerServiceEndpointInfoSourceCallback.cs} (84%) diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs index 763adfc90d2..cacbf42875a 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointInfoSourceTests.cs @@ -161,9 +161,9 @@ await runners.ExecuteAsync(async () => public async Task ServerSourceNoPruneDuringDumpTest(TargetFrameworkMoniker appTfm) { EndpointInfoSourceCallback callback = new(_outputHelper); - var operatonTrackerService = new OperationTrackerService(); - MockDumpService dumpService = new(operatonTrackerService); - await using ServerSourceHolder sourceHolder = await _endpointUtilities.StartServerAsync(callback, dumpService, operatonTrackerService); + var operationTrackerService = new OperationTrackerService(); + MockDumpService dumpService = new(operationTrackerService); + await using ServerSourceHolder sourceHolder = await _endpointUtilities.StartServerAsync(callback, dumpService, operationTrackerService); AppRunner runner = _endpointUtilities.CreateAppRunner(sourceHolder.TransportName, appTfm); @@ -244,7 +244,7 @@ public async Task DumpAsync(IEndpointInfo endpointInfo, DumpType mode, C } finally { - operationRegistration.Dispose(); + operationRegistration?.Dispose(); } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs index 3cc6344c7f3..ddaf5ffafb8 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/EndpointUtilities.cs @@ -40,7 +40,7 @@ public async Task StartServerAsync(EndpointInfoSourceCallbac callbacks.Add(sourceCallback); if (null != dumpService) { - callbacks.Add(new DumpServiceEndpointInfoSourceCallback(operationTrackerService)); + callbacks.Add(new OperationTrackerServiceEndpointInfoSourceCallback(operationTrackerService)); } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs index 60148a135de..a65e67ee739 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs @@ -89,7 +89,7 @@ public static IHost CreateHost( services.AddSingleton(); services.ConfigureCollectionRules(); services.ConfigureEgress(); -; + services.AddSingleton(); services.ConfigureStorage(context.Configuration); servicesCallback?.Invoke(services); diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs index 2829ebcc466..dd45f144d59 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs @@ -87,15 +87,8 @@ protected override async Task ExecuteCoreAsync( EgressOperation egressOperation = new EgressOperation( async (outputStream, token) => { - IDisposable operationRegistration = _operationTrackerService.Register(EndpointInfo); - try - { - await TraceUtilities.CaptureTraceAsync(startCompletionSource, EndpointInfo, configuration, duration, outputStream, token); - } - finally - { - operationRegistration.Dispose(); - } + using IDisposable operationRegistration = _operationTrackerService.Register(EndpointInfo); + await TraceUtilities.CaptureTraceAsync(startCompletionSource, EndpointInfo, configuration, duration, outputStream, token); }, egressProvider, fileName, diff --git a/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs b/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs index 7213aedd0aa..0b335bfba07 100644 --- a/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs +++ b/src/Tools/dotnet-monitor/DiagnosticsMonitorCommandHandler.cs @@ -252,7 +252,7 @@ public static IHostBuilder CreateHostBuilder(IConsole console, string[] urls, st services.AddHostedServiceForwarder(); services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.ConfigureOperationStore(); services.ConfigureEgress(); diff --git a/src/Tools/dotnet-monitor/EndpointInfo/ServerEndpointInfoSource.cs b/src/Tools/dotnet-monitor/EndpointInfo/ServerEndpointInfoSource.cs index 5e777b682bc..c98b7ccf172 100644 --- a/src/Tools/dotnet-monitor/EndpointInfo/ServerEndpointInfoSource.cs +++ b/src/Tools/dotnet-monitor/EndpointInfo/ServerEndpointInfoSource.cs @@ -5,6 +5,7 @@ using Microsoft.Diagnostics.Monitoring.WebApi; using Microsoft.Diagnostics.NETCore.Client; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using System; using System.Collections.Generic; @@ -43,6 +44,7 @@ internal sealed class ServerEndpointInfoSource : BackgroundService, IEndpointInf private readonly DiagnosticPortOptions _portOptions; private readonly OperationTrackerService _operationTrackerService; + private readonly ILogger _logger; /// /// Constructs a that aggregates diagnostic endpoints @@ -51,11 +53,13 @@ internal sealed class ServerEndpointInfoSource : BackgroundService, IEndpointInf public ServerEndpointInfoSource( IOptions portOptions, IEnumerable callbacks = null, - OperationTrackerService operationTrackerService = null) + OperationTrackerService operationTrackerService = null, + ILogger logger = null) { _callbacks = callbacks ?? Enumerable.Empty(); _operationTrackerService = operationTrackerService; _portOptions = portOptions.Value; + _logger = logger; BoundedChannelOptions channelOptions = new(PendingRemovalChannelCapacity) { @@ -269,6 +273,7 @@ private async Task CheckEndpointAsync(EndpointInfo info, CancellationToken } catch (OperationCanceledException) when (timeoutSource.IsCancellationRequested) { + _logger?.EndpointTimeout(info.ProcessId.ToString(System.Globalization.CultureInfo.InvariantCulture)); return false; } catch (Exception ex) when (ex is not OperationCanceledException) diff --git a/src/Tools/dotnet-monitor/LoggingEventIds.cs b/src/Tools/dotnet-monitor/LoggingEventIds.cs index 07b3ab34499..05c3d0ccf0b 100644 --- a/src/Tools/dotnet-monitor/LoggingEventIds.cs +++ b/src/Tools/dotnet-monitor/LoggingEventIds.cs @@ -59,5 +59,6 @@ internal static class LoggingEventIds public const int InvalidActionReference = 49; public const int InvalidActionResultReference = 50; public const int ActionSettingsTokenizationNotSupported = 51; + public const int EndpointTimeout = 52; } } diff --git a/src/Tools/dotnet-monitor/LoggingExtensions.cs b/src/Tools/dotnet-monitor/LoggingExtensions.cs index fc914e034fc..0f196f975ee 100644 --- a/src/Tools/dotnet-monitor/LoggingExtensions.cs +++ b/src/Tools/dotnet-monitor/LoggingExtensions.cs @@ -277,6 +277,12 @@ internal static class LoggingExtensions logLevel: LogLevel.Error, formatString: Strings.LogFormatString_ActionSettingsTokenizationNotSupported); + private static readonly Action _endpointTimeout = + LoggerMessage.Define( + eventId: new EventId(LoggingEventIds.EndpointTimeout, "EndpointTimeout"), + logLevel: LogLevel.Warning, + formatString: Strings.LogFormatString_EndpointTimeout); + public static void EgressProviderInvalidOptions(this ILogger logger, string providerName) { _egressProviderInvalidOptions(logger, providerName, null); @@ -500,5 +506,10 @@ public static void ActionSettingsTokenizationNotSupported(this ILogger logger, s { _actionSettingsTokenizationNotSupported(logger, settingsType, null); } + + public static void EndpointTimeout(this ILogger logger, string processId) + { + _endpointTimeout(logger, processId, null); + } } } diff --git a/src/Tools/dotnet-monitor/DumpServiceEndpointInfoSourceCallback.cs b/src/Tools/dotnet-monitor/OperationTrackerServiceEndpointInfoSourceCallback.cs similarity index 84% rename from src/Tools/dotnet-monitor/DumpServiceEndpointInfoSourceCallback.cs rename to src/Tools/dotnet-monitor/OperationTrackerServiceEndpointInfoSourceCallback.cs index a6f0c33a0e6..ff721f935eb 100644 --- a/src/Tools/dotnet-monitor/DumpServiceEndpointInfoSourceCallback.cs +++ b/src/Tools/dotnet-monitor/OperationTrackerServiceEndpointInfoSourceCallback.cs @@ -9,12 +9,11 @@ namespace Microsoft.Diagnostics.Tools.Monitor { - //TODO Rename this - internal sealed class DumpServiceEndpointInfoSourceCallback : IEndpointInfoSourceCallbacks + internal sealed class OperationTrackerServiceEndpointInfoSourceCallback : IEndpointInfoSourceCallbacks { private readonly OperationTrackerService _operationTrackerService; - public DumpServiceEndpointInfoSourceCallback(OperationTrackerService operationTrackerService) + public OperationTrackerServiceEndpointInfoSourceCallback(OperationTrackerService operationTrackerService) { _operationTrackerService = operationTrackerService ?? throw new ArgumentNullException(nameof(operationTrackerService)); } diff --git a/src/Tools/dotnet-monitor/Strings.Designer.cs b/src/Tools/dotnet-monitor/Strings.Designer.cs index ddc3f928a1e..a00b0e70e07 100644 --- a/src/Tools/dotnet-monitor/Strings.Designer.cs +++ b/src/Tools/dotnet-monitor/Strings.Designer.cs @@ -816,6 +816,15 @@ internal static string LogFormatString_EgressProvideUnableToFindPropertyKey { } } + /// + /// Looks up a localized string similar to Unexpected timeout from process {processId}. Process will no longer be monitored.. + /// + internal static string LogFormatString_EndpointTimeout { + get { + return ResourceManager.GetString("LogFormatString_EndpointTimeout", resourceCulture); + } + } + /// /// Looks up a localized string similar to WARNING: Authentication is enabled over insecure http transport. This can pose a security risk and is not intended for production environments.. /// diff --git a/src/Tools/dotnet-monitor/Strings.resx b/src/Tools/dotnet-monitor/Strings.resx index 6942a98984a..251cabaf1d0 100644 --- a/src/Tools/dotnet-monitor/Strings.resx +++ b/src/Tools/dotnet-monitor/Strings.resx @@ -512,6 +512,9 @@ 1. providerType: Type of the provider 2. keyName: Name of the property that could not be found + + Unexpected timeout from process {processId}. Process will no longer be monitored. + WARNING: Authentication is enabled over insecure http transport. This can pose a security risk and is not intended for production environments. Gets the format string that is printed in the 14:InsecureAutheticationConfiguration event.