diff --git a/Darling/Darling.Tests/AdminPageTests.cs b/Darling/Darling.Tests/AdminPageTests.cs index 5478aaeb24..4ad773cc58 100644 --- a/Darling/Darling.Tests/AdminPageTests.cs +++ b/Darling/Darling.Tests/AdminPageTests.cs @@ -43,7 +43,7 @@ public void TheServersTab_ReadsTheAdminRoute_NotListServers_AndDrawsDisabledRows [Theory] [InlineData("SERVER_COLUMNS", new[] { "display_name", "server_name", "auth", "engine", "version", "status", "freshness", "monthly_cost", "read_only", "added", "last_collected", "same_machine_as" })] - [InlineData("ROUTE_COLUMNS", new[] { "route_id", "enabled", "metric_match", "match_kind", "family", "applies_to", "status", "channels", "agent", "resolutions", "smtp_recipients", "modified_at_utc" })] + [InlineData("ROUTE_COLUMNS", new[] { "route_id", "enabled", "metric_match", "match_kind", "family", "applies_to", "severities", "status", "channels", "agent", "resolutions", "smtp_recipients", "modified_at_utc" })] [InlineData("SETTING_COLUMNS", new[] { "setting", "value" })] public void EachTab_DeclaresItsColumnKeys(string name, string[] keys) { diff --git a/Darling/Darling.Tests/AdminPendingNotesTests.cs b/Darling/Darling.Tests/AdminPendingNotesTests.cs index 4a2016a730..07528e5fb5 100644 --- a/Darling/Darling.Tests/AdminPendingNotesTests.cs +++ b/Darling/Darling.Tests/AdminPendingNotesTests.cs @@ -120,6 +120,54 @@ public void AddingARouteWithNoMatch_IsRefusedByTheFormBeforeARequest_Pure() Assert.Equal("blocking", named.GetProperty("payload").GetProperty("metric_match").GetString()); } + /* #6928: the Routes list names the severity filter in the desktop window's words (Both when the field is missing or unknown), the editor + reads it back from a route and the save sends it, and a word the page does not know is sent as both. */ + [Fact] + public void TheRoutesList_ShowsTheSeverityBesideTheScope_AndTheEditorRoundTripsIt_Pure() + { + var data = JsonNode.Parse(""" + {"routes":[ + {"route_id":1,"metric_match":"blocking","severities":"critical","enabled":true}, + {"route_id":2,"metric_match":"performance","severities":"warning","enabled":true}, + {"route_id":3,"metric_match":"reports","severities":"both","enabled":true}, + {"route_id":4,"metric_match":"agent-jobs","enabled":true}, + {"route_id":5,"metric_match":"waits","severities":"someday","enabled":true} + ]} + """); + var rows = Pure("routeRows", data).EnumerateArray().ToArray(); + Assert.Equal(new[] { "Critical only", "Warning only", "Both", "Both", "Both" }, rows.Select(r => r.GetProperty("severities").GetString())); + + var columns = Pure("ROUTE_COLUMNS").EnumerateArray().Select(c => c.GetProperty("key").GetString()).ToList(); + Assert.Equal(columns.IndexOf("applies_to") + 1, columns.IndexOf("severities")); + + Assert.Equal("critical", Pure("entriesFromRoute", new { route_id = 1, metric_match = "blocking", severities = "critical" }).GetProperty("severities").GetString()); + Assert.Equal("both", Pure("entriesFromRoute", new { route_id = 1, metric_match = "blocking" }).GetProperty("severities").GetString()); + Assert.Equal("both", Pure("entriesFromRoute", (object?)null).GetProperty("severities").GetString()); + + string Sent(string severities) => Pure("routePayload", new { id = (int?)null, match = "blocking", perHour = "10", perDay = "50", lease = "30", expiry = "120", fallback = "", scope = "", level = "", resolutions = "", enabled = true, severities }) + .GetProperty("payload").GetProperty("severities").GetString()!; + Assert.Equal("warning", Sent("warning")); + Assert.Equal("critical", Sent("critical")); + Assert.Equal("both", Sent("both")); + Assert.Equal("both", Sent("someday")); + } + + /* #6936: Edit then Save on the Routes tab sends the version the form read (expected_modified_at), because the service refuses an edit + without it with a 400. The test drives the page itself: it opens the editor on a stored Critical only route, picks Warning only, + presses Save and reads the write the page made. */ + [Fact] + public void SavingAnEditedRoute_SendsTheVersionTheFormRead_AndTheChosenSeverity() + { + var writes = AdminServerEditBehaviourTests.Run("routeEdit:warning").GetProperty("writes").EnumerateArray().ToArray(); + + var write = Assert.Single(writes); + Assert.Equal("/api/notification-routes/7", write.GetProperty("url").GetString()); + var body = write.GetProperty("body"); + Assert.Equal("2026-01-01T00:00:00.1234567Z", body.GetProperty("expected_modified_at").GetString()); + Assert.Equal("warning", body.GetProperty("severities").GetString()); + Assert.Equal("blocking", body.GetProperty("metric_match").GetString()); + } + [Fact] public void OnlyASeatThatCanEdit_GetsTheDecideColumn_Pure() { diff --git a/Darling/Darling.Tests/AgentActionGateSeverityLiveTests.cs b/Darling/Darling.Tests/AgentActionGateSeverityLiveTests.cs new file mode 100644 index 0000000000..686b8148ba --- /dev/null +++ b/Darling/Darling.Tests/AgentActionGateSeverityLiveTests.cs @@ -0,0 +1,89 @@ +/* + * 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.Threading.Tasks; +using Microsoft.Extensions.Logging.Abstractions; +using Npgsql; +using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; +using Xunit; + +namespace Darling.Tests; + +/* #1776 own-store: the fact mints its own scratch database through ScratchPostgres, so the class is deliberately NOT [Collection("live-postgres")]. */ + +/// +/// #6928: the agent-action gate re-resolves a request's routes at the tier each alert arrived at, so a Critical alert dispatched under a +/// fleet route is not refused because a server route that takes only Warning (agent Off) would have won the lookup with no tier. +/// +public sealed class AgentActionGateSeverityLiveTests +{ + private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + + [Fact] + public async Task ACriticalAlertDispatchedUnderTheFleetRoute_IsNotRefusedByAWarningOnlyServerRouteThatIsOff() + { + Assert.SkipWhen(string.IsNullOrEmpty(ConnectionString), "Set DARLING_TEST_PG to run the agent gate severity pin (it mints its own scratch database)."); + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(ConnectionString!, ct); + await using (var migrate = new NpgsqlConnection(scratch.ConnectionString)) + { + await migrate.OpenAsync(ct); + await PgMigrations.MigrateAsync(migrate, ct); + } + + await using var owner = NpgsqlDataSource.Create(scratch.ConnectionString); + var bodySucceeded = false; + try + { + var proxies = new WebhookProxies(null, null, null, null); + /* A server-scoped Blocking route that takes Warning only, with the agent Off, and a fleet-wide Blocking route with the agent at Act. */ + await using (var seed = owner.CreateCommand("INSERT INTO config.config_monitored_servers (server_id, name, host) VALUES (7, 'srv-7', 'host-a') ON CONFLICT DO NOTHING")) + { + await seed.ExecuteNonQueryAsync(ct); + } + + await NotificationRouteWriter.SaveAsync( + owner, new NotificationRouteDraft { MetricMatch = "Blocking", ScopeServerId = 7, Severities = "warning", AgentLevel = 0 }, + null, proxies, null, null, 30, ct); + await NotificationRouteWriter.SaveAsync( + owner, new NotificationRouteDraft { MetricMatch = "Blocking", AgentLevel = 2 }, null, proxies, null, null, 30, ct); + + var store = new InvestigationRequestStore(owner, 30); + /* A Critical alert dispatches under the fleet route: request level 2. */ + var opened = await store.OpenOrJoinAsync( + new InvestigationAlert(1, 7, "srv-7", "blocking", "Blocking", DateTime.UtcNow, Array.Empty(), null, 20, 100, LeaseMinutes: 15, ExpiryMinutes: 240) + { AutonomyLevel = 2, Severity = "critical" }, ct); + var request = opened.RequestId!.Value; + var claim = (await store.ClaimAsync(request, "agent-a", ct)).ClaimId!.Value; + + var writes = 0; + var hooks = new AgentActionHooks + { + LoadLevels = (connection, _, token) => DarlingAgentActionGate.LoadLevelsAsync(connection, NullLogger.Instance, token), + ReadBefore = (_, _, _) => Task.FromResult(null), + Write = (_, _, _) => { writes++; return Task.FromResult("{}"); }, + }; + var call = new AgentActionCall("create_mute_rule", "mute", new AgentAction(request, claim, "agent-a", "known batch", + "[{\"tool\":\"get_blocking\",\"arguments\":{},\"key_numbers\":\"12\"}]"), new AgentActionTarget(7, "srv-7", null, null)); + + var done = await store.RunAgentActionAsync(call, hooks, ct); + + Assert.True(done.Done, done.Refusal?.Message); + Assert.Equal(1, writes); + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, static (_, _) => Task.CompletedTask); + } + } +} diff --git a/Darling/Darling.Tests/AgentActionGateTests.cs b/Darling/Darling.Tests/AgentActionGateTests.cs index 5c6fcf5e47..bd1be2deb4 100644 --- a/Darling/Darling.Tests/AgentActionGateTests.cs +++ b/Darling/Darling.Tests/AgentActionGateTests.cs @@ -218,7 +218,7 @@ private static InvestigationAlert Alert(DateTime at, int level = 2, string metri private AgentActionHooks Hooks(int? resolved = 2, AgentActionRefusal? kind = null) => new() { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => resolved), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => resolved), KindRule = kind is null ? null : (_, _, _, _) => Task.FromResult(kind), ReadBefore = (_, _, _) => Task.FromResult("{\"reason\":\"old\"}"), Write = (_, _, _) => { _writes++; return Task.FromResult("{\"reason\":\"new\"}"); }, @@ -363,7 +363,7 @@ await RunAsync(async (store, connection, ct) => LoadLevels = (_, _, _) => { var level = ++calls == 1 ? 2 : 1; - return Task.FromResult>((_, _) => level); + return Task.FromResult>((_, _, _) => level); }, ReadBefore = (_, _, _) => Task.FromResult(null), Write = (_, _, _) => { _writes++; return Task.FromResult(null); }, @@ -387,7 +387,7 @@ await RunAsync(async (store, connection, ct) => await ExecAsync(connection, "UPDATE config.config_service SET agent_actions_enabled = true", ct); var hooks = new AgentActionHooks { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => 2), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => 2), /* Runs in the gate's transaction after the switch check passed and before action_called commits: a person turns the switch off now, so only the write's re-check can see it. */ ReadBefore = async (_, _, token) => @@ -486,7 +486,7 @@ await RunAsync(async (store, connection, ct) => LoadLevels = (_, _, _) => { loads++; - return Task.FromResult>((metric, _) => { seen.Add(metric); return 2; }); + return Task.FromResult>((metric, _, _) => { seen.Add(metric); return 2; }); }, ReadBefore = (_, _, _) => Task.FromResult(null), Write = (_, _, _) => { _writes++; return Task.FromResult(null); }, @@ -553,7 +553,7 @@ CREATE TRIGGER test_fail_every_done BEFORE INSERT ON config.investigation_journa private AgentActionHooks WriteHooks(ServerChangeSignal? signal, bool mark, Func? then = null) => new() { ServerChange = signal, - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => 2), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => 2), ReadBefore = (_, _, _) => Task.FromResult("{\"state\":\"old\"}"), Write = async (_, _, token) => { diff --git a/Darling/Darling.Tests/AgentChangeUndoLiveTests.cs b/Darling/Darling.Tests/AgentChangeUndoLiveTests.cs index abd6aff841..05d2f8a014 100644 --- a/Darling/Darling.Tests/AgentChangeUndoLiveTests.cs +++ b/Darling/Darling.Tests/AgentChangeUndoLiveTests.cs @@ -277,7 +277,7 @@ await RunAsync(async (undo, connection, ct) => var writes = 0; var hooks = new AgentActionHooks { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => 2), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => 2), ReadBefore = (_, _, _) => Task.FromResult(null), Write = (_, _, _) => { writes++; return Task.FromResult("{}"); }, }; diff --git a/Darling/Darling.Tests/AgentLevel2StoreTests.cs b/Darling/Darling.Tests/AgentLevel2StoreTests.cs index fa1c989235..b9b9f249f0 100644 --- a/Darling/Darling.Tests/AgentLevel2StoreTests.cs +++ b/Darling/Darling.Tests/AgentLevel2StoreTests.cs @@ -527,7 +527,7 @@ private static async Task RunTheGateAsMcpAsync(NpgsqlConnection owner, ScratchPo "[{\"tool\":\"get_blocking\",\"arguments\":{},\"key_numbers\":\"12\"}]"), new AgentActionTarget(7, "srv-7", null, null)); AgentActionHooks Hooks(int level) => new() { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => level), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => level), ReadBefore = (_, _, _) => Task.FromResult(null), Write = (_, _, _) => { writes++; return Task.FromResult("{}"); }, }; diff --git a/Darling/Darling.Tests/AgentSetupWebLiveTests.cs b/Darling/Darling.Tests/AgentSetupWebLiveTests.cs index 37341f26a0..5f226c8a87 100644 --- a/Darling/Darling.Tests/AgentSetupWebLiveTests.cs +++ b/Darling/Darling.Tests/AgentSetupWebLiveTests.cs @@ -246,6 +246,17 @@ await WithWebRoutesAsync("admin", async (owner, _, Send, ct) => Assert.Equal("Analysis: Waits|ops@example.com|3|60|500|false", await ScalarAsync(owner, $"SELECT metric_match || '|' || smtp_recipients || '|' || agent_level || '|' || agent_max_per_hour || '|' || agent_max_per_day || '|' || enabled::text FROM config_notification_routes WHERE route_id = {seeded}", ct)); + /* #6928: the severity filter. A new route is Both, an edit sets it (any case), an edit that leaves it out keeps it. */ + Assert.Equal("both", await ScalarAsync(owner, $"SELECT severities FROM config_notification_routes WHERE route_id = {seeded}", ct)); + Assert.Equal(200, (await Send("PUT", $"/api/notification-routes/{seeded}", Json, "{\"severities\":\"Critical\"}")).Status); + Assert.Equal("critical", await ScalarAsync(owner, $"SELECT severities FROM config_notification_routes WHERE route_id = {seeded}", ct)); + Assert.Equal(200, (await Send("PUT", $"/api/notification-routes/{seeded}", Json, "{\"enabled\":true}")).Status); + Assert.Equal("critical", await ScalarAsync(owner, $"SELECT severities FROM config_notification_routes WHERE route_id = {seeded}", ct)); + var warningOnly = await Send("POST", "/api/notification-routes", Json, "{\"metric_match\":\"blocking\",\"agent_level\":1,\"severities\":\"warning\"}"); + Assert.Equal(200, warningOnly.Status); + Assert.Equal("warning", await ScalarAsync(owner, $"SELECT severities FROM config_notification_routes WHERE route_id = {(int)warningOnly.Body!["route_id"]!}", ct)); + Assert.Equal(200, (await Send("PUT", $"/api/notification-routes/{seeded}", Json, "{\"severities\":\"both\"}")).Status); + /* Level 0 (Off) and Inherit are both settable. */ Assert.Equal(200, (await Send("PUT", $"/api/notification-routes/{seeded}", Json, "{\"agent_level\":0}")).Status); Assert.Equal("0", await ScalarAsync(owner, $"SELECT agent_level FROM config_notification_routes WHERE route_id = {seeded}", ct)); @@ -401,6 +412,8 @@ async Task Refused(string method, string path, string json, string contains) await Refused("POST", "/api/notification-routes", "{\"metric_match\":\"blocking\"}", "at least one destination or the agent channel"); await Refused("POST", "/api/notification-routes", "{\"metric_match\":\"blocking\",\"agent_level\":1,\"fallback_route_id\":99999}", "Fallback route 99999 does not exist"); await Refused("PUT", $"/api/notification-routes/{seeded}", $"{{\"fallback_route_id\":{seeded}}}", "its own fallback"); + await Refused("POST", "/api/notification-routes", "{\"metric_match\":\"blocking\",\"agent_level\":1,\"severities\":\"major\"}", "severities is critical, warning or both"); + await Refused("POST", "/api/notification-routes", "{\"metric_match\":\"blocking\",\"agent_level\":1,\"severities\":3}", "severities must be text"); await Refused("POST", "/api/notification-routes", "{\"metric_match\":\"blocking\",\"agent_level\":\"one\"}", "agent_level must be a whole number"); await Refused("POST", "/api/notification-routes", "{\"metric_match\":\"blocking\",\"agent_level\":1,\"agent_max_per_hour\":1.5}", "agent_max_per_hour must be a whole number"); diff --git a/Darling/Darling.Tests/AlertResolutionDeliveryTests.cs b/Darling/Darling.Tests/AlertResolutionDeliveryTests.cs index 3b906a8ebf..93028e2317 100644 --- a/Darling/Darling.Tests/AlertResolutionDeliveryTests.cs +++ b/Darling/Darling.Tests/AlertResolutionDeliveryTests.cs @@ -365,6 +365,35 @@ public async Task ASecondClearWithNoFireBetween_SendsNothing() Assert.Equal(AlertDelivery.ChannelNotApplicable, rig.History.Records[^1].NotificationType); } + /// #6928: a clear goes to the channels the fire reached, not to wherever the severity filter would send it now. The fire + /// was Critical and went to the Critical-only route's endpoint. The routes' filters are then swapped (the other endpoint's route has the lower id, so a re-match at the clear would pick it), and the clear still goes to + /// that endpoint and never to the other. + [Fact] + public async Task AClear_GoesToTheChannelTheFireReached_WhateverTheSeverityFiltersSayAtTheClear() + { + using var criticalEndpoint = new CapturingWebhookEndpoint(); + using var warningEndpoint = new CapturingWebhookEndpoint(); + var config = new DarlingConfig(); + config.Webhooks.SlackSendResolutions = true; + NotificationRoute[] RoutesWith(string first, string second) => new[] + { + new NotificationRoute(2, Metric, "", criticalEndpoint.Url, "", "", "", true) { Severities = first }, + new NotificationRoute(1, Metric, "", warningEndpoint.Url, "", "", "", true) { Severities = second }, + }; + config.NotificationRoutes = RoutesWith("critical", "warning"); + var rig = Rig.Build(config); + + await rig.Deliverer.DeliverAsync(Fire() with { Severity = AlertSeverityLevel.Critical }, TestContext.Current.CancellationToken); + Assert.Single(criticalEndpoint.Bodies); + Assert.Empty(warningEndpoint.Bodies); + + config.NotificationRoutes = RoutesWith("warning", "critical"); + await rig.Deliverer.DeliverResolutionAsync(CpuResolution(), cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(2, criticalEndpoint.Bodies.Count); + Assert.Empty(warningEndpoint.Bodies); + } + private static AlertOutcome Fire() => new(ServerKey, Server, Metric, "97%", "90%", Context: null, DetailText: null, NumericCurrentValue: 97, NumericThresholdValue: 90, Muted: false, Severity: null); diff --git a/Darling/Darling.Tests/DarlingManagedRolesTests.cs b/Darling/Darling.Tests/DarlingManagedRolesTests.cs index 0468c9b5f4..9c7c99c1fb 100644 --- a/Darling/Darling.Tests/DarlingManagedRolesTests.cs +++ b/Darling/Darling.Tests/DarlingManagedRolesTests.cs @@ -586,7 +586,7 @@ and viewer writes nothing here. */ Assert.DoesNotMatch(@"GRANT [A-Z ,()a-z_]*(INSERT|UPDATE|DELETE)[^;]* ON config\.investigation_request_alerts TO (?!admin;|mcp;)", flat); /* #6400: mcp (start_practice_request) copies a case's alert rows too, by the ten columns the copy sets, and never updates or deletes a link. */ - Assert.Contains("GRANT INSERT (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json) ON config.investigation_request_alerts TO mcp;", flat, StringComparison.Ordinal); + Assert.Contains("GRANT INSERT (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json, severity) ON config.investigation_request_alerts TO mcp;", flat, StringComparison.Ordinal); Assert.DoesNotMatch(@"GRANT [A-Z ,()a-z_]*(UPDATE|DELETE)[^;]* ON config\.investigation_request_alerts TO mcp;", flat); Assert.DoesNotMatch(@"GRANT (INSERT|UPDATE|DELETE)[^;]* ON config\.(investigation|agent_environment)[a-z_]* TO [a-z_, ]*viewer", flat); } diff --git a/Darling/Darling.Tests/ExternalAlertDrainLifecycleLiveTests.cs b/Darling/Darling.Tests/ExternalAlertDrainLifecycleLiveTests.cs index 1eb7863501..cf7849c8d7 100644 --- a/Darling/Darling.Tests/ExternalAlertDrainLifecycleLiveTests.cs +++ b/Darling/Darling.Tests/ExternalAlertDrainLifecycleLiveTests.cs @@ -512,7 +512,7 @@ await History.RecordAlertAsync(new AlertHistoryRecord( return delivery; }, - (_, _, _, _, _) => Task.FromResult(PagerDutyAnswer), + (_, _, _, _, _, _) => Task.FromResult(PagerDutyAnswer), History, context => IsMuted(context), name => string.Equals(name, "SQL01", StringComparison.OrdinalIgnoreCase) ? new ExternalAlertServer(7, Sql01DisplayName) : null, diff --git a/Darling/Darling.Tests/ExternalAlertDrainLiveTests.cs b/Darling/Darling.Tests/ExternalAlertDrainLiveTests.cs index f3f818adcb..1603a2082b 100644 --- a/Darling/Darling.Tests/ExternalAlertDrainLiveTests.cs +++ b/Darling/Darling.Tests/ExternalAlertDrainLiveTests.cs @@ -218,6 +218,25 @@ await WithDrainAsync(async rig => }); } + /// #6928: the PagerDuty close is routed at the severity the fire went out at, so a severity filter on a route sends the close + /// to the route the fire used. A critical fire closes as Critical and a warning fire as Warning. + [Theory] + [InlineData("critical", AlertSeverityLevel.Critical)] + [InlineData("warning", AlertSeverityLevel.Warning)] + public async Task TheClear_AsksForThePagerDutyCloseAtTheSeverityTheFireWentOutAt(string fireSeverity, AlertSeverityLevel expected) + { + await WithDrainAsync(async rig => + { + await rig.SubmitAsync("fire", "disk", server: "SQL01", severity: fireSeverity); + await rig.Drain.RunPassAsync(rig.Ct); + var clear = await rig.SubmitAsync("clear", "disk", server: "SQL01"); + await rig.Drain.RunPassAsync(rig.Ct); + + Assert.Equal("delivered", await rig.StatusAsync(clear)); + Assert.Equal(new AlertSeverityLevel?[] { expected }, rig.PagerDutySeverities.ToArray()); + }); + } + [Fact] public async Task AClearWithNoFire_IsIgnored_AndAsksForNoResolve() { @@ -487,6 +506,29 @@ await WithDrainAsync(async rig => }); } + /// #6928: an external alert that fired Warning and then climbed to Critical closes the PagerDuty incident at both tiers, so a + /// Warning-only route's key is resolved as well as the Critical-only route's. + [Fact] + public async Task TheClear_AfterAWarningThatEscalatedToCritical_AsksForTheCloseAtBothTiers() + { + await WithDrainAsync(async rig => + { + await rig.SubmitAsync("fire", "disk", server: "SQL01", severity: "warning"); + await rig.Drain.RunPassAsync(rig.Ct); + await rig.SubmitAsync("fire", "disk", server: "SQL01", severity: "critical"); + await rig.Drain.RunPassAsync(rig.Ct); + Assert.Equal("critical", await rig.ScalarAsync("SELECT fired_severity FROM config.external_alert_state WHERE alert_key = 'disk'")); + Assert.Equal("warning,critical", await rig.ScalarAsync("SELECT fired_severities FROM config.external_alert_state WHERE alert_key = 'disk'")); + + var clear = await rig.SubmitAsync("clear", "disk", server: "SQL01"); + await rig.Drain.RunPassAsync(rig.Ct); + + Assert.Equal("delivered", await rig.StatusAsync(clear)); + Assert.Equal( + new AlertSeverityLevel?[] { AlertSeverityLevel.Warning, AlertSeverityLevel.Critical }, rig.PagerDutySeverities.ToArray()); + }); + } + [Fact] public async Task AFireOpenedByARetryThatSucceeds_IsConfirmed_AndItsClearResolvesIt() { @@ -577,9 +619,10 @@ await History.RecordAlertAsync(new AlertHistoryRecord( AlertContextSerializer.Serialize(outcome.Context!))); return delivery; }, - (metric, serverName, serverKey, _, _) => + (metric, serverName, serverKey, _, firedSeverities, _) => { PagerDutyResolves.Add(metric.Split(':')[2]); + PagerDutySeverities.AddRange(firedSeverities); PagerDutyIdentities.Add(serverName + "|" + serverKey); return Task.FromResult(AlertChannelOutcome.Delivered); }, @@ -598,6 +641,9 @@ await History.RecordAlertAsync(new AlertHistoryRecord( public List Sent { get; } = new(); public List PagerDutyResolves { get; } = new(); + /// The severity each PagerDuty resolve was asked to route at (#6928): the tier the fire went out at. + public List PagerDutySeverities { get; } = new(); + /// The server name and server key each PagerDuty resolve was asked to close, as name|key. public List PagerDutyIdentities { get; } = new(); @@ -618,16 +664,17 @@ public async Task AddSourceAsync(string name) => await ScalarAsync($"INSERT INTO config.external_alert_source (source_name, role_name) VALUES ('{name}', 'role_{name}') RETURNING source_id"), CultureInfo.InvariantCulture); - public async Task SubmitAsync(string kind, string key, string? server = null, double? value = null, string source = "nagios") + public async Task SubmitAsync(string kind, string key, string? server = null, double? value = null, string source = "nagios", string severity = "warning") { await using var command = new NpgsqlCommand( "INSERT INTO config.external_alert_pending (source_id, kind, alert_key, severity, title, server_name, value, expires_at)" - + " VALUES ($1, $2, $3, 'warning', 'a title', $4, $5, now() + interval '15 minutes') RETURNING pending_id", _seed); + + " VALUES ($1, $2, $3, $6, 'a title', $4, $5, now() + interval '15 minutes') RETURNING pending_id", _seed); command.Parameters.Add(new NpgsqlParameter { TypedValue = _sourceIds[source] }); command.Parameters.Add(new NpgsqlParameter { TypedValue = kind }); command.Parameters.Add(new NpgsqlParameter { TypedValue = key }); command.Parameters.Add(new NpgsqlParameter { Value = (object?)server ?? DBNull.Value, NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Text }); command.Parameters.Add(new NpgsqlParameter { Value = (object?)value ?? DBNull.Value, NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Double }); + command.Parameters.Add(new NpgsqlParameter { TypedValue = severity }); return Convert.ToInt64(await command.ExecuteScalarAsync(Ct), CultureInfo.InvariantCulture); } diff --git a/Darling/Darling.Tests/ExternalAlertRungTests.cs b/Darling/Darling.Tests/ExternalAlertRungTests.cs index 04537ac643..8692c3ad7f 100644 --- a/Darling/Darling.Tests/ExternalAlertRungTests.cs +++ b/Darling/Darling.Tests/ExternalAlertRungTests.cs @@ -127,7 +127,7 @@ public async Task MigratingAFreshStore_CreatesTheTablesAndTheColumn_AndRerunning "pending_id:bigint:NO,source_id:integer:NO,kind:text:NO,alert_key:text:NO,severity:text:NO,title:text:YES,detail:text:YES,server_name:text:YES,value:double precision:YES,submitted_at:timestamp with time zone:NO,expires_at:timestamp with time zone:NO,status:text:NO,claimed_at:timestamp with time zone:YES,service_instance:text:YES,attempts:integer:NO,next_attempt_at:timestamp with time zone:YES,reason:text:YES,completed_at:timestamp with time zone:YES", await ColumnsAsync(connection, "external_alert_pending", ct)); Assert.Equal( - "source_id:integer:NO,alert_key:text:NO,server_label:text:NO,server_id:integer:NO,server_key:text:NO,server_name:text:NO,firing:boolean:NO,fired_severity:text:YES,last_fired_at:timestamp with time zone:YES", + "source_id:integer:NO,alert_key:text:NO,server_label:text:NO,server_id:integer:NO,server_key:text:NO,server_name:text:NO,firing:boolean:NO,fired_severity:text:YES,last_fired_at:timestamp with time zone:YES,fired_severities:text:YES", await ColumnsAsync(connection, "external_alert_state", ct)); Assert.Equal("external_source:text:YES", await ScalarAsync(connection, "SELECT column_name || ':' || data_type || ':' || is_nullable FROM information_schema.columns WHERE table_schema = 'config' AND table_name = 'config_mute_rules' AND column_name = 'external_source'", ct)); diff --git a/Darling/Darling.Tests/InvestigationPracticeTests.cs b/Darling/Darling.Tests/InvestigationPracticeTests.cs index bb858d6a9f..2d37f0669a 100644 --- a/Darling/Darling.Tests/InvestigationPracticeTests.cs +++ b/Darling/Darling.Tests/InvestigationPracticeTests.cs @@ -336,7 +336,7 @@ await RunAsync(async (dataSource, store, ct) => var writes = 0; var hooks = new AgentActionHooks { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => 2), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => 2), ReadBefore = (_, _, _) => Task.FromResult("{}"), Write = (_, _, _) => { writes++; return Task.FromResult("{}"); }, }; diff --git a/Darling/Darling.Tests/MigrationDataMovingRungCensusPins.cs b/Darling/Darling.Tests/MigrationDataMovingRungCensusPins.cs index 5bd785e1c8..4aed064f33 100644 --- a/Darling/Darling.Tests/MigrationDataMovingRungCensusPins.cs +++ b/Darling/Darling.Tests/MigrationDataMovingRungCensusPins.cs @@ -369,6 +369,14 @@ public sealed class MigrationDataMovingRungCensusPins + "same rung adds is null on every stored row, so each statement reads the table once and takes milliseconds. The ADD " + "COLUMN is nullable with no default and the DROP NOT NULL on server_id is catalog-only, so neither rewrites a row. It does " + "not move the lock-wait floor, so the multiple stays 5."), + new( + 229, + SetsTheFloor: false, + "ADD COLUMN IF NOT EXISTS severities text NOT NULL DEFAULT 'both' plus ADD CONSTRAINT ... CHECK on config.config_notification_routes " + + "(created V131), the notification routes registry (#6928). The ADD COLUMN with a constant default is catalog-only. The CHECK " + + "reads the table once to validate it, and the table is a hand-edited registry of at most a few hundred rows (one per route " + + "an operator made), so the scan takes milliseconds. It reads no collected data and spends none of the lock-wait budget, so " + + "the multiple stays 5."), ]; /// diff --git a/Darling/Darling.Tests/NotificationRouteSeverityLiveTests.cs b/Darling/Darling.Tests/NotificationRouteSeverityLiveTests.cs new file mode 100644 index 0000000000..7ea97a5bcf --- /dev/null +++ b/Darling/Darling.Tests/NotificationRouteSeverityLiveTests.cs @@ -0,0 +1,300 @@ +/* + * 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.Security.Cryptography; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Npgsql; +using PerformanceMonitor.Darling.Service; +using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; +using Xunit; + +namespace Darling.Tests; + +/* #1776 own-store: every fact mints its own scratch database (and the MCP facts a role with a per-run name), so the class is deliberately + NOT [Collection("live-postgres")]. */ + +/// +/// #6928: the route severity filter against a live store. V229 on a store that already holds routes (they read both, the +/// constraint holds the three words, and applying the rung again changes nothing), and the MCP create, update and get round trip +/// AS the mcp role with the grants taken from the managed batch, including the invalid answer for a word that is +/// not critical, warning or both. +/// +public sealed class NotificationRouteSeverityLiveTests +{ + private static readonly string RolePassword = Convert.ToHexString(RandomNumberGenerator.GetBytes(16)); + + private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + + private static DarlingMcpServedPolicy OperatorPolicy() => new(new DarlingMcpWritePolicy.Effective(true, false, false, "ops-lead")); + + private static string Status(string answer) => JsonDocument.Parse(answer).RootElement.GetProperty("status").GetString()!; + + private static int RouteIdOf(string answer) => + JsonDocument.Parse(answer).RootElement.GetProperty("route").GetProperty("route_id").GetInt32(); + + [Fact] + public async Task V229_OnAStoreWithExistingRoutes_ReadsBoth_HoldsTheThreeWords_AndAppliesTwiceWithNoChange() + { + Assert.SkipWhen(string.IsNullOrEmpty(ConnectionString), "Set DARLING_TEST_PG to run the V229 live pin (it mints its own scratch database)."); + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(ConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + var bodySucceeded = false; + try + { + /* A store one rung behind: V229's constraint and column gone, two routes stored (one scoped, one not). */ + await ExecAsync(connection, + "ALTER TABLE config.config_notification_routes DROP CONSTRAINT ck_config_notification_routes_severities, DROP COLUMN severities", ct); + await ExecAsync(connection, "DELETE FROM darling_schema_version WHERE version >= 229", ct); + await ExecAsync(connection, + "INSERT INTO config.config_notification_routes (metric_match, slack_url) VALUES ('Deadlocks Detected', 'sealed-a')", ct); + await ExecAsync(connection, + "INSERT INTO config.config_notification_routes (metric_match, slack_url, scope_server_id) VALUES ('High CPU', 'sealed-b', 11)", ct); + + await PgMigrations.MigrateAsync(connection, ct); + + Assert.Equal("both|both", await ScalarAsync(connection, + "SELECT string_agg(severities, '|' ORDER BY route_id) FROM config.config_notification_routes", ct)); + + /* The three words are accepted and anything else is refused by the CHECK. */ + foreach (var word in new[] { "critical", "warning", "both" }) + { + await ExecAsync(connection, $"UPDATE config.config_notification_routes SET severities = '{word}' WHERE metric_match = 'Deadlocks Detected'", ct); + } + + var refused = await Assert.ThrowsAsync(() => ExecAsync(connection, + "UPDATE config.config_notification_routes SET severities = 'urgent' WHERE metric_match = 'Deadlocks Detected'", ct)); + Assert.Equal("23514", refused.SqlState); + var nullRefused = await Assert.ThrowsAsync(() => ExecAsync(connection, + "UPDATE config.config_notification_routes SET severities = NULL WHERE metric_match = 'Deadlocks Detected'", ct)); + Assert.Equal("23502", nullRefused.SqlState); + + /* The rung applied again changes nothing: the stored word survives and the constraint is not added twice. */ + await ExecAsync(connection, "UPDATE config.config_notification_routes SET severities = 'critical' WHERE metric_match = 'High CPU'", ct); + await ExecAsync(connection, "DELETE FROM darling_schema_version WHERE version >= 229", ct); + await PgMigrations.MigrateAsync(connection, ct); + Assert.Equal("both|critical", await ScalarAsync(connection, + "SELECT string_agg(severities, '|' ORDER BY route_id) FROM config.config_notification_routes", ct)); + Assert.Equal("1", await ScalarAsync(connection, + "SELECT count(*) FROM pg_constraint WHERE conname = 'ck_config_notification_routes_severities'", ct)); + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, static (_, _) => Task.CompletedTask); + } + } + + [Fact] + public async Task Mcp_CreateUpdateAndGet_RoundTripTheSeverityFilter_AndABadWordIsInvalid() + { + await WithMcpRoleAsync(async (owner, asMcp, ct) => + { + /* Left out, a new route takes both. */ + var plain = await DarlingMcpAlertTools.CreateNotificationRoute(asMcp, "Blocking", served: OperatorPolicy(), cancellationToken: ct); + Assert.Equal("created", Status(plain)); + Assert.Equal("both", await ScalarAsync(owner, $"SELECT severities FROM config.config_notification_routes WHERE route_id = {RouteIdOf(plain)}", ct)); + + /* Any case is accepted and stored lower. */ + var created = await DarlingMcpAlertTools.CreateNotificationRoute( + asMcp, "Deadlocks", severities: "Critical", served: OperatorPolicy(), cancellationToken: ct); + Assert.Equal("created", Status(created)); + var routeId = RouteIdOf(created); + Assert.Equal("critical", await ScalarAsync(owner, $"SELECT severities FROM config.config_notification_routes WHERE route_id = {routeId}", ct)); + Assert.Equal("critical", await ReadSeveritiesAsync(asMcp, routeId, ct)); + + /* An update changes it, and a field left out keeps it. */ + Assert.Equal("updated", Status(await DarlingMcpAlertTools.UpdateNotificationRoute( + asMcp, routeId, severities: "warning", served: OperatorPolicy(), cancellationToken: ct))); + Assert.Equal("warning", await ReadSeveritiesAsync(asMcp, routeId, ct)); + Assert.Equal("updated", Status(await DarlingMcpAlertTools.UpdateNotificationRoute( + asMcp, routeId, enabled: false, served: OperatorPolicy(), cancellationToken: ct))); + Assert.Equal("warning", await ReadSeveritiesAsync(asMcp, routeId, ct)); + + /* A bad word is invalid on both tools, and nothing changes or is stored. */ + var badCreate = await DarlingMcpAlertTools.CreateNotificationRoute( + asMcp, "Blocking Wait", severities: "urgent", served: OperatorPolicy(), cancellationToken: ct); + Assert.Equal("invalid", Status(badCreate)); + Assert.Contains("severities is critical, warning or both.", badCreate, StringComparison.Ordinal); + Assert.Equal("2", await ScalarAsync(owner, "SELECT count(*) FROM config.config_notification_routes", ct)); + Assert.Equal("invalid", Status(await DarlingMcpAlertTools.UpdateNotificationRoute( + asMcp, routeId, severities: "urgent", served: OperatorPolicy(), cancellationToken: ct))); + Assert.Equal("warning", await ScalarAsync(owner, $"SELECT severities FROM config.config_notification_routes WHERE route_id = {routeId}", ct)); + }); + } + + /// The viewer and web writer: changing the severity filter of an agent route writes a route_agent_changed row naming it with the + /// words before and after, and the same change to a route with no agent level writes nothing. + [Fact] + public async Task Writer_ChangingTheSeverityFilterOfAnAgentRoute_IsJournaled_AndOfARouteWithNoAgentLevelIsNot() + { + Assert.SkipWhen(string.IsNullOrEmpty(ConnectionString), "Set DARLING_TEST_PG to run the route severity journal pin (it mints its own scratch database)."); + var ct = TestContext.Current.CancellationToken; + + await using var scratch = await ScratchPostgres.CreateAsync(ConnectionString!, ct); + await using (var connection = new NpgsqlConnection(scratch.ConnectionString)) + { + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + } + + await using var owner = NpgsqlDataSource.Create(scratch.ConnectionString); + var bodySucceeded = false; + try + { + var proxies = new WebhookProxies(null, null, null, null); + var agentRoute = await NotificationRouteWriter.SaveAsync( + owner, new NotificationRouteDraft { MetricMatch = "Blocking", SmtpRecipients = "ops@example.com", AgentLevel = 2 }, null, proxies, null, null, 30, ct); + var plainRoute = await NotificationRouteWriter.SaveAsync( + owner, new NotificationRouteDraft { MetricMatch = "Deadlocks", SmtpRecipients = "ops@example.com" }, null, proxies, null, null, 30, ct); + const string Journal = "SELECT count(*) FROM config.investigation_journal WHERE event = 'route_agent_changed'"; + + /* Both to warning: Critical alerts move to a broader agent route, so the change is journaled. */ + var stored = (await NotificationRouteWriter.ReadStoredAsync(owner, agentRoute, 30, ct))!; + var typed = stored.Clone(); + typed.Severities = "warning"; + await NotificationRouteWriter.SaveAsync(owner, typed, stored, proxies, null, null, 30, ct, "bob"); + Assert.Equal("1", await ScalarAsync(owner, Journal, ct)); + Assert.Equal("person:bob|changed severities|both|warning", await ScalarAsync(owner, + "SELECT actor || '|' || reason || '|' || (detail->'before'->>'severities') || '|' || (detail->'after'->>'severities') FROM config.investigation_journal WHERE event = 'route_agent_changed'", ct)); + + /* A route with no agent level is not in the agent lookup. */ + var plainStored = (await NotificationRouteWriter.ReadStoredAsync(owner, plainRoute, 30, ct))!; + var plainTyped = plainStored.Clone(); + plainTyped.Severities = "critical"; + await NotificationRouteWriter.SaveAsync(owner, plainTyped, plainStored, proxies, null, null, 30, ct, "bob"); + Assert.Equal("critical", await ScalarAsync(owner, $"SELECT severities FROM config.config_notification_routes WHERE route_id = {plainRoute}", ct)); + Assert.Equal("1", await ScalarAsync(owner, Journal, ct)); + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, static (_, _) => Task.CompletedTask); + } + } + + /// The MCP update tool: an operator who changes the severity filter of an agent route gets a route_agent_changed row (through the + /// update function's before row), and the same change to a route with no agent level writes none. + [Fact] + public async Task Mcp_ChangingTheSeverityFilterOfAnAgentRoute_IsJournaled_AndOfARouteWithNoAgentLevelIsNot() + { + await WithMcpRoleAsync(async (owner, asMcp, ct) => + { + var agentRoute = RouteIdOf(await DarlingMcpAlertTools.CreateNotificationRoute( + asMcp, "Blocking", agent_level: 2, served: OperatorPolicy(), cancellationToken: ct)); + var plainRoute = RouteIdOf(await DarlingMcpAlertTools.CreateNotificationRoute( + asMcp, "Deadlocks", served: OperatorPolicy(), cancellationToken: ct)); + const string Journal = "SELECT count(*) FROM config.investigation_journal WHERE event = 'route_agent_changed'"; + Assert.Equal("1", await ScalarAsync(owner, Journal, ct)); + + Assert.Equal("updated", Status(await DarlingMcpAlertTools.UpdateNotificationRoute( + asMcp, agentRoute, severities: "warning", served: OperatorPolicy(), cancellationToken: ct))); + Assert.Equal("2", await ScalarAsync(owner, Journal, ct)); + Assert.Equal("operator:ops-lead|changed severities|both|warning", await ScalarAsync(owner, + $"SELECT actor || '|' || reason || '|' || (detail->'before'->>'severities') || '|' || (detail->'after'->>'severities') FROM config.investigation_journal WHERE route_id = {agentRoute} AND event = 'route_agent_changed' ORDER BY journal_id DESC LIMIT 1", ct)); + + /* The same word again changes nothing, and a route with no agent level writes no row. */ + await DarlingMcpAlertTools.UpdateNotificationRoute(asMcp, agentRoute, severities: "warning", served: OperatorPolicy(), cancellationToken: ct); + Assert.Equal("updated", Status(await DarlingMcpAlertTools.UpdateNotificationRoute( + asMcp, plainRoute, severities: "critical", served: OperatorPolicy(), cancellationToken: ct))); + Assert.Equal("2", await ScalarAsync(owner, Journal, ct)); + }); + } + + private static async Task ReadSeveritiesAsync(NpgsqlDataSource source, int routeId, CancellationToken ct) => + JsonDocument.Parse(await DarlingMcpAlertTools.GetNotificationRoutes(source, ct)).RootElement + .GetProperty("routes").EnumerateArray().Single(r => r.GetProperty("route_id").GetInt32() == routeId) + .GetProperty("severities").GetString()!; + + private static async Task WithMcpRoleAsync(Func body) + { + Assert.SkipWhen(string.IsNullOrEmpty(ConnectionString), "Set DARLING_TEST_PG to a Postgres connection string to run the route severity live tests."); + + var ct = TestContext.Current.CancellationToken; + var roleName = "route_sev_" + Guid.NewGuid().ToString("N")[..8]; + await using var scratch = await ScratchPostgres.CreateAsync(ConnectionString!, ct); + await using (var connection = new NpgsqlConnection(scratch.ConnectionString)) + { + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + } + + await using var owner = NpgsqlDataSource.Create(scratch.ConnectionString); + var bodySucceeded = false; + try + { + /* The functions and the journal trigger are created by the owner, as the managed batch does; the batch's own grants, retargeted. */ + await ExecAsync(owner, DarlingManagedRoles.BuildNotificationRouteFunctionsSql("config"), ct); + await ExecAsync(owner, DarlingManagedRoles.BuildJournalEventRulesSql("config"), ct); + await ExecAsync(owner, $"CREATE ROLE {roleName} LOGIN NOSUPERUSER PASSWORD '{RolePassword}'", ct); + foreach (var statement in ViewerGrantReplay.StatementsFor(roleName, scratch.DatabaseName, "mcp")) + { + await ExecAsync(owner, statement, ct); + } + + var executes = ViewerGrantReplay.BatchStatements().Where(s => + s.StartsWith("GRANT EXECUTE ON FUNCTION config.create_notification_route", StringComparison.Ordinal) + || s.StartsWith("GRANT EXECUTE ON FUNCTION config.update_notification_route", StringComparison.Ordinal) + || s.StartsWith("GRANT EXECUTE ON FUNCTION config.set_notification_route_agent", StringComparison.Ordinal)).ToList(); + Assert.Equal(3, executes.Count); + foreach (var execute in executes) + { + var at = execute.LastIndexOf(" TO ", StringComparison.Ordinal); + Assert.Equal("mcp", execute[(at + 4)..].Trim()); + await ExecAsync(owner, execute[..at] + " TO " + roleName, ct); + } + + await using var asMcp = NpgsqlDataSource.Create(new NpgsqlConnectionStringBuilder(scratch.ConnectionString) + { + Username = roleName, + Password = RolePassword, + Pooling = false, + }.ConnectionString); + + await body(owner, asMcp, ct); + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunOwnedAsync(bodySucceeded, () => ExecAsync(owner, $"DROP OWNED BY {roleName}; DROP ROLE IF EXISTS {roleName};", CancellationToken.None)); + } + } + + private static async Task ExecAsync(NpgsqlConnection connection, string sql, CancellationToken ct) + { + await using var command = new NpgsqlCommand(sql, connection); + await command.ExecuteNonQueryAsync(ct); + } + + private static async Task ExecAsync(NpgsqlDataSource source, string sql, CancellationToken ct) + { + await using var command = source.CreateCommand(sql); + await command.ExecuteNonQueryAsync(ct); + } + + private static async Task ScalarAsync(NpgsqlConnection connection, string sql, CancellationToken ct) + { + await using var command = new NpgsqlCommand(sql, connection); + return Convert.ToString(await command.ExecuteScalarAsync(ct), CultureInfo.InvariantCulture); + } + + private static async Task ScalarAsync(NpgsqlDataSource source, string sql, CancellationToken ct) + { + await using var command = source.CreateCommand(sql); + return Convert.ToString(await command.ExecuteScalarAsync(ct), CultureInfo.InvariantCulture); + } +} diff --git a/Darling/Darling.Tests/NotificationRouteSeverityTests.cs b/Darling/Darling.Tests/NotificationRouteSeverityTests.cs new file mode 100644 index 0000000000..df9667744c --- /dev/null +++ b/Darling/Darling.Tests/NotificationRouteSeverityTests.cs @@ -0,0 +1,319 @@ +/* + * 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.Collections.Generic; +using System.Linq; +using System.Text.Json; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging.Abstractions; +using PerformanceMonitor.Alerting; +using PerformanceMonitor.Darling.Service; +using PerformanceMonitor.Notifications; +using Xunit; + +namespace Darling.Tests; + +/// +/// #6928: a notification route can be limited by severity (Critical only, Warning only, or both). These pin the router's +/// filter (default both, an excluded severity falls through to the next route and then the defaults), the tier rule the send +/// uses, the issue's three-channel example driven through the deliverer to loopback endpoints, and the PagerDuty close +/// riding the severity the fire went out at. +/// +public sealed class NotificationRouteSeverityTests +{ + private const string Metric = "High CPU"; + + private static NotificationRoute Route(int id, string match, string severities = "both", string slack = "", string teams = "", + string generic = "", string pagerDuty = "") => + new(id, match, teams, slack, generic, pagerDuty, "", true) { Severities = severities }; + + private static DarlingAlertSettings Defaults(string pagerDutyKey = "") + { + var config = new DarlingConfig(); + config.Webhooks.PagerDutyRoutingKey = pagerDutyKey; + return new DarlingAlertSettings(config); + } + + /// A route nobody gave a filter to reads both, so every route stored before the filter keeps its effect. + [Fact] + public void ARouteWithNoSeverityFilter_ReadsBoth_AndTakesEveryTier() + { + var route = new NotificationRoute(1, Metric, "", "https://slack.example.invalid/a", "", "", "", true); + + Assert.Equal("both", route.Severities); + Assert.True(route.AcceptsSeverity(AlertSeverityLevel.Critical)); + Assert.True(route.AcceptsSeverity(AlertSeverityLevel.Warning)); + Assert.True(route.AcceptsSeverity(null)); + + var decision = NotificationRouter.Resolve(Metric, new[] { route }, Defaults(), severity: AlertSeverityLevel.Warning); + Assert.Equal("https://slack.example.invalid/a", decision.Slack.Destination); + Assert.Equal("both", NotificationRouteKinds.NormalizeSeverities(null)); + Assert.Equal("both", NotificationRouteKinds.NormalizeSeverities("urgent")); + Assert.Equal("critical", NotificationRouteKinds.NormalizeSeverities(" Critical ")); + } + + /// The issue's example: a Critical-only channel, a Warning-only channel and a both channel. Each alert lands on its own. + [Fact] + public void TheThreeChannelExample_EachAlertLandsWhereItShould() + { + var routes = new[] + { + Route(1, Metric, "critical", slack: "https://slack.example.invalid/critical"), + Route(2, Metric, "warning", teams: "https://teams.example.invalid/warning"), + Route(3, Metric, "both", generic: "https://generic.example.invalid/both"), + }; + + var critical = NotificationRouter.Resolve(Metric, routes, Defaults(), severity: AlertSeverityLevel.Critical); + Assert.Equal("https://slack.example.invalid/critical", critical.Slack.Destination); + Assert.Null(critical.Teams.Destination); + Assert.Equal("https://generic.example.invalid/both", critical.Generic.Destination); + + var warning = NotificationRouter.Resolve(Metric, routes, Defaults(), severity: AlertSeverityLevel.Warning); + Assert.Null(warning.Slack.Destination); + Assert.Equal("https://teams.example.invalid/warning", warning.Teams.Destination); + Assert.Equal("https://generic.example.invalid/both", warning.Generic.Destination); + } + + /// A route whose filter excludes the alert is skipped: the next matching route answers, and when none does the defaults do. + [Fact] + public void AnExcludedSeverity_FallsThroughToTheNextRoute_ThenToTheDefaults() + { + var exactCritical = Route(1, Metric, "critical", pagerDuty: "rk-exact-critical"); + var familyBoth = Route(2, AlertFamily.Of(Metric), "both", pagerDuty: "rk-family"); + var settings = Defaults("rk-default"); + + Assert.Equal("rk-exact-critical", + NotificationRouter.Resolve(Metric, new[] { exactCritical, familyBoth }, settings, severity: AlertSeverityLevel.Critical).PagerDuty.Destination); + + var next = NotificationRouter.Resolve(Metric, new[] { exactCritical, familyBoth }, settings, severity: AlertSeverityLevel.Warning); + Assert.Equal("rk-family", next.PagerDuty.Destination); + Assert.Equal(RouteSource.Family, next.PagerDuty.Source); + + var defaulted = NotificationRouter.Resolve(Metric, new[] { exactCritical }, settings, severity: AlertSeverityLevel.Warning); + Assert.Equal("rk-default", defaulted.PagerDuty.Destination); + Assert.Equal(RouteSource.Default, defaulted.PagerDuty.Source); + Assert.True(defaulted.IsAllDefaults); + + /* A caller that does not know the tier is taken by every route, which is how Lite and the agent gate behave. */ + Assert.Equal("rk-exact-critical", NotificationRouter.Resolve(Metric, new[] { exactCritical, familyBoth }, settings).PagerDuty.Destination); + } + + /// The agent route search drops an excluded route too, so a Warning alert does not open an investigation under a Critical-only route. + [Fact] + public void ResolveAgent_SkipsARouteWhoseFilterExcludesTheAlert() + { + var criticalAgent = Route(1, Metric, "critical") with { AgentLevel = NotificationRoute.AgentLevelInvestigate }; + + Assert.Same(criticalAgent, NotificationRouter.ResolveAgent(Metric, new[] { criticalAgent }, severity: AlertSeverityLevel.Critical)); + Assert.Null(NotificationRouter.ResolveAgent(Metric, new[] { criticalAgent }, severity: AlertSeverityLevel.Warning)); + } + + /// The tier the send gives a route filter: the fire site's tier when it carries one, else the badge (red is Critical, every other badge Warning). + [Theory] + [InlineData("Capture Down", null, AlertSeverityLevel.Critical)] // red badge + [InlineData("High CPU", null, AlertSeverityLevel.Warning)] // amber badge + [InlineData("Blocking Detected", null, AlertSeverityLevel.Warning)] // ALERT badge + [InlineData("A Metric Nobody Mapped", null, AlertSeverityLevel.Warning)] // INFO badge + [InlineData("Capture Down", AlertSeverityLevel.Warning, AlertSeverityLevel.Warning)] // fire site beats a red badge + [InlineData("High CPU", AlertSeverityLevel.Critical, AlertSeverityLevel.Critical)] // fire site beats an amber badge + public void TheTier_IsTheFireSitesElseTheBadge(string metric, AlertSeverityLevel? fireSite, AlertSeverityLevel expected) => + Assert.Equal(expected, AlertSeverity.RouteLevelOf(metric, fireSite)); + + /// The issue's three channels driven through the deliverer: a Critical alert and a Warning alert reach the loopback endpoints their routes name. + [Fact] + public async Task TheThreeChannelExample_ThroughTheDeliverer_ReachesTheEndpointsEachAlertBelongsTo() + { + using var criticalSlack = new CapturingWebhookEndpoint(); + using var warningTeams = new CapturingWebhookEndpoint(); + using var bothGeneric = new CapturingWebhookEndpoint(); + var config = new DarlingConfig(); + config.NotificationRoutes = new[] + { + Route(1, Metric, "critical", slack: criticalSlack.Url), + Route(2, Metric, "warning", teams: warningTeams.Url), + Route(3, Metric, "both", generic: bothGeneric.Url), + }; + var settings = new DarlingAlertSettings(config); + var history = new DiscardingHistoryStore(); + var webhooks = new WebhookAlertService(settings, DarlingAlertDeliverer.Branding, NullLogger.Instance, history); + var deliverer = new DarlingAlertDeliverer(settings, history, webhooks, NullLogger.Instance); + + /* Two servers, so the two fires are not folded by the per-server cooldown. */ + await deliverer.DeliverAsync(Fire("7", "CRIT-SRV", AlertSeverityLevel.Critical), TestContext.Current.CancellationToken); + await deliverer.DeliverAsync(Fire("8", "WARN-SRV", AlertSeverityLevel.Warning), TestContext.Current.CancellationToken); + + Assert.Contains("CRIT-SRV", Assert.Single(criticalSlack.Bodies), StringComparison.Ordinal); + Assert.Contains("WARN-SRV", Assert.Single(warningTeams.Bodies), StringComparison.Ordinal); + Assert.Equal(2, bothGeneric.Bodies.Count); + Assert.Contains(bothGeneric.Bodies, b => b.Contains("CRIT-SRV", StringComparison.Ordinal)); + Assert.Contains(bothGeneric.Bodies, b => b.Contains("WARN-SRV", StringComparison.Ordinal)); + } + + /// The PagerDuty close routes at the severity the fire went out at: each tier's close reaches the key its fire's route carried. + [Theory] + [InlineData(AlertSeverityLevel.Critical, "rk-critical")] + [InlineData(AlertSeverityLevel.Warning, "rk-warning")] + public async Task ThePagerDutyClose_UsesTheFiresSeverity_ToPickTheKey(AlertSeverityLevel firedSeverity, string expectedKey) + { + var config = new DarlingConfig(); + config.Webhooks.PagerDutyAutoResolve = true; + config.NotificationRoutes = new[] + { + Route(1, Metric, "critical", pagerDuty: "rk-critical"), + Route(2, Metric, "warning", pagerDuty: "rk-warning"), + }; + var settings = new DarlingAlertSettings(config); + var posted = new List(); + var webhooks = new WebhookAlertService(settings, DarlingAlertDeliverer.Branding, NullLogger.Instance, new DiscardingHistoryStore()) + { + PostPagerDutyAsyncOverride = (_, payload) => + { + posted.Add(payload); + return Task.FromResult(null); + }, + }; + + var outcome = await webhooks.TrySendPagerDutyResolveAsync( + Metric, "SQL01", "7", "back to normal", firedSeverity, TestContext.Current.CancellationToken); + + Assert.Equal(AlertChannelOutcome.Delivered, outcome); + var body = JsonDocument.Parse(Assert.Single(posted)).RootElement; + Assert.Equal(expectedKey, body.GetProperty("routing_key").GetString()); + Assert.Equal("resolve", body.GetProperty("event_action").GetString()); + } + + /// An episode that fired at Warning and climbed to Critical closes the incident on each route's key, and a route that + /// takes both tiers is closed once. + [Fact] + public async Task ThePagerDutyClose_ForAnEpisodeAtBothTiers_ReachesEachKeyOnce() + { + var config = new DarlingConfig(); + config.Webhooks.PagerDutyAutoResolve = true; + config.NotificationRoutes = new[] + { + Route(1, Metric, "critical", pagerDuty: "rk-critical"), + Route(2, Metric, "warning", pagerDuty: "rk-warning"), + }; + var posted = new List(); + var webhooks = new WebhookAlertService(new DarlingAlertSettings(config), DarlingAlertDeliverer.Branding, NullLogger.Instance, new DiscardingHistoryStore()) + { + PostPagerDutyAsyncOverride = (_, payload) => + { + posted.Add(payload); + return Task.FromResult(null); + }, + }; + + var outcome = await webhooks.TrySendPagerDutyResolveAsync( + Metric, "SQL01", "7", "back to normal", new AlertSeverityLevel?[] { AlertSeverityLevel.Warning, AlertSeverityLevel.Critical }, + TestContext.Current.CancellationToken); + + Assert.Equal(AlertChannelOutcome.Delivered, outcome); + Assert.Equal( + new[] { "rk-critical", "rk-warning" }, + posted.Select(p => JsonDocument.Parse(p).RootElement.GetProperty("routing_key").GetString()!).OrderBy(k => k, StringComparer.Ordinal).ToArray()); + + posted.Clear(); + var both = new DarlingConfig(); + both.Webhooks.PagerDutyAutoResolve = true; + both.NotificationRoutes = new[] { Route(1, Metric, "both", pagerDuty: "rk-both") }; + var bothWebhooks = new WebhookAlertService(new DarlingAlertSettings(both), DarlingAlertDeliverer.Branding, NullLogger.Instance, new DiscardingHistoryStore()) + { + PostPagerDutyAsyncOverride = (_, payload) => + { + posted.Add(payload); + return Task.FromResult(null); + }, + }; + await bothWebhooks.TrySendPagerDutyResolveAsync( + Metric, "SQL01", "7", "back to normal", new AlertSeverityLevel?[] { AlertSeverityLevel.Warning, AlertSeverityLevel.Critical }, + TestContext.Current.CancellationToken); + Assert.Single(posted); + } + + /// The deliverer path: Warning-only key A, Critical-only key B, auto-resolve on. The alert fires Warning, escalates to + /// Critical and clears, and the clear resolves the incident on A as well as on B. + [Fact] + public async Task AWarningThatEscalatesToCritical_ClosesThePagerDutyIncidentOnBothKeys() + { + var config = new DarlingConfig(); + config.Webhooks.PagerDutyAutoResolve = true; + config.NotificationRoutes = new[] + { + Route(1, Metric, "warning", pagerDuty: "rk-warning"), + Route(2, Metric, "critical", pagerDuty: "rk-critical"), + }; + var settings = new DarlingAlertSettings(config); + var posted = new List(); + var history = new CapturingHistoryStore(); + var webhooks = new WebhookAlertService(settings, DarlingAlertDeliverer.Branding, NullLogger.Instance, history) + { + PostPagerDutyAsyncOverride = (_, payload) => + { + posted.Add(payload); + return Task.FromResult(null); + }, + }; + var deliverer = new DarlingAlertDeliverer(settings, history, webhooks, NullLogger.Instance); + + await deliverer.DeliverAsync(Fire("7", "SQL01", AlertSeverityLevel.Warning), TestContext.Current.CancellationToken); + await deliverer.DeliverAsync( + Fire("7", "SQL01", AlertSeverityLevel.Critical) with { Context = new AlertContext { EscalationBypassesCooldown = true } }, + TestContext.Current.CancellationToken); + posted.Clear(); + await deliverer.DeliverResolutionAsync( + new AlertResolution("7", "SQL01", Metric, "CPU Resolved", "SQL01: CPU back to normal"), + cancellationToken: TestContext.Current.CancellationToken); + + var resolves = posted + .Select(p => JsonDocument.Parse(p).RootElement) + .Where(e => e.GetProperty("event_action").GetString() == "resolve") + .Select(e => e.GetProperty("routing_key").GetString()!) + .OrderBy(k => k, StringComparer.Ordinal) + .ToArray(); + Assert.Equal(new[] { "rk-critical", "rk-warning" }, resolves); + } + + /// A chat channel: the same escalation sends the clear to the Warning-only channel as well as the Critical-only one. + [Fact] + public async Task AWarningThatEscalatesToCritical_SendsTheClearToTheWarningOnlyChatChannelToo() + { + using var warningSlack = new CapturingWebhookEndpoint(); + using var criticalSlack = new CapturingWebhookEndpoint(); + var config = new DarlingConfig(); + config.Webhooks.SlackSendResolutions = true; + config.NotificationRoutes = new[] + { + Route(1, Metric, "warning", slack: warningSlack.Url), + Route(2, Metric, "critical", slack: criticalSlack.Url), + }; + var settings = new DarlingAlertSettings(config); + var history = new CapturingHistoryStore(); + var webhooks = new WebhookAlertService(settings, DarlingAlertDeliverer.Branding, NullLogger.Instance, history); + var deliverer = new DarlingAlertDeliverer(settings, history, webhooks, NullLogger.Instance); + + await deliverer.DeliverAsync(Fire("7", "SQL01", AlertSeverityLevel.Warning), TestContext.Current.CancellationToken); + await deliverer.DeliverAsync( + Fire("7", "SQL01", AlertSeverityLevel.Critical) with { Context = new AlertContext { EscalationBypassesCooldown = true } }, + TestContext.Current.CancellationToken); + Assert.Single(warningSlack.Bodies); + Assert.Single(criticalSlack.Bodies); + await deliverer.DeliverResolutionAsync( + new AlertResolution("7", "SQL01", Metric, "CPU Resolved", "SQL01: CPU back to normal"), + cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(2, warningSlack.Bodies.Count); + Assert.Equal(2, criticalSlack.Bodies.Count); + Assert.Contains("Resolved", warningSlack.Bodies[1], StringComparison.Ordinal); + } + + private static AlertOutcome Fire(string serverKey, string server, AlertSeverityLevel severity) => + new(serverKey, server, Metric, "97%", "90%", Context: null, DetailText: null, + NumericCurrentValue: 97, NumericThresholdValue: 90, Muted: false, Severity: severity); +} diff --git a/Darling/Darling.Tests/NotificationRoutesRungTests.cs b/Darling/Darling.Tests/NotificationRoutesRungTests.cs index c2c0686767..d8614304e2 100644 --- a/Darling/Darling.Tests/NotificationRoutesRungTests.cs +++ b/Darling/Darling.Tests/NotificationRoutesRungTests.cs @@ -159,7 +159,7 @@ public void TheViewerCarve_ClassifiesEveryColumn_MatchingTheParentForSharedNames var parent = DarlingManagedRoles.ViewerRestrictedConfigTables.Single(a => a.Table == "config_notification"); Assert.Equal(new[] { "teams_url", "slack_url", "generic_url", "pagerduty_routing_key", "signing_key", "previous_signing_key" }, acl.SecretColumns); - Assert.Equal(new[] { "route_id", "metric_match", "smtp_recipients", "configured_channels", "enabled", "modified_at", "scope_server_id", "scope_tag_id" }.Concat(AgentColumns).Concat(new[] { "fallback_route_id", "previous_signing_key_until" }), acl.NonSecretColumns); + Assert.Equal(new[] { "route_id", "metric_match", "smtp_recipients", "configured_channels", "enabled", "modified_at", "scope_server_id", "scope_tag_id" }.Concat(AgentColumns).Concat(new[] { "fallback_route_id", "previous_signing_key_until", "severities" }), acl.NonSecretColumns); foreach (var column in DestinationColumns) { @@ -169,7 +169,7 @@ public void TheViewerCarve_ClassifiesEveryColumn_MatchingTheParentForSharedNames } /* Every column in the DDL is in exactly one list. */ - var ddlColumns = new[] { "route_id", "metric_match" }.Concat(DestinationColumns).Concat(new[] { "configured_channels", "enabled", "modified_at", "scope_server_id", "scope_tag_id" }).Concat(AgentColumns).Concat(new[] { "signing_key", "previous_signing_key", "previous_signing_key_until", "fallback_route_id" }).ToList(); + var ddlColumns = new[] { "route_id", "metric_match" }.Concat(DestinationColumns).Concat(new[] { "configured_channels", "enabled", "modified_at", "scope_server_id", "scope_tag_id" }).Concat(AgentColumns).Concat(new[] { "signing_key", "previous_signing_key", "previous_signing_key_until", "fallback_route_id", "severities" }).ToList(); var classified = acl.SecretColumns.Concat(acl.NonSecretColumns).ToList(); Assert.Equal(ddlColumns.OrderBy(c => c, StringComparer.Ordinal), classified.OrderBy(c => c, StringComparer.Ordinal)); Assert.Equal(classified.Count, classified.Distinct(StringComparer.Ordinal).Count()); @@ -243,7 +243,7 @@ public void TheServiceReader_TheViewerWriter_AndTheDdl_NameTheSameColumnsInTheSa below is unchanged. */ /* V193 (#5998): the signing key columns come last on the service read; the viewer reads them by their own statements. */ /* #6003 (D8): fallback_route_id follows them, set by init like the agent columns; the viewer reads and writes it since #6036. */ - Assert.Equal(new[] { "route_id", "metric_match" }.Concat(DestinationColumns).Concat(new[] { "enabled", "scope_server_id", "scope_tag_id" }).Concat(AgentColumns).Concat(new[] { "signing_key", "previous_signing_key", "previous_signing_key_until", "fallback_route_id" }), serviceSelect); + Assert.Equal(new[] { "route_id", "metric_match" }.Concat(DestinationColumns).Concat(new[] { "enabled", "scope_server_id", "scope_tag_id" }).Concat(AgentColumns).Concat(new[] { "signing_key", "previous_signing_key", "previous_signing_key_until", "fallback_route_id", "severities" }), serviceSelect); Assert.EndsWith("ORDER BY route_id", StoreConfigProvider.NotificationRoutesSelectSql, StringComparison.Ordinal); /* The constructor's parameter order IS the SELECT's order, so a reader ordinal cannot re-map a column. */ @@ -257,7 +257,7 @@ below is unchanged. */ ColumnsOf(ViewerDataService.NotificationRoutesSelectNoSecretSql, "SELECT", "FROM")); var insert = ColumnsOf(ViewerDataService.NotificationRouteInsertSql, "(", ")"); - Assert.Equal(new[] { "metric_match" }.Concat(DestinationColumns).Concat(new[] { "enabled", "scope_server_id", "scope_tag_id" }).Concat(AgentColumns).Concat(new[] { "fallback_route_id", "modified_at" }), insert); + Assert.Equal(new[] { "metric_match" }.Concat(DestinationColumns).Concat(new[] { "enabled", "scope_server_id", "scope_tag_id" }).Concat(AgentColumns).Concat(new[] { "fallback_route_id", "severities", "modified_at" }), insert); Assert.DoesNotContain("route_id", insert); Assert.Contains("RETURNING route_id", ViewerDataService.NotificationRouteInsertSql, StringComparison.Ordinal); @@ -471,6 +471,7 @@ public async Task InsertRoute_BumpsTheBeacon_ReloadsThroughTheProvider_AndResolv insert.Parameters.Add(new NpgsqlParameter { TypedValue = NotificationRoute.DefaultAgentExpiryMinutes }); insert.Parameters.Add(new NpgsqlParameter { Value = DBNull.Value, NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Boolean }); insert.Parameters.Add(new NpgsqlParameter { Value = DBNull.Value, NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Integer }); /* fallback_route_id (#6036) */ + insert.Parameters.Add(new NpgsqlParameter { TypedValue = "both" }); /* severities (V229, #6928) */ routeId = Convert.ToInt32(await insert.ExecuteScalarAsync(ct), CultureInfo.InvariantCulture); } diff --git a/Darling/Darling.Tests/PlanForceCheckLiveTests.cs b/Darling/Darling.Tests/PlanForceCheckLiveTests.cs index 48946d7bfa..af37a674c7 100644 --- a/Darling/Darling.Tests/PlanForceCheckLiveTests.cs +++ b/Darling/Darling.Tests/PlanForceCheckLiveTests.cs @@ -363,7 +363,7 @@ await RunAsync(async (dataSource, connection, at, ct) => new AgentActionTarget(ServerId, "srv-7", NewPlan.ToString(CultureInfo.InvariantCulture), Database)); var hooks = new ServerActionHooks { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => 3), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => 3), ReadCredential = (serverId, token) => store.ReadServerCredentialStateAsync(dataSource, serverId, token), }; var decision = await store.AuthorizeServerActionAsync(call, hooks, ct); diff --git a/Darling/Darling.Tests/RungRewindArtifacts.cs b/Darling/Darling.Tests/RungRewindArtifacts.cs index 123cae6d21..8277f25bc5 100644 --- a/Darling/Darling.Tests/RungRewindArtifacts.cs +++ b/Darling/Darling.Tests/RungRewindArtifacts.cs @@ -595,6 +595,12 @@ ALTER TABLE config.config_alert_settings DROP COLUMN IF EXISTS blocking_critical_per_hour, DROP COLUMN IF EXISTS blocking_warn_wait_seconds, DROP COLUMN IF EXISTS blocking_critical_wait_seconds;"), + // V229 (#6928): the severity filter on the notification routes. The column is the probe's newest sentinel. The rewind drops the column + // (its CHECK goes with it), so a re-run of V229 does everything again. Safe on a store that never had it. + new(229, "hasRouteSeverities", + @"ALTER TABLE config.config_notification_routes DROP COLUMN IF EXISTS severities; +ALTER TABLE IF EXISTS config.external_alert_state DROP COLUMN IF EXISTS fired_severities; +ALTER TABLE IF EXISTS config.investigation_request_alerts DROP COLUMN IF EXISTS severity;"), ]; /// diff --git a/Darling/Darling.Tests/ServerActionGateTests.cs b/Darling/Darling.Tests/ServerActionGateTests.cs index 37e5637457..ba78a3ea96 100644 --- a/Darling/Darling.Tests/ServerActionGateTests.cs +++ b/Darling/Darling.Tests/ServerActionGateTests.cs @@ -132,7 +132,7 @@ private static ServerActionCall Call(Guid request, Guid claim, string action = S private static ServerActionHooks Hooks(InvestigationRequestStore store, NpgsqlDataSource source, int? resolved = 3, Action? onCredential = null) => new() { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => resolved), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => resolved), ReadCredential = (serverId, ct) => { onCredential?.Invoke(); @@ -293,7 +293,7 @@ await RunAsync(async (store, connection, source, ct) => var writes = 0; var hooks = new AgentActionHooks { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => level), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => level), ReadBefore = (_, _, _) => Task.FromResult("{\"reason\":\"old\"}"), Write = (_, _, _) => { writes++; return Task.FromResult("{\"reason\":\"new\"}"); }, }; @@ -314,7 +314,7 @@ await RunAsync(async (store, connection, source, ct) => /// write's own transaction opens, which is where a person changes something mid-flight. private AgentActionHooks WriteHooks(Func resolved, Func inFlight) => new() { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => resolved()), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => resolved()), ReadBefore = async (_, _, token) => { await inFlight(token); @@ -465,7 +465,7 @@ await RunAsync(async (store, connection, source, ct) => await using var read = new NpgsqlCommand( "SELECT CASE WHEN EXISTS (SELECT 1 FROM config.server_tag_map WHERE server_id = 7) THEN agent_level ELSE 2 END FROM config.config_notification_routes WHERE route_id = 4001", c, t); var level = Convert.ToInt32(await read.ExecuteScalarAsync(token), CultureInfo.InvariantCulture); - return (_, _) => level; + return (_, _, _) => level; }, ReadBefore = (_, _, _) => Task.FromResult("{\"reason\":\"old\"}"), Write = async (_, _, token) => @@ -557,7 +557,7 @@ await RunAsync(async (store, connection, source, ct) => var bothReached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); AgentActionHooks Hooks(string key) => new() { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => 3), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => 3), ReadBefore = (_, _, _) => Task.FromResult(null), Write = async (c, t, token) => { @@ -594,7 +594,7 @@ await RunAsync(async (store, connection, source, ct) => var hooks = new AgentActionHooks { ServerChange = signal, - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => 3), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => 3), ReadBefore = (_, _, _) => Task.FromResult("{\"reason\":\"old\"}"), Write = async (_, _, token) => { @@ -628,7 +628,7 @@ await RunAsync(async (store, connection, source, ct) => var hooks = new AgentActionHooks { ServerChange = new ServerChangeSignal(), - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => 3), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => 3), ReadBefore = (_, _, _) => Task.FromResult("{\"reason\":\"old\"}"), Write = async (_, _, token) => { diff --git a/Darling/Darling.Tests/ServerActionSwitchWriteLiveTests.cs b/Darling/Darling.Tests/ServerActionSwitchWriteLiveTests.cs index 016e89b16b..9cd16bb2ae 100644 --- a/Darling/Darling.Tests/ServerActionSwitchWriteLiveTests.cs +++ b/Darling/Darling.Tests/ServerActionSwitchWriteLiveTests.cs @@ -149,7 +149,7 @@ await WithStoreAsync(async (owner, ct) => new AgentActionTarget(SqlServerId, "alpha-sql-01", null, null)); var hooks = new ServerActionHooks { - LoadLevels = (_, _, _) => Task.FromResult>((_, _) => 3), + LoadLevels = (_, _, _) => Task.FromResult>((_, _, _) => 3), ReadCredential = (serverId, token) => store.ReadServerCredentialStateAsync(owner, serverId, token), }; diff --git a/Darling/Darling.Tests/ViewerNotificationRouteScopeTests.cs b/Darling/Darling.Tests/ViewerNotificationRouteScopeTests.cs index 746a93034f..b950a7e4b2 100644 --- a/Darling/Darling.Tests/ViewerNotificationRouteScopeTests.cs +++ b/Darling/Darling.Tests/ViewerNotificationRouteScopeTests.cs @@ -78,7 +78,7 @@ public void TheSql_ReadsAndWritesBothScopeColumns_InEveryStatement() Assert.Contains("scope_server_id = $9", ViewerDataService.NotificationRouteUpdateSql, StringComparison.Ordinal); Assert.Contains("scope_tag_id = $10", ViewerDataService.NotificationRouteUpdateSql, StringComparison.Ordinal); /* #5632: the agent columns, $10-$14, follow the scope pair. */ - Assert.Contains("$8, $9, $10, $11, $12, $13, $14, $15, $16, (now()", ViewerDataService.NotificationRouteInsertSql, StringComparison.Ordinal); + Assert.Contains("$8, $9, $10, $11, $12, $13, $14, $15, $16, $17, (now()", ViewerDataService.NotificationRouteInsertSql, StringComparison.Ordinal); } /* ---- the picker ---------------------------------------------------------------------------------------- */ diff --git a/Darling/Darling.Tests/ViewerNotificationRouteSeveritiesTests.cs b/Darling/Darling.Tests/ViewerNotificationRouteSeveritiesTests.cs new file mode 100644 index 0000000000..c6ccd2c6d6 --- /dev/null +++ b/Darling/Darling.Tests/ViewerNotificationRouteSeveritiesTests.cs @@ -0,0 +1,134 @@ +/* + * 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.Linq; +using System.Threading.Tasks; +using Npgsql; +using PerformanceMonitor.Darling.Storage; +using PerformanceMonitor.Darling.Viewer; +using PerformanceMonitor.Notifications; +using Xunit; + +namespace Darling.Tests; + +/// +/// #6928 (V229): the viewer's part of the route severity filter. The store-version probe maps the new column to 229, the list names +/// the filter in words, and a route's choice round-trips through the viewer's insert, update and sealed save. A store below V229 has no +/// column, so the list still opens and every route reads as Both. +/// +/* #1776 own-store: its one live fact mints its own scratch database through ScratchPostgres and touches nothing on the shared + one, so it is not serialized against the live-postgres collection. */ +public sealed class ViewerNotificationRouteSeveritiesTests +{ + /// The first sixteen sentinels, all present (the V16 to V31 floor), so only the one named after them decides the answer. + private static int Map(bool hasRouteSeverities) => ViewerDataService.MapProbedSchemaVersion( + true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, + hasRouteSeverities: hasRouteSeverities); + + [Fact] + public void TheProbeSentinel_MapsTheRouteSeveritiesColumn_ToV229_AndItsAbsence_ToAnOlderVersion() + { + Assert.Equal(229, Map(true)); + Assert.True(Map(false) < 229); + Assert.Equal(229, ViewerDataService.RouteSeveritiesStoreSchemaVersion); + } + + [Theory] + [InlineData("both", "Both")] + [InlineData("critical", "Critical only")] + [InlineData("warning", "Warning only")] + [InlineData("CRITICAL", "Critical only")] + [InlineData("", "Both")] + [InlineData("something-new", "Both")] + public void TheGridWords_NameTheFilter_AndAnUnknownWordReadsAsBoth(string stored, string expected) => + Assert.Equal(expected, new NotificationRouteRow { Severities = stored }.SeveritiesDisplay); + + [Fact] + public void ANewRoute_DefaultsToBoth_AndTheCloneAndTheDraftKeepTheChoice() + { + Assert.Equal(NotificationRouteKinds.SeveritiesBoth, new NotificationRouteRow().Severities); + var row = new NotificationRouteRow { MetricMatch = "performance", SmtpRecipients = "a@example.invalid", Severities = "critical" }; + Assert.Equal("critical", row.Clone().Severities); + Assert.Equal("critical", row.ToDraft().Severities); + Assert.Null(ViewerDataService.ValidateNotificationRoute(row)); + Assert.NotNull(ViewerDataService.ValidateNotificationRoute(new NotificationRouteRow { MetricMatch = "performance", SmtpRecipients = "a@example.invalid", Severities = "major" })); + } + + private static string? BaseConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + + private static async Task ExecAsync(NpgsqlConnection connection, string sql, System.Threading.CancellationToken ct) + { + await using var command = new NpgsqlCommand(sql, connection); + await command.ExecuteNonQueryAsync(ct); + } + + [Fact] + public async Task Live_TheSeverityRoundTripsThroughTheViewer_AndAStoreBelowV229StillOpensTheList() + { + var baseConnectionString = BaseConnectionString; + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), "Set DARLING_TEST_PG to run the viewer's route severity round trip."); + + var ct = TestContext.Current.CancellationToken; + var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + using var key = PasswordPrivateKey.Generate(); + var bodySucceeded = false; + try + { + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + await ExecAsync(connection, "INSERT INTO config.config_service (id) VALUES (1) ON CONFLICT (id) DO NOTHING", ct); + await ExecAsync(connection, "INSERT INTO config.config_notification (id) VALUES (1) ON CONFLICT (id) DO NOTHING", ct); + + await using (var viewer = new ViewerDataService(scratch.ConnectionString)) + { + Assert.True(await viewer.GetStoreSchemaVersionAsync(ct) >= 229); + + var inserted = await viewer.InsertNotificationRouteAsync( + new NotificationRouteRow { MetricMatch = "performance", SmtpRecipients = "a@example.invalid", Severities = "critical" }, ct); + var plain = await viewer.InsertNotificationRouteAsync( + new NotificationRouteRow { MetricMatch = "reports", SmtpRecipients = "b@example.invalid" }, ct); + var sealedId = await viewer.SaveNotificationRouteSealedAsync( + new NotificationRouteRow { MetricMatch = "agent-jobs", SlackUrl = "https://hooks.example.invalid/w", Severities = "warning" }, + null, new NotificationRow(), new ViewerPasswordSealer(key.PublicKey), null, ct); + + var read = await viewer.GetNotificationRoutesAsync(ct); + Assert.Equal("critical", read.Single(r => r.RouteId == inserted).Severities); + Assert.Equal("both", read.Single(r => r.RouteId == plain).Severities); + Assert.Equal("warning", read.Single(r => r.RouteId == sealedId).Severities); + Assert.Equal("Critical only", read.Single(r => r.RouteId == inserted).SeveritiesDisplay); + + var edited = read.Single(r => r.RouteId == inserted).Clone(); + edited.Severities = "warning"; + await viewer.UpdateNotificationRouteAsync(edited, ct); + Assert.Equal("warning", (await viewer.GetNotificationRoutesAsync(ct)).Single(r => r.RouteId == inserted).Severities); + + await Assert.ThrowsAsync(() => viewer.InsertNotificationRouteAsync( + new NotificationRouteRow { MetricMatch = "performance", SmtpRecipients = "c@example.invalid", Severities = "major" }, ct)); + } + + /* A store below V229 has no column: the list still reads, and every route is Both. */ + await ExecAsync(connection, "ALTER TABLE config.config_notification_routes DROP COLUMN severities", ct); + await using (var older = new ViewerDataService(scratch.ConnectionString)) + { + Assert.True(await older.GetStoreSchemaVersionAsync(ct) < 229); + var rows = await older.GetNotificationRoutesAsync(ct); + Assert.Equal(3, rows.Count); + Assert.All(rows, r => Assert.Equal("both", r.Severities)); + } + + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (_, _) => await Task.CompletedTask); + await scratch.DisposeAsync(); + } + } +} diff --git a/Darling/Darling.Tests/admin-server-edit-harness.mjs b/Darling/Darling.Tests/admin-server-edit-harness.mjs index a76d124216..514abd9946 100644 --- a/Darling/Darling.Tests/admin-server-edit-harness.mjs +++ b/Darling/Darling.Tests/admin-server-edit-harness.mjs @@ -1248,6 +1248,27 @@ try { writes: requests.filter((r) => r.method === "PUT").length, }; }, + /* #6936: the Routes tab on an editing seat. Edit on route 7 (a stored Critical only route), choose `severity` in the Severity box, Save: + returns the write the page sent (method, url, body). The service answers 200 so the page repaints; the body is what is under test. */ + routeEdit: async (severity) => { + state.canEdit = true; + const route = { route_id: 7, metric_match: "blocking", match_kind: "exact_metric", family: "performance", configured_channels: [], smtp_recipients: [], + enabled: true, modified_at_utc: "2026-01-01T00:00:00.1234567Z", scope_server_id: null, scope_tag_id: null, agent_level: null, agent_max_per_hour: 10, + agent_max_per_day: 50, agent_lease_minutes: 30, agent_expiry_minutes: 120, send_resolutions: null, fallback_route_id: null, severities: "critical" }; + state.responder = (method, url) => { + if (method === "GET" && url === "/api/read/get_notification_routes") return { status: 200, body: { routes: [route] } }; + if (method === "GET" && url === "/api/agent-setup/access") return { status: 200, body: { can_edit_routes: true, can_edit_profiles: true } }; + if (method === "PUT" && url === "/api/notification-routes/7") return { status: 200, body: { status: "updated", route_id: 7 } }; + return { status: 404, body: { error: "no such request " + url } }; + }; + await mountPage("routes"); + await fire(byAttr(main, "data-action", "edit-route")[0], "click"); + await settle(30); + await typeInto(field("severities"), severity); + await fire(byAttr(main, "data-action", "save-route")[0], "click"); + await settle(30); + return { writes: requests.filter((r) => r.method === "PUT").map((r) => ({ method: r.method, url: r.url, body: r.body })) }; + }, /* One call of an admin.js export, {"fn": "buildEditBody", "args": [...]} on stdin, printed as {"result": ...}; or {"calls": [{"fn": ..., "args": [...]}, ...]} for a table of calls in one process, printed as {"results": [...]}. An export that is not a function (EDIT_FIELDS) is returned as it is. */ diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingAlertDeliverer.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingAlertDeliverer.cs index 47de15553a..271f4b9bb2 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingAlertDeliverer.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingAlertDeliverer.cs @@ -112,8 +112,8 @@ public DarlingAlertDeliverer( /// public Task TryResolvePagerDutyAsync( string firingMetric, string serverName, string serverKey, string? detailText, - CancellationToken cancellationToken = default) => - _webhooks.TrySendPagerDutyResolveAsync(firingMetric, serverName, serverKey, detailText, cancellationToken); + IReadOnlyCollection firedSeverities, CancellationToken cancellationToken = default) => + _webhooks.TrySendPagerDutyResolveAsync(firingMetric, serverName, serverKey, detailText, firedSeverities, cancellationToken); public Task DeliverAsync(AlertOutcome outcome, CancellationToken cancellationToken = default) => DeliverAndReportAsync(outcome, cancellationToken); @@ -338,7 +338,22 @@ private sealed class DeliveredFire /// against, read the way the viewer reads a row (). public string? DetailText { get; set; } - public Dictionary Channels { get; } = new(StringComparer.Ordinal); + /// Every destination each channel was fired to during the episode, each once (#6928). A severity filter can send a + /// Warning fire and the Critical fire after it to different routes, and the clear owes each of them its close. + public Dictionary> Channels { get; } = new(StringComparer.Ordinal); + + public void AddDestination(string channel, string destination, int? routeId) + { + if (!Channels.TryGetValue(channel, out var list)) + { + Channels[channel] = list = new List<(string Destination, int? RouteId)>(); + } + + if (!list.Contains((destination, routeId))) + { + list.Add((destination, routeId)); + } + } public HashSet PagerDutyDedupKeys { get; } = new(StringComparer.Ordinal); @@ -411,7 +426,7 @@ private void NoteDeliveredFire( fire.DetailText = detailText; foreach (var (channel, where, routeId) in delivered) { - fire.Channels[channel] = (where, routeId); + fire.AddDestination(channel, where, routeId); if (channel == NotificationRouter.PagerDutyChannel) { fire.PagerDutyDedupKeys.Add(dedupKey); @@ -553,33 +568,38 @@ private async Task SendResolutionAsync( var routeGone = new List(); foreach (var channel in s_channelOrder) { - if (!fire.Channels.TryGetValue(channel, out var reached)) + if (!fire.Channels.TryGetValue(channel, out var reachedList)) { continue; } - /* The route that carried the fire, as it is now. Deleted or disabled since the fire: the same answer as the - history path below (#5855 review), so the clear goes nowhere on that channel and the row says why. */ - var route = RouteNow(reached.RouteId, routes, out var gone); - if (gone) + /* #6928: a severity filter can fire one channel to different destinations during an episode, so each is closed. */ + foreach (var reached in reachedList) { - if (!fire.RouteGoneChannels.Contains(channel)) + /* The route that carried the fire, as it is now. Deleted or disabled since the fire: the same answer as the + history path below (#5855 review), so the clear goes nowhere on that destination and the row says why. */ + var route = RouteNow(reached.RouteId, routes, out var gone); + if (gone) { - fire.RouteGoneChannels.Add(channel); - } + if (!fire.RouteGoneChannels.Contains(channel)) + { + fire.RouteGoneChannels.Add(channel); + } - continue; - } + continue; + } - if (channel == NotificationRouter.PagerDutyChannel - && (!options.ClosePagerDuty || fire.PagerDutyDedupKeys.Count == 0)) - { - continue; - } + if (channel == NotificationRouter.PagerDutyChannel + && (!options.ClosePagerDuty || fire.PagerDutyDedupKeys.Count == 0)) + { + continue; + } - if (NotificationRouteResolutions.SendsResolutions(route, channel, flags)) - { - targets.Add((channel, reached.Destination)); + if (NotificationRouteResolutions.SendsResolutions(route, channel, flags) + && !targets.Contains((channel, reached.Destination))) + { + targets.Add((channel, reached.Destination)); + } } } @@ -659,7 +679,12 @@ the clear. */ error = await _webhooks.TrySendResolutionAsync(channel, destination, facts, serverId, genericDedupKey: null, cancellationToken); } - outcomes[channel] = error is null ? AlertChannelOutcome.Delivered : AlertChannelOutcome.Failed; + /* A channel with several destinations is Failed when any of them failed. */ + if (error is not null || !outcomes.ContainsKey(channel)) + { + outcomes[channel] = error is null ? AlertChannelOutcome.Delivered : AlertChannelOutcome.Failed; + } + if (error is not null && channel != NotificationRouter.EmailChannel) { webhookError ??= $"{channel}: {error}"; @@ -746,7 +771,7 @@ the clear. */ }; string? firstDedupKey = null; - /* Newest first: a channel takes the destination its newest delivered fire used, as the memory path does. */ + /* Every delivered fire of the episode adds its destination, as the memory path does (#6928). */ foreach (var row in history.Rows) { if (AlertContextSerializer.TryReadRoute(row.ContextJson) is not { } route) @@ -768,25 +793,30 @@ matches the trigger whenever the #3313 filter dropped none of them. The triage l #3313 filter dropped none of them. */ var genericDedupKey = destination.SentKey ?? WebhookAlertService.DeriveGenericDedupKey(resolution.ServerKey, resolution.ServerName, resolution.MetricName, context); - if (destination.Outcome != AlertRouteOutcomes.Delivered - || fire.Channels.ContainsKey(destination.Channel) || fire.RouteGoneChannels.Contains(destination.Channel)) + /* #6928: every delivered fire's destination counts, not only the newest, because a severity filter can send the + Warning fire and the Critical fire of one episode to different routes, and each owes its close. */ + if (destination.Outcome != AlertRouteOutcomes.Delivered) { - if (destination.Outcome == AlertRouteOutcomes.Delivered && destination.Channel == NotificationRouter.PagerDutyChannel) - { - fire.PagerDutyDedupKeys.Add(dedupKey); - } - else if (destination.Outcome == AlertRouteOutcomes.Delivered && destination.Channel == NotificationRouter.GenericChannel) - { - fire.GenericDedupKeys.Add(genericDedupKey); - } - continue; } + if (destination.Channel == NotificationRouter.PagerDutyChannel) + { + fire.PagerDutyDedupKeys.Add(dedupKey); + } + else if (destination.Channel == NotificationRouter.GenericChannel) + { + fire.GenericDedupKeys.Add(genericDedupKey); + } + var where = CurrentDestination(destination.Channel, destination.RouteId, routes, out var gone); if (gone) { - fire.RouteGoneChannels.Add(destination.Channel); + if (!fire.RouteGoneChannels.Contains(destination.Channel)) + { + fire.RouteGoneChannels.Add(destination.Channel); + } + continue; } @@ -795,15 +825,7 @@ matches the trigger whenever the #3313 filter dropped none of them. The triage l continue; } - fire.Channels[destination.Channel] = (where, destination.RouteId); - if (destination.Channel == NotificationRouter.PagerDutyChannel) - { - fire.PagerDutyDedupKeys.Add(dedupKey); - } - else if (destination.Channel == NotificationRouter.GenericChannel) - { - fire.GenericDedupKeys.Add(genericDedupKey); - } + fire.AddDestination(destination.Channel, where, destination.RouteId); } } diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs index 2a4d0333f2..fc85a093db 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs @@ -273,6 +273,8 @@ column that exists precisely so a role denied the URLs can still say which chann "send_resolutions", /* V193 (#5997): the fallback route (D8) and when the previous signing key stops being sent. Non-secret. */ "fallback_route_id", "previous_signing_key_until", + /* V229 (#6928): the route's severity filter (both, critical or warning). Non-secret. */ + "severities", }, SecretColumns: new[] { "teams_url", "slack_url", "generic_url", "pagerduty_routing_key", "signing_key", "previous_signing_key" }), }; @@ -1808,8 +1810,8 @@ GRANT UPDATE ({McpMonitoredServerUpdateColumns}) -- (the address list is plain text). All three re-read the row FOR UPDATE and refuse an agent route inside the transaction, -- unless the actor the service passes is an operator-class token (#6542 part 2). Granted to mcp only. {BuildNotificationRouteFunctionsSql(config)} -GRANT EXECUTE ON FUNCTION {config}.create_notification_route(text, boolean, integer, integer, boolean, integer) TO {mcp}; -GRANT EXECUTE ON FUNCTION {config}.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer) TO {mcp}; +GRANT EXECUTE ON FUNCTION {config}.create_notification_route(text, boolean, integer, integer, boolean, integer, text) TO {mcp}; +GRANT EXECUTE ON FUNCTION {config}.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer, text) TO {mcp}; GRANT EXECUTE ON FUNCTION {config}.set_notification_route_destinations(integer, text, text[], text, text, text, text, text) TO {mcp}; GRANT EXECUTE ON FUNCTION {config}.set_notification_route_agent(integer, text, text[], integer, integer, integer, integer, integer, integer, boolean, jsonb) TO {mcp}; -- #6394: the MCP generate_webhook_signing_key and remove_webhook_signing_key tools. mcp holds no UPDATE on a signing key column of {config}.config_notification @@ -2008,7 +2010,7 @@ GRANT INSERT (request_id, route_id, server_id, server_name, family, first_metric -- every start, so no migration rung carries this. GRANT INSERT (request_id, route_id, server_id, server_name, family, first_metric, story_path_hash, first_alert_at, last_alert_at, window_until, opened_at, expires_at, lease_minutes, autonomy_level, alert_count, practice_of) ON {config}.investigation_requests TO {mcp}; -GRANT INSERT (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json) +GRANT INSERT (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json, severity) ON {config}.investigation_request_alerts TO {mcp}; GRANT INSERT (request_id, verdict, corrected_verdict, missed_step, comment, author) ON {config}.investigation_feedback TO {mcp}; GRANT UPDATE (verdict, corrected_verdict, missed_step, comment, author, created_at) ON {config}.investigation_feedback TO {mcp}; @@ -2829,13 +2831,16 @@ IF v_last_status IN ('pending', 'in_progress') THEN /// agent route inside the transaction, unless the actor the service passes is an operator-class token (#6542 part 2: MCP is full admin). Created and REVOKEd from PUBLIC here; the caller adds the GRANT EXECUTE to mcp only. /// internal static string BuildNotificationRouteFunctionsSql(string config) => $@" +-- #6928 (V229): p_severities joins the argument list, so the six-argument function is dropped and stays uncallable. NULL is both. +DROP FUNCTION IF EXISTS {config}.create_notification_route(text, boolean, integer, integer, boolean, integer); CREATE OR REPLACE FUNCTION {config}.create_notification_route( p_metric_match text, p_enabled boolean, p_scope_server_id integer, p_scope_tag_id integer, p_send_resolutions boolean, - p_fallback_route_id integer) + p_fallback_route_id integer, + p_severities text DEFAULT NULL) RETURNS integer LANGUAGE plpgsql SECURITY DEFINER @@ -2843,8 +2848,12 @@ SECURITY DEFINER AS $fn$ DECLARE v_match text := btrim(COALESCE(p_metric_match, '')); + v_severities text := lower(btrim(COALESCE(p_severities, 'both'))); v_id integer; BEGIN + IF v_severities NOT IN ('both', 'critical', 'warning') THEN + RAISE EXCEPTION 'severities is critical, warning or both.' USING ERRCODE = 'DL400'; + END IF; IF v_match = '' THEN RAISE EXCEPTION 'Choose a family or enter an exact metric name.' USING ERRCODE = 'DL400'; END IF; @@ -2870,17 +2879,20 @@ AND NOT EXISTS (SELECT 1 FROM {config}.config_notification_routes WHERE route_id -- goes where it went before (the next matching route, else the Settings channels), and no caller of this function can name a URL, a key or an address list. -- The agent columns keep their defaults too: this function never makes an agent route. INSERT INTO {config}.config_notification_routes - (metric_match, enabled, scope_server_id, scope_tag_id, send_resolutions, fallback_route_id, modified_at) + (metric_match, enabled, scope_server_id, scope_tag_id, send_resolutions, fallback_route_id, severities, modified_at) VALUES - (v_match, COALESCE(p_enabled, true), p_scope_server_id, p_scope_tag_id, p_send_resolutions, p_fallback_route_id, + (v_match, COALESCE(p_enabled, true), p_scope_server_id, p_scope_tag_id, p_send_resolutions, p_fallback_route_id, v_severities, (now() AT TIME ZONE 'UTC')) RETURNING route_id INTO v_id; RETURN v_id; END $fn$; -REVOKE ALL ON FUNCTION {config}.create_notification_route(text, boolean, integer, integer, boolean, integer) FROM PUBLIC; +REVOKE ALL ON FUNCTION {config}.create_notification_route(text, boolean, integer, integer, boolean, integer, text) FROM PUBLIC; -- #6542 part 2: the actor is an argument now and the before row comes back, so the old signature is dropped and stays uncallable. DROP FUNCTION IF EXISTS {config}.update_notification_route(integer, text[], text, boolean, integer, integer, boolean, integer); +-- #6928 (V229): p_severities joins the argument list, so the nine-argument function is dropped and stays uncallable. It is read only when +-- p_columns names severities. +DROP FUNCTION IF EXISTS {config}.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer); CREATE OR REPLACE FUNCTION {config}.update_notification_route( p_route_id integer, p_actor text, @@ -2890,7 +2902,8 @@ INSERT INTO {config}.config_notification_routes p_scope_server_id integer, p_scope_tag_id integer, p_send_resolutions boolean, - p_fallback_route_id integer) + p_fallback_route_id integer, + p_severities text DEFAULT NULL) RETURNS jsonb LANGUAGE plpgsql SECURITY DEFINER @@ -2907,11 +2920,12 @@ SECURITY DEFINER v_tag integer; v_resolutions boolean; v_fallback integer; + v_severities text; BEGIN IF p_columns IS NULL OR cardinality(p_columns) = 0 OR EXISTS (SELECT 1 FROM unnest(p_columns) AS c(name) - WHERE c.name IS NULL OR c.name NOT IN ('metric_match', 'enabled', 'scope', 'send_resolutions', 'fallback_route_id')) THEN - RAISE EXCEPTION 'The columns to change are metric_match, enabled, scope, send_resolutions and fallback_route_id.' USING ERRCODE = 'DL400'; + WHERE c.name IS NULL OR c.name NOT IN ('metric_match', 'enabled', 'scope', 'send_resolutions', 'fallback_route_id', 'severities')) THEN + RAISE EXCEPTION 'The columns to change are metric_match, enabled, scope, send_resolutions, fallback_route_id and severities.' USING ERRCODE = 'DL400'; END IF; -- The row is read again here, under its lock, so the agent-route refusal and every check below are on what is stored now -- and not on what the caller read a moment ago. @@ -2925,7 +2939,7 @@ IF NOT FOUND THEN -- What a route matches and which servers it covers decide which servers an agent route covers, so a route with an -- explicit agent setting (Off included) keeps both in the Viewer. A route's own destinations do not stop a change -- to what it matches: the destinations are set over MCP too. (#6231) - IF ('metric_match' = ANY (p_columns) OR 'scope' = ANY (p_columns)) AND v_row.agent_level IS NOT NULL AND NOT v_operator THEN + IF ('metric_match' = ANY (p_columns) OR 'scope' = ANY (p_columns) OR 'severities' = ANY (p_columns)) AND v_row.agent_level IS NOT NULL AND NOT v_operator THEN RAISE EXCEPTION 'That route has an agent setting (Off), so what it matches and which servers it covers are changed in the Viewer or on the web dashboard, not over MCP. Turning it on, send_resolutions and fallback_route_id can still be changed here.' USING ERRCODE = 'DL403'; END IF; -- An Off route keeps servers away from a broader agent route, so turning it off widens agent coverage: that is done in the Viewer @@ -2939,6 +2953,10 @@ IF NOT FOUND THEN v_tag := CASE WHEN 'scope' = ANY (p_columns) THEN p_scope_tag_id ELSE v_row.scope_tag_id END; v_resolutions := CASE WHEN 'send_resolutions' = ANY (p_columns) THEN p_send_resolutions ELSE v_row.send_resolutions END; v_fallback := CASE WHEN 'fallback_route_id' = ANY (p_columns) THEN p_fallback_route_id ELSE v_row.fallback_route_id END; + v_severities := CASE WHEN 'severities' = ANY (p_columns) THEN lower(btrim(COALESCE(p_severities, ''))) ELSE v_row.severities END; + IF v_severities NOT IN ('both', 'critical', 'warning') THEN + RAISE EXCEPTION 'severities is critical, warning or both.' USING ERRCODE = 'DL400'; + END IF; IF v_match = '' THEN RAISE EXCEPTION 'Choose a family or enter an exact metric name.' USING ERRCODE = 'DL400'; END IF; @@ -2972,6 +2990,7 @@ AND NOT EXISTS (SELECT 1 FROM {config}.config_notification_routes WHERE route_id scope_tag_id = v_tag, send_resolutions = v_resolutions, fallback_route_id = v_fallback, + severities = v_severities, modified_at = (now() AT TIME ZONE 'UTC') WHERE route_id = p_route_id; -- The row as it was before this change, in the journal's words: set_notification_route_agent journals it as the before of a @@ -2979,10 +2998,11 @@ AND NOT EXISTS (SELECT 1 FROM {config}.config_notification_routes WHERE route_id RETURN jsonb_build_object( 'metric_match', btrim(v_row.metric_match), 'scope_server_id', v_row.scope_server_id, 'scope_tag_id', v_row.scope_tag_id, 'agent_level', v_row.agent_level, 'agent_max_per_hour', v_row.agent_max_per_hour, 'agent_max_per_day', v_row.agent_max_per_day, - 'agent_lease_minutes', v_row.agent_lease_minutes, 'agent_expiry_minutes', v_row.agent_expiry_minutes, 'enabled', v_row.enabled); + 'agent_lease_minutes', v_row.agent_lease_minutes, 'agent_expiry_minutes', v_row.agent_expiry_minutes, 'enabled', v_row.enabled, + 'severities', v_row.severities); END $fn$; -REVOKE ALL ON FUNCTION {config}.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer) FROM PUBLIC; +REVOKE ALL ON FUNCTION {config}.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer, text) FROM PUBLIC; -- #6542: a named operator token sets a route's agent level, its four limits and its fallback over MCP through this function, and -- the change is journaled as route_agent_changed under the token's name (operator:), in the same transaction. Levels 0 to 3 are -- set here (MCP is full admin). A level 3 server action still needs that server's switch (config_server_actions) and its remediation login. @@ -3064,6 +3084,10 @@ IF p_before IS NOT NULL THEN IF (p_before ->> 'metric_match') IS DISTINCT FROM btrim(v_row.metric_match) THEN v_changed := array_append(v_changed, 'metric_match'); END IF; IF (p_before ->> 'scope_server_id')::integer IS DISTINCT FROM v_row.scope_server_id THEN v_changed := array_append(v_changed, 'scope_server_id'); END IF; IF (p_before ->> 'scope_tag_id')::integer IS DISTINCT FROM v_row.scope_tag_id THEN v_changed := array_append(v_changed, 'scope_tag_id'); END IF; + -- #6928: the severity filter moves alerts between agent routes, so it is named like the match and the scope. Only a route with an agent setting. + IF (p_before ->> 'severities') IS DISTINCT FROM v_row.severities AND ((p_before ->> 'agent_level') IS NOT NULL OR v_level IS NOT NULL) THEN + v_changed := array_append(v_changed, 'severities'); + END IF; END IF; IF v_level IS DISTINCT FROM v_row.agent_level THEN v_changed := array_append(v_changed, 'agent_level'); END IF; IF v_hour IS DISTINCT FROM v_row.agent_max_per_hour THEN v_changed := array_append(v_changed, 'agent_max_per_hour'); END IF; @@ -3089,11 +3113,13 @@ IF p_before IS NOT NULL AND ((p_before ->> 'agent_level') IS NOT NULL OR v_level v_before := CASE WHEN COALESCE(p_created, false) THEN NULL WHEN p_before IS NOT NULL THEN p_before ELSE jsonb_build_object( 'metric_match', btrim(v_row.metric_match), 'scope_server_id', v_row.scope_server_id, 'scope_tag_id', v_row.scope_tag_id, 'agent_level', v_row.agent_level, 'agent_max_per_hour', v_row.agent_max_per_hour, 'agent_max_per_day', v_row.agent_max_per_day, - 'agent_lease_minutes', v_row.agent_lease_minutes, 'agent_expiry_minutes', v_row.agent_expiry_minutes, 'enabled', v_row.enabled) END; + 'agent_lease_minutes', v_row.agent_lease_minutes, 'agent_expiry_minutes', v_row.agent_expiry_minutes, 'enabled', v_row.enabled, + 'severities', v_row.severities) END; v_after := jsonb_build_object( 'metric_match', btrim(v_row.metric_match), 'scope_server_id', v_row.scope_server_id, 'scope_tag_id', v_row.scope_tag_id, 'agent_level', v_level, 'agent_max_per_hour', v_hour, 'agent_max_per_day', v_day, - 'agent_lease_minutes', v_lease, 'agent_expiry_minutes', v_expiry, 'enabled', v_row.enabled); + 'agent_lease_minutes', v_lease, 'agent_expiry_minutes', v_expiry, 'enabled', v_row.enabled, + 'severities', v_row.severities); INSERT INTO {config}.investigation_journal (event_time, request_id, route_id, server_id, event, actor, reason, detail) VALUES (now(), NULL, p_route_id, v_row.scope_server_id, 'route_agent_changed', p_actor, CASE WHEN COALESCE(p_created, false) THEN 'created route' ELSE 'changed ' || array_to_string(v_changed, ', ') END, diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWebAgentSetup.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWebAgentSetup.cs index b08c41f5d4..2251913a69 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWebAgentSetup.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWebAgentSetup.cs @@ -56,7 +56,7 @@ internal static string RefusedMessage(bool managedStore, bool adminWritesOff = f private static readonly string[] s_routeKeys = [ "metric_match", "enabled", "scope_server_id", "scope_tag_id", "agent_level", "agent_max_per_hour", "agent_max_per_day", - "agent_lease_minutes", "agent_expiry_minutes", "send_resolutions", "fallback_route_id", + "agent_lease_minutes", "agent_expiry_minutes", "send_resolutions", "fallback_route_id", "severities", "teams_webhook_url", "slack_webhook_url", "generic_webhook_url", "pagerduty_routing_key", "email_recipients", "expected_modified_at", ]; @@ -320,6 +320,17 @@ internal static async Task ProfilesBodyAsync(NpgsqlDataSource postgr draft.SendResolutions = kind == JsonValueKind.Null ? null : body["send_resolutions"]!.GetValue(); } + /* #6928: the severity filter. A word other than both, critical or warning is refused by the writer's validation with its sentence. */ + if (body.ContainsKey("severities")) + { + if (body["severities"] is not JsonValue severities || severities.GetValueKind() != JsonValueKind.String) + { + return "severities must be text."; + } + + draft.Severities = severities.GetValue(); + } + if (!TryInt(body, "scope_server_id", out var hasServer, out var server, out var error) || !TryInt(body, "scope_tag_id", out var hasTag, out var tag, out error) || !TryInt(body, "agent_level", out var hasLevel, out var level, out error) diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWebhookSecrets.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWebhookSecrets.cs index b592f48107..ced6795573 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWebhookSecrets.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWebhookSecrets.cs @@ -349,6 +349,9 @@ the old row cached (a cleared fallback kept receiving the expired requests). A m Add(r.AgentExpiryMinutes.ToString(System.Globalization.CultureInfo.InvariantCulture)); Add(r.FallbackRouteId?.ToString(System.Globalization.CultureInfo.InvariantCulture)); Add(r.OffChannels is null ? "-" : string.Join(",", r.OffChannels.OrderBy(c => c, StringComparer.Ordinal))); + + /* #6928: the severity filter. Delivery reads it from the cached route, so an edit of only this member must reopen the cache. */ + Add(r.Severities); } return text.ToString(); diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs index efed41c5d0..d294c5b083 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs @@ -3763,7 +3763,7 @@ name stays a label in the server_id 0 bucket. */ externalAlertStore, alertsEnabled: () => alertSettings.AlertsEnabled, (outcome, token) => deliverer.DeliverAndReportAsync(outcome, token), - (metric, serverName, serverKey, detail, token) => deliverer.TryResolvePagerDutyAsync(metric, serverName, serverKey, detail, token), + (metric, serverName, serverKey, detail, severity, token) => deliverer.TryResolvePagerDutyAsync(metric, serverName, serverKey, detail, severity, token), historyStore, muteRuleService.IsAlertMuted, name => diff --git a/Darling/PerformanceMonitor.Darling.Service/ExternalAlertDrain.cs b/Darling/PerformanceMonitor.Darling.Service/ExternalAlertDrain.cs index 1071d31f35..f6f3e4a48f 100644 --- a/Darling/PerformanceMonitor.Darling.Service/ExternalAlertDrain.cs +++ b/Darling/PerformanceMonitor.Darling.Service/ExternalAlertDrain.cs @@ -75,7 +75,7 @@ public sealed class ExternalAlertDrain private readonly ExternalAlertStore _store; private readonly Func _alertsEnabled; private readonly Func> _deliver; - private readonly Func> _resolvePagerDuty; + private readonly Func, CancellationToken, Task> _resolvePagerDuty; private readonly IAlertHistoryStore _history; private readonly Func _isMuted; private readonly Func _resolveServer; @@ -86,7 +86,7 @@ public sealed class ExternalAlertDrain /// The live master switch every alert family consults (#3464). Off, a pass only expires rows. /// The record-and-send seam: . - /// (firing metric, server name, server key, detail, token): closes the PagerDuty incident + /// (firing metric, server name, server key, detail, every severity the episode's fires went out at, token): closes the PagerDuty incident /// of a fire, and only with auto-resolve on: . /// The mute check every family shares (MuteRuleService.IsAlertMuted). /// Maps the submitted server name to a monitored server, or null when it is not registered; @@ -95,7 +95,7 @@ public ExternalAlertDrain( ExternalAlertStore store, Func alertsEnabled, Func> deliver, - Func> resolvePagerDuty, + Func, CancellationToken, Task> resolvePagerDuty, IAlertHistoryStore history, Func isMuted, Func resolveServer, @@ -354,6 +354,30 @@ await TryMarkAsync(row.PendingId, ExternalAlertOutcome.Delivered, return ExternalAlertOutcome.Delivered; } + /// The tier a fire went out at (#6928), read back from the state's stored word, so the close is routed as the fire was: + /// to the destination of the route that took that severity. Null when none was confirmed, which every route takes. + private static AlertSeverityLevel? SeverityOfFired(string? firedSeverity) => + string.Equals(firedSeverity, "critical", StringComparison.OrdinalIgnoreCase) ? AlertSeverityLevel.Critical + : string.Equals(firedSeverity, "warning", StringComparison.OrdinalIgnoreCase) ? AlertSeverityLevel.Warning + : null; + + /// Every tier an episode's fires went out at (#6928), so the close reaches the destination of each tier's route: a + /// Warning-only route's incident stays open when the alert climbs to Critical unless the close names Warning too. An episode + /// opened before the list was kept has only the newest word. Empty (one close with no tier) when none was confirmed. + private static List SeveritiesOfFired(ExternalAlertFiringState state) + { + var tiers = new List(); + foreach (var word in (state.FiredSeverities ?? state.FiredSeverity ?? "").Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)) + { + if (SeverityOfFired(word) is { } tier && !tiers.Contains(tier)) + { + tiers.Add(tier); + } + } + + return tiers; + } + private async Task HandleClearAsync(ExternalAlertPendingRow row, CancellationToken cancellationToken) { var server = Identify(row); @@ -376,7 +400,8 @@ fire must not move the incident this clear closes. */ /* The PagerDuty resolve goes first: it posts only with auto-resolve on and a PagerDuty destination for this alert, and if it fails the firing state is left alone so the retry still finds a fire to close. */ - var pagerDuty = await _resolvePagerDuty(firingMetric, firedLabel, firedKey, message, cancellationToken); + var pagerDuty = await _resolvePagerDuty( + firingMetric, firedLabel, firedKey, message, SeveritiesOfFired(state), cancellationToken); if (pagerDuty == AlertChannelOutcome.Failed) { return await RetryOrFailAsync(row, "the PagerDuty resolve failed"); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingAgentActionGate.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingAgentActionGate.cs index 814577d8e4..24f10cbe6e 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingAgentActionGate.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingAgentActionGate.cs @@ -116,11 +116,11 @@ internal static string RefusalAnswer(AgentActionRefusal refusal) => /// that one list, so a route an operator lowered refuses the next change, not the next reload. A failed tag read logs /// through and leaves a tag-scoped route unexpanded, so it matches nothing: fail closed. /// - internal static async Task> LoadLevelsAsync(NpgsqlConnection connection, ILogger logger, + internal static async Task> LoadLevelsAsync(NpgsqlConnection connection, ILogger logger, CancellationToken cancellationToken) { var routes = await StoreConfigProvider.ReadNotificationRoutesAsync(connection, logger, cancellationToken); - return (metric, serverId) => NotificationRouter.ResolveAgent(metric, routes, serverId)?.AgentLevel; + return (metric, serverId, severity) => NotificationRouter.ResolveAgent(metric, routes, serverId, severity)?.AgentLevel; } /// The request id of a refused argument, when one parses, so its refusal is journaled with its request. diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.RouteAdmin.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.RouteAdmin.cs index d48b45dd0b..8f0fe6d719 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.RouteAdmin.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.RouteAdmin.cs @@ -48,7 +48,7 @@ public sealed partial class DarlingMcpAlertTools "Creates a route that decides WHICH alerts it covers and, when you pass them, where they go. " + DestinationsHelp + " " + "With no destination field the route is stored with every destination empty, so it has none of its own and each channel goes where it went before: to the next less specific matching route that sets that channel, else to the Settings > Notifications channels. " + "What it can set: metric_match (" + MatchHelp + "), a scope of one server or one tag (not both; * only with a scope), enabled, " + - "send_resolutions (inherit, always or never: the clear message to the channels this route resolves to) and fallback_route_id (an existing route). " + + "send_resolutions (inherit, always or never: the clear message to the channels this route resolves to), fallback_route_id (an existing route) and severities (" + SeveritiesHelp + "). " + "The database role this tool runs as has no INSERT on the routes table and no read or write on a destination column: the route is written by the store's own " + "config.create_notification_route function and its destinations by config.set_notification_route_destinations, in one transaction, so a destination that is refused leaves no route behind. " + "A destination is sealed for this route's own row, so it cannot be copied to another route. scope_server_id and scope_tag_id are ids from get_notification_routes or the server and tag reads; an id that names nothing is invalid. " + @@ -61,6 +61,7 @@ public static Task CreateNotificationRoute( [Description("Scope the route to this server tag id and every tag under it. Omit for fleet-wide. Not with scope_server_id.")] int? scope_tag_id = null, [Description("inherit (default: each channel's own setting), always (send the clear message to the channels this route resolves to) or never.")] string? send_resolutions = null, [Description("The route_id of an existing route an expired investigation request goes to first. Omit for none.")] int? fallback_route_id = null, + [Description("Which alerts the route takes by severity: critical, warning or both (default, case-insensitive). An alert that is not Critical counts as Warning.")] string? severities = null, [Description("The route's own Teams webhook URL (absolute http or https). Sealed for this route; never shown back. Omit for none.")] string? teams_webhook_url = null, [Description("The route's own Slack webhook URL (absolute http or https). Sealed for this route; never shown back. Omit for none.")] string? slack_webhook_url = null, [Description("The route's own generic webhook URL (absolute http or https). Sealed for this route; never shown back. Omit for none.")] string? generic_webhook_url = null, @@ -80,13 +81,13 @@ public static Task CreateNotificationRoute( new RouteDestinationRequest(teams_webhook_url, slack_webhook_url, generic_webhook_url, pagerduty_routing_key, email_recipients), DarlingPasswordKey.Current, logger, cancellationToken, new RouteAgentRequest(agent_level, agent_max_per_hour, agent_max_per_day, agent_lease_minutes, agent_expiry_minutes), - served?.Policy.OperatorActor, StoreRolesAreRunByOperator(postgresConfig)); + served?.Policy.OperatorActor, StoreRolesAreRunByOperator(postgresConfig), severities); /// The create: the route and its destinations in one transaction (a sealed value is bound to the route's id, so the id comes first). internal static async Task CreateNotificationRouteAsync( NpgsqlDataSource postgres, string metric_match, bool enabled, int? scope_server_id, int? scope_tag_id, string? send_resolutions, int? fallback_route_id, RouteDestinationRequest destinations, IPasswordKeyRing ring, ILogger? logger, CancellationToken cancellationToken, - RouteAgentRequest? agent = null, string? operatorActor = null, bool selfManagedStore = true) + RouteAgentRequest? agent = null, string? operatorActor = null, bool selfManagedStore = true, string? severities = null) { try { @@ -95,6 +96,11 @@ internal static async Task CreateNotificationRouteAsync( return Outcome("invalid", badResolutions); } + if (ParseSeverities(severities, out var severitiesWord) is { } badSeverities) + { + return Outcome("invalid", badSeverities); + } + var match = NormalizeRouteMatch(metric_match); agent ??= RouteAgentRequest.None; if (agent.Any) @@ -122,7 +128,7 @@ internal static async Task CreateNotificationRouteAsync( : new WebhookProxies("", "", "", ""); await using var connection = await postgres.OpenConnectionAsync(cancellationToken); await using var transaction = await connection.BeginTransactionAsync(cancellationToken); - const string createSql = "SELECT config.create_notification_route($1, $2, $3, $4, $5, $6)"; + const string createSql = "SELECT config.create_notification_route($1, $2, $3, $4, $5, $6, $7)"; await using (var command = new NpgsqlCommand(createSql, connection) { Transaction = transaction }) { command.CommandTimeout = McpCommandDeadlines.ReadSeconds; @@ -134,6 +140,7 @@ internal static async Task CreateNotificationRouteAsync( ? new NpgsqlParameter { TypedValue = resolutions.Value } : new NpgsqlParameter { Value = DBNull.Value, NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Boolean }); NotificationRouteWriter.AddNullableInt(command, fallback_route_id); + command.Parameters.Add(new NpgsqlParameter { TypedValue = severitiesWord }); routeId = Convert.ToInt32(await command.ExecuteScalarAsync(cancellationToken), System.Globalization.CultureInfo.InvariantCulture); } @@ -158,7 +165,7 @@ internal static async Task CreateNotificationRouteAsync( return Outcome("invalid", ex.Message); } - var fields = "metric_match, enabled, scope, send_resolutions, fallback_route_id" + (change is { Fields.Count: > 0 } ? ", " + DestinationFieldNames(change) : "") + var fields = "metric_match, enabled, scope, send_resolutions, fallback_route_id, severities" + (change is { Fields.Count: > 0 } ? ", " + DestinationFieldNames(change) : "") + (agent.Any ? ", " + string.Join(", ", agent.Columns) : ""); LogRouteWrite(logger, "create_notification_route", CallerOf(), routeId, fields); var stored = (await new PgNotificationRouteStore(postgres).LoadSummariesAsync(cancellationToken)).FirstOrDefault(r => r.RouteId == routeId); @@ -188,7 +195,7 @@ internal static async Task CreateNotificationRouteAsync( "An operator token changes any route, an agent route included: its match, scope, enabled flag, send_resolutions, fallback, destinations and agent fields. A change to the agent fields, or to the match, scope or enabled flag of a route with an agent setting, is journaled as route_agent_changed under operator: and the token's name. " + "A route's own destinations do not stop a change to what it matches. " + "Fields: metric_match (" + MatchHelp + "), enabled, scope (scope_server_id or scope_tag_id, the other is cleared; fleet_wide true clears both), " + - "send_resolutions (inherit, always or never) and fallback_route_id (an existing route; clear_fallback true removes it), plus the destination fields. A field left out keeps its stored value. " + + "send_resolutions (inherit, always or never), severities (" + SeveritiesHelp + ") and fallback_route_id (an existing route; clear_fallback true removes it), plus the destination fields. A field left out keeps its stored value. " + "An operator token sets agent_level 0 to 3, or clear_agent_level true to put the level back to Inherit (null: the next level decides), not both. Clearing the level keeps the four limits, as the Viewer's Inherit does. " + "The same rules the Viewer's route window applies hold: * needs a scope, a route is scoped to one server or one tag, a route is not its own fallback, a webhook is an absolute http or https URL, recipients are plain addresses. " + "A route whose generic webhook has a signing key keeps its generic URL until the key is removed (remove_webhook_signing_key, or the Viewer). " + @@ -210,6 +217,7 @@ public static Task UpdateNotificationRoute( [Description("inherit, always or never. Omit to keep.")] string? send_resolutions = null, [Description("The route_id of an existing route an expired investigation request goes to first. Omit to keep.")] int? fallback_route_id = null, [Description("true removes the fallback route.")] bool? clear_fallback = null, + [Description("Which alerts the route takes by severity: critical, warning or both (case-insensitive). Omit to keep.")] string? severities = null, [Description("The route's own Teams webhook URL (absolute http or https). Sealed for this route; never shown back. Omit to keep; - removes it.")] string? teams_webhook_url = null, [Description("The route's own Slack webhook URL (absolute http or https). Sealed for this route; never shown back. Omit to keep; - removes it.")] string? slack_webhook_url = null, [Description("The route's own generic webhook URL (absolute http or https). Sealed for this route; never shown back. Omit to keep; - removes it.")] string? generic_webhook_url = null, @@ -230,14 +238,14 @@ public static Task UpdateNotificationRoute( new RouteDestinationRequest(teams_webhook_url, slack_webhook_url, generic_webhook_url, pagerduty_routing_key, email_recipients), DarlingPasswordKey.Current, logger, cancellationToken, new RouteAgentRequest(agent_level, agent_max_per_hour, agent_max_per_day, agent_lease_minutes, agent_expiry_minutes, clear_agent_level == true), - served?.Policy.OperatorActor, StoreRolesAreRunByOperator(postgresConfig)); + served?.Policy.OperatorActor, StoreRolesAreRunByOperator(postgresConfig), severities); /// The edit: the match, scope and flag columns and the destinations in one transaction. internal static async Task UpdateNotificationRouteAsync( NpgsqlDataSource postgres, int route_id, string? metric_match, bool? enabled, int? scope_server_id, int? scope_tag_id, bool? fleet_wide, string? send_resolutions, int? fallback_route_id, bool? clear_fallback, RouteDestinationRequest destinations, IPasswordKeyRing ring, ILogger? logger, CancellationToken cancellationToken, RouteAgentRequest? agent = null, string? operatorActor = null, - bool selfManagedStore = true) + bool selfManagedStore = true, string? severities = null) { try { @@ -327,6 +335,21 @@ internal static async Task UpdateNotificationRouteAsync( } } + var severitiesNow = existing.Severities; + if (severities is not null) + { + if (ParseSeverities(severities, out var parsedSeverities) is { } badSeverities) + { + return Outcome("invalid", badSeverities); + } + + if (parsedSeverities != existing.Severities) + { + severitiesNow = parsedSeverities; + columns.Add("severities"); + } + } + var fallback = existing.FallbackRouteId; if (clear_fallback == true) { @@ -347,7 +370,7 @@ internal static async Task UpdateNotificationRouteAsync( /* The Viewer journals a match, scope or enabled change to a route with an agent setting (NotificationRouteWriter.AgentChange), so an operator's change to one is journaled too, by the agent function, with the before row the update function returns. */ var journalsMatchOrFlag = operatorActor is not null && existing.AgentLevel is not null - && columns.Any(c => c is "metric_match" or "scope" or "enabled"); + && columns.Any(c => c is "metric_match" or "scope" or "enabled" or "severities"); if (agent.Any) { @@ -393,7 +416,7 @@ internal static async Task UpdateNotificationRouteAsync( string? before = null; if (columns.Count > 0) { - const string updateSql = "SELECT config.update_notification_route($1, $2, $3, $4, $5, $6, $7, $8, $9)::text"; + const string updateSql = "SELECT config.update_notification_route($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)::text"; await using var command = new NpgsqlCommand(updateSql, connection) { Transaction = transaction }; command.CommandTimeout = McpCommandDeadlines.ReadSeconds; command.Parameters.Add(new NpgsqlParameter { TypedValue = route_id }); @@ -407,6 +430,7 @@ internal static async Task UpdateNotificationRouteAsync( ? new NpgsqlParameter { TypedValue = resolutions.Value } : new NpgsqlParameter { Value = DBNull.Value, NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Boolean }); NotificationRouteWriter.AddNullableInt(command, fallback); + command.Parameters.Add(new NpgsqlParameter { TypedValue = severitiesNow }); before = await command.ExecuteScalarAsync(cancellationToken) as string; } @@ -618,6 +642,7 @@ internal static void LogRouteWrite(ILogger? logger, string tool, string caller, ["metric_match"] = existing.MetricMatch, ["enabled"] = existing.Enabled, ["send_resolutions"] = existing.SendResolutions switch { true => "always", false => "never", _ => "inherit" }, + ["severities"] = existing.Severities, }; if (existing.ScopeServerId.HasValue) { @@ -673,6 +698,27 @@ private static string NormalizeRouteMatch(string? match) return AlertFamily.NormalizeFamily(text) ?? text; } + private const string SeveritiesHelp = "critical, warning or both; both is the default and takes every alert. An alert that is not Critical counts as Warning. A route whose filter excludes an alert does not match it, and matching goes on to the next route and ends at the default channels"; + + /// The severity filter word (#6928): critical, warning or both, any case. Blank is both. Anything else is refused with the sentence returned. + private static string? ParseSeverities(string? text, out string word) + { + word = NotificationRouteKinds.SeveritiesBoth; + var trimmed = (text ?? "").Trim().ToLowerInvariant(); + if (trimmed.Length == 0) + { + return null; + } + + if (trimmed is NotificationRouteKinds.SeveritiesBoth or NotificationRouteKinds.SeveritiesCritical or NotificationRouteKinds.SeveritiesWarning) + { + word = trimmed; + return null; + } + + return "severities is critical, warning or both."; + } + private static string? ParseSendResolutions(string? text, out bool? value) { value = null; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs index ff5a1824ea..c77d6a1a83 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs @@ -703,6 +703,7 @@ than a second opt-in behind one. The thresholds take the house _threshold_ "and agent_max_per_day; Darling only, Lite has no agent channel); it never changes a destination. " + "send_resolutions says whether the route's destinations also get the message that an alert has cleared: null " + "follows each channel's setting (get_alert_settings delivery.send_resolutions), true always sends it, false never does. " + + "severities (critical, warning or both) says which alerts the route takes by severity: both is the default and takes every alert, and a route whose filter excludes an alert does not match it, so matching goes on to the next route. " + "Routes are authored in the Viewer's Settings window or here (create_notification_route / update_notification_route); " + "they can also be disabled or deleted (set_notification_route_enabled / delete_notification_route).")] public static async Task GetNotificationRoutes( @@ -977,6 +978,7 @@ private static object RouteRecreateUndo(PgNotificationRouteStore.RouteSummary ro ["metric_match"] = route.MetricMatch, ["enabled"] = route.Enabled, ["send_resolutions"] = route.SendResolutions switch { true => "always", false => "never", _ => "inherit" }, + ["severities"] = route.Severities, }; if (route.ScopeServerId.HasValue) { @@ -1041,6 +1043,8 @@ private static object RouteRecreateUndo(PgNotificationRouteStore.RouteSummary ro /* #5825: null follows each channel's "send resolutions" setting (get_alert_settings, delivery.send_resolutions); true or false overrides it for this route's destinations. */ send_resolutions = r.SendResolutions, + /* #6928 (V229): which alerts the route takes by severity: both, critical or warning. */ + severities = r.Severities, /* #6036 (D8 of #5632 Phase C): the route an expired investigation request goes to first. Null = none; the name is the fallback route's metric_match. */ fallback_route_id = r.FallbackRouteId, diff --git a/Darling/PerformanceMonitor.Darling.Service/PgNotificationRouteStore.cs b/Darling/PerformanceMonitor.Darling.Service/PgNotificationRouteStore.cs index 6b6a3116ac..065b5f88db 100644 --- a/Darling/PerformanceMonitor.Darling.Service/PgNotificationRouteStore.cs +++ b/Darling/PerformanceMonitor.Darling.Service/PgNotificationRouteStore.cs @@ -89,6 +89,9 @@ public sealed record RouteSummary( /// The route an expired investigation request goes to first (#6036, D8 of #5632 Phase C), or null for none. public int? FallbackRouteId { get; init; } + /// The severity filter (V229, #6928): both, critical or warning. + public string Severities { get; init; } = NotificationRouteKinds.SeveritiesBoth; + /// The fallback route's name (its metric_match), or null when there is no fallback. public string? FallbackRouteName { get; init; } @@ -103,7 +106,7 @@ public sealed record RouteSummary( /// the one a superuser-run test never notices. public const string SelectSummariesSql = @" SELECT route_id, metric_match, configured_channels, smtp_recipients, enabled, modified_at, scope_server_id, scope_tag_id, - agent_level, agent_max_per_hour, agent_max_per_day, agent_lease_minutes, agent_expiry_minutes, send_resolutions, fallback_route_id + agent_level, agent_max_per_hour, agent_max_per_day, agent_lease_minutes, agent_expiry_minutes, send_resolutions, fallback_route_id, severities FROM config_notification_routes ORDER BY route_id"; @@ -170,6 +173,7 @@ public async Task> LoadSummariesAsync(CancellationTo AgentExpiryMinutes = reader.GetInt32(12), SendResolutions = reader.IsDBNull(13) ? null : reader.GetBoolean(13), FallbackRouteId = reader.IsDBNull(14) ? null : reader.GetInt32(14), + Severities = NotificationRouteKinds.NormalizeSeverities(reader.GetString(15)), }); } diff --git a/Darling/PerformanceMonitor.Darling.Service/StoreConfigProvider.cs b/Darling/PerformanceMonitor.Darling.Service/StoreConfigProvider.cs index 0c6883ad1f..d44f6cd33e 100644 --- a/Darling/PerformanceMonitor.Darling.Service/StoreConfigProvider.cs +++ b/Darling/PerformanceMonitor.Darling.Service/StoreConfigProvider.cs @@ -2790,7 +2790,7 @@ internal static void MarkSlotsTheFileDeclares(MonitoredServer server, DarlingCon public const string NotificationRoutesSelectSql = "SELECT route_id, metric_match, teams_url, slack_url, generic_url, pagerduty_routing_key, smtp_recipients, enabled, scope_server_id, scope_tag_id, " + "agent_level, agent_max_per_hour, agent_max_per_day, agent_lease_minutes, agent_expiry_minutes, send_resolutions, " - + NotificationRouteSigningColumns + ", fallback_route_id " + + NotificationRouteSigningColumns + ", fallback_route_id, severities " + "FROM config_notification_routes ORDER BY route_id"; /// A route's signing key columns (V193, #5998), last in both the service's and the viewer's route SELECT. @@ -2913,6 +2913,8 @@ internal static async Task> ReadNotificationRou clockAhead), /* V193 (#6003, D8): where an unclaimed, expired investigation request goes instead. */ FallbackRouteId = reader.IsDBNull(19) ? null : reader.GetInt32(19), + /* V229 (#6928): the severity filter; a word this build does not know reads as both. */ + Severities = NotificationRouteKinds.NormalizeSeverities(reader.GetString(20)), }); } } diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin-agent-setup.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin-agent-setup.js index 15169de552..f6add02599 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin-agent-setup.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin-agent-setup.js @@ -58,7 +58,7 @@ export function entriesFromRoute(r) { if (!r) { return { id: null, match: "", scope: "", enabled: true, level: "", perHour: AGENT_LIMIT_DEFAULTS.perHour, perDay: AGENT_LIMIT_DEFAULTS.perDay, - lease: AGENT_LIMIT_DEFAULTS.lease, expiry: AGENT_LIMIT_DEFAULTS.expiry, resolutions: "", fallback: "", + lease: AGENT_LIMIT_DEFAULTS.lease, expiry: AGENT_LIMIT_DEFAULTS.expiry, resolutions: "", fallback: "", severities: "both", saved: [], hooks: {}, removes: {}, recipients: "", recipientsWas: "", modifiedAt: null, }; } @@ -76,6 +76,8 @@ export function entriesFromRoute(r) { expiry: String(r.agent_expiry_minutes ?? AGENT_LIMIT_DEFAULTS.expiry), resolutions: r.send_resolutions === true ? "true" : r.send_resolutions === false ? "false" : "", fallback: r.fallback_route_id != null ? String(r.fallback_route_id) : "", + /* #6928: a route read without the field, or with a word this page does not know, edits as Both. */ + severities: r.severities === "critical" || r.severities === "warning" ? r.severities : "both", saved: Array.isArray(r.configured_channels) ? r.configured_channels : [], hooks: {}, removes: {}, @@ -138,6 +140,7 @@ export function routePayload(entries) { agent_expiry_minutes: nums[3], send_resolutions: entries.resolutions === "" ? null : entries.resolutions === "true", fallback_route_id: fallback, + severities: entries.severities === "critical" || entries.severities === "warning" ? entries.severities : "both", ...(entries.id != null ? { expected_modified_at: entries.modifiedAt } : {}), }, }; @@ -206,6 +209,7 @@ function routeForm(options) { const controls = { match: text("match", entries.match, { placeholder: "family, 'Analysis: ', exact metric or *" }), scope: select("scope", scopeChoices, entries.scope), + severities: select("severities", [{ value: "both", label: "Both (Warning and Critical)" }, { value: "critical", label: "Critical only" }, { value: "warning", label: "Warning only" }], entries.severities || "both"), level: select("level", AGENT_LEVEL_CHOICES, entries.level), perHour: text("perHour", entries.perHour, { inputmode: "numeric" }), perDay: text("perDay", entries.perDay, { inputmode: "numeric" }), @@ -239,7 +243,8 @@ function routeForm(options) { const recipients = text("recipients", entries.recipients, { placeholder: "name@example.com, other@example.com" }); const read = () => { const e = { - id: entries.id, enabled: enabled.checked, saved: entries.saved, recipientsWas: entries.recipientsWas, hooks: {}, removes: {}, + /* #6936: modifiedAt rides along, or an edit goes out without expected_modified_at and the service refuses it with a 400. */ + id: entries.id, modifiedAt: entries.modifiedAt, enabled: enabled.checked, saved: entries.saved, recipientsWas: entries.recipientsWas, hooks: {}, removes: {}, }; for (const k of Object.keys(controls)) e[k] = controls[k].value; for (const d of WEBHOOK_DESTINATIONS) { @@ -256,7 +261,7 @@ function routeForm(options) { onClick: () => sendRoute("save", read(), options.repaint), }); const parts = [ - field("Match:", controls.match), field("Applies to:", controls.scope), field("Agent:", controls.level), + field("Match:", controls.match), field("Applies to:", controls.scope), field("Severity:", controls.severities), field("Agent:", controls.level), field("Requests per hour:", controls.perHour), field("Requests per day:", controls.perDay), field("Lease minutes:", controls.lease), field("Expiry minutes:", controls.expiry), field("Send resolutions:", controls.resolutions), field("Fallback route:", controls.fallback), diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin.js index caf8e871af..5a43735836 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin.js @@ -92,6 +92,7 @@ export const ROUTE_COLUMNS = [ { key: "match_kind", label: "Kind" }, { key: "family", label: "Family" }, { key: "applies_to", label: "Applies to" }, + { key: "severities", label: "Severity" }, { key: "status", label: "Status" }, { key: "channels", label: "Channels set", wrap: true }, { key: "agent", label: "Agent" }, @@ -308,6 +309,13 @@ export function routeAgentText(r) { return "Off"; } +/** The Severity column (#6928), in the desktop routes window's words: which alerts the route takes by severity. A route read without + the field, or with a word this page does not know, reads as Both, the same as the service. */ +export function routeSeveritiesText(r) { + const word = r && typeof r.severities === "string" ? r.severities.trim().toLowerCase() : ""; + return word === "critical" ? "Critical only" : word === "warning" ? "Warning only" : "Both"; +} + /** The Send resolutions column (#5825), in the route dialog's words: send_resolutions null follows each channel's own "Send resolutions" setting, true always sends the cleared message to this route's destinations, false never does. A route read without the field reads as the channel setting, the same as null. */ @@ -326,6 +334,7 @@ export function routeRows(data) { match_kind: r.match_kind, family: r.family, applies_to: routeScopeText(r), + severities: routeSeveritiesText(r), status: routeStatusText(r), channels: Array.isArray(r.configured_channels) ? r.configured_channels.filter((c) => !isSecretKey(c)).join(", ") : null, agent: routeAgentText(r), diff --git a/Darling/PerformanceMonitor.Darling.Storage/ExternalAlertStore.cs b/Darling/PerformanceMonitor.Darling.Storage/ExternalAlertStore.cs index 8ab14a81a6..aa48d152bf 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/ExternalAlertStore.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/ExternalAlertStore.cs @@ -87,7 +87,8 @@ public sealed record ExternalAlertFireResult( /// the incident under these, whatever the server registry says by then. /// public sealed record ExternalAlertFiringState( - bool Firing, string? FiredSeverity, DateTime? LastFiredAtUtc, int ServerId, string ServerKey, string ServerName); + bool Firing, string? FiredSeverity, DateTime? LastFiredAtUtc, int ServerId, string ServerKey, string ServerName, + string? FiredSeverities = null); /// /// The service's side of the external alert queue (#5654): claims what a source submitted through @@ -299,7 +300,7 @@ UPDATE config.external_alert_pending /// Reads one subject's firing state. $1 source_id, $2 alert_key, $3 server_label (the normalized submitted name). public const string GetStateSql = @" -SELECT firing, fired_severity, last_fired_at, server_id, server_key, server_name +SELECT firing, fired_severity, last_fired_at, server_id, server_key, server_name, fired_severities FROM config.external_alert_state WHERE source_id = $1 AND alert_key = $2 AND server_label = $3"; @@ -329,6 +330,7 @@ ON CONFLICT (source_id, alert_key, server_label) DO UPDATE SET server_key = CASE WHEN config.external_alert_state.firing THEN config.external_alert_state.server_key ELSE EXCLUDED.server_key END, server_name = CASE WHEN config.external_alert_state.firing THEN config.external_alert_state.server_name ELSE EXCLUDED.server_name END, fired_severity = CASE WHEN config.external_alert_state.firing THEN config.external_alert_state.fired_severity ELSE NULL END, + fired_severities = CASE WHEN config.external_alert_state.firing THEN config.external_alert_state.fired_severities ELSE NULL END, firing = true, last_fired_at = EXCLUDED.last_fired_at RETURNING server_id, server_key, server_name @@ -339,10 +341,16 @@ SELECT COALESCE((SELECT firing FROM prev), false), (SELECT fired_severity FROM p /// /// Records that a fire's send went out (#5654), so the severity is the one the operator was told. $1 source_id, /// $2 alert_key, $3 server_label, $4 severity. Touches only a subject that is firing. + /// fired_severities (#6928) is every severity sent during the open episode, comma separated, each once, so the + /// clear can close what a Warning-only route opened after the alert climbed to Critical. fired_severity stays the newest. /// public const string ConfirmFireSql = @" UPDATE config.external_alert_state -SET fired_severity = $4 +SET fired_severity = $4, + fired_severities = CASE + WHEN fired_severities IS NULL THEN $4::text + WHEN position(',' || $4::text || ',' IN ',' || fired_severities || ',') > 0 THEN fired_severities + ELSE fired_severities || ',' || $4::text END WHERE source_id = $1 AND alert_key = $2 AND server_label = $3 AND firing"; @@ -672,7 +680,8 @@ public async Task ReapAsync(TimeSpan claimedFor, CancellationToken cancella reader.IsDBNull(2) ? null : reader.GetFieldValue(2), reader.GetInt32(3), reader.GetString(4), - reader.GetString(5)); + reader.GetString(5), + reader.IsDBNull(6) ? null : reader.GetString(6)); } /// diff --git a/Darling/PerformanceMonitor.Darling.Storage/InvestigationPracticeStore.cs b/Darling/PerformanceMonitor.Darling.Storage/InvestigationPracticeStore.cs index 0c54dbe3c5..82e9b865e9 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/InvestigationPracticeStore.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/InvestigationPracticeStore.cs @@ -179,8 +179,8 @@ INSERT INTO config.investigation_requests /// The case's alert rows, copied without the dedup key. $1 new request, $2 case. internal const string CopyAlertsSql = @" INSERT INTO config.investigation_request_alerts - (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json) -SELECT $1, alert_time, server_id, metric_name, NULL, current_value, threshold_value, muted, detail_text, context_json + (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json, severity) +SELECT $1, alert_time, server_id, metric_name, NULL, current_value, threshold_value, muted, detail_text, context_json, severity FROM config.investigation_request_alerts WHERE request_id = $2"; diff --git a/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.AgentAction.cs b/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.AgentAction.cs index bddba77ca9..bc297e3b36 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.AgentAction.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.AgentAction.cs @@ -16,6 +16,7 @@ using Npgsql; using NpgsqlTypes; using PerformanceMonitor.Common; +using PerformanceMonitor.Notifications; namespace PerformanceMonitor.Darling.Storage; @@ -62,7 +63,7 @@ public sealed class AgentActionHooks /// at most once per gate pass, so a request with several metrics does not read the route table (and its scope inputs) again /// for each one. /// - public required Func>> LoadLevels { get; init; } + public required Func>> LoadLevels { get; init; } /// Check 7, the kind rule; null when the kind has none. public Func>? KindRule { get; init; } @@ -160,7 +161,7 @@ FROM config.investigation_requests /// Every metric that opened or joined the request, for the re-resolve: the lowest level wins. internal const string AgentActionMetricsSql = @" -SELECT DISTINCT metric_name +SELECT DISTINCT metric_name, severity FROM config.investigation_request_alerts WHERE request_id = $1"; @@ -528,18 +529,18 @@ await ServerActionSwitchLock.AcquireSharedAsync(connection, transaction, call.Ta /// after the request opened resolves to null and lowers the request to 0: fail closed (pinned live). /// private static async Task ResolveRequestLevelAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, - AgentActionRequest request, Func>> loadLevels, + AgentActionRequest request, Func>> loadLevels, int floor, CancellationToken cancellationToken) { var level = request.AutonomyLevel; if (level >= floor) { var resolve = await loadLevels(connection, transaction, cancellationToken); - foreach (var metric in request.Metrics) + foreach (var (metric, severity) in request.Metrics) { if (level < floor) break; - level = Math.Min(level, resolve(metric, request.ServerId) ?? 0); + level = Math.Min(level, resolve(metric, request.ServerId, severity) ?? 0); } } @@ -548,7 +549,7 @@ private static async Task ResolveRequestLevelAsync(NpgsqlConnection connect private sealed record AgentActionRequest(string State, Guid? ClaimId, string? ClaimedBy, DateTime? LeaseUntil, DateTime ExpiresAt, int ServerId, string ServerName, Guid? PracticeOf, int AutonomyLevel, int ClaimCount, - IReadOnlyList Metrics); + IReadOnlyList<(string Metric, AlertSeverityLevel? Severity)> Metrics); private async Task ReadAgentActionRequestAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, Guid requestId, CancellationToken cancellationToken) @@ -572,21 +573,30 @@ private sealed record AgentActionRequest(string State, Guid? ClaimId, string? Cl reader.IsDBNull(7) ? null : reader.GetGuid(7), reader.GetInt16(8), reader.GetInt32(9), - Array.Empty()); + Array.Empty<(string, AlertSeverityLevel?)>()); } - var metrics = new List { firstMetric }; + /* #6928: each alert's metric at the tier it arrived at, so the live routes are read at the tier dispatch used: a Critical alert + dispatched under one route is not re-resolved under a Warning-only route that dispatch skipped. The first metric stands + alone only when no alert row names it (an old request), and resolves with no tier. */ + var metrics = new List<(string Metric, AlertSeverityLevel? Severity)>(); await using (var command = Command(connection, transaction, AgentActionMetricsSql, P(requestId))) await using (var reader = await command.ExecuteReaderAsync(cancellationToken)) { while (await reader.ReadAsync(cancellationToken)) { var metric = reader.GetString(0); - if (!metrics.Contains(metric, StringComparer.Ordinal)) - metrics.Add(metric); + var severity = reader.IsDBNull(1) ? null + : string.Equals(reader.GetString(1), InvestigationEvents.Critical, StringComparison.Ordinal) ? AlertSeverityLevel.Critical + : (AlertSeverityLevel?)AlertSeverityLevel.Warning; + if (!metrics.Contains((metric, severity))) + metrics.Add((metric, severity)); } } + if (!metrics.Any(m => string.Equals(m.Metric, firstMetric, StringComparison.Ordinal))) + metrics.Insert(0, (firstMetric, null)); + return row with { Metrics = metrics }; } diff --git a/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.ServerAction.cs b/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.ServerAction.cs index 58a32c6fc9..206cb584cb 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.ServerAction.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.ServerAction.cs @@ -11,6 +11,7 @@ using System.Threading.Tasks; using Npgsql; using PerformanceMonitor.Common; +using PerformanceMonitor.Notifications; namespace PerformanceMonitor.Darling.Storage; @@ -24,7 +25,7 @@ public sealed record ServerActionCall(string Tool, string ServerAction, AgentAct /// . It never resolves the secret. public sealed class ServerActionHooks { - public required Func>> LoadLevels { get; init; } + public required Func>> LoadLevels { get; init; } public required Func> ReadCredential { get; init; } } @@ -131,7 +132,7 @@ await JournalRefusedAsync(connection, transaction, now, request is null ? null : /// private async Task ServerActionConditionsRefusalAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, AgentActionRequest? request, string serverAction, AgentAction action, AgentActionTarget target, - Func>> loadLevels, DateTime now, bool practice, + Func>> loadLevels, DateTime now, bool practice, CancellationToken cancellationToken) { if (ClaimOrPracticeRefusal(request, action, now, practice) is { } claimRefusal) diff --git a/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs b/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs index af5a67ff9f..6236495d42 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs @@ -149,8 +149,8 @@ INSERT INTO config.investigation_requests internal const string AlertRowsSql = @" INSERT INTO config.investigation_request_alerts - (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json) -SELECT $1, $2, $3, $4, k, $6, $7, $8, $9, $10 + (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json, severity) +SELECT $1, $2, $3, $4, k, $6, $7, $8, $9, $10, $11 FROM unnest($5::text[]) AS k"; internal const string JournalSql = @" @@ -294,8 +294,8 @@ FROM config.investigation_requests AS r /* #6008: the source's alert rows, without the dedup key, so the brief's alerts section reads what the source's read. $1 new, $2 source. */ internal const string CopyFollowUpAlertsSql = @" INSERT INTO config.investigation_request_alerts - (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json) -SELECT $1, alert_time, server_id, metric_name, NULL, current_value, threshold_value, muted, detail_text, context_json + (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json, severity) +SELECT $1, alert_time, server_id, metric_name, NULL, current_value, threshold_value, muted, detail_text, context_json, severity FROM config.investigation_request_alerts WHERE request_id = $2"; @@ -1682,7 +1682,9 @@ private Task InsertAlertRowsAsync(NpgsqlConnection connection, NpgsqlTransa new NpgsqlParameter { TypedValue = facts?.ThresholdValue, NpgsqlDbType = NpgsqlDbType.Double }, new NpgsqlParameter { TypedValue = facts?.Muted, NpgsqlDbType = NpgsqlDbType.Boolean }, new NpgsqlParameter { TypedValue = DerivedStoreScrub.Text(facts?.DetailText), NpgsqlDbType = NpgsqlDbType.Text }, - new NpgsqlParameter { TypedValue = DerivedStoreScrub.Json(facts?.ContextJson), NpgsqlDbType = NpgsqlDbType.Text }); + new NpgsqlParameter { TypedValue = DerivedStoreScrub.Json(facts?.ContextJson), NpgsqlDbType = NpgsqlDbType.Text }, + /* #6928: the tier this alert arrived at, so the agent-action gate resolves its route at that tier. */ + new NpgsqlParameter { TypedValue = AlertSeverityWord(alert.Severity), NpgsqlDbType = NpgsqlDbType.Text }); } /// diff --git a/Darling/PerformanceMonitor.Darling.Storage/NotificationRouteWriter.cs b/Darling/PerformanceMonitor.Darling.Storage/NotificationRouteWriter.cs index 07184ec9d9..be655c035e 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/NotificationRouteWriter.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/NotificationRouteWriter.cs @@ -62,6 +62,9 @@ public sealed class NotificationRouteDraft public bool? SendResolutions { get; set; } public int? FallbackRouteId { get; set; } + /// The severity filter (V229, #6928): both, critical or warning. + public string Severities { get; set; } = NotificationRouteKinds.SeveritiesBoth; + /// When the route was last saved, as the writer's screen read it (the store's modified_at, UTC). On a stored draft /// handed to it is the version the save expects: a route saved by anyone since is refused /// as a conflict instead of overwritten (#6274). Null or default skips the check. @@ -93,8 +96,8 @@ public static class NotificationRouteWriter /// Inserts one route. route_id is GENERATED ALWAYS AS IDENTITY, so it is never in the column list. public const string InsertSql = @" INSERT INTO config_notification_routes - (metric_match, " + DestinationColumns + @", enabled, scope_server_id, scope_tag_id, " + AgentColumns + @", send_resolutions, fallback_route_id, modified_at) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, (now() AT TIME ZONE 'UTC')) + (metric_match, " + DestinationColumns + @", enabled, scope_server_id, scope_tag_id, " + AgentColumns + @", send_resolutions, fallback_route_id, severities, modified_at) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, (now() AT TIME ZONE 'UTC')) RETURNING route_id"; public const string UpdateSql = @" @@ -115,6 +118,7 @@ UPDATE config_notification_routes SET agent_expiry_minutes = $15, send_resolutions = $16, fallback_route_id = $17, + severities = $18, modified_at = (now() AT TIME ZONE 'UTC') WHERE route_id = $1"; @@ -215,7 +219,7 @@ await InvestigationPersonJournal.InsertAsync( /// plaintext) and the address list. Only a writer with the service's login reads them. public const string SelectStoredSql = "SELECT route_id, metric_match, teams_url, slack_url, generic_url, pagerduty_routing_key, smtp_recipients, enabled, " - + "scope_server_id, scope_tag_id, " + AgentColumns + ", send_resolutions, fallback_route_id, modified_at " + + "scope_server_id, scope_tag_id, " + AgentColumns + ", send_resolutions, fallback_route_id, modified_at, severities " + "FROM config_notification_routes WHERE route_id = $1"; /// The proxy of each webhook channel on the settings row (the row id 1 the Viewer reads). @@ -272,6 +276,11 @@ await InvestigationPersonJournal.InsertAsync( return "A route cannot be its own fallback route."; } + if (SeveritiesRefusal(row.Severities) is { } severities) + { + return severities; + } + if (row.AgentLevel is { } level && level is not (NotificationRoute.AgentLevelOff or NotificationRoute.AgentLevelInvestigate or NotificationRoute.AgentLevelAct or NotificationRoute.AgentLevelServerAct)) { return "The agent channel is Inherit, Off, Investigate, Act or Act on servers."; @@ -392,6 +401,13 @@ public static (int Count, string[] Domains) DescribeRecipients(string? recipient return await command.ExecuteScalarAsync(cancellationToken) is null ? $"Fallback route {fallbackId} does not exist." : null; } + /// The sentence for a severity filter that is not both, critical or warning (case-insensitive), else null (#6928). + public static string? SeveritiesRefusal(string? severities) => + severities?.Trim().ToLowerInvariant() is null or "" or NotificationRouteKinds.SeveritiesBoth + or NotificationRouteKinds.SeveritiesCritical or NotificationRouteKinds.SeveritiesWarning + ? null + : "severities is critical, warning or both."; + /// Binds the agent columns, the resolution override and the fallback, in the order every insert and update takes them. public static void AddAgentColumns(NpgsqlCommand command, NotificationRouteDraft row) { @@ -410,6 +426,8 @@ public static void AddAgentColumns(NpgsqlCommand command, NotificationRouteDraft : new NpgsqlParameter { Value = DBNull.Value, NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Boolean }); /* #6036: the fallback route, bound last. NULL is none. */ AddNullableInt(command, row.FallbackRouteId); + /* #6928 (V229): the severity filter, bound after the fallback. */ + command.Parameters.Add(new NpgsqlParameter { TypedValue = NotificationRouteKinds.NormalizeSeverities(row.Severities) }); } /// Binds an integer that may be SQL NULL. @@ -633,6 +651,7 @@ private static NotificationRouteDraft ReadStoredRow(NpgsqlDataReader reader) SendResolutions = reader.IsDBNull(15) ? null : reader.GetBoolean(15), FallbackRouteId = reader.IsDBNull(16) ? null : reader.GetInt32(16), ModifiedAtUtc = reader.IsDBNull(17) ? null : DateTime.SpecifyKind(reader.GetDateTime(17), DateTimeKind.Utc), + Severities = reader.GetString(18), }; } @@ -727,7 +746,7 @@ public static async Task SaveAsync( update.Parameters.Add(new NpgsqlParameter { TypedValue = resolved.Enabled }); // $8 AddNullableInt(update, resolved.ScopeServerId); // $9 AddNullableInt(update, resolved.ScopeTagId); // $10 - AddAgentColumns(update, resolved); // $11-$17 + AddAgentColumns(update, resolved); // $11-$18 await update.ExecuteNonQueryAsync(cancellationToken); if (login is not null && AgentChange(before, resolved) is { } change) { @@ -797,6 +816,12 @@ void Compare(string name, object? was, object? now) Compare("metric_match", before?.MetricMatch.Trim(), after?.MetricMatch.Trim()); Compare("scope_server_id", before?.ScopeServerId, after?.ScopeServerId); Compare("scope_tag_id", before?.ScopeTagId, after?.ScopeTagId); + /* #6928: the severity filter moves alerts between agent routes. A route with no agent setting is not in the agent lookup. */ + if (before?.AgentLevel is not null || after?.AgentLevel is not null) + { + Compare("severities", NotificationRouteKinds.NormalizeSeverities(before?.Severities), NotificationRouteKinds.NormalizeSeverities(after?.Severities)); + } + Compare("agent_level", before?.AgentLevel, after?.AgentLevel); Compare("agent_max_per_hour", before?.AgentMaxPerHour, after?.AgentMaxPerHour); Compare("agent_max_per_day", before?.AgentMaxPerDay, after?.AgentMaxPerDay); @@ -823,6 +848,7 @@ void Compare(string name, object? was, object? now) ["agent_lease_minutes"] = row.AgentLeaseMinutes, ["agent_expiry_minutes"] = row.AgentExpiryMinutes, ["enabled"] = row.Enabled, + ["severities"] = NotificationRouteKinds.NormalizeSeverities(row.Severities), }; var detail = new System.Text.Json.Nodes.JsonObject { ["before"] = Describe(before), ["after"] = Describe(after) }; diff --git a/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs b/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs index 5b7a30ad75..0430980aea 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs @@ -2977,6 +2977,7 @@ holds the ordering. */ new Migration(226, "server-aws-rds-web-or-express", V226Sql), new Migration(227, "threshold-override-tag-scope", V227Sql), new Migration(228, "blocking-band-tiers", V228Sql), + new Migration(229, "notification-route-severities", V229Sql), }; /// @@ -3064,6 +3065,34 @@ ALTER TABLE config.config_alert_settings ADD COLUMN IF NOT EXISTS blocking_warn_wait_seconds double precision NOT NULL DEFAULT 10.0; ALTER TABLE config.config_alert_settings ADD COLUMN IF NOT EXISTS blocking_critical_wait_seconds double precision NOT NULL DEFAULT 60.0;"; + /// V229 (#6928): config.config_notification_routes.severities, the severity filter of a notification route: both (the + /// default, Warning and Critical alerts alike, which is what every route did before), critical or warning. NOT NULL with + /// the default, so every stored route reads both and keeps its effect, and a CHECK holds the three words. An alert that is not + /// Critical counts as Warning. A route whose filter excludes an alert does not match it, and matching goes on to the next route. + /// Idempotent: the column and the constraint are each added only when absent. No backfill and no view refresh: a registry table. + /// Also config.external_alert_state.fired_severities, every severity an external alert sent during its open episode + /// (comma separated), so its clear closes the incident of each tier's route. Null on an episode already open, which reads as + /// the newest severity alone. And config.investigation_request_alerts.severity, the tier (critical or warning) each + /// alert row of an agent request arrived at, so the agent-action gate re-resolves each alert's route at the tier dispatch used. Null on + /// a row written before, which resolves with no tier. + /// + private const string V229Sql = @" +ALTER TABLE config.config_notification_routes + ADD COLUMN IF NOT EXISTS severities text NOT NULL DEFAULT 'both'; +ALTER TABLE config.external_alert_state + ADD COLUMN IF NOT EXISTS fired_severities text; +ALTER TABLE config.investigation_request_alerts + ADD COLUMN IF NOT EXISTS severity text; +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'ck_config_notification_routes_severities' + AND conrelid = 'config.config_notification_routes'::regclass) THEN + ALTER TABLE config.config_notification_routes + ADD CONSTRAINT ck_config_notification_routes_severities + CHECK (severities IN ('both', 'critical', 'warning')); + END IF; +END +$$;"; /// /// V189 (#5867): collect.query_store_compose_stamp_failures, the failure record of the Query Store compose builder: one row diff --git a/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs b/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs index 0db43132aa..e6b397cbec 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs @@ -16,5 +16,5 @@ namespace PerformanceMonitor.Darling.Storage; /// public static class StorageVersion { - public const int SchemaVersion = 228; + public const int SchemaVersion = 229; } diff --git a/Darling/PerformanceMonitor.Darling.Viewer/NotificationRouteEditDialog.xaml b/Darling/PerformanceMonitor.Darling.Viewer/NotificationRouteEditDialog.xaml index 02430e69e8..f18797d186 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/NotificationRouteEditDialog.xaml +++ b/Darling/PerformanceMonitor.Darling.Viewer/NotificationRouteEditDialog.xaml @@ -46,9 +46,17 @@ - + + + + + + + + diff --git a/Darling/PerformanceMonitor.Darling.Viewer/NotificationRouteEditDialog.xaml.cs b/Darling/PerformanceMonitor.Darling.Viewer/NotificationRouteEditDialog.xaml.cs index b8b0ad1de2..65ccf6535a 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/NotificationRouteEditDialog.xaml.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/NotificationRouteEditDialog.xaml.cs @@ -33,6 +33,11 @@ public partial class NotificationRouteEditDialog : Window internal const string ResolutionsChannelChoice = "Use channel setting"; internal const string ResolutionsAlwaysChoice = "Always"; internal const string ResolutionsNeverChoice = "Never"; + + /* #6928: the severity filter's three choices, in the order they are offered. */ + internal const string SeveritiesBothChoice = "Both (Warning and Critical)"; + internal const string SeveritiesCriticalChoice = "Critical only"; + internal const string SeveritiesWarningChoice = "Warning only"; private const string AgentOffChoice = "Off"; /* #5912: shown only on a route already stored at Level 2, and never chosen from here (the write path for Act is a later slice). */ private const string AgentActChoice = "Act (Level 2: investigate and change alert settings)"; @@ -94,6 +99,12 @@ public NotificationRouteEditDialog(ViewerRouteScopeInputs? scopeInputs, Notifica var storedResolutions = existing?.SendResolutions; SendResolutionsCombo.SelectedItem = SendResolutionsCombo.Items.Cast().First(i => Equals(i.Tag, storedResolutions)); + SeveritiesCombo.Items.Add(new ComboBoxItem { Content = SeveritiesBothChoice, Tag = NotificationRouteKinds.SeveritiesBoth }); + SeveritiesCombo.Items.Add(new ComboBoxItem { Content = SeveritiesCriticalChoice, Tag = NotificationRouteKinds.SeveritiesCritical }); + SeveritiesCombo.Items.Add(new ComboBoxItem { Content = SeveritiesWarningChoice, Tag = NotificationRouteKinds.SeveritiesWarning }); + var storedSeverities = NotificationRouteKinds.NormalizeSeverities(existing?.Severities); + SeveritiesCombo.SelectedItem = SeveritiesCombo.Items.Cast().First(i => Equals(i.Tag, storedSeverities)); + /* #6036: the fallback picker lists every OTHER route, with a none choice; the route being edited is never offered. */ FallbackRouteCombo.Items.Add(new ComboBoxItem { Content = FallbackNoneChoice, Tag = null }); foreach (var other in (allRoutes ?? System.Linq.Enumerable.Empty()).Where(r => r.RouteId != (existing?.RouteId ?? 0))) @@ -226,6 +237,7 @@ private void Save_Click(object sender, RoutedEventArgs e) Route.PagerDutyRoutingKey = ViewerWebhookSealing.CarryKept(PagerDutyKeyBox.Text, _stored?.PagerDutyRoutingKey); Route.SmtpRecipients = SmtpRecipientsBox.Text.Trim(); Route.Enabled = EnabledCheckBox.IsChecked == true; + Route.Severities = NotificationRouteKinds.NormalizeSeverities((SeveritiesCombo.SelectedItem as ComboBoxItem)?.Tag as string); var error = ReadAgent(); error ??= ViewerDataService.ValidateNotificationRoute(Route); diff --git a/Darling/PerformanceMonitor.Darling.Viewer/NotificationRoutesWindow.xaml b/Darling/PerformanceMonitor.Darling.Viewer/NotificationRoutesWindow.xaml index f1f61049ac..11580d8760 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/NotificationRoutesWindow.xaml +++ b/Darling/PerformanceMonitor.Darling.Viewer/NotificationRoutesWindow.xaml @@ -52,6 +52,8 @@ + + diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.NotificationRoutes.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.NotificationRoutes.cs index 1c339dc20d..6b64f53101 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.NotificationRoutes.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.NotificationRoutes.cs @@ -43,21 +43,38 @@ public sealed partial class ViewerDataService projections carry them, so a read-only seat sees which routes send alerts to agents. */ private const string NotificationRouteAgentColumns = NotificationRouteWriter.AgentColumns; - /// The full row, for an admin seat, with the two scope columns (V181, #5750). Ordered by - /// route_id; the window re-sorts the grid newest first. The resolver's order is the scope tier, then the - /// metric level, then the route id, so the id order is no longer the order routes are consulted in. + /* The columns both projections share, so the V229 variants below differ only by the one appended column. */ + private const string NotificationRoutesAdminColumns = + "route_id, metric_match, " + NotificationRouteDestinationColumns + ", enabled, modified_at, configured_channels, scope_server_id, scope_tag_id, " + + NotificationRouteAgentColumns + ", send_resolutions, fallback_route_id"; + + private const string NotificationRoutesNoSecretColumns = + "route_id, metric_match, smtp_recipients, enabled, modified_at, configured_channels, scope_server_id, scope_tag_id, " + + NotificationRouteAgentColumns + ", send_resolutions, fallback_route_id"; + + /// The store version that added config_notification_routes.severities (V229, #6928). A store below it has no column, so + /// the list reads the statements without it and every route shows as Both. + internal const int RouteSeveritiesStoreSchemaVersion = 229; + + /// The full row, for an admin seat, with the two scope columns (V181, #5750), for a store below V229 (no + /// severities column). Ordered by route_id; the window re-sorts the grid newest first. The resolver's order is the + /// scope tier, then the metric level, then the route id, so the id order is no longer the order routes are consulted in. public const string NotificationRoutesSelectSql = - "SELECT route_id, metric_match, " + NotificationRouteDestinationColumns + ", enabled, modified_at, configured_channels, scope_server_id, scope_tag_id, " - + NotificationRouteAgentColumns + ", send_resolutions, fallback_route_id " - + "FROM config_notification_routes ORDER BY route_id"; + "SELECT " + NotificationRoutesAdminColumns + " FROM config_notification_routes ORDER BY route_id"; /// The secret-free projection a read-only viewer seat reads: the four webhook-class /// destinations are omitted (they 42501 for that role), smtp_recipients stays (an address list is /// not a secret, on the parent either), and configured_channels carries presence. public const string NotificationRoutesSelectNoSecretSql = - "SELECT route_id, metric_match, smtp_recipients, enabled, modified_at, configured_channels, scope_server_id, scope_tag_id, " - + NotificationRouteAgentColumns + ", send_resolutions, fallback_route_id " - + "FROM config_notification_routes ORDER BY route_id"; + "SELECT " + NotificationRoutesNoSecretColumns + " FROM config_notification_routes ORDER BY route_id"; + + /// The full row with the severity filter as the last column (V229, #6928), for a store that has it. + public const string NotificationRoutesSelectWithSeveritiesSql = + "SELECT " + NotificationRoutesAdminColumns + ", severities FROM config_notification_routes ORDER BY route_id"; + + /// The secret-free projection with the severity filter as the last column (V229, #6928). The column is not a secret. + public const string NotificationRoutesSelectNoSecretWithSeveritiesSql = + "SELECT " + NotificationRoutesNoSecretColumns + ", severities FROM config_notification_routes ORDER BY route_id"; /// Inserts one route (#6231: the one copy is , which the MCP tools share). public const string NotificationRouteInsertSql = NotificationRouteWriter.InsertSql; @@ -75,12 +92,18 @@ public sealed partial class ViewerDataService public async Task> GetNotificationRoutesAsync(CancellationToken cancellationToken = default) { var rows = new List(); - await using var command = _dataSource.CreateCommand(IsReadOnly ? NotificationRoutesSelectNoSecretSql : NotificationRoutesSelectSql); + /* V229 (#6928): a store below it has no severities column, so the list reads the older statements and every route shows as Both. */ + var version = _cachedStoreSchemaVersion ??= await GetStoreSchemaVersionAsync(cancellationToken); + var hasSeverities = version is int known && known >= RouteSeveritiesStoreSchemaVersion; + var sql = IsReadOnly + ? (hasSeverities ? NotificationRoutesSelectNoSecretWithSeveritiesSql : NotificationRoutesSelectNoSecretSql) + : (hasSeverities ? NotificationRoutesSelectWithSeveritiesSql : NotificationRoutesSelectSql); + await using var command = _dataSource.CreateCommand(sql); command.CommandTimeout = ViewerCommandDeadlines.CurrentInteractiveReadSeconds; await using var reader = await command.ExecuteReaderAsync(cancellationToken); while (await reader.ReadAsync(cancellationToken)) { - rows.Add(IsReadOnly ? ReadRouteRowNoSecret(reader) : ReadRouteRow(reader)); + rows.Add(IsReadOnly ? ReadRouteRowNoSecret(reader, hasSeverities) : ReadRouteRow(reader, hasSeverities)); } foreach (var row in rows.Where(r => r.FallbackRouteId.HasValue)) @@ -301,7 +324,7 @@ private static void ValidateRoute(NotificationRouteRow row) } } - private static NotificationRouteRow ReadRouteRow(NpgsqlDataReader reader) => new() + private static NotificationRouteRow ReadRouteRow(NpgsqlDataReader reader, bool hasSeverities) => new() { RouteId = reader.GetInt32(0), MetricMatch = reader.GetString(1), @@ -322,11 +345,12 @@ private static void ValidateRoute(NotificationRouteRow row) AgentExpiryMinutes = reader.GetInt32(16), SendResolutions = reader.IsDBNull(17) ? null : reader.GetBoolean(17), FallbackRouteId = reader.IsDBNull(18) ? null : reader.GetInt32(18), + Severities = hasSeverities ? NotificationRouteKinds.NormalizeSeverities(reader.GetString(19)) : NotificationRouteKinds.SeveritiesBoth, }; /// The secret-free reader: the four carved destinations stay empty (the seat cannot author them /// anyway) and every later column shifts ordinal, the ReadNotificationRowNoSecret idiom. - private static NotificationRouteRow ReadRouteRowNoSecret(NpgsqlDataReader reader) => new() + private static NotificationRouteRow ReadRouteRowNoSecret(NpgsqlDataReader reader, bool hasSeverities) => new() { RouteId = reader.GetInt32(0), MetricMatch = reader.GetString(1), @@ -347,6 +371,7 @@ private static void ValidateRoute(NotificationRouteRow row) AgentExpiryMinutes = reader.GetInt32(12), SendResolutions = reader.IsDBNull(13) ? null : reader.GetBoolean(13), FallbackRouteId = reader.IsDBNull(14) ? null : reader.GetInt32(14), + Severities = hasSeverities ? NotificationRouteKinds.NormalizeSeverities(reader.GetString(15)) : NotificationRouteKinds.SeveritiesBoth, }; } @@ -396,6 +421,18 @@ public sealed class NotificationRouteRow /// true always sends it, false never does. public bool? SendResolutions { get; set; } + /// The severity filter (V229, #6928): , + /// or . A route the filter excludes an alert from is skipped and matching goes on. + public string Severities { get; set; } = NotificationRouteKinds.SeveritiesBoth; + + /// The Severity column: Both, Critical only or Warning only (#6928). + public string SeveritiesDisplay => NotificationRouteKinds.NormalizeSeverities(Severities) switch + { + NotificationRouteKinds.SeveritiesCritical => "Critical only", + NotificationRouteKinds.SeveritiesWarning => "Warning only", + _ => "Both", + }; + /// The route an expired investigation request goes to first (#6036, D8 of #5632 Phase C), or null for none. /// Never the route itself; the delete clears it when that route is deleted. public int? FallbackRouteId { get; set; } @@ -487,6 +524,7 @@ public string ChannelsDisplay AgentExpiryMinutes = AgentExpiryMinutes, SendResolutions = SendResolutions, FallbackRouteId = FallbackRouteId, + Severities = Severities, ModifiedAtUtc = ModifiedAtUtc == default ? null : ModifiedAtUtc, }; @@ -512,6 +550,7 @@ public string ChannelsDisplay SendResolutions = SendResolutions, FallbackRouteId = FallbackRouteId, FallbackRouteName = FallbackRouteName, + Severities = Severities, ScopeDisplay = ScopeDisplay, StatusDisplay = StatusDisplay, }; diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs index 804d58102c..c6642defba 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs @@ -1383,7 +1383,10 @@ per the V71 finding. */ EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid = to_regclass('config.config_threshold_overrides') AND attname = 'scope_tag_id' AND NOT attisdropped), /* V228 (#6927) adds the four settable blocking band columns to the alert settings row. Found by catalog like the V226 column above. The warn per hour column is the sentinel, the first of the four. Named only in this probe line, never in prose, per the V71 finding. */ - EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid = to_regclass('config.config_alert_settings') AND attname = 'blocking_warn_per_hour' AND NOT attisdropped)"; + EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid = to_regclass('config.config_alert_settings') AND attname = 'blocking_warn_per_hour' AND NOT attisdropped), + /* V229 (#6928) adds the severity filter to the notification routes. Found by catalog like the V225 column above. Named only in + this probe line, never in prose, per the V71 finding. */ + EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid = to_regclass('config.config_notification_routes') AND attname = 'severities' AND NOT attisdropped)"; /// The store schema version this viewer build requires — the highest migration it knows /// (). The connect-time gate blocks a store below this. @@ -1405,7 +1408,7 @@ per the V71 finding. */ await using var reader = await command.ExecuteReaderAsync(cancellationToken); if (await reader.ReadAsync(cancellationToken)) { - return MapProbedSchemaVersion(reader.GetBoolean(0), reader.GetBoolean(1), reader.GetBoolean(2), reader.GetBoolean(3), reader.GetBoolean(4), reader.GetBoolean(5), reader.GetBoolean(6), reader.GetBoolean(7), reader.GetBoolean(8), reader.GetBoolean(9), reader.GetBoolean(10), reader.GetBoolean(11), reader.GetBoolean(12), reader.GetBoolean(13), reader.GetBoolean(14), reader.GetBoolean(15), reader.GetBoolean(16), reader.GetBoolean(17), reader.GetBoolean(18), reader.GetBoolean(19), reader.GetBoolean(20), reader.GetBoolean(21), reader.GetBoolean(22), reader.GetBoolean(23), reader.GetBoolean(24), reader.GetBoolean(25), reader.GetBoolean(26), reader.GetBoolean(27), reader.GetBoolean(28), reader.GetBoolean(29), reader.GetBoolean(30), reader.GetBoolean(31), reader.GetBoolean(32), reader.GetBoolean(33), reader.GetBoolean(34), reader.GetBoolean(35), reader.GetBoolean(36), reader.GetBoolean(37), reader.GetBoolean(38), reader.GetBoolean(39), reader.GetBoolean(40), reader.GetBoolean(41), reader.GetBoolean(42), reader.GetBoolean(43), reader.GetBoolean(44), reader.GetBoolean(45), reader.GetBoolean(46), reader.GetBoolean(47), reader.GetBoolean(48), reader.GetBoolean(49), reader.GetBoolean(50), reader.GetBoolean(51), reader.GetBoolean(52), reader.GetBoolean(53), reader.GetBoolean(54), reader.GetBoolean(55), reader.GetBoolean(56), reader.GetBoolean(57), reader.GetBoolean(58), reader.GetBoolean(59), reader.GetBoolean(60), reader.GetBoolean(61), reader.GetBoolean(62), reader.GetBoolean(63), reader.GetBoolean(64), reader.GetBoolean(65), reader.GetBoolean(66), reader.GetBoolean(67), reader.GetBoolean(68), reader.GetBoolean(69), reader.GetBoolean(70), reader.GetBoolean(71), reader.GetBoolean(72), reader.GetBoolean(73), reader.GetBoolean(74), reader.GetBoolean(75), reader.GetBoolean(76), reader.GetBoolean(77), reader.GetBoolean(78), reader.GetBoolean(79), reader.GetBoolean(80), reader.GetBoolean(81), reader.GetBoolean(82), reader.GetBoolean(83), reader.GetBoolean(84), reader.GetBoolean(85), reader.GetBoolean(86), reader.GetBoolean(87), reader.GetBoolean(88), reader.GetBoolean(89), reader.GetBoolean(90), reader.GetBoolean(91), reader.GetBoolean(92), reader.GetBoolean(93), reader.GetBoolean(94), reader.GetBoolean(95), reader.GetBoolean(96), reader.GetBoolean(97), reader.GetBoolean(98), reader.GetBoolean(99), reader.GetBoolean(100), reader.GetBoolean(101), reader.GetBoolean(102), reader.GetBoolean(103), reader.GetBoolean(104), reader.GetBoolean(105), reader.GetBoolean(106), reader.GetBoolean(107), reader.GetBoolean(108), reader.GetBoolean(109), reader.GetBoolean(110), reader.GetBoolean(111), reader.GetBoolean(112), reader.GetBoolean(113), reader.GetBoolean(114), reader.GetBoolean(115), reader.GetBoolean(116), reader.GetBoolean(117), reader.GetBoolean(118), reader.GetBoolean(119), reader.GetBoolean(120), reader.GetBoolean(121), reader.GetBoolean(122), reader.GetBoolean(123), reader.GetBoolean(124), reader.GetBoolean(125), reader.GetBoolean(126), reader.GetBoolean(127), reader.GetBoolean(128), reader.GetBoolean(129), reader.GetBoolean(130), reader.GetBoolean(131), reader.GetBoolean(132), reader.GetBoolean(133), reader.GetBoolean(134), reader.GetBoolean(135), reader.GetBoolean(136), reader.GetBoolean(137), reader.GetBoolean(138), reader.GetBoolean(139), reader.GetBoolean(140), reader.GetBoolean(141), reader.GetBoolean(142), reader.GetBoolean(143), reader.GetBoolean(144), reader.GetBoolean(145), reader.GetBoolean(146), reader.GetBoolean(147), reader.GetBoolean(148), reader.GetBoolean(149), reader.GetBoolean(150), reader.GetBoolean(151), reader.GetBoolean(152), reader.GetBoolean(153), reader.GetBoolean(154), reader.GetBoolean(155), reader.GetBoolean(156), reader.GetBoolean(157), reader.GetBoolean(158), reader.GetBoolean(159), reader.GetBoolean(160), reader.GetBoolean(161), reader.GetBoolean(162), reader.GetBoolean(163), reader.GetBoolean(164), reader.GetBoolean(165), reader.GetBoolean(166), reader.GetBoolean(167), reader.GetBoolean(168), reader.GetBoolean(169), reader.GetBoolean(170), reader.GetBoolean(171), reader.GetBoolean(172), reader.GetBoolean(173), reader.GetBoolean(174), reader.GetBoolean(175), reader.GetBoolean(176), reader.GetBoolean(177), reader.GetBoolean(178), reader.GetBoolean(179), reader.GetBoolean(180), reader.GetBoolean(181), reader.GetBoolean(182), reader.GetBoolean(183), reader.GetBoolean(184), reader.GetBoolean(185), reader.GetBoolean(186), reader.GetBoolean(187), reader.GetBoolean(188), reader.GetBoolean(189), reader.GetBoolean(190), reader.GetBoolean(191), reader.GetBoolean(192), reader.GetBoolean(193), reader.GetBoolean(194), reader.GetBoolean(195), reader.GetBoolean(196), reader.GetBoolean(197), reader.GetBoolean(198), reader.GetBoolean(199), reader.GetBoolean(200), reader.GetBoolean(201), reader.GetBoolean(202), reader.GetBoolean(203)); + return MapProbedSchemaVersion(reader.GetBoolean(0), reader.GetBoolean(1), reader.GetBoolean(2), reader.GetBoolean(3), reader.GetBoolean(4), reader.GetBoolean(5), reader.GetBoolean(6), reader.GetBoolean(7), reader.GetBoolean(8), reader.GetBoolean(9), reader.GetBoolean(10), reader.GetBoolean(11), reader.GetBoolean(12), reader.GetBoolean(13), reader.GetBoolean(14), reader.GetBoolean(15), reader.GetBoolean(16), reader.GetBoolean(17), reader.GetBoolean(18), reader.GetBoolean(19), reader.GetBoolean(20), reader.GetBoolean(21), reader.GetBoolean(22), reader.GetBoolean(23), reader.GetBoolean(24), reader.GetBoolean(25), reader.GetBoolean(26), reader.GetBoolean(27), reader.GetBoolean(28), reader.GetBoolean(29), reader.GetBoolean(30), reader.GetBoolean(31), reader.GetBoolean(32), reader.GetBoolean(33), reader.GetBoolean(34), reader.GetBoolean(35), reader.GetBoolean(36), reader.GetBoolean(37), reader.GetBoolean(38), reader.GetBoolean(39), reader.GetBoolean(40), reader.GetBoolean(41), reader.GetBoolean(42), reader.GetBoolean(43), reader.GetBoolean(44), reader.GetBoolean(45), reader.GetBoolean(46), reader.GetBoolean(47), reader.GetBoolean(48), reader.GetBoolean(49), reader.GetBoolean(50), reader.GetBoolean(51), reader.GetBoolean(52), reader.GetBoolean(53), reader.GetBoolean(54), reader.GetBoolean(55), reader.GetBoolean(56), reader.GetBoolean(57), reader.GetBoolean(58), reader.GetBoolean(59), reader.GetBoolean(60), reader.GetBoolean(61), reader.GetBoolean(62), reader.GetBoolean(63), reader.GetBoolean(64), reader.GetBoolean(65), reader.GetBoolean(66), reader.GetBoolean(67), reader.GetBoolean(68), reader.GetBoolean(69), reader.GetBoolean(70), reader.GetBoolean(71), reader.GetBoolean(72), reader.GetBoolean(73), reader.GetBoolean(74), reader.GetBoolean(75), reader.GetBoolean(76), reader.GetBoolean(77), reader.GetBoolean(78), reader.GetBoolean(79), reader.GetBoolean(80), reader.GetBoolean(81), reader.GetBoolean(82), reader.GetBoolean(83), reader.GetBoolean(84), reader.GetBoolean(85), reader.GetBoolean(86), reader.GetBoolean(87), reader.GetBoolean(88), reader.GetBoolean(89), reader.GetBoolean(90), reader.GetBoolean(91), reader.GetBoolean(92), reader.GetBoolean(93), reader.GetBoolean(94), reader.GetBoolean(95), reader.GetBoolean(96), reader.GetBoolean(97), reader.GetBoolean(98), reader.GetBoolean(99), reader.GetBoolean(100), reader.GetBoolean(101), reader.GetBoolean(102), reader.GetBoolean(103), reader.GetBoolean(104), reader.GetBoolean(105), reader.GetBoolean(106), reader.GetBoolean(107), reader.GetBoolean(108), reader.GetBoolean(109), reader.GetBoolean(110), reader.GetBoolean(111), reader.GetBoolean(112), reader.GetBoolean(113), reader.GetBoolean(114), reader.GetBoolean(115), reader.GetBoolean(116), reader.GetBoolean(117), reader.GetBoolean(118), reader.GetBoolean(119), reader.GetBoolean(120), reader.GetBoolean(121), reader.GetBoolean(122), reader.GetBoolean(123), reader.GetBoolean(124), reader.GetBoolean(125), reader.GetBoolean(126), reader.GetBoolean(127), reader.GetBoolean(128), reader.GetBoolean(129), reader.GetBoolean(130), reader.GetBoolean(131), reader.GetBoolean(132), reader.GetBoolean(133), reader.GetBoolean(134), reader.GetBoolean(135), reader.GetBoolean(136), reader.GetBoolean(137), reader.GetBoolean(138), reader.GetBoolean(139), reader.GetBoolean(140), reader.GetBoolean(141), reader.GetBoolean(142), reader.GetBoolean(143), reader.GetBoolean(144), reader.GetBoolean(145), reader.GetBoolean(146), reader.GetBoolean(147), reader.GetBoolean(148), reader.GetBoolean(149), reader.GetBoolean(150), reader.GetBoolean(151), reader.GetBoolean(152), reader.GetBoolean(153), reader.GetBoolean(154), reader.GetBoolean(155), reader.GetBoolean(156), reader.GetBoolean(157), reader.GetBoolean(158), reader.GetBoolean(159), reader.GetBoolean(160), reader.GetBoolean(161), reader.GetBoolean(162), reader.GetBoolean(163), reader.GetBoolean(164), reader.GetBoolean(165), reader.GetBoolean(166), reader.GetBoolean(167), reader.GetBoolean(168), reader.GetBoolean(169), reader.GetBoolean(170), reader.GetBoolean(171), reader.GetBoolean(172), reader.GetBoolean(173), reader.GetBoolean(174), reader.GetBoolean(175), reader.GetBoolean(176), reader.GetBoolean(177), reader.GetBoolean(178), reader.GetBoolean(179), reader.GetBoolean(180), reader.GetBoolean(181), reader.GetBoolean(182), reader.GetBoolean(183), reader.GetBoolean(184), reader.GetBoolean(185), reader.GetBoolean(186), reader.GetBoolean(187), reader.GetBoolean(188), reader.GetBoolean(189), reader.GetBoolean(190), reader.GetBoolean(191), reader.GetBoolean(192), reader.GetBoolean(193), reader.GetBoolean(194), reader.GetBoolean(195), reader.GetBoolean(196), reader.GetBoolean(197), reader.GetBoolean(198), reader.GetBoolean(199), reader.GetBoolean(200), reader.GetBoolean(201), reader.GetBoolean(202), reader.GetBoolean(203), reader.GetBoolean(204)); } return null; @@ -1430,7 +1433,7 @@ per the V71 finding. */ /// is unit-tested without a live store; any schema bump past the newest arm trips the pinning test that keeps /// this in step with . /// - internal static int MapProbedSchemaVersion(bool hasConfigControlPlane, bool hasAlertDeliveryOverride, bool hasAnalysisState, bool hasAlertTuningKnobs, bool hasDefaultTraceEvents, bool hasIndexObjectStatsLatestIndex, bool hasCollectionLogHypertableOrPlainPg, bool hasJobHistory, bool hasAgentStatus, bool hasGenericWebhook, bool hasDeadlocksDatabaseName, bool hasQueryStoreReplicaRole, bool hasLongQueryCompletions, bool hasWebDashboardConfig, bool hasCustomViews, bool hasServerTags, bool hasConnectionRefireKnobs = false, bool hasAgCollectors = false, bool hasAgAlertKnobs = false, bool hasAgLatencyColumns = false, bool hasAgDisconnectRefire = false, bool hasPayloadDimensions = false, bool hasDimFloorIndexes = false, bool hasBlockingWaitThreshold = false, bool hasQueryStoreIntervalIdentity = false, bool hasPagerDutyWebhook = false, bool hasPagerDutyProxy = false, bool hasCollectorState = false, bool hasPlanCorrection = false, bool hasPvsStats = false, bool hasPvsPressureKnobs = false, bool hasDatabaseStateAlert = false, bool hasServerTagColour = false, bool hasQueryStatsHostObject = false, bool hasFindingDrillDown = false, bool hasStoreMetrics = false, bool hasPlanDimGzip = false, bool hasSelfAlertKnobs = false, bool hasJobMetricsColumns = false, bool hasJobCadenceKnob = false, bool hasBackfillSwitch = false, bool hasCollectorMemoryKnobs = false, bool hasDatabaseStateEdgeMemory = false, bool hasIncidentOccurrences = false, bool hasPlanXmlCompressionKnob = false, bool hasMonitoredServerEngine = false, bool hasPgBlockingEdges = false, bool hasQueryStorePlanMap = false, bool hasPgStatementText = false, bool hasQueryStoreText = false, bool hasPlanContentRetentionKnob = false, bool hasQueryStoreHealth = false, bool hasQueryStoreTextHash = false, bool hasComposeTimeoutKnob = false, bool hasFileGrowthAlert = false, bool hasCollectionLogFanoutRollup = false, bool hasTempDbMaxSize = false, bool hasServerEngineKind = false, bool hasPgDatabaseStats = false, bool hasPgIndexUsageStats = false, bool hasPgTableBloatStats = false, bool hasPgSessionStates = false, bool hasPgPlanCaptureReadiness = false, bool hasPgWriteStats = false, bool hasPgExtensionAvailability = false, bool hasPgLockStats = false, bool hasPgColumnStats = false, bool hasPgReplicationStats = false, bool hasPgBufferUsage = false, bool hasPgIndexBloat = false, bool hasPgPerDatabaseAttribution = false, bool hasPgWaitSampling = false, bool hasPgKernelStats = false, bool hasPgPredicateStats = false, bool hasPgPlanCapture = false, bool hasPgMajorVersion = false, bool hasPg18IoBytes = false, bool hasPgServerConfig = false, bool hasPgDeadlocks = false, bool hasPgDeadlockIdentity = false, bool hasCollectorCost = false, bool hasPgCpuUtilization = false, bool hasPlanForceActions = false, bool hasCollectionLogPhaseSplit = false, bool hasCollectionLogDrainForensics = false, bool hasCollectionLogFetchPhaseSums = false, bool hasStoreLogSelfMonitoring = false, bool hasCollectorStallProbes = false, bool hasRemediationCredentialAndActor = false, bool hasPgIndexBloatEstimate = false, bool hasPgCpuCapacityHeadroom = false, bool hasCustomAlertCore = false, bool hasMuteRuleReloadBeacon = false, bool hasBuiltinAlertPersistence = false, bool hasRetentionHoldRatioKnobs = false, bool hasDeadlockRateBandKnobs = false, bool hasOversizedPlanBacklog = false, bool hasPgAlertCountKnobs = false, bool hasFleetSweepState = false, bool hasFleetSweepCadenceKnobs = false, bool hasCollectorScheduleDatabases = false, bool hasSelfDiskWarnGbFloor = false, bool hasDeltaFamilyIntervalColumns = false, bool hasDeltaFamilyIntervalCompletion = false, bool hasPgLogEvents = false, bool hasPgLogEventMetrics = false, bool hasNotificationRoutes = false, bool hasPerfmonCounterType = false, bool hasPgNumbackendsAndSampledMs = false, bool hasTimeHonesty = false, bool hasLrqExclusionKnob = false, bool hasPgDatabaseSizeStatsAndHostMemory = false, bool hasQsCaptureModeRouteKnobToast = false, bool hasPgServerConfigDatabaseRoleOverrides = false, bool hasPostmasterStartTime = false, bool hasCheckpointsTimed = false, bool hasCollectionCaveats = false, bool hasIndexObjectStatsServerTimeIndex = false, bool hasQueryStoreIntervalLatest = false, bool hasRawChunkIntervalRungHistory = false, bool hasQueryStoreIntervalWide = false, bool hasManagedConfVerdicts = false, bool hasComposeTimeoutSixty = false, bool hasReadLatency = false, bool hasHotLivenessTouch = false, bool hasCollectionLogWatermarkAndJobHistoryIndexes = false, bool hasAgGroupId = false, bool hasCaggGroupIndexDrop = false, bool hasIntervalFirstExecIndexes = false, bool hasIntervalWideFirstExecIndex = false, bool hasQueryStoreIntervalEnd = false, bool hasCheckpointLongestSync = false, bool hasMuteRuleServerId = false, bool hasInstallId = false, bool hasInstallIdTableOid = false, bool hasCollectorRunAt = false, bool hasQueryStoreTopDaily = false, bool hasSlowReads = false, bool hasStoreStatementHistory = false, bool hasQueryStatsHourLedger = false, bool hasPasswordKey = false, bool hasPagerDutyAutoResolve = false, bool hasLegacyPinCandidates = false, bool hasPlanRegressionDaily = false, bool hasAwsRole = false, bool hasPgIoStatsHourly = false, bool hasQueryStoreIntervalWidePartitioned = false, bool hasQueryStoreIntervalLatestPartitioned = false, bool hasQueryStoreComposeStamp = false, bool hasServiceVersion = false, bool hasPlanRegressionDailyRowcount = false, bool hasProcedureStatsTotals = false, bool hasRunnableQueueSettings = false, bool hasQueryStoreWatermarks = false, bool hasClientTimeoutSettings = false, bool hasExternalAlerts = false, bool hasNotificationRouteScope = false, bool hasReadLatencyCapacityFields = false, bool hasReadCaller = false, bool hasInvestigations = false, bool hasPlanRegressionPartialDays = false, bool hasCollectionLogHourly = false, bool hasAlertResolutionDelivery = false, bool hasInvestigationAlertFacts = false, bool hasComposeStampFailures = false, bool hasStoreMetricsSizeEstimated = false, bool hasAgentLevel2Store = false, bool hasQueryTagsStore = false, bool hasPhaseCStore = false, bool hasQueryTagSightingsDatabaseKey = false, bool hasQueryStoreForcedPlans = false, bool hasServerActionSwitches = false, bool hasForceChecks = false, bool hasPseudonymKey = false, bool hasBlockingHeadEvidence = false, bool hasExcludedVolumes = false, bool hasForcedPlanVariantsAndGuides = false, bool hasAgPrimaryReplica = false, bool hasHypotheticalIndexLogin = false, bool hasCustomViewFolders = false, bool hasCustomAlertRuleExpiry = false, bool hasInheritableCollectorEnabled = false, bool hasChangeWatches = false, bool hasMuteRuleStartsAt = false, bool hasQueryStoreFetchTopN = false, bool hasAgentRouteTagGuard = false, bool hasClientTimeoutsMultiples = false, bool hasServerMachineIdentity = false, bool hasQueryStoreReplica = false, bool hasKnownPatterns = false, bool hasAgentTableIndexes = false, bool hasClientTimeoutsOnByDefault = false, bool hasQueryStoreHints = false, bool hasServerActionRecompile = false, bool hasQueryStoreSelfKeys = false, bool hasJobHistorySuccessIndex = false, bool hasQueryStoreHashHourly = false, bool hasAgentAlerts = false, bool hasDatabaseProtection = false, bool hasIdentityCapacity = false, bool hasServerAwsRds = false, bool hasServerAwsRdsWebOrExpress = false, bool hasThresholdOverrideTagScope = false, bool hasBlockingBands = false) + internal static int MapProbedSchemaVersion(bool hasConfigControlPlane, bool hasAlertDeliveryOverride, bool hasAnalysisState, bool hasAlertTuningKnobs, bool hasDefaultTraceEvents, bool hasIndexObjectStatsLatestIndex, bool hasCollectionLogHypertableOrPlainPg, bool hasJobHistory, bool hasAgentStatus, bool hasGenericWebhook, bool hasDeadlocksDatabaseName, bool hasQueryStoreReplicaRole, bool hasLongQueryCompletions, bool hasWebDashboardConfig, bool hasCustomViews, bool hasServerTags, bool hasConnectionRefireKnobs = false, bool hasAgCollectors = false, bool hasAgAlertKnobs = false, bool hasAgLatencyColumns = false, bool hasAgDisconnectRefire = false, bool hasPayloadDimensions = false, bool hasDimFloorIndexes = false, bool hasBlockingWaitThreshold = false, bool hasQueryStoreIntervalIdentity = false, bool hasPagerDutyWebhook = false, bool hasPagerDutyProxy = false, bool hasCollectorState = false, bool hasPlanCorrection = false, bool hasPvsStats = false, bool hasPvsPressureKnobs = false, bool hasDatabaseStateAlert = false, bool hasServerTagColour = false, bool hasQueryStatsHostObject = false, bool hasFindingDrillDown = false, bool hasStoreMetrics = false, bool hasPlanDimGzip = false, bool hasSelfAlertKnobs = false, bool hasJobMetricsColumns = false, bool hasJobCadenceKnob = false, bool hasBackfillSwitch = false, bool hasCollectorMemoryKnobs = false, bool hasDatabaseStateEdgeMemory = false, bool hasIncidentOccurrences = false, bool hasPlanXmlCompressionKnob = false, bool hasMonitoredServerEngine = false, bool hasPgBlockingEdges = false, bool hasQueryStorePlanMap = false, bool hasPgStatementText = false, bool hasQueryStoreText = false, bool hasPlanContentRetentionKnob = false, bool hasQueryStoreHealth = false, bool hasQueryStoreTextHash = false, bool hasComposeTimeoutKnob = false, bool hasFileGrowthAlert = false, bool hasCollectionLogFanoutRollup = false, bool hasTempDbMaxSize = false, bool hasServerEngineKind = false, bool hasPgDatabaseStats = false, bool hasPgIndexUsageStats = false, bool hasPgTableBloatStats = false, bool hasPgSessionStates = false, bool hasPgPlanCaptureReadiness = false, bool hasPgWriteStats = false, bool hasPgExtensionAvailability = false, bool hasPgLockStats = false, bool hasPgColumnStats = false, bool hasPgReplicationStats = false, bool hasPgBufferUsage = false, bool hasPgIndexBloat = false, bool hasPgPerDatabaseAttribution = false, bool hasPgWaitSampling = false, bool hasPgKernelStats = false, bool hasPgPredicateStats = false, bool hasPgPlanCapture = false, bool hasPgMajorVersion = false, bool hasPg18IoBytes = false, bool hasPgServerConfig = false, bool hasPgDeadlocks = false, bool hasPgDeadlockIdentity = false, bool hasCollectorCost = false, bool hasPgCpuUtilization = false, bool hasPlanForceActions = false, bool hasCollectionLogPhaseSplit = false, bool hasCollectionLogDrainForensics = false, bool hasCollectionLogFetchPhaseSums = false, bool hasStoreLogSelfMonitoring = false, bool hasCollectorStallProbes = false, bool hasRemediationCredentialAndActor = false, bool hasPgIndexBloatEstimate = false, bool hasPgCpuCapacityHeadroom = false, bool hasCustomAlertCore = false, bool hasMuteRuleReloadBeacon = false, bool hasBuiltinAlertPersistence = false, bool hasRetentionHoldRatioKnobs = false, bool hasDeadlockRateBandKnobs = false, bool hasOversizedPlanBacklog = false, bool hasPgAlertCountKnobs = false, bool hasFleetSweepState = false, bool hasFleetSweepCadenceKnobs = false, bool hasCollectorScheduleDatabases = false, bool hasSelfDiskWarnGbFloor = false, bool hasDeltaFamilyIntervalColumns = false, bool hasDeltaFamilyIntervalCompletion = false, bool hasPgLogEvents = false, bool hasPgLogEventMetrics = false, bool hasNotificationRoutes = false, bool hasPerfmonCounterType = false, bool hasPgNumbackendsAndSampledMs = false, bool hasTimeHonesty = false, bool hasLrqExclusionKnob = false, bool hasPgDatabaseSizeStatsAndHostMemory = false, bool hasQsCaptureModeRouteKnobToast = false, bool hasPgServerConfigDatabaseRoleOverrides = false, bool hasPostmasterStartTime = false, bool hasCheckpointsTimed = false, bool hasCollectionCaveats = false, bool hasIndexObjectStatsServerTimeIndex = false, bool hasQueryStoreIntervalLatest = false, bool hasRawChunkIntervalRungHistory = false, bool hasQueryStoreIntervalWide = false, bool hasManagedConfVerdicts = false, bool hasComposeTimeoutSixty = false, bool hasReadLatency = false, bool hasHotLivenessTouch = false, bool hasCollectionLogWatermarkAndJobHistoryIndexes = false, bool hasAgGroupId = false, bool hasCaggGroupIndexDrop = false, bool hasIntervalFirstExecIndexes = false, bool hasIntervalWideFirstExecIndex = false, bool hasQueryStoreIntervalEnd = false, bool hasCheckpointLongestSync = false, bool hasMuteRuleServerId = false, bool hasInstallId = false, bool hasInstallIdTableOid = false, bool hasCollectorRunAt = false, bool hasQueryStoreTopDaily = false, bool hasSlowReads = false, bool hasStoreStatementHistory = false, bool hasQueryStatsHourLedger = false, bool hasPasswordKey = false, bool hasPagerDutyAutoResolve = false, bool hasLegacyPinCandidates = false, bool hasPlanRegressionDaily = false, bool hasAwsRole = false, bool hasPgIoStatsHourly = false, bool hasQueryStoreIntervalWidePartitioned = false, bool hasQueryStoreIntervalLatestPartitioned = false, bool hasQueryStoreComposeStamp = false, bool hasServiceVersion = false, bool hasPlanRegressionDailyRowcount = false, bool hasProcedureStatsTotals = false, bool hasRunnableQueueSettings = false, bool hasQueryStoreWatermarks = false, bool hasClientTimeoutSettings = false, bool hasExternalAlerts = false, bool hasNotificationRouteScope = false, bool hasReadLatencyCapacityFields = false, bool hasReadCaller = false, bool hasInvestigations = false, bool hasPlanRegressionPartialDays = false, bool hasCollectionLogHourly = false, bool hasAlertResolutionDelivery = false, bool hasInvestigationAlertFacts = false, bool hasComposeStampFailures = false, bool hasStoreMetricsSizeEstimated = false, bool hasAgentLevel2Store = false, bool hasQueryTagsStore = false, bool hasPhaseCStore = false, bool hasQueryTagSightingsDatabaseKey = false, bool hasQueryStoreForcedPlans = false, bool hasServerActionSwitches = false, bool hasForceChecks = false, bool hasPseudonymKey = false, bool hasBlockingHeadEvidence = false, bool hasExcludedVolumes = false, bool hasForcedPlanVariantsAndGuides = false, bool hasAgPrimaryReplica = false, bool hasHypotheticalIndexLogin = false, bool hasCustomViewFolders = false, bool hasCustomAlertRuleExpiry = false, bool hasInheritableCollectorEnabled = false, bool hasChangeWatches = false, bool hasMuteRuleStartsAt = false, bool hasQueryStoreFetchTopN = false, bool hasAgentRouteTagGuard = false, bool hasClientTimeoutsMultiples = false, bool hasServerMachineIdentity = false, bool hasQueryStoreReplica = false, bool hasKnownPatterns = false, bool hasAgentTableIndexes = false, bool hasClientTimeoutsOnByDefault = false, bool hasQueryStoreHints = false, bool hasServerActionRecompile = false, bool hasQueryStoreSelfKeys = false, bool hasJobHistorySuccessIndex = false, bool hasQueryStoreHashHourly = false, bool hasAgentAlerts = false, bool hasDatabaseProtection = false, bool hasIdentityCapacity = false, bool hasServerAwsRds = false, bool hasServerAwsRdsWebOrExpress = false, bool hasThresholdOverrideTagScope = false, bool hasBlockingBands = false, bool hasRouteSeverities = false) { /* V71 (the PostgreSQL blocking-edges rung): a table-existence sentinel and now the newest-first arm. A collector table would ordinarily get no arm at all — see the V63-V69 note below — but the TOP @@ -1639,6 +1642,13 @@ store maps to EXACTLY StorageVersion.SchemaVersion rather than falling through t /* V226 (#6893): the AWS RDS Web or Express fact on the server registry. Column-existence sentinel, newest-first, and the TOP rung, so a fully-migrated store maps to EXACTLY StorageVersion.SchemaVersion rather than falling through to 225 and showing a spurious upgrade banner on a store that is current. Named only in the probe line, not this prose, per the V71 finding. */ + /* V229 (#6928): the severity filter on the notification routes. Column-existence sentinel, newest-first, and the TOP rung, so a + fully-migrated store maps to EXACTLY StorageVersion.SchemaVersion rather than falling through to 226 and showing a spurious upgrade + banner on a store that is current. Named only in the probe line, not this prose, per the V71 finding. */ + if (hasRouteSeverities) + { + return 229; + } /* V228 (#6927): the settable blocking band columns on the alert settings row. Column-existence sentinel, newest-first, and the TOP rung, so a fully-migrated store maps to EXACTLY StorageVersion.SchemaVersion rather than falling through to 227 and showing a spurious upgrade banner on a store that is current. Named only in the probe line, not this prose, per the V71 finding. */ diff --git a/Darling/tools/provision-roles.sql b/Darling/tools/provision-roles.sql index 934dd85c4e..45423d5c84 100644 --- a/Darling/tools/provision-roles.sql +++ b/Darling/tools/provision-roles.sql @@ -599,6 +599,15 @@ $v184$; GRANT SELECT (send_resolutions) ON config.config_notification_routes TO viewer; $v187$; END IF; + -- V229 (#6928): a route's severity filter. Non-secret. Granted on its own column inside the routes table's guard, so a + -- store below V229 still commits. + IF EXISTS (SELECT 1 FROM pg_catalog.pg_attribute + WHERE attrelid = 'config.config_notification_routes'::regclass + AND attname = 'severities' AND NOT attisdropped) THEN + EXECUTE $v229$ +GRANT SELECT (severities) ON config.config_notification_routes TO viewer; +$v229$; + END IF; END IF; END $$; -- V187 (#5825): the four per-channel "send resolutions" flags. Non-secret (behaviour toggles, like @@ -715,6 +724,15 @@ $v184$; GRANT SELECT (send_resolutions) ON config.config_notification_routes TO mcp; $v187$; END IF; + -- V229 (#6928): a route's severity filter. Non-secret. Granted on its own column inside the routes table's guard, so a + -- store below V229 still commits. + IF EXISTS (SELECT 1 FROM pg_catalog.pg_attribute + WHERE attrelid = 'config.config_notification_routes'::regclass + AND attname = 'severities' AND NOT attisdropped) THEN + EXECUTE $v229$ +GRANT SELECT (severities) ON config.config_notification_routes TO mcp; +$v229$; + END IF; END IF; END $$; -- V187 (#5825): the four per-channel "send resolutions" flags. Non-secret (behaviour toggles, like @@ -1533,13 +1551,16 @@ GRANT EXECUTE ON FUNCTION config.request_analyze_now(integer, text) TO viewer; -- (the address list is plain text). All three re-read the row FOR UPDATE and refuse an agent route inside the transaction, -- unless the actor the service passes is an operator-class token (#6542 part 2). Granted to mcp only. BEGIN; +-- #6928 (V229): p_severities joins the argument list, so the six-argument function is dropped and stays uncallable. NULL is both. +DROP FUNCTION IF EXISTS config.create_notification_route(text, boolean, integer, integer, boolean, integer); CREATE OR REPLACE FUNCTION config.create_notification_route( p_metric_match text, p_enabled boolean, p_scope_server_id integer, p_scope_tag_id integer, p_send_resolutions boolean, - p_fallback_route_id integer) + p_fallback_route_id integer, + p_severities text DEFAULT NULL) RETURNS integer LANGUAGE plpgsql SECURITY DEFINER @@ -1547,8 +1568,12 @@ SET search_path = pg_catalog, pg_temp AS $fn$ DECLARE v_match text := btrim(COALESCE(p_metric_match, '')); + v_severities text := lower(btrim(COALESCE(p_severities, 'both'))); v_id integer; BEGIN + IF v_severities NOT IN ('both', 'critical', 'warning') THEN + RAISE EXCEPTION 'severities is critical, warning or both.' USING ERRCODE = 'DL400'; + END IF; IF v_match = '' THEN RAISE EXCEPTION 'Choose a family or enter an exact metric name.' USING ERRCODE = 'DL400'; END IF; @@ -1574,17 +1599,20 @@ BEGIN -- goes where it went before (the next matching route, else the Settings channels), and no caller of this function can name a URL, a key or an address list. -- The agent columns keep their defaults too: this function never makes an agent route. INSERT INTO config.config_notification_routes - (metric_match, enabled, scope_server_id, scope_tag_id, send_resolutions, fallback_route_id, modified_at) + (metric_match, enabled, scope_server_id, scope_tag_id, send_resolutions, fallback_route_id, severities, modified_at) VALUES - (v_match, COALESCE(p_enabled, true), p_scope_server_id, p_scope_tag_id, p_send_resolutions, p_fallback_route_id, + (v_match, COALESCE(p_enabled, true), p_scope_server_id, p_scope_tag_id, p_send_resolutions, p_fallback_route_id, v_severities, (now() AT TIME ZONE 'UTC')) RETURNING route_id INTO v_id; RETURN v_id; END $fn$; -REVOKE ALL ON FUNCTION config.create_notification_route(text, boolean, integer, integer, boolean, integer) FROM PUBLIC; +REVOKE ALL ON FUNCTION config.create_notification_route(text, boolean, integer, integer, boolean, integer, text) FROM PUBLIC; -- #6542 part 2: the actor is an argument now and the before row comes back, so the old signature is dropped and stays uncallable. DROP FUNCTION IF EXISTS config.update_notification_route(integer, text[], text, boolean, integer, integer, boolean, integer); +-- #6928 (V229): p_severities joins the argument list, so the nine-argument function is dropped and stays uncallable. It is read only when +-- p_columns names severities. +DROP FUNCTION IF EXISTS config.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer); CREATE OR REPLACE FUNCTION config.update_notification_route( p_route_id integer, p_actor text, @@ -1594,7 +1622,8 @@ CREATE OR REPLACE FUNCTION config.update_notification_route( p_scope_server_id integer, p_scope_tag_id integer, p_send_resolutions boolean, - p_fallback_route_id integer) + p_fallback_route_id integer, + p_severities text DEFAULT NULL) RETURNS jsonb LANGUAGE plpgsql SECURITY DEFINER @@ -1611,11 +1640,12 @@ DECLARE v_tag integer; v_resolutions boolean; v_fallback integer; + v_severities text; BEGIN IF p_columns IS NULL OR cardinality(p_columns) = 0 OR EXISTS (SELECT 1 FROM unnest(p_columns) AS c(name) - WHERE c.name IS NULL OR c.name NOT IN ('metric_match', 'enabled', 'scope', 'send_resolutions', 'fallback_route_id')) THEN - RAISE EXCEPTION 'The columns to change are metric_match, enabled, scope, send_resolutions and fallback_route_id.' USING ERRCODE = 'DL400'; + WHERE c.name IS NULL OR c.name NOT IN ('metric_match', 'enabled', 'scope', 'send_resolutions', 'fallback_route_id', 'severities')) THEN + RAISE EXCEPTION 'The columns to change are metric_match, enabled, scope, send_resolutions, fallback_route_id and severities.' USING ERRCODE = 'DL400'; END IF; -- The row is read again here, under its lock, so the agent-route refusal and every check below are on what is stored now -- and not on what the caller read a moment ago. @@ -1629,7 +1659,7 @@ BEGIN -- What a route matches and which servers it covers decide which servers an agent route covers, so a route with an -- explicit agent setting (Off included) keeps both in the Viewer. A route's own destinations do not stop a change -- to what it matches: the destinations are set over MCP too. (#6231) - IF ('metric_match' = ANY (p_columns) OR 'scope' = ANY (p_columns)) AND v_row.agent_level IS NOT NULL AND NOT v_operator THEN + IF ('metric_match' = ANY (p_columns) OR 'scope' = ANY (p_columns) OR 'severities' = ANY (p_columns)) AND v_row.agent_level IS NOT NULL AND NOT v_operator THEN RAISE EXCEPTION 'That route has an agent setting (Off), so what it matches and which servers it covers are changed in the Viewer or on the web dashboard, not over MCP. Turning it on, send_resolutions and fallback_route_id can still be changed here.' USING ERRCODE = 'DL403'; END IF; -- An Off route keeps servers away from a broader agent route, so turning it off widens agent coverage: that is done in the Viewer @@ -1643,6 +1673,10 @@ BEGIN v_tag := CASE WHEN 'scope' = ANY (p_columns) THEN p_scope_tag_id ELSE v_row.scope_tag_id END; v_resolutions := CASE WHEN 'send_resolutions' = ANY (p_columns) THEN p_send_resolutions ELSE v_row.send_resolutions END; v_fallback := CASE WHEN 'fallback_route_id' = ANY (p_columns) THEN p_fallback_route_id ELSE v_row.fallback_route_id END; + v_severities := CASE WHEN 'severities' = ANY (p_columns) THEN lower(btrim(COALESCE(p_severities, ''))) ELSE v_row.severities END; + IF v_severities NOT IN ('both', 'critical', 'warning') THEN + RAISE EXCEPTION 'severities is critical, warning or both.' USING ERRCODE = 'DL400'; + END IF; IF v_match = '' THEN RAISE EXCEPTION 'Choose a family or enter an exact metric name.' USING ERRCODE = 'DL400'; END IF; @@ -1676,6 +1710,7 @@ BEGIN scope_tag_id = v_tag, send_resolutions = v_resolutions, fallback_route_id = v_fallback, + severities = v_severities, modified_at = (now() AT TIME ZONE 'UTC') WHERE route_id = p_route_id; -- The row as it was before this change, in the journal's words: set_notification_route_agent journals it as the before of a @@ -1683,10 +1718,11 @@ BEGIN RETURN jsonb_build_object( 'metric_match', btrim(v_row.metric_match), 'scope_server_id', v_row.scope_server_id, 'scope_tag_id', v_row.scope_tag_id, 'agent_level', v_row.agent_level, 'agent_max_per_hour', v_row.agent_max_per_hour, 'agent_max_per_day', v_row.agent_max_per_day, - 'agent_lease_minutes', v_row.agent_lease_minutes, 'agent_expiry_minutes', v_row.agent_expiry_minutes, 'enabled', v_row.enabled); + 'agent_lease_minutes', v_row.agent_lease_minutes, 'agent_expiry_minutes', v_row.agent_expiry_minutes, 'enabled', v_row.enabled, + 'severities', v_row.severities); END $fn$; -REVOKE ALL ON FUNCTION config.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer) FROM PUBLIC; +REVOKE ALL ON FUNCTION config.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer, text) FROM PUBLIC; -- #6542: a named operator token sets a route's agent level, its four limits and its fallback over MCP through this function, and -- the change is journaled as route_agent_changed under the token's name (operator:), in the same transaction. Levels 0 to 3 are -- set here (MCP is full admin). A level 3 server action still needs that server's switch (config_server_actions) and its remediation login. @@ -1768,6 +1804,10 @@ BEGIN IF (p_before ->> 'metric_match') IS DISTINCT FROM btrim(v_row.metric_match) THEN v_changed := array_append(v_changed, 'metric_match'); END IF; IF (p_before ->> 'scope_server_id')::integer IS DISTINCT FROM v_row.scope_server_id THEN v_changed := array_append(v_changed, 'scope_server_id'); END IF; IF (p_before ->> 'scope_tag_id')::integer IS DISTINCT FROM v_row.scope_tag_id THEN v_changed := array_append(v_changed, 'scope_tag_id'); END IF; + -- #6928: the severity filter moves alerts between agent routes, so it is named like the match and the scope. Only a route with an agent setting. + IF (p_before ->> 'severities') IS DISTINCT FROM v_row.severities AND ((p_before ->> 'agent_level') IS NOT NULL OR v_level IS NOT NULL) THEN + v_changed := array_append(v_changed, 'severities'); + END IF; END IF; IF v_level IS DISTINCT FROM v_row.agent_level THEN v_changed := array_append(v_changed, 'agent_level'); END IF; IF v_hour IS DISTINCT FROM v_row.agent_max_per_hour THEN v_changed := array_append(v_changed, 'agent_max_per_hour'); END IF; @@ -1793,11 +1833,13 @@ BEGIN v_before := CASE WHEN COALESCE(p_created, false) THEN NULL WHEN p_before IS NOT NULL THEN p_before ELSE jsonb_build_object( 'metric_match', btrim(v_row.metric_match), 'scope_server_id', v_row.scope_server_id, 'scope_tag_id', v_row.scope_tag_id, 'agent_level', v_row.agent_level, 'agent_max_per_hour', v_row.agent_max_per_hour, 'agent_max_per_day', v_row.agent_max_per_day, - 'agent_lease_minutes', v_row.agent_lease_minutes, 'agent_expiry_minutes', v_row.agent_expiry_minutes, 'enabled', v_row.enabled) END; + 'agent_lease_minutes', v_row.agent_lease_minutes, 'agent_expiry_minutes', v_row.agent_expiry_minutes, 'enabled', v_row.enabled, + 'severities', v_row.severities) END; v_after := jsonb_build_object( 'metric_match', btrim(v_row.metric_match), 'scope_server_id', v_row.scope_server_id, 'scope_tag_id', v_row.scope_tag_id, 'agent_level', v_level, 'agent_max_per_hour', v_hour, 'agent_max_per_day', v_day, - 'agent_lease_minutes', v_lease, 'agent_expiry_minutes', v_expiry, 'enabled', v_row.enabled); + 'agent_lease_minutes', v_lease, 'agent_expiry_minutes', v_expiry, 'enabled', v_row.enabled, + 'severities', v_row.severities); INSERT INTO config.investigation_journal (event_time, request_id, route_id, server_id, event, actor, reason, detail) VALUES (now(), NULL, p_route_id, v_row.scope_server_id, 'route_agent_changed', p_actor, CASE WHEN COALESCE(p_created, false) THEN 'created route' ELSE 'changed ' || array_to_string(v_changed, ', ') END, @@ -1896,8 +1938,8 @@ BEGIN END $fn$; REVOKE ALL ON FUNCTION config.set_notification_route_destinations(integer, text, text[], text, text, text, text, text) FROM PUBLIC; -GRANT EXECUTE ON FUNCTION config.create_notification_route(text, boolean, integer, integer, boolean, integer) TO mcp; -GRANT EXECUTE ON FUNCTION config.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer) TO mcp; +GRANT EXECUTE ON FUNCTION config.create_notification_route(text, boolean, integer, integer, boolean, integer, text) TO mcp; +GRANT EXECUTE ON FUNCTION config.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer, text) TO mcp; GRANT EXECUTE ON FUNCTION config.set_notification_route_destinations(integer, text, text[], text, text, text, text, text) TO mcp; GRANT EXECUTE ON FUNCTION config.set_notification_route_agent(integer, text, text[], integer, integer, integer, integer, integer, integer, boolean, jsonb) TO mcp; -- #6394: the MCP generate_webhook_signing_key and remove_webhook_signing_key tools. mcp holds no UPDATE on a signing key column of config.config_notification @@ -3191,7 +3233,7 @@ BEGIN -- statements set and nothing more. BRING-YOUR-OWN STORES: re-run this script to add them. GRANT INSERT (request_id, route_id, server_id, server_name, family, first_metric, story_path_hash, first_alert_at, last_alert_at, window_until, opened_at, expires_at, lease_minutes, autonomy_level, alert_count, practice_of) ON config.investigation_requests TO mcp; - GRANT INSERT (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json) + GRANT INSERT (request_id, alert_time, server_id, metric_name, dedup_key, current_value, threshold_value, muted, detail_text, context_json, severity) ON config.investigation_request_alerts TO mcp; GRANT INSERT (request_id, verdict, corrected_verdict, missed_step, comment, author) ON config.investigation_feedback TO mcp; GRANT UPDATE (verdict, corrected_verdict, missed_step, comment, author, created_at) ON config.investigation_feedback TO mcp; @@ -3407,8 +3449,8 @@ BEGIN 'config.request_snapshot_now(integer, text, text)', 'config.request_active_queries_live(integer, text)', 'config.request_analyze_now(integer, text)', - 'config.create_notification_route(text, boolean, integer, integer, boolean, integer)', - 'config.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer)', + 'config.create_notification_route(text, boolean, integer, integer, boolean, integer, text)', + 'config.update_notification_route(integer, text, text[], text, boolean, integer, integer, boolean, integer, text)', 'config.set_notification_route_agent(integer, text, text[], integer, integer, integer, integer, integer, integer, boolean, jsonb)', 'config.set_notification_route_destinations(integer, text, text[], text, text, text, text, text)', 'config.request_mcp_command(text, integer, integer, text)', diff --git a/Lite.Tests/NotificationRoutesMcpTests.cs b/Lite.Tests/NotificationRoutesMcpTests.cs index e6b59e6dad..66ebcdf57b 100644 --- a/Lite.Tests/NotificationRoutesMcpTests.cs +++ b/Lite.Tests/NotificationRoutesMcpTests.cs @@ -453,6 +453,8 @@ delegates to it (DarlingRoutePayloadTests pins that), so this is still the one b /* #6036: the fallback route (D8 of #5632 Phase C) is Darling-only; Lite has no request events. */ "fallback_route_id", "fallback_route_name", + /* #6928: route severity is Darling-only. */ + "severities", }; Assert.All(darlingOnly, f => Assert.Contains(f, darling)); Assert.Equal(darling.Except(darlingOnly).OrderBy(n => n, StringComparer.Ordinal), lite.OrderBy(n => n, StringComparer.Ordinal)); diff --git a/PerformanceMonitor.Notifications/AlertSeverity.cs b/PerformanceMonitor.Notifications/AlertSeverity.cs index d0ca0bda9f..2b2084462e 100644 --- a/PerformanceMonitor.Notifications/AlertSeverity.cs +++ b/PerformanceMonitor.Notifications/AlertSeverity.cs @@ -28,6 +28,11 @@ public enum AlertSeverityLevel /// internal static class AlertSeverity { + /// The tier a notification route's severity filter sees for an alert (#6928): the tier graded at the fire site + /// when it carries one, else the metric's own badge (red is Critical, every other badge is Warning). + public static AlertSeverityLevel RouteLevelOf(string metricName, AlertSeverityLevel? overrideLevel) => + overrideLevel ?? (ForMetric(metricName).HexColor == "#DC2626" ? AlertSeverityLevel.Critical : AlertSeverityLevel.Warning); + /// /// When non-null, forces the tier regardless of — for metrics /// graded at runtime (#1136). null uses the per-metric map below. diff --git a/PerformanceMonitor.Notifications/EmailSendCore.cs b/PerformanceMonitor.Notifications/EmailSendCore.cs index 03d5e92815..04239cb977 100644 --- a/PerformanceMonitor.Notifications/EmailSendCore.cs +++ b/PerformanceMonitor.Notifications/EmailSendCore.cs @@ -209,7 +209,8 @@ reuses DerivePagerDutyDedupKey so the link correlates with the SAME incident Pag and a recipient list without them is not a configuration. #4751: `smtpConfigured` above only asks that SOME enabled route names recipients, so the fall-through to the parent's list can now be blank — a firing no route covers, with no default list, has nobody to send to. */ - emailRoute = NotificationRouter.Resolve(metricName, _settings.NotificationRoutes, _settings, NotificationRouter.ParseServerId(serverId)); + emailRoute = NotificationRouter.Resolve(metricName, _settings.NotificationRoutes, _settings, NotificationRouter.ParseServerId(serverId), + AlertSeverity.RouteLevelOf(metricName, context?.SeverityOverride)); var recipients = emailRoute.Email.Destination ?? _settings.SmtpRecipients; if (string.IsNullOrWhiteSpace(recipients)) diff --git a/PerformanceMonitor.Notifications/InvestigationDispatchCore.cs b/PerformanceMonitor.Notifications/InvestigationDispatchCore.cs index 2b402d5a4a..64140ac520 100644 --- a/PerformanceMonitor.Notifications/InvestigationDispatchCore.cs +++ b/PerformanceMonitor.Notifications/InvestigationDispatchCore.cs @@ -131,9 +131,9 @@ public static InvestigationAlertFacts Facts( /// #5821: whether the generic webhook this alert posts to has {{investigation_json}} in its body template, resolved /// through the same router the send uses. Only then is the request read back: the token is in no shipped template. /// - public bool GenericPostCarriesToken(string serverId, string metricName) => + public bool GenericPostCarriesToken(string serverId, string metricName, AlertSeverityLevel? severity = null) => WebhookAlertService.TemplateCarriesInvestigationToken(_settings.GenericWebhookBodyTemplate) - && NotificationRouter.Resolve(metricName, _settings.NotificationRoutes, _settings, NotificationRouter.ParseServerId(serverId)) + && NotificationRouter.Resolve(metricName, _settings.NotificationRoutes, _settings, NotificationRouter.ParseServerId(serverId), severity) .Generic.Destination is not null; /// @@ -165,7 +165,8 @@ public bool GenericPostCarriesToken(string serverId, string metricName) => } var routeServerId = NotificationRouter.ParseServerId(serverId); - var route = NotificationRouter.ResolveAgent(metricName, _settings.NotificationRoutes, routeServerId); + var route = NotificationRouter.ResolveAgent(metricName, _settings.NotificationRoutes, routeServerId, + AlertSeverity.RouteLevelOf(metricName, context?.SeverityOverride)); if (route is null) { return null; @@ -303,7 +304,7 @@ ruling Q3 (#3653) is "label, not discount". */ try { (InvestigationRequestRow Row, string? Brief)? read = null; - if (readRequest is not null && GenericPostCarriesToken(serverId, metricName)) + if (readRequest is not null && GenericPostCarriesToken(serverId, metricName, AlertSeverity.RouteLevelOf(metricName, context?.SeverityOverride))) { try { diff --git a/PerformanceMonitor.Notifications/NotificationRoute.cs b/PerformanceMonitor.Notifications/NotificationRoute.cs index 3cecbaef73..dbb796035c 100644 --- a/PerformanceMonitor.Notifications/NotificationRoute.cs +++ b/PerformanceMonitor.Notifications/NotificationRoute.cs @@ -166,6 +166,14 @@ public bool IsOffFor(string channel) => /// each channel's own setting, true sends it and false does not, whatever the channel says. public bool? SendResolutions { get; init; } + /// The severity filter (severities, #6928): (the default, every route + /// before the filter), or . + /// A route whose filter excludes an alert's severity does not match it, and matching goes on to the next route. + public string Severities { get; init; } = NotificationRouteKinds.SeveritiesBoth; + + /// Whether this route's severity filter takes an alert of (#6928). Null takes every route. + public bool AcceptsSeverity(AlertSeverityLevel? severity) => NotificationRouteKinds.SeveritiesAccept(Severities, severity); + /// Whether this route sets the agent channel at all (#5632): either level, as opposed to inheriting. public bool HasAgentLevel => AgentLevel.HasValue; diff --git a/PerformanceMonitor.Notifications/NotificationRouteKinds.cs b/PerformanceMonitor.Notifications/NotificationRouteKinds.cs index 91e72e79e4..d65cd772d8 100644 --- a/PerformanceMonitor.Notifications/NotificationRouteKinds.cs +++ b/PerformanceMonitor.Notifications/NotificationRouteKinds.cs @@ -48,4 +48,33 @@ public static string MatchKind(string? metricMatch) => public static string? FamilyOf(string? metricMatch) => IsWildcard(metricMatch) ? null : AlertFamily.NormalizeFamily(metricMatch) ?? AlertFamily.Of(metricMatch ?? string.Empty); + + /// severities value (#6928): a route that takes Warning and Critical alerts alike. The default, and what every route was before the filter existed. + public const string SeveritiesBoth = "both"; + + /// severities value (#6928): a route that takes Critical alerts only. + public const string SeveritiesCritical = "critical"; + + /// severities value (#6928): a route that takes Warning alerts only. Every alert that is not Critical counts as Warning here. + public const string SeveritiesWarning = "warning"; + + /// The stored word for a route's severity filter: both, critical or warning. Blank, null or + /// anything unknown reads as both, so a value this build does not know widens to today's behaviour and never hides a route. + public static string NormalizeSeverities(string? value) => + value?.Trim().ToLowerInvariant() switch + { + SeveritiesCritical => SeveritiesCritical, + SeveritiesWarning => SeveritiesWarning, + _ => SeveritiesBoth, + }; + + /// Whether a route with severity filter takes an alert of . + /// A null severity (a caller that does not know the alert's tier) is taken by every route. + public static bool SeveritiesAccept(string? severities, AlertSeverityLevel? severity) => + severity is null || NormalizeSeverities(severities) switch + { + SeveritiesCritical => severity == AlertSeverityLevel.Critical, + SeveritiesWarning => severity == AlertSeverityLevel.Warning, + _ => true, + }; } diff --git a/PerformanceMonitor.Notifications/NotificationRouter.cs b/PerformanceMonitor.Notifications/NotificationRouter.cs index e8252783b7..54f49741de 100644 --- a/PerformanceMonitor.Notifications/NotificationRouter.cs +++ b/PerformanceMonitor.Notifications/NotificationRouter.cs @@ -199,12 +199,16 @@ public static class NotificationRouter /// (#5750) is the product's server id for the firing - Lite's hash id, Darling's /// registry id - or null for a firing with no server (a fleet-level preview): only fleet-wide routes can match /// then. Optional so a caller with no server keeps compiling. + /// (#6928) is the alert's tier: a route whose severity filter excludes it is dropped before + /// the order below runs, so matching goes on to the next route and ends at the defaults. Null (a caller that does not + /// know the tier) is taken by every route, which is today's behaviour. /// public static NotificationRouteDecision Resolve( string metricName, IReadOnlyList? routes, IAlertSettings defaults, - int? serverId = null) + int? serverId = null, + AlertSeverityLevel? severity = null) { if (metricName is null) { @@ -217,7 +221,7 @@ public static NotificationRouteDecision Resolve( } var family = AlertFamily.Of(metricName); - var ordered = OrderedCandidates(metricName, routes, serverId); + var ordered = OrderedCandidates(metricName, routes, serverId, severity); /* The winning route for the ledger's headline: the most specific one that matched at all, whether or not it ended up supplying every channel. Per-channel provenance sits on each destination. */ @@ -261,14 +265,15 @@ or not it ended up supplying every channel. Per-channel provenance sits on each public static NotificationRoute? ResolveAgent( string metricName, IReadOnlyList? routes, - int? serverId = null) + int? serverId = null, + AlertSeverityLevel? severity = null) { if (metricName is null) { throw new ArgumentNullException(nameof(metricName)); } - foreach (var (route, _, _, _) in OrderedCandidates(metricName, routes, serverId)) + foreach (var (route, _, _, _) in OrderedCandidates(metricName, routes, serverId, severity)) { if (route.AgentLevel is { } level) { @@ -294,14 +299,15 @@ or not it ended up supplying every channel. Per-channel provenance sits on each private static List<(NotificationRoute Route, RouteScopeTier Tier, int Level, RouteSource Source)> OrderedCandidates( string metricName, IReadOnlyList? routes, - int? serverId) + int? serverId, + AlertSeverityLevel? severity) { var candidates = new List<(NotificationRoute Route, RouteScopeTier Tier, int Level, RouteSource Source)>(); if (routes is not null) { foreach (var route in routes) { - if (route is null || !route.CanMatch || !route.AppliesTo(serverId)) + if (route is null || !route.CanMatch || !route.AppliesTo(serverId) || !route.AcceptsSeverity(severity)) { continue; } diff --git a/PerformanceMonitor.Notifications/WebhookAlertService.cs b/PerformanceMonitor.Notifications/WebhookAlertService.cs index 5ed0ecb5d0..1fb177c9a1 100644 --- a/PerformanceMonitor.Notifications/WebhookAlertService.cs +++ b/PerformanceMonitor.Notifications/WebhookAlertService.cs @@ -240,7 +240,8 @@ the alert log on first touch per key (#1145), unless the store is null (no seedi comes from this one snapshot. With an adapter that does not override SnapshotForDelivery (Lite) it is the live settings. */ var delivery = _settings.SnapshotForDelivery(); - var route = NotificationRouter.Resolve(metricName, delivery.NotificationRoutes, delivery, NotificationRouter.ParseServerId(serverId)); + var route = NotificationRouter.Resolve(metricName, delivery.NotificationRoutes, delivery, NotificationRouter.ParseServerId(serverId), + AlertSeverity.RouteLevelOf(metricName, context?.SeverityOverride)); /* #5469: the PagerDuty auto-resolve behaviour reaches the cooldown only when PagerDuty will actually be sent this alert: the flag on AND a destination for this metric. The flag is read @@ -2903,9 +2904,20 @@ private static bool IsWellFormedJson(string payload, out string? error) /// is off or PagerDuty is not routed for the metric, or /// otherwise. Never throws except for the caller's own cancellation. /// + public Task TrySendPagerDutyResolveAsync( + string firingMetric, string serverName, string serverId, string? detailText, + AlertSeverityLevel? firedSeverity = null, CancellationToken cancellationToken = default) => + TrySendPagerDutyResolveAsync(firingMetric, serverName, serverId, detailText, new[] { firedSeverity }, cancellationToken); + + /// + /// The same close for an episode that fired at more than one tier (#6928). The route is resolved at each of + /// (a null entry resolves with no tier, as before the filter), and a resolve goes to every + /// distinct routing key that gives, so a Warning-only route's incident is closed after the alert climbed to Critical and a + /// Critical-only route took the later fires. Delivered when a resolve posted and none failed, Failed when any failed. + /// public async Task TrySendPagerDutyResolveAsync( string firingMetric, string serverName, string serverId, string? detailText, - CancellationToken cancellationToken = default) + IReadOnlyCollection firedSeverities, CancellationToken cancellationToken = default) { try { @@ -2915,26 +2927,44 @@ public async Task TrySendPagerDutyResolveAsync( return AlertChannelOutcome.NotAttempted; } - var route = NotificationRouter.Resolve(firingMetric, delivery.NotificationRoutes, delivery, NotificationRouter.ParseServerId(serverId)); - if (route.PagerDuty.Destination is not { } pagerDutyKey) + /* #6928: routed at each severity the fire went out at, so the close lands on the key each fire's route carried, not on a + route the severity filter would send another tier to. */ + var keys = new List(); + foreach (var severity in firedSeverities.Count == 0 ? new AlertSeverityLevel?[] { null } : firedSeverities) + { + var route = NotificationRouter.Resolve(firingMetric, delivery.NotificationRoutes, delivery, NotificationRouter.ParseServerId(serverId), severity); + if (route.PagerDuty.Destination is { } pagerDutyKey && !keys.Contains(pagerDutyKey, StringComparer.Ordinal)) + { + keys.Add(pagerDutyKey); + } + } + + if (keys.Count == 0) { return AlertChannelOutcome.NotAttempted; } - var error = await TrySendPagerDutyAlertAsync( - delivery, autoResolve: true, pagerDutyKey, firingMetric, serverName, "resolved", "", serverId, - context: null, triageUrl: null, detailText, _utcNow(), displayName: null, - resolveIncident: true, cancellationToken: cancellationToken); - if (error is null) + var failed = false; + foreach (var pagerDutyKey in keys) { - /* As the built-in pairs do after a delivered close: the cooldown entry the fire stamped describes an - incident PagerDuty just closed, so the next fire of the same key must open a new one instead of being - throttled against it. Only a resolve that posted clears it (#5654). */ - _cooldown.ClearMetric(serverId, firingMetric, null); - return AlertChannelOutcome.Delivered; + var error = await TrySendPagerDutyAlertAsync( + delivery, autoResolve: true, pagerDutyKey, firingMetric, serverName, "resolved", "", serverId, + context: null, triageUrl: null, detailText, _utcNow(), displayName: null, + resolveIncident: true, cancellationToken: cancellationToken); + if (error is null) + { + /* As the built-in pairs do after a delivered close: the cooldown entry the fire stamped describes an + incident PagerDuty just closed, so the next fire of the same key must open a new one instead of being + throttled against it. Only a resolve that posted clears it (#5654). */ + _cooldown.ClearMetric(serverId, firingMetric, null); + } + else + { + failed = true; + } } - return AlertChannelOutcome.Failed; + return failed ? AlertChannelOutcome.Failed : AlertChannelOutcome.Delivered; } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { diff --git a/docs/darling/admin.md b/docs/darling/admin.md index 1668dc998b..a45e2a54cb 100644 --- a/docs/darling/admin.md +++ b/docs/darling/admin.md @@ -190,6 +190,7 @@ An older build ignores what those migrations added, so it can read some newer da - **V220**: an older build does not know the Job History statistics index ([#6798](https://github.com/erikdarlingdata/PerformanceMonitor/issues/6798)). Rolling back leaves `idx_job_history_job_success` on `collect.job_history` in place. The older build's Job History read can still use it, and the collector keeps it current on every insert. Nothing needs doing before you roll back. Leave the index in place: the viewer recognizes a V220 store by it, so a store whose index was dropped answers 219 to the viewer while its schema version says 220, the viewer refuses it, and the migrator does not build the index again. - **V221**: an older build does not know the Query Store hash-hourly table ([#6800](https://github.com/erikdarlingdata/PerformanceMonitor/issues/6800)). Rolling back leaves `collect.query_store_hash_hourly`, the day tables `collect.query_store_hash_daily`, `collect.query_store_hash_daily_built` and `collect.query_store_hash_daily_failures`, and the `hash_built_seq` column of `collect.query_store_compose_stamp_built` in place. The older build neither reads nor fills them, so the tables stop growing and their hours and days go stale. Nothing to do before rolling back. If you roll forward again, the next hourly passes rebuild the stale hours, newest first, and the next cleanup removes the hash-hourly rows of any hour the older build retired without deleting them (it deletes by the retention floor, not by the hours it just retired) - **V227**: an older build does not know a threshold override held for a server tag ([#6926](https://github.com/erikdarlingdata/PerformanceMonitor/issues/6926)). Rolling back leaves the `scope_tag_id` column on `config.config_threshold_overrides` (and `server_id` nullable), and any tag override rows stay. An older build reads the whole overrides table when it loads, and a row with no server stops that read, so it applies **no** threshold override at all (server or database ones included) until the tag rows are gone. Before rolling back past V227, remove every tag override (the Viewer's Threshold Overrides window, the web Admin tab, or `set_alert_threshold_override` with `clear` and a tag), or run `DELETE FROM config.config_threshold_overrides WHERE server_id IS NULL;` as the owner. +- **V229**: an older build does not know the severity filter on a notification route ([#6928](https://github.com/erikdarlingdata/PerformanceMonitor/issues/6928)). Rolling back leaves `config.config_notification_routes.severities` in place. The older build ignores it. A route set to `critical` or `warning` takes alerts of both severities again. A Warning alert then reaches the channels of a route that was meant for Critical alerts only. Before you roll back, run `get_notification_routes` and look at every route whose `severities` is not `both`. If its channels must not see the other severity, change the route's destinations or disable it. A route set to `both` is unchanged. If you roll forward again, the filters you set are still there. ### Upgrading from a build that predates the script diff --git a/docs/darling/alerts-and-notifications.md b/docs/darling/alerts-and-notifications.md index 7e4e11ef67..087c30b7eb 100644 --- a/docs/darling/alerts-and-notifications.md +++ b/docs/darling/alerts-and-notifications.md @@ -277,6 +277,19 @@ A bring-your-own store that was provisioned before 3.12 needs the same step for Destinations are bearer secrets, so the `viewer` and `mcp` roles never read a webhook URL or key: the table's generated `configured_channels` column says *which* channels a route sets, and the MCP surface administers routes in full: `create_notification_route` makes a route (with no destination of its own, each channel goes where it went before, to the next matching route and then to the channels configured in Settings), `update_notification_route` changes a route's match, scope, enabled flag, resolution override and fallback, and both set a route's own destinations (a Teams, Slack or generic webhook URL, a PagerDuty routing key, email recipients; `-` removes one). The service seals each webhook value for the route's own row, in the transaction that creates or edits the route, exactly as the Viewer does, so a value copied to another route does not open there; an answer never shows a webhook URL or key: it says whether one is saved (with the host for Teams and Slack only), it shows the recipient list in full, and the undo of a destination change carries the earlier recipient list and says to pass an earlier webhook URL or key again. `set_notification_route_enabled`, `set_notification_route_fallback` and `delete_notification_route` change the rest. A route with an explicit agent setting (Off included) keeps its match and scope in the Viewer, and an agent route is changed only in the Viewer. The `mcp` role has no INSERT on the table, no write on the destination columns and no read of the webhook and key columns (it reads the recipient list and `configured_channels`); the route tools write through `config.create_notification_route`, `config.update_notification_route` and `config.set_notification_route_destinations`, functions that store only a sealed value or an empty one for a webhook and refuse an agent route. `config.set_notification_route_destinations` hands back only whether a webhook or key was saved before, for the columns the call names, and never a stored value; for a changed webhook value it accepts only the sealed form (`sealed:v1:`, the key id and base64, at most 16384 characters), and for a changed recipient list it refuses control characters, angle brackets, double quotes and more than 50 entries. Every writer (the Viewer's settings and route windows, the MCP route tools) refuses a Teams, Slack or generic webhook URL at save unless it is an absolute http or https URL with a host and no user name or password and whose host is not a link-local or cloud instance address written out, and refuses an email recipient list that is not comma separated plain addresses (the function the `mcp` role writes through checks only the shapes listed above, and the service checks the rest before it calls it), and a direct webhook post never connects to a link-local address (169.254.0.0/16, fe80::/10), whether it came from the URL, a DNS answer or a redirect (a channel that posts through a proxy is only checked against the URL's own address at save, because Darling cannot see what the proxy connects to); loopback and private addresses stay allowed for internal receivers. Routes hot-reload through the same `config_version` beacon as the parent row (the V131 trigger), so a change lands on the next firing with no restart. +#### Limiting a route to Critical or Warning alerts (V229, #6928) + +A route takes alerts of both severities unless you limit it. The **severities** setting of a route is `both` (the default, and what every route you made earlier has), `critical` or `warning`. A route set to `critical` takes Critical alerts only. A route set to `warning` takes Warning alerts only. With one route per severity you can send production warnings to a team channel and keep the on-call channel for Critical alerts. + +A route whose setting leaves an alert out does not match it. Matching goes on to the next route in the order above, and to the default channels when no route is left. It is the same fall-through as a route that is disabled or that names a different server. + +- **Which severity an alert has.** The alert's own severity decides when it carries one. An alert that is graded when it fires, such as an external alert or a custom alert rule, carries the severity it fired at. Otherwise the alert's badge decides: a red badge is Critical, and every other badge (Warning, Alert, Info) counts as Warning. So a Warning-only route also takes alerts that are not Critical. +- **A Warning alert that becomes Critical** while it is open is delivered again as a new Critical alert. The Critical route takes it from then on. +- **A resolution** goes to the channels that the alert's fire reached. The severity setting is not read again when the alert clears. Editing a route between the fire and the clear does not move the clear. The PagerDuty close for an external alert uses the severity that the fire went out at. It reaches the routing key of the route that opened the incident. +- **Agent requests** follow the same filter: a Warning alert does not open an investigation under a route limited to `critical`. +- **Set it** in the route dialog of the Viewer or in the Routes list of the web dashboard. In MCP, use the `severities` argument of `create_notification_route` and `update_notification_route`. The value is `critical`, `warning` or `both`, in any case. Any other value is refused as `invalid`. `get_notification_routes` returns `severities` for every route. +- **Lite** does not have this setting. A route you make in Lite always takes both severities. + #### Send resolutions (#5825) A resolution is the message that an alert has cleared: the condition that fired it is no longer there. Each channel (Slack, Teams, the generic webhook and email) has a **Send resolutions** box in the Viewer's Settings > Notifications, off by default. When it is on, a resolution goes only to the destinations of that channel that the alert's fire reached; an alert whose fire was not sent gets no resolution. A service restart between the fire and the clear does not lose this: the clear reads the fire's alert-history row, which names each channel that delivered and the route that supplied it, and sends to that route's destination as it is now (or the channel's own destination when no route supplied it). A route deleted or disabled since the fire gets nothing, with or without a restart between the fire and the clear: nothing is sent to that channel, and the history row says so. A mute rule stops a resolution when it matches the alert's fire, including a rule scoped to a database, wait type, job or query. Each resolution carries the server, the alert, when it fired and cleared, how long it lasted, the last value its fire sent and the fire's triage link. In alert history, the resolution row records what its send did: `get_alert_history` reports `resolution_sent_at` (UTC, set when a channel delivered), `resolution_channels` and `resolution_outcome` (`sent`, `failed: `, `muted` or `route deleted or disabled: `), null on every other row; the Viewer's Alert History grid has a **Resolution** column and Alert Details shows the three fields; the web Alerts page shows them in the row's detail. A route can override the channel setting for its own destinations: in the route dialog, **Send resolutions** is `Use channel setting` (the default), `Always` or `Never`. PagerDuty is not one of the four: its incidents close through **Auto-resolve the incident when the alert's condition clears**. Over MCP, `get_alert_settings` reports the four settings as `delivery.send_resolutions`, `update_alert_settings` changes them, and `get_notification_routes` reports each route's `send_resolutions` (null means the channel decides). A bring-your-own store needs `tools/provision-roles.sql` run again after the service upgrades the store to V187, so the `viewer` and `mcp` roles can read the new columns. diff --git a/docs/darling/configuration-mcp.md b/docs/darling/configuration-mcp.md index 93fa74f9cf..9d361029b5 100644 --- a/docs/darling/configuration-mcp.md +++ b/docs/darling/configuration-mcp.md @@ -59,7 +59,7 @@ The embedded MCP server, over Streamable HTTP bound to `localhost` by default (s - **Tool reading guides** - `get_tool_guide(tools, topics)` (#3898, both editions). A converted tool's description is split at a `<>` marker: `tools/list` serves the short head, which keeps every fact needed to read the answer correctly and ends "Reading guide: get_tool_guide.", and this tool returns the long-form guide by tool name, plus named cross-tool topics such as `system_health_empty_windows`. Unknown names are reported, not errors; called with no arguments it lists the tools that have guides and the topics. The `get_health_parser_*` family is converted first. - **Alert + health-overview tools** — the fleet-triage reads the fleet edition previously lacked, each a stored read over the monitoring store (no live hit): - - *Alerts* — `get_alert_history` (what fired, value vs threshold, delivery success/failure, muted, and since V131 `route` — where the post went — fleet-wide by default, or scoped to a server; `external_source` names the external alert source an alert came from and is null on every built-in and custom alert), `get_alert_settings` (the current alert config the service is using — per-alert enable/thresholds, cooldown, excluded databases, delivery mode, analysis cadence), `get_mute_rules` (the alert mute rules in force, so a suppressed server is distinguishable from a healthy-quiet one; each rule carries `external_source`, null unless the rule is limited to one external alert source), `get_notification_routes` (the alert-family taxonomy and the [notification routes](alerts-and-notifications.md#notification-routes--sending-alert-families-to-different-channels) — which channels each route sets, never the destination values). + - *Alerts* — `get_alert_history` (what fired, value vs threshold, delivery success/failure, muted, and since V131 `route` — where the post went — fleet-wide by default, or scoped to a server; `external_source` names the external alert source an alert came from and is null on every built-in and custom alert), `get_alert_settings` (the current alert config the service is using — per-alert enable/thresholds, cooldown, excluded databases, delivery mode, analysis cadence), `get_mute_rules` (the alert mute rules in force, so a suppressed server is distinguishable from a healthy-quiet one; each rule carries `external_source`, null unless the rule is limited to one external alert source), `get_notification_routes` (the alert-family taxonomy and the [notification routes](alerts-and-notifications.md#notification-routes--sending-alert-families-to-different-channels) — which channels each route sets and its `severities`, never the destination values). - *Health overview* — `get_server_summary` (one-shot per-server CPU / memory / recent blocking / recent deadlocks), `get_daily_summary` (a day's composite health band — Healthy / Warning / Critical — folded through the shared `DailyHealthBandCalculator`, plus the signals behind it). - **Fleet sweep reports (Darling-only)** — `get_sweep_reports` reads the scheduled [Fleet Sweep Reports](https://github.com/erikdarlingdata/PerformanceMonitor/issues/3466) — the stateful whole-fleet summaries the service persists at the configured cadence (`get_alert_settings`' `fleet_sweep` group; default hourly): the sweep timeline over a caller-set window with each sweep's document embedded, the newest sweep in full (per-server verdicts with band transitions pre-computed, and the would-have-paged ledger whenever a sweep ran under the alert master switch — present-and-empty on a muted sweep with nothing to report, ABSENT on an alerts-on sweep where no such check was made), and the watch-item worklist with its hysteresis position. One `sweep_id` (as a STRING — the ids are too large for a JSON number to round-trip) fetches one sweep in full. The same `FleetSweepPresentation` builders and store reads the web feed's `/api/sweeps` routes serve, so an agent and a browser cannot disagree about what a sweep looks like. The description carries the two field contracts an agent must read: `alerts_enabled: false` on a sweep means DELIVERY WAS OFF while it ran, and `instruments_alive: false` means the sweep could not prove its own data sources — quiet is not clean, and the `instrument_liveness` block says which instrument went unproven. A stored read; no monitored server is touched. @@ -72,7 +72,7 @@ The embedded MCP server, over Streamable HTTP bound to `localhost` by default (s The create/update/delete tools are the one view-authoring **write** surface; create/update run the SAME `ValidateDefinition` authority as `validate_custom_view`, so an invalid definition is rejected before it stores; every tool routes through the SAME store + validator + compile-and-run + catalog the web viewer's editor uses (no divergent second implementation). This write surface is part of what the MCP token gates — see [What a token can reach](network-endpoints.md) below. -- **Alert-tuning write tools (Darling-only)** — `update_alert_settings`, `create_mute_rule`, `update_mute_rule`, `delete_mute_rule`, `purge_expired_mute_rules` (deletes every expired rule, as the Viewer's Purge Expired does, and returns each one it deleted with `undo: null` because an expired rule had no effect), `set_mute_rule_enabled`, and the route writes `create_notification_route` / `update_notification_route` / `set_notification_route_enabled` / `set_notification_route_fallback` / `delete_notification_route`, and `update_notification_channels` (the default channels the Viewer sets in Settings > Notifications: SMTP host, port, SSL, user, password, from and recipients; the Teams, Slack and generic webhook URLs, the generic headers, body template and proxies; the PagerDuty routing key, region, proxy and auto-resolve. Only the fields passed change and `-` clears a text field. Each secret is sealed by the service the way the Viewer seals it and is shown in an answer only as set or not set; the `mcp` role holds no read or write on a secret column and writes them through the store's `config.set_notification_channels` function, which makes the Viewer's refusals on the stored row (a change to the host, port, SSL flag, user or a proxy while a secret tied to it is saved needs the secret again; a saved signing key blocks a generic URL or proxy change). The undo restores only the non-secret settings.) (both route tools set a route's own destinations: Teams, Slack and generic webhook URLs, a PagerDuty routing key and email recipients, sealed for the route's own row and shown in an answer only as saved or none (with the host for Teams and Slack), apart from the recipient list, which is shown in full; `create_notification_route` with no destination makes a route with every channel empty, so each channel goes where it went before (the next matching route, else the channels configured in Settings); `update_notification_route` changes the match, scope, enabled flag, resolution override, fallback and destinations of any route that is not an agent route, and the match or scope only on a route with no explicit agent setting (a route with an agent setting of Off keeps its match and scope in the Viewer, and the store function refuses the change for any login); both refuse an agent route, and the answer of an edit gives the previous values and the undo call, which for a destination says to pass the earlier value again. Each write is logged with the tool, the identity the service knows for the token (its first 8 SHA-256 hex characters, never the token), the client's own name labelled `client says` (any client can send any name) and the names of the fields changed, never a value), plus `dismiss_alert_history` / `restore_alert_history` (the flag the web dashboard's Dismiss sets, and the call that undoes it; nothing is deleted). None of these is callable by an agent Darling sends to investigate an alert let an MCP client TUNE the alert engine the fleet shares (`create_mute_rule` / `update_mute_rule` also take an optional lower-case `external_source` that limits a rule to the alerts one external source raised; a name that is not a valid source name is refused, and `update_mute_rule` clears it with an explicit null) — the SAME config `get_alert_settings` / `get_mute_rules` read and the Viewer's Settings window writes. `update_alert_settings` is a PARTIAL update of the alert configuration: read via `get_alert_settings`, change fields, and send only those back in the same nested shape; every field is validated against the SAME ranges/enums the Settings window enforces BEFORE any write, an out-of-range or unknown field returns `{status:"invalid"}` and writes nothing, and the write self-bumps `config_version` so the running service hot-reloads within one collection sweep. `create_mute_rule` / `update_mute_rule` / `delete_mute_rule` / `set_mute_rule_enabled` reuse the SAME `PgMuteRuleStore` `get_mute_rules` reads through (and the same GUID id-generation the Viewer's mute-create path uses). `set_mute_rule_enabled` is the headless twin of the Viewer's mute-rule checkbox: it writes `enabled` and nothing else, so a rule can leave force and return keeping its id, its reason and the `created_at_utc` the `Stale Mute Rules` self-alert ages it from — where a delete-and-re-create resets that clock and delivers the suppressed alerts in between. `update_mute_rule` is the headless twin of the Viewer's Edit dialog on the same terms: a PARTIAL update (send only the fields to change, in the shape `get_mute_rules` returns; an explicit JSON `null` clears a field, so a bounded rule can be made permanent again) written through the store's own full-row `UpdateAsync` — whose SET list does not name `created_at_utc`, so no edit, however sweeping, resets the staleness clock the way the delete/recreate workaround did for every field the enable flag cannot express. `update_alert_settings` spans the two singleton config rows the alert engine reads: every knob is on `config_alert_settings` except the per-fingerprint DELIVERY cooldown, which is stored as `config_notification.email_cooldown_minutes` and reported and accepted as `delivery.cooldown_minutes` — a channel-neutral name, because that one number is the only throttle on a Slack / Teams / PagerDuty / generic-webhook post as well as on email, so a headless deployment with no SMTP configured at all is still governed by it (the stored spelling is accepted as a write-only alias). Note the two cooldowns are different stages: top-level `cooldown_minutes` gates whether the engine FIRES, `delivery.cooldown_minutes` bounds the resulting post — twice over, once per alert FINGERPRINT and once per METRIC across the whole fleet for a RE-notification, so one fault on forty servers does not cost forty posts an hour and the servers that second bound holds back are named on the post that does go out. A first notice is never held back by either bound, and `delivery.mode` `PerEvent` opts out of the per-metric one. None touches a monitored SQL Server or the collected data — only the shared alert configuration; the one tool that sets delivery credentials is `update_notification_channels`, which sets the SMTP password, the webhook URLs and the keys sealed and never shows them back (the `mcp` role itself can read and write exactly the one non-secret cooldown column of the notification table and cannot read or write any secret column; the sealed values reach the store through the `config.set_notification_channels` function). It is part of what the MCP token gates — see [What a token can reach](network-endpoints.md) below. +- **Alert-tuning write tools (Darling-only)** — `update_alert_settings`, `create_mute_rule`, `update_mute_rule`, `delete_mute_rule`, `purge_expired_mute_rules` (deletes every expired rule, as the Viewer's Purge Expired does, and returns each one it deleted with `undo: null` because an expired rule had no effect), `set_mute_rule_enabled`, and the route writes `create_notification_route` / `update_notification_route` / `set_notification_route_enabled` / `set_notification_route_fallback` / `delete_notification_route`, and `update_notification_channels` (the default channels the Viewer sets in Settings > Notifications: SMTP host, port, SSL, user, password, from and recipients; the Teams, Slack and generic webhook URLs, the generic headers, body template and proxies; the PagerDuty routing key, region, proxy and auto-resolve. Only the fields passed change and `-` clears a text field. Each secret is sealed by the service the way the Viewer seals it and is shown in an answer only as set or not set; the `mcp` role holds no read or write on a secret column and writes them through the store's `config.set_notification_channels` function, which makes the Viewer's refusals on the stored row (a change to the host, port, SSL flag, user or a proxy while a secret tied to it is saved needs the secret again; a saved signing key blocks a generic URL or proxy change). The undo restores only the non-secret settings.) (both route tools set a route's own destinations: Teams, Slack and generic webhook URLs, a PagerDuty routing key and email recipients, sealed for the route's own row and shown in an answer only as saved or none (with the host for Teams and Slack), apart from the recipient list, which is shown in full; `create_notification_route` takes the same `severities` word (default `both`, so a route takes alerts of both severities unless you limit it, and any other value is refused as `invalid`), and with no destination makes a route with every channel empty, so each channel goes where it went before (the next matching route, else the channels configured in Settings); `update_notification_route` changes the match, scope, enabled flag, resolution override, severity filter (`severities`: `critical`, `warning` or `both`), fallback and destinations of any route that is not an agent route, and the match or scope only on a route with no explicit agent setting (a route with an agent setting of Off keeps its match and scope in the Viewer, and the store function refuses the change for any login); both refuse an agent route, and the answer of an edit gives the previous values and the undo call, which for a destination says to pass the earlier value again. Each write is logged with the tool, the identity the service knows for the token (its first 8 SHA-256 hex characters, never the token), the client's own name labelled `client says` (any client can send any name) and the names of the fields changed, never a value), plus `dismiss_alert_history` / `restore_alert_history` (the flag the web dashboard's Dismiss sets, and the call that undoes it; nothing is deleted). None of these is callable by an agent Darling sends to investigate an alert let an MCP client TUNE the alert engine the fleet shares (`create_mute_rule` / `update_mute_rule` also take an optional lower-case `external_source` that limits a rule to the alerts one external source raised; a name that is not a valid source name is refused, and `update_mute_rule` clears it with an explicit null) — the SAME config `get_alert_settings` / `get_mute_rules` read and the Viewer's Settings window writes. `update_alert_settings` is a PARTIAL update of the alert configuration: read via `get_alert_settings`, change fields, and send only those back in the same nested shape; every field is validated against the SAME ranges/enums the Settings window enforces BEFORE any write, an out-of-range or unknown field returns `{status:"invalid"}` and writes nothing, and the write self-bumps `config_version` so the running service hot-reloads within one collection sweep. `create_mute_rule` / `update_mute_rule` / `delete_mute_rule` / `set_mute_rule_enabled` reuse the SAME `PgMuteRuleStore` `get_mute_rules` reads through (and the same GUID id-generation the Viewer's mute-create path uses). `set_mute_rule_enabled` is the headless twin of the Viewer's mute-rule checkbox: it writes `enabled` and nothing else, so a rule can leave force and return keeping its id, its reason and the `created_at_utc` the `Stale Mute Rules` self-alert ages it from — where a delete-and-re-create resets that clock and delivers the suppressed alerts in between. `update_mute_rule` is the headless twin of the Viewer's Edit dialog on the same terms: a PARTIAL update (send only the fields to change, in the shape `get_mute_rules` returns; an explicit JSON `null` clears a field, so a bounded rule can be made permanent again) written through the store's own full-row `UpdateAsync` — whose SET list does not name `created_at_utc`, so no edit, however sweeping, resets the staleness clock the way the delete/recreate workaround did for every field the enable flag cannot express. `update_alert_settings` spans the two singleton config rows the alert engine reads: every knob is on `config_alert_settings` except the per-fingerprint DELIVERY cooldown, which is stored as `config_notification.email_cooldown_minutes` and reported and accepted as `delivery.cooldown_minutes` — a channel-neutral name, because that one number is the only throttle on a Slack / Teams / PagerDuty / generic-webhook post as well as on email, so a headless deployment with no SMTP configured at all is still governed by it (the stored spelling is accepted as a write-only alias). Note the two cooldowns are different stages: top-level `cooldown_minutes` gates whether the engine FIRES, `delivery.cooldown_minutes` bounds the resulting post — twice over, once per alert FINGERPRINT and once per METRIC across the whole fleet for a RE-notification, so one fault on forty servers does not cost forty posts an hour and the servers that second bound holds back are named on the post that does go out. A first notice is never held back by either bound, and `delivery.mode` `PerEvent` opts out of the per-metric one. None touches a monitored SQL Server or the collected data — only the shared alert configuration; the one tool that sets delivery credentials is `update_notification_channels`, which sets the SMTP password, the webhook URLs and the keys sealed and never shows them back (the `mcp` role itself can read and write exactly the one non-secret cooldown column of the notification table and cannot read or write any secret column; the sealed values reach the store through the `config.set_notification_channels` function). It is part of what the MCP token gates — see [What a token can reach](network-endpoints.md) below. - **Webhook signing key tools (Darling-only, #6394)** — `generate_webhook_signing_key` and `remove_webhook_signing_key`, each with an optional `route_id` (none is the default destination's key in Settings > Notifications, a number is that route's own generic webhook). `generate_webhook_signing_key` makes the key in the service, seals it for the destination's saved generic URL and proxy as the Viewer does, and stores it through the store's `config.set_webhook_signing_key` function; the `mcp` role has no read or write on a signing key column, and the service reads the saved URL and proxy on its own store connection. When a key is already stored the call rotates it: the current key moves to the previous slot and keeps signing for 7 days. The answer shows the new key once, and no answer ever shows a stored or previous key; the key goes only over TLS or to a caller on the service's machine, and a second generate inside the 7 day changeover of the first is refused unless the call passes `replace_pending_rotation: true`. A destination with no saved generic webhook URL, and a `route_id` that does not exist, are refused and write nothing. `remove_webhook_signing_key` clears the key, the previous key and the window, and says whether a key was there. Both are write tools, never offered to an agent token. `update_notification_route` refuses to change the generic URL of a route that has a key until the key is removed. - **Server-onboarding write tools (Darling-only)** — `add_servers` (BULK) and `remove_server` (which drops this install's own Extended Events sessions on the server and leaves the shared ones: `PerformanceMonitor_Deadlock`, `PerformanceMonitor_BlockedProcess`, `PerformanceMonitor_Attention` and the old `PerformanceMonitor_LongQueryCompletions`; once the server is removed, [`--drop-xe-sessions --print-sql`](collector-settings.md#drop-the-extended-events-sessions-a-removed-server-left-behind---drop-xe-sessions) prints the statements that drop what is left, because the named form `--drop-xe-sessions ` works only before the removal) let an MCP client stand up or tear down FLEET monitoring conversationally ("monitor these twenty servers with this login"), the service-side twin of the Viewer's Add / Manage Servers dialogs. `edit_server` changes one saved server in place and is served under the same `mcp.allowServerAdmin` setting as `add_servers` and `remove_server`; an edit that points Windows authentication at a connection whose server identity is not verified is refused as `add_servers` refuses it, before any connection is made. `add_servers` takes a JSON **array** of server objects (`host` required; optional `display_name` / `database` / `read_only_intent` / `multi_subnet_failover`; `auth` `Windows`/`SQL` with `username`+`password` for SQL; and the exposed TLS options `encrypt_mode` `Optional`/`Mandatory`/`Strict` + `trust_server_certificate`) and processes them **in order**: it validates each entry, PROBES the connection in-process (reusing the same `DarlingServerConnector.ProbeAsync` the `--test-connection` verb runs — the service holds the network path + credentials, so no `test_connect` command plane is needed), skips a case-folded duplicate (`duplicate`) of an already-monitored server or an earlier entry, DPAPI-encrypts the SQL password (the service identity, so it round-trips at collection time), and INSERTs the row mirroring the service's own seed shape. A server that fails to connect is `connection_failed` and the batch continues. `auth` also takes the two non-interactive Microsoft Entra modes (#3484). With `ServicePrincipal`, `username` is the app's client id and `password` is its client secret. The other mode is `ManagedIdentity`. Only the interactive Entra modes (MFA, device code, default credential) are `invalid`, because they need a signed-in user and cannot run headless. Off Windows, a literal SQL password or client secret is refused as `invalid` too, because encryption at rest is Windows DPAPI. Give an `env:NAME` or `file:/run/secrets/` reference instead, the same two forms that `darling.json` takes for every secret. A server is refused as `collides` when its probed connection lands in a database that another monitored server already covers. That happens when it names one database but connects to a different one, such as a wrong Initial Catalog. Adding it gives one database two identities, so its history is stored twice and its alerts fire twice. A server whose id is already held by a different server is refused as `collides` as well, because two identities can hash to one id. Nothing is written for a `collides` entry. If saving an entry fails, that entry and every entry after it are `not_saved` and are not probed. The entries before it stay `added`. `remove_server` DELETEs a monitored server by name (resolved the same way every `server_name` is) — already-collected history is kept. Both write only the monitoring store's `config.config_monitored_servers` registry; neither runs anything on a monitored server beyond the one-time probe. **The SQL password travels to the endpoint inside `add_servers`' request** and is DPAPI-encrypted at rest (never returned) — it is part of what the MCP token gates, and it puts a credential on the wire; see [What a token can reach](network-endpoints.md) below.