Declare each collector's PostgreSQL extension dependency and pin the README to it

Darling/Darling.Tests/ReadmeDerivedCountPinTests.cs

@@ -77,6 +77,84 @@ public sealed class ReadmeDerivedCountPinTests
                 IsInRecovery = false,
             }));
 
+    /// <summary>
+    /// A self-hosted PostgreSQL 17 writer, for the gates that need a target to answer.
+    /// </summary>
+    private static CollectorTargetInfo SelfHostedPg17 => new()
+    {
+        Engine = CollectorTargetEngine.PostgreSql,
+        PostgresMajorVersion = 17,
+        PostgresVersionNum = 170007,
+        IsAurora = false,
+        IsInRecovery = false,
+    };
+
+    /// <summary>
+    /// Every collector/extension dependency the catalog declares, as <c>collector/extension</c> — the
+    /// authority the permissions paragraph's list is pinned to (#3187).
+    ///
+    /// <para><b>Not <c>PgExtensionAvailabilityCollector</c>'s roster</b>, which is the nearby list that
+    /// looks like this one and answers a different question. That roster exists so ABSENCE is reportable,
+    /// so it carries <c>hypopg</c>, <c>pg_trgm</c> and <c>pg_cron</c> — which no collector reads — and
+    /// deliberately omits <c>pg_wait_sampling</c>, which a collector does. Pinning prose to it would be
+    /// wrong in both directions at once.</para>
+    /// </summary>
+    private static IReadOnlySet<string> DependencyPairs =>
+        CollectorCatalog.All
+            .SelectMany(c => c.RequiredPgExtensions.Select(e => $"{c.Name}/{e.ExtensionName}"))
+            .ToHashSet(StringComparer.Ordinal);
+
+    /// <summary>
+    /// The declared extensions whose install needs <c>shared_preload_libraries</c> and a server restart.
+    /// The expensive half of the paragraph's advice: a <c>CREATE EXTENSION</c> is a statement, this is a
+    /// maintenance window.
+    /// </summary>
+    private static IReadOnlySet<string> PreloadExtensions =>
+        CollectorCatalog.All
+            .SelectMany(c => c.RequiredPgExtensions)
+            .Where(e => e.InstallKind == PgExtensionInstallKind.SharedPreloadLibraries)
+            .Select(e => e.ExtensionName)
+            .ToHashSet(StringComparer.Ordinal);
+
+    /// <summary>
+    /// The declared extensions that have to be created in EVERY database rather than only the connect
+    /// database — derived from the collector's own <c>RunsPerDatabase</c> rather than declared a second
+    /// time, because a per-database read is what makes its extension per-database.
+    ///
+    /// <para>Reached by reflection, and that is the one weakness here: <c>RunsPerDatabase</c> is declared
+    /// on <c>ICollectorDefinition&lt;TRow&gt;</c> rather than on the non-generic surface
+    /// <see cref="CollectorCatalog.All"/> exposes, so it cannot be asked without the row type. A missing
+    /// method is asserted rather than treated as false — a reflection lookup that silently stops finding
+    /// anything would turn this pin into a claim that no extension is per-database.</para>
+    /// </summary>
+    private static IReadOnlySet<string> PerDatabaseExtensions
+    {
+        get
+        {
+            var found = new HashSet<string>(StringComparer.Ordinal);
+
+            foreach (var definition in CollectorCatalog.All.Where(c => c.RequiredPgExtensions.Count > 0))
+            {
+                var method = definition.GetType().GetMethod(
+                    "RunsPerDatabase", new[] { typeof(CollectorTargetInfo) });
+
+                Assert.True(method is not null,
+                    $"{definition.Name}: RunsPerDatabase(CollectorTargetInfo) was not found by reflection, so "
+                    + "the per-database set cannot be derived and this pin would silently report the empty set.");
+
+                if ((bool)method!.Invoke(definition, new object[] { SelfHostedPg17 })!)
+                {
+                    foreach (var extension in definition.RequiredPgExtensions)
+                    {
+                        found.Add(extension.ExtensionName);
+                    }
+                }
+            }
+
+            return found;
+        }
+    }
+
     /// <summary>
     /// The pinned sentences. Each entry is a NAMED pattern plus the derived values its capture groups must
     /// equal, so a failure says which sentence and which number rather than "a regex did not match".
@@ -114,6 +192,56 @@ public sealed class ReadmeDerivedCountPinTests
             () => new[] { PostgresCollectors });
     }
 
