Add metric highlights

[drewnoakes]

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?
  • [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
• Does the change require an update in our Aspire docs?
  • [ ] Yes
    • Link to aspire-docs issue:
  • [x] No, not initially.

Microsoft Reviewers: Open in CodeFlow

[leslierichardson95]

Can I get a build of this or at least a demo vid before I give it a formal blessing???

@adamint and @drewnoakes if possible, can either of you make it to a UX board review meeting and/or sync with David Culbertson this week? I wanted to do one earlier but one of us was always OOF in the past few weeks. :)

[JamesNK]

I haven't looked at any code yet. This is from testing.

Add a spacer between view dropdown and duration dropdown: I think other toolbars use FluentDividers.

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:

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):

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

I don't know what happened, but while testing one of the charts stopped updating: 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.

It might be related to this error in the console:

fail: Aspire.Hosting.Dashboard.Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost[111]
      Unhandled exception in circuit 'hYYAo8UMq3S58Ff2g215fTIFV_By0GPW3yIyuAYCwjo'.
      System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
         at System.Collections.Generic.List`1.Enumerator.MoveNext()
         at Aspire.Dashboard.Model.InstrumentViewModel.UpdateDataAsync(OtlpInstrumentSummary instrument, List`1 matchedDimensions) in C:\Development\Source\aspire\src\Aspire.Dashboard\Model\InstrumentViewModel.cs:line 23
         at Aspire.Dashboard.Components.ChartContainer.UpdateInstrumentDataAsync(OtlpInstrumentData instrument) in C:\Development\Source\aspire\src\Aspire.Dashboard\Components\Controls\Chart\ChartContainer.cs:line 113
         at Aspire.Dashboard.Components.ChartContainer.UpdateDataAsync() in C:\Development\Source\aspire\src\Aspire.Dashboard\Components\Controls\Chart\ChartContainer.cs:line 93
         at Aspire.Dashboard.Components.ChartContainer.DisposeAsync() in C:\Development\Source\aspire\src\Aspire.Dashboard\Components\Controls\Chart\ChartContainer.cs:line 70
         at Microsoft.AspNetCore.Components.RenderTree.Renderer.<>c__DisplayClass85_0.<<Dispose>g__HandleAsyncExceptions|1>d.MoveNext()

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

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:

Edit: Yes, there was a max width so the graph and filters could be displayed side by side on hi-res displays.

I see the view title is in the query string. Is there some kind of internal identifer for views? It should be what is added to the query string. If there isn't an internal identifer for views then there probably should be one. For example, a view has a Name and a DisplayName. That way people can give views a unique identifer without worrying about conflicts, e.g. two people adding views titled "Database":

This button for a chart has filters and options. I don't think showing the filter button (highlighted in green) is right, specially since we're using it for another reason on the same screen. Since the popup is a mix of graph options, I think it should be a cog. A cog makes it clear that you're configuring the graph options.

The graph panels are hard up against the bottom of the page. The spacing between rows of panels should be added to the bottom:

Clicking on dashpages menu option doesn't show anything, just the default "Select an instrument". That doesn't make sense in this situation. It should show a list of pages like clicking on a meter does for instruments:

An instrument page shows the instrument title and description. A dashpage doesn't, and you can only see what page you're on by looking at the tree. Should the dashpage title (and a description if there is one) be displayed? That would make it more consistent with other pages in the metrics area. And if we're going for a Grafana like feel, Grafana also displays the title at the top of a dashboard. I'm not sure if this would feel good or not, but I'd at least like to try it out and see.

How are dashpages sorted in the tree? Meters and instruments are alphabetical. Should dashpages be alphabetical?

[adamint]

How are dashpages sorted in the tree? Meters and instruments are alphabetical. Should dashpages be alphabetical?

We are displaying dashpages in the order that they are defined in dashpages.json. We could do either

[adamint]

An instrument page shows the instrument title and description. A dashpage doesn't, and you can only see what page you're on by looking at the tree. Should the dashpage title (and a description if there is one) be displayed? That would make it more consistent with other pages in the metrics area. And if we're going for a Grafana like feel, Grafana also displays the title at the top of a dashboard.

Previously, the title was just shown on mobile, now it's shown on desktop too. How do you like this?

[adamint]

Clicking on dashpages menu option doesn't show anything, just the default "Select an instrument". That doesn't make sense in this situation. It should show a list of pages like clicking on a meter does for instruments:

I reused our existing data grid for this!

[adamint]

The graph panels are hard up against the bottom of the page. The spacing between rows of panels should be added to the bottom:

Fixed.

This button for a chart has filters and options. I don't think showing the filter button (highlighted in green) is right, specially since we're using it for another reason on the same screen. Since the popup is a mix of graph options, I think it should be a cog. A cog makes it clear that you're configuring the graph options.

Changed to a cog

[adamint]

I see the view title is in the query string. Is there some kind of internal identifer for views? It should be what is added to the query string. If there isn't an internal identifer for views then there probably should be one. For example, a view has a Name and a DisplayName. That way people can give views a unique identifer without worrying about conflicts, e.g. two people adding views titled "Database":

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

[adamint]

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]

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?

[adamint]

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.

[adamint]

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.

[JamesNK]

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]

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]

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]

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]

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]

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]

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.

[radical]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).