Fix various bugs in dashboard metrics (#5522)

JamesNK · web-flow · commit f73983722301 · 2024-09-03T11:46:21.000-04:00
* Fix various bugs in dashboard metrics

* Comment

* Add tests
diff --git a/src/Aspire.Dashboard/Components/Controls/Chart/MetricTable.razor b/src/Aspire.Dashboard/Components/Controls/Chart/MetricTable.razor
@@ -21,8 +21,6 @@
 <div id="metric-table-container" style="height: 40vh; overflow-y: auto; width:1200px;">
     @* ItemKey is to preserve row focus by associating rows with their associated time *@
     <FluentDataGrid
-        ResizeLabel="@AspireFluentDataGridHeaderCell.GetResizeLabel(ControlsStringsLoc)"
-        ResizeType="DataGridResizeType.Discrete"
         Items="@_metricsView"
         ItemSize="46"
         Virtualize="true"
@@ -37,7 +35,7 @@
             {
                 foreach (var (percentile, underlineColor) in percentileColumns)
                 {
-                    <AspireTemplateColumn Title="@((_metricsView.FirstOrDefault() as HistogramMetricView)?.Percentiles[percentile].Name ?? (_instrument is not null ? $"P{percentile} {InstrumentUnitResolver.ResolveDisplayedUnit(_instrument, titleCase: true, pluralize: true)}" : $"P{percentile}"))">
+                    <TemplateColumn Title="@((_metricsView.FirstOrDefault() as HistogramMetricView)?.Percentiles[percentile].Name ?? (_instrument is not null ? $"P{percentile} {InstrumentUnitResolver.ResolveDisplayedUnit(_instrument, titleCase: true, pluralize: true)}" : $"P{percentile}"))">
                         @if (context is HistogramMetricView histogramMetric)
                         {
                             var percentileData = histogramMetric.Percentiles[percentile];
@@ -51,13 +49,13 @@
                                 <FluentIcon Style="vertical-align: text-bottom" Value="@icon" Title="@title"/>
                             }
                         }
-                    </AspireTemplateColumn>
+                    </TemplateColumn>
                 }
             }
             else if (_metrics.Values.All(value => value is MetricValueView))
             {
                 <!-- if we're switching between grid types, this could be false -->
-                <AspireTemplateColumn Title="@_unitColumnHeader">
+                <TemplateColumn Title="@_unitColumnHeader">
                     @{
                         var metricValueView = context as MetricValueView;
                     }
@@ -78,11 +76,11 @@
                             <FluentIcon Style="vertical-align: text-bottom" Value="@icon" Title="@title"/>
                         }
                     }
