Repository navigation
Dashboard: preserve resource selection in telemetry pages and metric charts - #21431
David Fowler (davidfowl) wants to merge 3 commits into
Conversation
…harts - Structured logs and traces resolve the route's resource again when its telemetry arrives, so a page opened before the resource sent telemetry stops showing (and marking viewed) every resource's data. - Metrics keeps the open chart's resource in the URL for a multi-resource selection, so changing duration or view no longer drops back to the aggregate instrument list. Fixes #21419 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 21431Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 21431" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟡 Changes recommended
The unresolved-route all-resource read subscription can irrecoverably suppress worker unread counts when worker telemetry arrives first or in the same batch, requiring a fix and regression coverage.
1 open issue
What changed in this PR
Addresses resource-selection loss tracked in #21419 following the dashboard redesign in #21412.
Changes:
- Re-resolves resource-scoped logs and traces pages when telemetry becomes available.
- Preserves the active metrics chart resource during option changes in multi-resource selections.
- Adds regression tests for delayed telemetry and chart navigation.
| File | Description |
|---|---|
| tests/Aspire.Dashboard.Components.Tests/Pages/TracesTests.cs | Tests trace scoping after delayed telemetry. |
| tests/Aspire.Dashboard.Components.Tests/Pages/StructuredLogsTests.cs | Tests log scoping and unrelated unread-error preservation. |
| tests/Aspire.Dashboard.Components.Tests/Pages/MetricsTests.cs | Tests chart resource retention when switching views. |
| src/Aspire.Dashboard/Components/Pages/Traces.razor.cs | Resolves pending route resources and refreshes traces. |
| src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor.cs | Resolves pending route resources and updates log subscriptions. |
| src/Aspire.Dashboard/Components/Pages/Metrics.razor.cs | Retains selected chart resources in navigation URLs. |
Review effort: Balanced
… unresolved Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
## Description Selecting a resource that has not sent telemetry should not unexpectedly move users from Traces or Structured logs to Overview. Keep the current telemetry page and let its existing empty state explain the absence of data, matching Metrics. UX-30 only. This updates selection URL routing in `ResourcesLayout`; it does not change telemetry arrival handling or chart scope in #21431, or redesign Home/Details. Selecting a single parameter still opens Overview. Based on freshly fetched `microsoft/aspire` upstream `main` at `9f0961d0c7173732f45ce88d571c8b53f8059934`. ### User-facing usage While on Traces or Structured logs, select a resource with no telemetry. The resource selection updates without leaving the page. Traces shows its existing "No traces" empty state. Ctrl/Cmd-click to select multiple resources, or select all to clear the explicit selection, without changing telemetry pages. ### Validation - `.\restore.cmd` - passed. - Before the production fix, the following command reproduced two failures: Traces and Structured logs generated `/resources/cache` instead of their telemetry URLs. Four cases passed, including Metrics and the single-parameter exception: ```powershell dotnet test --project tests\Aspire.Dashboard.Components.Tests\Aspire.Dashboard.Components.Tests.csproj --no-launch-profile -- --filter-method '*.Telemetry_ChangingTheSelectionStaysOnTheCurrentTabEvenWithoutTelemetry' --filter-method '*.Telemetry_SelectingOnlyAParameterNavigatesToOverview' --filter-not-trait 'quarantined=true' --filter-not-trait 'outerloop=true' ``` - After the fix, the following command passed all 87 cases, including no-telemetry single/multiple/no selection, parameter-only navigation, and the rendered Traces empty state: ```powershell dotnet test --project tests\Aspire.Dashboard.Components.Tests\Aspire.Dashboard.Components.Tests.csproj --no-launch-profile -- --filter-class '*.ResourcesLayoutTests' --filter-not-trait 'quarantined=true' --filter-not-trait 'outerloop=true' ``` - `git diff --check` - passed. - Real-browser verification against this branch's build passed using a temporary Playwright fixture, with six interactions covering both pages at all three viewport sizes. The fixture seeded one trace and one structured log for `api`, and no telemetry for `cache`. It navigated through the normal tab UI, verified the populated `api` page, then selected `cache` at each target viewport. Assertions checked the scoped URL, resource title, selected tab, explicit empty message, and absence of document horizontal overflow. The fixture and browser were disposed after capture; the capture harness is not part of this PR. ```powershell dotnet test --project tests\Aspire.Dashboard.Playwright.Tests\Aspire.Dashboard.Playwright.Tests.csproj --no-launch-profile -- --filter-method '*.CaptureAffectedStates' --filter-not-trait 'quarantined=true' --filter-not-trait 'outerloop=true' ``` ### Screenshots / Recordings Actual Chromium viewport captures from this branch, using synthetic test resources only. Each screenshot shows `cache` selected after switching from a resource with data. Traces remains at `/traces/resource/cache` with **No traces found**; Structured logs remains at `/structuredlogs/resource/cache` with **No structured logs found**. At **390 x 844** and **768 x 1024**, selection used the responsive resource picker: open the drawer, select `cache`, and verify it closes. At **1440 x 900**, selection used the persistent resource pane. All six images were visually inspected: resource scope and empty-state text remain readable, the empty message stays within the viewport, and document/body width equals viewport width. At mobile width, the existing tab strip truncates trailing labels and exposes its overflow action; the page heading identifies the current telemetry view. | Viewport (pixels) | Traces: selected no-data resource | Structured logs: selected no-data resource | | --- | --- | --- | | Mobile **390 x 844** |  |  | | Tablet **768 x 1024** |  |  | | Desktop **1440 x 900** |  |  | Fixes # (issue) ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [ ] Yes - If yes, did you have an API Review for it? - [ ] Yes - [ ] No - Did you add `<remarks />` and `<code />` elements on your triple slash comments? - [ ] Yes - [ ] No - [x] No - Does the change make any security assumptions or guarantees? - [ ] Yes - If yes, have you done a threat model and had a security review? - [ ] Yes - [ ] No - [x] No Co-authored-by: David Pine <7679720+IEvangelist@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
David Fowler (davidfowl)
added
the
agent:working
Copilot SWE Agent
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Merged Local tests: 🤖 Posted by AspireAgent (AI Agent) |
David Fowler (davidfowl)
removed
the
agent:working
Tests selector13 / 101 PR test projects · 2 PR jobs, from 6 changed files. Selected PR test projects (13 / 101)
Selected PR jobs (2)
How these were chosen — grouped by what changed
🔧 📦 affected project 🧪 🧪 🧪 Job reasons
Selection computed for commit |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Description
Fixes the two remaining selection-loss review findings tracked in #21419 (follow-up to #21412).
1. Structured logs / traces lose the resource scope when telemetry arrives later.
If a resource's logs or traces page is opened before that resource has sent telemetry, the route resolves to "(All)" and the view model keeps a
nullResourceKey. When telemetry later arrives,GetSelectionTelemetryKeysreturnsnull("use the page's resource key"), so the page lists every resource's data under the selected resource. For structured logs, the read subscription with anullkey also marks unrelated resources' errors as viewed.Fix: when new telemetry resources are reported,
StructuredLogsandTracesre-resolve the route's resource if the page is still on "(All)". This sets the selected resource, resource key/keys and subscription, then refreshes the grid.Metricsalready does the same inOnResourcesUpdatedAsync. Until the route resolves, structured logs uses a non-read subscription and doesn't mark errors as viewed. Other resources' errors that arrive first, or in the same batch, therefore keep their unread counts.2. Metrics chart loses its resource in a multi-resource selection.
With
apiandworkerselected andapi's instrument chart open, changing duration or graph/table view rebuilt the URL without/resource/api, so the page dropped back to the aggregate instrument list.Fix: for multi-selection,
GetUrlFromSerializableViewModelkeeps the resource name when an instrument chart is open and the resource is part of the current selection. The aggregate list and stale names still drop it.Regression tests (all three fail on
mainand pass with this change):StructuredLogsTests.ResourcesLayout_SelectedResourceSendsTelemetryAfterPageOpened_ShowsOnlyThatResource: onlyapilogs are shown.worker's errors stay unviewed, both the one sent beforeapi's first telemetry and the one sent after.TracesTests.ResourcesLayout_SelectedResourceSendsTelemetryAfterPageOpened_ShowsOnlyThatResource: onlyapitraces are shown.MetricsTests.ResourcesLayout_MultiSelectionInstrumentChart_ChangingViewKeepsActiveResource: the path stays/metrics/resource/api, with the instrument, view and selection kept.Before the fix, the Metrics test navigated to
/metricsinstead of/metrics/resource/api. The logs and traces tests timed out waiting for the selected resource to becomeapi(it stayed "(All)"). The fullAspire.Dashboard.Components.Testssuite passes locally (1164/1164).No visual/layout change, so there are no screenshots.
Fixes #21419
Checklist
🤖 Posted by AspireAgent (AI Agent)