Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **Top Procedures collects on Azure SQL Database** ([#1833]) - `procedure_stats` was one of two database-scoped collectors that never opted into the Azure per-database connection path: it ran once on the server entry's own connection - whose catalog defaults to `master` when the entry's Database field is blank - and its Azure variant's `WHERE s.database_id = DB_ID()` then matched only master's procedures. Zero user rows, logged `SUCCESS`, empty Top Procedures grid that looked healthy, while Top Queries (which does run per database) kept working right next to it - the asymmetry that fingerprinted the bug. The collector now declares `RunsPerDatabase` on Azure like `query_stats` and its five database-scoped siblings, and the per-database connection makes `DB_ID()` mean each user database in turn - exactly what that predicate was written for. Both Lite and Darling get it (shared definition). The other collector with the same gap is Query Store, which needs a per-database rework on Azure rather than a one-line override - tracked as [#1836], with the invisible-zero-rows Collection Health gap that hid both defects tracked as [#1837].

- **Chart x-axis timestamps now honor the Local/Server/UTC time toggle** ([#1831]) - Lite plots chart X values in server time and converts for display at render, but the ONE render surface doing the axis labels - the single shared `LabelFormatter` all three apps reach - never consulted the display mode, so every chart's bottom axis showed server time no matter what the toggle said, while grids, slicers, tooltips, and the crosshair all converted around it. Refresh and reconnect could not help: they re-plotted the same unconverted labels. The formatter now converts each label through the `UiTimeContext` hook Lite wired at startup for exactly this purpose and never used here; the Darling Viewer pre-converts its plotted X and deliberately leaves the hook at identity, so the change is a no-op there by construction - the double-conversion trap a Viewer-style port into Lite would have hit is documented at the site and pinned by test. Three companions ride along, all the same defect in different clothes: the display-mode toggle now re-plots the charts immediately (it refreshed nine grids and six slicers and zero charts, so even the fixed formatter only showed on the next data cycle); the Queries heatmap's hand-built axis labels convert like that chart's own tooltip always did; and the three history windows switch to the shared formatter, gaining both the conversion and the date-change labels. The deprecated Dashboard's startup hack that force-pinned the dropdown to ServerTime ("charts always render in server time") is retired - and its saved preference, which was written on every change and read never, is finally restored at startup. New tests pin the formatter's conversion, the Viewer's identity no-op, and the display-date boundary behavior - nothing anywhere tested the formatter before.

- **High CPU alerts no longer record 0 as their value in Alert History** ([#1830]) - the alert fired with no numeric value, and the history stores' fallback tried to parse the display text `"87% (Total CPU)"` - which ends with a parenthesis, so the trailing-`%` trim did nothing, the parse failed, and the `: 0` arm silently stored zero for every High CPU row ever written, in Lite and Darling alike (the deprecated Dashboard stores the text and was immune). Detection, thresholds, and every notification surface were always correct - the toast, email, and webhook all carry the text - so only the audit trail and the MCP alert tool were corrupted, which is exactly what made it invisible. The engine now passes real numerics for High CPU, Blocking Detected, and Deadlocks Detected; per-event delivery carries each incident's occurrence count (and the overflow trailer's COUNT - its `"+N more incident(s)"` text is unparseable by design and was the second live instance of the same coercion); and both stores' fallback goes through a shared leading-numeric parser so a future decorated text cannot re-coin a silent 0. Parses with CurrentCulture on purpose - the producers format with it, and the reporter's own locale writes `92,5%`. Two tests that PINNED the null-numerics behavior are flipped to pin the fix, and a store round-trip now asserts the exact field case - `"87% (Total CPU)"`, no numerics - lands as 87, not 0.
Expand Down Expand Up @@ -1964,4 +1966,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#1823]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1823
[#1828]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1828
[#1831]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1831
[#1833]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1833
[#1836]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1836
[#1837]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1837
[#1830]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1830
12 changes: 12 additions & 0 deletions Lite.Tests/ProcedureStatsCollectorDefinitionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,18 @@ public void BuildQuery_Azure_SingleDatabase_TokenLeftInPlace()
Assert.Contains("/*EXCLUSION_FILTER*/", plan.Text, StringComparison.Ordinal);
}

[Fact]
public void RunsPerDatabase_OnAzureOnly()
{
/* #1833: without the per-database override, the Azure variant ran once on the server
entry's own connection (master when the Database field is blank), where its
database_id = DB_ID() filter matched nothing — zero user rows, logged SUCCESS. The
per-database connection is what makes that predicate mean each user database. */
Assert.True(ProcedureStatsCollector.Instance.RunsPerDatabase(new CollectorTargetInfo { IsAzureSqlDb = true }));
Assert.False(ProcedureStatsCollector.Instance.RunsPerDatabase(new CollectorTargetInfo()));
Assert.False(ProcedureStatsCollector.Instance.RunsPerDatabase(new CollectorTargetInfo { IsAzureManagedInstance = true }));
}

[Fact]
public void BuildQuery_HandlesConvertToVarchar130_NeverTruncated()
{
Expand Down
11 changes: 11 additions & 0 deletions PerformanceMonitor.Collectors/ProcedureStatsCollector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,17 @@ the xml type still return. dm_exec_text_query_plan(plan_handle, 0, -1) returns a

public override string TargetTable => "procedure_stats";

/// <summary>
/// #1833: on Azure SQL Database this collector must run per database, like query_stats and the
/// other database-scoped collectors already do. Without the override it ran once on the server
/// entry's own connection — whose catalog defaults to master when the Database field is blank —
/// and the Azure variant's <c>WHERE s.database_id = DB_ID()</c> then filtered to master's
/// procedures: zero user rows, logged SUCCESS, and an empty Top Procedures grid that looked
/// healthy. The per-database connection makes <c>DB_ID()</c> each user database in turn, which
/// is exactly what that predicate was written for.
/// </summary>
public override bool RunsPerDatabase(CollectorTargetInfo target) => target.IsAzureSqlDb;

public override CollectorQuery BuildQuery(CollectorContext context)
{
var planSelect = context.CapturePlanXml ? PlanSelectFragment : "";
Expand Down
Loading