Skip to content

Lite: Queries-tab comparisons read the grid's UTC window, not a server-shifted one (#4284) #4302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 2 commits into from
Sep 25, 2026
Merged

Lite: Queries-tab comparisons read the grid's UTC window, not a server-shifted one (#4284) #4302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da09844
Fix #4284: Queries-tab comparisons now read the grid's UTC window
erikdarlingdata Sep 25, 2026
9a3cdc9
Merge remote-tracking branch 'origin/dev' into fix/4284-lite-comparis…
erikdarlingdata Sep 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions Lite.Tests/ComparisonWindowUtcTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
* Copyright (c) 2026 Erik Darling, Darling Data LLC
*
* This file is part of the SQL Server Performance Monitor Lite.
*
* Licensed under the MIT License. See LICENSE file in the project root for full license information.
*/

using System;
using System.IO;
using System.Runtime.CompilerServices;
using System.Text.RegularExpressions;
using PerformanceMonitorLite.Controls;
using PerformanceMonitorLite.Services;
using Xunit;

namespace PerformanceMonitorLite.Tests;

/// <summary>
/// #4284: <c>GetQueryStatsComparisonAsync</c>/<c>GetProcedureStatsComparisonAsync</c>/<c>GetQueryStoreComparisonAsync</c>
/// (<c>Lite/Services/LocalDataService.QueryStats.cs</c> and <c>.QueryStore.cs</c>) compare their bounds straight
/// against UTC <c>collection_time</c>, with no offset conversion of their own. Two callers used to hand them a
/// server-local window instead: a custom range (ServerTab.Comparison.cs's old parameterless
/// <c>GetComparisonRange()</c>, and ServerTab.Refresh.cs's <c>cStart</c>/<c>cEnd</c>) and a slicer drag
/// (ServerTab.Slicers.cs's <c>fromServer</c>/<c>toServer</c>). On a server not on UTC, that shifted the
/// comparison window by the server's offset. Pins <see cref="ServerTab.ShiftComparisonRange"/>'s pure day-shift
/// arithmetic directly, and the call sites via source scan (the handlers are UI-bound, same approach
/// QueryWindowTruncationTests took for #4279/#4231).
/// </summary>
public sealed class ComparisonWindowUtcTests
{
/// <summary>
/// #4284 ruling item 2/6: the shift is a fixed duration (1 or 7 days), so it commutes with whatever UTC
/// offset built the "current" window -- a server at UTC+5 and one at UTC-7 both get a baseline that is
/// exactly their own current window moved back by the same number of days, covering the SAME span
/// (ruling item 3: after a slicer drag, the baseline is the dragged window shifted back, not the whole
/// preset range).
/// </summary>
[Theory]
[InlineData(300)] // UTC+5
[InlineData(-420)] // UTC-7
public void ShiftComparisonRange_MatchesGridsUtcWindow_ShiftedByOneOrSevenDays(int utcOffsetMinutes)
{
// The same derivation CompareToCombo_SelectionChanged and the six ServerTab.Refresh.cs call sites
// now use for "current": server-local pickers converted back to UTC by LocalDataService.GetQueriesTabWindowUtc.
var fromServerLocal = new DateTime(2026, 3, 10, 9, 0, 0, DateTimeKind.Unspecified);
var toServerLocal = new DateTime(2026, 3, 10, 17, 0, 0, DateTimeKind.Unspecified);
var (currentStartUtc, currentEndUtc) = LocalDataService.GetQueriesTabWindowUtc(24, fromServerLocal, toServerLocal, utcOffsetMinutes);

var yesterday = ServerTab.ShiftComparisonRange(1, currentStartUtc, currentEndUtc);
Assert.NotNull(yesterday);
Assert.Equal(currentStartUtc.AddDays(-1), yesterday!.Value.From);
Assert.Equal(currentEndUtc.AddDays(-1), yesterday.Value.To);
Assert.Equal(currentEndUtc - currentStartUtc, yesterday.Value.To - yesterday.Value.From);

var lastWeek = ServerTab.ShiftComparisonRange(2, currentStartUtc, currentEndUtc);
Assert.NotNull(lastWeek);
Assert.Equal(currentStartUtc.AddDays(-7), lastWeek!.Value.From);
Assert.Equal(currentEndUtc.AddDays(-7), lastWeek.Value.To);

var sameDayLastWeek = ServerTab.ShiftComparisonRange(3, currentStartUtc, currentEndUtc);
Assert.Equal(lastWeek, sameDayLastWeek);

Assert.Null(ServerTab.ShiftComparisonRange(0, currentStartUtc, currentEndUtc));
Assert.Null(ServerTab.ShiftComparisonRange(-1, currentStartUtc, currentEndUtc));
}

/// <summary>
/// #4284 ruling item 6's source pin: every Refresh...ComparisonAsync call site passes a UTC bound -- never
/// the server-local fromServer/toServer a slicer drag produces, nor a cStart built from fromDate in
/// ServerTab.Refresh.cs. Confirmed by checking out ServerTab.Comparison.cs, ServerTab.Refresh.cs and
/// ServerTab.Slicers.cs from origin/dev at 80948a66 (pre-#4284) and re-running this test: every assertion
/// below failed against that pre-fix source.
/// </summary>
[Fact]
public void ComparisonCallSites_TakeUtcBounds_NotServerLocalOnes()
{
var comparisonSource = File.ReadAllText(ControlsFile("ServerTab.Comparison.cs"));
var refreshSource = File.ReadAllText(ControlsFile("ServerTab.Refresh.cs"));
var slicersSource = File.ReadAllText(ControlsFile("ServerTab.Slicers.cs"));
var combinedSource = comparisonSource + "\n" + refreshSource + "\n" + slicersSource;

// No caller anywhere still calls the old parameterless GetComparisonRange() -- it now requires the
// caller's own current UTC window as an argument.
Assert.False(Regex.IsMatch(combinedSource, @"GetComparisonRange\(\)"),
"a caller still invokes the parameterless GetComparisonRange() (#4284) -- it now takes the " +
"caller's current UTC window as an argument.");

// IsQueryStatsComparisonActive tests the combo directly (ruling item 2), not through GetComparisonRange.
Assert.Contains("IsQueryStatsComparisonActive => CompareToCombo != null && CompareToCombo.SelectedIndex > 0;", comparisonSource);

// Slicer handlers: comparisons take e.StartUtc/e.EndUtc, never the server-local fromServer/toServer
// the grid read beside them uses.
var slicerComparisonCallsOnUtc = Regex.Matches(slicersSource,
@"Refresh(?:QueryStats|ProcStats|QueryStore)ComparisonAsync\(e\.StartUtc,\s*e\.EndUtc\)").Count;
Assert.True(slicerComparisonCallsOnUtc == 3,
$"expected all 3 OnXSlicerChanged comparison calls to pass e.StartUtc, e.EndUtc (found {slicerComparisonCallsOnUtc}) " +
"-- fromServer/toServer are server-local and the comparison reads compare straight against UTC " +
"collection_time (#4284).");
Assert.False(Regex.IsMatch(slicersSource, @"Refresh(?:QueryStats|ProcStats|QueryStore)ComparisonAsync\([^)]*fromServer,\s*toServer\)"),
"a slicer comparison call still passes server-local fromServer/toServer (#4284).");

// ServerTab.Refresh.cs: no comparison call site builds its own cStart from fromDate any more.
Assert.False(Regex.IsMatch(refreshSource, @"var\s+cStart\d?\s*="),
"a comparison call site in ServerTab.Refresh.cs still builds a server-local cStart from fromDate (#4284).");

// The six Refresh.cs comparison calls each share the IDENTICAL windowStart/windowEnd tuple the
// banner call right beside them uses -- computed once, handed to both (ruling item 1).
var pairedCalls = Regex.Matches(refreshSource,
@"Refresh(?:QueryStats|ProcStats|QueryStore)ComparisonAsync\((windowStart\d?),\s*(windowEnd\d?)\);\s*" +
@"await RefreshWindowTruncatedBannerAsync\(QueryWindowRelation\.\w+,\s*\w+,\s*\1,\s*\2\);");
Assert.True(pairedCalls.Count == 6,
$"expected all 6 ServerTab.Refresh.cs comparison calls to share the SAME windowStart/windowEnd tuple " +
$"the adjacent banner call uses (found {pairedCalls.Count}) -- a divergent tuple means the comparison " +
"and the banner could disagree about the window again (#4284).");

// CompareToCombo_SelectionChanged routes its three comparison refreshes through the same
// GetQueriesTabWindowUtc tuple the grid reads use, not a bare DateTime.UtcNow/fromDate pair.
Assert.Contains(
"var (currentStart, currentEnd) = LocalDataService.GetQueriesTabWindowUtc(hoursBack, fromDate, toDate, ServerTimeHelper.UtcOffsetMinutes);",
comparisonSource);
}

private static string ControlsFile(string name) => Path.Combine(ControlsDir(), name);

private static string ControlsDir([CallerFilePath] string thisFile = "") =>
Path.GetFullPath(Path.Combine(Path.GetDirectoryName(thisFile)!, "..", "Lite", "Controls"));
}
20 changes: 12 additions & 8 deletions Lite.Tests/QueryWindowTruncationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,10 +446,15 @@ public void SlicerBannerWindow_MatchesTheGridsUtcWindow_ForANonUtcServer()

