Add metric highlights #5750

drewnoakes · 2024-09-17T12:13:23Z

Description

For the last month, @adamint and I have been working on feature/dashpages. We're now preparing it for review for merge into main.

A dashpage is a composite metric view, displayed in the metrics page when data for the relevant instruments are available.

Here's an example dashpage, showing multiple instruments in separate tiled charts:

Currently dashpages are defined in a JSON file that ships with the dashboard. We include a default configuration.

Fixes #5289
Fixes #5295
Fixes #5303
Fixes #5298
Fixes #5287
Fixes #5347

Checklist

Is this feature complete?
- Yes. Ready to ship.
- No. Follow-up changes expected.
Are you including unit tests for the changes and scenario tests if relevant?
- 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
- 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
- No
Does the change require an update in our Aspire docs?
- Yes
  - Link to aspire-docs issue:
- No, not initially.

Microsoft Reviewers: Open in CodeFlow

This will make it easier to add a block for rendering a dashpage.

* Populate dashpages from configuration Fixes #5289 This change causes the dashboard to look for a file named `dashpages.json` in the current directory. If found, it's parsed to provide the set of dashpages shown on the metrics page. * Test fixes bunit doesn't support when `OnInitializedAsync` actually yields the thread, so provide bunit tests with a stub implementation of `IDashpagePersistence.GetDashpagesAsync` that returns an empty set synchronously. In future we may extend this type to allow test setup with dashpage definitions.

Fixes #5303

The list of available dashpages in the Metrics UI is dynamic, depending on the selected resource. A dashpage will only be shown if: - metrics are available for at least one chart, and - all required metrics are available - all required resources are available

…into feature/dashpages

# Conflicts: # src/Aspire.Dashboard/Components/Controls/Chart/ChartContainer.cs # src/Aspire.Dashboard/Components/Controls/Chart/PlotlyChart.razor # src/Aspire.Dashboard/Components/Controls/Chart/SingleMetricChartContainer.razor # src/Aspire.Dashboard/Components/Pages/Metrics.razor # src/Aspire.Dashboard/Components/Pages/Metrics.razor.cs # src/Aspire.Dashboard/Components/Pages/Metrics.razor.css # src/Aspire.Dashboard/Resources/Metrics.Designer.cs # src/Aspire.Dashboard/Resources/Metrics.resx # src/Aspire.Dashboard/wwwroot/js/app-metrics.js # src/Shared/StringComparers.cs

* fix merge, clean up * save current dashpage * beautify dashpages * beautify chart filters in dashpages * beautification * fix bugs from merge * improve table, chart appearance on mobile * fix metric table height in dashpage, add dashpage header on mobile * add fixedplacement to filter popover * fix test * continue to work on styling * use unique ids for metric tables * cleanup * Fix broken tests * Code formatting * Avoid horizontal scrollbar in dashpage view * Use integer for div id This is consistent with other code that ensures unique IDs. * Use correct string comparer * Rename and doc property --------- Co-authored-by: Drew Noakes <[email protected]>

… necessary for popovers within a card to display correctly. Sets fixedplacement in chart filter popup to allow breaking out of parent popup

adamint · 2024-09-30T18:30:19Z

Opening the attribute popup inside the filter popup doesn't work:

Fixed.

I expected the filters popup to be closable by clicking anyway on the page outside of the popup. This is how popups work everywhere else in Aspire (and web pages in general). However, only clicking in the panel will close the popup:

I think this due to how cards work, and thus can't be changed without adding a document click listener. Is this a correct assumption @vnbaaij?

adamint · 2024-09-30T18:31:50Z

http.client.request.duration has frozen. Closing and opening the filter popup causes values to update. I'm guessing an error was thrown and the timer that regularly updates data and renders stopped working. Closing and opening the filter causes a render and so data updates once.

I was unable to reproduce this issue. Do you have repro steps?

…ince this component will be initialized on each popup open

adamint · 2024-09-30T18:41:05Z

Add a spacer between view dropdown and duration dropdown:

Done.

Some other state is preserved when switching between views. "Show count" stays enable.

I was able to reproduce this, now show count works properly inside the card.