-                </AspireTemplateColumn>
+                </TemplateColumn>
             }
             @if (_exemplars.Count > 0)
             {
-                <AspireTemplateColumn Title="@Loc[nameof(ControlsStrings.MetricTableExemplarsColumnHeader)]">
+                <TemplateColumn Title="@Loc[nameof(ControlsStrings.MetricTableExemplarsColumnHeader)]">
                     @if (context.Exemplars.Count > 0)
                     {
                         @* min-width ensures a consistent button width up to 999 metrics *@
@@ -95,7 +93,7 @@
                     {
                         <span>0</span>
                     }
-                </AspireTemplateColumn>
+                </TemplateColumn>
             }
         </ChildContent>
         <EmptyContent>
diff --git a/src/Aspire.Dashboard/Components/Dialogs/ExemplarsDialog.razor b/src/Aspire.Dashboard/Components/Dialogs/ExemplarsDialog.razor
@@ -14,27 +14,25 @@
 @inject IStringLocalizer<ControlsStrings> ControlsStringsLoc
 
 <div style="max-height: 44vh; overflow-y: auto;">
-    <FluentDataGrid ResizeLabel="@AspireFluentDataGridHeaderCell.GetResizeLabel(ControlsStringsLoc)"
-                    ResizeType="DataGridResizeType.Discrete"
-                    Items="@MetricView"
+    <FluentDataGrid Items="@MetricView"
                     ItemSize="46"
                     GridTemplateColumns="2fr 1fr 1fr 1fr"
                     TGridItem="ChartExemplar">
         <ChildContent>
-            <AspireTemplateColumn Title="@Loc[nameof(Dialogs.ExemplarsDialogTraceColumnHeader)]" TooltipText="@(context => GetTitle(context))" Tooltip="true">
+            <TemplateColumn Title="@Loc[nameof(Dialogs.ExemplarsDialogTraceColumnHeader)]" TooltipText="@(context => GetTitle(context))" Tooltip="true">
                 <span style="padding-left:5px; border-left-width: 5px; border-left-style: solid; border-left-color: @(context.Span != null ? ColorGenerator.Instance.GetColorHexByKey(OtlpApplication.GetResourceName(context.Span.Source, Content.Applications)) : "transparent");">
                     @GetTitle(context)
                 </span>
-            </AspireTemplateColumn>
-            <AspireTemplateColumn Title="@Loc[nameof(Dialogs.ExemplarsDialogTimestampColumnHeader)]" TooltipText="@(context => FormatHelpers.FormatDateTime(TimeProvider, TimeProvider.ToLocal(context.Start), MillisecondsDisplay.None, CultureInfo.CurrentCulture))" Tooltip="true">
+            </TemplateColumn>
+            <TemplateColumn Title="@Loc[nameof(Dialogs.ExemplarsDialogTimestampColumnHeader)]" TooltipText="@(context => FormatHelpers.FormatDateTime(TimeProvider, TimeProvider.ToLocal(context.Start), MillisecondsDisplay.None, CultureInfo.CurrentCulture))" Tooltip="true">
                 @FormatHelpers.FormatTimeWithOptionalDate(TimeProvider, TimeProvider.ToLocal(context.Start), MillisecondsDisplay.Truncated)
-            </AspireTemplateColumn>
-            <AspireTemplateColumn Title="@Loc[nameof(Dialogs.ExemplarsDialogValueColumnHeader)]">
+            </TemplateColumn>
+            <TemplateColumn Title="@Loc[nameof(Dialogs.ExemplarsDialogValueColumnHeader)]">
                 @FormatMetricValue(context.Value)
-            </AspireTemplateColumn>
-            <AspireTemplateColumn Title="@Loc[nameof(Dialogs.ExemplarsDialogDetailsColumnHeader)]">
+            </TemplateColumn>
+            <TemplateColumn Title="@Loc[nameof(Dialogs.ExemplarsDialogDetailsColumnHeader)]">
                 <FluentButton Appearance="Appearance.Lightweight" OnClick="@(() => OnViewDetailsAsync(context))">View</FluentButton>
-            </AspireTemplateColumn>
+            </TemplateColumn>
         </ChildContent>
         <EmptyContent>
             <FluentIcon Icon="Icons.Regular.Size24.ChartMultiple" />&nbsp;@Loc[nameof(ControlsStrings.MetricTableNoMetricsFound)]
diff --git a/src/Aspire.Dashboard/Components/Pages/Metrics.razor b/src/Aspire.Dashboard/Components/Pages/Metrics.razor
@@ -91,7 +91,7 @@
                                             TGridItem="OtlpInstrumentSummary">
                                             <ChildContent>
                                                 <TemplateColumn Title="@Loc[nameof(Dashboard.Resources.Metrics.MetricsInsturementNameGridNameColumnHeader)]">
-                                                    <FluentAnchor Href="@DashboardUrls.MetricsUrl(resource: PageViewModel.SelectedApplication.Name, meter: context.Parent.MeterName, instrument: context.Name)" Appearance="Appearance.Lightweight">
+                                                    <FluentAnchor Href="@DashboardUrls.MetricsUrl(resource: PageViewModel.SelectedApplication.Name, meter: context.Parent.MeterName, instrument: context.Name, duration: DurationMinutes, view: ViewKindName)" Appearance="Appearance.Lightweight">
                                                         @context.Name
                                                     </FluentAnchor>
                                                 </TemplateColumn>
diff --git a/src/Aspire.Dashboard/Components/Pages/Metrics.razor.cs b/src/Aspire.Dashboard/Components/Pages/Metrics.razor.cs
@@ -43,7 +43,7 @@ public partial class Metrics : IDisposable, IPageWithSessionAndUrlState<Metrics.
 
     [Parameter]
     [SupplyParameterFromQuery(Name = "duration")]
-    public int DurationMinutes { get; set; }
+    public int? DurationMinutes { get; set; }
 
     [Parameter]
     [SupplyParameterFromQuery(Name = "view")]
@@ -238,15 +238,11 @@ private Task HandleSelectedTreeItemChangedAsync()
 
     public string GetUrlFromSerializableViewModel(MetricsPageState serializable)
     {
-        var duration = PageViewModel.SelectedDuration.Id != s_defaultDuration
-            ? (int?)serializable.DurationMinutes
-            : null;
-
         var url = DashboardUrls.MetricsUrl(
             resource: serializable.ApplicationName,
             meter: serializable.MeterName,
             instrument: serializable.InstrumentName,
-            duration: duration,
+            duration: serializable.DurationMinutes,
             view: serializable.ViewKind);
 
         return url;
diff --git a/src/Aspire.Dashboard/wwwroot/js/app-metrics.js b/src/Aspire.Dashboard/wwwroot/js/app-metrics.js
@@ -119,7 +119,12 @@ export function initializeChart(id, traces, exemplarTrace, rangeStartTime, range
 
     const resizeObserver = new ResizeObserver(entries => {
         for (let entry of entries) {
-            Plotly.Plots.resize(entry.target);
+            // Don't resize if not visible.
+            var display = window.getComputedStyle(entry.target).display;
+            var isHidden = !display || display === "none";
+            if (!isHidden) {
+                Plotly.Plots.resize(entry.target);
+            }
         }
     });
     plot.then(plotyDiv => {
diff --git a/tests/Aspire.Dashboard.Components.Tests/Pages/MetricsTests.cs b/tests/Aspire.Dashboard.Components.Tests/Pages/MetricsTests.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Web;
 using Aspire.Dashboard.Components.Controls;
 using Aspire.Dashboard.Components.Pages;
 using Aspire.Dashboard.Components.Resize;
@@ -14,6 +15,7 @@
 using Microsoft.Extensions.DependencyInjection;
 using OpenTelemetry.Proto.Metrics.V1;
 using Xunit;
+using static Aspire.Dashboard.Components.Pages.Metrics;
 using static Aspire.Tests.Shared.Telemetry.TelemetryTestHelpers;
 
 namespace Aspire.Dashboard.Components.Tests.Pages;
@@ -41,13 +43,87 @@ public void ChangeResource_MeterAndInstrumentNotOnNewResources_InstrumentCleared
             expectedInstrumentNameAfterChange: null);
     }
 
+    [Fact]
+    public void InitialLoad_HasSessionState_RedirectUsingState()
+    {
+        // Arrange
+        var testSessionStorage = new TestSessionStorage
+        {
+            OnGetAsync = key =>
+            {
+                if (key == BrowserStorageKeys.MetricsPageState)
+                {
+                    var state = new MetricsPageState
+                    {
+                        ApplicationName = "TestApp",
+                        MeterName = "test-meter",
+                        InstrumentName = "test-instrument",
+                        DurationMinutes = 720,
+                        ViewKind = MetricViewKind.Table.ToString()
+                    };
+                    return (true, state);
+                }
+                else
+                {
+                    throw new InvalidOperationException("Unexpected key: " + key);
+                }
+            }
+        };
+        MetricsSetupHelpers.SetupMetricsPage(this, sessionStorage: testSessionStorage);
+
+        var navigationManager = Services.GetRequiredService<NavigationManager>();
+        navigationManager.NavigateTo(DashboardUrls.MetricsUrl());
+
+        Uri? loadRedirect = null;
+        navigationManager.LocationChanged += (s, a) =>
+        {
+            loadRedirect = new Uri(a.Location);
+        };
+
+        var telemetryRepository = Services.GetRequiredService<TelemetryRepository>();
+        telemetryRepository.AddMetrics(new AddContext(), new RepeatedField<ResourceMetrics>
+        {
+            new ResourceMetrics
+            {
+                Resource = CreateResource(name: "TestApp"),
+                ScopeMetrics =
+                {
+                    new ScopeMetrics
+                    {
+                        Scope = CreateScope(name: "test-meter"),
+                        Metrics =
+                        {
+                            CreateSumMetric(metricName: "test-instrument", startTime: s_testTime.AddMinutes(1))
+                        }
+                    }
+                }
+            }
+        });
+
+        // Act
+        var cut = RenderComponent<Metrics>(builder =>
+        {
+            builder.AddCascadingValue(new ViewportInformation(IsDesktop: true, IsUltraLowHeight: false, IsUltraLowWidth: false));
+        });
+
+        // Assert
+        Assert.NotNull(loadRedirect);
+        Assert.Equal("/metrics/resource/TestApp", loadRedirect.AbsolutePath);
+
+        var query = HttpUtility.ParseQueryString(loadRedirect.Query);
+        Assert.Equal("test-meter", query["meter"]);
+        Assert.Equal("test-instrument", query["instrument"]);
+        Assert.Equal("720", query["duration"]);
+        Assert.Equal(MetricViewKind.Table.ToString(), query["view"]);
+    }
+
     private void ChangeResourceAndAssertInstrument(string app1InstrumentName, string app2InstrumentName, string? expectedInstrumentNameAfterChange)
     {
         // Arrange
         MetricsSetupHelpers.SetupMetricsPage(this);
 
         var navigationManager = Services.GetRequiredService<NavigationManager>();
-        navigationManager.NavigateTo(DashboardUrls.MetricsUrl(resource: "TestApp", meter: "test-meter", instrument: app1InstrumentName));
+        navigationManager.NavigateTo(DashboardUrls.MetricsUrl(resource: "TestApp", meter: "test-meter", instrument: app1InstrumentName, duration: 720, view: MetricViewKind.Table.ToString()));
 
         var telemetryRepository = Services.GetRequiredService<TelemetryRepository>();
         telemetryRepository.AddMetrics(new AddContext(), new RepeatedField<ResourceMetrics>
@@ -110,5 +186,8 @@ private void ChangeResourceAndAssertInstrument(string app1InstrumentName, string
             // Meter is cleared if instrument is cleared.
             Assert.Equal("test-meter", viewModel.SelectedMeter!.MeterName);
         }
+
+        Assert.Equal(MetricViewKind.Table, viewModel.SelectedViewKind);
+        Assert.Equal(TimeSpan.FromMinutes(720), viewModel.SelectedDuration.Id);
     }
 }
diff --git a/tests/Aspire.Dashboard.Components.Tests/Shared/MetricsSetupHelpers.cs b/tests/Aspire.Dashboard.Components.Tests/Shared/MetricsSetupHelpers.cs
@@ -47,7 +47,7 @@ internal static void SetupPlotlyChart(TestContext context)
         context.Services.AddSingleton<IDialogService, DialogService>();
     }
 
-    internal static void SetupMetricsPage(TestContext context)
+    internal static void SetupMetricsPage(TestContext context, ISessionStorage? sessionStorage = null)
     {
         var version = typeof(FluentMain).Assembly.GetName().Version!;
 
@@ -78,7 +78,7 @@ internal static void SetupMetricsPage(TestContext context)
         context.Services.AddSingleton<DimensionManager>();
         context.Services.AddSingleton<IDialogService, DialogService>();
         context.Services.AddSingleton<BrowserTimeProvider, TestTimeProvider>();
-        context.Services.AddSingleton<ISessionStorage, TestSessionStorage>();
+        context.Services.AddSingleton<ISessionStorage>(sessionStorage ?? new TestSessionStorage());
         context.Services.AddSingleton<ILocalStorage, TestLocalStorage>();
         context.Services.AddSingleton<ShortcutManager>();
         context.Services.AddSingleton<LibraryConfiguration>();
diff --git a/tests/Aspire.Dashboard.Components.Tests/Shared/TestSessionStorage.cs b/tests/Aspire.Dashboard.Components.Tests/Shared/TestSessionStorage.cs
@@ -7,13 +7,27 @@ namespace Aspire.Dashboard.Components.Tests.Shared;
 
 public sealed class TestSessionStorage : ISessionStorage
 {
+    public Func<string, (bool Success, object? Value)>? OnGetAsync { get; set; }
+    public Action<string, object?>? OnSetAsync { get; set; }
+
     public Task<StorageResult<T>> GetAsync<T>(string key)
     {
+        if (OnGetAsync is { } callback)
+        {
+            var (success, value) = callback(key);
+            return Task.FromResult(new StorageResult<T>(Success: success, Value: (T)(value ?? default(T))!));
+        }
+
         return Task.FromResult<StorageResult<T>>(new StorageResult<T>(Success: false, Value: default));
     }
 
     public Task SetAsync<T>(string key, T value)
     {
+        if (OnSetAsync is { } callback)
+        {
+            callback(key, value);
+        }
+
         return Task.CompletedTask;
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -21,8 +21,6 @@`
`21`	`21`	`<div id="metric-table-container" style="height: 40vh; overflow-y: auto; width:1200px;">`
`22`	`22`	`@* ItemKey is to preserve row focus by associating rows with their associated time *@`
`23`	`23`	`<FluentDataGrid`
`24`		`- ResizeLabel="@AspireFluentDataGridHeaderCell.GetResizeLabel(ControlsStringsLoc)"`
`25`		`- ResizeType="DataGridResizeType.Discrete"`
`26`	`24`	`Items="@_metricsView"`
`27`	`25`	`ItemSize="46"`
`28`	`26`	`Virtualize="true"`
`@@ -37,7 +35,7 @@`
`37`	`35`	`{`
`38`	`36`	`foreach (var (percentile, underlineColor) in percentileColumns)`
`39`	`37`	`{`
`40`		`- <AspireTemplateColumn Title="@((_metricsView.FirstOrDefault() as HistogramMetricView)?.Percentiles[percentile].Name ?? (_instrument is not null ? $"P{percentile} {InstrumentUnitResolver.ResolveDisplayedUnit(_instrument, titleCase: true, pluralize: true)}" : $"P{percentile}"))">`
	`38`	`+ <TemplateColumn Title="@((_metricsView.FirstOrDefault() as HistogramMetricView)?.Percentiles[percentile].Name ?? (_instrument is not null ? $"P{percentile} {InstrumentUnitResolver.ResolveDisplayedUnit(_instrument, titleCase: true, pluralize: true)}" : $"P{percentile}"))">`
`41`	`39`	`@if (context is HistogramMetricView histogramMetric)`
`42`	`40`	`{`
`43`	`41`	`var percentileData = histogramMetric.Percentiles[percentile];`
`@@ -51,13 +49,13 @@`
`51`	`49`	`<FluentIcon Style="vertical-align: text-bottom" Value="@icon" Title="@title"/>`
`52`	`50`	`}`
`53`	`51`	`}`
`54`		`- </AspireTemplateColumn>`
	`52`	`+ </TemplateColumn>`
`55`	`53`	`}`
`56`	`54`	`}`
`57`	`55`	`else if (_metrics.Values.All(value => value is MetricValueView))`
`58`	`56`	`{`
`59`	`57`	`<!-- if we're switching between grid types, this could be false -->`
`60`		`- <AspireTemplateColumn Title="@_unitColumnHeader">`
	`58`	`+ <TemplateColumn Title="@_unitColumnHeader">`
`61`	`59`	`@{`
`62`	`60`	`var metricValueView = context as MetricValueView;`
`63`	`61`	`}`
`@@ -78,11 +76,11 @@`
`78`	`76`	`<FluentIcon Style="vertical-align: text-bottom" Value="@icon" Title="@title"/>`
`79`	`77`	`}`
`80`	`78`	`}`
`81`		`- </AspireTemplateColumn>`
	`79`	`+ </TemplateColumn>`
`82`	`80`	`}`
`83`	`81`	`@if (_exemplars.Count > 0)`
`84`	`82`	`{`
`85`		`- <AspireTemplateColumn Title="@Loc[nameof(ControlsStrings.MetricTableExemplarsColumnHeader)]">`
	`83`	`+ <TemplateColumn Title="@Loc[nameof(ControlsStrings.MetricTableExemplarsColumnHeader)]">`
`86`	`84`	`@if (context.Exemplars.Count > 0)`
`87`	`85`	`{`
`88`	`86`	`@* min-width ensures a consistent button width up to 999 metrics *@`
`@@ -95,7 +93,7 @@`
`95`	`93`	`{`
`96`	`94`	`<span>0</span>`
`97`	`95`	`}`
`98`		`- </AspireTemplateColumn>`
	`96`	`+ </TemplateColumn>`
`99`	`97`	`}`
`100`	`98`	`</ChildContent>`
`101`	`99`	`<EmptyContent>`
Original file line number	Diff line number	Diff line change
`@@ -7,13 +7,27 @@ namespace Aspire.Dashboard.Components.Tests.Shared;`
`7`	`7`
`8`	`8`	`public sealed class TestSessionStorage : ISessionStorage`
`9`	`9`	`{`
	`10`	`+ public Func<string, (bool Success, object? Value)>? OnGetAsync { get; set; }`
	`11`	`+ public Action<string, object?>? OnSetAsync { get; set; }`
	`12`	`+`
`10`	`13`	`public Task<StorageResult<T>> GetAsync<T>(string key)`
`11`	`14`	`{`
	`15`	`+ if (OnGetAsync is { } callback)`
	`16`	`+ {`
	`17`	`+ var (success, value) = callback(key);`
	`18`	`+ return Task.FromResult(new StorageResult<T>(Success: success, Value: (T)(value ?? default(T))!));`
	`19`	`+ }`
	`20`	`+`
`12`	`21`	`return Task.FromResult<StorageResult<T>>(new StorageResult<T>(Success: false, Value: default));`
`13`	`22`	`}`
`14`	`23`
`15`	`24`	`public Task SetAsync<T>(string key, T value)`
`16`	`25`	`{`
	`26`	`+ if (OnSetAsync is { } callback)`
	`27`	`+ {`
	`28`	`+ callback(key, value);`
	`29`	`+ }`
	`30`	`+`
`17`	`31`	`return Task.CompletedTask;`
`18`	`32`	`}`
`19`	`33`	`}`