/// <summary>
/// #4279 revert-proof: text-scans SOURCE (matching <see cref="QueriesTabGridReads_RouteThroughSharedWindowFloorHelper"/>'s
/// mechanism) so a future edit that quietly goes back to feeding the banner server-local
/// fromServer/toServer or cStart/cStart2/cStart3 fails a test even though those names still compile fine
/// (they are plain <c>DateTime</c>s either way). Confirmed by reverting ServerTab.Slicers.cs and
/// ServerTab.Refresh.cs to 3b8d9e12 (pre-fix): both assertions below failed before the fix.
/// mechanism) so a future edit that quietly goes back to feeding the banner server-local fromServer/toServer
/// fails a test even though those names still compile fine (they are plain <c>DateTime</c>s either way).
/// Confirmed by reverting ServerTab.Slicers.cs and ServerTab.Refresh.cs to 3b8d9e12 (pre-#4279-fix): both
/// assertions below failed before that fix.
///
/// <para>#4284 updated the ServerTab.Refresh.cs half: the banner call now shares its windowStart/windowEnd
/// tuple (formerly bannerStart/bannerEnd) with the comparison call beside it, rather than the comparison
/// keeping its own server-local cStart -- see <see cref="ComparisonWindowUtcTests.ComparisonCallSites_TakeUtcBounds_NotServerLocalOnes"/>
/// for that half's pin.</para>
/// </summary>
[Fact]
public void WindowTruncatedBannerCallSites_TakeUtcBounds_NotServerLocalOnes()
Expand All @@ -466,12 +471,11 @@ public void WindowTruncatedBannerCallSites_TakeUtcBounds_NotServerLocalOnes()

