Take the default trace's archival-empty cutoff from the server's clock

[erikdarlingdata]

default_trace_events.event_time is the monitored server's LOCAL wall clock — the .trc files store local time and ft.StartTime ships verbatim — but the collector's archival-empty branch bound its cutoff from the host's context.CollectionTime, which is naive UTC. Two clocks, one comparison, no error.

What it delivers instead of 24 hours

ft.StartTime > @cutoff_time with @cutoff_time = hostUtcNow - 24h. Substituting StartTime = utc + offset:

server offset	window actually read	consequence
UTC	24 h	correct — which is why this survived review and CI
UTC-7	17 h	a 7-hour hole in default-trace history that nothing later refills, because the next cycle's watermark starts above it
UTC+10	34 h	re-reads 10 hours of events already aged into parquet; v_default_trace_events UNIONs hot + parquet with no dedup, so those rows double-count — the exact outcome this bounded window exists to prevent — and recur every archival cycle

Only this branch was ever exposed. The steady-state bound is the watermark, which IS a previously-stored event_time and therefore already in the server's clock; the true-first-run bound is a 1900 sentinel no clock can misread. The archival branch fires only when the hot store has been emptied by retention/archival on a server that has collected before, so it is both the rarest path and the one nobody watches.

The fix, and the precedent it matches

The branch binds NULL and the query's COALESCE(@cutoff_time, DATEADD(HOUR, -24, SYSDATETIME())) derives the floor on the server. Asserting NULL rather than a computed value is what makes it stick: any host clock reintroduced here has to put a value back in that parameter.

JobHistoryCollector already does exactly this, and its remarks say why — "computed server-side against GETDATE(), since run_datetime is the server's LOCAL wall clock", and its ArchivalEmptyFilter is built the same way. That collector's doc also records that it ports the idiom from this one. So the later port got the frame right and the original it was ported from kept the skew — the same shape #2971 found between DarlingAlertReadAdapter and the Lite original it came from.

COALESCE is non-sargable and that costs nothing here: fn_trace_gettable materializes every file it is handed, so no predicate on ft.StartTime pushes down. The class remarks already say so.

Verified

The suites are net10.0-windows and cannot execute on macOS, so the real test sources were compiled into a net10.0 shim harness and run — DefaultTraceEventsCollectorDefinitionTests against the actual PerformanceMonitor.Collectors build, and CollectorTimestampFrameTests against the collector source.

	with the fix	with the host-UTC bound restored
DefaultTraceEventsCollectorDefinitionTests (19 facts)	19 passed	1 failed — BuildQuery_Watermark_AndFirstRunGuard_TrueFirstRunVsArchivalEmpty, reporting the offending 7/8/2026 12:00:00 PM
CollectorTimestampFrameTests (3 facts)	3 passed	1 failed with SYSDATETIME() removed from the cutoff expression

Two of those failures are the point: the updated assertion and the new pin both go red on the pre-fix code rather than passing either way. Assert.Equal in the harness compares collections structurally, mirroring xUnit — the reference-equality default made two unrelated passing pins look like failures until that was corrected.

The new frame pin

CollectorTimestampFrameTests gains DefaultTraceEvents_DerivesEveryCutoffFromTheServersClock: this collector must carry a local clock and no UTC clock, the exact inverse of the memory_pressure_events pin sitting beside it. Both facts stay per-TABLE. There is no store-wide rule to appeal to and no name-keyed one either — system_health.event_time is UTC under the same column name, which is why StoreSqlClockDisciplineTests lists event_time in AmbiguousFrameColumns and cannot judge either.

[claude]

Reviewed the diff (DefaultTraceEventsCollector.cs + both test suites). This looks correct and well-scoped:

• Bug is real and the fix addresses it. ft.StartTime (from fn_trace_gettable) is genuinely server-local, and the archival-empty branch was the only one binding a host-derived value (context.CollectionTime.AddHours(-24), naive UTC) against it. Binding NULL and letting COALESCE(@cutoff_time, DATEADD(HOUR, -24, SYSDATETIME())) resolve server-side is the right shape — confirmed SqlServerTargetProvider.ToSqlParameter and RemoteCollectorService.DefinitionRunner.ToSqlParameter both already map a null CollectorParameter.Value to DBNull.Value for DateTime2, so the COALESCE fallback actually gets exercised rather than binding a literal null datetime.
• Precedent check holds up. JobHistoryCollector.ArchivalEmptyFilter does use the same GETDATE()-relative, server-side-computed pattern for the same reason (run_datetime is server-local), so this isn't a novel idiom being introduced.
• No Lite/Darling parity drift. DefaultTraceEventsCollector lives in the shared PerformanceMonitor.Collectors project referenced by both PerformanceMonitorLite.csproj and PerformanceMonitor.Darling.Service.csproj, so the fix applies identically to both SKUs from one source. Both test suites were updated in lockstep (Lite.Tests/DefaultTraceEventsCollectorDefinitionTests.cs for the rendered SQL/parameter binding, Darling/Darling.Tests/CollectorTimestampFrameTests.cs for the source-level local-vs-UTC-clock pin), and I didn't find any other test still asserting the old ft.StartTime > @cutoff_time text.
• Format-string substitution is safe. CutoffExpression is formatted once (24 → literal), then spliced into QueryTemplateFormat as a plain positional argument — it isn't re-parsed for {} placeholders, so there's no double-substitution risk even though the outer template also uses {0}/{1}.
• Style: existing AS aliasing, column = expr aliasing, and OPTION(RECOMPILE) are all untouched/unaffected, consistent with CONTRIBUTING.md.

No correctness, parity, security, or performance issues found. Nice catch, and the added CollectorTimestampFrameTests case is a good permanent guard against this regression pattern recurring on other server-local-clock columns.