diff --git a/Darling/Darling.Tests/AlertNotebookAuthoredTemplateTests.cs b/Darling/Darling.Tests/AlertNotebookAuthoredTemplateTests.cs index 8dd3b64a69..e00b9ebf0b 100644 --- a/Darling/Darling.Tests/AlertNotebookAuthoredTemplateTests.cs +++ b/Darling/Darling.Tests/AlertNotebookAuthoredTemplateTests.cs @@ -240,6 +240,10 @@ public void Budget_ReadCellsHaveLimitsExceptTheTrendRead_ComposedWindowsAreAtMos $"authored read cell names a barred read: {read}"); var parameters = Assert.IsType(cell["params"]); + /* Data-driven off the endpoint's OWN limitless-read set (#4223) rather than one hardcoded + name, so a new bucketed/no-limit read (get_wait_trend, get_memory_grants, + get_resource_semaphore) is covered the moment its family adds a row -- nothing here has + to change. */ if (AlertNotebookEndpoint.s_authoredLimitlessTrendReads.Contains(read)) { Assert.False(parameters.ContainsKey("limit"), $"{read} must not carry a limit param"); diff --git a/Darling/Darling.Tests/AlertNotebookTemplatePoisonWaitTests.cs b/Darling/Darling.Tests/AlertNotebookTemplatePoisonWaitTests.cs new file mode 100644 index 0000000000..76d3308861 --- /dev/null +++ b/Darling/Darling.Tests/AlertNotebookTemplatePoisonWaitTests.cs @@ -0,0 +1,227 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Globalization; +using System.Linq; +using System.Text.Json.Nodes; +using PerformanceMonitor.Darling.Service; +using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Notifications; +using Xunit; + +namespace Darling.Tests; + +/// +/// Pins for #4223's authored Poison Wait alert-notebook template +/// ( routing "Poison Wait" to +/// authored/poison-wait / BuildPoisonWaitCells). Follows the same shape as +/// AlertNotebookTemplateBlockingTests/Deadlocks' shared theories, which already cover this +/// family (AlertNotebookAuthoredTemplateTests' AllAuthoredMetrics enumerates +/// itself); this file pins the shape only this +/// family owns: the RESOURCE_SEMAPHORE gate on the two memory-grant reads, and the wait-type binding on +/// get_wait_trend. +/// +public sealed class AlertNotebookTemplatePoisonWaitTests +{ + private static readonly DateTime WindowEnd = new(2026, 1, 1, 12, 0, 0, DateTimeKind.Utc); + private static readonly DateTime WindowStart = WindowEnd - TimeSpan.FromHours(24); + private const string AsOf = "2026-01-01T12:00:00Z"; + + private static DarlingAlertReader.AlertHistoryReadRow RowWithWaitType(string? waitType) => + new(WindowEnd, 1, "SRV1", "Poison Wait", 1, 1, true, "email", null, false, null, false, + ContextJson: waitType is null + ? null + : AlertContextSerializer.Serialize(new AlertContext { WaitType = waitType })); + + /* ═══════════════════════════ routing ═══════════════════════════ */ + + [Fact] + public void AuthoredTemplate_PoisonWait_RoutesToItsOwnTemplate() + { + var template = AlertNotebookEndpoint.AuthoredTemplate("Poison Wait"); + + Assert.NotNull(template); + Assert.Equal("authored/poison-wait", template!.Value.Id); + Assert.Equal(AlertNotebookEndpoint.PoisonWaitTemplateVersion, template.Value.Version); + } + + /// RED on dev: before this PR, "Poison Wait" has no registered row, so + /// returns null and this assertion fails. + [Fact] + public void AuthoredTemplate_PoisonWait_IsNotNull() + { + Assert.NotNull(AlertNotebookEndpoint.AuthoredTemplate("Poison Wait")); + } + + /* ═══════════════════════════ cell list ═══════════════════════════ */ + + [Fact] + public void BuildPoisonWaitCells_ResourceSemaphore_IncludesBothSemaphoreReads() + { + var template = AlertNotebookEndpoint.AuthoredTemplate("Poison Wait")!.Value; + var row = RowWithWaitType("RESOURCE_SEMAPHORE"); + + var cells = template.BuildCells( + "Poison Wait", "SRV1", AsOf, WindowStart, WindowEnd, null, row, "Unknown"); + + var reads = cells.OfType() + .Where(c => (string?)c["type"] == "read") + .Select(c => (string)c["read"]!) + .ToArray(); + + Assert.Contains("get_resource_semaphore", reads); + Assert.Contains("get_memory_grants", reads); + Assert.Contains("get_wait_stats", reads); + Assert.Contains("get_pg_wait_stats", reads); + Assert.Contains("get_waiting_tasks", reads); + Assert.Contains("get_wait_trend", reads); + } + + /// Case-insensitive ordinal match. + [Fact] + public void BuildPoisonWaitCells_ResourceSemaphoreLowercase_StillIncludesBothSemaphoreReads() + { + var template = AlertNotebookEndpoint.AuthoredTemplate("Poison Wait")!.Value; + var row = RowWithWaitType("resource_semaphore"); + + var cells = template.BuildCells( + "Poison Wait", "SRV1", AsOf, WindowStart, WindowEnd, null, row, "Unknown"); + + var reads = cells.OfType() + .Where(c => (string?)c["type"] == "read") + .Select(c => (string)c["read"]!) + .ToArray(); + + Assert.Contains("get_resource_semaphore", reads); + Assert.Contains("get_memory_grants", reads); + } + + [Theory] + [InlineData("THREADPOOL")] + [InlineData("RESOURCE_SEMAPHORE_QUERY_COMPILE")] + public void BuildPoisonWaitCells_OtherWaitType_ExcludesBothSemaphoreReads(string waitType) + { + var template = AlertNotebookEndpoint.AuthoredTemplate("Poison Wait")!.Value; + var row = RowWithWaitType(waitType); + + var cells = template.BuildCells( + "Poison Wait", "SRV1", AsOf, WindowStart, WindowEnd, null, row, "Unknown"); + + var reads = cells.OfType() + .Where(c => (string?)c["type"] == "read") + .Select(c => (string)c["read"]!) + .ToArray(); + + Assert.DoesNotContain("get_resource_semaphore", reads); + Assert.DoesNotContain("get_memory_grants", reads); + } + + [Fact] + public void BuildPoisonWaitCells_WaitTrend_WaitTypeParamEqualsTheIncidentsExactly() + { + var template = AlertNotebookEndpoint.AuthoredTemplate("Poison Wait")!.Value; + var row = RowWithWaitType("THREADPOOL"); + + var cells = template.BuildCells( + "Poison Wait", "SRV1", AsOf, WindowStart, WindowEnd, null, row, "Unknown"); + + var trend = cells.OfType() + .Single(c => (string?)c["type"] == "read" && (string?)c["read"] == "get_wait_trend"); + var parameters = Assert.IsType(trend["params"]); + + Assert.Equal("THREADPOOL", (string?)parameters["wait_type"]); + } + + [Fact] + public void BuildPoisonWaitCells_NoWaitType_OmitsTrend_AddsANoteInstead() + { + var template = AlertNotebookEndpoint.AuthoredTemplate("Poison Wait")!.Value; + + var cells = template.BuildCells( + "Poison Wait", "SRV1", AsOf, WindowStart, WindowEnd, null, null, "Unknown"); + + var reads = cells.OfType() + .Where(c => (string?)c["type"] == "read") + .Select(c => (string)c["read"]!) + .ToArray(); + + Assert.DoesNotContain("get_wait_trend", reads); + Assert.Contains(cells.OfType(), c => (string?)c["type"] == "markdown"); + } + + /* ═══════════════════════════ degrade ═══════════════════════════ */ + + [Fact] + public void BuildPoisonWaitCells_NoMatchedIncidentOrRow_DegradesWithTheSameCellCount() + { + var template = AlertNotebookEndpoint.AuthoredTemplate("Poison Wait")!.Value; + + var withRow = template.BuildCells( + "Poison Wait", "SRV1", AsOf, WindowStart, WindowEnd, null, + RowWithWaitType("THREADPOOL"), "Unknown"); + var withoutRow = template.BuildCells( + "Poison Wait", "SRV1", AsOf, WindowStart, WindowEnd, null, null, "Unknown"); + + /* Both degrade to "no wait type" cell counts identically -- the with-row case here carries a wait + type (adds the trend read), the without-row case adds the note markdown cell instead: same count, + different content. */ + Assert.Equal(withRow.Count, withoutRow.Count); + } + + /* ═══════════════════════════ validates ═══════════════════════════ */ + + [Fact] + public void BuildPoisonWaitCells_PassesValidateNotebookDefinition() + { + var template = AlertNotebookEndpoint.AuthoredTemplate("Poison Wait")!.Value; + var cells = template.BuildCells( + "Poison Wait", "SRV1", AsOf, WindowStart, WindowEnd, null, + RowWithWaitType("RESOURCE_SEMAPHORE"), "Unknown"); + + var definition = new JsonObject { ["kind"] = "notebook", ["cells"] = cells }; + var validation = DarlingWebEndpoints.ValidateNotebookDefinition(definition); + + Assert.True(validation.IsValid, validation.Error); + } + + [Fact] + public void BuildPoisonWaitCells_NoWaitType_AlsoPassesValidateNotebookDefinition() + { + var template = AlertNotebookEndpoint.AuthoredTemplate("Poison Wait")!.Value; + var cells = template.BuildCells( + "Poison Wait", "SRV1", AsOf, WindowStart, WindowEnd, null, null, "Unknown"); + + var definition = new JsonObject { ["kind"] = "notebook", ["cells"] = cells }; + var validation = DarlingWebEndpoints.ValidateNotebookDefinition(definition); + + Assert.True(validation.IsValid, validation.Error); + } + + /* ═══════════════════════════ step 1: the context builder pin ═══════════════════════════ */ + + [Fact] + public void PoisonWaitContext_SerializesAndRehydratesTheWaitType() + { + var context = new AlertContext { WaitType = "RESOURCE_SEMAPHORE" }; + + var json = AlertContextSerializer.Serialize(context); + + Assert.True(AlertContextSerializer.TryDeserialize(json, out var rehydrated)); + Assert.Equal("RESOURCE_SEMAPHORE", rehydrated.WaitType); + Assert.Equal("RESOURCE_SEMAPHORE", AlertContextSerializer.TryReadWaitType(json)); + } + + [Fact] + public void PoisonWaitContext_LegacyJsonWithNoWaitTypeMember_RehydratesNull() + { + const string legacyJson = "{\"Details\":[]}"; + + Assert.Null(AlertContextSerializer.TryReadWaitType(legacyJson)); + } +} diff --git a/Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.Templates.PoisonWait.cs b/Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.Templates.PoisonWait.cs new file mode 100644 index 0000000000..0db26cc1fe --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.Templates.PoisonWait.cs @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Text.Json.Nodes; +using PerformanceMonitor.Darling.Analysis; +using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Notifications; + +namespace PerformanceMonitor.Darling.Service; + +internal static partial class AlertNotebookEndpoint +{ + /// Poison Wait template version (#4223). Bumped only if this template's SHAPE changes. + internal const int PoisonWaitTemplateVersion = 1; + + /// The one wait type this alert fires on, RESOURCE_SEMAPHORE (ordinal, case-insensitive) — the + /// only type whose remedy is "raise a memory grant/worker ceiling", so its two supporting reads + /// ('s memory-grant cells) are wasted panels on every other poison + /// type and are gated on this string rather than shown unconditionally. + internal const string ResourceSemaphoreWaitType = "RESOURCE_SEMAPHORE"; + + /// Poison Wait (#4223, both engines): header, status, get_wait_stats / + /// get_pg_wait_stats (each engine's own top-waits read, titled by engine so the wrong-engine read + /// answers "not applicable" on its own — the mechanical map's #2719 convention), get_waiting_tasks, + /// get_resource_semaphore + get_memory_grants ONLY when the firing wait type is + /// RESOURCE_SEMAPHORE, then get_wait_trend for the wait type the alert names — or, when no + /// incident/row carries a wait type, a note cell in its place rather than a trend call that would 400 on + /// a missing required wait_type. + /// + /// The wait type rides the alert's context as structured data (#4223 step 1): + /// , set at both fire sites + /// (AlertEngine.CheckPoisonWaitsAsync's worst-graded , + /// DarlingWorker.EvaluatePgPoisonWaitAsync's finding.Subject) — the same value each engine's + /// mute context already keys on (AlertMuteContext.WaitType), so this template reuses the field + /// rather than re-deriving it from prose. + /// + private static JsonArray BuildPoisonWaitCells( + string? metric, string? serverName, string? asOf, DateTime windowStart, DateTime windowEnd, + AlertIncident? incident, DarlingAlertReader.AlertHistoryReadRow? row, string status) + { + /* The wait type rides the ROW's persisted context (AlertContext.WaitType), not the incident — + AlertIncident carries no wait-type member; the two engines' Poison Wait fire sites set the + context-level field directly (see this method's doc comment). */ + var waitType = row?.ContextJson is string contextJson + ? AlertContextSerializer.TryReadWaitType(contextJson) + : null; + + var cells = new JsonArray + { + HeaderCell(metric, serverName, incident, row), + StatusCell(status), + AuthoredReadCell("get_wait_stats", "Top waits (SQL Server)", serverName, asOf, + ("hours", "24"), ("limit", "20")), + AuthoredReadCell("get_pg_wait_stats", "Top waits (PostgreSQL)", serverName, asOf, + ("hours", "24"), ("limit", "20")), + AuthoredReadCell("get_waiting_tasks", "Waiting tasks", serverName, asOf, + ("hours", "1"), ("limit", "30")), + }; + + if (string.Equals(waitType, ResourceSemaphoreWaitType, StringComparison.OrdinalIgnoreCase)) + { + cells.Add(AuthoredReadCell("get_resource_semaphore", "Resource semaphore", serverName, asOf, + ("hours", "24"))); + cells.Add(AuthoredReadCell("get_memory_grants", "Memory grants", serverName, asOf, + ("hours", "1"))); + } + + /* A timeline on wait_stats/wait_time_ms, filtered to the firing wait type + when one is known -- built inline (not through TimelinePanel, which only knows a database_name + filter) rather than widening that shared helper for one family's dimension. */ + var wallPanel = new JsonObject + { + ["type"] = "panel", + ["title"] = "Wait time over time", + ["source"] = "wait_stats", + ["measure"] = "wait_time_ms", + ["aggregate"] = "sum", + ["viz"] = "line", + ["timeBucket"] = "hour", + ["range"] = new JsonObject + { + ["windowStart"] = windowStart.ToString("o", System.Globalization.CultureInfo.InvariantCulture), + ["windowEnd"] = windowEnd.ToString("o", System.Globalization.CultureInfo.InvariantCulture), + }, + }; + if (!string.IsNullOrWhiteSpace(waitType)) + { + wallPanel["filters"] = new JsonArray + { + new JsonObject + { + ["dimension"] = "wait_type", + ["op"] = "eq", + ["value"] = waitType, + }, + }; + } + cells.Add(wallPanel); + + if (string.IsNullOrWhiteSpace(waitType)) + { + /* get_wait_trend REQUIRES wait_type (PReqText) — emitting the cell with none would 400. A + markdown cell is the notebook's own prose slot (ValidateNotebookDefinition's "markdown" arm), + so the degrade reads as a note rather than a missing panel. */ + cells.Add(new JsonObject + { + ["type"] = "markdown", + ["text"] = "No matched incident carries a wait type, so the wait trend (which requires one) is omitted.", + }); + } + else + { + cells.Add(AuthoredReadCell("get_wait_trend", "Wait trend", serverName, asOf, + ("hours", "24"), ("wait_type", waitType))); + } + + return cells; + } +} diff --git a/Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.cs b/Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.cs index 7ced47398b..7952c87c77 100644 --- a/Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.cs +++ b/Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.cs @@ -396,6 +396,8 @@ internal static readonly (string[] Metrics, AuthoredTemplateEntry Entry)[] s_aut new AuthoredTemplateEntry("authored/cpu", CpuTemplateVersion, BuildCpuCells)), (new[] { "Long-Running Query" }, new AuthoredTemplateEntry("authored/long-running-query", LongRunningQueryTemplateVersion, BuildLongRunningQueryCells)), + (new[] { "Poison Wait" }, + new AuthoredTemplateEntry("authored/poison-wait", PoisonWaitTemplateVersion, BuildPoisonWaitCells)), (new[] { "PostgreSQL Replication Slot Retention" }, new AuthoredTemplateEntry("authored/pg-replication-slot", PgReplicationSlotTemplateVersion, BuildPgReplicationSlotCells)), (new[] { "PostgreSQL Vacuum Horizon Blocked" }, @@ -455,6 +457,12 @@ internal static bool IsDeclaredNoNotebook(string? metric) => internal static readonly IReadOnlySet s_authoredLimitlessTrendReads = new HashSet(StringComparer.Ordinal) { "get_deadlock_trend", + /* #4223 Poison Wait: get_wait_trend has a bucket-count budget, not a row cap (PReqText("wait_type") + with no PLimit). get_memory_grants and get_resource_semaphore declare no limit param at all + (DarlingWebEndpoints.CatalogDescriptors). */ + "get_wait_trend", + "get_memory_grants", + "get_resource_semaphore", /* #4223: get_top_queries_by_cpu / get_top_procedures_by_cpu / get_cpu_scheduler_pressure declare no 'limit' param at all -- the first two cap with 'top' (carried explicitly below), the last has no row cap to carry. */ diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs index be1e7df745..a5eefb5468 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs @@ -6211,6 +6211,10 @@ reasoning as the Tier 0 delivery loop above. */ { new(finding.Subject, new[] { finding.CurrentValue }), }, + /* #4223: the wait type as structured data, the same string the mute context + below already keys on (WaitType = finding.Subject) — an alert-notebook reader + can branch on it without parsing the subject back out of the incident. */ + WaitType = finding.Subject, }, DetailText: null, finding.NumericCurrentValue, diff --git a/PerformanceMonitor.Alerting/AlertEngine.cs b/PerformanceMonitor.Alerting/AlertEngine.cs index 2f7997a635..17b86cf219 100644 --- a/PerformanceMonitor.Alerting/AlertEngine.cs +++ b/PerformanceMonitor.Alerting/AlertEngine.cs @@ -1667,7 +1667,10 @@ the collector lags the cooldown — see the field's own doc comment. Only a coll context (the #1136 low-disk grading path) while Darling's deliverer folds the outcome's in only when the context has none (#2090); setting both makes the two agree by construction on both SKUs. */ - var poisonContext = new AlertContext { SeverityOverride = worst.Severity }; + /* #4223: the worst-graded wait type as structured data, the same value the mute + context above already keys on — an alert-notebook reader can branch on it without + parsing the detail heading or the prose. */ + var poisonContext = new AlertContext { SeverityOverride = worst.Severity, WaitType = worst.WaitType }; foreach (var finding in findings) { poisonContext.Details.Add(new AlertDetailItem diff --git a/PerformanceMonitor.Notifications/AlertContext.cs b/PerformanceMonitor.Notifications/AlertContext.cs index 770c4b51f8..e22f154499 100644 --- a/PerformanceMonitor.Notifications/AlertContext.cs +++ b/PerformanceMonitor.Notifications/AlertContext.cs @@ -75,6 +75,17 @@ public class AlertContext /// this records whether the fan-out was allowed to run at all, one decision upstream of it. /// public AlertRoutingDto? Routing { get; set; } + + /// + /// The wait type this firing is ABOUT (#4223 Poison Wait notebook), as structured data rather than + /// something a reader has to regex out of 's prose heading or DetailText. + /// Set by the Poison Wait fire sites on both engines — the SQL Server evaluator's worst-graded + /// WaitType, the PostgreSQL evaluator's Subject (its own type:event display string, + /// e.g. "IPC:BtreePage") — the same value each engine's mute context already keys on + /// (AlertMuteContext.WaitType). Null on every other alert and on any row written before this + /// member existed. + /// + public string? WaitType { get; set; } } /// @@ -307,7 +318,10 @@ public record AlertContextDto( List? Incidents = null, [property: JsonConverter(typeof(JsonStringEnumConverter))] AlertSeverityLevel? Severity = null, AlertRouteDto? Route = null, - AlertRoutingDto? Routing = null); + AlertRoutingDto? Routing = null, + /* #4223 Poison Wait: trailing and nullable like Route/Routing, so a row written before this member + existed rehydrates to null ("this row carries no wait type") rather than a fabricated value. */ + string? WaitType = null); /// /// The persisted routing DECISION for an analysis finding (#3712): trailing and nullable on @@ -595,10 +609,41 @@ either engine persists its grade here with no store change on either side. */ /* #3598: where the posts went. Already the persisted shape, so it rides through as-is. */ context.Route, /* #3712: whether the fan-out was allowed to run, and why. Already the persisted shape. */ - context.Routing); + context.Routing, + /* #4223: the wait type this firing is about, as structured data. Already the persisted shape. */ + context.WaitType); return JsonSerializer.Serialize(dto); } + /// + /// The wait type a persisted alert-history row carries (#4223 Poison Wait notebook), or null when + /// it carries none — any non-Poison-Wait alert, a row written before the member existed, or unparseable + /// JSON. Reads the one property rather than rehydrating the whole context, for the reason + /// gives. + /// + public static string? TryReadWaitType(string? contextJson) + { + if (string.IsNullOrWhiteSpace(contextJson)) + return null; + + try + { + using var document = JsonDocument.Parse(contextJson); + if (document.RootElement.ValueKind != JsonValueKind.Object + || !document.RootElement.TryGetProperty(nameof(AlertContextDto.WaitType), out var waitType) + || waitType.ValueKind != JsonValueKind.String) + { + return null; + } + + return waitType.GetString(); + } + catch (JsonException) + { + return null; + } + } + /// /// The routing decision a persisted alert-history row carries (#3712), or null when it carries none /// — an engine alert, a row written before the member existed, or unparseable JSON. Reads the one property @@ -779,6 +824,10 @@ public static bool TryDeserialize(string? json, out AlertContext context) /* #3712: the corroboration gate's decision, null on engine alerts and pre-#3712 rows. */ context.Routing = dto.Routing; + /* #4223: the wait type this firing is about, null on every non-Poison-Wait alert and every row + written before this member existed. */ + context.WaitType = dto.WaitType; + foreach (var d in dto.Details) { var item = new AlertDetailItem