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

deprecated/Dashboard.Tests/PerfmonPerSecondSqlTests.cs

@@ -0,0 +1,102 @@
+/*
+ * Performance Monitor Dashboard
+ * Copyright (c) 2026 Darling Data, LLC
+ * Licensed under the MIT License - see LICENSE file for details
+ */
+
+using PerformanceMonitorDashboard.Analysis;
+using Xunit;
+
+namespace PerformanceMonitorDashboard.Tests;
+
+/// <summary>
+/// #3527/#3561: cntr_value_delta spans one COLLECTION INTERVAL, not one second — read raw, the
+/// perfmon analysis reads overstate by the cadence (60x at 60s, 300x at 5min). All three reads
+/// must divide by the row's measured sample_interval_seconds and skip interval &lt;= 0 rows
+/// (no delta was knowable: first sighting, reset, gap) — together or the baseline comparison is
+/// cross-unit. The Dashboard has no test database, so these pin the SQL text, the mirror of the
+/// Darling twin's #3560 pins (PgFactCollector.PerfmonSql / PgAnomalyDetector.BatchRequestWindowSql).
+/// </summary>
+public class PerfmonPerSecondSqlTests
+{
+    /* ---------------- read 1: the fact collector ---------------- */
+
+    [Fact]
+    public void PerfmonFactSql_SelectsTheMeasuredInterval_AndFiltersUnknowableRows()
+    {
+        var sql = SqlServerFactCollector.PerfmonSql;
+
+        /* The interval rides both the CTE and the outer select so the C# division has its divisor. */
+        Assert.Contains("sample_interval_seconds,", sql);
+        Assert.Contains("SELECT counter_name, cntr_value, cntr_value_delta, sample_interval_seconds", sql);
+
+        /* Interval <= 0 rows are filtered INSIDE the CTE, so rn = 1 lands on the newest row a rate
+           can honestly be derived from — a counter with only interval-0 rows emits no fact, never 0. */
+        Assert.Contains("AND   sample_interval_seconds > 0", sql);
+        var filterAt = sql.IndexOf("AND   sample_interval_seconds > 0", System.StringComparison.Ordinal);
+        var rnFilterAt = sql.IndexOf("FROM latest WHERE rn = 1", System.StringComparison.Ordinal);
+        Assert.True(filterAt >= 0 && filterAt < rnFilterAt, "the interval filter must sit inside the latest CTE, before rn = 1");
+    }
+
+    /* ---------------- read 2: the anomaly detector's window ---------------- */
+
+    [Fact]
+    public void BatchRequestWindow_DividesByMeasuredInterval_AndSkipsUnknowableRows()
+    {
+        var sql = SqlServerAnomalyDetector.BatchRequestWindowSql;
+
+        Assert.Contains("AVG(cntr_value_delta * 1.0 / NULLIF(sample_interval_seconds, 0))", sql);
+        Assert.Contains("MAX(cntr_value_delta * 1.0 / NULLIF(sample_interval_seconds, 0))", sql);
+        Assert.Contains("sample_interval_seconds > 0", sql);
+
+        /* A raw AVG/MAX of the delta is exactly the #3527 defect — pin its absence. */
+        Assert.DoesNotContain("AVG(cntr_value_delta)", sql);
+        Assert.DoesNotContain("MAX(cntr_value_delta)", sql);
+    }
+
+    /* ---------------- read 3: the baseline arm ---------------- */
+
+    [Fact]
+    public void BatchRequestBaseline_PopulationIsPerSecond_RestartSignatureStaysRaw()
+    {
+        var sql = SqlServerBaselineProvider.GetBaselineQuery(SqlServerMetricNames.BatchRequests);
+
+        Assert.NotNull(sql);
+
+        /* The baseline population is the per-second rate, in the detector's window unit. */
+        Assert.Contains("cntr_value_delta * 1.0 / NULLIF(sample_interval_seconds, 0) AS v", sql);
+        Assert.Contains("AVG(v) AS mean_val", sql);
+        Assert.Contains("STDEV(v) AS stddev_val", sql);
+        Assert.Contains("AND   sample_interval_seconds > 0", sql);
+
+        /* The restart signature stays on the RAW delta: its > 1000 bar predates the division and
+           marks a counter reset regardless of cadence. */
+        Assert.Contains("LAG(cntr_value_delta) OVER (ORDER BY collection_time) AS prev_value", sql);
+        Assert.Contains("WHERE NOT (cntr_value_delta = 0 AND ISNULL(prev_value, 0) > 1000)", sql);
+
+        /* A raw AVG/STDEV of the delta is exactly the #3527 defect — pin its absence. */
+        Assert.DoesNotContain("AVG(cntr_value_delta)", sql);
+        Assert.DoesNotContain("STDEV(cntr_value_delta)", sql);
+    }
+
+    /* ---------------- the consistency constraint ---------------- */
+
+    /// <summary>
+    /// The whole fix is the three reads agreeing: the window statistic, the baseline population,
+    /// and the fact value all divide by the measured interval, or the z-score compares across
+    /// units. The fact collector's division happens in C# (delta / sample_interval_seconds after
+    /// the SQL delivers the divisor), so its SQL is pinned for the divisor column and the
+    /// interval filter; the two aggregate reads divide in the SQL itself.
+    /// </summary>
+    [Fact]
+    public void AllThreeReads_FilterUnknowableRows()
+    {
+        var factSql = SqlServerFactCollector.PerfmonSql;
+        var windowSql = SqlServerAnomalyDetector.BatchRequestWindowSql;
+        var baselineSql = SqlServerBaselineProvider.GetBaselineQuery(SqlServerMetricNames.BatchRequests);
+
+        Assert.Contains("sample_interval_seconds > 0", factSql);
+        Assert.Contains("sample_interval_seconds > 0", windowSql);
+        Assert.Contains("sample_interval_seconds > 0", baselineSql);
+    }
+}