+    /// <summary>
+    /// The pinned NAME SETS — the same guard as <see cref="Pins"/> one resolution finer. A count going
+    /// stale and a list going stale are the same defect, and the list is the one that shipped wrong: the
+    /// permissions paragraph was once written naming four of the six collectors that need an extension,
+    /// with every check green on that commit, because an enumeration is only better than a count if
+    /// something breaks when it is incomplete.
+    ///
+    /// <para>Each entry captures ONE span of the README and turns it into a set, which is then required to
+    /// equal what the catalog declares — in BOTH directions. Missing catches the defect that shipped;
+    /// extra catches its mirror image, a paragraph claiming a dependency nothing has.</para>
+    /// </summary>
+    private static IEnumerable<(
+        string Name,
+        Regex Pattern,
+        Func<IReadOnlySet<string>> Expected,
+        Func<string, IReadOnlySet<string>> Extract)> SetPins()
+    {
+        /* `collector` (`extension`  /  `collector` (the `extension` module — the second shape is
+           pg_wait_sampling, whose extension and collector share a name. The FIRST backticked name inside
+           the parentheses is the extension, which is what lets an entry carry a trailing aside
+           ("pg_stat_kcache, which sits on top of pg_stat_statements") without that aside reading as a
+           second dependency. */
+        var pair = new Regex(@"`([a-z0-9_]+)` \((?:the )?`([a-z0-9_]+)`");
+
+        yield return (
+            "the extension-dependent collector list",
+            new Regex(@"The collectors that need something installed, named rather than counted: (.+?)\. Every other collector needs nothing installed"),
+            () => DependencyPairs,
+            span => pair.Matches(span)
+                .Select(m => $"{m.Groups[1].Value}/{m.Groups[2].Value}")
+                .ToHashSet(StringComparer.Ordinal));
+
+        yield return (
+            "the shared_preload_libraries sentence",
+            new Regex(@"\*\*Needing `shared_preload_libraries` and a server restart\*\* \(a parameter-group change plus a reboot on Aurora/RDS\), and inert until then whatever else is installed: (.+?)\."),
+            () => PreloadExtensions,
+            BacktickedNames);
+
+        yield return (
+            "the per-database extension sentence",
+            new Regex(@"\*\*Additionally per database\*\*, because the collectors that read them run per database and `CREATE EXTENSION` in one database does not create them in another: (.+?)\."),
+            () => PerDatabaseExtensions,
+            BacktickedNames);
+    }
+
+    private static IReadOnlySet<string> BacktickedNames(string span) =>
+        Regex.Matches(span, @"`([a-z0-9_]+)`")
+            .Select(m => m.Groups[1].Value)
+            .ToHashSet(StringComparer.Ordinal);
+
     [Fact]
     public void EveryPinnedSentence_MatchesTheCatalog()
     {
@@ -132,6 +260,21 @@ public void EveryPinnedSentence_MatchesTheCatalog()
     /// <see cref="DarlingCliCommandsTests"/>, which already derives them from the catalog for the real output.
     /// The README's copy of that output was the part nothing held.
     /// </summary>
+    [Fact]
+    public void EveryPinnedNameSet_MatchesTheCatalog()
+    {
+        var readme = ReadReadme();
+
+        /* Both facts over SetPins() iterate it, so an EMPTY SetPins() would pass both while checking
+           nothing — a pin that cannot fail, which is the shape this whole file exists to refuse. */
+        Assert.NotEmpty(SetPins());
+
+        foreach (var (name, pattern, expected, extract) in SetPins())
+        {
+            AssertSetPin(readme, name, pattern, expected(), extract);
+        }
+    }
+
     [Fact]
     public void TestConnectionTranscript_DenominatorsMatchThePostgresCollectorCount()
     {
@@ -154,6 +297,12 @@ public void TestConnectionTranscript_DenominatorsMatchThePostgresCollectorCount(
     /// does it: mutate a COPY of the README so each pinned number is wrong, and require the identical
     /// extraction to REPORT the mutation. If any of these passed, that pin's regex matched nothing and its
     /// assertion in <see cref="EveryPinnedSentence_MatchesTheCatalog"/> was doing nothing.
+    ///
+    /// <para>Both families are proved here rather than only the numeric one. A set pin cannot be mutated by
+    /// bumping a digit, so it is mutated by DROPPING a name — once per member, so that every name in the
+    /// sentence is individually shown to be load-bearing. One mutation per pin would only prove the
+    /// sentence is read at all, which is not the property that failed: the paragraph was read, and was
+    /// short two names.</para>
     /// </summary>
     [Fact]
     public void EveryPin_ReportsAnInjectedDrift()
@@ -189,6 +338,82 @@ public void EveryPin_ReportsAnInjectedDrift()
                 Assert.Equal(want[i] + 1, values[i]);
             }
         }
+
+        Assert.NotEmpty(SetPins());
+
+        foreach (var (name, pattern, expected, extract) in SetPins())
+        {
+            var real = pattern.Match(readme);
+            Assert.True(real.Success, $"{name}: pattern matched nothing, so its pin is vacuous.");
+
+            var want = expected();
+            Assert.NotEmpty(want);
+
+            foreach (var member in want)
+            {
+                /* The token the README actually spells. A pair member is collector/extension and the README
+                   spells both, so either half proves the point; the extension half is taken because it is
+                   the half that went missing. */
+                var slash = member.IndexOf('/', StringComparison.Ordinal);
+                var token = slash < 0 ? member : member[(slash + 1)..];
+
+                var span = real.Groups[1];
+                var at = span.Value.IndexOf($"`{token}`", StringComparison.Ordinal);
+                Assert.True(at >= 0,
+                    $"{name}: `{token}` is not inside the pinned span, so dropping it would prove nothing "
+                    + "about this pin. Either the sentence spells it differently or the span is wrong.");
+
+                var mutated = readme
+                    .Remove(span.Index + at, token.Length + 2)
+                    .Insert(span.Index + at, "`zzz_drift_probe`");
+
+                var after = pattern.Match(mutated);
+                Assert.True(after.Success, $"{name}: the mutation broke the pattern, so this proves nothing.");
+
+                Assert.False(extract(after.Groups[1].Value).Contains(member),
+                    $"{name}: dropping `{token}` from the README still extracted {member}, so this pin does "
+                    + "not actually depend on that name being there.");
+            }
+        }
+    }
+
+    /// <summary>
+    /// One set pin, compared both ways. Missing is the defect that shipped — a dependency the product has
+    /// and the documentation does not mention. Extra is its mirror image and just as wrong to publish.
+    /// </summary>
+    private static void AssertSetPin(
+        string readme,
+        string name,
+        Regex pattern,
+        IReadOnlySet<string> expected,
+        Func<string, IReadOnlySet<string>> extract)
+    {
+        var match = pattern.Match(readme);
+
+        Assert.True(match.Success,
+            $"Darling/README.md no longer contains {name} in the pinned shape ({pattern}). It restates a set "
+            + "the collector catalog declares, so keep it parseable — a reworded sentence has to fail here "
+            + "rather than quietly stop being checked, which is the whole reason this file asserts the match "
+            + "before it compares anything.");
+
+        var actual = extract(match.Groups[1].Value);
+
+        var missing = expected.Except(actual, StringComparer.Ordinal)
+            .OrderBy(v => v, StringComparer.Ordinal).ToArray();
+        var extra = actual.Except(expected, StringComparer.Ordinal)
+            .OrderBy(v => v, StringComparer.Ordinal).ToArray();
+
+        Assert.True(missing.Length == 0,
+            $"{name}: the collector catalog declares {string.Join(", ", missing)}, which Darling/README.md "
+            + "does not name. Add it to that sentence — a declared dependency the permissions section omits "
+            + "is one nobody installing this product is told to install, and it fails as a non-fatal skip "
+            + "that stores nothing rather than as an error.");
+
+        Assert.True(extra.Length == 0,
+            $"{name}: Darling/README.md names {string.Join(", ", extra)}, which no collector declares as a "
+            + "dependency. Either remove it from the sentence, or declare it on the collector that needs it "
+            + "(RequiredPgExtensions) — do not widen this sentence to match a list built for another "
+            + "question, such as pg_extension_availability's reportable-absence roster.");
     }
 
     private static void AssertPin(string readme, string name, Regex pattern, int[] expected)

