From e907f3053774438e050f79427d083c80850e9cfb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 12 Apr 2024 11:22:22 -0700 Subject: [PATCH] Fix dashboard log watching- Deleted the timeout (#3656) Fixes #3612 Co-authored-by: David Fowler --- .../Dashboard/DashboardLifecycleHook.cs | 82 +++++++++---------- .../Dashboard/DashboardOptions.cs | 8 -- 2 files changed, 37 insertions(+), 53 deletions(-) diff --git a/src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs b/src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs index 5e01aab8d7..41bddc4148 100644 --- a/src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs +++ b/src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Concurrent; using System.Diagnostics; using System.Text.Json; using System.Text.Json.Nodes; @@ -45,7 +46,7 @@ public Task BeforeStartAsync(DistributedApplicationModel model, CancellationToke AddDashboardResource(model); } - _dashboardLogsTask = WatchDashboardLogsAsync(); + _dashboardLogsTask = WatchDashboardLogsAsync(_shutdownCts.Token); return Task.CompletedTask; } @@ -205,58 +206,49 @@ private void ConfigureAspireDashboardResource(IResource dashboardResource) })); } - private async Task WatchDashboardLogsAsync() + private async Task WatchDashboardLogsAsync(CancellationToken cancellationToken) { - async Task GetDashboardResourceIdentifier() - { - string? dashboardResourceId = null; - - try - { - var timeout = Debugger.IsAttached ? Timeout.InfiniteTimeSpan : dashboardOptions.Value.DashboardStartupTimeout; + var loggerCache = new ConcurrentDictionary(StringComparer.Ordinal); + var defaultDashboardLogger = loggerFactory.CreateLogger("Aspire.Hosting.Dashboard"); - // The dashboard resource isn't immediately available so watch for it. - // Wait for dashboard to startup and be reported before giving up. - using var cts = new CancellationTokenSource(timeout); - using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cts.Token, _shutdownCts.Token); + var dashboardResourceTasks = new Dictionary(); - await foreach (var notification in resourceNotificationService.WatchAsync(linkedCts.Token)) - { - // We want to capture the replica of the dashboard resource. This will allow us to get the logs for the dashboard. - // When the resource id is not the same as the resource name, it's the replica. - if (StringComparers.ResourceName.Equals(notification.Resource.Name, KnownResourceNames.AspireDashboard) && - notification.ResourceId != notification.Resource.Name) - { - dashboardResourceId = notification.ResourceId; - break; - } - } - } - catch (OperationCanceledException) + try + { + await foreach (var notification in resourceNotificationService.WatchAsync(cancellationToken)) { - if (distributedApplicationLogger.IsEnabled(LogLevel.Debug)) + // Track all dashboard resources and start watching their logs. + // TODO: In the future when resources can restart, we should handle purging the taskCache. + if (StringComparers.ResourceName.Equals(notification.Resource.Name, KnownResourceNames.AspireDashboard) && !dashboardResourceTasks.ContainsKey(notification.ResourceId)) { - distributedApplicationLogger.LogDebug("Timed out waiting for dashboard resource to start."); + dashboardResourceTasks[notification.ResourceId] = WatchResourceLogsAsync(notification.ResourceId, loggerCache, defaultDashboardLogger, resourceLoggerService, loggerFactory, cancellationToken); } } - - return dashboardResourceId; } - - if (await GetDashboardResourceIdentifier().ConfigureAwait(false) is not string dashboardResourceId) + catch (OperationCanceledException) { - // The only way this can happen is if DCP is unhealthy, so log a warning. - distributedApplicationLogger.LogWarning("Failed to get the dashboard logs."); - return; + // Expected when the application is shutting down. + } + catch (Exception ex) + { + defaultDashboardLogger.LogError(ex, "Error reading dashboard logs."); } - var loggerCache = new Dictionary(StringComparer.Ordinal); - var defaultDashboardLogger = loggerFactory.CreateLogger("Aspire.Hosting.Dashboard"); + // The watch task should already be logging exceptions, so we don't need to log them here. + await Task.WhenAll(dashboardResourceTasks.Values).ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing); + } + private static async Task WatchResourceLogsAsync(string dashboardResourceId, + ConcurrentDictionary loggerCache, + ILogger defaultDashboardLogger, + ResourceLoggerService resourceLoggerService, + ILoggerFactory loggerFactory, + CancellationToken cancellationToken) + { try { // We turned on the JSON formatter for the logger, so we can log the log lines as JSON. - await foreach (var batch in resourceLoggerService.WatchAsync(dashboardResourceId).WithCancellation(_shutdownCts.Token).ConfigureAwait(false)) + await foreach (var batch in resourceLoggerService.WatchAsync(dashboardResourceId).WithCancellation(cancellationToken).ConfigureAwait(false)) { foreach (var logLine in batch) { @@ -302,21 +294,21 @@ private async Task WatchDashboardLogsAsync() } } - private static void LogMessage(ILoggerFactory loggerFactory, Dictionary loggerCache, DashboardLogMessage logMessage) + private static void LogMessage(ILoggerFactory loggerFactory, ConcurrentDictionary loggerCache, DashboardLogMessage logMessage) { - if (!loggerCache.TryGetValue(logMessage.Category, out var logger)) + var logger = loggerCache.GetOrAdd(logMessage.Category, static (string category, ILoggerFactory loggerFactory) => { // Looks strange to see Aspire.Hosting.Dashboard.Aspire.Dashboard.Category, // so trim the prefix and append Aspire.Hosting.Why is this important? // Well there are logs emitting from categories that don't start with Aspire.Dashboard so we want to prefix all logs so that they can be controlled by config. - var categoryTrimmed = logMessage.Category.StartsWith("Aspire.Dashboard.") ? - logMessage.Category["Aspire.Dashboard.".Length..] : logMessage.Category; + var categoryTrimmed = category.StartsWith("Aspire.Dashboard.") ? + category["Aspire.Dashboard.".Length..] : category; - loggerCache[logMessage.Category] = logger = loggerFactory.CreateLogger($"Aspire.Hosting.Dashboard.{categoryTrimmed}"); - } + return loggerFactory.CreateLogger($"Aspire.Hosting.Dashboard.{categoryTrimmed}"); + }, + loggerFactory); // TODO: We should log the state as well. - logger.Log(logMessage.LogLevel, logMessage.EventId, logMessage.Message, null, (s, _) => s); } } diff --git a/src/Aspire.Hosting/Dashboard/DashboardOptions.cs b/src/Aspire.Hosting/Dashboard/DashboardOptions.cs index 9d31c87072..12067c11fc 100644 --- a/src/Aspire.Hosting/Dashboard/DashboardOptions.cs +++ b/src/Aspire.Hosting/Dashboard/DashboardOptions.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Globalization; using Aspire.Hosting.Dcp; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Options; @@ -16,7 +15,6 @@ internal class DashboardOptions public string? OtlpEndpointUrl { get; set; } public string? OtlpApiKey { get; set; } public string AspNetCoreEnvironment { get; set; } = "Production"; - public TimeSpan DashboardStartupTimeout { get; set; } = TimeSpan.FromSeconds(30); } internal class ConfigureDefaultDashboardOptions(IConfiguration configuration, IOptions dcpOptions) : IConfigureOptions @@ -31,12 +29,6 @@ public void Configure(DashboardOptions options) options.OtlpApiKey = configuration["AppHost:OtlpApiKey"]; options.AspNetCoreEnvironment = configuration["ASPNETCORE_ENVIRONMENT"] ?? "Production"; - - if (configuration["AppHost:DashboardStartupTimeout"] is { Length: > 0 } dashboardStartupTimeoutValue && - !string.IsNullOrEmpty(dashboardStartupTimeoutValue)) - { - options.DashboardStartupTimeout = TimeSpan.FromSeconds(int.Parse(dashboardStartupTimeoutValue, CultureInfo.InvariantCulture)); - } } }