[dashboard-oop] Launch dashboard as exe #1717
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change causes the dashboard to be run as an orchestrated project, just like any other Aspire resource. To achieve this, we: - Break the project dependency from AppHost upon Dashboard. - Until we have support for dotnet tool resources, having the sample projects depend upon the dashboard directly, and register them with the DAB. - Copy `TimestampParser.DisplayFormat` into `Aspire.Hosting.Dashboard.ConsoleLogsConfigurationExtensions` where it was used, and add comments to keep them in sync. - Have the dashboard wire up its own web application. - Remove `DashboardWebApplicationHost`. - Remove dashboard-specific code from `DistributedApplicationBuilder`. - Update URLs to resources defined in the dashboard. They no longer need (or work with) the `_content/Aspire.Dashboard/` prefix. - Remove workarounds/configuration to support static web assets, as the new structure supports this automatically. - Link `KnownProperties` and `KnownResourceTypes` into the hosting project, so both projects can share the definition without needing an assembly reference. - Change the MSBuild SDK for the dashboard from Razor Class Library to Web project. - Remove explicit framework reference. - Remove redundant using directives (the new SDK has different implicit usings). - Make the dashboard an executable. - Update the CSS bundle file name (from `Aspire.Dashboard.bundle.scp.css` to `Aspire.Dashboard.styles.css`) This work is incomplete for a few reasons: 1. Aspire users won't be able to create a project reference to the dashboard. That only works in this solution. We need support for resources defined via `dotnet tool` packages before we can fix that. 2. We don't launch a browser to the dashboard any more. You have to find the URL in the logs and launch it. We will need to address those issues before the changes here can be merged. --- To test this locally, run any of the sample applications. To find the dashboard's URL, navigate to `%TEMP%` and find the relevant log files. For example, `%TEMP%\aspire.oweozx1d.bmh\dashboard-5buv5so_out_eedbad51-0294-4ff8-9add-001f27499206` (your IDs will be different). The first log message should read _Now listening on: https://localhost:49337_ or similar. Open that URL in a browser.
Also adds a readme explaining dashboard configuration, and sets some metadata on the package for the icon, readme, etc.
mitchdenny
added
area-dashboard
area-app-model
davidfowl
reviewed
Jan 19, 2024
davidfowl
reviewed
Jan 22, 2024
davidfowl
reviewed
Jan 22, 2024
davidfowl
reviewed
Jan 22, 2024
davidfowl
reviewed
Jan 22, 2024
davidfowl
reviewed
Jan 22, 2024
| @@ -24,7 +24,7 @@ namespace Aspire.Dashboard.Model; | |||
| internal sealed class DashboardClient : IDashboardClient | |||
| { | |||
| private const string DashboardServiceUrlVariableName = "DOTNET_DASHBOARD_GRPC_ENDPOINT_URL"; | |||
| private const string DashboardServiceUrlDefaultValue = "http://localhost:18999"; | |||
| private const string DashboardServiceUrlDefaultValue = "https://localhost:18999"; | |||
There was a problem hiding this comment.
Actually, the client shouldn't have a fallback @drewnoakes, it should be specified as input #1554
JamesNK
reviewed
Jan 22, 2024
JamesNK
reviewed
Jan 22, 2024
JamesNK
reviewed
Jan 22, 2024
Co-authored-by: Drew Noakes <[email protected]>
drewnoakes
reviewed
Jan 22, 2024
| @@ -73,6 +79,11 @@ public async Task RunApplicationAsync(CancellationToken cancellationToken = defa | |||
| AspireEventSource.Instance.DcpModelCreationStart(); | |||
| try | |||
| { | |||
| if (!_model.Resources.Any(r => r.Name == DashboardReservedResourceName)) | |||
There was a problem hiding this comment.
Suggested change
| if (!_model.Resources.Any(r => r.Name == DashboardReservedResourceName)) | |
| if (!_model.Resources.Any(r => StringComparers.ResourceNames.Equals(r.Name, DashboardReservedResourceName))) |
drewnoakes
reviewed
Jan 22, 2024
| @@ -85,7 +96,60 @@ public async Task RunApplicationAsync(CancellationToken cancellationToken = defa | |||
| { | |||
| AspireEventSource.Instance.DcpModelCreationStop(); | |||
| } | |||
| } | |||
|
|
|||
| private const string DashboardReservedResourceName = "aspire-dashboard"; | |||
There was a problem hiding this comment.
Consider creating:
internal static class KnownResourceNames
{
public const string DashboardReservedResourceName = "aspire-dashboard";
}We have some Known* classes already that follow this pattern and allow reuse across projects.
drewnoakes
reviewed
Jan 22, 2024
drewnoakes
changed the title
drewnoakes
changed the title
drewnoakes
reviewed
Jan 23, 2024
| { | ||
| if (Environment.GetEnvironmentVariable("DOTNET_ASPIRE_SHOW_DASHBOARD_RESOURCES") is { } showDashboardResourcesValue) | ||
| { | ||
| return bool.TryParse(showDashboardResourcesValue, out var parsedShowDashboardResourcesValue) && !parsedShowDashboardResourcesValue; |
There was a problem hiding this comment.
Some VS env vars use 1 and 0. Is there precedent on only accepting valid bool strings?
drewnoakes
reviewed
Jan 23, 2024
| @@ -31,6 +32,8 @@ internal sealed class DcpOptions | |||
| /// </example> | |||
| public string? ExtensionsPath { get; set; } | |||
|
|
|||
| public string? DashboardPath { get; set; } | |||
There was a problem hiding this comment.
Would be great to see API doc on this member, similar to that of the prior two properties, with an example value.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
I pushed a fix for a unit test. The most recent run failed because the tests in Kicking off another build in case it's transient. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Yep, it was transient. |
drewnoakes
added a commit
that referenced
this pull request
Jan 24, 2024
* Add PR validation for feature branches. * Run dashboard out-of-process (#1667) * Run dashboard out-of-process This change causes the dashboard to be run as an orchestrated project, just like any other Aspire resource. To achieve this, we: - Break the project dependency from AppHost upon Dashboard. - Until we have support for dotnet tool resources, having the sample projects depend upon the dashboard directly, and register them with the DAB. - Copy `TimestampParser.DisplayFormat` into `Aspire.Hosting.Dashboard.ConsoleLogsConfigurationExtensions` where it was used, and add comments to keep them in sync. - Have the dashboard wire up its own web application. - Remove `DashboardWebApplicationHost`. - Remove dashboard-specific code from `DistributedApplicationBuilder`. - Update URLs to resources defined in the dashboard. They no longer need (or work with) the `_content/Aspire.Dashboard/` prefix. - Remove workarounds/configuration to support static web assets, as the new structure supports this automatically. - Link `KnownProperties` and `KnownResourceTypes` into the hosting project, so both projects can share the definition without needing an assembly reference. - Change the MSBuild SDK for the dashboard from Razor Class Library to Web project. - Remove explicit framework reference. - Remove redundant using directives (the new SDK has different implicit usings). - Make the dashboard an executable. - Update the CSS bundle file name (from `Aspire.Dashboard.bundle.scp.css` to `Aspire.Dashboard.styles.css`) This work is incomplete for a few reasons: 1. Aspire users won't be able to create a project reference to the dashboard. That only works in this solution. We need support for resources defined via `dotnet tool` packages before we can fix that. 2. We don't launch a browser to the dashboard any more. You have to find the URL in the logs and launch it. We will need to address those issues before the changes here can be merged. --- To test this locally, run any of the sample applications. To find the dashboard's URL, navigate to `%TEMP%` and find the relevant log files. For example, `%TEMP%\aspire.oweozx1d.bmh\dashboard-5buv5so_out_eedbad51-0294-4ff8-9add-001f27499206` (your IDs will be different). The first log message should read _Now listening on: https://localhost:49337_ or similar. Open that URL in a browser. * Make dashboard packable as a dotnet tool Also adds a readme explaining dashboard configuration, and sets some metadata on the package for the icon, readme, etc. * [feature/dashboard-oop] Add initial workload targets and manifest entry for dashboard (#1705) * Add initial workload targets and manifest entry for dashboard * Do a better job handling trailing slashes for workload tool paths * Update based on PR feedback * Publishing as a tool by default breaks the workload build * Remove extra parenthesis from import condition * Updated workload package name * [dashboard-oop] Dynamic dashboard port (#1750) * Expose resource service URIs on DashboardServiceHost The resource service will no longer bind to a default, hardcoded port. This encourages port collisions. Instead, when no URL is set in the environment, a port is chosen dynamically. When the app model launches the dashboard, it must pass the URIs exposed via `DashboardServiceHost.GetUrisAsync` via the `DOTNET_DASHBOARD_GRPC_ENDPOINT_URL` environment variable to the spawned instance. That work is happening concurrently, and will use this API is a subsequent change. * Resource service listens on a single URI which must be loopback * Require parsed URI to be absolute * [feature/dashboard-oop] Ensure the dashboard builds as a package for the workload (#1767) * Ensure the dashboard actually builds as a package for the workload * Publish then pack the dashboard as part of workload build * [dashboard-oop] Launch dashboard as exe (#1717) * Run dashboard out-of-process This change causes the dashboard to be run as an orchestrated project, just like any other Aspire resource. To achieve this, we: - Break the project dependency from AppHost upon Dashboard. - Until we have support for dotnet tool resources, having the sample projects depend upon the dashboard directly, and register them with the DAB. - Copy `TimestampParser.DisplayFormat` into `Aspire.Hosting.Dashboard.ConsoleLogsConfigurationExtensions` where it was used, and add comments to keep them in sync. - Have the dashboard wire up its own web application. - Remove `DashboardWebApplicationHost`. - Remove dashboard-specific code from `DistributedApplicationBuilder`. - Update URLs to resources defined in the dashboard. They no longer need (or work with) the `_content/Aspire.Dashboard/` prefix. - Remove workarounds/configuration to support static web assets, as the new structure supports this automatically. - Link `KnownProperties` and `KnownResourceTypes` into the hosting project, so both projects can share the definition without needing an assembly reference. - Change the MSBuild SDK for the dashboard from Razor Class Library to Web project. - Remove explicit framework reference. - Remove redundant using directives (the new SDK has different implicit usings). - Make the dashboard an executable. - Update the CSS bundle file name (from `Aspire.Dashboard.bundle.scp.css` to `Aspire.Dashboard.styles.css`) This work is incomplete for a few reasons: 1. Aspire users won't be able to create a project reference to the dashboard. That only works in this solution. We need support for resources defined via `dotnet tool` packages before we can fix that. 2. We don't launch a browser to the dashboard any more. You have to find the URL in the logs and launch it. We will need to address those issues before the changes here can be merged. --- To test this locally, run any of the sample applications. To find the dashboard's URL, navigate to `%TEMP%` and find the relevant log files. For example, `%TEMP%\aspire.oweozx1d.bmh\dashboard-5buv5so_out_eedbad51-0294-4ff8-9add-001f27499206` (your IDs will be different). The first log message should read _Now listening on: https://localhost:49337_ or similar. Open that URL in a browser. * Make dashboard packable as a dotnet tool Also adds a readme explaining dashboard configuration, and sets some metadata on the package for the icon, readme, etc. * Launching dashboard via DCP executable. * Work in progress checkin. * Remove unnecessary environment sets. * Move dashboard path resolution to dcp options. * PR feedback. * Read ASPNETCORE_ENVIRONMENT ... from environment. * Update src/Aspire.Hosting/Dashboard/DashboardServiceHost.cs Co-authored-by: Drew Noakes <[email protected]> * Update src/Aspire.Hosting/Dashboard/DashboardServiceHost.cs Co-authored-by: Drew Noakes <[email protected]> * Exclude dashboard resources from being watched. * Filtering dashboard resources from view. * Revert dashboard servicehost to parent branch state. * Changes after integrating feature branch. * Fix unit tests --------- Co-authored-by: Drew Noakes <[email protected]> * Rename and format (#1776) * Formatting Addresses diagnostics shown by the IDE. If we don't want these changes, we should reconfigure or disable those diagnostics. * Rename method and log if delays are observed * Replace `Trace.WriteLine` with `_logger.LogWarning` * Start dashboard first for playground scenario (#1775) * Add KnownResourceNames * Ensure dashboard is started first When F5'ing one of the playground solutions in the dotnet/aspire repo, the dashboard is added as the last resource. This means it starts after all other resources, which is not what we want. This change ensures it is always started first. * Wait for dashboard to be online before starting other resources. (#1778) * Add wait for dashboard start. * WIP to merge with parent branch. * Wait for dashboard if it is a Aspire resource. * Add dashboard setup across playground. * Update src/Aspire.Hosting/Dcp/ApplicationExecutor.cs Co-authored-by: Drew Noakes <[email protected]> * Add short delay in polling loop. * Fix up get request. --------- Co-authored-by: Drew Noakes <[email protected]> * Add API docs to DcpOptions (#1777) * [feature/dashboard-oop] Publish dashboard for specific RIDs before packing (#1771) * Publish dashboard for specific RIDs before packing * Add comments explaining the need to keep runtime lists in sync * Remove unused item for icon * Ensure it is clear the runtimes list matches the supported runtimes from DCP * Allow parallel publishing * Fix build of OrleansAppHost (#1794) * [dashboard-oop] Update dashboard attribute config for playground projects (#1799) * Update attribute handling for playground projects * Update comments on default path to dashboard * [dashboard-oop] Support starting dashboard as DLL or executable (#1798) * Support starting dashboard as DLL or executable The workload ships an executable, but `ApplicationExecutor` was written to run a DLL via `dotnet <dll>`. This change checks whether the dashboard is a DLL or not, and configures the executable spec accordingly. * Fix compile error Co-authored-by: David Negstad <[email protected]> --------- Co-authored-by: David Negstad <[email protected]> * Fix dashboard startup ordering. (#1804) * Fix dashboard startup ordering. * Workaround pathing bug. * Fix up AspireDashboardDir (keep it in Directory.Build.props in repo root). * [dashboard-oop] PR feedback (#1805) * Rename environment variable This hasn't shipped anywhere yet, so it's fine to rename it now. * Remove unused DashboardExtensions * Update exception message * DashboardServiceHost logs when skipping hosting resource service --------- Co-authored-by: Jose Perez Rodriguez <[email protected]> Co-authored-by: Mitch Denny <[email protected]> Co-authored-by: David Negstad <[email protected]> Co-authored-by: James Newton-King <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Draft, do not merge. My intention of this PR is to bring together a few different moving parts. Currently there are some things hardcoded that shouldn't be and some rough edges that we don't want on the feature branch.
Microsoft Reviewers: Open in CodeFlow