var refreshSource = File.ReadAllText(ControlsFile("ServerTab.Refresh.cs"));
var refreshBannerCallsOnHelperOutput = Regex.Matches(refreshSource,
@"RefreshWindowTruncatedBannerAsync\(\s*QueryWindowRelation\.\w+,\s*\w+,\s*bannerStart\d?,\s*bannerEnd\d?\)").Count;
@"RefreshWindowTruncatedBannerAsync\(\s*QueryWindowRelation\.\w+,\s*\w+,\s*windowStart\d?,\s*windowEnd\d?\)").Count;
Assert.True(refreshBannerCallsOnHelperOutput == 6,
$"expected all 6 ServerTab.Refresh.cs banner calls to pass a GetQueriesTabWindowUtc result " +
$"(bannerStart/bannerEnd) (found {refreshBannerCallsOnHelperOutput}) -- cStart/cStart2/cStart3 are " +
"server-local under a custom range and feed the (deliberately untouched -- see ServerTab.Comparison.cs) " +
"comparison calls on the same lines, not the banner (#4279).");
$"(windowStart/windowEnd) (found {refreshBannerCallsOnHelperOutput}) -- a server-local cStart must " +
"not feed the banner (#4279/#4284).");
Assert.Equal(6, Regex.Matches(refreshSource, @"LocalDataService\.GetQueriesTabWindowUtc\(").Count);
}

