Skip to content

Darling viewer: Server time mode follows the server's time zone across a daylight saving change (#4766) - #4783

Merged
erikdarlingdata merged 3 commits into
devfrom
fix/4766-server-clock-dst
Sep 29, 2026
Merged

erikdarlingdata merged 3 commits into
devfrom
fix/4766-server-clock-dst

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Part of #4766 (the Darling viewer; Lite follows in a separate PR)

Why

In Server time mode (the default) the Darling viewer added one UTC offset to every time. That offset came from the newest collected server_properties row and was cached for the life of the tab, and the viewer never read the server's time zone id. For a server in a zone with daylight saving time, chart times and picker ranges on the far side of a change were an hour off the server's wall clock. Job History, System Events (the Default Trace) and the Agent status next run also subtracted the latest offset in SQL, which is an hour off across a change in UTC and Local modes, and a System Events window whose edge sat within an hour of a change could keep or drop the wrong events.

What changes

  • PerformanceMonitor.Analysis/Baselines/ServerClock.cs (new, next to BaselineLocalClock). ServerClock.Resolve(timeZoneId, utcOffsetMinutes) picks the zone id when it resolves, else the fixed offset, else UTC. It shares ONE zone lookup with BaselineLocalClock (TryFindZone is now internal). Members: ToServerLocal(utc), ToUtc(serverLocal), OffsetMinutesAt(utc). It does not reuse LocalClockWindow, which holds one transition where a viewer range can cross two.
  • ToUtc never throws. A skipped local time (spring forward) moves forward by the gap, so 02:30 on the US change day is 03:30 daylight time (07:30 UTC). A repeated local time (fall back) takes the first occurrence, the daylight offset (01:30 is 05:30 UTC). The ends of the calendar clamp instead of throwing.
  • ViewerTimeHelper: Server mode and its inverse convert through a ServerClock (ActiveServerClock, with new ServerClock overloads next to the existing fixed-offset overloads, which delegate to them). Local and UTC modes are unchanged for stored UTC values; a server-clock column shown in UTC or Local mode now converts through the zone too. A server-clock column shown in Server mode is still shown as stored. UtcOffsetMinutes still works: its setter installs a fixed-offset clock.
  • ViewerDataService.GetServerClockAsync reads time_zone_id with utc_offset_minutes from the same newest row (ServerClockSql). An older schema without the time_zone_id column (42703) falls back to the offset-only read this method did before.
  • ViewerServerTab: the clock is read again on every refresh (RefreshServerClockAsync, replacing the once-per-tab EnsureServerOffsetLoadedAsync). A failed read or nothing collected yet keeps the clock the tab already has.
  • Job History: the SQL now returns the stored server-local run_datetime and MAX(run_datetime) for the last successful run, and pre-filters the lower bound with the latest offset widened by one hour. GetJobHistoryAsync reads each server's clock (GetServerClocksAsync, with the same 42703 fallback), converts each time with ToUtc, and ApplyJobHistoryWindow then filters the exact window, orders newest first by real UTC time and trims to the limit. Only a lower bound exists here, so only that side is widened. The cross-server row limit inside the SQL still orders by the latest offset (a sort key only); the per-job average and last-success window keep the previous unwidened filter, so a fixed-offset server returns exactly what it did before.
  • System Events (Default Trace): DefaultTraceEventsByWindowSql returns the raw server-local event_time (event_time_local) and keeps the latest-offset expression only as a pre-filter, widened by one hour on both sides (>= $2 - interval '1 hour', <= $3 + interval '1 hour'). The read has no LIMIT, so the widening drops nothing, and the collection_time >= $5 floor stays on the caller's start (a truly qualifying event was collected at or after its real UTC time, so the floor cannot drop it). GetDefaultTraceEventsAsync reads the server's clock (GetServerClocksAsync, UTC when absent), and the new ReadDefaultTraceEventsAsync converts each event with ToUtc, keeps only events inside [startUtc, endUtc] exactly, applies the existing significance gate, and returns them newest first with a stable sort (ties keep the reader's order, as the SQL had no tie-break). DefaultTraceEventRow gains EventTimeUtc; EventTimeLocal renders as before.
  • Agent status: GetAgentStatusAsync no longer joins the latest offset. The SQL (BuildAgentStatusSql) returns next_scheduled_run raw as next_scheduled_run_local, and ReadAgentStatusRowsAsync converts it with that row's server clock, so a next run on the far side of a change lands at its real UTC time. It was the only server-local time that SELECT shifted.
  • ClockFor(clocks, serverId) is the one place the "no clock means UTC" rule lives; ReadJobHistoryRow and both new reads use it. Doc comments that said the de-skew happens in SQL now say it happens in C# (DefaultTraceEventRow, ViewerJobHistoryRow, ViewerAgentStatusRow, the service-side DarlingDefaultTraceReader note and the DarlingMcpDefaultTraceToolsTests note that pointed at the viewer's SQL).
  • Two pins changed on purpose. ViewerSystemEventsTests.DefaultTraceEventsByWindowSql_... now pins the raw event_time_local projection, the two hour-wide bounds and ORDER BY event_time_local DESC instead of the three subtracted-offset expressions. ServerLocalReadFrameDisciplineTests expects the viewer's System Events file to carry two offset-expression sites (the pre-filter bounds) instead of three, exactly one bare dte.event_time (the raw projection), and the C# conversion clock.ToUtc(reader.GetDateTime(0)); the other four readers keep their counts, and the discriminator test pins the new pattern in both directions.

Known edges: near a change, a row at the row-limit boundary, or a last-success run inside the repeated hour (the SQL takes the maximum of the local time), can differ by up to an hour from the exact answer. A stored server-local time in the repeated hour cannot say which occurrence it was, so it takes the first.

What still subtracts the latest offset in the viewer (git grep -n "make_interval(mins =>" -- Darling/PerformanceMonitor.Darling.Viewer), all pre-filters or approximate ordering, none of it a value shown:

  • ViewerDataService.JobHistory.cs: approx_run_utc, the sort key for the cross-server row limit (the exact order is applied in C#).
  • ViewerDataService.JobHistory.cs: the per-server top-rows pre-filter (run_datetime >= $1 + offset - interval '1 hour').
  • ViewerDataService.JobHistory.cs: the job_stats window for the average successful duration and last success (the previous unwidened filter, noted above).
  • ViewerDataService.SystemEvents.cs: the two Default Trace window pre-filter bounds.

Test plan

  • Darling.Tests builds with 0 warnings.
  • ServerClockTests (18): US Eastern before and after the 2026-11-01 fall back (-4 h then -5 h) and the spring forward; 2026-03-08 02:30 converts to 07:30 UTC without throwing; 2026-11-01 01:30 converts to its first occurrence (05:30 UTC); an id that does not resolve uses the offset; a null or blank id uses the offset; neither gives UTC; a resolvable id wins over a stale offset; a year of hourly round trips; calendar ends clamp.
  • ViewerServerClockTests: Server mode shows the wall clock on both sides of a change; a picker range across the change converts back to 12:00 and 13:00 UTC; a skipped picker time does not throw; Local and UTC modes do not depend on the clock; the process-wide clock drives ForDisplay and the inverse; ServerClockSql pin. ViewerJobHistoryServerClockTests: a job run stored at server-local time on each side of a change lands at the right UTC time (read through ReadJobHistoryRow from a DataTable reader); skipped and repeated hours do not throw; fixed-offset and no-clock servers convert as before; the exact window, order and limit; the SQL shape pins.
  • New ViewerDefaultTraceServerClockTests (9), pure C# over a DataTable reader through ReadDefaultTraceEventsAsync: events stored at server-local time on each side of the spring forward and the fall back land at 13:00, 12:00, 12:00 and 13:00 UTC; skipped and repeated hours do not throw; a window with its edge within an hour of the fall back (05:30 to 07:30 UTC) keeps exactly the 01:45 and 02:10 events and drops 00:59, 01:15 and 02:40; the same for the spring forward (06:30 to 08:30 UTC), whichever side's offset the newest snapshot carries; the window edges are inclusive and the one-hour margin from the SQL is trimmed; rows come back newest first with ties in reader order; a NULL time and an ErrorLog below the severity gate are dropped; fixed-offset and no-clock servers convert as before; the other columns come through.
  • New ViewerAgentStatusServerClockTests (5): a next run on the far side of a change converts to 13:00 UTC (winter, snapshot taken in summer) and 12:00 UTC (summer, snapshot taken in winter); each row uses its own server's clock, a server with no clock reads as UTC, and no next run stays empty; skipped and repeated hours do not throw; the SQL returns next_scheduled_run raw with no make_interval, offset join or _utc alias, in both the fleet and single-server shapes.
  • Fails on the old behavior: with ServerClock.Resolve temporarily made to ignore the zone id and use the fixed offset (not committed, restored, git status clean of it), ServerClockTests, ViewerServerClockTests, ViewerJobHistoryServerClockTests and the two new classes ran Total: 52, Errors: 0, Failed: 22. The 22: ServerClockTests 8, ViewerServerClockTests 6, ViewerJobHistoryServerClockTests 2, ViewerDefaultTraceServerClockTests 4 (each side of a change, skipped and repeated hours, both window-edge tests), ViewerAgentStatusServerClockTests 2 (far side of a change, skipped and repeated hours).
  • After merging origin/dev: Darling.Tests builds with 0 warnings; the named classes (ViewerSystemEventsTests, ViewerCommandTimeoutTests, ViewerTimeHelperTests, both new classes, ServerClockTests, ViewerServerClockTests, ViewerJobHistoryServerClockTests, plus ServerLocalReadFrameDisciplineTests, EventWindowedReadsCarryTheFloorTests, DatabaseFilterTests, DarlingMcpDefaultTraceToolsTests) ran Total: 244, Failed: 0.
  • Full Darling.Tests run once, without a test database, after the merge: Total: 17343, Errors: 0, Failed: 0, Skipped: 1137, Not Run: 1 (the skipped tests are the live PostgreSQL ones). All touched files keep their CRLF line endings.
  • Live PostgreSQL tests, including the Job History and Default Trace ones (JobHistoryPerServerTopNLiveTests, JobHistoryGroupedStatsMatchWindowFunctionLiveTests, EventWindowedReadsAreBoundedLivePostgresTests): no test database was available, so CI's PostgreSQL job is the first run. Read through, they should hold: they compare the previous SQL with the new one at fixed or zero offsets, and no seeded row falls in the extra hour outside the window (the Default Trace comparison seeds no offset row, so the raw and the subtracted event_time are equal). That is unverified.

Not done in this PR

  • Lite (Lite/Services/ServerTimeHelper.cs) follows in a separate PR.
  • The service-side readers (Darling.Service/Mcp/*, the analysis trace-anchor and drill-down reads) still subtract the latest offset in SQL. They are not part of the viewer change.

CHANGELOG

SECTION: Fixed
ENTRY:

…s a daylight saving change (part of #4766)

Server time mode added one UTC offset to every time, so a chart time or a picker range on the far side of a daylight saving change was an hour off the server's wall clock. A new ServerClock (next to BaselineLocalClock, sharing its zone lookup) converts naive UTC to the server's wall clock and back with the time zone id where it resolves, else the fixed offset, else UTC. The reverse conversion never throws: a skipped local time moves forward by the gap and a repeated one takes the first occurrence.

The viewer now reads time_zone_id with the offset from the same newest server_properties row (falling back to the offset alone on a store without the column), converts through the clock in Server mode and its inverse, and reads the clock again on every refresh instead of once per tab. Job History converts each stored server-local run time in C# with the clock: the SQL window is widened by an hour and the exact window and newest-first order are applied after the conversion.
…follow the server's time zone across a daylight saving change (part of #4766)

The Default Trace events and the SQL Agent next scheduled run are stored on the monitored server's own wall clock, and the viewer subtracted the newest collected UTC offset in SQL. That is an hour off for any event or run on the far side of a daylight saving change from the newest snapshot, and a window whose edge sat within an hour of the change kept or dropped the wrong events.

Both reads now return the server-local time raw and convert it in C# with the server's ServerClock (its time zone where SQL Server reports one, else the collected offset, else UTC). The Default Trace SQL keeps the latest-offset expression only as a pre-filter widened by an hour on both sides, the window is applied exactly after the conversion, and the rows come back newest first by a stable sort. The Agent status SQL drops its offset join.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant