Deprecated Dashboard analysis perfmon reads divide by the measured interval (#3561)

[erikdarlingdata]

Fixes #3561

Mirror of #3560 (Lite + Darling) into the deprecated Dashboard's frozen analysis twins. cntr_value_delta spans one collection interval, not one second, so the raw reads overstated by the cadence (60x at 60s, 300x at 5min) against thresholds defined in requests/sec. All three reads move together — the consistency constraint is the whole fix:

• SqlServerFactCollector.Resources.cs — the PERFMON_*_SEC facts divide the delta by the row's measured sample_interval_seconds; interval <= 0 rows (unknowable delta: first sighting, reset, gap) are filtered inside the CTE so rn = 1 lands on the newest usable row. A counter with only interval-0 rows emits no fact, never 0. The raw delta and the divisor ride the metadata, and a C# guard keeps a raw or zero value from escaping if the SQL filter ever regresses.
• SqlServerAnomalyDetector.cs — the batch-request window AVG/MAX divide by NULLIF(sample_interval_seconds, 0) with sample_interval_seconds > 0 filtered, keeping the window statistic in the same requests/sec unit as the BatchRequestFloor/BatchRequestFallback bars (both defined in requests/sec).
• SqlServerBaselineProvider.cs — the batch_requests baseline arm computes its population as the per-second rate (AVG(v)/STDEV(v)); the restart signature stays on the raw delta (its > 1000 bar predates the division and marks a reset regardless of cadence), exactly as in Lite's Perfmon analysis reads divide the per-interval delta by its measured interval #3560 treatment.

Test seam: the window and perfmon SQL move to public const fields (the Darling twin's #3560-tested shape, and Dashboard.Tests' existing FailedJobsQuery.Sql pin idiom) and GetBaselineQuery goes internal (InternalsVisibleTo) — the Dashboard has no test database, so the new PerfmonPerSecondSqlTests pins the SQL text of all three reads, including the absence of the raw AVG(cntr_value_delta)/MAX(cntr_value_delta)/STDEV(cntr_value_delta) defect shapes.

Testing

• dotnet build deprecated/Dashboard/Dashboard.csproj — Build succeeded, 0 Warning(s)
• dotnet build deprecated/Dashboard.Tests/Dashboard.Tests.csproj — Build succeeded, 0 Warning(s)
• Full Dashboard.Tests unit suite (MTP exe): 788 passed, 0 failed (includes the 4 new pins)
• Semantics verified live on SQL Server 2022 via a temp-table clone of collect.perfmon_stats seeded with the Perfmon analysis reads divide the per-interval delta by its measured interval #3560 fixture: fact read returns the newest usable row (interval-0 skipped, only-interval-0 counter absent), window read yields per-second avg/peak (100.00 from delta 6000 over 60s), baseline arm yields mean 100.00 with the restart zero excluded and interval-0 rows skipped.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QMYFB4qv5tqGwMmdPsa4Zx

[claude]

Non-blocking observation, out of scope for this PR: collect.perfmon_stats.cntr_value_per_second (the computed column defined in install/02_create_tables.sql, cntr_value_delta / NULLIF(sample_interval_seconds, 0)) does integer division — both operands are integral types, so it truncates fractional per-second rates (e.g. 150/60 → 2, not 2.5). This PR correctly avoids it by using * 1.0 / for float division in all three reads it touches, but DatabaseService.ResourceMetrics.Perfmon.cs (lines 48 and 126) still selects that computed column directly for the perfmon chart UI, so that read path still carries a truncated-rate defect distinct from the #3527/#3561 cadence bug this PR fixes. Worth a follow-up issue if not already tracked — not asking for it here since it's an unrelated file/read path.

[claude]

Reviewed against CONTRIBUTING.md and cross-checked against the Lite/Darling twins (#3560) this PR mirrors.

Correctness — verified line-by-line against PgFactCollector.Resources.cs, PgAnomalyDetector.cs, and PgBaselineProvider.cs:

• PerfmonSql: interval filter sits inside the latest CTE before rn = 1, so a counter with only interval≤0 rows correctly emits no fact rather than a 0. The C# if (intervalSeconds <= 0) continue; guard matches Darling's defense-in-depth exactly.
• BatchRequestWindowSql: AVG/MAX divide by NULLIF(sample_interval_seconds, 0) with sample_interval_seconds > 0 also in the WHERE, matching Darling's BatchRequestWindowSql field-for-field.
• Baseline BatchRequests arm: LAG(cntr_value_delta) is computed inside the same sample_interval_seconds > 0-filtered CTE the outer query reads from, which reproduces Lite's QUALIFY-over-filtered-rowset semantics correctly (QUALIFY evaluates after the window function over the WHERE-surviving rows) — the rewrite from QUALIFY to an explicit outer WHERE NOT (...) preserves "first zero after a >1000 sample only" behavior. The restart signature intentionally stays on the raw delta, which is correct per the PR description (the >1000 bar predates the division and marks a reset regardless of cadence).
• All divisions in SQL use * 1.0 / (float division); the C# division casts to double. No integer-truncation or divide-by-zero paths.

Parity — this is the deprecated Full Dashboard, not Lite or Darling, so the strict Lite/Darling parity requirement doesn't directly apply. That said, the PR explicitly positions itself as a mirror of #3560, and I confirmed the SQL and C# shapes do faithfully mirror the Darling implementation (down to the sample_interval_seconds metadata key and the intervalSeconds <= 0 guard placement).

Style — new comments explain why at length per convention, block-comment style is followed. OPTION(RECOMPILE) isn't present on these reads, but that matches every other query in this file already (none of the file's existing analysis reads use it), so this isn't a regression introduced by this PR.

Tests — PerfmonPerSecondSqlTests.cs pins the SQL text for all three reads, including the absence of the raw (undivided) AVG/MAX/STDEV shapes, which is a good regression guard given the Dashboard has no test database. GetBaselineQuery → internal is safe; InternalsVisibleTo Dashboard.Tests already existed in the csproj.

Left one non-blocking inline comment about an unrelated, pre-existing integer-division truncation bug in the cntr_value_per_second computed column (used by a different, untouched read path) — flagging for awareness, not asking for a fix here.

[claude]

LGTM — verified the three reads' SQL and C# division/interval-filtering logic against the Lite/Darling (#3560) twins line-by-line, confirmed the new SQL-text pin tests are correct, and checked for parity/style issues. No substantive findings; left one non-blocking inline note about a pre-existing, unrelated truncation bug in a different read path.