Repository navigation
Darling viewer: Server time mode follows the server's time zone across a daylight saving change (#4766) - #4783
Merged
Merged
Conversation
…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.
erikdarlingdata
marked this pull request as ready for review
September 29, 2026 10:39
This was referenced Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_propertiesrow 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 toBaselineLocalClock).ServerClock.Resolve(timeZoneId, utcOffsetMinutes)picks the zone id when it resolves, else the fixed offset, else UTC. It shares ONE zone lookup withBaselineLocalClock(TryFindZoneis nowinternal). Members:ToServerLocal(utc),ToUtc(serverLocal),OffsetMinutesAt(utc). It does not reuseLocalClockWindow, which holds one transition where a viewer range can cross two.ToUtcnever 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 aServerClock(ActiveServerClock, with newServerClockoverloads 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.UtcOffsetMinutesstill works: its setter installs a fixed-offset clock.ViewerDataService.GetServerClockAsyncreadstime_zone_idwithutc_offset_minutesfrom the same newest row (ServerClockSql). An older schema without thetime_zone_idcolumn (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-tabEnsureServerOffsetLoadedAsync). A failed read or nothing collected yet keeps the clock the tab already has.run_datetimeandMAX(run_datetime)for the last successful run, and pre-filters the lower bound with the latest offset widened by one hour.GetJobHistoryAsyncreads each server's clock (GetServerClocksAsync, with the same 42703 fallback), converts each time withToUtc, andApplyJobHistoryWindowthen 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.DefaultTraceEventsByWindowSqlreturns the raw server-localevent_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 thecollection_time >= $5floor 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).GetDefaultTraceEventsAsyncreads the server's clock (GetServerClocksAsync, UTC when absent), and the newReadDefaultTraceEventsAsyncconverts each event withToUtc, 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).DefaultTraceEventRowgainsEventTimeUtc;EventTimeLocalrenders as before.GetAgentStatusAsyncno longer joins the latest offset. The SQL (BuildAgentStatusSql) returnsnext_scheduled_runraw asnext_scheduled_run_local, andReadAgentStatusRowsAsyncconverts 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;ReadJobHistoryRowand 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-sideDarlingDefaultTraceReadernote and theDarlingMcpDefaultTraceToolsTestsnote that pointed at the viewer's SQL).ViewerSystemEventsTests.DefaultTraceEventsByWindowSql_...now pins the rawevent_time_localprojection, the two hour-wide bounds andORDER BY event_time_local DESCinstead of the three subtracted-offset expressions.ServerLocalReadFrameDisciplineTestsexpects the viewer's System Events file to carry two offset-expression sites (the pre-filter bounds) instead of three, exactly one baredte.event_time(the raw projection), and the C# conversionclock.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: thejob_statswindow 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.Testsbuilds 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 drivesForDisplayand the inverse;ServerClockSqlpin.ViewerJobHistoryServerClockTests: a job run stored at server-local time on each side of a change lands at the right UTC time (read throughReadJobHistoryRowfrom aDataTablereader); 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.ViewerDefaultTraceServerClockTests(9), pure C# over aDataTablereader throughReadDefaultTraceEventsAsync: 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.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 returnsnext_scheduled_runraw with nomake_interval, offset join or_utcalias, in both the fleet and single-server shapes.ServerClock.Resolvetemporarily made to ignore the zone id and use the fixed offset (not committed, restored,git statusclean of it),ServerClockTests,ViewerServerClockTests,ViewerJobHistoryServerClockTestsand the two new classes ranTotal: 52, Errors: 0, Failed: 22. The 22:ServerClockTests8,ViewerServerClockTests6,ViewerJobHistoryServerClockTests2,ViewerDefaultTraceServerClockTests4 (each side of a change, skipped and repeated hours, both window-edge tests),ViewerAgentStatusServerClockTests2 (far side of a change, skipped and repeated hours).origin/dev:Darling.Testsbuilds with 0 warnings; the named classes (ViewerSystemEventsTests,ViewerCommandTimeoutTests,ViewerTimeHelperTests, both new classes,ServerClockTests,ViewerServerClockTests,ViewerJobHistoryServerClockTests, plusServerLocalReadFrameDisciplineTests,EventWindowedReadsCarryTheFloorTests,DatabaseFilterTests,DarlingMcpDefaultTraceToolsTests) ranTotal: 244, Failed: 0.Darling.Testsrun 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.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 subtractedevent_timeare equal). That is unverified.Not done in this PR
Lite/Services/ServerTimeHelper.cs) follows in a separate PR.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:
REF:
[Darling viewer: Server time mode follows the server's time zone across a daylight saving change (#4766) #4783]: Darling viewer: Server time mode follows the server's time zone across a daylight saving change (#4766) #4783