Flow dashboard logs through the app host's logger #3523
Conversation
dotnet-issue-labeler
bot
added
the
area-codeflow
|
Don't forgot to add tests. |
davidfowl
added
the
area-app-model
| @@ -199,7 +199,7 @@ private async Task WatchDashboardLogsAsync() | |||
| try | |||
| { | |||
| // The dashboard resource isn't immediately available so watch for it. | |||
| // Wait 1 min for dashboard to startup and be reported before giving up. | |||
| // Wait for dashboard to startup and be reported before giving up. | |||
| using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30)); | |||
There was a problem hiding this comment.
Get this timeout from options? 30 seconds isn't much time if you are trying to debug something.
There was a problem hiding this comment.
Alternatively: increase for DEBUG builds, or detect if a debugger is attached (or both).
There was a problem hiding this comment.
I made it a setting and also removed the timeout when debugging.
|
This needs to show no logs by default. I'll make that change so that it's config enabled. |
dotnet-policy-service
bot
added
the
needs-author-action
dotnet-policy-service
bot
removed
the
needs-author-action
davidfowl
force-pushed
the
davidfowl/dashboard-logs
branch
from
127630b
to
46e6918
Compare
- Make the timeout waiting for dashboard logs configurable - Disable all dashboard logging by default - Allow config override for default logging categories - Added tests
davidfowl
force-pushed
the
davidfowl/dashboard-logs
branch
from
46e6918
to
306bc7f
Compare
|
/backport to release/8.0 |
|
Started backporting to release/8.0: https://github.com/dotnet/aspire/actions/runs/8642812237 |
| @@ -76,11 +76,15 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options) | |||
|
|
|||
| _innerBuilder.Logging.AddFilter("Microsoft.Hosting.Lifetime", LogLevel.Warning); | |||
| _innerBuilder.Logging.AddFilter("Microsoft.AspNetCore.Server.Kestrel", LogLevel.Error); | |||
| _innerBuilder.Logging.AddFilter("Aspire.Hosting.Dashboard", LogLevel.None); | |||
There was a problem hiding this comment.
If the Dashboard crashes, the user doesn't get any indication of what happened, right? They just see a hang for a minute and then hopefully an error??
We should be at least logging errors by default.
There was a problem hiding this comment.
We can’t. I tried the playground samples and lots of other errors showed up.
I think it could be done if we filtered to the startup category by default, but it can’t be all errors.
Fixes #2911
This pull request primarily focuses on enhancing the
DashboardLifecycleHookclass in thesrc/Aspire.Hosting/Dashboard/DashboardLifecycleHook.csfile. The changes include the addition of new dependencies, the implementation of theIAsyncDisposableinterface, and the introduction of a method to watch and handle dashboard logs. Additionally, there are modifications to theDistributedApplicationBuilderclass and updates to the unit tests to accommodate these changes.Key changes include:
Addition of new dependencies:
src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs: New dependencies were added to theDashboardLifecycleHookclass, includingResourceNotificationService,ResourceLoggerService, andILoggerFactory. This change allows the class to handle resource notifications and logs, and to create loggers.Implementation of the
IAsyncDisposableinterface:src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs: TheDashboardLifecycleHookclass now implements theIAsyncDisposableinterface, which provides a mechanism for releasing unmanaged resources asynchronously. A newDisposeAsyncmethod was added to handle the disposal of resources.Introduction of a method to watch and handle dashboard logs:
src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs: A new method,WatchDashboardLogsAsync, was introduced to watch and handle dashboard logs. This method includes logic to parse and log JSON formatted log lines.Updates to the
DistributedApplicationBuilderclass:src/Aspire.Hosting/DistributedApplicationBuilder.cs: An additional filter was added to theDistributedApplicationBuilderclass to handle logging for theAspire.Hosting.Dashboard.Updates to unit tests:
tests/Aspire.Hosting.Tests/Dashboard/DashboardResourceTests.cs: Unit tests were updated to accommodate the changes in theDashboardLifecycleHookclass and theDistributedApplicationBuilderclass. This includes the addition of a new test,DashboardLifecycleHookWatchesWarningLogs, to verify the new log watching functionality. [1] [2] [3]Microsoft Reviewers: Open in CodeFlow