query_stats.creation_time is compared against a naive-UTC bound, so PARAMETER_SENSITIVITY is scored against the wrong plan population

[erikdarlingdata]

query_stats.creation_time is compared against a naive-UTC bound, so PARAMETER_SENSITIVITY is scored against the wrong plan population on every non-UTC server

Confirmed live on the production fleet, not hypothetical. Every production SQL Server target reports utc_offset_minutes = -240 (UTC-4). The only target at 0 is a dev box. So this affects the whole monitored SQL Server fleet across both stores, not one server's configuration quirk.

The defect

Six sites compare a server-local column against a bound derived from DateTime.UtcNow:

• Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.QueryPerf.cs
• Darling/PerformanceMonitor.Darling.Analysis/PgDrillDownCollector.Queries.cs (two sites)
• Lite/Analysis/DuckDbFactCollector.QueryPerf.cs
• Lite/Analysis/DrillDownCollector.Queries.cs (two sites)

Each reads creation_time <= $n, where $n is AnalysisContext.TimeRangeStart — which both analysis services derive from DateTime.UtcNow. query_stats.creation_time is the monitored server's local clock (documented at QueryStatsCollector.cs:167).

Why the predicate exists, and what breaks

The predicate means "this plan was compiled before the analysis window opened." That is what makes a large min/max worker-time spread evidence of parameter sensitivity rather than of a plan that simply has not lived long enough to have seen varied parameters yet. It is the guard that separates a real signal from an artefact of plan age.

At UTC-4 the arithmetic works out as follows. A plan compiled at UTC instant T is stored with creation_time = T - 4h. The predicate therefore admits any plan where T - 4h <= W, i.e. T <= W + 4h, where W is the window start. On the default 4-hour window the window is [W, W + 4h] — so the predicate admits every plan compiled inside the window, which is precisely the population it exists to exclude.

A negative offset is the false-positive direction. Recently-compiled plans are admitted, and their partial-life worker-time spread is scored as if it were full-life variance. So PARAMETER_SENSITIVITY facts and the drill-downs hanging off them have been generated against the wrong population fleet-wide — over-reporting rather than silently suppressing.

For completeness, the other direction is worse but does not apply here: at a positive offset (say UTC+10) the predicate admits only plans compiled at least 10 hours before the window, so on a 4-hour window essentially nothing qualifies and the finding class silently stops being produced altogether. Any deployment east of UTC gets that behaviour instead.

Scope note

AnalysisContext.ServerUtcOffset exists but is TimeSpan.Zero in both live SKUs. The only non-zero assignment in the tree is in the retired Dashboard project, and its semantic is the inverse — it converts a server-local window back to UTC for persistence — so it is not the seam to reuse without deciding what it means here.

The retired Dashboard carries the same shape at Analysis/SqlServerFactCollector.QueryPerf.cs, Analysis/SqlServerDrillDownCollector.Queries.cs and Services/DatabaseService.QueryPerformance.Stats.cs. Out of scope unless that SKU is still shipped.

Not verified

The predicted effect on findings output has not been measured against stored analysis_findings rows — that comparison would confirm the over-reporting empirically and is the obvious next step, but it needs a store query rather than source reading. Nobody has checked whether the finding counts actually look inflated.

The fix direction is a per-server de-skew in SQL from the collected server_properties.utc_offset_minutes, which is the pattern already used elsewhere in this codebase. That is six SQL edits across two dialects and it changes what the detector emits, so it wants live verification rather than a source-only change.

[erikdarlingdata]

Closed by #2992 (064a01656a). Closing by hand — this repository merges to dev, so a closing keyword never fires.

Both dialects now de-skew creation_time from the collected offset before comparing it to the UTC window, verified against real PostgreSQL 17 and real DuckDB 1.5.5 on a five-plan fixture straddling the bound: at −240 the old form selected 5 where the correct answer is 3, at 0 both give 3, and at +600 the old form selected 1.

Two corrections to this issue's own text, both from that measurement:

• The positive-offset case thins the population sharply but does not zero it. I wrote that the finding class would "silently stop being produced altogether"; a plan cached longer than the offset still qualifies, which is why one of the five fixture plans survives at +600. Direction and sign were right, the magnitude was overstated.
• The retired Dashboard does not carry this bug. I listed it as having the same shape. The shape matches, but its AnalyzeAsync derives the window from GetServerClockAsync() (SYSDATETIME()), so it compares server-local to server-local and is correct — which is also precisely why ServerUtcOffset exists there with the inverse contract, making the trap this issue flagged real for a sharper reason than the name.

Nothing asserted the buggy behaviour. Every seeded creation_time in either suite was three days before the window with no collected offset, so all of them were offset-tolerant with a wide margin — the defect was invisible to the existing pins rather than pinned in place. StoreSqlClockDisciplineTests is structurally blind to it: creation_time never appears in a DDL string literal, so it is not in that scan's scraped vocabulary. And no cross-SKU guard existed at all — nothing compared Darling's analysis SQL to Lite's until now.

compile_age_seconds was the tempting seam and is unreachable. The collector computes it for exactly this purpose but deliberately does not store it, so the collected offset is the only available route.

Still not measured

The empirical confirmation this issue asked for has not been run — nobody has re-scored stored findings to show the counts were inflated as predicted. Three read-only queries are written and verified to parse against the real migrated schema; they need a store I cannot reach. The substantive one re-scores every PARAMETER_SENSITIVITY finding from the last 30 days both ways over that finding's own window and buckets the result. Prediction at a negative offset: new <= old everywhere, with a non-zero would-stop-firing count.