deprecated/Dashboard/Analysis/SqlServerAnomalyDetector.cs

@@ -832,6 +832,24 @@ FROM collect.file_io_stats
         }
     }
 
+    // Batch-request window: per-second rate per sample (#3527) — cntr_value_delta spans one collection
+    // interval, so divide by the row's MEASURED sample_interval_seconds. Interval <= 0 marks an
+    // unknowable delta (first sighting/reset/gap) and the row is skipped, never read as 0. Keeps the
+    // window statistic in the same requests/sec unit as the baseline and the
+    // BatchRequestFloor/Fallback thresholds.
+    public const string BatchRequestWindowSql = @"
+SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+
+SELECT
+    AVG(cntr_value_delta * 1.0 / NULLIF(sample_interval_seconds, 0)) AS avg_batch,
+    MAX(cntr_value_delta * 1.0 / NULLIF(sample_interval_seconds, 0)) AS peak_batch,
+    COUNT(*) AS sample_count
+FROM collect.perfmon_stats
+WHERE collection_time >= @windowStart AND collection_time <= @windowEnd
+AND   counter_name = 'Batch Requests/sec'
+AND   cntr_value_delta >= 0
+AND   sample_interval_seconds > 0;";
+
     /// <summary>
     /// Detects batch requests/sec anomalies using z-score against time-bucketed baseline.
     /// </summary>
@@ -849,17 +867,7 @@ private async Task DetectBatchRequestAnomalies(AnalysisContext context, List<Fac
             await connection.OpenAsync();
 
             using var cmd = connection.CreateCommand();
-            cmd.CommandText = @"
-SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
-
-SELECT
-    AVG(cntr_value_delta) AS avg_batch,
-    MAX(cntr_value_delta) AS peak_batch,
-    COUNT(*) AS sample_count
-FROM collect.perfmon_stats
-WHERE collection_time >= @windowStart AND collection_time <= @windowEnd
-AND   counter_name = 'Batch Requests/sec'
-AND   cntr_value_delta >= 0;";
+            cmd.CommandText = BatchRequestWindowSql;
 
             cmd.Parameters.Add(new SqlParameter("@windowStart", context.TimeRangeStart));
             cmd.Parameters.Add(new SqlParameter("@windowEnd", context.TimeRangeEnd));

deprecated/Dashboard/Analysis/SqlServerBaselineProvider.cs

@@ -190,7 +190,8 @@ public async Task<BaselineBucket> GetBaselineAsync(string metricName, DateTime a
         }
     }
 
-    private static string? GetBaselineQuery(string metricName)
+    // Internal for Dashboard.Tests (InternalsVisibleTo): the #3527 per-second pins read the arm SQL.
+    internal static string? GetBaselineQuery(string metricName)
     {
         // All queries return: hour_of_day, day_of_week, mean_val, stddev_val, sample_count
         // Day-of-week normalization: (DATEPART(weekday, x) + @@DATEFIRST - 1) % 7 gives Sunday=0
@@ -217,21 +218,28 @@ GROUP BY DATEPART(HOUR, collection_time),
             // Cumulative counter — restart exclusion via CTE with LAG.
             // server_start_time is inline in collect.perfmon_stats.
             // Exclude samples within 5 min of a detected restart.
+            // #3527: v is the per-second rate — the per-interval delta divided by the row's measured
+            // sample_interval_seconds — so the baseline population is in the same requests/sec unit as
+            // the detector's window statistic. Interval <= 0 rows (unknowable delta) are skipped, never
+            // read as 0. The restart signature stays on the RAW delta: its > 1000 bar predates the
+            // division and marks a counter reset regardless of cadence.
             SqlServerMetricNames.BatchRequests => @"
 SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
 
 ;WITH filtered AS (
     SELECT collection_time, cntr_value_delta,
+           cntr_value_delta * 1.0 / NULLIF(sample_interval_seconds, 0) AS v,
            LAG(cntr_value_delta) OVER (ORDER BY collection_time) AS prev_value
     FROM collect.perfmon_stats
     WHERE collection_time >= @windowStart AND collection_time < @windowEnd
     AND   counter_name = 'Batch Requests/sec'
     AND   cntr_value_delta >= 0
+    AND   sample_interval_seconds > 0
 )
 SELECT DATEPART(HOUR, collection_time) AS hour_of_day,
        (DATEPART(WEEKDAY, collection_time) + @@DATEFIRST - 1) % 7 AS day_of_week,
-       AVG(cntr_value_delta) AS mean_val,
-       STDEV(cntr_value_delta) AS stddev_val,
+       AVG(v) AS mean_val,
+       STDEV(v) AS stddev_val,
        COUNT(*) AS sample_count,
        COUNT(DISTINCT CAST(collection_time AS DATE)) AS distinct_days
 FROM filtered

deprecated/Dashboard/Analysis/SqlServerFactCollector.Resources.cs

@@ -197,35 +197,46 @@ FROM collect.memory_grant_stats
         }
     }
 
