Default trace: only strip rollover suffixes in the filename (#1636)

CHANGELOG.md

@@ -26,6 +26,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Lite + Darling: Linux hosts on SQL Server 2025 CU1+ get real other-process CPU again** ([#1630]) - the [#1048] guard NULLed `other_process_cpu_utilization` on EVERY Linux host, because the SCHEDULER_MONITOR ring buffer always reported `SystemIdle = 0` on Linux. SQL Server 2025 CU1 fixes the ring-buffer metrics (KB5078298), so the blanket guard was discarding a derivable value there forever. The guard now keys on the actual condition (`@is_linux = 1 AND system_idle = 0`) instead of the platform, so older Linux builds keep their honest NULL and 2025 CU1+ derives the real figure - version-proof by construction. Community fix by @argpna ([#1629]); applied to the shared collector (Lite + Darling) and the deprecated Dashboard's collection proc + NOC health query.
+- **Lite + Darling: default trace collection works on SQL Server on Linux** ([#1633]) - the rollover-file path normalization assumed the trace file always carries a `_N` suffix (true on Windows); the initial file on Linux is plain `log.trc`, so the strip re-appended the extension onto the full path (`log.trc.trc`), `sys.fn_trace_gettable` raised Msg 19049 on the nonexistent file, and NO default-trace events were ever collected on Linux. The strip now falls back to the base path when there is no suffix to remove. Community fix by @argpna ([#1632]), live-verified against a 2025 Linux container; shared collector + the deprecated Dashboard's proc.
+- **Full Dashboard (deprecated): the `report.trace_flag_changes` view no longer fails with Msg 245 on a trace-flag toggle** ([#1637]) - the view compared the bit-typed `previous_status`/`status` columns against `N'OFF'`/`N'ON'` literals; bit outranks nvarchar in type precedence, so SQL Server converted the literal to bit and the view failed the moment a toggle row flowed through it. It now compares `0`/`1`. Community fix by @argpna ([#1635]).
+- **Default trace: the rollover strip no longer mangles paths whose DIRECTORY contains an underscore** ([#1636]) - follow-up to [#1633]: the strip keyed on the LAST underscore anywhere in the path, so a relocated trace in an underscore-named directory with an unsuffixed file (`/var/opt/my_sql/log/log.trc`) was cut at the directory underscore (`/var/opt/my.trc` - Msg 19049 again). An underscore now only counts as a rollover suffix when it sits after the last path separator (both separator families, since targets are Windows and Linux); truth-tabled live against SQL 2022 across nine path shapes.
 - **Lite + Darling: Azure SQL DB collection works again when the client is firewalled out at the logical server but allowed at the DATABASE level** ([#1634]) - reported against 3.2.0 by a user monitoring an Elastic Pool database ([#1631]): every database-scoped collector failed with `40615 - Cannot open server ... Client with IP address ... is not allowed to access the server`, and the long-query XE session could not enumerate databases, even though the database itself was fully reachable from SSMS. **This is a regression of the [#857] single-database fallback, introduced by [#1506].** That fix correctly stopped reading 40615 ("client IP not allowed at the logical server") as a statement about a login's RIGHTS to read `master` - it is a reachability error, and misreading it had permanently wedged a different user whose public IP rotated daily. But it also dropped 40615 from the single-database FALLBACK path, on the stated premise that "falling back to a user database is futile because the same rule blocks that connection too." That premise is wrong: Azure SQL Database evaluates **database-level** IP firewall rules BEFORE server-level ones, and a client whose IP matches a database-level rule (`sp_set_database_firewall_rule`) is granted a connection to that database with **no** server-level rule permitting it - while `master`, where server-level rules live, still requires one. "Blocked at the server, allowed at the database" is therefore a real, supported configuration that Microsoft explicitly recommends ("use database-level IP firewall rules whenever possible"), and it is precisely the [#857] case. The two questions - *"does this login lack rights to master?"* and *"should database-scoped collection degrade to single-database?"* - are now separated in the shared `SqlErrorClassification`: the rights list is unchanged (40615 still correctly excluded, so every [#1506] invariant holds), and a new fallback list is that set plus 40615, which is what both SKUs' Azure database-enumeration catch sites actually ask. Acting on it is safe because [#1506]'s other fix survives untouched: the resulting verdict is a revocable throttle, not a latch - it expires after 15 minutes, is discarded when a server returns from an outage, and is skipped entirely when there is no target database to fall back to. A client genuinely firewalled out with no database-level rule therefore just fails its fallback attempt with the same 40615, logs an ordinary collector error, and recovers on the next successful connect. Pinned from both suites, independently: 40615 is still not a rights denial, AND it does trigger the fallback; the rights set remains a strict subset of the fallback set; and the disjointness invariant is now asserted against the broader fallback set (40613 stays transient-and-retryable, never a reason to degrade). Lite + Darling.
 - **Darling: collectors no longer fail with `22021: invalid byte sequence for encoding "UTF8": 0x00`** ([#1614]) - SQL Server NVARCHAR allows embedded NUL characters and query text from `sys.dm_exec_sql_text` sometimes carries them, but Postgres `text` columns reject the byte, so one NUL-laden cached query failed the entire `query_stats` COPY batch every cycle (`DBCC FREEPROCCACHE` never helped because the app re-caches the same query). The Postgres row writer now strips NULs from every collected string - query text, plan XML, deadlock and blocked-process XML included - at the single COPY choke point.
 - **Darling Web: a custom view no longer resets the picked time range / server / filters every 60 seconds** ([#1619]) - the background refresh that keeps the dashboard live rebuilt each view from its declared default, snapping any range/server/filter change back within a minute. A per-view scope memory now survives the re-render, so a picked scope sticks until you change it or hard-reload.
@@ -530,7 +534,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#1626]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1626
 [#1627]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1627
 [#1631]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1631
+[#1629]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1629
+[#1630]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1630
+[#1632]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1632
+[#1633]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1633
 [#1634]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1634
+[#1635]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1635
+[#1636]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1636
+[#1637]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1637
 [#1617]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1617
 [#1621]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1621
 [#1601]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1601

Lite.Tests/CpuUtilizationCollectorDefinitionTests.cs

@@ -46,7 +46,10 @@ public void BuildQuery_RingBuffer_SuppressesOtherCpu_OnlyWhenSystemIdleIsZero()
             CollectorTestContext.Make(s_deltas, isAzureSqlDb: false));
 
         Assert.Contains("WHEN @is_linux = 1 AND x.system_idle = 0", plan.Text, StringComparison.Ordinal);
-        Assert.DoesNotContain("WHEN @is_linux = 1\n", plan.Text, StringComparison.Ordinal);
+
+        /* Normalize line endings first: on a CRLF checkout the template contains "= 1\r\n", which
+           the "\n"-suffixed needle would never match, making this guard vacuously pass. */
+        Assert.DoesNotContain("WHEN @is_linux = 1\n", plan.Text.Replace("\r\n", "\n"), StringComparison.Ordinal);
     }
 
     [Fact]

Lite.Tests/DefaultTraceEventsCollectorDefinitionTests.cs

@@ -91,6 +91,24 @@ nonexistent file. The CASE must fall back to the base t.path when CHARINDEX find
         Assert.Contains("ELSE t.path", text, StringComparison.Ordinal);
     }
 
+    [Fact]
+    public void BuildQuery_RolloverPathStrip_OnlyStripsUnderscoresInTheFilename()
+    {
+        /* A trace DIRECTORY containing an underscore with an unsuffixed file (#1636:
+           /var/opt/my_sql/log/log.trc) must NOT strip — the last underscore has to sit AFTER the
+           last path separator (either family, since targets are Windows and Linux) to count as a
+           rollover suffix. Otherwise LEFT cuts at the directory underscore and produces a
+           nonexistent path (/var/opt/my.trc → Msg 19049). */
+        var text = DefaultTraceEventsCollector.Instance.BuildQuery(MakeContext()).Text;
+
+        Assert.Contains(
+            @"AND (CHARINDEX(N'\', REVERSE(t.path)) = 0 OR CHARINDEX(N'_', REVERSE(t.path)) < CHARINDEX(N'\', REVERSE(t.path)))",
+            text, StringComparison.Ordinal);
+        Assert.Contains(
+            "AND (CHARINDEX(N'/', REVERSE(t.path)) = 0 OR CHARINDEX(N'_', REVERSE(t.path)) < CHARINDEX(N'/', REVERSE(t.path)))",
+            text, StringComparison.Ordinal);
+    }
+
     [Fact]
     public void BuildQuery_CapturesTheCuratedEventSet()
     {

PerformanceMonitor.Collectors/CpuUtilizationCollector.cs

@@ -72,12 +72,13 @@ @start_time datetime2(7),
 
 /* Detect SQL Server on Linux. SystemIdle reports 0 in the SCHEDULER_MONITOR
    ring buffer on some Linux/SQL Server version combos, so 100 - SystemIdle - ProcessUtilization
-   fabricates a host figure that pins total CPU at 100% (Issue #1048). Prior to SQL Server
-   2025 CU1 no DMV exposes true host CPU when that happens, so other_process is stored as NULL
-   there. sys.dm_os_linux_cpu_stats (2025 CU1+) exposes real host CPU jiffies but is a cumulative
-   counter requiring a two-sample delta, not a point-in-time snapshot like SCHEDULER_MONITOR, so
-   it isn't used here. sys.dm_os_host_info is 2017+; referenced via sp_executesql so SQL 2016
-   never binds it (@is_linux = 0). */
+   fabricates a host figure that pins total CPU at 100% (Issue #1048). The ring-buffer metrics
+   fix shipped in SQL Server 2025 CU1 (KB5078298 fix 4796293), so real SystemIdle values start
+   there, not at 2025 RTM. Prior to that no DMV exposes true host CPU when SystemIdle is 0, so
+   other_process is stored as NULL. sys.dm_os_linux_cpu_stats (2025 CU1+) exposes real host CPU
+   time but is a cumulative counter requiring a two-sample delta, not a point-in-time snapshot
+   like SCHEDULER_MONITOR, so it isn't used here. sys.dm_os_host_info is 2017+; referenced via
+   sp_executesql so SQL 2016 never binds it (@is_linux = 0). */
 IF OBJECT_ID(N'sys.dm_os_host_info', N'V') IS NOT NULL
     EXEC sys.sp_executesql
         N'SELECT @linux = CASE WHEN hi.host_platform = N''Linux'' THEN 1 ELSE 0 END FROM sys.dm_os_host_info AS hi;',

PerformanceMonitor.Collectors/DefaultTraceEventsCollector.cs

@@ -114,12 +114,14 @@
 
     /* The curated, config-slice-free event set (see the class remarks). A {0} placeholder is spliced with
        the per-server excluded-database clause on ft.DatabaseName; the rollover-file base-path normalization
-       (strip _NN + re-append the extension) reads every retained file. The strip is guarded by a CHARINDEX
-       check: SQL Server on Linux names the initial trace file without a rollover suffix (log.trc, not
-       log_1.trc), so an unconditional strip finds no underscore, LEFT takes the full path unchanged, and
-       appending the extension back doubles it (log.trc.trc) - fn_trace_gettable then errors on the
-       nonexistent file (Msg 19049) and no events are collected. Falling back to t.path when CHARINDEX
-       returns 0 keeps the Windows behavior (which always has the _N suffix) unchanged. READ UNCOMMITTED
+       (strip _NN + re-append the extension) reads every retained file. The strip only fires when the
+       LAST underscore sits in the FILENAME, i.e. after the last path separator (either family — the
+       collector runs against Windows and Linux targets): SQL Server on Linux names the initial trace
+       file without a rollover suffix (log.trc, not log_1.trc), so an unguarded strip mangles the path
+       (log.trc.trc — #1633), and a trace DIRECTORY containing an underscore would mangle it a second
+       way (/var/opt/my_sql/log/log.trc -> /var/opt/my.trc — #1636); fn_trace_gettable then errors on
+       the nonexistent file (Msg 19049) and no events are collected. Falling back to t.path in both
+       cases keeps the Windows behavior (which always has the _N suffix) unchanged. READ UNCOMMITTED
        like every collector; OPTION(RECOMPILE) because the cutoff selectivity varies wildly between the
        all-history first run and the tiny steady-state windows. */
     private const string QueryTemplate = @"
@@ -152,6 +154,8 @@
 (
     CASE
         WHEN CHARINDEX(N'_', REVERSE(t.path)) > 0
+        AND (CHARINDEX(N'\', REVERSE(t.path)) = 0 OR CHARINDEX(N'_', REVERSE(t.path)) < CHARINDEX(N'\', REVERSE(t.path)))
+        AND (CHARINDEX(N'/', REVERSE(t.path)) = 0 OR CHARINDEX(N'_', REVERSE(t.path)) < CHARINDEX(N'/', REVERSE(t.path)))
         THEN LEFT(t.path, LEN(t.path) - CHARINDEX(N'_', REVERSE(t.path))) + RIGHT(t.path, 4)
         ELSE t.path
     END,
@@ -229,7 +233,7 @@
         var (exclusionClause, exclusionParameters) = BuildNullSafeDatabaseExclusion(context.ExcludedDatabases);
         var exclusionSplice = exclusionClause.Length == 0 ? string.Empty : "\r\n" + exclusionClause;
 
         var text = string.Format(CultureInfo.InvariantCulture, QueryTemplate, exclusionSplice);
 
         /* Cutoff selection (the Dashboard's first-run guard, ported):
              - watermark present            -> steady state, collect newer than it.

deprecated/Dashboard/Services/DatabaseService.NocHealth.cs

@@ -477,14 +477,15 @@ end_time DESC
 
                 /* SystemIdle reports 0 in the SCHEDULER_MONITOR ring buffer on some Linux/SQL
                    Server version combos, so 100 - SystemIdle - ProcessUtilization fabricates a
-                   host figure that pins total CPU at 100% forever (Issue #1048). Prior to SQL
-                   Server 2025 CU1 no DMV exposes true host CPU when that happens, so report
-                   other/host CPU as NULL and let the alert engine fall back to the SQL-only
-                   figure. sys.dm_os_linux_cpu_stats (2025 CU1+) exposes real host CPU jiffies
-                   but is a cumulative counter requiring a two-sample delta, not a point-in-time
-                   snapshot like SCHEDULER_MONITOR, so it isn't used here. sys.dm_os_host_info is
-                   2017+; referenced via sp_executesql so SQL 2016 (no Linux build) never binds it
-                   (@is_linux stays 0). */
+                   host figure that pins total CPU at 100% forever (Issue #1048). The ring-buffer
+                   metrics fix shipped in SQL Server 2025 CU1 (KB5078298 fix 4796293), so real
+                   SystemIdle values start there, not at 2025 RTM. Prior to that no DMV exposes
+                   true host CPU when SystemIdle is 0, so report other/host CPU as NULL and let
+                   the alert engine fall back to the SQL-only figure. sys.dm_os_linux_cpu_stats
+                   (2025 CU1+) exposes real host CPU time but is a cumulative counter requiring a
+                   two-sample delta, not a point-in-time snapshot like SCHEDULER_MONITOR, so it
+                   isn't used here. sys.dm_os_host_info is 2017+; referenced via sp_executesql so
+                   SQL 2016 (no Linux build) never binds it (@is_linux stays 0). */
                 IF OBJECT_ID(N'sys.dm_os_host_info', N'V') IS NOT NULL
                 BEGIN
                     EXEC sys.sp_executesql

install/18_collect_cpu_utilization_stats.sql

@@ -59,12 +59,14 @@ BEGIN
     Detect SQL Server on Linux. On some Linux/SQL Server version combos the
     SCHEDULER_MONITOR ring buffer reports SystemIdle = 0, so 100 - SystemIdle -
     ProcessUtilization fabricates a host figure that pins total CPU at 100%
-    forever (Issue #1048). Prior to SQL Server 2025 CU1 there is no DMV that
-    exposes true host CPU when that happens, so we store NULL for
-    other_process_cpu_utilization instead of a false value. sys.dm_os_linux_cpu_stats
-    (2025 CU1+) exposes real host CPU jiffies but is a cumulative counter
-    requiring a two-sample delta, not a point-in-time snapshot like
-    SCHEDULER_MONITOR, so it isn't used here.
+    forever (Issue #1048). The ring-buffer metrics fix shipped in SQL Server
+    2025 CU1 (KB5078298 fix 4796293), so real SystemIdle values start there,
+    not at 2025 RTM. Prior to that there is no DMV that exposes true host CPU
+    when SystemIdle is 0, so we store NULL for other_process_cpu_utilization
+    instead of a false value. sys.dm_os_linux_cpu_stats (2025 CU1+) exposes
+    real host CPU time but is a cumulative counter requiring a two-sample
+    delta, not a point-in-time snapshot like SCHEDULER_MONITOR, so it isn't
+    used here.
 
     sys.dm_os_host_info exists only on SQL Server 2017+. It is referenced through
     sp_executesql so SQL Server 2016 (which has no Linux build) never binds it and

install/29_collect_default_trace.sql

@@ -359,15 +359,19 @@ BEGIN
         CROSS APPLY sys.fn_trace_gettable
         (
             /*
-            SQL Server on Linux names the initial trace file without a rollover suffix
-            (log.trc, not log_1.trc). An unconditional strip finds no underscore, LEFT
-            takes the full path unchanged, and appending the extension back doubles it
-            (log.trc.trc) - fn_trace_gettable then errors on the nonexistent file
-            (Msg 19049) and no events are collected. Fall back to st.path when
-            CHARINDEX returns 0.
+            Strip the _N rollover suffix only when the LAST underscore sits in the
+            FILENAME, i.e. after the last path separator (either family). SQL Server
+            on Linux names the initial trace file without a rollover suffix (log.trc,
+            not log_1.trc), so an unguarded strip mangles the path (log.trc.trc,
+            issue 1633), and a trace DIRECTORY containing an underscore would mangle
+            it a second way (/var/opt/my_sql/log/log.trc -> /var/opt/my.trc, issue
+            1636) - fn_trace_gettable then errors on the nonexistent file (Msg 19049)
+            and no events are collected. Fall back to st.path in both cases.
             */
             CASE
                 WHEN CHARINDEX(N'_', REVERSE(st.path)) > 0
+                AND (CHARINDEX(N'\', REVERSE(st.path)) = 0 OR CHARINDEX(N'_', REVERSE(st.path)) < CHARINDEX(N'\', REVERSE(st.path)))
+                AND (CHARINDEX(N'/', REVERSE(st.path)) = 0 OR CHARINDEX(N'_', REVERSE(st.path)) < CHARINDEX(N'/', REVERSE(st.path)))
                 THEN LEFT(st.path, LEN(st.path) - CHARINDEX(N'_', REVERSE(st.path))) + RIGHT(st.path, 4)
                 ELSE st.path
             END,