Expand Down
68 changes: 37 additions & 31 deletions Lite/Controls/ServerTab.Comparison.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,12 @@ private async void CompareToCombo_SelectionChanged(object sender, SelectionChang
// Also refresh comparison grids
try
{
var currentEnd = toDate ?? DateTime.UtcNow;
var currentStart = fromDate ?? currentEnd.AddHours(-hoursBack);
/* #4284: GetQueryStatsComparisonAsync/GetProcedureStatsComparisonAsync/GetQueryStoreComparisonAsync
compare currentStart/currentEnd straight against UTC collection_time, with no offset conversion
of their own -- the SAME UTC window the Top Queries/Top Procedures/Query Store grid reads get via
LocalDataService.GetQueriesTabWindowUtc, computed once here and handed to all three so the
current window matches the grid on any server not on UTC. */
var (currentStart, currentEnd) = LocalDataService.GetQueriesTabWindowUtc(hoursBack, fromDate, toDate, ServerTimeHelper.UtcOffsetMinutes);
await RefreshQueryStatsComparisonAsync(currentStart, currentEnd);
await RefreshProcStatsComparisonAsync(currentStart, currentEnd);
await RefreshQueryStoreComparisonAsync(currentStart, currentEnd);
Expand All @@ -52,38 +56,38 @@ private async void CompareToCombo_SelectionChanged(object sender, SelectionChang
}

/// <summary>
/// Computes the reference time range for the comparison overlay based on the
/// current Compare dropdown selection and the active time range.
/// Returns null if "None" is selected.
/// #4284: pure day-shift for the Compare-to selection -- Yesterday is 1 day back, Last week and Same day
/// last week are 7 days back, None (index &lt;= 0) is no comparison. Basis-agnostic: the shift is a fixed
/// duration, so it commutes with whatever basis (UTC or server-local) the caller's start/end are already
/// in. Pulled out of <see cref="GetComparisonRange"/> as a static helper, the way #4279 pulled
/// <see cref="LocalDataService.GetQueriesTabWindowUtc"/> out of its instance callers, so a test can drive
/// it directly without constructing the WPF UserControl.
/// </summary>
private (DateTime From, DateTime To)? GetComparisonRange()
{
if (CompareToCombo == null || CompareToCombo.SelectedIndex <= 0) return null;

var hoursBack = GetHoursBack();
DateTime? fromDate = null, toDate = null;
if (IsCustomRange)
{
var fromLocal = GetDateTimeFromPickers(FromDatePicker!, FromHourCombo, FromMinuteCombo);
var toLocal = GetDateTimeFromPickers(ToDatePicker!, ToHourCombo, ToMinuteCombo);
if (fromLocal.HasValue && toLocal.HasValue)
{
fromDate = ServerTimeHelper.DisplayTimeToServerTime(fromLocal.Value, ServerTimeHelper.CurrentDisplayMode);
toDate = ServerTimeHelper.DisplayTimeToServerTime(toLocal.Value, ServerTimeHelper.CurrentDisplayMode);
}
}

var currentEnd = toDate ?? DateTime.UtcNow;
var currentStart = fromDate ?? currentEnd.AddHours(-hoursBack);

return CompareToCombo.SelectedIndex switch
internal static (DateTime From, DateTime To)? ShiftComparisonRange(int selectedIndex, DateTime currentStart, DateTime currentEnd)
=> selectedIndex switch
{
1 => (currentStart.AddDays(-1), currentEnd.AddDays(-1)), // Yesterday
2 => (currentStart.AddDays(-7), currentEnd.AddDays(-7)), // Last week
3 => (currentStart.AddDays(-7), currentEnd.AddDays(-7)), // Same day last week
_ => null
};
}

/// <summary>
/// #4284: computes the comparison baseline for the Queries tab's three comparison reads
/// (GetQueryStatsComparisonAsync / GetProcedureStatsComparisonAsync / GetQueryStoreComparisonAsync), which
/// compare their bounds straight against UTC collection_time with no offset conversion of their own.
/// <paramref name="currentStartUtc"/>/<paramref name="currentEndUtc"/> MUST be the same UTC window the
/// grid beside the caller read (<see cref="LocalDataService.GetQueriesTabWindowUtc"/>, or a slicer's own
/// <c>SlicerRangeEventArgs.StartUtc</c>/<c>EndUtc</c>) -- a server-local pair here silently shifts the
/// baseline by the server's UTC offset, which is what #4284 fixed. Returns null if "None" is selected.
///
/// <para>Not used by <c>RefreshOverviewAsync</c>'s correlated-lanes comparison (ServerTab.Refresh.cs):
/// that caller's underlying reads (GetCpuUtilizationAsync, GetTotalWaitTrendAsync) take server-local
/// fromDate/toDate, not UTC, so it derives its own range with <see cref="ShiftComparisonRange"/> in that
/// basis instead of calling through here.</para>
/// </summary>
private (DateTime From, DateTime To)? GetComparisonRange(DateTime currentStartUtc, DateTime currentEndUtc)
=> CompareToCombo == null ? null : ShiftComparisonRange(CompareToCombo.SelectedIndex, currentStartUtc, currentEndUtc);

/* #2933: CompareToCombo_SelectionChanged sets no in-flight flag, so two Compare changes overlap and
the LATER-STARTING read can land first, leaving the grid showing the baseline the operator moved
Expand All @@ -100,7 +104,9 @@ Only the comparison loaders take this. ServerTab's main-tab Refresh*Async loader
serves in TimeDisplayMode_SelectionChanged, ServerTab.DrillDown.cs and ServerTab.Grids.cs. */
private readonly PerformanceMonitor.Ui.ScopedLoadGenerations _loads = new();

private bool IsQueryStatsComparisonActive => GetComparisonRange() != null;
/* #4284: tests the combo directly rather than through GetComparisonRange, which now needs the caller's
current UTC window as an argument and has none to give here. */
private bool IsQueryStatsComparisonActive => CompareToCombo != null && CompareToCombo.SelectedIndex > 0;

private void SetQueryStatsComparisonMode(bool active, (DateTime From, DateTime To)? baselineRange = null)
{
Expand All @@ -119,7 +125,7 @@ private void SetQueryStatsComparisonMode(bool active, (DateTime From, DateTime T
private async System.Threading.Tasks.Task RefreshQueryStatsComparisonAsync(DateTime currentStart, DateTime currentEnd)
{
var gen = _loads.Claim(nameof(RefreshQueryStatsComparisonAsync));
var baselineRange = GetComparisonRange();
var baselineRange = GetComparisonRange(currentStart, currentEnd);
if (baselineRange == null)
{
SetQueryStatsComparisonMode(false);
Expand Down Expand Up @@ -159,7 +165,7 @@ private void SetProcStatsComparisonMode(bool active, (DateTime From, DateTime To
private async System.Threading.Tasks.Task RefreshProcStatsComparisonAsync(DateTime currentStart, DateTime currentEnd)
{
var gen = _loads.Claim(nameof(RefreshProcStatsComparisonAsync));
var baselineRange = GetComparisonRange();
var baselineRange = GetComparisonRange(currentStart, currentEnd);
if (baselineRange == null)
{
SetProcStatsComparisonMode(false);
Expand Down Expand Up @@ -198,7 +204,7 @@ private void SetQueryStoreComparisonMode(bool active, (DateTime From, DateTime T
private async System.Threading.Tasks.Task RefreshQueryStoreComparisonAsync(DateTime currentStart, DateTime currentEnd)
{
var gen = _loads.Claim(nameof(RefreshQueryStoreComparisonAsync));
var baselineRange = GetComparisonRange();
var baselineRange = GetComparisonRange(currentStart, currentEnd);
if (baselineRange == null)
{
SetQueryStoreComparisonMode(false);
Expand Down
Loading
Loading