-    /// <summary>
-    /// Collects key perfmon throughput counters: Batch Requests/sec, compilations, recompilations.
-    /// Unscored context that distinguishes a busy server from a sick one (used by the AI surfaces).
-    /// </summary>
-    private async Task CollectPerfmonFactsAsync(AnalysisContext context, List<Fact> facts)
-    {
-        try
-        {
-            using var connection = new SqlConnection(_connectionString);
-            await connection.OpenAsync();
-
-            using var cmd = connection.CreateCommand();
-            cmd.CommandText = @"
+    // #3527: cntr_value_delta spans one COLLECTION INTERVAL, not one second — at a 60s cadence
+    // the raw delta is 60x the true rate. The honest rate divides by the row's MEASURED
+    // sample_interval_seconds; interval <= 0 marks an unknowable delta (first sighting, counter
+    // reset, gap), so those rows are filtered rather than emitted as 0 — rn = 1 lands on the
+    // newest row a rate can honestly be derived from.
+    public const string PerfmonSql = @"
 SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
 
 ;WITH latest AS (
     SELECT
         counter_name,
         cntr_value,
         cntr_value_delta,
+        sample_interval_seconds,
         ROW_NUMBER() OVER (PARTITION BY counter_name ORDER BY collection_time DESC) AS rn
     FROM collect.perfmon_stats
     WHERE collection_time >= @startTime
     AND   collection_time <= @endTime
     AND   counter_name IN ('Batch Requests/sec', 'SQL Compilations/sec', 'SQL Re-Compilations/sec')
+    AND   sample_interval_seconds > 0
 )
-SELECT counter_name, cntr_value, cntr_value_delta
+SELECT counter_name, cntr_value, cntr_value_delta, sample_interval_seconds
 FROM latest WHERE rn = 1";
 
+    /// <summary>
+    /// Collects key perfmon throughput counters: Batch Requests/sec, compilations, recompilations.
+    /// Unscored context that distinguishes a busy server from a sick one (used by the AI surfaces).
+    /// Fact values are per-second rates: the per-interval delta divided by the row's measured
+    /// sample_interval_seconds (#3527); the raw delta and the divisor ride the metadata.
+    /// </summary>
+    private async Task CollectPerfmonFactsAsync(AnalysisContext context, List<Fact> facts)
+    {
+        try
+        {
+            using var connection = new SqlConnection(_connectionString);
+            await connection.OpenAsync();
+
+            using var cmd = connection.CreateCommand();
+            cmd.CommandText = PerfmonSql;
+
             cmd.Parameters.Add(new SqlParameter("@startTime", context.TimeRangeStart));
             cmd.Parameters.Add(new SqlParameter("@endTime", context.TimeRangeEnd));
 
@@ -235,6 +246,7 @@ AND   counter_name IN ('Batch Requests/sec', 'SQL Compilations/sec', 'SQL Re-Com
                 var counterName = reader.GetString(0);
                 var cntrValue = reader.IsDBNull(1) ? 0L : Convert.ToInt64(reader.GetValue(1));
                 var deltaValue = reader.IsDBNull(2) ? 0L : Convert.ToInt64(reader.GetValue(2));
+                var intervalSeconds = reader.IsDBNull(3) ? 0L : Convert.ToInt64(reader.GetValue(3));
 
                 var (factKey, source) = counterName switch
                 {
@@ -246,8 +258,11 @@ AND   counter_name IN ('Batch Requests/sec', 'SQL Compilations/sec', 'SQL Re-Com
 
                 if (factKey == null) continue;
 
-                // All remaining counters are per-second rates — use the delta.
-                var value = (double)deltaValue;
+                // The delta spans one collection interval — divide by the measured interval for the
+                // per-second rate (#3527). The SQL already filters interval <= 0 (unknowable delta);
+                // this guard keeps a raw or zero value from ever escaping if that filter regresses.
+                if (intervalSeconds <= 0) continue;
+                var value = deltaValue / (double)intervalSeconds;
 
                 facts.Add(new Fact
                 {
@@ -258,7 +273,8 @@ AND   counter_name IN ('Batch Requests/sec', 'SQL Compilations/sec', 'SQL Re-Com
                     Metadata = new Dictionary<string, double>
                     {
                         ["cntr_value"] = cntrValue,
-                        ["delta_cntr_value"] = deltaValue
+                        ["delta_cntr_value"] = deltaValue,
+                        ["sample_interval_seconds"] = intervalSeconds
                     }
                 });
             }