Say the RDS log was refused, instead of that it held no plans

CHANGELOG.md

@@ -73,6 +73,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Lite's portable ZIP is self-contained, which HALVED it** ([#2501]) - `Publish Lite` is now `-r win-x64 --self-contained` in both `build.yml` and `nightly.yml`, so neither Lite artifact has a .NET prerequisite any more and the failure [#2489] documented stops existing: a tester who unzips onto a stock Windows Server no longer meets the .NET host's bare `You must install .NET to run this application` before a line of our code runs. **The size went the opposite way from what bundling a runtime suggests.** The old publish was RID-agnostic, so it copied every platform its packages ship - **537 MB of `runtimes\` on a 565 MB tree** (osx 130, linux-x64 116, linux-arm64 70, win-arm64 56, then win-x86, musl, loongarch64 and riscv64), of which only the **52 MB `win-x64`** folder could ever load on Windows. `DuckDB.NET.Bindings.Full` is most of it, SkiaSharp and SqlClient behind it. Dropping ~485 MB of unloadable native payload beats the cost of bundling .NET, WPF and ASP.NET Core by roughly two to one: measured on one commit and one SDK, **565 MB tree / 212.7 MB zipped becomes 277 MB / 114.2 MB**. It matters most for the **nightly** ZIP, which is the UAT download and is not offered as a `Setup.exe` at all. **A RID-specific publish needed two more files than the flag.** `Lite/packages.lock.json` had only a `net10.0-windows7.0` target, and a RID restore adds `net10.0-windows7.0/win-x64` to it - after which the `dotnet restore --locked-mode` that BOTH workflows run before the publish fails `NU1004: the project's runtime identifiers have changed`, because locked mode compares the PROJECT's RID set (empty) against the lock file's (win-x64). Reproduced locally; that is a red CI run on every PR, not the future `--no-restore` trap it was filed as. The fix is `<RuntimeIdentifiers>win-x64</RuntimeIdentifiers>` in `PerformanceMonitorLite.csproj`, so the project itself asks for that graph and one committed lock file satisfies the RID-less locked-mode restore and the RID publish alike; `RuntimeIdentifiers` (plural) sets no RID on the build, so a plain `dotnet build` stays RID-agnostic and `Lite.Tests` is untouched. **SignPath needed nothing** - the `Lite` artifact-configuration slug already receives both shapes today, and the signed re-zip reads `signed/Lite/*`, inheriting whatever shape `publish/Lite` has. Auto-update is unaffected; the ZIP is not a Velopack channel. `LiteRuntimePrerequisiteDocsTests` went red on the flag alone (3 of its 7 facts) and was rewritten to state every claim BOTH ways round: [#2499]'s version asserted only that the docs DID name the runtimes, so two of its facts stayed green while the prose went stale. It now also derives the lock file's RID coverage from the `-r` flags in the workflows, and every new assertion was proven red with its fix reverted.
 
 ### Fixed