- apply correct classes - only use aspect-ratio and max-width on single metric view - change the resize observer to watch the chart container itself. It was resizing to 100% because its *parent* takes up 100% of the width

adamint · 2024-09-30T19:35:38Z

I think there has been a regression on standalone metrics pages. The graph extends to fill the entire page. I believe it use to have a max width:

This has now been fixed. The new resize observer was resizing the plot to be larger than the maximum width we had set.

src/Aspire.Dashboard/dashpages.json

JamesNK · 2024-09-30T23:22:58Z

Currently, there is no internal identifier for views. That display name is also unique

Please add one. Even if titles are unique now, this is going in the URL so there is the chance that people will bookmark it. Changing it to not use the title will break URLs.

It doesn't seem like a difficult change and it helps avoid the URL changing in the future.

JamesNK · 2024-09-30T23:23:50Z

I think this due to how cards work, and thus can't be changed without adding a document click listener. Is this a correct assumption @vnbaaij?

What are you using cards for that is special? If cards are a problem, why not use a regular div and apply the same styling?

JamesNK · 2024-09-30T23:25:11Z

I was unable to reproduce this issue. Do you have repro steps?

No. It looks like a thread safety problem: collection being modified at the same time as it is being enumerated

vnbaaij · 2024-10-01T05:43:14Z

I think this due to how cards work, and thus can't be changed without adding a document click listener. Is this a correct assumption @vnbaaij?

What are you using cards for that is special? If cards are a problem, why not use a regular div and apply the same styling?

I generally don't use cards for a popup. For DataGrid resize popup we indeed just use a div with some script to detect clicking outside.

adamint · 2024-10-01T15:41:32Z

Multiple popups can be opened at the same time (would be fixed by fixing the issue above, because clicking outside of the popup would close it). Also, switching between views leaves the popup open (also would be fixed by the above):

This no longer seems like it's occurring

adamint · 2024-10-01T16:34:49Z

Currently, there is no internal identifier for views. That display name is also unique

Please add one. Even if titles are unique now, this is going in the URL so there is the chance that people will bookmark it. Changing it to not use the title will break URLs.

It doesn't seem like a difficult change and it helps avoid the URL changing in the future.

Added

adamint · 2024-10-01T16:40:48Z

I was unable to reproduce this issue. Do you have repro steps?

No. It looks like a thread safety problem: collection being modified at the same time as it is being enumerated

I changed the collection to a ConcurrentBag, so this shouldn't occur again.

# Conflicts: # src/Aspire.Dashboard/Components/Controls/Chart/ChartContainer.cs # src/Aspire.Dashboard/Components/Controls/Chart/ChartFilters.razor # src/Aspire.Dashboard/Components/Pages/Metrics.razor.cs

radical · 2024-10-03T05:40:38Z

/azp run

azure-pipelines · 2024-10-03T05:40:49Z

Azure Pipelines successfully started running 1 pipeline(s).

* Separate dashpage id and display name * sort dashpages consistently * Change dashpages -> highlights * Update dashpage query parameter to highlight, /dashpages -> /highlights * update page directive * change InstrumentViewModel.DataUpdateSubscriptions to prevent enumeration during collection modification, which can happen asynchronously * Remove extra parameters after merge * fix test * update test expectation, remove comparer * change dashpage -> highlight * add warning comment * put highlight id in path --------- Co-authored-by: Adam Ratzman <[email protected]>

drewnoakes and others added 30 commits August 13, 2024 23:00

Add "dashpage" as known dictionary word

2537f0f

Add "plotly" as a spelling word

d4eb709

Refactor markup

816fe70

This will make it easier to add a block for rendering a dashpage.

Show dashpages in the metrics tree view

f582577

Add API doc

e1be2e3

Add definition types for dashpage configuration

4a2840a

Add a collection of dashpages, and a selected dashpage

06c2867

Allow a dashpage to be selected in the tree

2abe4c3

Support rendering dashpages in content area

27fb380

Add TODO and placeholder for DashpageChart component

ce1d94c

Bump log messages to warnings

8f6a9a3

Make chart sizes responsive

28aa1f9

add DashpageCharts component

31d0813

add chartwrapper and tablewrapper components

52b2616