Darling/README.md

@@ -676,7 +676,7 @@ GRANT pg_read_all_data TO darling_monitor;   -- PostgreSQL 14+, see below
 
 **`pgstattuple` is the extension `pg_index_bloat` MEASURES with, through its `pgstatindex` function**, and it is a per-database extension rather than a grant: `CREATE EXTENSION pgstattuple;` in each database you want index bloat for. Without it the collector logs a `PERMISSIONS` skip naming the missing function (SQLSTATE 42883) and stores nothing for that database. The extension grants `EXECUTE` to `pg_stat_scan_tables`, which `pg_monitor` already carries, so no further grant is needed once it exists. It reads every page of each index it measures, so it is bounded by a per-cycle byte budget and a per-index ceiling, and an index past that ceiling is reported as unmeasured rather than estimated: #2561 considered porting the ioguix statistics-based estimator and rejected it, because under exactly these permissions the exact function works and the estimator is blind.
 
-`pg_stat_statements` must be present for `pg_statement_stats`, which means the extension in `shared_preload_libraries` (a restart, or a parameter-group change plus reboot on Aurora/RDS) and `CREATE EXTENSION pg_stat_statements;` in the database Darling connects to. The extension tracks **all** databases in the cluster keyed by `dbid`, so one installation in the connect database covers the whole instance. Six collectors need something installed, named rather than counted: `pg_statement_stats` (`pg_stat_statements`, above), `pg_index_bloat` (`pgstattuple`), `pg_buffer_usage` (`pg_buffercache`), `pg_kernel_stats` (`pg_stat_kcache`, which sits on top of `pg_stat_statements`), `pg_predicate_stats` (`pg_qualstats`), and `pg_wait_sampling` (the `pg_wait_sampling` module). `pgstattuple` and `pg_qualstats` are **per database** — `CREATE EXTENSION` in one database does not create them in another — and `pg_wait_sampling` is preload-only, so like `pg_stat_statements` it needs `shared_preload_libraries` and a restart rather than a `CREATE EXTENSION`. Every other collector needs nothing installed: they read core catalogs and Aurora's built-in functions. All six degrade identically when their dependency is absent — the query raises `42P01` or `42883`, which classifies as `ObjectMissing` and is recorded as a non-fatal `PERMISSIONS` skip — and `pg_extension_availability` reports which install would light up the five that are true extensions. It deliberately does NOT cover `pg_wait_sampling`: preload-only modules never appear in `pg_available_extensions` even on a server actively running them, so listing it there would manufacture a permanent false `absent` (#2564, fixed in #2584). Check that one with `SHOW shared_preload_libraries;` instead.
+`pg_stat_statements` must be present for `pg_statement_stats`, which means the extension in `shared_preload_libraries` (a restart, or a parameter-group change plus reboot on Aurora/RDS) and `CREATE EXTENSION pg_stat_statements;` in the database Darling connects to. The extension tracks **all** databases in the cluster keyed by `dbid`, so one installation in the connect database covers the whole instance. The collectors that need something installed, named rather than counted: `pg_statement_stats` (`pg_stat_statements`, above), `pg_index_bloat` (`pgstattuple`), `pg_buffer_usage` (`pg_buffercache`), `pg_kernel_stats` (`pg_stat_kcache`, which sits on top of `pg_stat_statements`), `pg_predicate_stats` (`pg_qualstats`), and `pg_wait_sampling` (the `pg_wait_sampling` module). Every other collector needs nothing installed: they read core catalogs and Aurora's built-in functions. **Needing `shared_preload_libraries` and a server restart** (a parameter-group change plus a reboot on Aurora/RDS), and inert until then whatever else is installed: `pg_stat_statements`, `pg_stat_kcache`, `pg_qualstats` and `pg_wait_sampling`. **Additionally per database**, because the collectors that read them run per database and `CREATE EXTENSION` in one database does not create them in another: `pgstattuple` and `pg_qualstats`. Every one of them degrades identically when its dependency is absent — the query raises `42P01` or `42883`, which classifies as `ObjectMissing` and is recorded as a non-fatal `PERMISSIONS` skip — and `pg_extension_availability` reports which install would light up every one of them except `pg_wait_sampling`. It deliberately does NOT cover `pg_wait_sampling`, which was on that roster and was taken off (#2564, fixed in #2584). The roster is a **reportable-absence** list rather than a list of what collectors depend on, and those are different sets: it carries extensions no collector reads and omits this one, which a collector does read — which is why a dependency is declared on the collector rather than read off the roster. Check `pg_wait_sampling` with `SHOW shared_preload_libraries;` instead.
 
 ### What gets collected
 

PerformanceMonitor.Collectors/CollectorDefinitionBase.cs

@@ -44,6 +44,14 @@ public abstract class CollectorDefinitionBase<TRow> : ICollectorDefinition<TRow>
 
     public virtual IReadOnlyList<string> StateKeys => System.Array.Empty<string>();
 
+    /* Declared here as well as on the interface, unlike a member that only ever reads through
+       ICollectorSchemaInfo: the interface's default implementation is not reachable through the class type,
+       so without this a derived definition could not `override` it and the interface map would keep
+       resolving to the empty default no matter what the definition said. TargetEngine carries the same
+       pair for the same reason. */
+    public virtual IReadOnlyList<PgExtensionDependency> RequiredPgExtensions =>
+        System.Array.Empty<PgExtensionDependency>();
+
     public virtual int? PerItemRowCountWarnThreshold => null;
 
     public virtual int? PerItemTextByteBudget => null;