+- **RDS plan capture reported SUCCESS "no new plans" when the AWS call was DENIED** ([#2633]) - the ingestor caught every failure, warned, and returned zero rows, and the runner turned zero into a `collection_log` row asserting the log had been opened and held nothing. Measured on the PostgreSQL monitoring host: the row said SUCCESS while the app log said `rds:DescribeDBLogFiles` was denied by IAM - nothing had been read. A regression against the route it replaced, since the `pg_read_file` path answers the same situation with PERMISSIONS and names the grant. An authorization refusal now degrades to PERMISSIONS with a message saying the grant is on the MONITORING HOST's IAM role rather than the database login, and that nothing was read; every other failure stays loud, because a permanent-sounding status on a transient fault is how an outage gets read as a configuration choice.
+- **`IsAwsRds` was never set on a PostgreSQL target, so plain RDS PostgreSQL could never capture plans** ([#2633]) - it is probed with a T-SQL detection query on the SQL Server path only, so `pg_plan_capture`'s `IsAurora || IsAwsRds` dispatch had an unreachable half and a managed non-Aurora instance fell to the `pg_read_file` route, where there is no filesystem to read and the failure names a database grant that would never have helped. Now derived from the endpoint. Aurora was unaffected because `IsAurora` carried the routing, which is exactly why the fleet could not show it.
 - **A summary count taken over a capped result read as a fact about the server** ([#2629]) - caught in the new `get_pg_extensions` before it shipped: at a 50-row limit it reported `installed: 10` for a server with 13, because 50 rows was all it had looked at. State totals, the measured-index count and the ungranted-lock count are now WITHHELD when the result is truncated, with `truncated: true` and a sentence saying why, rather than renamed to something nobody wants to read.
 - **`pg_wait_sampling` excluded only `Activity`, so `Client`/`ClientRead` was 100% of the profile** ([#2630]) - its Aurora sibling excludes `Activity`, `Client` and `Timeout`, with a rationale measured on production, and the sampler restated a shorter list. On the first target profiled with real client connections, `ClientRead` was 2,717,290 of 2,717,989 samples and every real event rounded to zero; the unfiltered profile held 17,864,575 samples of those three types against 2,150 of everything else. Both collectors now splice ONE definition of what does not count as a wait - they answer the same question from different sources, and #2625 tells operators to read one instead of the other. The predicate coalesces before it compares, so the CPU row - a backend that was NOT waiting, and this collector's distinctive signal - survives a filter that would otherwise silently discard it as NULL.
 - **An unknown column `format:` rendered as raw text instead of failing** ([#2629]) - the web grid renderer falls through silently, so a column declaring a format that does not exist still renders, looks populated, and is simply wrong. Caught while writing one. Every format `server-tabs.js` declares is now pinned against the renderer's own vocabulary.

Darling/Darling.Tests/RdsLogUnavailableTests.cs

@@ -0,0 +1,162 @@
+/*
+ * 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.RegularExpressions;
+using Npgsql;
+using PerformanceMonitor.Darling.Service.Targets;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #2633: "could not read the log" and "read the log, it held nothing" were the same value, and the store
+/// reported the second one.
+///
+/// <para>
+/// Measured on the PostgreSQL monitoring host after deploying the RDS log-API route. <c>collection_log</c>
+/// said <c>SUCCESS | rows=0 | no new auto_explain plans in the RDS log window</c>. The app log said
+/// <c>rds:DescribeDBLogFiles</c> was denied by IAM. Nothing had been read; the row asserted the log was
+/// opened and was empty.
+/// </para>
+///
+/// <para>
+/// It was also a REGRESSION against the route it replaced: <c>pg_read_file</c> answers the same situation
+/// with <c>PERMISSIONS</c> and a message naming the grant. The managed path — the one a real fleet is on —
+/// was the one that went quiet, and the app-log warning is not a substitute for the column collection
+/// health is actually read from.
+/// </para>
+/// </summary>
+public sealed class RdsLogUnavailableTests
+{
+    /// <summary>
+    /// The message shape measured on the fleet, verbatim past the identifiers. Matched on the SENTENCE and
+    /// not only on an SDK exception type, because the refusal arrives in more than one shape depending on
+    /// the call.
+    /// </summary>
+    private const string FleetDenial =
+        "User: arn:aws:sts::000000000000:assumed-role/example-monitor-role/i-0example is not authorized to "
+        + "perform: rds:DescribeDBLogFiles on resource: arn:aws:rds:us-east-1:000000000000:db:example-1 "
+        + "because no identity-based policy allows the rds:DescribeDBLogFiles action";
+
+    [Fact]
+    public void TheFleetsOwnDenialMessage_IsRecognisedAsAnAuthorizationRefusal()
+        => Assert.True(RdsLogUnavailableException.IsAuthorizationRefusal(new InvalidOperationException(FleetDenial)));
+
+    [Theory]
+    [InlineData("AccessDenied")]
+    [InlineData("AccessDeniedException: not authorised")]
+    [InlineData("User: x is not authorized to perform: rds:DownloadDBLogFilePortion")]
+    public void TheOtherShapesTheSdkUses_AreRecognisedToo(string message)
+        => Assert.True(RdsLogUnavailableException.IsAuthorizationRefusal(new InvalidOperationException(message)));
+
+    /// <summary>
+    /// It is found through the INNER exception too — the SDK wraps, and a refusal that arrives nested must
+    /// not be classified as an unknown fault and reported as a hard error.
+    /// </summary>
+    [Fact]
+    public void ARefusalNestedInsideAWrapper_IsStillFound()
+        => Assert.True(RdsLogUnavailableException.IsAuthorizationRefusal(
+            new InvalidOperationException("reading the log failed", new InvalidOperationException(FleetDenial))));
+
+    /// <summary>
+    /// And everything else stays LOUD. A throttle, a failover or an endpoint that stopped resolving is not
+    /// a configuration choice, and giving it a permanent-sounding status is how a real outage gets read as
+    /// one. The store's own rule: an unclassified failure must be loud rather than quietly swallowed.
+    /// </summary>
+    [Theory]
+    [InlineData("Rate exceeded")]
+    [InlineData("The DB instance is currently in a failover state")]
+    [InlineData("The specified DB instance was not found")]
+    [InlineData("A connection attempt failed")]
+    public void ATransientOrUnknownFailure_IsNotAnAuthorizationRefusal(string message)
+        => Assert.False(RdsLogUnavailableException.IsAuthorizationRefusal(new InvalidOperationException(message)));
+
+    [Fact]
+    public void TheExceptionCarriesTheClassificationAndTheOriginalMessage()
+    {
+        var inner = new InvalidOperationException(FleetDenial);
+
+        var ex = new RdsLogUnavailableException(
+            inner.Message, RdsLogUnavailableException.IsAuthorizationRefusal(inner), inner);
+
+        Assert.True(ex.IsAuthorizationFailure);
+        Assert.Same(inner, ex.InnerException);
+        Assert.Contains("rds:DescribeDBLogFiles", ex.Message, StringComparison.Ordinal);
+    }
+
+    /// <summary>
+    /// The source pin on the half that caused the defect. The ingestor must not turn a failure into a row
+    /// COUNT: returning zero there is indistinguishable from an empty log one frame later, and the runner
+    /// stamps that with a sentence claiming the log was read.
+    /// </summary>
+    [Fact]
+    public void TheIngestorRethrows_RatherThanReturningZeroRowsOnFailure()
+    {
+        var source = File.ReadAllText(Path.Combine(RepoRoot(),
+            "Darling", "PerformanceMonitor.Darling.Service", "Targets", "RdsPlanIngestor.cs"));
+
+        var catchIndex = source.IndexOf("catch (Exception ex) when (ex is not OperationCanceledException)", StringComparison.Ordinal);
+        Assert.True(catchIndex >= 0, "The ingestor's tolerant catch is gone — this pin needs re-anchoring.");
+
+        var body = source[catchIndex..];
+        var close = body.IndexOf("\n        }", StringComparison.Ordinal);
+        body = close > 0 ? body[..close] : body;
+
+        Assert.Contains("throw new RdsLogUnavailableException", body, StringComparison.Ordinal);
+        Assert.DoesNotMatch(new Regex(@"return\s+0\s*;"), body);
+    }
+
+    /// <summary>
+    /// And the pin on the second defect: <c>IsAwsRds</c> was never assigned on the PostgreSQL connect path,
+    /// so <c>pg_plan_capture</c>'s <c>IsAurora || IsAwsRds</c> dispatch had an unreachable half and plain
+    /// RDS PostgreSQL — managed, no filesystem, not Aurora — fell to the <c>pg_read_file</c> route.
+    ///
+    /// <para>The fleet could never have shown this: every PostgreSQL target on it is Aurora, so
+    /// <c>IsAurora</c> carried the routing and the dead half was never load-bearing.</para>
+    /// </summary>
+    [Fact]
+    public void ThePostgresConnectPath_SetsIsAwsRds()
+    {
+        var source = File.ReadAllText(Path.Combine(RepoRoot(),
+            "Darling", "PerformanceMonitor.Darling.Service", "DarlingServerConnector.cs"));
+
+        var pgIndex = source.IndexOf("Engine = CollectorTargetEngine.PostgreSql,", StringComparison.Ordinal);
+        Assert.True(pgIndex >= 0, "The PostgreSQL target construction moved — this pin needs re-anchoring.");
+
+        var block = source[pgIndex..];
+        var close = block.IndexOf("\n            },", StringComparison.Ordinal);
+        block = close > 0 ? block[..close] : block;
+
+        Assert.Contains("IsAwsRds", block, StringComparison.Ordinal);
+    }
+
+    /// <summary>
+    /// The derivation itself, against the endpoint shapes it has to separate. A managed host is RDS; a
+    /// self-hosted one is not, and must keep the file route that works there.
+    /// </summary>
+    [Theory]
+    [InlineData("example-1.abcdefghijkl.us-east-1.rds.amazonaws.com", true)]
+    [InlineData("example-cluster.cluster-abcdefghijkl.us-east-1.rds.amazonaws.com", true)]
+    [InlineData("localhost", false)]
+    [InlineData("db.internal.example.com", false)]
+    public void TheEndpointDecidesWhetherATargetIsManaged(string host, bool expected)
+        => Assert.Equal(expected, RdsEndpoint.TryParse(host) is not null);
+
+    private static string RepoRoot()
+    {
+        var directory = new DirectoryInfo(AppContext.BaseDirectory);
+        while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "PerformanceMonitor.sln")))
+        {
+            directory = directory.Parent;
+        }
+
+        return directory?.FullName ?? throw new InvalidOperationException("PerformanceMonitor.sln not found above the test output directory.");
+    }
+}

Darling/PerformanceMonitor.Darling.Service/DarlingServerConnector.cs

@@ -13,6 +13,7 @@
 using Microsoft.Data.SqlClient;
 using Microsoft.Extensions.Logging;
 using Npgsql;
+using PerformanceMonitor.Darling.Service.Targets;
 using PerformanceMonitor.Collectors;
 using PerformanceMonitor.Common;
 
@@ -370,6 +371,15 @@ private static async Task<ServerRuntime> ConnectPostgresAsync(
                 PostgresMajorVersion = majorVersion,
                 PostgresVersionNum = versionNum,
                 IsAurora = isAurora,
+                /* #2633: derived from the ENDPOINT, because nothing else here can see it. IsAwsRds is
+                   probed with a T-SQL detection query on the SQL Server path, so before this it was
+                   silently false for every PostgreSQL target — which made the second half of
+                   pg_plan_capture's `IsAurora || IsAwsRds` dispatch unreachable and sent plain RDS
+                   PostgreSQL down the pg_read_file route, where a managed instance has no filesystem to
+                   read and the failure names a grant that would never have helped. Aurora was unaffected
+                   because IsAurora carries it, which is why the fleet never showed this. */
+                IsAwsRds = RdsEndpoint.TryParse(
+                    new NpgsqlConnectionStringBuilder(connectionString).Host) is not null,
                 IsInRecovery = isInRecovery,
             },
             StorageName = storageName,

Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs

@@ -4477,6 +4477,30 @@ await DarlingObservability.LogCollectionAsync(
                 _postgres!, runtime, collectorName, "SESSION_MISSING", 0, 0, 0, ex.Message, fanout: null, _logger, cancellationToken);
             return 0;
         }
+        catch (RdsLogUnavailableException ex) when (ex.IsAuthorizationFailure)
+        {
+            /* #2633: the AWS call was DENIED, so nothing was read. Degraded to PERMISSIONS rather than
+               ERROR for the same reason a 42501 from the pg_read_file route is — a least-privilege
+               deployment is an expected state an operator can act on, and screaming every cycle about it
+               would bury real faults — but it must NOT be recorded as a successful empty read, which is
+               what returning zero rows used to make it.
+
+               Only the authorization case lands here. A throttle, a failover or an endpoint that stopped
+               resolving falls through to the general handler and stays loud, because a permanent-sounding
+               status on a transient fault is how an outage gets read as a configuration choice. */
+            _logger.LogWarning("  [{Server}] {Collector} => PERMISSIONS: the RDS log API refused the call",
+                server.Config.DisplayName, collectorName);
+
+            await DarlingObservability.LogCollectionAsync(
+                _postgres!, runtime, collectorName, "PERMISSIONS", 0, 0, 0,
+                $"{ex.Message} — the MONITORING HOST's IAM role lacks a grant this source needs, which is "
+                + "not a database grant: plan capture on managed PostgreSQL reads the server log through "
+                + "the RDS API, so the role needs rds:DescribeDBLogFiles and rds:DownloadDBLogFilePortion "
+                + "on the target instance. Nothing was read this cycle — this is NOT 'no plans were "
+                + "captured'.",
+                fanout: null, _logger, cancellationToken);
+            return 0;
+        }
         catch (SqlException ex) when (ex.Number == 1222 && CollectorCatalog.YieldsOnLockTimeout(collectorName))
         {
             /* The 1-second LOCK_TIMEOUT guard doing its job (#1805): the snapshot sweep stepped aside