From d1831f6d20f923687ecc27d1dfbc8c036dd4b83d Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 16 Oct 2026 06:20:52 -0400 Subject: [PATCH 1/5] Per-server threshold overrides: loader, engine and PostgreSQL resolution, set_alert_threshold_override, get_alert_settings server view (#5916) --- .../DarlingThresholdOverrides.cs | 233 +++++++++++ .../DarlingWebEndpoints.cs | 2 +- .../DarlingWorker.cs | 45 ++- .../Mcp/DarlingMcpAlertTools.cs | 16 +- .../Mcp/DarlingMcpHostService.cs | 4 + .../Mcp/DarlingMcpThresholdOverrideTools.cs | 370 ++++++++++++++++++ .../Mcp/DarlingMcpWritePolicy.cs | 6 +- 7 files changed, 670 insertions(+), 6 deletions(-) create mode 100644 Darling/PerformanceMonitor.Darling.Service/DarlingThresholdOverrides.cs create mode 100644 Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingThresholdOverrides.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingThresholdOverrides.cs new file mode 100644 index 0000000000..4a9c307f03 --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingThresholdOverrides.cs @@ -0,0 +1,233 @@ +/* + * 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.Globalization; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Npgsql; +using PerformanceMonitor.Alerting; + +namespace PerformanceMonitor.Darling.Service; + +/// +/// The tunable thresholds Darling can hold an override for (#5916): one row per wire name, which is the name +/// get_alert_settings and update_alert_settings already use for the global value (for example +/// cpu.threshold_percent). Every registry key of #5910 () is here, plus the two +/// PostgreSQL count thresholds, which are Darling's own and are not on the engine's settings contract. PostgreSQL +/// predictor constants are not settings and have no entry. The bounds are the ones update_alert_settings and +/// the Viewer's Settings window enforce on the global value, so an override can never hold a value the global setting +/// would refuse. +/// +internal static class DarlingThresholdKeys +{ + /// One tunable threshold: its wire name, the engine key (null for a PostgreSQL count threshold), the scope it + /// can take and its bounds. + internal sealed record Entry(string Wire, AlertThresholdKey? Engine, AlertThresholdScope Scope, int Min, int Max); + + internal const string PgBlockingCount = "blocking.pg_count_threshold"; + internal const string PgDeadlockCount = "deadlocks.pg_count_threshold"; + + private static readonly Entry[] All = + [ + new("cpu.threshold_percent", AlertThresholdKey.CpuThresholdPercent, AlertThresholdScope.Server, 1, 100), + new("blocking.count_threshold", AlertThresholdKey.BlockingCountThreshold, AlertThresholdScope.Server, PostgresAlertEvaluator.CountThresholdFloor, int.MaxValue), + new("blocking.wait_threshold_seconds", AlertThresholdKey.BlockingWaitSecondsThreshold, AlertThresholdScope.Server, 0, int.MaxValue), + new("deadlocks.count_threshold", AlertThresholdKey.DeadlockCountThreshold, AlertThresholdScope.Server, PostgresAlertEvaluator.CountThresholdFloor, int.MaxValue), + new("runnable_queue.warning_per_scheduler", AlertThresholdKey.RunnableQueueWarningPerScheduler, AlertThresholdScope.Server, 1, int.MaxValue), + new("runnable_queue.critical_per_scheduler", AlertThresholdKey.RunnableQueueCriticalPerScheduler, AlertThresholdScope.Server, 1, int.MaxValue), + new("client_timeouts.warning_per_minute", AlertThresholdKey.ClientTimeoutsWarningPerMinute, AlertThresholdScope.Server, 1, int.MaxValue), + new("client_timeouts.critical_per_minute", AlertThresholdKey.ClientTimeoutsCriticalPerMinute, AlertThresholdScope.Server, 1, int.MaxValue), + new("long_running_query.threshold_minutes", AlertThresholdKey.LongRunningQueryThresholdMinutes, AlertThresholdScope.Server, 1, int.MaxValue), + new("tempdb_space.threshold_percent", AlertThresholdKey.TempDbSpaceThresholdPercent, AlertThresholdScope.Server, 1, 100), + new("low_disk.threshold_percent", AlertThresholdKey.LowDiskThresholdPercent, AlertThresholdScope.Server, 0, 100), + new("low_disk.threshold_gb", AlertThresholdKey.LowDiskThresholdGb, AlertThresholdScope.Server, 0, int.MaxValue), + new("low_disk.critical_free_percent", AlertThresholdKey.DiskCriticalFreePercent, AlertThresholdScope.Server, 0, 100), + new("low_disk.critical_free_gb", AlertThresholdKey.DiskCriticalFreeGb, AlertThresholdScope.Server, 0, int.MaxValue), + new("pvs.threshold_percent", AlertThresholdKey.PvsThresholdPercent, AlertThresholdScope.ServerAndDatabase, 0, 100), + new("pvs.floor_gb", AlertThresholdKey.PvsFloorGb, AlertThresholdScope.ServerAndDatabase, 0, int.MaxValue), + new("file_growth.rise_mb", AlertThresholdKey.FileGrowthRiseMb, AlertThresholdScope.ServerAndDatabase, 0, int.MaxValue), + new("file_growth.volume_percent", AlertThresholdKey.FileGrowthVolumePercent, AlertThresholdScope.ServerAndDatabase, 0, 100), + new("file_growth.lookback_minutes", AlertThresholdKey.FileGrowthLookbackMinutes, AlertThresholdScope.Server, 5, 1440), + new("long_running_job.multiplier", AlertThresholdKey.LongRunningJobMultiplier, AlertThresholdScope.Server, 2, 20), + new("failed_job.lookback_minutes", AlertThresholdKey.FailedJobLookbackMinutes, AlertThresholdScope.Server, 1, 1440), + new(PgBlockingCount, null, AlertThresholdScope.Server, PostgresAlertEvaluator.CountThresholdFloor, int.MaxValue), + new(PgDeadlockCount, null, AlertThresholdScope.Server, PostgresAlertEvaluator.CountThresholdFloor, int.MaxValue), + ]; + + private static readonly Dictionary ByWire = BuildByWire(); + private static readonly Dictionary ByEngine = BuildByEngine(); + + private static Dictionary BuildByWire() + { + var map = new Dictionary(StringComparer.Ordinal); + foreach (var e in All) + map[e.Wire] = e; + return map; + } + + private static Dictionary BuildByEngine() + { + var map = new Dictionary(); + foreach (var e in All) + { + if (e.Engine is { } k) + map[k] = e; + } + + return map; + } + + /// Every tunable threshold, in display order. + internal static IReadOnlyList Entries => All; + + internal static bool TryGet(string? wire, out Entry entry) + { + entry = null!; + return wire is not null && ByWire.TryGetValue(wire.Trim(), out entry!); + } + + internal static Entry For(AlertThresholdKey key) => ByEngine[key]; + + /// + /// Why a key cannot be overridden, or null when it is a tunable threshold. The reasons name the class of key so an + /// operator or agent sees why: an override moves a threshold, never a switch, a cadence or a destination. + /// + internal static (string Reason, string Message)? RefusalFor(string? key) + { + var k = (key ?? string.Empty).Trim(); + if (k.Length == 0) + return ("key_required", "key is required: a threshold such as cpu.threshold_percent (see get_alert_settings with server_name)."); + if (TryGet(k, out _)) + return null; + if (k.StartsWith("self_alerts.", StringComparison.OrdinalIgnoreCase)) + return ("not_tunable", $"'{k}' is a self-monitoring setting (self_alerts.*) about the monitor itself, not a monitored server. It has no per-server override."); + if (k.EndsWith(".enabled", StringComparison.OrdinalIgnoreCase) || string.Equals(k, "enabled", StringComparison.OrdinalIgnoreCase)) + return ("not_tunable", $"'{k}' is an enable flag. An override changes a threshold, never turns an alert on or off; mute the alert to silence it."); + if (k.Contains("cooldown", StringComparison.OrdinalIgnoreCase)) + return ("not_tunable", $"'{k}' is a cooldown. It spaces notifications and has no per-server override."); + if (k.Contains("route", StringComparison.OrdinalIgnoreCase) || k.StartsWith("delivery.", StringComparison.OrdinalIgnoreCase)) + return ("not_tunable", $"'{k}' is a route or delivery setting. It has no per-server override."); + return ("not_in_registry", $"'{k}' is not a tunable threshold. See get_alert_settings with server_name for the keys that can be overridden."); + } + + /// Why or is refused for , or null. + internal static (string Reason, string Message)? ValidateValue(Entry entry, int value, string? database) + { + if (value < entry.Min || value > entry.Max) + { + var max = entry.Max == int.MaxValue ? "2147483647" : entry.Max.ToString(CultureInfo.InvariantCulture); + return ("out_of_bounds", $"'{entry.Wire}' must be an integer between {entry.Min.ToString(CultureInfo.InvariantCulture)} and {max}, the same bounds the global setting has."); + } + + if (!string.IsNullOrWhiteSpace(database) && entry.Scope != AlertThresholdScope.ServerAndDatabase) + return ("database_scope", $"'{entry.Wire}' is compared against a server-level figure, so it takes a server override only, not a database_name."); + return null; + } +} + +/// +/// The per-server and per-database threshold overrides the alert engine and the PostgreSQL count checks resolve through +/// (#5916): the Darling loader behind . It holds one immutable snapshot of +/// config.config_threshold_overrides and answers from it, so the sweep path does no I/O. The service reloads it at +/// start and on every control-plane reload (the table is on the config_version beacon), and a failed load keeps the +/// snapshot already in force, as the mute rules do. Lite passes no overrides and stays advise-only. +/// +internal sealed class DarlingThresholdOverrides : IAlertThresholdOverrides +{ + internal const string SelectSql = + "SELECT server_id, database_name, threshold_key, value FROM config.config_threshold_overrides"; + + private readonly ILogger? _logger; + private volatile Dictionary<(int Server, string Database, string Key), int> _snapshot = new(); + + internal DarlingThresholdOverrides(ILogger? logger = null) => _logger = logger; + + /// How many overrides are in force. + internal int Count => _snapshot.Count; + + /// + /// Replaces the snapshot with the table's rows. The read is awaited before the swap and nothing here catches, so a + /// read that throws leaves the overrides already in force. A row whose key is not a tunable threshold, whose value is + /// not an integer inside the key's bounds, or whose database name sits on a server-only key is skipped and logged, + /// never applied: the engine uses the value as-is. + /// + internal async Task LoadAsync(NpgsqlDataSource postgres, CancellationToken cancellationToken = default) + { + var next = new Dictionary<(int, string, string), int>(); + await using var command = postgres.CreateCommand(SelectSql); + command.CommandTimeout = ServiceCommandDeadlines.SerialLoopSeconds; + await using var reader = await command.ExecuteReaderAsync(cancellationToken); + while (await reader.ReadAsync(cancellationToken)) + { + var serverId = reader.GetInt32(0); + var database = reader.IsDBNull(1) ? string.Empty : reader.GetString(1); + var key = reader.GetString(2); + var raw = reader.GetDouble(3); + if (Accept(key, raw, database, out var value) is { } problem) + { + _logger?.LogWarning( + "Threshold override for server {ServerId} key '{Key}' database '{Database}' was skipped: {Problem}", + serverId, key, database, problem); + continue; + } + + next[(serverId, database, key)] = value; + } + + _snapshot = next; + } + + /// Replaces the snapshot from rows already in hand (the loader's own tests, and any caller that read the table). + internal void Replace(IEnumerable<(int ServerId, string? Database, string Key, double Value)> rows) + { + var next = new Dictionary<(int, string, string), int>(); + foreach (var (serverId, database, key, raw) in rows) + { + var db = database ?? string.Empty; + if (Accept(key, raw, db, out var value) is null) + next[(serverId, db, key)] = value; + } + + _snapshot = next; + } + + private static string? Accept(string key, double raw, string database, out int value) + { + value = 0; + if (!DarlingThresholdKeys.TryGet(key, out var entry)) + return "not a tunable threshold"; + if (raw != Math.Floor(raw) || raw < int.MinValue || raw > int.MaxValue) + return "not an integer"; + value = (int)raw; + return DarlingThresholdKeys.ValidateValue(entry, value, database.Length == 0 ? null : database)?.Message; + } + + /// + public int Resolve(AlertThresholdKey key, string serverKey, string? database, int globalValue) => + ResolveWire(DarlingThresholdKeys.For(key).Wire, serverKey, database, globalValue); + + /// + /// The value of the wire-named threshold for the server (and database): a database override first, then the + /// server's, else . is the store server id as text, the + /// engine's server key. This is the one resolution the PostgreSQL count thresholds use too. + /// + internal int ResolveWire(string wire, string serverKey, string? database, int globalValue) + { + if (!int.TryParse(serverKey, NumberStyles.Integer, CultureInfo.InvariantCulture, out var serverId)) + return globalValue; + var snapshot = _snapshot; + if (snapshot.Count == 0) + return globalValue; + if (!string.IsNullOrEmpty(database) && snapshot.TryGetValue((serverId, database, wire), out var perDatabase)) + return perDatabase; + return snapshot.TryGetValue((serverId, string.Empty, wire), out var perServer) ? perServer : globalValue; + } +} diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs index 89773cc85b..8adc845f0f 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs @@ -6316,7 +6316,7 @@ internal static IReadOnlyDictionary BuildReadDispatch(I : int.TryParse(First(c, "server_id"), NumberStyles.Integer, CultureInfo.InvariantCulture, out var detailsServerId) ? DarlingMcpAlertTools.GetAlertDetails(pg, detailsServerId, detailsMetric, detailsTime, c.RequestAborted) : DarlingMcpAlertTools.GetAlertDetails(pg, 0, "", detailsTime, c.RequestAborted), - ["get_alert_settings"] = (c, pg, an) => DarlingMcpAlertTools.GetAlertSettings(pg, c.RequestAborted), + ["get_alert_settings"] = (c, pg, an) => DarlingMcpAlertTools.GetAlertSettings(pg, Server(c), c.RequestAborted), ["get_mute_rules"] = (c, pg, an) => DarlingMcpAlertTools.GetMuteRules(pg, QueryBool(c, "enabled_only", true), c.RequestAborted), ["get_notification_routes"] = (c, pg, an) => DarlingMcpAlertTools.GetNotificationRoutes(pg, c.RequestAborted), ["list_investigation_requests"] = (c, pg, an) => DarlingMcpInvestigationTools.ListInvestigationRequests(pg, Str(c, "state"), Server(c), Str(c, "family"), QueryInt(c, "limit", null, 20), c.RequestAborted), diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs index ffd30a9aa2..240136adff 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs @@ -825,6 +825,11 @@ warning that matters here. */ /* Set once by ExecuteAsync before the loop starts; the observability writes need it. */ private NpgsqlDataSource? _postgres; + + /* #5916: the per-server and per-database threshold overrides the engine and the two PostgreSQL count checks resolve + through. Loaded at start and on every control-plane reload (the table is on the config_version beacon); empty until then, + which is the global threshold everywhere. */ + private DarlingThresholdOverrides _thresholdOverrides = new(); /* #5366: the service's password key at run time, set once at start; null before it. */ private DarlingPasswordKeyRuntime? _passwordKeyRuntime; @@ -1463,6 +1468,7 @@ public DarlingWorker(ILogger logger, ILoggerFactory loggerFactory _readAdmission = readAdmission; _mcpTlsCertState = mcpTlsCertState; _logger = logger; + _thresholdOverrides = new DarlingThresholdOverrides(logger); _loggerFactory = loggerFactory; _mcpState = mcpState; _webState = webState; @@ -3367,6 +3373,7 @@ are hoisted here because the AN3 analysis-notification path below shares them. T var muteRuleService = new MuteRuleService( new PgMuteRuleStore(postgres), _loggerFactory.CreateLogger()); await LoadMuteRulesAsync(muteRuleService); + await LoadThresholdOverridesAsync(); /* #5632 Phase A: Level 1 investigations. The dispatcher opens or joins a request after an alert row is written (the deliverer and the finding sender below call it), and the sweep loop expires lapsed leases and old requests. Both @@ -6693,6 +6700,7 @@ flips its observed row FALSE even though the disabled server drops out of the lo await DarlingObservability.SyncServerEnabledStatesAsync(_postgres!, _logger, cancellationToken); await LoadMuteRulesAsync(muteRuleService); + await LoadThresholdOverridesAsync(); /* view.ConfigVersion rather than _lastConfigVersion: the caller has not advanced the watermark yet (it advances from this method's RETURN value), so the field still holds the PREVIOUS version @@ -6759,6 +6767,33 @@ await DarlingRemovedServerSessions.DropAsync( /// a stale mute set. The elapsed is the store read's alone — LoadAsync's expiry purge runs only /// after a read that succeeded, and swallows its own write failures. /// + /// + /// Reloads the threshold overrides (#5916). A failed read keeps the overrides already in force and is counted, as the + /// mute-rule reload is, so a store blip never drops a tuned threshold back to the global one. + /// + private async Task LoadThresholdOverridesAsync() + { + var postgres = _postgres; + if (postgres is null) + { + return; + } + + var readClock = Stopwatch.StartNew(); + try + { + await _thresholdOverrides.LoadAsync(postgres); + } + catch (Exception ex) + { + _logger.LogWarning( + "Could not reload threshold overrides after {ElapsedMs} ms - the {Overrides} override(s) already in force " + + "stay in force until a read succeeds: {Message}", + readClock.ElapsedMilliseconds, _thresholdOverrides.Count, ex.Message); + _readFailures.RecordReadFailure(null, "threshold-override reload", readClock.ElapsedMilliseconds); + } + } + private async Task LoadMuteRulesAsync(MuteRuleService muteRuleService) { var readClock = Stopwatch.StartNew(); @@ -7622,7 +7657,9 @@ so neither can break the sweep. */ escalationSeedReader: EscalationSeed.ReaderFor(historyStore), /* #3013: the process counter every swallowed condition read is tallied on. Passed explicitly rather than defaulted inside the engine so a test constructs its own and cannot pollute it. */ - readFailures: AlertReadFailureCounter.Shared); + readFailures: AlertReadFailureCounter.Shared, + /* #5916: a threshold held for one server or database. Lite passes none. */ + overrides: _thresholdOverrides); } /// @@ -8477,7 +8514,8 @@ collection_time would put a report in the wrong window and move it every cycle. /* #3444: read ONCE, then handed to the gate and the delivered message alike, so the two cannot quote different numbers when a store reload swaps config.Alerts mid-evaluation. The rate tiers the fire is GRADED on (#3653, below) are read at the same spot for the same reason. */ - var threshold = alertSettings.PgDeadlockCountThreshold; + var threshold = _thresholdOverrides.ResolveWire( + DarlingThresholdKeys.PgDeadlockCount, snapshot.ServerKey, null, alertSettings.PgDeadlockCountThreshold); var rateTiers = alertSettings.DeadlockRateThresholds; var decision = RollingCountAlertGate.Evaluate( @@ -8686,7 +8724,8 @@ row per root — never the only thing standing between a real distinct root and var cooldownElapsed = !LastFiredStamp.TryGet(_lastPgBlockingAlert, key, now, out var last) || now - last >= cooldown; /* #3444: read ONCE, for the reason EvaluatePgDeadlocksAsync gives at the same spot. */ - var threshold = alertSettings.PgBlockingCountThreshold; + var threshold = _thresholdOverrides.ResolveWire( + DarlingThresholdKeys.PgBlockingCount, snapshot.ServerKey, null, alertSettings.PgBlockingCountThreshold); var decision = RollingCountAlertGate.Evaluate( count, threshold, watermark, cooldownElapsed, suppressed: false); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs index 2bc6a41e10..a34ca621c2 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs @@ -302,6 +302,7 @@ internal static async Task GetAlertDetails( } [McpServerTool(Name = "get_alert_settings"), Description("Gets the alert configuration currently in effect: enabled flags, thresholds, cooldowns, delivery mode, and cadences. cooldown_minutes gates whether an alert FIRES; delivery.cooldown_minutes separately bounds the resulting post, per alert fingerprint and, for a re-notification, per metric across every monitored server. Darling: an unseeded store answers status unavailable and darling.json's defaults still govern until it seeds. Lite always answers its live in-memory settings. An empty knob list (e.g. long_running_query's exclusions) means cleared, not a default in force.<>Gets the current alert configuration the service is using: which alerts are enabled and their thresholds (CPU, blocking, deadlocks, poison waits, runnable queue (runnable tasks per scheduler), client timeouts (attentions per minute), long-running queries/jobs, tempdb, low disk, failed jobs, database state, Availability Group health, connection loss), the cooldown, excluded databases, the deadlock/blocking delivery mode and cooldown, the scheduled-analysis cadence, and the fleet-sweep cadence. TWO different cooldowns are reported and they govern different stages: top-level cooldown_minutes gates whether the alert engine FIRES at all, while delivery.cooldown_minutes bounds the resulting Slack/Teams/PagerDuty/webhook/email post twice over: once per alert FINGERPRINT, and once per METRIC across the whole fleet for a RE-notification. The second bound is why one fault on forty servers does not cost forty posts an hour; the servers it holds back are named on the post that does go out, under an 'Other Servers Affected' section. A first notice is never held back by either bound, and PerEvent delivery mode opts out of the per-metric one. A channel going quiet with alerts still in get_alert_history is delivery.cooldown_minutes, not cooldown_minutes. The self_alerts group holds the thresholds for alerts about the MONITOR STORE itself rather than a monitored server — those arrive with Server: 'Monitor Store' by default, or with the store's own peers.storeName label when the operator set that file-only field (a multi-store estate names each store on its own self-alerts), so an alert naming either spelling is tuned here and nowhere else, including Retention Held's warn/critical ratios. Mute rules match the alert row's server spelling, so on a store with storeName set, scope self-alert mutes to that label, not to 'Monitor Store'. The health_bands group is NOT an alert: its two tiers decide what band a server's card, the worst-first ranking and get_fleet_overview's counts read, in deadlocks per HOUR normalised over whatever window was asked for — so the same pair means the same condition on a 1-hour read and a 24-hour one. Tuning deadlocks.count_threshold does not move the band and tuning health_bands does not move the alert. The file_growth group's rise_mb is megabytes per HOUR, averaged over file_growth.lookback_minutes — a rate, not a total for the window: 10240 means 10 GB/hr whether the lookback is 5 minutes or 24 hours, and the engine scales it to the window (a 5-minute lookback asks for 853 MB inside it, a 24-hour one for 240 GB). The fleet_sweep group is NOT an alert family either, and its cadence is a SECOND cadence, separate from the scheduled-analysis one: fleet_sweep.enabled turns the scheduled whole-fleet sweep report on or off, and fleet_sweep.interval_minutes (15–1440, default 60 — hourly) is how often it runs. The alerts_enabled master switch deliberately does not govern sweep production, only delivery: sweeps keep running under alerts_enabled: false — that is when they carry the would-have-paged ledger — so muting the fleet does not blind the report surface. Separately, deadlocks.pg_count_threshold and blocking.pg_count_threshold are the PostgreSQL versions of those two alerts' count gates, reported inside those same groups, and they are deliberately NOT the same numbers as the count_threshold beside them: the two engines count with different instruments (captured deadlock graphs versus deadlocks parsed from the server log; engine-recorded blocked-process reports versus a periodic SAMPLE of pg_stat_activity), and while both engines' cards now band deadlocks through the same health_bands tiers, a PostgreSQL server still has no blocking band to calibrate against. The enabled switch in each group governs BOTH engines; the two thresholds do not move each other. On a store with no PostgreSQL targets both PostgreSQL keys are inert. poison_wait.threshold_ms is RETIRED: since the Poison Wait alert grades ACCUMULATED wait over a ten-minute window, this value is stored and reported for compatibility but consulted by nothing; poison_wait.threshold_ms_note says so beside it on every read, and update_alert_settings accepts it with a warning rather than refusing a round-tripped payload. poison_wait.enabled is the live switch. client_timeouts.enabled is the Client Timeouts alert's switch (OFF by default: no measured normal rate exists yet), and client_timeouts.warning_per_minute and client_timeouts.critical_per_minute are its bars: client attentions (a client cancelled a query or its timeout expired) per minute over the attention collector's last runs (about 3 minutes), as the engine applies them (Warning at least 1, Critical never below Warning). analysis.uncorroborated_route is where a notify-worthy but UNCORROBORATED finding goes — one fact in its chain and no matched co-fire check: 'digest' (the default) keeps it off every paging channel and puts it in the daily Analysis Singles Digest and the web/MCP surfaces, 'page' restores the earlier paging of every notify-worthy finding; a corroborated finding pages under either. It is the EFFECTIVE route, never null, resolved from the knob's two homes with the STORE winning: the settings row's analysis_uncorroborated_route column (V137) when it holds a route, else darling.json's analysis.uncorroboratedRoute (read once at service start), else the shipped 'digest'. analysis.uncorroborated_route_source says which home decided — 'store', 'file' or 'default' — and analysis.uncorroborated_route_note restates the precedence beside it; a store column left NULL (every store the morning after the V137 upgrade, and every fresh seed) reads 'file'. Set it with update_alert_settings or the Viewer's Settings window (Notifications > Automated Analysis): live in the running service within one collection sweep, consulted on the next scheduled-analysis delivery, no restart; send null through update_alert_settings to clear the column and hand the decision back to the file. long_running_query.excluded_program_name_prefixes and long_running_query.excluded_logins are the Long-Running Query alert's OPT-OUT knob: a session whose program_name STARTS WITH an entry of the first list, or whose login_name IS an entry of the second (both case-insensitive, no wildcard grammar), is NOT EVALUATED by that alert at all — not read into the decision, not counted, not fingerprinted — which is the opposite of a mute rule (a mute silences a fire already decided; an exclusion means the session never reaches the decision, and excluding a paging program resolves the open incident). The exclusion is applied in the read ahead of long_running_query.max_results, so an excluded session never consumes a result slot, and the fired alert's card carries Excluded Count / Excluded By Program Prefix / Excluded By Login items saying how many SESSIONS each list removed on that evaluation (a session matching both counts once, under the prefix). The lists ship SEEDED from a 7-day read of one large production store, whose long-running population fell into four classes: (1) SQL Agent job steps — program_name prefix 'SQLAgent - TSQL JobStep', ~460 sessions a week across 11 jobs, medians 35–62 min — the default prefix; (2) the NT AUTHORITY\\SYSTEM and NT AUTHORITY\\NETWORK SERVICE logins — the permanent multi-day CDC-shaped background — the default logins; (3) the application's admin login — deliberately NOT a default, because it runs the job wave but also real ad-hoc long-runners, and the job-step prefix already covers its share; (4) named humans — never excluded, they are what the page is for. The seeds are DEFAULTS: an empty list here means the operator cleared it and that arm excludes nothing. SMTP/webhook delivery credentials are managed separately and are not reported here. delivery.send_resolutions says, per channel (slack, teams, webhook for the generic webhook, email), whether the message that an alert has cleared is also sent to the destinations its fire reached; all four are off by default, and a notification route can override them (get_notification_routes, send_resolutions). The credentials themselves are not reported — configure them in the standalone Darling Viewer app's Settings window (Notifications section), which connects to this store (including remotely, not just localhost) rather than requiring desktop access to this specific box.")] public static async Task GetAlertSettings( NpgsqlDataSource postgres, + [Description("Optional (#5916): a server name. Adds server_view: every tunable threshold with its global value, the value in effect for that server and its source (global or server_override), and that server's database overrides.")] string? server_name = null, CancellationToken cancellationToken = default) { try @@ -325,7 +326,20 @@ the shipped 15 instead would state a number nobody wrote. */ "unavailable", "No notification row is present in the store yet, so the delivery cooldown cannot be reported. The service seeds it alongside the alert-settings row on startup (or the Viewer's Settings window writes it); until then the service runs on its darling.json defaults."); - return JsonSerializer.Serialize(BuildAlertSettingsPayload(s, deliveryCooldown), McpHelpers.JsonOptions); + if (string.IsNullOrWhiteSpace(server_name)) + return JsonSerializer.Serialize(BuildAlertSettingsPayload(s, deliveryCooldown), McpHelpers.JsonOptions); + + /* #5916: the one server's effective thresholds. Resolved after the shared snapshot so an unknown server answers + its own error and the global read above is unchanged. */ + var (resolved, resolveError) = await DarlingServerResolver.ResolveOrErrorAsync(postgres, server_name, cancellationToken); + if (resolveError is not null) + return resolveError; + + var node = JsonSerializer.SerializeToNode(BuildAlertSettingsPayload(s, deliveryCooldown), McpHelpers.JsonOptions)!.AsObject(); + node["server_view"] = JsonSerializer.SerializeToNode( + await DarlingMcpThresholdOverrideTools.BuildServerViewAsync(postgres, s, resolved.ServerId, resolved.ServerName, cancellationToken), + McpHelpers.JsonOptions); + return node.ToJsonString(McpHelpers.JsonOptions); } catch (Exception ex) when (ex is not OperationCanceledException) { diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs index 5f27eaa599..a01fe673ef 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs @@ -1177,6 +1177,10 @@ find the accumulator in a process-wide static that only the web host set -- so t host share it. Typed-generic AddSingleton, for the seat census (McpServiceParameterDiSeatCensusTests). */ services.AddSingleton(InvestigationOpenSignal.Shared); + /* #5916: set_alert_threshold_override is a Level 2 action tool offered under both arms and takes the served policy as a + service parameter to tell them apart. Typed-generic AddSingleton, for the seat census (McpServiceParameterDiSeatCensusTests). */ + services.AddSingleton(new DarlingMcpServedPolicy(writes)); + /* The write switch (DarlingMcpWritePolicy). Once the SDK has gathered every registered tool into McpServerOptions.ToolCollection, drop the writes the settings do not offer and stamp every tool's annotation hints. PostConfigure runs after the SDK's own options setup, so it sees the full set, and diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs new file mode 100644 index 0000000000..e025b9f5ba --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs @@ -0,0 +1,370 @@ +/* + * 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.ComponentModel; +using System.Globalization; +using System.Text.Json; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using ModelContextProtocol.Server; +using Npgsql; +using NpgsqlTypes; +using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Storage; + +namespace PerformanceMonitor.Darling.Service.Mcp; + +/// +/// What the MCP host serves, as a service parameter a tool can take (#5916): the one +/// the host started with. A Level 2 action tool needs it to tell its two arms apart. +/// +internal sealed record DarlingMcpServedPolicy(DarlingMcpWritePolicy.Effective Policy); + +/// +/// set_alert_threshold_override (#5916): set or clear a threshold for one server, or one database on it, without +/// touching the global value. Offered under both arms. Under the writes arm it is an ordinary write tool and +/// agent_action is ignored. Under the investigations arm it is a Level 2 action: agent_action is required and +/// the change goes through , for the request's own server only, attributed to the +/// agent and the request. An agent changes a threshold by override only; update_alert_settings is never offered to it. +/// +[McpServerToolType] +internal static class DarlingMcpThresholdOverrideTools +{ + internal const string ToolName = "set_alert_threshold_override"; + internal const string Kind = "threshold_override"; + + private static readonly Regex SetByShape = new("^[A-Za-z0-9._:-]{1,64}$", RegexOptions.Compiled | RegexOptions.CultureInvariant); + + /// The table, schema-qualified; the mcp role holds SELECT, INSERT, UPDATE and DELETE on it (#5920). + internal const string Table = "config.config_threshold_overrides"; + + internal const string PreconditionMessage = + "The service's mcp store role has no grant on config.config_threshold_overrides. A bring-your-own store needs " + + "Darling/tools/provision-roles.sql re-run (it is idempotent; single-table grants only reach a store when the script " + + "runs again) as a superuser. Managed and compose stores grant it themselves on start. Call again after the script has run."; + + [McpServerTool(Name = ToolName), Description( + "Sets or clears a threshold for ONE server (or one database on it), leaving the global value alone. Changes the " + + "monitoring store only; the service picks it up within its reload interval. <> " + + "key is a tunable integer threshold named as get_alert_settings names it (for example cpu.threshold_percent, " + + "blocking.pg_count_threshold); get_alert_settings with server_name lists the keys, the global value, the effective value " + + "and where it comes from. value is an integer inside the same bounds the global setting has. clear: true removes the " + + "override so the global value applies again. database_name applies only to pvs.* and file_growth.rise_mb / " + + "file_growth.volume_percent. Refused: a key outside that list, a self_alerts.*, enable, cooldown or route key, an " + + "out-of-bounds value. To silence an alert, mute it: an override moves a threshold and never turns an alert off. " + + "set_by names who made the change (writes arm; default mcp). On an investigation request's agent arm, agent_action is " + + "REQUIRED and the change is for the request's server only; the writes arm ignores agent_action. " + + "status ok, cleared, refused, precondition (the store role lacks the grant; the message names provision-roles.sql).")] + public static async Task SetAlertThresholdOverride( + NpgsqlDataSource postgres, + DarlingMcpServedPolicy served, + ILogger logger, + [Description("Server name, or the start of it.")] string server_name, + [Description("A tunable threshold, as get_alert_settings names it, e.g. cpu.threshold_percent.")] string key, + [Description("The override value (integer).")] int? value = null, + [Description("Database on that server (pvs.*, file_growth.rise_mb, file_growth.volume_percent). Omit for the whole server.")] string? database_name = null, + [Description("true removes the override.")] bool clear = false, + [Description("Who made the change (writes arm). 1-64 of A-Za-z0-9._:-. Default mcp.")] string? set_by = null, + [Description("Investigations arm only: the agent_action object (request_id, claim_id, agent_id, reason, evidence). Ignored under the writes arm.")] string? agent_action = null, + CancellationToken cancellationToken = default) => + await SetAsync(postgres, served.Policy, new InvestigationRequestStore(postgres, ServiceCommandDeadlines.SerialLoopSeconds), + logger, server_name, key, value, database_name, clear, set_by, agent_action, cancellationToken); + + /// The tool body over a given store, so a test drives either arm. + internal static async Task SetAsync( + NpgsqlDataSource postgres, DarlingMcpWritePolicy.Effective policy, InvestigationRequestStore store, ILogger logger, + string server_name, string key, int? value, string? database_name, bool clear, string? set_by, string? agent_action, + CancellationToken cancellationToken) + { + try + { + if (DarlingThresholdKeys.RefusalFor(key) is { } keyRefusal) + return Refused(keyRefusal.Reason, keyRefusal.Message); + DarlingThresholdKeys.TryGet(key, out var entry); + var wire = entry.Wire; + var database = string.IsNullOrWhiteSpace(database_name) ? null : database_name.Trim(); + + if (!clear) + { + if (value is null) + return Refused("value_required", "value is required (an integer), or pass clear: true to remove the override."); + if (DarlingThresholdKeys.ValidateValue(entry, value.Value, database) is { } valueRefusal) + return Refused(valueRefusal.Reason, valueRefusal.Message); + } + else if (database is not null && entry.Scope != PerformanceMonitor.Alerting.AlertThresholdScope.ServerAndDatabase) + { + return Refused("database_scope", $"'{wire}' takes a server override only, not a database_name."); + } + + string? person = null; + if (policy.Writes) + { + person = string.IsNullOrWhiteSpace(set_by) ? "mcp" : set_by.Trim(); + if (!SetByShape.IsMatch(person)) + return Refused("set_by_invalid", "set_by must be 1-64 of A-Za-z0-9._:-."); + } + + var (resolved, error) = await DarlingServerResolver.ResolveOrErrorAsync(postgres, server_name, cancellationToken); + if (error is not null) + return error; + + if (!await HoldsGrantAsync(postgres, cancellationToken)) + return McpHelpers.Status("precondition", PreconditionMessage); + + var applied = new Applied(); + /* The gate validates the argument itself (check 1); this parse only names the agent and request on the row. */ + _ = InvestigationToolCore.ParseAgentAction(agent_action, out var action); + + async Task ReadBefore(NpgsqlConnection c, NpgsqlTransaction t, CancellationToken ct) + { + applied.Previous = await ReadValueAsync(c, t, resolved.ServerId, database, wire, ct); + return applied.Previous is null ? null : JsonSerializer.Serialize(new { value = applied.Previous }); + } + + async Task Write(NpgsqlConnection c, NpgsqlTransaction t, CancellationToken ct) + { + /* Attribution: the writes arm names the person; an agent change names the agent and the request. */ + var by = person ?? "agent:" + action?.AgentId; + Guid? requestId = person is null ? action?.RequestId : null; + applied.Previous = await ReadValueAsync(c, t, resolved.ServerId, database, wire, ct); + if (clear) + await DeleteAsync(c, t, resolved.ServerId, database, wire, ct); + else + await UpsertAsync(c, t, resolved.ServerId, database, wire, value!.Value, by, requestId, ct); + applied.SetBy = by; + applied.RequestId = requestId; + return clear ? null : JsonSerializer.Serialize(new { value }); + } + + if (policy.Writes) + { + await using var connection = await postgres.OpenConnectionAsync(cancellationToken); + await using var transaction = await connection.BeginTransactionAsync(cancellationToken); + await Write(connection, transaction, cancellationToken); + await transaction.CommitAsync(cancellationToken); + } + else + { + var outcome = await DarlingAgentActionGate.RunAsync( + policy, postgres, store, logger, ToolName, Kind, agent_action, + new AgentActionTarget(resolved.ServerId, resolved.ServerName, wire, database), + (_, _, context, _) => Task.FromResult( + action is not null && !clear ? AgentKindRule(entry, value!.Value) : null), + ReadBefore, Write, cancellationToken); + if (outcome.Refusal is { } refusal) + return DarlingAgentActionGate.RefusalAnswer(refusal); + } + + return JsonSerializer.Serialize(new + { + status = clear ? "cleared" : "ok", + server = resolved.ServerName, + server_id = resolved.ServerId, + key = wire, + database = database, + value = clear ? (int?)null : value, + previous_override = applied.Previous is { } p ? (int?)(int)p : null, + set_by = applied.SetBy, + request_id = applied.RequestId, + message = clear + ? "The override is removed; the global value applies to this server again." + : "The override is stored. The service applies it within its reload interval; the global value is unchanged." + }, McpHelpers.JsonOptions); + } + catch (PostgresException ex) when (ex.SqlState == PostgresErrorCodes.InsufficientPrivilege) + { + return McpHelpers.Status("precondition", PreconditionMessage); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + return McpHelpers.FormatError(ToolName, ex); + } + } + + private sealed class Applied + { + internal double? Previous; + internal string? SetBy; + internal Guid? RequestId; + } + + /// + /// Check 7, the kind rule: an agent never turns an alert off by a threshold. A value no reading can reach (a percent bar at + /// 100 for CPU and tempdb, or the integer ceiling) is a mute by another name, so the agent is told to mute instead. + /// + internal static AgentActionRefusal? AgentKindRule(DarlingThresholdKeys.Entry entry, int value) + { + var unreachable = value == int.MaxValue + || (value == 100 && (entry.Wire == "cpu.threshold_percent" || entry.Wire == "tempdb_space.threshold_percent")); + return unreachable + ? new AgentActionRefusal("threshold_disables_alert", + $"{value.ToString(CultureInfo.InvariantCulture)} on '{entry.Wire}' would turn the alert off. A threshold override only moves the bar; to silence an alert, create a mute rule.") + : null; + } + + private static string Refused(string reason, string message) => + JsonSerializer.Serialize(new { status = "refused", reason, message }, McpHelpers.JsonOptions); + + /// True when the connected role may write the overrides table. A missing grant is the BYO store before the + /// provisioning script re-run (#5920), which the tool names rather than answering with a raw permission error. + internal static async Task HoldsGrantAsync(NpgsqlDataSource postgres, CancellationToken cancellationToken) + { + await using var command = postgres.CreateCommand( + "SELECT COALESCE(has_table_privilege(current_user, to_regclass('" + Table + "'), 'SELECT') " + + "AND has_table_privilege(current_user, to_regclass('" + Table + "'), 'INSERT') " + + "AND has_table_privilege(current_user, to_regclass('" + Table + "'), 'UPDATE') " + + "AND has_table_privilege(current_user, to_regclass('" + Table + "'), 'DELETE'), false)"); + command.CommandTimeout = McpCommandDeadlines.ReadSeconds; + return await command.ExecuteScalarAsync(cancellationToken) is true; + } + + /// + /// The get_alert_settings view for one server (#5916): every tunable threshold with its global value, the value in + /// effect for this server and where that value comes from (global or server_override), and the database + /// overrides the server holds. A read of the overrides table, so it answers what the service will apply after its next reload. + /// + internal static async Task BuildServerViewAsync( + NpgsqlDataSource postgres, DarlingAlertReader.AlertSettingsReadRow s, int serverId, string serverName, + CancellationToken cancellationToken) + { + var rows = new System.Collections.Generic.List<(string? Database, string Key, double Value, string SetBy, Guid? RequestId, DateTime UpdatedAt)>(); + string? note = null; + try + { + await using var command = postgres.CreateCommand( + "SELECT database_name, threshold_key, value, set_by, request_id, updated_at FROM " + Table + " WHERE server_id = $1"); + command.CommandTimeout = McpCommandDeadlines.ReadSeconds; + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Integer, Value = serverId }); + await using var reader = await command.ExecuteReaderAsync(cancellationToken); + while (await reader.ReadAsync(cancellationToken)) + { + rows.Add((reader.IsDBNull(0) ? null : reader.GetString(0), reader.GetString(1), reader.GetDouble(2), reader.GetString(3), + reader.IsDBNull(4) ? null : reader.GetGuid(4), reader.GetDateTime(5))); + } + } + catch (PostgresException ex) when (ex.SqlState == PostgresErrorCodes.InsufficientPrivilege) + { + note = PreconditionMessage; + } + + var thresholds = new System.Collections.Generic.List(); + foreach (var entry in DarlingThresholdKeys.Entries) + { + var global = GlobalValue(s, entry.Wire); + double? server = null; + foreach (var r in rows) + { + if (r.Database is null && r.Key == entry.Wire) + server = r.Value; + } + + thresholds.Add(new + { + key = entry.Wire, + global_value = global, + effective_value = server ?? global, + source = server is null ? "global" : "server_override", + database_scope = entry.Scope == PerformanceMonitor.Alerting.AlertThresholdScope.ServerAndDatabase + }); + } + + var databaseOverrides = new System.Collections.Generic.List(); + var serverOverrides = new System.Collections.Generic.List(); + foreach (var r in rows) + { + var row = new + { + key = r.Key, database = r.Database, value = r.Value, set_by = r.SetBy, request_id = r.RequestId, + updated_at = r.UpdatedAt.ToString("o", CultureInfo.InvariantCulture) + }; + (r.Database is null ? serverOverrides : databaseOverrides).Add(row); + } + + return new + { + server = serverName, + server_id = serverId, + note = note ?? "effective_value is the global value with this server's override applied; a database override (database_overrides) applies to that database only. Set one with set_alert_threshold_override.", + thresholds, + server_overrides = serverOverrides, + database_overrides = databaseOverrides + }; + } + + private static int GlobalValue(DarlingAlertReader.AlertSettingsReadRow s, string wire) => wire switch + { + "cpu.threshold_percent" => s.CpuThresholdPercent, + "blocking.count_threshold" => s.BlockingCountThreshold, + "blocking.wait_threshold_seconds" => s.BlockingWaitSecondsThreshold, + "deadlocks.count_threshold" => s.DeadlockCountThreshold, + "runnable_queue.warning_per_scheduler" => s.RunnableQueueWarningPerScheduler, + "runnable_queue.critical_per_scheduler" => s.RunnableQueueCriticalPerScheduler, + "client_timeouts.warning_per_minute" => s.ClientTimeoutsWarningPerMinute, + "client_timeouts.critical_per_minute" => s.ClientTimeoutsCriticalPerMinute, + "long_running_query.threshold_minutes" => s.LongRunningQueryThresholdMinutes, + "tempdb_space.threshold_percent" => s.TempDbSpaceThresholdPercent, + "low_disk.threshold_percent" => s.LowDiskThresholdPercent, + "low_disk.threshold_gb" => s.LowDiskThresholdGb, + "low_disk.critical_free_percent" => s.DiskCriticalFreePercent, + "low_disk.critical_free_gb" => s.DiskCriticalFreeGb, + "pvs.threshold_percent" => s.PvsThresholdPercent, + "pvs.floor_gb" => s.PvsFloorGb, + "file_growth.rise_mb" => s.FileGrowthRiseMb, + "file_growth.volume_percent" => s.FileGrowthVolumePercent, + "file_growth.lookback_minutes" => s.FileGrowthLookbackMinutes, + "long_running_job.multiplier" => s.LongRunningJobMultiplier, + "failed_job.lookback_minutes" => s.FailedJobLookbackMinutes, + DarlingThresholdKeys.PgBlockingCount => s.PgBlockingCountThreshold, + DarlingThresholdKeys.PgDeadlockCount => s.PgDeadlockCountThreshold, + _ => throw new ArgumentOutOfRangeException(nameof(wire), wire, "Not a tunable threshold."), + }; + + private static NpgsqlCommand Command(NpgsqlConnection c, NpgsqlTransaction t, string sql, int serverId, string? database, string wire) + { + var command = new NpgsqlCommand(sql, c, t) { CommandTimeout = ServiceCommandDeadlines.SerialLoopSeconds }; + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Integer, Value = serverId }); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = (object?)database ?? DBNull.Value }); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = wire }); + return command; + } + + internal static async Task ReadValueAsync(NpgsqlConnection c, NpgsqlTransaction t, int serverId, string? database, string wire, + CancellationToken ct) + { + await using var command = Command(c, t, + "SELECT value FROM " + Table + " WHERE server_id = $1 AND coalesce(database_name, '') = coalesce($2::text, '') AND threshold_key = $3", + serverId, database, wire); + return await command.ExecuteScalarAsync(ct) is double d ? d : null; + } + + private static async Task DeleteAsync(NpgsqlConnection c, NpgsqlTransaction t, int serverId, string? database, string wire, CancellationToken ct) + { + await using var command = Command(c, t, + "DELETE FROM " + Table + " WHERE server_id = $1 AND coalesce(database_name, '') = coalesce($2::text, '') AND threshold_key = $3", + serverId, database, wire); + await command.ExecuteNonQueryAsync(ct); + } + + internal static async Task UpsertAsync(NpgsqlConnection c, NpgsqlTransaction t, int serverId, string? database, string wire, int value, + string setBy, Guid? requestId, CancellationToken ct) + { + await using var command = Command(c, t, + "INSERT INTO " + Table + " (server_id, database_name, threshold_key, value, set_by, request_id) " + + "VALUES ($1, $2, $3, $4, $5, $6) " + + "ON CONFLICT (server_id, (coalesce(database_name, '')), threshold_key) DO UPDATE " + + "SET value = EXCLUDED.value, set_by = EXCLUDED.set_by, request_id = EXCLUDED.request_id, updated_at = now()", + serverId, database, wire); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Double, Value = (double)value }); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = setBy }); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Uuid, Value = (object?)requestId ?? DBNull.Value }); + await command.ExecuteNonQueryAsync(ct); + } +} diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpWritePolicy.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpWritePolicy.cs index 3bf0f0f146..2f0e90ead8 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpWritePolicy.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpWritePolicy.cs @@ -76,6 +76,7 @@ internal static class DarlingMcpWritePolicy "delete_mute_rule", "set_mute_rule_enabled", "update_alert_settings", + "set_alert_threshold_override", "delete_notification_route", "set_notification_route_enabled", "create_custom_alert_rule", @@ -114,7 +115,10 @@ internal static class DarlingMcpWritePolicy /// mute, custom-rule and threshold override tools add to it (#5915, #5916). A name in is /// never offered under the investigations arm, even if listed here. /// - internal static readonly IReadOnlySet AgentActionTools = new HashSet(StringComparer.Ordinal); + internal static readonly IReadOnlySet AgentActionTools = new HashSet(StringComparer.Ordinal) + { + "set_alert_threshold_override", + }; /// /// The write tools never offered to agents (#5912): the one global alert settings row, the analysis finding mute, From a3855ab5f4b2cc90a10e1c96c8079c4e421120ae Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 16 Oct 2026 06:28:15 -0400 Subject: [PATCH 2/5] Threshold overrides: register the tool, update the tool-count and classification pins (#5916) --- .../DarlingMcpAlertToolsTests.cs | 6 ++--- .../DarlingMcpInvestigationToolsTests.cs | 3 ++- .../Darling.Tests/DarlingWebEndpointsTests.cs | 2 +- .../McpServiceParameterDiSeatCensusTests.cs | 4 +-- .../DarlingWebEndpoints.cs | 2 ++ .../Mcp/DarlingMcpAlertTools.cs | 2 +- .../Mcp/DarlingMcpHostService.cs | 1 + .../Mcp/DarlingMcpInstructions.cs | 2 +- .../Mcp/DarlingMcpThresholdOverrideTools.cs | 25 +++++++++++-------- 9 files changed, 28 insertions(+), 19 deletions(-) diff --git a/Darling/Darling.Tests/DarlingMcpAlertToolsTests.cs b/Darling/Darling.Tests/DarlingMcpAlertToolsTests.cs index 277dc94aa5..6d103d300b 100644 --- a/Darling/Darling.Tests/DarlingMcpAlertToolsTests.cs +++ b/Darling/Darling.Tests/DarlingMcpAlertToolsTests.cs @@ -113,10 +113,10 @@ public void ParamContract_MatchesContract(string toolName, string expectedCsv) } [Fact] - public void ParamContract_AlertSettings_TakesNoInputParameters() + public void ParamContract_AlertSettings_TakesOnlyTheOptionalServerName() { - /* Only the injected NpgsqlDataSource, which is not [Description]-decorated — an empty input schema. */ - Assert.Empty(McpParams("get_alert_settings")); + /* The injected NpgsqlDataSource is not [Description]-decorated; the one input is the optional server_name (#5916). */ + Assert.Equal(new[] { "server_name" }, McpParams("get_alert_settings").Select(p => p.Name).ToArray()); } [Fact] diff --git a/Darling/Darling.Tests/DarlingMcpInvestigationToolsTests.cs b/Darling/Darling.Tests/DarlingMcpInvestigationToolsTests.cs index fbacacba99..39b169722c 100644 --- a/Darling/Darling.Tests/DarlingMcpInvestigationToolsTests.cs +++ b/Darling/Darling.Tests/DarlingMcpInvestigationToolsTests.cs @@ -138,7 +138,8 @@ public void Annotations_ClaimAndSubmit_AreNeitherReadOnlyNorDestructive_TheReads Assert.False(byName[name].DestructiveHint); } - Assert.DoesNotContain(byName.Keys, DarlingMcpWritePolicy.IsWrite); + /* The Level 2 action tools (#5912, #5916) are the writes an agent may call under this arm, through the gate. */ + Assert.DoesNotContain(byName.Keys.Where(n => !DarlingMcpWritePolicy.AgentActionTools.Contains(n)), DarlingMcpWritePolicy.IsWrite); } [Fact] diff --git a/Darling/Darling.Tests/DarlingWebEndpointsTests.cs b/Darling/Darling.Tests/DarlingWebEndpointsTests.cs index 40daddd98b..def0f6c313 100644 --- a/Darling/Darling.Tests/DarlingWebEndpointsTests.cs +++ b/Darling/Darling.Tests/DarlingWebEndpointsTests.cs @@ -170,7 +170,7 @@ mute_analysis_finding. wait_for_investigation_requests (#5822) is a long poll, n "analyze_server", "assign_server_tag", "backtest_custom_alert_rule", "claim_investigation_request", "create_custom_alert_rule", "create_custom_view", "create_mute_rule", "create_server_tag", "delete_custom_alert_rule", "delete_custom_view", "delete_mute_rule", "delete_notification_route", "delete_server_tag", "describe_custom_view_catalog", "edit_server", "get_custom_alert_rule", "get_custom_view", "get_tool_guide", "list_custom_alert_rules", "list_custom_alert_templates", "list_custom_views", "mute_analysis_finding", "remove_server", "run_custom_view_panel", - "set_mute_rule_enabled", "set_notification_route_enabled", "submit_investigation_report", "test_custom_alert_rule", "unassign_server_tag", "update_alert_settings", "update_custom_alert_rule", "update_custom_view", "update_mute_rule", "update_server_tag", "validate_custom_alert_rule", "validate_custom_view", + "set_alert_threshold_override", "set_mute_rule_enabled", "set_notification_route_enabled", "submit_investigation_report", "test_custom_alert_rule", "unassign_server_tag", "update_alert_settings", "update_custom_alert_rule", "update_custom_view", "update_mute_rule", "update_server_tag", "validate_custom_alert_rule", "validate_custom_view", "wait_for_investigation_requests", }, DarlingWebEndpoints.ExcludedToolNames.OrderBy(n => n, StringComparer.Ordinal).ToArray()); diff --git a/Darling/Darling.Tests/McpServiceParameterDiSeatCensusTests.cs b/Darling/Darling.Tests/McpServiceParameterDiSeatCensusTests.cs index 0f5479551d..15f03f3921 100644 --- a/Darling/Darling.Tests/McpServiceParameterDiSeatCensusTests.cs +++ b/Darling/Darling.Tests/McpServiceParameterDiSeatCensusTests.cs @@ -96,9 +96,9 @@ DI. InvestigationBriefComposer added deliberately (#5632 Phase A): claim_investi AddSingleton overload (its reader behind it, AddSingleton). ReadAdmission added by #5506 part 3: get_read_latency's live admission counts, registered via the typed-generic AddSingleton overload. InvestigationOpenSignal added by #5822: the long poll waits on - the process's one open signal, registered via the typed-generic AddSingleton overload. */ + the process's one open signal, registered via the typed-generic AddSingleton overload. DarlingMcpServedPolicy added by #5916: set_alert_threshold_override tells its two arms apart by the served policy, registered via the typed-generic AddSingleton overload. */ Assert.Equal( - new List { "AnalyzerConfig", "DarlingAnalysisService", "DarlingMcpAddServersPolicy", "ILogger", "InvestigationBriefComposer", "InvestigationOpenSignal", "MonitoredServerRegistryState", "NpgsqlDataSource", "PostgresConfig", "ReadAdmission", "ReadLatencyRecorder", "StoreHostProfileCache" }, + new List { "AnalyzerConfig", "DarlingAnalysisService", "DarlingMcpAddServersPolicy", "DarlingMcpServedPolicy", "ILogger", "InvestigationBriefComposer", "InvestigationOpenSignal", "MonitoredServerRegistryState", "NpgsqlDataSource", "PostgresConfig", "ReadAdmission", "ReadLatencyRecorder", "StoreHostProfileCache" }, serviceParameterTypeNames); } diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs index 8adc845f0f..562e41b951 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs @@ -137,6 +137,8 @@ internal static (string Name, ReadToolHandler Handler)? TestOnlyExtraDispatchEnt "delete_custom_view", "run_custom_view_panel", "update_alert_settings", + /* #5916: a threshold override is set over MCP; the web grid has its own admin-login route, not a generic read. */ + "set_alert_threshold_override", "set_notification_route_enabled", /* #5632 Phase A: claim and submit write investigation state for an agent over MCP; the web serves the two investigation reads only (/api/read). */ diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs index a34ca621c2..ce9a4a9ea9 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs @@ -302,7 +302,7 @@ internal static async Task GetAlertDetails( } [McpServerTool(Name = "get_alert_settings"), Description("Gets the alert configuration currently in effect: enabled flags, thresholds, cooldowns, delivery mode, and cadences. cooldown_minutes gates whether an alert FIRES; delivery.cooldown_minutes separately bounds the resulting post, per alert fingerprint and, for a re-notification, per metric across every monitored server. Darling: an unseeded store answers status unavailable and darling.json's defaults still govern until it seeds. Lite always answers its live in-memory settings. An empty knob list (e.g. long_running_query's exclusions) means cleared, not a default in force.<>Gets the current alert configuration the service is using: which alerts are enabled and their thresholds (CPU, blocking, deadlocks, poison waits, runnable queue (runnable tasks per scheduler), client timeouts (attentions per minute), long-running queries/jobs, tempdb, low disk, failed jobs, database state, Availability Group health, connection loss), the cooldown, excluded databases, the deadlock/blocking delivery mode and cooldown, the scheduled-analysis cadence, and the fleet-sweep cadence. TWO different cooldowns are reported and they govern different stages: top-level cooldown_minutes gates whether the alert engine FIRES at all, while delivery.cooldown_minutes bounds the resulting Slack/Teams/PagerDuty/webhook/email post twice over: once per alert FINGERPRINT, and once per METRIC across the whole fleet for a RE-notification. The second bound is why one fault on forty servers does not cost forty posts an hour; the servers it holds back are named on the post that does go out, under an 'Other Servers Affected' section. A first notice is never held back by either bound, and PerEvent delivery mode opts out of the per-metric one. A channel going quiet with alerts still in get_alert_history is delivery.cooldown_minutes, not cooldown_minutes. The self_alerts group holds the thresholds for alerts about the MONITOR STORE itself rather than a monitored server — those arrive with Server: 'Monitor Store' by default, or with the store's own peers.storeName label when the operator set that file-only field (a multi-store estate names each store on its own self-alerts), so an alert naming either spelling is tuned here and nowhere else, including Retention Held's warn/critical ratios. Mute rules match the alert row's server spelling, so on a store with storeName set, scope self-alert mutes to that label, not to 'Monitor Store'. The health_bands group is NOT an alert: its two tiers decide what band a server's card, the worst-first ranking and get_fleet_overview's counts read, in deadlocks per HOUR normalised over whatever window was asked for — so the same pair means the same condition on a 1-hour read and a 24-hour one. Tuning deadlocks.count_threshold does not move the band and tuning health_bands does not move the alert. The file_growth group's rise_mb is megabytes per HOUR, averaged over file_growth.lookback_minutes — a rate, not a total for the window: 10240 means 10 GB/hr whether the lookback is 5 minutes or 24 hours, and the engine scales it to the window (a 5-minute lookback asks for 853 MB inside it, a 24-hour one for 240 GB). The fleet_sweep group is NOT an alert family either, and its cadence is a SECOND cadence, separate from the scheduled-analysis one: fleet_sweep.enabled turns the scheduled whole-fleet sweep report on or off, and fleet_sweep.interval_minutes (15–1440, default 60 — hourly) is how often it runs. The alerts_enabled master switch deliberately does not govern sweep production, only delivery: sweeps keep running under alerts_enabled: false — that is when they carry the would-have-paged ledger — so muting the fleet does not blind the report surface. Separately, deadlocks.pg_count_threshold and blocking.pg_count_threshold are the PostgreSQL versions of those two alerts' count gates, reported inside those same groups, and they are deliberately NOT the same numbers as the count_threshold beside them: the two engines count with different instruments (captured deadlock graphs versus deadlocks parsed from the server log; engine-recorded blocked-process reports versus a periodic SAMPLE of pg_stat_activity), and while both engines' cards now band deadlocks through the same health_bands tiers, a PostgreSQL server still has no blocking band to calibrate against. The enabled switch in each group governs BOTH engines; the two thresholds do not move each other. On a store with no PostgreSQL targets both PostgreSQL keys are inert. poison_wait.threshold_ms is RETIRED: since the Poison Wait alert grades ACCUMULATED wait over a ten-minute window, this value is stored and reported for compatibility but consulted by nothing; poison_wait.threshold_ms_note says so beside it on every read, and update_alert_settings accepts it with a warning rather than refusing a round-tripped payload. poison_wait.enabled is the live switch. client_timeouts.enabled is the Client Timeouts alert's switch (OFF by default: no measured normal rate exists yet), and client_timeouts.warning_per_minute and client_timeouts.critical_per_minute are its bars: client attentions (a client cancelled a query or its timeout expired) per minute over the attention collector's last runs (about 3 minutes), as the engine applies them (Warning at least 1, Critical never below Warning). analysis.uncorroborated_route is where a notify-worthy but UNCORROBORATED finding goes — one fact in its chain and no matched co-fire check: 'digest' (the default) keeps it off every paging channel and puts it in the daily Analysis Singles Digest and the web/MCP surfaces, 'page' restores the earlier paging of every notify-worthy finding; a corroborated finding pages under either. It is the EFFECTIVE route, never null, resolved from the knob's two homes with the STORE winning: the settings row's analysis_uncorroborated_route column (V137) when it holds a route, else darling.json's analysis.uncorroboratedRoute (read once at service start), else the shipped 'digest'. analysis.uncorroborated_route_source says which home decided — 'store', 'file' or 'default' — and analysis.uncorroborated_route_note restates the precedence beside it; a store column left NULL (every store the morning after the V137 upgrade, and every fresh seed) reads 'file'. Set it with update_alert_settings or the Viewer's Settings window (Notifications > Automated Analysis): live in the running service within one collection sweep, consulted on the next scheduled-analysis delivery, no restart; send null through update_alert_settings to clear the column and hand the decision back to the file. long_running_query.excluded_program_name_prefixes and long_running_query.excluded_logins are the Long-Running Query alert's OPT-OUT knob: a session whose program_name STARTS WITH an entry of the first list, or whose login_name IS an entry of the second (both case-insensitive, no wildcard grammar), is NOT EVALUATED by that alert at all — not read into the decision, not counted, not fingerprinted — which is the opposite of a mute rule (a mute silences a fire already decided; an exclusion means the session never reaches the decision, and excluding a paging program resolves the open incident). The exclusion is applied in the read ahead of long_running_query.max_results, so an excluded session never consumes a result slot, and the fired alert's card carries Excluded Count / Excluded By Program Prefix / Excluded By Login items saying how many SESSIONS each list removed on that evaluation (a session matching both counts once, under the prefix). The lists ship SEEDED from a 7-day read of one large production store, whose long-running population fell into four classes: (1) SQL Agent job steps — program_name prefix 'SQLAgent - TSQL JobStep', ~460 sessions a week across 11 jobs, medians 35–62 min — the default prefix; (2) the NT AUTHORITY\\SYSTEM and NT AUTHORITY\\NETWORK SERVICE logins — the permanent multi-day CDC-shaped background — the default logins; (3) the application's admin login — deliberately NOT a default, because it runs the job wave but also real ad-hoc long-runners, and the job-step prefix already covers its share; (4) named humans — never excluded, they are what the page is for. The seeds are DEFAULTS: an empty list here means the operator cleared it and that arm excludes nothing. SMTP/webhook delivery credentials are managed separately and are not reported here. delivery.send_resolutions says, per channel (slack, teams, webhook for the generic webhook, email), whether the message that an alert has cleared is also sent to the destinations its fire reached; all four are off by default, and a notification route can override them (get_notification_routes, send_resolutions). The credentials themselves are not reported — configure them in the standalone Darling Viewer app's Settings window (Notifications section), which connects to this store (including remotely, not just localhost) rather than requiring desktop access to this specific box.")] public static async Task GetAlertSettings( NpgsqlDataSource postgres, - [Description("Optional (#5916): a server name. Adds server_view: every tunable threshold with its global value, the value in effect for that server and its source (global or server_override), and that server's database overrides.")] string? server_name = null, + [Description("Optional: a server name. Adds server_view: each tunable threshold with its global value, effective value and source, plus that server's database overrides.")] string? server_name = null, CancellationToken cancellationToken = default) { try diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs index a01fe673ef..f1cd19fa7e 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs @@ -1464,6 +1464,7 @@ classes above. */ /* #5632 Phase A: the Level 1 investigation tools. The two reads are ordinary reads; claim and submit are served only under DarlingMcpWritePolicy's investigations arm (a token, mcp.allowWrites off). */ .WithGeminiCompatibleTools() + .WithGeminiCompatibleTools() .WithGeminiCompatibleTools() .WithGeminiCompatibleTools() /* The cross-server fleet overview — get_fleet_overview (#1562) — the roll-up only the central diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs index 937d5ca3b9..39c6f902eb 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs @@ -135,7 +135,7 @@ internal static string ToolCensus(IReadOnlyCollection catalog, IReadOnly /// are identical, so a new tool moves this or fails there. /// private const string CatalogCensus = - "This server exposes 188 tools. 95 are the same names Lite exposes. The remaining 93 are unique to Darling: " + + "This server exposes 189 tools. 95 are the same names Lite exposes. The remaining 94 are unique to Darling: " + "thirty-five are the PostgreSQL reads (`get_pg_*`), plus Custom Views, custom-alert-rule, alert-tuning, server-tag, " + "onboarding and investigation-wait tools, and central-store-only reads (`get_fleet_overview`, `get_ag_health` — Availability Groups — " + "`get_sweep_reports`, `get_store_metrics`, `get_store_log`). `get_blocking` = Lite's `get_blocked_process_reports`."; diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs index e025b9f5ba..82e6d4d655 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs @@ -26,7 +26,12 @@ namespace PerformanceMonitor.Darling.Service.Mcp; /// What the MCP host serves, as a service parameter a tool can take (#5916): the one /// the host started with. A Level 2 action tool needs it to tell its two arms apart. /// -internal sealed record DarlingMcpServedPolicy(DarlingMcpWritePolicy.Effective Policy); +public sealed class DarlingMcpServedPolicy +{ + internal DarlingMcpServedPolicy(DarlingMcpWritePolicy.Effective policy) => Policy = policy; + + internal DarlingMcpWritePolicy.Effective Policy { get; } +} /// /// set_alert_threshold_override (#5916): set or clear a threshold for one server, or one database on it, without @@ -36,7 +41,7 @@ internal sealed record DarlingMcpServedPolicy(DarlingMcpWritePolicy.Effective Po /// agent and the request. An agent changes a threshold by override only; update_alert_settings is never offered to it. /// [McpServerToolType] -internal static class DarlingMcpThresholdOverrideTools +public sealed class DarlingMcpThresholdOverrideTools { internal const string ToolName = "set_alert_threshold_override"; internal const string Kind = "threshold_override"; @@ -327,36 +332,36 @@ internal static async Task BuildServerViewAsync( _ => throw new ArgumentOutOfRangeException(nameof(wire), wire, "Not a tunable threshold."), }; - private static NpgsqlCommand Command(NpgsqlConnection c, NpgsqlTransaction t, string sql, int serverId, string? database, string wire) + private static NpgsqlCommand Command(NpgsqlConnection connection, NpgsqlTransaction transaction, string sql, int serverId, string? database, string wire) { - var command = new NpgsqlCommand(sql, c, t) { CommandTimeout = ServiceCommandDeadlines.SerialLoopSeconds }; + var command = new NpgsqlCommand(sql, connection) { Transaction = transaction, CommandTimeout = McpCommandDeadlines.ReadSeconds }; command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Integer, Value = serverId }); command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = (object?)database ?? DBNull.Value }); command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = wire }); return command; } - internal static async Task ReadValueAsync(NpgsqlConnection c, NpgsqlTransaction t, int serverId, string? database, string wire, + internal static async Task ReadValueAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, int serverId, string? database, string wire, CancellationToken ct) { - await using var command = Command(c, t, + await using var command = Command(connection, transaction, "SELECT value FROM " + Table + " WHERE server_id = $1 AND coalesce(database_name, '') = coalesce($2::text, '') AND threshold_key = $3", serverId, database, wire); return await command.ExecuteScalarAsync(ct) is double d ? d : null; } - private static async Task DeleteAsync(NpgsqlConnection c, NpgsqlTransaction t, int serverId, string? database, string wire, CancellationToken ct) + private static async Task DeleteAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, int serverId, string? database, string wire, CancellationToken ct) { - await using var command = Command(c, t, + await using var command = Command(connection, transaction, "DELETE FROM " + Table + " WHERE server_id = $1 AND coalesce(database_name, '') = coalesce($2::text, '') AND threshold_key = $3", serverId, database, wire); await command.ExecuteNonQueryAsync(ct); } - internal static async Task UpsertAsync(NpgsqlConnection c, NpgsqlTransaction t, int serverId, string? database, string wire, int value, + internal static async Task UpsertAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, int serverId, string? database, string wire, int value, string setBy, Guid? requestId, CancellationToken ct) { - await using var command = Command(c, t, + await using var command = Command(connection, transaction, "INSERT INTO " + Table + " (server_id, database_name, threshold_key, value, set_by, request_id) " + "VALUES ($1, $2, $3, $4, $5, $6) " + "ON CONFLICT (server_id, (coalesce(database_name, '')), threshold_key) DO UPDATE " + From b1a8078b1098f4dbcc1369709f17931983469f5f Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 16 Oct 2026 06:33:48 -0400 Subject: [PATCH 3/5] Threshold overrides: behaviour tests, live tests, README section (#5916) --- .../ThresholdOverrideLiveTests.cs | 229 ++++++++++++++++++ .../Darling.Tests/ThresholdOverrideTests.cs | 186 ++++++++++++++ Darling/README.md | 12 + 3 files changed, 427 insertions(+) create mode 100644 Darling/Darling.Tests/ThresholdOverrideLiveTests.cs create mode 100644 Darling/Darling.Tests/ThresholdOverrideTests.cs diff --git a/Darling/Darling.Tests/ThresholdOverrideLiveTests.cs b/Darling/Darling.Tests/ThresholdOverrideLiveTests.cs new file mode 100644 index 0000000000..b8a9574cc7 --- /dev/null +++ b/Darling/Darling.Tests/ThresholdOverrideLiveTests.cs @@ -0,0 +1,229 @@ +/* + * 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.Text.Json.Nodes; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging.Abstractions; +using Npgsql; +using PerformanceMonitor.Alerting; +using PerformanceMonitor.Common; +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 through ScratchPostgres and never touches another test's rows, + so the class is deliberately NOT [Collection("live-postgres")]. */ + +/// +/// #5916 against a live store: set_alert_threshold_override under each arm (the writes arm ignores +/// agent_action; the investigations arm requires it, keeps to the request's server and records the agent and the +/// request), every refused key class leaves the table empty, the loader reads what the tool wrote, a role without the grant +/// gets precondition naming provision-roles.sql, and get_alert_settings with server_name shows the +/// effective value and its source. +/// +public sealed class ThresholdOverrideLiveTests +{ + private static readonly DateTime T0 = new(2026, 10, 16, 12, 0, 0, DateTimeKind.Utc); + private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + + private static readonly DarlingMcpWritePolicy.Effective WritesArm = new(true, false); + private static readonly DarlingMcpWritePolicy.Effective AgentArm = new(false, false, Investigations: true); + + private static async Task RunAsync(Func body) + { + var baseConnectionString = ConnectionString; + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the threshold override live tests."); + + var ct = TestContext.Current.CancellationToken; + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + await ExecAsync(connection, "INSERT INTO config.config_monitored_servers (server_id, name, host) VALUES (7, 'srv-7', 'h7'), (8, 'srv-8', 'h8')", ct); + await ExecAsync(connection, "INSERT INTO collect.servers (server_id, server_name, display_name, is_enabled, sql_major_version, created_date, modified_date) " + + "VALUES (7, 'srv-7', 'srv-7', TRUE, 15, now(), now()), (8, 'srv-8', 'srv-8', TRUE, 15, now(), now())", ct); + + var builder = new NpgsqlConnectionStringBuilder(scratch.ConnectionString) { SearchPath = PgSchemaGenerator.SearchPath }; + await using var dataSource = NpgsqlDataSource.Create(builder.ConnectionString); + var store = new InvestigationRequestStore(dataSource, 30, () => T0); + await body(dataSource, connection, store, ct); + } + + private static Task Set(NpgsqlDataSource ds, InvestigationRequestStore store, DarlingMcpWritePolicy.Effective policy, + string server, string key, int? value, string? database = null, bool clear = false, string? setBy = null, string? agentAction = null) => + DarlingMcpThresholdOverrideTools.SetAsync(ds, policy, store, NullLogger.Instance, server, key, value, database, clear, setBy, agentAction, + TestContext.Current.CancellationToken); + + private static string Status(string json) => JsonNode.Parse(json)!["status"]!.GetValue(); + + private static string Reason(string json) => JsonNode.Parse(json)!["reason"]!.GetValue(); + + [Fact] + public async Task WritesArm_SetsUpdatesAndClears_IgnoresAgentAction_AndTheLoaderReadsIt() + { + await RunAsync(async (ds, connection, store, ct) => + { + /* agent_action is ignored under the writes arm: a garbage value does not stop the write. */ + var set = await Set(ds, store, WritesArm, "srv-7", "cpu.threshold_percent", 95, setBy: "erik", agentAction: "not json"); + Assert.Equal("ok", Status(set)); + Assert.Equal("95", await ScalarAsync(connection, "SELECT value FROM config.config_threshold_overrides WHERE server_id = 7 AND threshold_key = 'cpu.threshold_percent'", ct)); + Assert.Equal("erik", await ScalarAsync(connection, "SELECT set_by FROM config.config_threshold_overrides WHERE server_id = 7", ct)); + Assert.Equal("", await ScalarAsync(connection, "SELECT request_id FROM config.config_threshold_overrides WHERE server_id = 7", ct)); + + Assert.Equal("ok", Status(await Set(ds, store, WritesArm, "srv-7", "cpu.threshold_percent", 90))); + Assert.Equal("1", await ScalarAsync(connection, "SELECT count(*) FROM config.config_threshold_overrides", ct)); + Assert.Equal("90", await ScalarAsync(connection, "SELECT value FROM config.config_threshold_overrides", ct)); + Assert.Equal("mcp", await ScalarAsync(connection, "SELECT set_by FROM config.config_threshold_overrides", ct)); + + /* A database override is its own row, and the two PostgreSQL count thresholds are keys too. */ + Assert.Equal("ok", Status(await Set(ds, store, WritesArm, "srv-7", "pvs.threshold_percent", 70, database: "shop"))); + Assert.Equal("ok", Status(await Set(ds, store, WritesArm, "srv-8", "blocking.pg_count_threshold", 4))); + Assert.Equal("ok", Status(await Set(ds, store, WritesArm, "srv-8", "deadlocks.pg_count_threshold", 6))); + + var loader = new DarlingThresholdOverrides(); + await loader.LoadAsync(ds, ct); + Assert.Equal(4, loader.Count); + Assert.Equal(90, loader.Resolve(AlertThresholdKey.CpuThresholdPercent, "7", null, 80)); + Assert.Equal(80, loader.Resolve(AlertThresholdKey.CpuThresholdPercent, "8", null, 80)); + Assert.Equal(70, loader.Resolve(AlertThresholdKey.PvsThresholdPercent, "7", "shop", 40)); + Assert.Equal(40, loader.Resolve(AlertThresholdKey.PvsThresholdPercent, "7", "ledger", 40)); + Assert.Equal(4, loader.ResolveWire(DarlingThresholdKeys.PgBlockingCount, "8", null, 20)); + Assert.Equal(20, loader.ResolveWire(DarlingThresholdKeys.PgBlockingCount, "7", null, 20)); + Assert.Equal(6, loader.ResolveWire(DarlingThresholdKeys.PgDeadlockCount, "8", null, 20)); + + var cleared = await Set(ds, store, WritesArm, "srv-7", "cpu.threshold_percent", null, clear: true); + Assert.Equal("cleared", Status(cleared)); + await loader.LoadAsync(ds, ct); + Assert.Equal(80, loader.Resolve(AlertThresholdKey.CpuThresholdPercent, "7", null, 80)); + }); + } + + [Theory] + [InlineData("no.such.key", 5, null, "not_in_registry")] + [InlineData("self_alerts.collection_stale_minutes", 30, null, "not_tunable")] + [InlineData("cpu.enabled", 1, null, "not_tunable")] + [InlineData("cooldown_minutes", 10, null, "not_tunable")] + [InlineData("route.agent_level", 2, null, "not_tunable")] + [InlineData("cpu.threshold_percent", 101, null, "out_of_bounds")] + [InlineData("cpu.threshold_percent", 90, "shop", "database_scope")] + public async Task ARefusedKeyOrValue_LeavesTheTableEmpty(string key, int value, string? database, string reason) + { + await RunAsync(async (ds, connection, store, ct) => + { + var answer = await Set(ds, store, WritesArm, "srv-7", key, value, database); + Assert.Equal("refused", Status(answer)); + Assert.Equal(reason, Reason(answer)); + Assert.Equal("0", await ScalarAsync(connection, "SELECT count(*) FROM config.config_threshold_overrides", ct)); + }); + } + + [Fact] + public async Task InvestigationsArm_RequiresAgentAction_KeepsToTheRequestsServer_AndRecordsTheAgentAndTheRequest() + { + await RunAsync(async (ds, connection, store, ct) => + { + await ExecAsync(connection, "INSERT INTO config.config_notification_routes (metric_match, agent_level) VALUES ('Blocking', 2)", ct); + var opened = await store.OpenOrJoinAsync( + new InvestigationAlert(1, 7, "srv-7", "blocking", "Blocking", T0, Array.Empty(), null, 20, 100, + LeaseMinutes: 15, ExpiryMinutes: 240) { AutonomyLevel = 2 }, ct); + var request = opened.RequestId!.Value; + var claim = (await store.ClaimAsync(request, "agent-a", ct)).ClaimId!.Value; + var arg = AgentActionArgumentTests.Arg(request.ToString(), claim.ToString()); + + /* Required: no agent_action, no change. */ + Assert.Equal(AgentActionCodes.Required, Reason(await Set(ds, store, AgentArm, "srv-7", "cpu.threshold_percent", 90))); + Assert.Equal("0", await ScalarAsync(connection, "SELECT count(*) FROM config.config_threshold_overrides", ct)); + + /* The request's server: the row names the agent (not the set_by argument) and the request. */ + var ok = await Set(ds, store, AgentArm, "srv-7", "cpu.threshold_percent", 90, setBy: "someone-else", agentAction: arg); + Assert.Equal("ok", Status(ok)); + Assert.Equal("agent:agent-a", await ScalarAsync(connection, "SELECT set_by FROM config.config_threshold_overrides", ct)); + Assert.Equal(request.ToString(), await ScalarAsync(connection, "SELECT request_id FROM config.config_threshold_overrides", ct)); + Assert.Equal("1", await ScalarAsync(connection, $"SELECT count(*) FROM config.investigation_journal WHERE request_id = '{request}' AND event = 'action_done'", ct)); + + /* Another server is refused, and so is a value that would turn the alert off. */ + Assert.Equal(AgentActionCodes.OtherServer, Reason(await Set(ds, store, AgentArm, "srv-8", "cpu.threshold_percent", 90, agentAction: arg))); + Assert.Equal("threshold_disables_alert", Reason(await Set(ds, store, AgentArm, "srv-7", "cpu.threshold_percent", 100, agentAction: arg))); + Assert.Equal("1", await ScalarAsync(connection, "SELECT count(*) FROM config.config_threshold_overrides", ct)); + Assert.Equal("90", await ScalarAsync(connection, "SELECT value FROM config.config_threshold_overrides", ct)); + }); + } + + [Fact] + public async Task WithoutTheGrant_TheToolAnswersPrecondition_AndNamesProvisionRolesSql() + { + await RunAsync(async (ds, connection, store, ct) => + { + var role = "ovr_noprov_" + Guid.NewGuid().ToString("N")[..8]; + await ExecAsync(connection, $"CREATE ROLE {role} LOGIN", ct); + try + { + await ExecAsync(connection, $"GRANT USAGE ON SCHEMA collect, config TO {role}; GRANT SELECT ON collect.servers TO {role};", ct); + var builder = new NpgsqlConnectionStringBuilder(ds.ConnectionString) { Username = role, SearchPath = PgSchemaGenerator.SearchPath }; + await using var restricted = NpgsqlDataSource.Create(builder.ConnectionString); + + var answer = await Set(restricted, store, WritesArm, "srv-7", "cpu.threshold_percent", 90); + Assert.Equal("precondition", Status(answer)); + Assert.Contains("provision-roles.sql", JsonNode.Parse(answer)!["message"]!.GetValue(), StringComparison.Ordinal); + Assert.Equal("0", await ScalarAsync(connection, "SELECT count(*) FROM config.config_threshold_overrides", ct)); + } + finally + { + await ExecAsync(connection, $"REVOKE ALL ON ALL TABLES IN SCHEMA collect, config FROM {role}; REVOKE ALL ON SCHEMA collect, config FROM {role}; DROP ROLE {role}", ct); + } + }); + } + + [Fact] + public async Task GetAlertSettings_WithServerName_ShowsTheEffectiveValueAndWhereItComesFrom() + { + await RunAsync(async (ds, connection, store, ct) => + { + await ExecAsync(connection, "INSERT INTO config.config_alert_settings (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); + Assert.Equal("ok", Status(await Set(ds, store, WritesArm, "srv-7", "cpu.threshold_percent", 91))); + Assert.Equal("ok", Status(await Set(ds, store, WritesArm, "srv-7", "pvs.threshold_percent", 66, database: "shop"))); + + var json = JsonNode.Parse(await DarlingMcpAlertTools.GetAlertSettings(ds, "srv-7", ct))!; + var view = json["server_view"]!; + JsonNode? Row(string key) => System.Linq.Enumerable.First(view["thresholds"]!.AsArray(), n => n!["key"]!.GetValue() == key); + Assert.Equal(91, Row("cpu.threshold_percent")!["effective_value"]!.GetValue()); + Assert.Equal("server_override", Row("cpu.threshold_percent")!["source"]!.GetValue()); + Assert.Equal("global", Row("blocking.count_threshold")!["source"]!.GetValue()); + Assert.Equal(66, view["database_overrides"]![0]!["value"]!.GetValue()); + Assert.Equal("shop", view["database_overrides"]![0]!["database"]!.GetValue()); + + /* Another server shows no override and the global value. */ + var other = JsonNode.Parse(await DarlingMcpAlertTools.GetAlertSettings(ds, "srv-8", ct))!["server_view"]!; + Assert.Equal("global", System.Linq.Enumerable.First(other["thresholds"]!.AsArray(), n => n!["key"]!.GetValue() == "cpu.threshold_percent")!["source"]!.GetValue()); + Assert.Empty(other["server_overrides"]!.AsArray()); + + /* No server_name: the payload is the one it always was. */ + Assert.Null(JsonNode.Parse(await DarlingMcpAlertTools.GetAlertSettings(ds, null, ct))!["server_view"]); + }); + } + + 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 ScalarAsync(NpgsqlConnection connection, string sql, CancellationToken ct) + { + await using var command = new NpgsqlCommand(sql, connection); + return Convert.ToString(await command.ExecuteScalarAsync(ct), CultureInfo.InvariantCulture) ?? ""; + } +} diff --git a/Darling/Darling.Tests/ThresholdOverrideTests.cs b/Darling/Darling.Tests/ThresholdOverrideTests.cs new file mode 100644 index 0000000000..9e2d2020ee --- /dev/null +++ b/Darling/Darling.Tests/ThresholdOverrideTests.cs @@ -0,0 +1,186 @@ +/* + * 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 PerformanceMonitor.Alerting; +using PerformanceMonitor.Darling.Service; +using PerformanceMonitor.Darling.Service.Mcp; +using Xunit; +using static Darling.Tests.AlertEngineTests; + +namespace Darling.Tests; + +/// +/// #5916, no store: the keys an override may take, the bounds, the loader's resolution (a server's override moves that +/// server and leaves another alone; a database override applies to that database only), the engine moved by the loader, +/// and the two PostgreSQL count thresholds following it. The store paths are in . +/// +public sealed class ThresholdOverrideTests +{ + [Theory] + [InlineData("no.such.key", "not_in_registry")] + [InlineData("cpu.threshold", "not_in_registry")] + [InlineData("self_alerts.collection_stale_minutes", "not_tunable")] + [InlineData("self_alerts.disk_free_warn_percent", "not_tunable")] + [InlineData("cpu.enabled", "not_tunable")] + [InlineData("enabled", "not_tunable")] + [InlineData("cooldown_minutes", "not_tunable")] + [InlineData("delivery.cooldown_minutes", "not_tunable")] + [InlineData("route.metric_match", "not_tunable")] + [InlineData("", "key_required")] + public void AKeyThatIsNotATunableThreshold_IsRefused_WithItsClass(string key, string reason) + { + var refusal = DarlingThresholdKeys.RefusalFor(key); + Assert.NotNull(refusal); + Assert.Equal(reason, refusal!.Value.Reason); + } + + [Fact] + public void EveryRegistryKey_AndBothPostgreSqlCountThresholds_AreTunable_AndNothingElseIs() + { + foreach (var key in Enum.GetValues()) + { + var entry = DarlingThresholdKeys.For(key); + Assert.Null(DarlingThresholdKeys.RefusalFor(entry.Wire)); + Assert.Equal(AlertThresholdKeys.ScopeOf(key), entry.Scope); + } + + Assert.Null(DarlingThresholdKeys.RefusalFor(DarlingThresholdKeys.PgBlockingCount)); + Assert.Null(DarlingThresholdKeys.RefusalFor(DarlingThresholdKeys.PgDeadlockCount)); + Assert.Equal(Enum.GetValues().Length + 2, DarlingThresholdKeys.Entries.Count); + } + + [Theory] + [InlineData("cpu.threshold_percent", 0)] + [InlineData("cpu.threshold_percent", 101)] + [InlineData("long_running_job.multiplier", 1)] + [InlineData("long_running_job.multiplier", 21)] + [InlineData("file_growth.lookback_minutes", 4)] + [InlineData("blocking.pg_count_threshold", 0)] + [InlineData("deadlocks.pg_count_threshold", -3)] + public void AValueOutsideTheGlobalSettingsBounds_IsRefused(string key, int value) + { + Assert.True(DarlingThresholdKeys.TryGet(key, out var entry)); + Assert.Equal("out_of_bounds", DarlingThresholdKeys.ValidateValue(entry, value, null)!.Value.Reason); + } + + [Fact] + public void ADatabaseNameIsAcceptedOnlyOnADatabaseScopedKey() + { + Assert.True(DarlingThresholdKeys.TryGet("pvs.threshold_percent", out var pvs)); + Assert.Null(DarlingThresholdKeys.ValidateValue(pvs, 60, "shop")); + Assert.True(DarlingThresholdKeys.TryGet("cpu.threshold_percent", out var cpu)); + Assert.Equal("database_scope", DarlingThresholdKeys.ValidateValue(cpu, 60, "shop")!.Value.Reason); + Assert.True(DarlingThresholdKeys.TryGet(DarlingThresholdKeys.PgBlockingCount, out var pg)); + Assert.Equal("database_scope", DarlingThresholdKeys.ValidateValue(pg, 5, "shop")!.Value.Reason); + } + + [Fact] + public void AnOverride_MovesItsServer_AndLeavesTheOther_AndADatabaseOverrideAppliesToThatDatabaseOnly() + { + var overrides = new DarlingThresholdOverrides(); + overrides.Replace(new (int, string?, string, double)[] + { + (101, null, "cpu.threshold_percent", 95), + (101, "shop", "pvs.threshold_percent", 70), + (101, null, "file_growth.rise_mb", 500), + }); + + Assert.Equal(95, overrides.Resolve(AlertThresholdKey.CpuThresholdPercent, "101", null, 80)); + Assert.Equal(80, overrides.Resolve(AlertThresholdKey.CpuThresholdPercent, "202", null, 80)); + + /* Database scope: the database that holds the row, and no other database, and not the server's other keys. */ + Assert.Equal(70, overrides.Resolve(AlertThresholdKey.PvsThresholdPercent, "101", "shop", 40)); + Assert.Equal(40, overrides.Resolve(AlertThresholdKey.PvsThresholdPercent, "101", "ledger", 40)); + Assert.Equal(40, overrides.Resolve(AlertThresholdKey.PvsThresholdPercent, "101", null, 40)); + Assert.Equal(40, overrides.Resolve(AlertThresholdKey.PvsThresholdPercent, "202", "shop", 40)); + + /* A server-level override on a database-scoped key covers its databases that hold no row of their own. */ + Assert.Equal(500, overrides.Resolve(AlertThresholdKey.FileGrowthRiseMb, "101", "shop", 1024)); + Assert.Equal(500, overrides.Resolve(AlertThresholdKey.FileGrowthRiseMb, "101", null, 1024)); + + /* A server key that is not a store id reads the global value. */ + Assert.Equal(80, overrides.Resolve(AlertThresholdKey.CpuThresholdPercent, "not-an-id", null, 80)); + } + + [Fact] + public void ARowTheGlobalSettingWouldRefuse_IsNotApplied() + { + var overrides = new DarlingThresholdOverrides(); + overrides.Replace(new (int, string?, string, double)[] + { + (101, null, "cpu.threshold_percent", 500), + (101, null, "cpu.threshold_percent_typo", 90), + (101, null, "self_alerts.collection_stale_minutes", 10), + (101, null, "blocking.count_threshold", 2.5), + (101, "shop", "cpu.threshold_percent", 90), + }); + + Assert.Equal(0, overrides.Count); + Assert.Equal(80, overrides.Resolve(AlertThresholdKey.CpuThresholdPercent, "101", null, 80)); + } + + /// The loader behind the engine: a blocking override on server 101 lifts that server's bar, server 202 keeps the global one. + [Fact] + public async Task TheLoader_MovesOneServersAlert_ThroughTheEngine_AndLeavesTheOther() + { + var overrides = new DarlingThresholdOverrides(); + overrides.Replace(new (int, string?, string, double)[] { (101, null, "blocking.count_threshold", 5) }); + + var h = new Harness { Overrides = overrides }; + h.Settings.BlockingEnabled = true; + h.Adapter.Blocking.Add(BlockingRow(55)); + h.Adapter.Blocking.Add(BlockingRow(56)); + var engine = h.Build(); + + await engine.EvaluateServerAsync(Harness.Snapshot() with { ServerKey = "101", ServerName = "SRV-A" }); + Assert.DoesNotContain(h.Deliverer.Outcomes, o => o.MetricName == "Blocking Detected"); + + await engine.EvaluateServerAsync(Harness.Snapshot() with { ServerKey = "202", ServerName = "SRV-B" }); + var fired = Assert.Single(h.Deliverer.Outcomes, o => o.MetricName == "Blocking Detected"); + Assert.Equal("202", fired.ServerKey); + } + + /// + /// The two PostgreSQL count checks read their bar through and hand it + /// to : the same count fires on the server whose bar was lowered and not on another. + /// + [Theory] + [InlineData(DarlingThresholdKeys.PgDeadlockCount)] + [InlineData(DarlingThresholdKeys.PgBlockingCount)] + public void EachPostgreSqlCountThreshold_FollowsItsServersOverride(string wire) + { + var overrides = new DarlingThresholdOverrides(); + overrides.Replace(new (int, string?, string, double)[] { (7, null, wire, 3) }); + + const int globalBar = 10; + const int count = 5; + bool Fires(string serverKey) => RollingCountAlertGate.Evaluate( + count, overrides.ResolveWire(wire, serverKey, null, globalBar), watermark: 0, cooldownElapsed: true, suppressed: false).Fire; + + Assert.True(Fires("7")); + Assert.False(Fires("8")); + } + + [Fact] + public void TheToolCannotLowerAnAgentsBarToNothing_AndLeavesOrdinaryValuesAlone() + { + Assert.True(DarlingThresholdKeys.TryGet("cpu.threshold_percent", out var cpu)); + Assert.Equal("threshold_disables_alert", DarlingMcpThresholdOverrideTools.AgentKindRule(cpu, 100)!.Reason); + Assert.Null(DarlingMcpThresholdOverrideTools.AgentKindRule(cpu, 95)); + Assert.True(DarlingThresholdKeys.TryGet("blocking.count_threshold", out var blocking)); + Assert.Equal("threshold_disables_alert", DarlingMcpThresholdOverrideTools.AgentKindRule(blocking, int.MaxValue)!.Reason); + Assert.Contains("set_alert_threshold_override", DarlingMcpWritePolicy.AgentActionTools); + Assert.DoesNotContain("update_alert_settings", DarlingMcpWritePolicy.AgentActionTools); + Assert.True(DarlingMcpWritePolicy.Effective.Off.Offers("set_alert_threshold_override") is false); + Assert.True(new DarlingMcpWritePolicy.Effective(true, false).Offers("set_alert_threshold_override")); + Assert.True(new DarlingMcpWritePolicy.Effective(false, false, Investigations: true).Offers("set_alert_threshold_override")); + } +} diff --git a/Darling/README.md b/Darling/README.md index 7b1ccd8c81..a71ee9b1c5 100644 --- a/Darling/README.md +++ b/Darling/README.md @@ -2051,3 +2051,15 @@ Give **one** form, never both — a PKCS#12 bundle or a PEM pair — and the pas The product **consumes** a certificate and does not manage a PKI — no ACME, and deliberately no self-signed fallback, which would buy encryption without authentication and train you to click through the warning. An internal CA is the normal answer on a LAN. Loading **fails closed**: a certificate that is missing, unreadable, ambiguous, or **expired** keeps the dashboard loopback-only (the token is still required on that loopback listener) with a Critical log line rather than quietly serving the LAN over HTTP — and because an expired certificate takes the dashboard down, the service warns for the last 30 days before that happens. The listener negotiates TLS 1.2 and 1.3 only. A TLS-terminating reverse proxy in front of the port remains a perfectly good alternative. MCP has its own `tls` block, described in the MCP section above. **Behind a reverse proxy, list the proxy in `trustedProxies`.** The connection from the proxy to this host is plain HTTP, so without it the session cookie is not marked `Secure` and an OIDC `redirect_uri` is built as `http://` (which an IdP refuses). With `"trustedProxies": ["10.0.0.5"]` (IPs or CIDRs) the host honors `X-Forwarded-Proto`, `X-Forwarded-For` and `X-Forwarded-Host` **from those addresses only** — from anyone else the headers are ignored. Two consequences: `allowFrom` is then checked against the forwarded client address, so it has to cover your operators rather than the proxy; and the sign-in throttle counts per client instead of per proxy. Set `web.publicBaseUrl` to the name the proxy serves so the Host allowlist admits it. Set it too when the proxy rewrites the `Host` header (nginx's `proxy_pass` does by default): a sign-in POST from a browser that does not send `Sec-Fetch-Site` is accepted only when its `Origin` or `Referer` names the host the request was addressed to, the `X-Forwarded-Host` a trusted proxy sent, or `web.publicBaseUrl`'s host (Unicode and punycode spellings of a name are the same name). A browser that sends `Sec-Fetch-Site: same-origin` needs none of that. An entry that does not parse keeps the dashboard loopback-only with a Critical line. `Strict-Transport-Security` is **off by default**. Set `"hsts": true` in the `network` block, once the dashboard is reached over HTTPS only, and responses to a request that came over HTTPS (the listener's own, or a trusted proxy's) carry it (one year, no subdomains). HSTS is kept per host name and ignores the port: once a browser has reached the dashboard over HTTPS by name, it uses HTTPS for every port of that same name for a year, including other services on that name that speak plain HTTP. The proxy must append the client's address to `X-Forwarded-For` or overwrite it, and `trustedProxies` must not cover clients that connect directly. Only the address of the connection itself can be loopback for `allowFrom`: a forwarded `127.0.0.1` or `::1` is judged against `allowFrom` like any other address. + +## Per-server threshold overrides + +Some alerts fire often on one server and never on another. A threshold override holds a value for one server, or for one database on it, that differs from the global value. The global setting is never changed. + +- **What can be overridden.** The integer thresholds `get_alert_settings` lists (for example `cpu.threshold_percent`, `blocking.count_threshold`, `low_disk.threshold_gb`) and the two PostgreSQL count thresholds (`blocking.pg_count_threshold`, `deadlocks.pg_count_threshold`). Database scope applies only to `pvs.threshold_percent`, `pvs.floor_gb`, `file_growth.rise_mb` and `file_growth.volume_percent`; a database override applies to that database only, and a server override covers the server's other databases. Switches, cooldowns, routes and `self_alerts.*` are refused. To silence an alert, mute it. +- **Bounds.** A value stays inside the bounds the global setting has. Anything else is refused and nothing is written. +- **Setting one.** `set_alert_threshold_override` (`server_name`, `key`, `value`, optional `database_name`; `clear: true` removes it). It is offered under `mcp.allowWrites`, and under `mcp.allowInvestigations` as a Level 2 action: the agent passes `agent_action`, may change only its request's server, and cannot set a value that would turn the alert off. Under `mcp.allowWrites` `agent_action` is ignored and `set_by` names who made the change. The row records `set_by`, and for an agent change `agent:` and the request id. +- **Seeing one.** `get_alert_settings` with `server_name` adds `server_view`: each threshold with its global value, the value in effect for that server and its source (`global` or `server_override`), plus the server's database overrides. +- **When it applies.** The service reloads overrides with the rest of its configuration (the table is on the `config_version` beacon), so a change applies within the reload interval. +- **Bring-your-own store.** The `mcp` role needs a grant on `config.config_threshold_overrides`. Re-run `Darling/tools/provision-roles.sql` once after upgrading; until then the tool answers `precondition` and names the script. Managed and compose stores grant it themselves. +- **Lite.** Lite keeps global thresholds only and has no override path. From f111a10247439062102607a013e9ad41b7acc018 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 16 Oct 2026 07:02:04 -0400 Subject: [PATCH 4/5] Threshold overrides: the Viewer grid and the web Admin tab, admin login only (#5916 part 4) Viewer: ThresholdOverridesWindow with ThresholdOverridesViewModel (key and bounds checked before any write), opened from Settings, Alert rules. Web: GET and PUT /api/threshold-overrides and an Admin 'Threshold Overrides' tab; the write is the tool's own body, 403 naming the admin login under the viewer role. The key table moved to PerformanceMonitor.Alerting and the store statements to Darling.Storage so the Viewer, the web route and the tool share them. --- .../ReadAdmissionCoverageCensusTests.cs | 2 + .../ThresholdOverrideGridTests.cs | 255 ++++++++++++++++++ .../ThresholdOverrideWebLiveTests.cs | 253 +++++++++++++++++ .../DarlingThresholdOverrides.cs | 116 -------- .../DarlingWebEndpoints.cs | 65 +++++ .../DarlingWebThresholdOverrides.cs | 156 +++++++++++ .../Mcp/DarlingMcpThresholdOverrideTools.cs | 52 +--- .../wwwroot/js/pages/admin.js | 192 ++++++++++++- .../ThresholdOverrideStore.cs | 146 ++++++++++ .../SettingsWindow.xaml | 7 + .../SettingsWindow.xaml.cs | 12 + .../ThresholdOverridesViewModel.cs | 131 +++++++++ .../ThresholdOverridesWindow.xaml | 79 ++++++ .../ThresholdOverridesWindow.xaml.cs | 116 ++++++++ .../ViewerDataService.ThresholdOverrides.cs | 62 +++++ Darling/README.md | 1 + .../DarlingThresholdKeys.cs | 129 +++++++++ 17 files changed, 1610 insertions(+), 164 deletions(-) create mode 100644 Darling/Darling.Tests/ThresholdOverrideGridTests.cs create mode 100644 Darling/Darling.Tests/ThresholdOverrideWebLiveTests.cs create mode 100644 Darling/PerformanceMonitor.Darling.Service/DarlingWebThresholdOverrides.cs create mode 100644 Darling/PerformanceMonitor.Darling.Storage/ThresholdOverrideStore.cs create mode 100644 Darling/PerformanceMonitor.Darling.Viewer/ThresholdOverridesViewModel.cs create mode 100644 Darling/PerformanceMonitor.Darling.Viewer/ThresholdOverridesWindow.xaml create mode 100644 Darling/PerformanceMonitor.Darling.Viewer/ThresholdOverridesWindow.xaml.cs create mode 100644 Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.ThresholdOverrides.cs create mode 100644 PerformanceMonitor.Alerting/DarlingThresholdKeys.cs diff --git a/Darling/Darling.Tests/ReadAdmissionCoverageCensusTests.cs b/Darling/Darling.Tests/ReadAdmissionCoverageCensusTests.cs index 391c871687..b21c0a946b 100644 --- a/Darling/Darling.Tests/ReadAdmissionCoverageCensusTests.cs +++ b/Darling/Darling.Tests/ReadAdmissionCoverageCensusTests.cs @@ -221,6 +221,8 @@ public void TheDedicatedWebReadRoutes_Admit() [WebFile + "GET \"/api/investigation-notes/pending\""] = "definition read: the notes agents proposed that nobody has decided (one small config table, capped at 200 rows; #5903)", [WebFile + "PUT \"/api/investigation-notes/{id:long}/approve\""] = "write", [WebFile + "PUT \"/api/investigation-notes/{id:long}/reject\""] = "write", + [WebFile + "GET \"/api/threshold-overrides\""] = "definition read: the threshold overrides, one small config table, plus the registry's names (#5916)", + [WebFile + "PUT \"/api/threshold-overrides\""] = "write", [WebFile + "POST \"/api/server-tags\""] = "write", [WebFile + "PATCH \"/api/server-tags/{id:int}\""] = "write", [WebFile + "DELETE \"/api/server-tags/{id:int}\""] = "write", diff --git a/Darling/Darling.Tests/ThresholdOverrideGridTests.cs b/Darling/Darling.Tests/ThresholdOverrideGridTests.cs new file mode 100644 index 0000000000..5bb6b98e3f --- /dev/null +++ b/Darling/Darling.Tests/ThresholdOverrideGridTests.cs @@ -0,0 +1,255 @@ +/* + * 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.Text.Json.Nodes; +using System.Threading; +using System.Threading.Tasks; +using PerformanceMonitor.Darling.Storage; +using PerformanceMonitor.Darling.Viewer; +using Xunit; + +namespace Darling.Tests; + +/// +/// The Viewer's threshold-overrides logic (#5916 part 4), with a stand-in for the store: an entry the tool would refuse (a key +/// outside the registry, a self-monitoring or enable key, a value outside the key's bounds, a database on a server-only key, a +/// value that is not a whole number, no server) is refused with a sentence and the store is never called; a good entry is written +/// once with the login shaped to what the store accepts; a read-only seat's refusal is a sentence, not a crash. +/// +public sealed class ThresholdOverridesViewModelTests +{ + private sealed class FakeGateway : IThresholdOverrideGateway + { + public List Calls { get; } = new(); + public bool ReadOnly { get; set; } + + public Task> ListThresholdOverridesAsync(CancellationToken cancellationToken = default) + { + Calls.Add("list"); + IReadOnlyList rows = new[] + { + new ThresholdOverrideRow(7, "srv-7", null, "cpu.threshold_percent", 95, 80, "erik", null, new DateTime(2026, 10, 16, 0, 0, 0, DateTimeKind.Utc)), + new ThresholdOverrideRow(7, "srv-7", "shop", "pvs.threshold_percent", 70, null, "agent:a", Guid.Parse("0f1e2d3c-0000-0000-0000-000000000000"), new DateTime(2026, 10, 16, 0, 0, 0, DateTimeKind.Utc)), + }; + return Task.FromResult(rows); + } + + public Task SaveThresholdOverrideAsync(int serverId, string? database, string key, int value, string setBy, CancellationToken cancellationToken = default) + { + if (ReadOnly) + throw new ViewerReadOnlyException(new InvalidOperationException("42501")); + Calls.Add($"save {serverId}|{database}|{key}|{value}|{setBy}"); + return Task.CompletedTask; + } + + public Task RemoveThresholdOverrideAsync(int serverId, string? database, string key, CancellationToken cancellationToken = default) + { + if (ReadOnly) + throw new ViewerReadOnlyException(new InvalidOperationException("42501")); + Calls.Add($"remove {serverId}|{database}|{key}"); + return Task.CompletedTask; + } + } + + [Theory] + [InlineData(null, "cpu.threshold_percent", "90", null, "Choose a server")] + [InlineData(7, "cpu.nonsense", "90", null, "not a tunable threshold")] + [InlineData(7, "", "90", null, "key is required")] + [InlineData(7, "self_alerts.disk_free", "90", null, "self-monitoring")] + [InlineData(7, "cpu.enabled", "1", null, "enable flag")] + [InlineData(7, "cpu.cooldown_minutes", "5", null, "cooldown")] + [InlineData(7, "cpu.threshold_percent", "101", null, "between 1 and 100")] + [InlineData(7, "cpu.threshold_percent", "0", null, "between 1 and 100")] + [InlineData(7, "cpu.threshold_percent", "-5", null, "between 1 and 100")] + [InlineData(7, "cpu.threshold_percent", "ninety", null, "whole number")] + [InlineData(7, "cpu.threshold_percent", "", null, "whole number")] + [InlineData(7, "cpu.threshold_percent", "9.5", null, "whole number")] + [InlineData(7, "cpu.threshold_percent", "90", "shop", "server override only")] + [InlineData(7, "long_running_job.multiplier", "21", null, "between 2 and 20")] + public async Task ARefusedEntry_SaysWhy_AndNeverReachesTheStore(int? serverId, string key, string value, string? database, string expected) + { + var gateway = new FakeGateway(); + var model = new ThresholdOverridesViewModel(gateway, "erik"); + + var outcome = await model.SaveAsync(serverId, key, value, database, TestContext.Current.CancellationToken); + + Assert.False(outcome.Ok); + Assert.Contains(expected, outcome.Message, StringComparison.OrdinalIgnoreCase); + Assert.Empty(gateway.Calls); + } + + [Fact] + public async Task AGoodEntry_IsWrittenOnce_WithTheLoginShapedForTheStore_AndADatabaseOnlyWhereTheKeyTakesOne() + { + var gateway = new FakeGateway(); + var model = new ThresholdOverridesViewModel(gateway, "CORP\\erik darling"); + + Assert.True((await model.SaveAsync(7, "cpu.threshold_percent", " 95 ", " ", TestContext.Current.CancellationToken)).Ok); + Assert.True((await model.SaveAsync(7, "pvs.threshold_percent", "70", " shop ", TestContext.Current.CancellationToken)).Ok); + Assert.True((await model.SaveAsync(8, "blocking.pg_count_threshold", "4", null, TestContext.Current.CancellationToken)).Ok); + + Assert.Equal( + new[] + { + "save 7||cpu.threshold_percent|95|CORP_erik_darling", + "save 7|shop|pvs.threshold_percent|70|CORP_erik_darling", + "save 8||blocking.pg_count_threshold|4|CORP_erik_darling", + }, + gateway.Calls); + } + + [Fact] + public async Task EveryKeyTheFormOffers_IsOneTheToolWouldAccept_AtItsOwnBounds() + { + foreach (var key in ThresholdOverridesViewModel.Keys) + { + Assert.True(PerformanceMonitor.Alerting.DarlingThresholdKeys.TryGet(key, out var entry), key); + Assert.Null(ThresholdOverridesViewModel.Validate(7, key, entry.Min.ToString(System.Globalization.CultureInfo.InvariantCulture), null, out _)); + Assert.Null(ThresholdOverridesViewModel.Validate(7, key, entry.Max.ToString(System.Globalization.CultureInfo.InvariantCulture), null, out _)); + } + + await Task.CompletedTask; + } + + [Fact] + public async Task ARemove_NeedsARow_AndNamesTheRowItRemoves() + { + var gateway = new FakeGateway(); + var model = new ThresholdOverridesViewModel(gateway, "erik"); + + var none = await model.RemoveAsync(null, TestContext.Current.CancellationToken); + Assert.False(none.Ok); + Assert.Empty(gateway.Calls); + + var rows = await model.LoadAsync(TestContext.Current.CancellationToken); + Assert.Equal(2, rows.Count); + Assert.Equal("Whole server", rows[0].DatabaseText); + Assert.Equal("95", rows[0].ValueText); + Assert.Equal("80", rows[0].GlobalValueText); + Assert.Equal("", rows[1].GlobalValueText); + Assert.Equal("0f1e2d3c", rows[1].RequestText); + + Assert.True((await model.RemoveAsync(rows[1], TestContext.Current.CancellationToken)).Ok); + Assert.Equal("remove 7|shop|pvs.threshold_percent", gateway.Calls.Last()); + } + + [Fact] + public async Task AReadOnlySeat_GetsASentence_NotAnException() + { + var gateway = new FakeGateway { ReadOnly = true }; + var model = new ThresholdOverridesViewModel(gateway, "erik"); + + var save = await model.SaveAsync(7, "cpu.threshold_percent", "90", null, TestContext.Current.CancellationToken); + Assert.False(save.Ok); + Assert.Contains("read-only", save.Message, StringComparison.OrdinalIgnoreCase); + + var rows = await model.LoadAsync(TestContext.Current.CancellationToken); + var remove = await model.RemoveAsync(rows[0], TestContext.Current.CancellationToken); + Assert.False(remove.Ok); + Assert.Contains("read-only", remove.Message, StringComparison.OrdinalIgnoreCase); + } +} + +/// +/// The Admin page's Threshold Overrides tab (#5916 part 4), run under Node through the Admin page harness: the rows the list +/// read becomes, the entries the form refuses before sending, the words each answer gets (a 403 under a viewer login carries the +/// service's admin-login sentence through), and who gets the form and the Remove buttons (a login that can write). +/// +[Trait("Cost", "Slow")] +public sealed class ThresholdOverrideAdminTabTests +{ + private static JsonElement Pure(string fn, params object?[] args) => + AdminServerEditBehaviourTests.Run("pure", JsonSerializer.Serialize(new { fn, args })).GetProperty("result"); + + private static readonly JsonNode Keys = JsonNode.Parse(""" + [{"key":"cpu.threshold_percent","min":1,"max":100,"database_scope":false}, + {"key":"pvs.threshold_percent","min":0,"max":100,"database_scope":true}] + """)!; + + private static JsonElement Validate(string server, string key, string value, string database) => + Pure("validateOverride", JsonSerializer.Deserialize(Keys.ToJsonString()), new { server, key, value, database }); + + [Fact] + public void TheRows_NameTheServerDatabaseAndGlobalValue_AndAWholeServerOverrideSaysSo_Pure() + { + var data = JsonNode.Parse(""" + {"overrides":[ + {"server_id":7,"server_name":"srv-7","database":null,"key":"cpu.threshold_percent","value":95,"global_value":80,"set_by":"erik","request_id":null,"updated_at":"2026-10-16T00:00:00.0000000Z"}, + {"server_id":7,"server_name":"srv-7","database":"shop","key":"pvs.threshold_percent","value":70,"global_value":null,"set_by":"agent:a","request_id":"0f1e2d3c-0000-0000-0000-000000000000","updated_at":"2026-10-16T00:00:00.0000000Z"} + ]} + """); + var rows = Pure("overrideRows", JsonSerializer.Deserialize(data!.ToJsonString())).EnumerateArray().ToList(); + Assert.Equal(2, rows.Count); + Assert.Equal("Whole server", rows[0].GetProperty("database_text").GetString()); + Assert.Equal(80, rows[0].GetProperty("global_value").GetInt32()); + Assert.Equal("shop", rows[1].GetProperty("database_text").GetString()); + Assert.Equal(JsonValueKind.Null, rows[1].GetProperty("global_value").ValueKind); + Assert.Equal("0f1e2d3c", rows[1].GetProperty("request").GetString()); + } + + [Theory] + [InlineData("", "cpu.threshold_percent", "90", "", "Choose a server")] + [InlineData("srv-7", "cpu.nonsense", "90", "", "tunable threshold")] + [InlineData("srv-7", "cpu.threshold_percent", "101", "", "between 1 and 100")] + [InlineData("srv-7", "cpu.threshold_percent", "0", "", "between 1 and 100")] + [InlineData("srv-7", "cpu.threshold_percent", "9.5", "", "whole number")] + [InlineData("srv-7", "cpu.threshold_percent", "abc", "", "whole number")] + [InlineData("srv-7", "cpu.threshold_percent", "90", "shop", "server override only")] + public void AnEntryTheServiceWouldRefuse_IsRefusedBeforeAnythingIsSent_Pure(string server, string key, string value, string database, string expected) + { + Assert.Contains(expected, Validate(server, key, value, database).GetString(), StringComparison.Ordinal); + } + + [Fact] + public void AGoodEntry_PassesTheFormCheck_Pure() + { + Assert.Equal(JsonValueKind.Null, Validate("srv-7", "cpu.threshold_percent", "90", "").ValueKind); + Assert.Equal(JsonValueKind.Null, Validate("srv-7", "pvs.threshold_percent", "70", "shop").ValueKind); + } + + [Fact] + public void TheAnswers_GetPlainWords_AndAViewerLoginsRefusalCarriesTheAdminLoginSentence_Pure() + { + var refusal = "Changing a threshold override needs a store login with the admin role. Managed and compose stores connect the web dashboard as the viewer role."; + var results = AdminServerEditBehaviourTests.Run("pure", JsonSerializer.Serialize(new + { + calls = new object[] + { + new { fn = "overrideOutcome", args = new object[] { "set", new { status = 200, body = new { status = "ok" } } } }, + new { fn = "overrideOutcome", args = new object[] { "clear", new { status = 200, body = new { status = "cleared" } } } }, + new { fn = "overrideOutcome", args = new object[] { "set", new { status = 403, body = new { error = refusal } } } }, + new { fn = "overrideOutcome", args = new object[] { "set", new { status = 400, body = new { error = "cpu.threshold_percent must be an integer between 1 and 100." } } } }, + new { fn = "overrideOutcome", args = new object[] { "set", new { status = 500, body = new { error = "The store did not answer." } } } }, + }, + })).GetProperty("results").EnumerateArray().ToList(); + + Assert.False(results[0].GetProperty("isError").GetBoolean()); + Assert.True(results[0].GetProperty("reread").GetBoolean()); + Assert.Contains("removed", results[1].GetProperty("message").GetString()); + Assert.True(results[2].GetProperty("isError").GetBoolean()); + Assert.Equal(refusal, results[2].GetProperty("message").GetString()); + Assert.False(results[2].GetProperty("reread").GetBoolean()); + Assert.Contains("between 1 and 100", results[3].GetProperty("message").GetString()); + Assert.DoesNotContain("answered", results[3].GetProperty("message").GetString()); + Assert.Contains("answered 500", results[4].GetProperty("message").GetString()); + } + + [Fact] + public void OnlyALoginThatCanWrite_GetsTheChangeColumn_Pure() + { + var readOnly = Pure("overrideColumns", false).EnumerateArray().Select(c => c.GetProperty("key").GetString()).ToList(); + var admin = Pure("overrideColumns", true).EnumerateArray().Select(c => c.GetProperty("key").GetString()).ToList(); + Assert.DoesNotContain("actions", readOnly); + Assert.Equal("actions", admin.Last()); + Assert.Equal(readOnly.Count + 1, admin.Count); + } +} diff --git a/Darling/Darling.Tests/ThresholdOverrideWebLiveTests.cs b/Darling/Darling.Tests/ThresholdOverrideWebLiveTests.cs new file mode 100644 index 0000000000..bd9bf556d1 --- /dev/null +++ b/Darling/Darling.Tests/ThresholdOverrideWebLiveTests.cs @@ -0,0 +1,253 @@ +/* + * 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.IO; +using System.Text; +using System.Text.Json.Nodes; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Npgsql; +using PerformanceMonitor.Common; +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 through ScratchPostgres and never touches another test's rows, + so the class is deliberately NOT [Collection("live-postgres")]. */ + +/// +/// The threshold-overrides web routes (#5916 part 4) on a live store, run as the managed admin role (a change is accepted, +/// stamped with the seat's name, and a bad key or bound is a 400 before any write) and as the managed viewer role (the +/// list works, a change answers 403 naming the admin login and leaves the row alone), the grants replayed from the shipped +/// provisioning SQL. +/// +public sealed class ThresholdOverrideWebLiveTests +{ + private const string RolePassword = "threshold-web-live-pw"; + + private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + + 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), System.Globalization.CultureInfo.InvariantCulture); + } + + private static async Task ExecAsync(NpgsqlDataSource source, string sql, CancellationToken ct) + { + await using var command = source.CreateCommand(sql); + await command.ExecuteNonQueryAsync(ct); + } + + /// Runs against the web routes with the web host's data source logging in as a role holding + /// exactly what the managed holds, on a scratch store with two servers and one stored override + /// (srv-8 cpu.threshold_percent = 77, set by "seed"). + private static async Task WithRoutesAsync( + string managedRole, Func>, CancellationToken, Task> body) + { + var baseConnectionString = ConnectionString; + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), "Set DARLING_TEST_PG to run the threshold override web live tests."); + + var ct = TestContext.Current.CancellationToken; + var roleName = "thr_seat_" + Guid.NewGuid().ToString("N")[..8]; + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var owner = await SeededAsync(scratch, ct); + + var bodySucceeded = false; + try + { + await ExecAsync(owner, $"CREATE ROLE {roleName} LOGIN NOSUPERUSER PASSWORD '{RolePassword}'", ct); + foreach (var statement in ViewerGrantReplay.StatementsFor(roleName, scratch.DatabaseName, managedRole)) + { + await ExecAsync(owner, statement, ct); + } + + var asSeat = new NpgsqlConnectionStringBuilder(scratch.ConnectionString) + { + Username = roleName, + Password = RolePassword, + SearchPath = PgSchemaGenerator.SearchPath, + Pooling = false, + }.ConnectionString; + await using var webLogin = NpgsqlDataSource.Create(asSeat); + + var builder = WebApplication.CreateBuilder(new WebApplicationOptions + { + ContentRootPath = Path.Combine(RepoFile.Root, "Darling", "PerformanceMonitor.Darling.Service"), + WebRootPath = "wwwroot", + }); + builder.WebHost.UseTestServer(); + builder.Logging.ClearProviders(); + builder.Services.AddSingleton(webLogin); + await using var app = builder.Build(); + DarlingWebEndpoints.MapAll(app, webLogin, new CollectorRuntimeState(), new CapturingTestLogger()); + await app.StartAsync(ct); + using var server = app.GetTestServer(); + + async Task<(int Status, JsonNode? Body)> Send(string method, string? contentType, string? json) + { + var context = await server.SendAsync(request => + { + request.Request.Method = method; + request.Request.Path = "/api/threshold-overrides"; + request.Request.Headers.Host = "localhost"; + if (contentType is not null) + { + request.Request.ContentType = contentType; + } + + request.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes(json ?? "")); + }); + using var reader = new StreamReader(context.Response.Body); + var text = await reader.ReadToEndAsync(ct); + return (context.Response.StatusCode, string.IsNullOrWhiteSpace(text) ? null : JsonNode.Parse(text)); + } + + await body(owner, Send, ct); + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (connection, token) => + { + NpgsqlConnection.ClearAllPools(); + await using var drop = new NpgsqlCommand($"DROP OWNED BY {roleName}; DROP ROLE IF EXISTS {roleName}", connection); + await drop.ExecuteNonQueryAsync(token); + }); + } + } + + private static async Task SeededAsync(ScratchPostgres scratch, CancellationToken ct) + { + await using (var connection = new NpgsqlConnection(scratch.ConnectionString)) + { + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + } + + var builder = new NpgsqlConnectionStringBuilder(scratch.ConnectionString) { SearchPath = PgSchemaGenerator.SearchPath }; + var source = NpgsqlDataSource.Create(builder.ConnectionString); + await ExecAsync(source, "INSERT INTO config.config_monitored_servers (server_id, name, host) VALUES (7, 'srv-7', 'h7'), (8, 'srv-8', 'h8')", ct); + await ExecAsync(source, "INSERT INTO collect.servers (server_id, server_name, display_name, is_enabled, sql_major_version, created_date, modified_date) " + + "VALUES (7, 'srv-7', 'srv-7', TRUE, 15, now(), now()), (8, 'srv-8', 'srv-8', TRUE, 15, now(), now())", ct); + await ExecAsync(source, "INSERT INTO config.config_alert_settings (id) VALUES (1) ON CONFLICT (id) DO NOTHING", ct); + await ExecAsync(source, "INSERT INTO config.config_threshold_overrides (server_id, threshold_key, value, set_by) VALUES (8, 'cpu.threshold_percent', 77, 'seed')", ct); + return source; + } + + private const string Json = "application/json"; + + [Fact] + public async Task AsTheAdminRole_TheListShowsTheOverrideAndItsGlobalValue_AndAChangeIsAcceptedAndStamped() + { + await WithRoutesAsync("admin", async (owner, send, ct) => + { + var list = await send("GET", null, null); + Assert.Equal(200, list.Status); + Assert.True(list.Body!["can_edit"]!.GetValue()); + var row = Assert.Single(list.Body["overrides"]!.AsArray())!; + Assert.Equal("srv-8", row["server_name"]!.GetValue()); + Assert.Equal("cpu.threshold_percent", row["key"]!.GetValue()); + Assert.Equal(77, row["value"]!.GetValue()); + Assert.Equal("seed", row["set_by"]!.GetValue()); + Assert.Equal(80, row["global_value"]!.GetValue()); + Assert.Contains(list.Body["servers"]!.AsArray(), s => s!["server_name"]!.GetValue() == "srv-7"); + Assert.Contains(list.Body["keys"]!.AsArray(), k => k!["key"]!.GetValue() == "pvs.threshold_percent" && k["database_scope"]!.GetValue()); + + /* A new override and a change to the seeded one. */ + var added = await send("PUT", Json, """{"server_name":"srv-7","key":"cpu.threshold_percent","value":95}"""); + Assert.Equal(200, added.Status); + Assert.Equal("ok", added.Body!["status"]!.GetValue()); + Assert.Equal("95", await ScalarAsync(owner, "SELECT value FROM config.config_threshold_overrides WHERE server_id = 7", ct)); + var by = await ScalarAsync(owner, "SELECT set_by FROM config.config_threshold_overrides WHERE server_id = 7", ct); + Assert.Matches("^[A-Za-z0-9._:-]{1,64}$", by!); + Assert.Equal("ok", (await send("PUT", Json, """{"server_name":"srv-8","key":"cpu.threshold_percent","value":60}""")).Body!["status"]!.GetValue()); + Assert.Equal("60", await ScalarAsync(owner, "SELECT value FROM config.config_threshold_overrides WHERE server_id = 8", ct)); + Assert.Equal("2", await ScalarAsync(owner, "SELECT count(*) FROM config.config_threshold_overrides", ct)); + + /* A database override is its own row; clear removes exactly the row named. */ + Assert.Equal(200, (await send("PUT", Json, """{"server_name":"srv-7","key":"pvs.threshold_percent","value":70,"database_name":"shop"}""")).Status); + Assert.Equal("3", await ScalarAsync(owner, "SELECT count(*) FROM config.config_threshold_overrides", ct)); + var cleared = await send("PUT", Json, """{"server_name":"srv-7","key":"pvs.threshold_percent","database_name":"shop","clear":true}"""); + Assert.Equal(200, cleared.Status); + Assert.Equal("cleared", cleared.Body!["status"]!.GetValue()); + Assert.Equal("2", await ScalarAsync(owner, "SELECT count(*) FROM config.config_threshold_overrides", ct)); + }); + } + + [Fact] + public async Task AsTheAdminRole_ABadKeyOrBoundOrBody_IsRefusedBeforeAnyWrite() + { + await WithRoutesAsync("admin", async (owner, send, ct) => + { + var before = await ScalarAsync(owner, "SELECT string_agg(server_id || ':' || threshold_key || ':' || value, ',' ORDER BY id) FROM config.config_threshold_overrides", ct); + + foreach (var (json, reason) in new[] + { + ("""{"server_name":"srv-7","key":"cpu.nonsense","value":5}""", "not_in_registry"), + ("""{"server_name":"srv-7","key":"self_alerts.enabled","value":1}""", "not_tunable"), + ("""{"server_name":"srv-7","key":"cpu.threshold_percent","value":101}""", "out_of_bounds"), + ("""{"server_name":"srv-7","key":"cpu.threshold_percent","value":0}""", "out_of_bounds"), + ("""{"server_name":"srv-7","key":"cpu.threshold_percent","value":50,"database_name":"shop"}""", "database_scope"), + ("""{"server_name":"srv-7","key":"cpu.threshold_percent"}""", "value_required"), + }) + { + var answer = await send("PUT", Json, json); + Assert.Equal(400, answer.Status); + Assert.Equal(reason, answer.Body!["reason"]!.GetValue()); + Assert.False(string.IsNullOrWhiteSpace(answer.Body["error"]!.GetValue())); + } + + Assert.Equal(400, (await send("PUT", Json, """{"server_name":"srv-7","key":"cpu.threshold_percent","value":"high"}""")).Status); + Assert.Equal(400, (await send("PUT", Json, """{"server_name":"srv-7","key":"cpu.threshold_percent","value":1.5}""")).Status); + Assert.Equal(400, (await send("PUT", Json, """{"key":"cpu.threshold_percent","value":50}""")).Status); + Assert.Equal(400, (await send("PUT", Json, "[1]")).Status); + Assert.Equal(400, (await send("PUT", Json, "not json")).Status); + Assert.Equal(415, (await send("PUT", "text/plain", """{"server_name":"srv-7","key":"cpu.threshold_percent","value":50}""")).Status); + + Assert.Equal(before, await ScalarAsync(owner, "SELECT string_agg(server_id || ':' || threshold_key || ':' || value, ',' ORDER BY id) FROM config.config_threshold_overrides", ct)); + }); + } + + [Fact] + public async Task AsTheViewerRole_TheListWorks_ButAChangeAnswers403NamingTheAdminLogin_AndChangesNothing() + { + await WithRoutesAsync("viewer", async (owner, send, ct) => + { + var list = await send("GET", null, null); + Assert.Equal(200, list.Status); + Assert.False(list.Body!["can_edit"]!.GetValue()); + Assert.Equal("srv-8", Assert.Single(list.Body["overrides"]!.AsArray())!["server_name"]!.GetValue()); + + foreach (var json in new[] + { + """{"server_name":"srv-7","key":"cpu.threshold_percent","value":95}""", + """{"server_name":"srv-8","key":"cpu.threshold_percent","value":60}""", + """{"server_name":"srv-8","key":"cpu.threshold_percent","clear":true}""", + }) + { + var answer = await send("PUT", Json, json); + Assert.Equal(403, answer.Status); + Assert.Equal(DarlingWebThresholdOverrides.RefusedMessage, answer.Body!["error"]!.GetValue()); + Assert.Contains("admin", answer.Body["error"]!.GetValue()); + } + + Assert.Equal("77", await ScalarAsync(owner, "SELECT value FROM config.config_threshold_overrides WHERE server_id = 8", ct)); + Assert.Equal("1", await ScalarAsync(owner, "SELECT count(*) FROM config.config_threshold_overrides", ct)); + }); + } +} diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingThresholdOverrides.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingThresholdOverrides.cs index 4a9c307f03..0a5f3c05df 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingThresholdOverrides.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingThresholdOverrides.cs @@ -17,122 +17,6 @@ namespace PerformanceMonitor.Darling.Service; -/// -/// The tunable thresholds Darling can hold an override for (#5916): one row per wire name, which is the name -/// get_alert_settings and update_alert_settings already use for the global value (for example -/// cpu.threshold_percent). Every registry key of #5910 () is here, plus the two -/// PostgreSQL count thresholds, which are Darling's own and are not on the engine's settings contract. PostgreSQL -/// predictor constants are not settings and have no entry. The bounds are the ones update_alert_settings and -/// the Viewer's Settings window enforce on the global value, so an override can never hold a value the global setting -/// would refuse. -/// -internal static class DarlingThresholdKeys -{ - /// One tunable threshold: its wire name, the engine key (null for a PostgreSQL count threshold), the scope it - /// can take and its bounds. - internal sealed record Entry(string Wire, AlertThresholdKey? Engine, AlertThresholdScope Scope, int Min, int Max); - - internal const string PgBlockingCount = "blocking.pg_count_threshold"; - internal const string PgDeadlockCount = "deadlocks.pg_count_threshold"; - - private static readonly Entry[] All = - [ - new("cpu.threshold_percent", AlertThresholdKey.CpuThresholdPercent, AlertThresholdScope.Server, 1, 100), - new("blocking.count_threshold", AlertThresholdKey.BlockingCountThreshold, AlertThresholdScope.Server, PostgresAlertEvaluator.CountThresholdFloor, int.MaxValue), - new("blocking.wait_threshold_seconds", AlertThresholdKey.BlockingWaitSecondsThreshold, AlertThresholdScope.Server, 0, int.MaxValue), - new("deadlocks.count_threshold", AlertThresholdKey.DeadlockCountThreshold, AlertThresholdScope.Server, PostgresAlertEvaluator.CountThresholdFloor, int.MaxValue), - new("runnable_queue.warning_per_scheduler", AlertThresholdKey.RunnableQueueWarningPerScheduler, AlertThresholdScope.Server, 1, int.MaxValue), - new("runnable_queue.critical_per_scheduler", AlertThresholdKey.RunnableQueueCriticalPerScheduler, AlertThresholdScope.Server, 1, int.MaxValue), - new("client_timeouts.warning_per_minute", AlertThresholdKey.ClientTimeoutsWarningPerMinute, AlertThresholdScope.Server, 1, int.MaxValue), - new("client_timeouts.critical_per_minute", AlertThresholdKey.ClientTimeoutsCriticalPerMinute, AlertThresholdScope.Server, 1, int.MaxValue), - new("long_running_query.threshold_minutes", AlertThresholdKey.LongRunningQueryThresholdMinutes, AlertThresholdScope.Server, 1, int.MaxValue), - new("tempdb_space.threshold_percent", AlertThresholdKey.TempDbSpaceThresholdPercent, AlertThresholdScope.Server, 1, 100), - new("low_disk.threshold_percent", AlertThresholdKey.LowDiskThresholdPercent, AlertThresholdScope.Server, 0, 100), - new("low_disk.threshold_gb", AlertThresholdKey.LowDiskThresholdGb, AlertThresholdScope.Server, 0, int.MaxValue), - new("low_disk.critical_free_percent", AlertThresholdKey.DiskCriticalFreePercent, AlertThresholdScope.Server, 0, 100), - new("low_disk.critical_free_gb", AlertThresholdKey.DiskCriticalFreeGb, AlertThresholdScope.Server, 0, int.MaxValue), - new("pvs.threshold_percent", AlertThresholdKey.PvsThresholdPercent, AlertThresholdScope.ServerAndDatabase, 0, 100), - new("pvs.floor_gb", AlertThresholdKey.PvsFloorGb, AlertThresholdScope.ServerAndDatabase, 0, int.MaxValue), - new("file_growth.rise_mb", AlertThresholdKey.FileGrowthRiseMb, AlertThresholdScope.ServerAndDatabase, 0, int.MaxValue), - new("file_growth.volume_percent", AlertThresholdKey.FileGrowthVolumePercent, AlertThresholdScope.ServerAndDatabase, 0, 100), - new("file_growth.lookback_minutes", AlertThresholdKey.FileGrowthLookbackMinutes, AlertThresholdScope.Server, 5, 1440), - new("long_running_job.multiplier", AlertThresholdKey.LongRunningJobMultiplier, AlertThresholdScope.Server, 2, 20), - new("failed_job.lookback_minutes", AlertThresholdKey.FailedJobLookbackMinutes, AlertThresholdScope.Server, 1, 1440), - new(PgBlockingCount, null, AlertThresholdScope.Server, PostgresAlertEvaluator.CountThresholdFloor, int.MaxValue), - new(PgDeadlockCount, null, AlertThresholdScope.Server, PostgresAlertEvaluator.CountThresholdFloor, int.MaxValue), - ]; - - private static readonly Dictionary ByWire = BuildByWire(); - private static readonly Dictionary ByEngine = BuildByEngine(); - - private static Dictionary BuildByWire() - { - var map = new Dictionary(StringComparer.Ordinal); - foreach (var e in All) - map[e.Wire] = e; - return map; - } - - private static Dictionary BuildByEngine() - { - var map = new Dictionary(); - foreach (var e in All) - { - if (e.Engine is { } k) - map[k] = e; - } - - return map; - } - - /// Every tunable threshold, in display order. - internal static IReadOnlyList Entries => All; - - internal static bool TryGet(string? wire, out Entry entry) - { - entry = null!; - return wire is not null && ByWire.TryGetValue(wire.Trim(), out entry!); - } - - internal static Entry For(AlertThresholdKey key) => ByEngine[key]; - - /// - /// Why a key cannot be overridden, or null when it is a tunable threshold. The reasons name the class of key so an - /// operator or agent sees why: an override moves a threshold, never a switch, a cadence or a destination. - /// - internal static (string Reason, string Message)? RefusalFor(string? key) - { - var k = (key ?? string.Empty).Trim(); - if (k.Length == 0) - return ("key_required", "key is required: a threshold such as cpu.threshold_percent (see get_alert_settings with server_name)."); - if (TryGet(k, out _)) - return null; - if (k.StartsWith("self_alerts.", StringComparison.OrdinalIgnoreCase)) - return ("not_tunable", $"'{k}' is a self-monitoring setting (self_alerts.*) about the monitor itself, not a monitored server. It has no per-server override."); - if (k.EndsWith(".enabled", StringComparison.OrdinalIgnoreCase) || string.Equals(k, "enabled", StringComparison.OrdinalIgnoreCase)) - return ("not_tunable", $"'{k}' is an enable flag. An override changes a threshold, never turns an alert on or off; mute the alert to silence it."); - if (k.Contains("cooldown", StringComparison.OrdinalIgnoreCase)) - return ("not_tunable", $"'{k}' is a cooldown. It spaces notifications and has no per-server override."); - if (k.Contains("route", StringComparison.OrdinalIgnoreCase) || k.StartsWith("delivery.", StringComparison.OrdinalIgnoreCase)) - return ("not_tunable", $"'{k}' is a route or delivery setting. It has no per-server override."); - return ("not_in_registry", $"'{k}' is not a tunable threshold. See get_alert_settings with server_name for the keys that can be overridden."); - } - - /// Why or is refused for , or null. - internal static (string Reason, string Message)? ValidateValue(Entry entry, int value, string? database) - { - if (value < entry.Min || value > entry.Max) - { - var max = entry.Max == int.MaxValue ? "2147483647" : entry.Max.ToString(CultureInfo.InvariantCulture); - return ("out_of_bounds", $"'{entry.Wire}' must be an integer between {entry.Min.ToString(CultureInfo.InvariantCulture)} and {max}, the same bounds the global setting has."); - } - - if (!string.IsNullOrWhiteSpace(database) && entry.Scope != AlertThresholdScope.ServerAndDatabase) - return ("database_scope", $"'{entry.Wire}' is compared against a server-level figure, so it takes a server override only, not a database_name."); - return null; - } -} - /// /// The per-server and per-database threshold overrides the alert engine and the PostgreSQL count checks resolve through /// (#5916): the Darling loader behind . It holds one immutable snapshot of diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs index 562e41b951..9d8354b8cf 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs @@ -627,6 +627,7 @@ ServerError arm is the only shape that can carry a 57014 sentence (a tool that c MapCustomAlerts(app, postgres, logger, readAdmission, readLatencyRecorder); MapMuteRules(app, postgres, logger); MapInvestigationNotes(app, new InvestigationRequestStore(postgres, McpCommandDeadlines.ReadSeconds), logger); + MapThresholdOverrides(app, postgres, logger); MapServerTags(app, postgres, logger); MapAdminServers(app, postgres); MapServers(app, postgres, logger); @@ -1198,6 +1199,70 @@ private static void MapInvestigationNotes(WebApplication app, InvestigationReque }); } + /// + /// The threshold-overrides grid's routes (#5916 part 4): GET /api/threshold-overrides lists the overrides and + /// PUT /api/threshold-overrides sets or clears one (the body and the answers are 's). + /// The write answers as the note decisions do (#5903): application/json required, born gated by the host's write gate, + /// 403 with when the store login holds no write grant on the table + /// (the viewer role of a managed or compose store), 400 for a refused key, value or server. Every body goes through the + /// statement sweep (#4348): set_by on an agent's override is text an agent named. + /// + private static void MapThresholdOverrides(WebApplication app, NpgsqlDataSource postgres, ILogger logger) + { + app.MapGet("/api/threshold-overrides", async (HttpContext context) => + { + var stopwatch = Stopwatch.StartNew(); + try + { + return DarlingWebStatementSweep.JsonText( + await DarlingWebThresholdOverrides.ListBodyAsync(postgres, context.RequestAborted), + "/api/threshold-overrides", logger, stopwatch.ElapsedMilliseconds, context.RequestAborted); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + DarlingWebFailureLog.Report(logger, "/api/threshold-overrides", stopwatch.ElapsedMilliseconds, ex, context.RequestAborted); + return Results.Json(DarlingWebFailureLog.Body(ex), statusCode: DarlingWebFailureLog.StatusCode(ex)); + } + }); + + app.MapPut("/api/threshold-overrides", async (HttpContext context) => + { + if (!IsJsonContentType(context.Request.ContentType)) + { + return UnsupportedMediaTypeResult(); + } + + JsonObject? body; + try + { + body = await JsonNode.ParseAsync(context.Request.Body, cancellationToken: context.RequestAborted) as JsonObject; + } + catch (JsonException) + { + body = null; + } + + if (body is null) + { + return ErrorResult("The body must be a JSON object: {server_name, key, value, database_name?, clear?}.", StatusCodes.Status400BadRequest); + } + + var stopwatch = Stopwatch.StartNew(); + try + { + var (status, answer) = await DarlingWebThresholdOverrides.ChangeAsync( + postgres, logger, body, DarlingWebSeat.FromContext(context).EditorPrincipal, context.RequestAborted); + return DarlingWebStatementSweep.JsonText( + answer, "/api/threshold-overrides", logger, stopwatch.ElapsedMilliseconds, context.RequestAborted, status); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + DarlingWebFailureLog.Report(logger, "/api/threshold-overrides", stopwatch.ElapsedMilliseconds, ex, context.RequestAborted); + return Results.Json(DarlingWebFailureLog.Body(ex), statusCode: DarlingWebFailureLog.StatusCode(ex)); + } + }); + } + /// The 403 sentence a note decision answers when the store refuses the write (SQLSTATE 42501): the web host runs on /// whatever login postgres.webConnectionString names, and deciding a note is an admin role write (#5903). internal const string NoteDecisionRefusedMessage = diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWebThresholdOverrides.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWebThresholdOverrides.cs new file mode 100644 index 0000000000..eabc4db880 --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWebThresholdOverrides.cs @@ -0,0 +1,156 @@ +/* + * 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.Text.Json; +using System.Text.Json.Nodes; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Npgsql; +using PerformanceMonitor.Alerting; +using PerformanceMonitor.Darling.Service.Investigations; +using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; + +namespace PerformanceMonitor.Darling.Service; + +/// +/// The threshold-overrides grid's routes (#5916 part 4), mapped in (the statement census allows no other mapper file): GET /api/threshold-overrides lists every override with its server, +/// the global value it replaces and who set it, and PUT /api/threshold-overrides sets or clears one. The write is the +/// set_alert_threshold_override tool's own body () run on the writes +/// arm, so the key registry, the bounds, the database scope and the store statement are the tool's, not a second copy. It answers as +/// the pending-note decisions do (#5903): application/json required, born gated by the host's write gate (a read-only seat +/// is refused every unsafe method), and 403 with when the store login holds no write grant on the table +/// (the viewer role of a managed or compose store, or a bring-your-own login that is not an admin one), 400 for a refused key, +/// value or server, a fixed-sentence 500 for any other store failure. The change is stamped with the seat's +/// as every web write is. +/// +internal static class DarlingWebThresholdOverrides +{ + /// The 403 sentence a refused write answers: the web host runs on whatever login postgres.webConnectionString + /// names, and writing an override is an admin role write. + internal const string RefusedMessage = + "Changing a threshold override needs a store login with the admin role. Managed and compose stores connect the web dashboard as the " + + "viewer role, which lists overrides but cannot change them: change them in the desktop Viewer, or (bring-your-own store) set " + + "postgres.webConnectionString to an admin login."; + + /// The list body: {overrides: [{server_id, server_name, database, key, value, global_value, set_by, request_id, + /// updated_at}], keys: [{key, min, max, database_scope}], servers: [{server_id, server_name}], can_edit, note}. + /// can_edit is whether the store login holds the write grant, so a page can say list-only before anyone tries. + internal static async Task ListBodyAsync(NpgsqlDataSource postgres, CancellationToken ct) + { + var overrides = new JsonArray(); + string? note = null; + try + { + foreach (var r in await ThresholdOverrideStore.ListAsync( + postgres, k => DarlingThresholdKeys.TryGet(k, out var e) ? e.Column : null, ct)) + { + overrides.Add(new JsonObject + { + ["server_id"] = r.ServerId, + ["server_name"] = r.ServerName, + ["database"] = r.Database, + ["key"] = r.Key, + ["value"] = r.Value, + ["global_value"] = r.GlobalValue, + ["set_by"] = r.SetBy, + ["request_id"] = r.RequestId, + ["updated_at"] = r.UpdatedAt.ToString("o", CultureInfo.InvariantCulture), + }); + } + } + catch (PostgresException ex) when (ex.SqlState == PostgresErrorCodes.InsufficientPrivilege) + { + note = DarlingMcpThresholdOverrideTools.PreconditionMessage; + } + + var keys = new JsonArray(); + foreach (var entry in DarlingThresholdKeys.Entries) + { + keys.Add(new JsonObject + { + ["key"] = entry.Wire, + ["min"] = entry.Min, + ["max"] = entry.Max, + ["database_scope"] = entry.Scope == AlertThresholdScope.ServerAndDatabase, + }); + } + + var servers = new JsonArray(); + await using (var command = postgres.CreateCommand( + "SELECT server_id, server_name FROM collect.servers WHERE is_enabled ORDER BY server_name")) + { + command.CommandTimeout = McpCommandDeadlines.ReadSeconds; + await using var reader = await command.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + servers.Add(new JsonObject { ["server_id"] = reader.GetInt32(0), ["server_name"] = reader.GetString(1) }); + } + + return new JsonObject + { + ["overrides"] = overrides, + ["keys"] = keys, + ["servers"] = servers, + ["can_edit"] = await DarlingMcpThresholdOverrideTools.HoldsGrantAsync(postgres, ct), + ["note"] = note, + }; + } + + /// + /// One write: the tool's writes-arm body, its answer mapped to an HTTP status. The tool answers precondition when the + /// store login has no write grant (checked before any statement) or the store refuses for privilege (SQLSTATE 42501): both + /// are this route's 403. refused is a key, value, database or set_by the tool will not write: 400. + /// + internal static async Task<(int Status, JsonNode Body)> ChangeAsync( + NpgsqlDataSource postgres, ILogger logger, JsonObject body, string editor, CancellationToken ct) + { + var server = body["server_name"]?.GetValueKind() == JsonValueKind.String ? body["server_name"]!.GetValue() : null; + var key = body["key"]?.GetValueKind() == JsonValueKind.String ? body["key"]!.GetValue() : null; + var database = body["database_name"]?.GetValueKind() == JsonValueKind.String ? body["database_name"]!.GetValue() : null; + var clear = body["clear"]?.GetValueKind() == JsonValueKind.True; + + int? value = null; + if (body["value"] is JsonValue v && v.GetValueKind() != JsonValueKind.Null) + { + if (v.GetValueKind() != JsonValueKind.Number || !v.TryGetValue(out var parsed)) + return (400, new JsonObject { ["error"] = "value must be an integer." }); + value = parsed; + } + + if (string.IsNullOrWhiteSpace(server)) + return (400, new JsonObject { ["error"] = "server_name is required." }); + + var json = await DarlingMcpThresholdOverrideTools.SetAsync( + postgres, new DarlingMcpWritePolicy.Effective(true, false), + new InvestigationRequestStore(postgres, McpCommandDeadlines.ReadSeconds), logger, + server, key ?? string.Empty, value, database, clear, ThresholdOverrideStore.SetByFor(editor), null, ct); + + var answer = JsonNode.Parse(json)!.AsObject(); + var status = answer["status"]?.GetValue(); + switch (status) + { + case "ok": + case "cleared": + return (200, answer); + case "precondition": + return (403, new JsonObject { ["error"] = RefusedMessage }); + case "refused": + case "invalid": + return (400, new JsonObject + { + ["error"] = answer["message"]?.GetValue() ?? "That override was refused.", + ["reason"] = answer["reason"]?.GetValue(), + }); + default: + return (500, new JsonObject { ["error"] = "The override could not be saved." }); + } + } +} diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs index 82e6d4d655..4116647feb 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs @@ -17,6 +17,7 @@ using ModelContextProtocol.Server; using Npgsql; using NpgsqlTypes; +using PerformanceMonitor.Alerting; using PerformanceMonitor.Common; using PerformanceMonitor.Darling.Storage; @@ -49,7 +50,7 @@ public sealed class DarlingMcpThresholdOverrideTools private static readonly Regex SetByShape = new("^[A-Za-z0-9._:-]{1,64}$", RegexOptions.Compiled | RegexOptions.CultureInvariant); /// The table, schema-qualified; the mcp role holds SELECT, INSERT, UPDATE and DELETE on it (#5920). - internal const string Table = "config.config_threshold_overrides"; + internal const string Table = ThresholdOverrideStore.Table; internal const string PreconditionMessage = "The service's mcp store role has no grant on config.config_threshold_overrides. A bring-your-own store needs " + @@ -130,7 +131,7 @@ internal static async Task SetAsync( async Task ReadBefore(NpgsqlConnection c, NpgsqlTransaction t, CancellationToken ct) { - applied.Previous = await ReadValueAsync(c, t, resolved.ServerId, database, wire, ct); + applied.Previous = await ThresholdOverrideStore.ReadValueAsync(c, t, resolved.ServerId, database, wire, ct); return applied.Previous is null ? null : JsonSerializer.Serialize(new { value = applied.Previous }); } @@ -139,11 +140,11 @@ internal static async Task SetAsync( /* Attribution: the writes arm names the person; an agent change names the agent and the request. */ var by = person ?? "agent:" + action?.AgentId; Guid? requestId = person is null ? action?.RequestId : null; - applied.Previous = await ReadValueAsync(c, t, resolved.ServerId, database, wire, ct); + applied.Previous = await ThresholdOverrideStore.ReadValueAsync(c, t, resolved.ServerId, database, wire, ct); if (clear) - await DeleteAsync(c, t, resolved.ServerId, database, wire, ct); + await ThresholdOverrideStore.DeleteAsync(c, t, resolved.ServerId, database, wire, ct); else - await UpsertAsync(c, t, resolved.ServerId, database, wire, value!.Value, by, requestId, ct); + await ThresholdOverrideStore.UpsertAsync(c, t, resolved.ServerId, database, wire, value!.Value, by, requestId, ct); applied.SetBy = by; applied.RequestId = requestId; return clear ? null : JsonSerializer.Serialize(new { value }); @@ -331,45 +332,4 @@ internal static async Task BuildServerViewAsync( DarlingThresholdKeys.PgDeadlockCount => s.PgDeadlockCountThreshold, _ => throw new ArgumentOutOfRangeException(nameof(wire), wire, "Not a tunable threshold."), }; - - private static NpgsqlCommand Command(NpgsqlConnection connection, NpgsqlTransaction transaction, string sql, int serverId, string? database, string wire) - { - var command = new NpgsqlCommand(sql, connection) { Transaction = transaction, CommandTimeout = McpCommandDeadlines.ReadSeconds }; - command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Integer, Value = serverId }); - command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = (object?)database ?? DBNull.Value }); - command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = wire }); - return command; - } - - internal static async Task ReadValueAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, int serverId, string? database, string wire, - CancellationToken ct) - { - await using var command = Command(connection, transaction, - "SELECT value FROM " + Table + " WHERE server_id = $1 AND coalesce(database_name, '') = coalesce($2::text, '') AND threshold_key = $3", - serverId, database, wire); - return await command.ExecuteScalarAsync(ct) is double d ? d : null; - } - - private static async Task DeleteAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, int serverId, string? database, string wire, CancellationToken ct) - { - await using var command = Command(connection, transaction, - "DELETE FROM " + Table + " WHERE server_id = $1 AND coalesce(database_name, '') = coalesce($2::text, '') AND threshold_key = $3", - serverId, database, wire); - await command.ExecuteNonQueryAsync(ct); - } - - internal static async Task UpsertAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, int serverId, string? database, string wire, int value, - string setBy, Guid? requestId, CancellationToken ct) - { - await using var command = Command(connection, transaction, - "INSERT INTO " + Table + " (server_id, database_name, threshold_key, value, set_by, request_id) " + - "VALUES ($1, $2, $3, $4, $5, $6) " + - "ON CONFLICT (server_id, (coalesce(database_name, '')), threshold_key) DO UPDATE " + - "SET value = EXCLUDED.value, set_by = EXCLUDED.set_by, request_id = EXCLUDED.request_id, updated_at = now()", - serverId, database, wire); - command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Double, Value = (double)value }); - command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = setBy }); - command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Uuid, Value = (object?)requestId ?? DBNull.Value }); - await command.ExecuteNonQueryAsync(ct); - } } diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin.js index 1d97bdf685..e761e4e0d4 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin.js @@ -7,11 +7,12 @@ */ /* Admin: a window onto what the desktop's Manage Servers, Notification Routes and Settings windows show. - Four tabs, three of them one existing read: + Five tabs, three of them one existing read: Servers GET /api/admin/servers (every configured server, enabled or not) Notification Routes get_notification_routes Alert Settings get_alert_settings Pending Notes GET /api/investigation-notes/pending, with Approve and Reject (#5903) + Threshold Overrides GET /api/threshold-overrides, with Save, Edit and Remove under an admin store login (#5916) Routes and Settings change nothing. The Servers tab also lets a sign-in that can make changes edit one server in place (#5240): an Edit button on each row opens a form above the grid, filled from GET /api/admin/servers/{id}. The active tab rides in the hash (#/admin/) and in module scope, so the 60 s repaint keeps it. @@ -659,6 +660,7 @@ const TABS = [ { id: "routes", label: "Notification Routes" }, { id: "settings", label: "Alert Settings" }, { id: "pending", label: "Pending Notes" }, + { id: "overrides", label: "Threshold Overrides" }, ]; /* The tab last shown, so a repaint with a bare #/admin returns to it. */ @@ -1375,7 +1377,193 @@ async function buildPending(body, generation) { mount(body, nodes); } -const BUILDERS = { servers: buildServers, routes: buildRoutes, settings: buildSettings, pending: buildPending }; +/* ─────────────────────────── threshold overrides (#5916) ─────────────────────────── */ + +/* A threshold held for one server (or one database on it) that differs from the global value. The list is GET + /api/threshold-overrides; a change is PUT /api/threshold-overrides, accepted only under a store login with the admin role. + A managed or compose store connects this page as the viewer role, so there the grid is a list: the form and the Remove + buttons are not drawn, and the note says where to change one. The service checks the key and the bounds again. */ +export const OVERRIDE_COLUMNS = [ + { key: "server_name", label: "Server" }, + { key: "database_text", label: "Database" }, + { key: "key", label: "Key" }, + { key: "value", label: "Value" }, + { key: "global_value", label: "Global value" }, + { key: "set_by", label: "Set by" }, + { key: "request", label: "Request" }, + { key: "updated_at", label: "Updated", format: "time" }, +]; + +const OVERRIDES_NOTE_EDITING = + "A threshold held for one server, or for one database on it, that differs from the global value. The alert engine uses it for that server only " + + "and applies a change within its reload interval. To silence an alert, mute it: an override moves a threshold and never turns an alert off."; +const OVERRIDES_NOTE_READ_ONLY = + "A threshold held for one server, or for one database on it, that differs from the global value. This sign-in lists them but cannot change them: " + + "a managed or compose store connects this page as the viewer role. Change an override in the desktop Viewer, or (bring-your-own store) set " + + "postgres.webConnectionString to an admin login."; +const OVERRIDE_DENIED = "Changing a threshold override needs"; + +/** The rows of the overrides grid from the list read's body. `database` and `server_id` stay on the row for Edit and Remove. */ +export function overrideRows(data) { + const list = data && Array.isArray(data.overrides) ? data.overrides : []; + return list.map((o) => ({ + server_id: o.server_id, + server_name: o.server_name, + database: o.database || null, + database_text: o.database || "Whole server", + key: o.key, + value: o.value, + global_value: Number.isFinite(o.global_value) ? o.global_value : null, + set_by: o.set_by, + request: shortId(o.request_id), + updated_at: o.updated_at, + })); +} + +/** Why a form entry is refused before anything is sent, or null: the key must be one the service lists, the value an integer + inside its bounds, and a database only on a key that takes one. The service applies the same rules again. */ +export function validateOverride(keys, form) { + const list = Array.isArray(keys) ? keys : []; + if (!form || !String(form.server || "").trim()) return "Choose a server."; + const entry = list.find((k) => k.key === String(form.key || "").trim()); + if (!entry) return "Choose a key from the list: only a tunable threshold can be overridden."; + const text = String(form.value == null ? "" : form.value).trim(); + if (!/^-?\d+$/.test(text)) return "The value must be a whole number."; + const n = Number(text); + if (!Number.isSafeInteger(n) || n < entry.min || n > entry.max) { + return entry.key + " must be a whole number between " + entry.min + " and " + entry.max + ", the same bounds the global setting has."; + } + if (String(form.database || "").trim() && entry.database_scope !== true) { + return entry.key + " is compared against a server-level figure, so it takes a server override only, not a database."; + } + return null; +} + +/** What a save or remove answer says, in plain words: { message, isError, reread }. An expired session says nothing here. */ +export function overrideOutcome(verb, res) { + if (!res || res.expired) return { message: null, isError: true, reread: false }; + if (res.status === 0) return { message: res.message || "Network error.", isError: true, reread: false }; + const body = res.body && typeof res.body === "object" ? res.body : null; + if (res.status === 200 && body && (body.status === "ok" || body.status === "cleared")) { + return { message: body.status === "cleared" ? "Override removed. The global value applies again." : "Override saved.", isError: false, reread: true }; + } + if (res.status === 403 && body && typeof body.error === "string" && body.error.indexOf(OVERRIDE_DENIED) === 0) { + return { message: body.error, isError: true, reread: false }; + } + if (res.status === 403) return { message: "This sign-in is read-only, so it cannot change overrides.", isError: true, reread: false }; + const said = body && typeof body.error === "string" && body.error ? " " + body.error : ""; + return { + message: "The override was not " + (verb === "clear" ? "removed" : "saved") + "." + (res.status === 400 ? "" : " The service answered " + res.status + ".") + said, + isError: true, + reread: false, + }; +} + +/* The form's entries and the last answer, in module scope so the 60 s repaint keeps what was typed. */ +const EMPTY_OVERRIDE_FORM = { server: "", key: "", database: "", value: "" }; +let overrideEntries = { ...EMPTY_OVERRIDE_FORM }; +let overrideStatus = null; +let overrideBusy = false; + +const overrideStatusNode = () => (overrideStatus ? (overrideStatus.isError ? errorStrip(overrideStatus.message) : noticeStrip(overrideStatus.message)) : null); + +function redrawOverrides() { + return shownTab === "overrides" && shownBody ? startBuilder("overrides", shownBody, ++renderGeneration) : undefined; +} + +async function changeOverride(verb, entry) { + if (overrideBusy) return; + overrideBusy = true; + let out; + try { + const payload = { server_name: entry.server, key: entry.key, database_name: entry.database || null }; + if (verb === "clear") payload.clear = true; + else payload.value = Number(String(entry.value).trim()); + out = overrideOutcome(verb, await apiWrite("PUT", "/api/threshold-overrides", payload)); + } catch (e) { + out = { message: "Could not change the override: " + (e && e.message ? e.message : String(e)), isError: true, reread: false }; + } + overrideBusy = false; + if (out.message) overrideStatus = { message: out.message, isError: out.isError }; + if (out.isError === false && verb !== "clear") overrideEntries = { ...EMPTY_OVERRIDE_FORM }; + await redrawOverrides(); +} + +function overrideFormNode(servers, keys) { + const field = (label, control) => el("label", { class: "tag-field" }, [el("span", { class: "mute-label", text: label }), control]); + const choose = (items, textOf) => [el("option", { value: "", text: "(choose)" })].concat(items.map((i) => el("option", { value: textOf(i), text: textOf(i) }))); + const server = el("select", { class: "tag-input", "data-field": "server" }, choose(servers, (s) => s.server_name)); + const key = el("select", { class: "tag-input", "data-field": "key" }, choose(keys, (k) => k.key)); + const database = el("input", { type: "text", class: "tag-input", "data-field": "database", placeholder: "blank = whole server" }); + const value = el("input", { type: "text", class: "tag-input", "data-field": "value", inputmode: "numeric" }); + server.value = overrideEntries.server; + key.value = overrideEntries.key; + database.value = overrideEntries.database; + value.value = overrideEntries.value; + const sync = () => { overrideEntries = { server: server.value, key: key.value, database: database.value, value: value.value }; }; + for (const c of [server, key, database, value]) c.addEventListener("input", sync); + const save = el("button", { + class: "btn primary", type: "button", text: "Save override", "data-action": "save-override", + onClick: async () => { + sync(); + const bad = validateOverride(keys, overrideEntries); + if (bad) { + overrideStatus = { message: bad, isError: true }; + await redrawOverrides(); + return; + } + await changeOverride("set", overrideEntries); + }, + }); + return el("div", { class: "tag-form", "data-override-form": "1" }, [ + field("Server:", server), field("Key:", key), field("Database:", database), field("Value:", value), save, + ]); +} + +function overrideActions(row) { + const edit = el("button", { + class: "btn", type: "button", text: "Edit", "aria-label": "Edit " + row.key + " for " + row.server_name, + onClick: async () => { + overrideEntries = { server: row.server_name, key: row.key, database: row.database || "", value: String(row.value) }; + await redrawOverrides(); + }, + }); + const remove = el("button", { + class: "btn", type: "button", text: "Remove", "aria-label": "Remove " + row.key + " for " + row.server_name, "data-action": "remove-override", + onClick: () => changeOverride("clear", { server: row.server_name, key: row.key, database: row.database }), + }); + return el("span", { class: "row-actions" }, [edit, remove]); +} + +/** The grid's columns: OVERRIDE_COLUMNS, plus Edit and Remove for a seat that can change overrides. */ +export function overrideColumns(canEdit) { + return canEdit ? OVERRIDE_COLUMNS.concat([{ key: "actions", label: "Change", csv: false, copy: false, render: overrideActions }]) : OVERRIDE_COLUMNS; +} + +async function buildOverrides(body, generation) { + const [session, res] = await Promise.all([getSession(), apiGet("/api/threshold-overrides")]); + if (generation !== renderGeneration) return; + const nodes = []; + const status = overrideStatusNode(); + if (status) nodes.push(status); + const bad = failure(res); + if (bad) return mount(body, nodes.concat([bad])); + /* Both must agree: the session says the seat may write at all (a read-only seat is refused every PUT), and the list says + the store login holds the write grant on this table. */ + const canEdit = session.can_edit === true && !!res.data && res.data.can_edit === true; + const keys = Array.isArray(res.data && res.data.keys) ? res.data.keys : []; + nodes.push(noticeStrip(canEdit ? OVERRIDES_NOTE_EDITING : OVERRIDES_NOTE_READ_ONLY)); + if (res.data && typeof res.data.note === "string" && res.data.note) nodes.push(noticeStrip(res.data.note)); + if (canEdit) nodes.push(overrideFormNode(Array.isArray(res.data.servers) ? res.data.servers : [], keys)); + nodes.push(VIZ.table({ overrides: overrideRows(res.data) }, { + rowsKey: "overrides", + columns: overrideColumns(canEdit), + emptyText: "No server holds an override: every server uses the global values.", + })); + mount(body, nodes); +} + +const BUILDERS = { servers: buildServers, routes: buildRoutes, settings: buildSettings, pending: buildPending, overrides: buildOverrides }; /* Run a tab's builder. A builder that throws leaves an error strip: in the Servers tab's table area when its boxes are on the page (the form above it is never replaced), else in the body. */ diff --git a/Darling/PerformanceMonitor.Darling.Storage/ThresholdOverrideStore.cs b/Darling/PerformanceMonitor.Darling.Storage/ThresholdOverrideStore.cs new file mode 100644 index 0000000000..16b9a81ef5 --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Storage/ThresholdOverrideStore.cs @@ -0,0 +1,146 @@ +/* + * 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.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Npgsql; +using NpgsqlTypes; + +namespace PerformanceMonitor.Darling.Storage; + +/// One row of the threshold-overrides grid (#5916 part 4): the override, its server's name, and the global value the +/// override replaces (null when the store has no global row or no such column yet). +public sealed record ThresholdOverrideRow( + int ServerId, string ServerName, string? Database, string Key, double Value, double? GlobalValue, + string SetBy, Guid? RequestId, DateTime UpdatedAt); + +/// +/// The statements behind config.config_threshold_overrides (#5916), shared by the set_alert_threshold_override tool, +/// the web admin route and the Viewer's grid so the three cannot write the table three ways. Each takes the caller's connection +/// and transaction (the tool journals its write in the same transaction) or a data source (the list). Key and bound checks are +/// not here: they are DarlingThresholdKeys' (PerformanceMonitor.Alerting), applied by every caller before any write. +/// +public static class ThresholdOverrideStore +{ + /// The table, schema-qualified; the mcp role holds SELECT, INSERT, UPDATE and DELETE on it (#5920). + public const string Table = "config.config_threshold_overrides"; + + private const int CommandTimeoutSeconds = 30; + + /// A person's login shaped to what the store's set_by accepts, 1-64 of A-Za-z0-9._:-: anything else becomes + /// _, an empty name becomes . The web seat's editor name and the Viewer's Windows login both pass here. + public static string SetByFor(string? name, string fallback = "web") + { + var chars = new char[Math.Min(name?.Length ?? 0, 64)]; + for (var i = 0; i < chars.Length; i++) + { + var c = name![i]; + chars[i] = char.IsAsciiLetterOrDigit(c) || c is '.' or '_' or ':' or '-' ? c : '_'; + } + + return chars.Length == 0 ? fallback : new string(chars); + } + + private static NpgsqlCommand Command(NpgsqlConnection connection, NpgsqlTransaction transaction, string sql, int serverId, string? database, string wire) + { + var command = new NpgsqlCommand(sql, connection) { Transaction = transaction, CommandTimeout = CommandTimeoutSeconds }; + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Integer, Value = serverId }); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = (object?)database ?? DBNull.Value }); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = wire }); + return command; + } + + public static async Task ReadValueAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, int serverId, string? database, string wire, + CancellationToken ct) + { + await using var command = Command(connection, transaction, + "SELECT value FROM " + Table + " WHERE server_id = $1 AND coalesce(database_name, '') = coalesce($2::text, '') AND threshold_key = $3", + serverId, database, wire); + return await command.ExecuteScalarAsync(ct) is double d ? d : null; + } + + public static async Task DeleteAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, int serverId, string? database, string wire, CancellationToken ct) + { + await using var command = Command(connection, transaction, + "DELETE FROM " + Table + " WHERE server_id = $1 AND coalesce(database_name, '') = coalesce($2::text, '') AND threshold_key = $3", + serverId, database, wire); + await command.ExecuteNonQueryAsync(ct); + } + + public static async Task UpsertAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, int serverId, string? database, string wire, int value, + string setBy, Guid? requestId, CancellationToken ct) + { + await using var command = Command(connection, transaction, + "INSERT INTO " + Table + " (server_id, database_name, threshold_key, value, set_by, request_id) " + + "VALUES ($1, $2, $3, $4, $5, $6) " + + "ON CONFLICT (server_id, (coalesce(database_name, '')), threshold_key) DO UPDATE " + + "SET value = EXCLUDED.value, set_by = EXCLUDED.set_by, request_id = EXCLUDED.request_id, updated_at = now()", + serverId, database, wire); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Double, Value = (double)value }); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = setBy }); + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Uuid, Value = (object?)requestId ?? DBNull.Value }); + await command.ExecuteNonQueryAsync(ct); + } + + /// The list statement: every override with its server's name, ordered by server, database then key. The name resolves through the + /// registry the override's foreign key points at, so a row always has one. + public const string ListSql = + "SELECT o.server_id, s.name, o.database_name, o.threshold_key, o.value, o.set_by, o.request_id, o.updated_at " + + "FROM " + Table + " o JOIN config.config_monitored_servers s ON s.server_id = o.server_id " + + "ORDER BY s.name, o.database_name NULLS FIRST, o.threshold_key"; + + /// The global row as JSON, so each key's own column is read by name and a column an older store lacks reads as absent + /// rather than failing the whole list. + public const string GlobalRowSql = "SELECT to_jsonb(a) FROM config.config_alert_settings a WHERE id = 1"; + + /// + /// Every override with the global value it replaces. maps a wire name to the global setting's + /// column on config.config_alert_settings (null for a key this build does not know, which keeps its row and shows no + /// global value). A role without SELECT on the table raises 42501, which the caller reports. + /// + public static async Task> ListAsync( + NpgsqlDataSource postgres, Func columnFor, CancellationToken ct) + { + JsonElement? global = null; + try + { + await using var globalCommand = postgres.CreateCommand(GlobalRowSql); + globalCommand.CommandTimeout = CommandTimeoutSeconds; + if (await globalCommand.ExecuteScalarAsync(ct) is string json) + global = JsonDocument.Parse(json).RootElement.Clone(); + } + catch (PostgresException ex) when (ex.SqlState == PostgresErrorCodes.InsufficientPrivilege) + { + global = null; + } + + var rows = new List(); + await using var command = postgres.CreateCommand(ListSql); + command.CommandTimeout = CommandTimeoutSeconds; + await using var reader = await command.ExecuteReaderAsync(ct); + while (await reader.ReadAsync(ct)) + { + var key = reader.GetString(3); + double? globalValue = null; + if (global is { } g && columnFor(key) is { } column + && g.TryGetProperty(column, out var element) && element.ValueKind == JsonValueKind.Number) + { + globalValue = element.GetDouble(); + } + + rows.Add(new ThresholdOverrideRow( + reader.GetInt32(0), reader.GetString(1), reader.IsDBNull(2) ? null : reader.GetString(2), key, reader.GetDouble(4), globalValue, + reader.GetString(5), reader.IsDBNull(6) ? null : reader.GetGuid(6), reader.GetDateTime(7))); + } + + return rows; + } +} diff --git a/Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml b/Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml index 85ece5503f..5124545fcc 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml +++ b/Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml @@ -415,6 +415,13 @@ + +