Update default dashpages (#5320)

c5e2cb6

Fixes #5303

isolate chartcontainer logic

4e83cf8

add ellipsis to dashpage instrument title

615bbb7

Merge branch 'feature/dashpages' of https://github.com/dotnet/aspire …

b298951

…into feature/dashpages

show filter in popup

6bf2014

fix merge issues

de2a719

Merge remote-tracking branch 'upstream/main' into feature/dashpages

938a79c

Formatting

1bb988f

Formatting

8d5f414

Remove unused component

cc0ca71

Required parameter updates

f3ace68

Remove commented code

171653f

Re-add area restricted to the dash chart container. This parameter is…

17cd038

… necessary for popovers within a card to display correctly. Sets fixedplacement in chart filter popup to allow breaking out of parent popup

Initialize ShowCounts in chart filter on component initialization - s…

c4adb9d

…ince this component will be initialized on each popup open

Adam Ratzman added 4 commits September 30, 2024 14:43

Add divider between chart type and time

752e794

fix test by adding required member

93a27af

Fix accessibility regression on mobile

d20f93b

Fix chart widths

394262a

- apply correct classes - only use aspect-ratio and max-width on single metric view - change the resize observer to watch the chart container itself. It was resizing to 100% because its *parent* takes up 100% of the width

adityamandaleeka reviewed Sep 30, 2024

View reviewed changes

src/Aspire.Dashboard/dashpages.json Outdated Show resolved Hide resolved

drewnoakes changed the title ~~Add Dashpages~~ Add metric highlights Oct 2, 2024

adamint and others added 3 commits October 2, 2024 13:24

Merge branch 'main' into feature/dashpages

f6c9ba9

# Conflicts: # src/Aspire.Dashboard/Components/Controls/Chart/ChartContainer.cs # src/Aspire.Dashboard/Components/Controls/Chart/ChartFilters.razor # src/Aspire.Dashboard/Components/Pages/Metrics.razor.cs

fix conflict

8fc9729

add import

a2c27cd

build-analysis bot mentioned this pull request Oct 2, 2024

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

3 tasks

adamint mentioned this pull request Oct 14, 2024

[WebToolsE2E][Aspire] After selecting an instrument in the Metrics instrument group, the corresponding instrument list cannot be located. #5596

Closed

joperezr modified the milestones: 9.0, 9.1 Jan 9, 2025

davidfowl removed this from the 9.1 milestone Jan 29, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metric highlights #5750

Add metric highlights #5750

drewnoakes commented Sep 17, 2024 •

edited

Loading

adamint commented Sep 30, 2024

adamint commented Sep 30, 2024

adamint commented Sep 30, 2024

adamint commented Sep 30, 2024

JamesNK commented Sep 30, 2024 •

edited

Loading

JamesNK commented Sep 30, 2024

JamesNK commented Sep 30, 2024

vnbaaij commented Oct 1, 2024

adamint commented Oct 1, 2024 •

edited

Loading

adamint commented Oct 1, 2024

adamint commented Oct 1, 2024 •

edited

Loading

radical commented Oct 3, 2024

azure-pipelines bot commented Oct 3, 2024

Add metric highlights #5750

Are you sure you want to change the base?

Add metric highlights #5750

Conversation

drewnoakes commented Sep 17, 2024 • edited Loading

Description

Checklist

Microsoft Reviewers: Open in CodeFlow

adamint commented Sep 30, 2024

adamint commented Sep 30, 2024

adamint commented Sep 30, 2024

adamint commented Sep 30, 2024

JamesNK commented Sep 30, 2024 • edited Loading

JamesNK commented Sep 30, 2024

JamesNK commented Sep 30, 2024

vnbaaij commented Oct 1, 2024

adamint commented Oct 1, 2024 • edited Loading

adamint commented Oct 1, 2024

adamint commented Oct 1, 2024 • edited Loading

radical commented Oct 3, 2024

azure-pipelines bot commented Oct 3, 2024

drewnoakes commented Sep 17, 2024 •

edited

Loading

JamesNK commented Sep 30, 2024 •

edited

Loading

adamint commented Oct 1, 2024 •

edited

Loading

adamint commented Oct 1, 2024 •

edited

Loading