diff --git a/CHANGELOG.md b/CHANGELOG.md index 18d4efad4e..7769a86802 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ 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 `win-x64` 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 +- **Eight PostgreSQL reads told a SQL Server target its vacuum was healthy, and stock PostgreSQL was told to go and check a collector that will never run** ([#2532]) - [#2530] taught the miss vocabulary the engine-KIND axis but wired only the *engine half* of the dispatch gate into it, so the PostgreSQL collectors' own `AppliesTo` gates were still invisible to it, and none of the eight `get_pg_*` reads asked the question at all. A SQL Server target asked `get_pg_xmin_horizon` got `no_holder` - "nothing is holding back the xmin horizon", a confident all-clear about a mechanism that engine does not have - and `get_pg_autovacuum_health` reported `no_pending_maintenance`, which is a fabricated clean bill of health rather than a weak one. `CollectorEngineCapability.TargetsWithEngineKind` now sweeps every target shape an engine kind permits, fixing `IsAurora` (nothing turns a stock PostgreSQL server into an Aurora one) and varying `PostgresMajorVersion`, `PostgresVersionNum` and `IsInRecovery`, all three of which an upgrade or a writer connection moves. So `pg_wait_stats` and `pg_statement_stats`, which read `aurora_stat_system_waits()` and `aurora_stat_statements()`, are permanent gaps on stock PostgreSQL and say so - while `pg_stat_io`'s PG16 floor and the writer-only autovacuum read keep the `unavailable` vocabulary that correctly sends an operator to look. The message names the real reason: a foreign DIALECT is stopped by the dispatch gate's engine half, a same-dialect gap is the collector's own gate, and one sentence for both would have told a PostgreSQL operator that a PostgreSQL collector is not written for PostgreSQL. Shipped with the guard [#2532] made its prerequisite: `EveryFactAPostgresGateReads_IsVariedBySweepOrFixedByKind` decodes the IL of every PostgreSQL `AppliesTo` and fails the build if the sweep leaves a fact it reads at its CLR default - the twin of [#2518]'s edition-axis guard, and the reason a sweep can be trusted not to over-claim silently. - **The Overview lanes skewed apart when one lane's numbers got long** ([#2533]) - the five lanes are five SEPARATE plots, and `SyncXAxes` only ever gave them the same X-axis LIMITS. Nothing gave them the same pixel geometry: each plot sizes its own left gutter from its own Y tick labels, so on a server whose wait time runs into six figures the Wait ms/sec lane began its data area ~27 px right of the I/O latency lane reading `0.4`, and the lanes stopped lining up on the time axis. Every value stayed correct throughout - which is why the tooltips still read true, and what identified this as layout rather than data. **Not a regression**, and the reporter's "I think this worked before" is right for his machine: `MinimumSize`, `Layout.Fixed` and `PixelPadding` have never appeared in any SKU on any branch, so nothing was removed - alignment was always incidental, holding only while the five lanes happened to need labels of similar width, and five- or six-digit wait ms/sec is what breaks the coincidence. The same build therefore looks right on one server and skewed on another. The lanes now share ONE gutter, sized to the widest lane's own natural gutter and re-derived on every refresh - a frozen constant would have to cover six-digit wait times and would then steal that width from every server whose lanes read in single digits, and it ratchets back down when the big values go away. The mechanism is `Axes.Left.MinimumSize` rather than a fixed layout, because a fixed layout pins all four sides and the bottom lane legitimately needs more bottom room than the other four - it alone draws the time labels. This is the second alignment defect in these lanes and the first one shipped with nothing that could have caught it, so the fix comes with headless render tests: ScottPlot draws without a window, so CI now builds lanes with deliberately mismatched magnitudes, renders them, and compares their data rectangles - shown failing at 20.02 px on the unfixed control before it passed. **Darling viewer only.** Lite's Overview control is the same five separate plots and carries the same latent defect, but nobody has reported hitting it there; the aligner lives in the shared `PerformanceMonitor.Ui`, so picking Lite up later is one call site. Reported by @ghauan. - **A SQL Server read aimed at a PostgreSQL target says so, instead of telling you to check collection** ([#2530]) - [#2511] taught the miss vocabulary the engine-EDITION axis, so a read served by a collector gated off on Azure SQL Database answers `not_collected` naming the engine. A PostgreSQL target got `unavailable` - "never collected, check that collection is running" - which is true and points at the wrong cause: those collectors do not apply to that engine at all. That was the honest answer while nothing in the store distinguished a PostgreSQL target from an unconnected one; now something does. The answer is derived the same way the edition axis is, from the collectors' own dispatch gate rather than from a list, so flipping a definition's target engine moves it. **Unknown targets keep making no claim** - a pre-v82 row, a server that has not connected since the rung landed, and a token a newer build wrote all stay silent, because the distinction added is "known to be PostgreSQL" and never "not known to be SQL Server". It runs both ways: a `get_pg_*` read aimed at a known SQL Server target now says the same thing in reverse. Limit worth knowing: only the reads [#2511] wired to the capability helper can answer on either axis, so a read like `get_database_config` - whose collector runs on every SQL Server, so the edition axis gave it nothing to say - still reports `unavailable` on a PostgreSQL target. - **A fresh store never seeded, so the control plane silently overrode `darling.json`** ([#2524]) - the seed INSERT named sixteen columns and supplied fifteen: a bound parameter was never written into the VALUES list, so every value after it shifted one column left and PostgreSQL refused the whole statement with `42601`. The failure was caught and logged and the service carried on with the file config, which is why it shipped - the damage came next, because the control plane is authoritative after first contact and was answering from a row that had never been written, where every toggle is false. So `web.enabled` and `mcp.enabled` were **true** in `darling.json`, **false** in the store, the store won as designed, and neither endpoint ever listened. That was the entire first-run experience of a container deployment, and the only clue was one line saying the config would seed on a later start - it would not, because the statement was malformed rather than racing. Found by running the shipped nightly image against a live target rather than by reading it, since no test drove the seed against an empty store. @@ -2877,6 +2878,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2520]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2520 [#2525]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2525 [#2530]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2530 +[#2532]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2532 +[#2518]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2518 [#2533]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2533 [#2480]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2480 [#2489]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2489 diff --git a/Darling/Darling.Tests/CollectorEngineCapabilityDerivationTests.cs b/Darling/Darling.Tests/CollectorEngineCapabilityDerivationTests.cs index d7c9e79e74..d2a259f7ea 100644 --- a/Darling/Darling.Tests/CollectorEngineCapabilityDerivationTests.cs +++ b/Darling/Darling.Tests/CollectorEngineCapabilityDerivationTests.cs @@ -322,25 +322,88 @@ public void TheKindAnswerMoves_WhenTheDefinitionsEngineMoves_InBothDirections() } /// - /// The kind axis reads the ENGINE half of the dispatch gate and nothing else — a definition's own - /// AppliesTo cannot make it claim a gap, however tightly shut. + /// A gate SHUT everywhere is a kind gap on both PostgreSQL tokens; a gate on a fact the sweep varies is + /// a gap on neither. This is the assertion #2532 replaced: until then the kind axis asked only the + /// ENGINE half of the dispatch gate, and a definition's own AppliesTo could not make it claim + /// anything at all. /// - /// That is a deliberate narrowing, not an oversight, and it is the same discipline the edition - /// axis applies to msdb access: AppliesTo on the PostgreSQL side reads Aurora-ness, version - /// floors and recovery state, and none of those are decided by the engine kind alone. Letting a shut - /// gate speak here would report "never will" for a collector an upgrade or a writer connection - /// enables — the confident-and-wrong message this whole mechanism exists to delete. + /// Why that narrowing was right then and wrong now. The discipline it protected — never + /// report a FIXABLE gate as permanent — is unchanged and is the whole subject of this file. What it + /// lacked was a sweep: with no way to separate "excluded on every target of this kind" from "excluded on + /// the one target shape somebody happened to construct", the only safe answer was to decline. Now + /// varies the PostgreSQL version floors and + /// the recovery state and fixes only , so the fixable gates + /// answer TRUE on their own merits rather than by the axis refusing to look — which is what the second + /// half of this test is for, and what + /// EveryFactAPostgresGateReads_IsVariedBySweepOrFixedByKind keeps true as gates are added. /// [Fact] - public void AShutAppliesToGate_IsNotAnEngineKindGap() + public void AShutAppliesToGate_IsAnEngineKindGap_ButAFixableOneIsNot() { var postgres = new SyntheticCollector { Gate = _ => false, TargetEngine = CollectorTargetEngine.PostgreSql }; - Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.Postgres)); - Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.AuroraPostgres)); + Assert.False(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.Postgres)); + Assert.False(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.AuroraPostgres)); - /* ...and the engine half still speaks, so the assertion above is not vacuous. */ + /* ...and the engine half still speaks, so a PostgreSQL definition is a gap on SQL Server for the + dialect reason rather than for this one. */ Assert.False(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.SqlServer)); + + /* The half that keeps the narrowing's point: a gate reading a fact an operator can move is not a + permanent gap on EITHER token. An upgrade crosses the floor; a writer connection leaves recovery. + A sweep that stopped varying either of these would report both of them as "never will". */ + foreach (var fixableGate in new Func[] + { + target => target.PostgresMajorVersion >= 16, + target => target.PostgresVersionNum >= 170005, + target => !target.IsInRecovery, + target => target.IsInRecovery, + }) + { + postgres.Gate = fixableGate; + Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.Postgres)); + Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.AuroraPostgres)); + } + + /* And the one fact the KIND fixes: Aurora-ness. Nothing an operator does turns a stock PostgreSQL + server into an Aurora one, so this is the only PostgreSQL gate shape that produces a permanent + gap — and it produces it on exactly one of the two tokens, in each direction. */ + postgres.Gate = target => target.IsAurora; + Assert.False(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.Postgres)); + Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.AuroraPostgres)); + + postgres.Gate = target => !target.IsAurora; + Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.Postgres)); + Assert.False(CollectorEngineCapability.IsCollectedOnEngineKind(postgres, MonitoredEngineKind.AuroraPostgres)); + } + + /// + /// The SQL Server side of the same question, and the reason the two axes do not answer each other's: + /// a collector gated off on Azure SQL Database is NOT a kind gap on sqlserver, because it runs on + /// every other SQL Server there is. The edition axis is what says so, with the edition named. + /// + /// This is what varying the two Azure + /// flags buys, and it is the property that would break silently if the SQL Server arm of that sweep ever + /// fixed them the way does: every + /// Azure-gated collector would become a permanent gap on every SQL Server, and the message would stop + /// naming the edition that is actually responsible. + /// + [Fact] + public void AnAzureOnlyGate_IsNotAKindGapOnSqlServer_ButIsStillAnEditionGap() + { + var collector = new SyntheticCollector + { + Gate = target => !target.IsAzureSqlDb, + TargetEngine = CollectorTargetEngine.SqlServer, + }; + + Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(collector, MonitoredEngineKind.SqlServer)); + Assert.False(CollectorEngineCapability.IsCollectedOnEngineEdition(collector, AzureSqlDb)); + Assert.True(CollectorEngineCapability.IsCollectedOnEngineEdition(collector, Enterprise)); + + /* A gate shut on every SQL Server shape IS a kind gap, so the assertion above is not vacuous. */ + collector.Gate = _ => false; + Assert.False(CollectorEngineCapability.IsCollectedOnEngineKind(collector, MonitoredEngineKind.SqlServer)); } /// @@ -408,11 +471,99 @@ public void TheShippedCatalogSplitsCleanlyOnTheKindAxis() foreach (var pgToken in new[] { MonitoredEngineKind.Postgres, MonitoredEngineKind.AuroraPostgres }) { Assert.All(sqlServer, c => Assert.False(CollectorEngineCapability.IsCollectedOnEngineKind(c, pgToken))); - Assert.All(postgres, c => Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(c, pgToken))); } Assert.All(sqlServer, c => Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(c, MonitoredEngineKind.SqlServer))); Assert.All(postgres, c => Assert.False(CollectorEngineCapability.IsCollectedOnEngineKind(c, MonitoredEngineKind.SqlServer))); + + /* Aurora is a strict superset of the surfaces the PostgreSQL collectors read, so every one of them + applies there. This is the half that would break if a new collector were written against something + Aurora removes rather than adds. */ + Assert.All(postgres, c => Assert.True( + CollectorEngineCapability.IsCollectedOnEngineKind(c, MonitoredEngineKind.AuroraPostgres), + $"{c.Name} is reported as a permanent gap on Aurora PostgreSQL")); + + /* Stock PostgreSQL is where the Aurora-only surfaces become a real gap (#2532), so the two tokens + genuinely differ — counted from the catalog rather than listed, and asserted as a PROPER subset so + neither "everything is a gap" nor "nothing is" passes. */ + var stockGaps = postgres + .Where(c => !CollectorEngineCapability.IsCollectedOnEngineKind(c, MonitoredEngineKind.Postgres)) + .Select(c => c.Name) + .ToArray(); + + Assert.NotEmpty(stockGaps); + Assert.True(stockGaps.Length < postgres.Length, + "every PostgreSQL collector is reported as a permanent gap on stock PostgreSQL, which would mean " + + "the sweep stopped producing shapes rather than that the gates changed: " + string.Join(", ", stockGaps)); + } + + /// + /// The measurement #2532 was filed on, named: aurora_stat_system_waits() is an Aurora built-in + /// that core PostgreSQL has no equivalent of in any version, so pg_wait_stats can never run on a + /// stock PostgreSQL target — and runs perfectly well on the Aurora one, which is the whole fleet today. + /// + /// The three collectors beside it are the control. pg_io_stats carries a PG16 floor and + /// pg_autovacuum_stats a writer-only gate; both are FIXABLE, both keep the unavailable + /// vocabulary that sends an operator to look, and a sweep that stopped varying either fact would report + /// them here as "never will". pg_blocking has no gate at all. + /// + [Fact] + public void AuroraOnlySurfaces_ArePermanentGapsOnStockPostgres_AndTheFixableGatesAreNot() + { + foreach (var auroraOnly in new[] { "pg_wait_stats", "pg_statement_stats" }) + { + Assert.False(CollectorEngineCapability.IsCollectedOnEngineKind(auroraOnly, MonitoredEngineKind.Postgres), auroraOnly); + Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(auroraOnly, MonitoredEngineKind.AuroraPostgres), auroraOnly); + } + + foreach (var fixable in new[] { "pg_io_stats", "pg_autovacuum_stats", "pg_blocking" }) + { + Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(fixable, MonitoredEngineKind.Postgres), fixable); + Assert.True(CollectorEngineCapability.IsCollectedOnEngineKind(fixable, MonitoredEngineKind.AuroraPostgres), fixable); + } + } + + /// + /// The two shapes of kind gap say different things, because they ARE different things: a foreign dialect + /// is stopped by the dispatch gate's engine half before the collector's own gate is consulted, while + /// pg_wait_stats on stock PostgreSQL is that collector's own AppliesTo. One sentence for + /// both would tell a PostgreSQL operator that a PostgreSQL collector is not written for PostgreSQL, + /// which is the sort of wrong that costs a reader their trust in the rest of the message. + /// + [Fact] + public void TheKindMessage_NamesTheRealReason_DialectOrTheCollectorsOwnGate() + { + var dialect = CollectorEngineCapability.NotCollectedMessage( + "aurora-01", CollectorEngineCapability.UnknownEngineEdition, MonitoredEngineKind.AuroraPostgres, "wait_stats"); + + Assert.NotNull(dialect); + Assert.Contains("aurora-01 runs Aurora PostgreSQL.", dialect, StringComparison.Ordinal); + Assert.Contains("is written against SQL Server", dialect, StringComparison.Ordinal); + Assert.Contains("the dispatch gate's engine half never sends it at another engine", dialect, StringComparison.Ordinal); + + var ownGate = CollectorEngineCapability.NotCollectedMessage( + "pg-01", CollectorEngineCapability.UnknownEngineEdition, MonitoredEngineKind.Postgres, "pg_wait_stats"); + + Assert.NotNull(ownGate); + Assert.Contains("pg-01 runs PostgreSQL.", ownGate, StringComparison.Ordinal); + Assert.Contains("its own AppliesTo gate excludes it", ownGate, StringComparison.Ordinal); + Assert.Contains("the aurora_stat_system_waits() cumulative wait counters", ownGate, StringComparison.Ordinal); + Assert.Contains("and never will.", ownGate, StringComparison.Ordinal); + + /* The dialect sentence must NOT appear on the own-gate message: it is the specific falsehood this + split exists to prevent. */ + Assert.DoesNotContain("is written against PostgreSQL", ownGate, StringComparison.Ordinal); + Assert.DoesNotContain("dispatch gate's engine half", ownGate, StringComparison.Ordinal); + Assert.DoesNotContain("check that collection is running", ownGate, StringComparison.OrdinalIgnoreCase); + + /* No EngineEdition claim about a PostgreSQL server, on either shape. */ + Assert.DoesNotContain("EngineEdition", dialect, StringComparison.Ordinal); + Assert.DoesNotContain("EngineEdition", ownGate, StringComparison.Ordinal); + + /* And the same read on the engine that DOES have the surface says nothing at all, so the read keeps + its own miss vocabulary. */ + Assert.Null(CollectorEngineCapability.NotCollectedMessage( + "aurora-01", CollectorEngineCapability.UnknownEngineEdition, MonitoredEngineKind.AuroraPostgres, "pg_wait_stats")); } /// @@ -507,58 +658,93 @@ public sealed class CollectorEngineCapabilitySweepDimensionTests TargetFacts.ToDictionary(property => property.Name, StringComparer.Ordinal); /// - /// How the sweep treats a fact. Derived by RUNNING the sweep and looking at what it produces, never + /// The two PostgreSQL tokens the store can record, which are the KIND axis's domain the way + /// is the edition axis's (#2532). Taken from the vocabulary rather than + /// typed out, so a third PostgreSQL flavour is in scope the moment it is added — and filtered by dialect + /// rather than by name, so it stays right if the tokens are ever renamed. + /// + private static readonly string[] PostgresKinds = MonitoredEngineKind.All + .Where(kind => MonitoredEngineKind.EngineOf(kind) == CollectorTargetEngine.PostgreSql) + .ToArray(); + + /// + /// How a sweep treats a fact. Derived by RUNNING the sweep and looking at what it produces, never /// declared — a declared role would be the hand-maintained list this test exists to replace. /// private enum SweepRole { - /// The sweep produces more than one value for it within a single edition. + /// The sweep produces more than one value for it within a single axis value. VariedBySweep, - /// Constant within an edition, but different between editions — the edition decides it. - FixedByEdition, + /// Constant within one axis value, but different between them — the axis decides it. + FixedByAxis, - /// The engine discriminator itself. Fixed at SQL Server because that is the question. + /// The engine discriminator itself. Fixed by the question rather than swept. EngineDiscriminator, - /// The same value in every shape of every edition — a gate reading it can only ever see one - /// answer, so a gate reading it is where over-claiming begins. + /// The same value in every shape of every axis value — a gate reading it can only ever see + /// one answer, so a gate reading it is where over-claiming begins. ConstantEverywhere, } - private static SweepRole RoleOf(PropertyInfo fact) + /// + /// The role a fact plays across one axis, given the shapes that axis produces for each of its values. + /// Parameterised over the shape families rather than hard-wired to an axis, because the two axes ask the + /// identical question of different sweeps and a second copy of this arithmetic is exactly the kind of + /// near-duplicate that drifts (#2532). + /// + private static SweepRole RoleOf(PropertyInfo fact, IReadOnlyList shapesPerAxisValue) { /* The engine is the precondition of the whole question, not a dimension of it: "does any target of - this SQL Server engine edition run this collector" fixes the engine by construction. Recognised by - TYPE rather than by name, so it stays recognised if the property is ever renamed. */ + this engine edition / engine kind run this collector" fixes the engine by construction. Recognised + by TYPE rather than by name, so it stays recognised if the property is ever renamed. */ if (fact.PropertyType == typeof(CollectorTargetEngine)) { return SweepRole.EngineDiscriminator; } - var perEdition = AllEngineEditions - .Select(edition => CollectorEngineCapability.TargetsWithEngineEdition(edition) - .Select(fact.GetValue) - .Distinct() - .ToArray()) + var perValue = shapesPerAxisValue + .Select(shapes => shapes.Select(fact.GetValue).Distinct().ToArray()) .ToArray(); - if (perEdition.Any(values => values.Length > 1)) + if (perValue.Any(values => values.Length > 1)) { return SweepRole.VariedBySweep; } - return perEdition.Select(values => values[0]).Distinct().Count() > 1 - ? SweepRole.FixedByEdition + return perValue.Select(values => values[0]).Distinct().Count() > 1 + ? SweepRole.FixedByAxis : SweepRole.ConstantEverywhere; } + /// The shapes the EDITION sweep produces, one array per engine edition. + private static CollectorTargetInfo[][] EditionShapes() => AllEngineEditions + .Select(edition => CollectorEngineCapability.TargetsWithEngineEdition(edition).ToArray()) + .ToArray(); + + /// The shapes the KIND sweep produces, one array per PostgreSQL token. + private static CollectorTargetInfo[][] PostgresKindShapes() => PostgresKinds + .Select(kind => CollectorEngineCapability.TargetsWithEngineKind(kind).ToArray()) + .ToArray(); + + private static SweepRole EditionRoleOf(PropertyInfo fact) => RoleOf(fact, EditionShapes()); + + private static SweepRole PostgresKindRoleOf(PropertyInfo fact) => RoleOf(fact, PostgresKindShapes()); + /// fact name → the SQL Server collectors whose AppliesTo reads it, decoded from IL. - private static SortedDictionary> FactsReadBySqlServerGates() + private static SortedDictionary> FactsReadBySqlServerGates() => + FactsReadByGatesOf(CollectorTargetEngine.SqlServer); + + /// The same, for the PostgreSQL definitions — the whole difference between the two guards, as + /// #2532 said it would be. + private static SortedDictionary> FactsReadByPostgresGates() => + FactsReadByGatesOf(CollectorTargetEngine.PostgreSql); + + private static SortedDictionary> FactsReadByGatesOf(CollectorTargetEngine engine) { var byFact = new SortedDictionary>(StringComparer.Ordinal); - foreach (var definition in CollectorCatalog.All.Where(c => c.TargetEngine == CollectorTargetEngine.SqlServer)) + foreach (var definition in CollectorCatalog.All.Where(c => c.TargetEngine == engine)) { foreach (var fact in FactsReadByGateOf(definition)) { @@ -720,7 +906,7 @@ public void EveryFactASqlServerGateReads_IsVariedBySweepOrFixedByEdition() var read = FactsReadBySqlServerGates(); var overClaiming = read - .Where(entry => RoleOf(FactByName[entry.Key]) == SweepRole.ConstantEverywhere) + .Where(entry => EditionRoleOf(FactByName[entry.Key]) == SweepRole.ConstantEverywhere) .Select(entry => $" {entry.Key} — read by {string.Join(", ", entry.Value)}") .ToArray(); @@ -809,7 +995,7 @@ public void TheSweep_IsTheFullCrossProductOfTheDimensionsItVaries() foreach (var edition in AllEngineEditions) { var shapes = CollectorEngineCapability.TargetsWithEngineEdition(edition).ToArray(); - var varied = TargetFacts.Where(fact => RoleOf(fact) == SweepRole.VariedBySweep).ToArray(); + var varied = TargetFacts.Where(fact => EditionRoleOf(fact) == SweepRole.VariedBySweep).ToArray(); Assert.NotEmpty(varied); @@ -844,12 +1030,12 @@ satisfied by a sweep that emitted one shape twice and skipped another. */ [Fact] public void EveryTargetFact_LandsInExactlyOneDerivedRole() { - var roles = TargetFacts.ToDictionary(fact => fact.Name, RoleOf, StringComparer.Ordinal); + var roles = TargetFacts.ToDictionary(fact => fact.Name, EditionRoleOf, StringComparer.Ordinal); Assert.NotEmpty(roles); Assert.Single(roles, role => role.Value == SweepRole.EngineDiscriminator); Assert.Contains(roles, role => role.Value == SweepRole.VariedBySweep); - Assert.Contains(roles, role => role.Value == SweepRole.FixedByEdition); + Assert.Contains(roles, role => role.Value == SweepRole.FixedByAxis); /* The two Azure flags are the edition-fixed pair, and they follow the edition in OPPOSITE directions — asserted here because "fixed by edition" is otherwise satisfied by a flag that is fixed at the @@ -862,4 +1048,185 @@ wrong value. */ Assert.All(azureShapes, shape => Assert.True(shape.IsAzureSqlDb && !shape.IsAzureManagedInstance)); Assert.All(miShapes, shape => Assert.True(shape.IsAzureManagedInstance && !shape.IsAzureSqlDb)); } + + /* ───────── the same guard, one engine over (#2532) ───────── */ + + /// + /// THE assertion, PostgreSQL side. Every fact a PostgreSQL gate reads + /// is a fact either varies or fixes by the + /// kind — so no PostgreSQL gate can be answered by a single defaulted value. + /// + /// Why this had to land before the axis did. #2530 deliberately shipped only the ENGINE half + /// of the dispatch gate on this axis, because a sweep without this guard over-claims silently: a fact the + /// sweep leaves at its CLR default fails every shape, and the derivation announces a permanent, + /// unfixable engine gap for a collector that runs perfectly well — on the very engine whose support this + /// work exists to make credible. #2532 made that the explicit prerequisite, and this is it. + /// + /// Nothing here is a list. The facts come out of the gates' IL (the same decoder the edition + /// half uses, filtered to the PostgreSQL definitions) and the roles come out of running the sweep. Fix + /// the SWEEP, never this test — there is no name to add here instead. + /// + [Fact] + public void EveryFactAPostgresGateReads_IsVariedBySweepOrFixedByKind() + { + var read = FactsReadByPostgresGates(); + + var overClaiming = read + .Where(entry => PostgresKindRoleOf(FactByName[entry.Key]) == SweepRole.ConstantEverywhere) + .Select(entry => $" {entry.Key} — read by {string.Join(", ", entry.Value)}") + .ToArray(); + + Assert.True( + overClaiming.Length == 0, + "A PostgreSQL collector's AppliesTo gate reads a CollectorTargetInfo fact that " + + "CollectorEngineCapability.TargetsWithEngineKind never varies. Every swept target therefore " + + "carries that fact's default, every one of them fails the gate, and the capability derivation " + + "reports a PERMANENT engine gap on both PostgreSQL tokens for a collector that runs perfectly " + + "well — the confident-and-wrong message #2511 exists to delete, one engine over.\n\n" + + "Fix the SWEEP, not this test: add the fact to TargetsWithEngineKind (varying it, or deriving it " + + "from the engine kind the way IsAurora is). There is no list here to add a name to.\n\n" + + string.Join("\n", overClaiming)); + } + + /// + /// The PostgreSQL half of the scan reads the gates the source actually carries — the same non-vacuity + /// floor the SQL Server half has, and for the same reason: a source-decoding guard that matches nothing + /// passes for free and converts an open question into false confidence. + /// + [Fact] + public void ThePostgresGateScan_ReadsTheGatesTheSourceActuallyCarries() + { + var definitions = CollectorCatalog.All + .Where(c => c.TargetEngine == CollectorTargetEngine.PostgreSql) + .ToArray(); + + Assert.True( + definitions.Length >= 8, + $"only {definitions.Length} PostgreSQL definitions found — the catalog walk is broken, not the catalog"); + + var read = FactsReadByPostgresGates(); + + /* The three facts today's PostgreSQL gates are written on. A floor, not an expected-set pin. */ + Assert.Contains(nameof(CollectorTargetInfo.IsAurora), read.Keys); + Assert.Contains(nameof(CollectorTargetInfo.IsInRecovery), read.Keys); + Assert.Contains(nameof(CollectorTargetInfo.PostgresMajorVersion), read.Keys); + + var byName = CollectorCatalog.All.ToDictionary(c => c.Name, StringComparer.Ordinal); + + /* The collector #2532 was filed on: one fact, exactly the one its source names. */ + Assert.Equal( + new[] { nameof(CollectorTargetInfo.IsAurora) }, + FactsReadByGateOf(byName["pg_wait_stats"]).ToArray()); + + /* A version floor and a recovery gate, so the decoder is discriminating rather than reporting + IsAurora for everything. */ + Assert.Equal( + new[] { nameof(CollectorTargetInfo.PostgresMajorVersion) }, + FactsReadByGateOf(byName["pg_io_stats"]).ToArray()); + + Assert.Equal( + new[] { nameof(CollectorTargetInfo.IsInRecovery) }, + FactsReadByGateOf(byName["pg_autovacuum_stats"]).ToArray()); + + /* And a gate that reads nothing at all — without this, "reports every fact for every collector" + would satisfy every assertion above. */ + Assert.Empty(FactsReadByGateOf(byName["pg_blocking"])); + + /* No SQL Server fact leaks into the PostgreSQL scan. The two halves differ only by the filter, so a + filter that stopped filtering would look exactly like a guard that was working. */ + Assert.DoesNotContain(nameof(CollectorTargetInfo.IsAzureSqlDb), read.Keys); + Assert.DoesNotContain(nameof(CollectorTargetInfo.HasMsdbAccess), read.Keys); + Assert.DoesNotContain(nameof(CollectorTargetInfo.SqlMajorVersion), read.Keys); + } + + /// + /// The kind sweep is the FULL cross product of the dimensions it varies, for the reason the edition + /// sweep is: the claim being made is "no target of this kind, under any COMBINATION of the other facts", + /// and a sweep that moved one axis at a time would report a conjunctive gate as a permanent gap because + /// the one shape that satisfies it was never generated. + /// + [Fact] + public void TheKindSweep_IsTheFullCrossProductOfTheDimensionsItVaries() + { + Assert.NotEmpty(PostgresKinds); + + foreach (var kind in PostgresKinds) + { + var shapes = CollectorEngineCapability.TargetsWithEngineKind(kind).ToArray(); + var varied = TargetFacts.Where(fact => PostgresKindRoleOf(fact) == SweepRole.VariedBySweep).ToArray(); + + Assert.NotEmpty(varied); + Assert.All(shapes, shape => Assert.Equal(CollectorTargetEngine.PostgreSql, shape.Engine)); + + var expected = varied.Aggregate( + 1, + (total, fact) => total * shapes.Select(fact.GetValue).Distinct().Count()); + + Assert.Equal(expected, shapes.Length); + + var fingerprints = shapes + .Select(shape => string.Join("|", varied.Select(fact => fact.GetValue(shape)))) + .ToArray(); + + Assert.Equal(shapes.Length, fingerprints.Distinct(StringComparer.Ordinal).Count()); + } + } + + /// + /// Every fact lands in exactly one derived role on the KIND axis too, and the roles are non-degenerate: + /// is the one the kind fixes — in OPPOSITE directions for the + /// two tokens, so "fixed by the kind" is not satisfied by a flag stuck at the wrong value — and the + /// version and recovery facts are the ones it varies. + /// + /// A kind sweep that collapsed to one shape per token would slide every fact into + /// and answer every question from a single target, with a gap + /// set that still looked plausible. This is the whole-type view that says so. + /// + [Fact] + public void EveryTargetFact_LandsInExactlyOneDerivedKindRole() + { + var roles = TargetFacts.ToDictionary(fact => fact.Name, PostgresKindRoleOf, StringComparer.Ordinal); + + Assert.NotEmpty(roles); + Assert.Single(roles, role => role.Value == SweepRole.EngineDiscriminator); + Assert.Contains(roles, role => role.Value == SweepRole.VariedBySweep); + + Assert.Equal(SweepRole.FixedByAxis, roles[nameof(CollectorTargetInfo.IsAurora)]); + Assert.Equal(SweepRole.VariedBySweep, roles[nameof(CollectorTargetInfo.IsInRecovery)]); + Assert.Equal(SweepRole.VariedBySweep, roles[nameof(CollectorTargetInfo.PostgresMajorVersion)]); + Assert.Equal(SweepRole.VariedBySweep, roles[nameof(CollectorTargetInfo.PostgresVersionNum)]); + + var stock = CollectorEngineCapability.TargetsWithEngineKind(MonitoredEngineKind.Postgres).ToArray(); + var aurora = CollectorEngineCapability.TargetsWithEngineKind(MonitoredEngineKind.AuroraPostgres).ToArray(); + + Assert.NotEmpty(stock); + Assert.NotEmpty(aurora); + Assert.All(stock, shape => Assert.False(shape.IsAurora)); + Assert.All(aurora, shape => Assert.True(shape.IsAurora)); + } + + /// + /// A kind this build does not recognise produces NO shapes, and the capability answer treats that as + /// silence rather than as "no shape runs it". + /// + /// This is the one place an empty sweep would be catastrophic instead of merely wrong: read as a + /// gap set, an empty sweep makes every collector a permanent gap on every unknown token — which is + /// exactly the row a store written by a NEWER service leaves behind, and exactly the guarantee #2530 + /// was told to keep. + /// + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("something-a-newer-build-writes")] + public void AnUnrecognisedKind_SweepsNothing_AndStillClaimsNothing(string? engineKind) + { + Assert.Empty(CollectorEngineCapability.TargetsWithEngineKind(engineKind)); + + Assert.All( + CollectorCatalog.All, + definition => Assert.True( + CollectorEngineCapability.IsCollectedOnEngineKind(definition, engineKind), + $"{definition.Name} was claimed as a permanent gap on an unrecognised engine kind")); + } } diff --git a/Darling/Darling.Tests/EngineCapabilityMissTests.cs b/Darling/Darling.Tests/EngineCapabilityMissTests.cs index e81c757680..45e5b8aa22 100644 --- a/Darling/Darling.Tests/EngineCapabilityMissTests.cs +++ b/Darling/Darling.Tests/EngineCapabilityMissTests.cs @@ -206,20 +206,54 @@ public void TheConnectorAndTheCapabilityHelper_ShareOneEditionTable() /// /// The prose table cannot outlive the gates it describes: every key must be a real collector, and every - /// key must be a collector that IS excluded somewhere — an entry for a collector nothing gates is prose - /// no message can ever reach, which is how a stale explanation survives unnoticed. + /// key must be a collector its OWN AppliesTo gate shuts out somewhere — an entry for a collector + /// nothing gates is prose no message can ever reach, which is how a stale explanation survives unnoticed. + /// + /// "Its own gate", not "any gap at all" (#2532). Since the kind axis landed, EVERY collector + /// is a permanent gap on some engine the store can record — a SQL Server one on both PostgreSQL tokens, + /// a PostgreSQL one on sqlserver — purely because of its DIALECT. So a check phrased as "is this a + /// gap somewhere" would now pass for every name anyone typed, which is the guard-that-stopped-guarding + /// shape. The dialect half is excluded deliberately, leaving exactly the original claim: the entry + /// describes a collector whose own precondition excludes it on some target it could otherwise be sent + /// at. That covers the twelve Azure SQL Database entries unchanged and the two Aurora-only PostgreSQL + /// ones on the same terms. /// [Fact] public void EveryCapturePathEntry_NamesARealCollectorThatIsActuallyGatedSomewhere() { - var catalog = CollectorCatalog.All.Select(c => c.Name).ToHashSet(StringComparer.Ordinal); - var azureGaps = GapsOn(AzureSqlDb).ToHashSet(StringComparer.Ordinal); + var catalog = CollectorCatalog.All.ToDictionary(c => c.Name, StringComparer.Ordinal); + var wired = EngineCapabilityReadWiringTests.AllWiredCollectors(); + + Assert.NotEmpty(wired); foreach (var name in CollectorEngineCapability.CapturePathByCollector.Keys) { - Assert.True(catalog.Contains(name), $"CapturePathByCollector names '{name}', which is not a CollectorCatalog collector"); - Assert.True(azureGaps.Contains(name), $"CapturePathByCollector still describes '{name}' as a gap, but its AppliesTo gate now lets it run on Azure SQL Database — drop the entry or re-check the gate"); + Assert.True(catalog.ContainsKey(name), $"CapturePathByCollector names '{name}', which is not a CollectorCatalog collector"); + Assert.True( + ExcludedByItsOwnGateSomewhere(catalog[name]) || wired.Contains(name), + $"CapturePathByCollector describes '{name}', but its AppliesTo gate now lets it run on every target " + + "of its own engine AND no shipped read asks the capability question about it — so the prose is " + + "unreachable. Drop the entry, re-check the gate, or wire the read that wanted it"); + } + } + + /// + /// True when a definition's own AppliesTo — not the dispatch gate's engine half — excludes it from + /// every target of some engine the store can record. Derived from the two axes rather than listed, and + /// deliberately asked only about engines this definition's DIALECT matches, so a foreign-dialect gap + /// (which every collector has, on every other engine) cannot answer it. + /// + private static bool ExcludedByItsOwnGateSomewhere(ICollectorSchemaInfo definition) + { + if (definition.TargetEngine == CollectorTargetEngine.SqlServer + && !CollectorEngineCapability.IsCollectedOnEngineEdition(definition, AzureSqlDb)) + { + return true; } + + return MonitoredEngineKind.All + .Where(kind => MonitoredEngineKind.EngineOf(kind) == definition.TargetEngine) + .Any(kind => !CollectorEngineCapability.IsCollectedOnEngineKind(definition, kind)); } /// @@ -284,8 +318,28 @@ public sealed class EngineCapabilityReadWiringTests private static readonly Regex CollectorConst = new( @"private const string (\w+) = ""([a-z_0-9]+)"";", RegexOptions.Compiled); + /// + /// Every collector name a shipped read asks the capability question about, across both SKUs. Exposed so + /// + /// can hold the prose table to the reads, rather than the reads being the only side that is checked. + /// + internal static SortedSet AllWiredCollectors() + { + var wired = new SortedSet(StringComparer.Ordinal); + + foreach (var directory in new[] { DarlingMcp, LiteMcp }) + { + foreach (var collectors in WiredReads(directory).Values) + { + wired.UnionWith(collectors); + } + } + + return wired; + } + /// tool name → the collector names its miss path asks the capability question about. - private static SortedDictionary> WiredReads(string mcpDirectory) + internal static SortedDictionary> WiredReads(string mcpDirectory) { var wired = new SortedDictionary>(StringComparer.Ordinal); @@ -335,7 +389,7 @@ private static HashSet ToolsIn(string mcpDirectory) [Theory] [InlineData(DarlingMcp)] [InlineData(LiteMcp)] - public void EveryWiredRead_NamesARealCollectorThatIsGatedOffOnAzureSqlDb(string mcpDirectory) + public void EveryWiredRead_NamesARealCollectorWhoseCapabilityBranchCanFire(string mcpDirectory) { var wired = WiredReads(mcpDirectory); var catalog = CollectorCatalog.All.Select(c => c.Name).ToHashSet(StringComparer.Ordinal); @@ -357,15 +411,45 @@ public void EveryWiredRead_NamesARealCollectorThatIsGatedOffOnAzureSqlDb(string $"{tool} names '{collector}', which has no CapturePathByCollector entry — its message would fall back " + "to the generic phrasing; add the noun phrase next to the others"); - Assert.False( - CollectorEngineCapability.IsCollectedOnEngineEdition(collector, CollectorEngineCapability.AzureSqlDatabaseEngineEdition), - $"{tool} asks about '{collector}', but that collector's AppliesTo gate now lets it run on Azure SQL " + - "Database, so the capability branch is dead code — either the gate changed and this read should stop " + - "asking, or the gate changed by accident"); + Assert.True( + ReachableOnSomeEngine(collector), + $"{tool} asks about '{collector}', but no engine the store can record produces a message for it, so " + + "the capability branch is dead code — either a gate opened up and this read should stop asking, or " + + "a gate changed by accident"); } } } + /// + /// True when SOME (engine kind, engine edition) pair the store can actually hold makes + /// speak about this collector — i.e. the + /// branch the read just added is reachable rather than dead. + /// + /// Weaker than the Azure-specific check it replaced, and deliberately so (#2532). That check + /// said "this collector is gated off on Azure SQL Database", which was the right claim while the EDITION + /// axis was the only one: a read wired to a collector that runs on every SQL Server had nothing to say + /// and the assertion caught it. The KIND axis changes that — every SQL Server collector is a permanent + /// gap on a PostgreSQL target and every PostgreSQL collector on a SQL Server one — so a read wired to + /// wait_stats now has something true and useful to say, and an assertion that still demanded an + /// Azure gap would refuse the wiring this issue is about. What is left is the property that actually + /// matters at a call site: the branch can fire. The sharper per-collector claims live where they belong, + /// in the derivation tests, over the gates rather than over the reads. + /// + private static bool ReachableOnSomeEngine(string collectorName) + { + var kinds = new string?[] { null }.Concat(MonitoredEngineKind.All); + var editions = new[] + { + CollectorEngineCapability.UnknownEngineEdition, + 3, /* Enterprise — a box edition, where nothing is an edition gap today */ + CollectorEngineCapability.AzureSqlDatabaseEngineEdition, + CollectorEngineCapability.AzureManagedInstanceEngineEdition, + }; + + return kinds.Any(kind => editions.Any(edition => + CollectorEngineCapability.NotCollectedMessage("probe", edition, kind, collectorName) is not null)); + } + /// /// The two SKUs wire the SAME reads to the SAME collectors. Compared only across tools BOTH surfaces /// expose, so a tool that exists on one SKU only (Darling's get_ag_health) is not reported as drift — diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs index d91be6e31a..4f01af7427 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs @@ -80,6 +80,16 @@ public static async Task GetPgAutovacuumHealth( if (rows.Count == 0) { + /* "No table on this server has dead tuples" is the healthy answer for a PostgreSQL target + and a fabricated one for a SQL Server target — the collector has never run there. Ask the + engine before making the claim (#2532). */ + var gated = await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_autovacuum_stats"); + if (gated != null) + { + return gated; + } + return JsonSerializer.Serialize(new { server = resolved.ServerName, diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgBlockingTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgBlockingTools.cs index a8d79b2029..6b3a71a4e4 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgBlockingTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgBlockingTools.cs @@ -150,6 +150,20 @@ is an outage. */ if (chains.Count == 0) { + /* THREE, once the store knows the engine (#2532). "No captures exist, check the collector" + is the right advice on a PostgreSQL target and the wrong cause on a SQL Server one, where + there is no collector to check. Asked only when there are no captures at all: a window + with captures is a window this collector ran in, so the engine cannot be in question. */ + if (captures.CapturesTotal == 0) + { + var gated = await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_blocking"); + if (gated != null) + { + return gated; + } + } + /* Two very different empty answers, and conflating them would be the whole failure mode of a sampled signal. No captures at all means the collector never ran — nothing is known about this window either way. Captures with no blocking is a real all-clear, bounded by diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs index f1798f7be2..55367b9040 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIoTools.cs @@ -69,6 +69,16 @@ public static async Task GetPgIoStats( if (rows.Count == 0) { + /* Ask the engine BEFORE offering the idle-server reading (#2532). "No combination recorded + activity" is a statement about a PostgreSQL instance; said about a SQL Server target it + is not a weak answer but a false one, and it is the one an agent asking by name gets. */ + var gated = await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_io_stats"); + if (gated != null) + { + return gated; + } + return JsonSerializer.Serialize(new { server = resolved.ServerName, diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSlotTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSlotTools.cs index 5a901d2560..683fba0886 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSlotTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSlotTools.cs @@ -69,6 +69,16 @@ public static async Task GetPgReplicationSlots( a cluster-wide all-clear would be drawing the one conclusion this result cannot support. */ if (rows.Count == 0) { + /* The engine question comes first (#2532): "this instance has no replication slots" is a + real finding on a PostgreSQL target and a false one on a SQL Server target, where the + collector has never run and never will. */ + var gated = await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_replication_slots"); + if (gated != null) + { + return gated; + } + return JsonSerializer.Serialize(new { server = resolved.ServerName, diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgStatementTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgStatementTools.cs index bd309e3d28..08e821ae95 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgStatementTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgStatementTools.cs @@ -48,12 +48,20 @@ public static async Task GetPgTopQueries( if (rows.Count == 0) { - return McpHelpers.Status( - "unavailable", - "No PostgreSQL query statistics for this server and window. If this server is SQL " - + "Server, use get_top_queries_by_cpu instead. If it is Aurora PostgreSQL, check that " - + "pg_stat_statements is installed in the database the collector connects to — on some " - + "clusters it exists only in the application database, not in postgres."); + /* The capability answer settles the dialect branch and the stock-PostgreSQL branch + whenever the store knows the engine (#2532), so this sentence is reached in exactly two + states: an Aurora target that produced no rows, which has one genuinely diagnosable + cause, and a row whose engine_kind is NULL, where no claim can be made. It names those + two rather than repeating the ones that can no longer reach this line. */ + return await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_statement_stats") + ?? McpHelpers.Status( + "unavailable", + "No PostgreSQL query statistics for this server and window. On Aurora, check that " + + "pg_stat_statements is installed in the database the collector connects to — on some " + + "clusters it exists only in the application database, not in postgres. Otherwise the " + + "store has not recorded this server's engine yet, and a target it cannot classify may " + + "not be a PostgreSQL one at all — check list_servers."); } var totalTimeMs = rows.Sum(r => r.TotalExecTimeMs); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWaitTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWaitTools.cs index 6ec76e2f3b..257e6d2156 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWaitTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWaitTools.cs @@ -50,11 +50,20 @@ public static async Task GetPgWaitStats( so, rather than letting a caller read "no waits" as "no waiting". */ if (rows.Count == 0) { - return McpHelpers.Status( - "unavailable", - "No PostgreSQL wait data for this server and window. If this server is SQL Server, " - + "use get_wait_stats instead; if it is PostgreSQL but not Aurora, cumulative wait " - + "counters are not available (core PostgreSQL does not provide them)."); + /* Both halves of the old ambiguity are answerable when the store knows the engine (#2532): + a SQL Server target gets the dialect answer, a stock PostgreSQL one gets the Aurora-only + answer, and both say the gap is permanent instead of inviting a hunt. So the sentence + below is reached in exactly two states — an Aurora target with no rows in the window, or + a row whose engine_kind is NULL — and it names those rather than repeating the two the + capability answer has already ruled out. */ + return await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_wait_stats") + ?? McpHelpers.Status( + "unavailable", + "No PostgreSQL wait data for this server and window. On Aurora, the pg_wait_stats " + + "collector may not have completed a cycle yet. Otherwise the store has not " + + "recorded this server's engine yet, and a target it cannot classify may not be a " + + "PostgreSQL one at all — check list_servers."); } var totalWaitMs = rows.Sum(r => r.TotalWaitTimeMs); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWraparoundTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWraparoundTools.cs index a044b08ada..58ed6fbe01 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWraparoundTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgWraparoundTools.cs @@ -95,11 +95,18 @@ public static async Task GetPgWraparoundRisk( if (rows.Count == 0) { - return McpHelpers.Status( - "unavailable", - "No PostgreSQL freeze-headroom data for this server and window. This collector runs on " - + "any PostgreSQL target, so an empty result means the server is SQL Server, or " - + "pg_wraparound_stats has not collected yet."); + /* This collector has no AppliesTo gate at all, so the capability answer above catches + every engine the store can classify (#2532). What is left is a PostgreSQL target that has + not collected yet, or a row whose engine_kind is NULL — naming "the server is SQL Server" + here would repeat a branch that can no longer reach this line. */ + return await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_wraparound_stats") + ?? McpHelpers.Status( + "unavailable", + "No PostgreSQL freeze-headroom data for this server and window. This collector runs on " + + "any PostgreSQL target, so either pg_wraparound_stats has not collected yet, or the " + + "store has not recorded this server's engine — and a target it cannot classify may " + + "not be a PostgreSQL one at all. Check list_servers."); } var databases = rows diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgXminTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgXminTools.cs index ef35c0ba27..9164c60723 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgXminTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgXminTools.cs @@ -79,6 +79,16 @@ public static async Task GetPgXminHorizon( holder" is a real finding that redirects the investigation rather than a dead end. */ if (rows.Count == 0) { + /* ...but only when this server COULD have a horizon. On a SQL Server target "nothing is + holding back the xmin horizon" is a confident all-clear about a mechanism that does not + exist there, which is the same defect one engine over (#2532). */ + var gated = await DarlingEngineCapability.NotCollectedStatusAsync( + postgres, resolved.ServerId, resolved.ServerName, "pg_xmin_horizon"); + if (gated != null) + { + return gated; + } + return JsonSerializer.Serialize(new { server = resolved.ServerName, diff --git a/PerformanceMonitor.Collectors/CollectorEngineCapability.cs b/PerformanceMonitor.Collectors/CollectorEngineCapability.cs index fc457617bb..8cff552023 100644 --- a/PerformanceMonitor.Collectors/CollectorEngineCapability.cs +++ b/PerformanceMonitor.Collectors/CollectorEngineCapability.cs @@ -50,11 +50,19 @@ namespace PerformanceMonitor.Collectors; /// connector stamps 0, and the edition axis therefore (correctly) declines to claim anything about it. Kind /// is the coarser and more permanent fact of the two: an edition can change under an operator (a migration /// to Azure SQL Database, an upgrade), while a target's DIALECT is what decides whether a collector's query -/// text could ever be sent at it. The kind axis is answered by the engine half of the collectors' own -/// dispatch gate () -/// for exactly the reason the edition axis asks : a hand-kept list of "what PostgreSQL does not have" would go stale in the -/// direction that keeps passing. +/// text could ever be sent at it. Both axes are answered by asking +/// — the collectors' own +/// dispatch gate — over every target shape the axis permits, for the same reason: a hand-kept list of "what +/// PostgreSQL does not have" would go stale in the direction that keeps passing. +/// +/// Each axis sweeps what it does not fix (#2532). The edition sweep fixes the two Azure flags +/// and varies msdb access, RDS-ness and the SQL major version; the kind sweep fixes +/// on the PostgreSQL side and varies the PostgreSQL version floors +/// and the recovery state. Same discipline both times: a fact an operator can change is never allowed to +/// produce a "never will", and a fact nothing can change is what a permanence claim is made of. That is why +/// pg_wait_stats — reading aurora_stat_system_waits(), which core PostgreSQL has in no version +/// — is a permanent gap on stock PostgreSQL, while pg_stat_io's PG16 floor and the writer-only +/// autovacuum read are not and keep the unavailable vocabulary. /// public static class CollectorEngineCapability { @@ -92,8 +100,10 @@ public static class CollectorEngineCapability /// falls out of date makes a message vaguer, never wrong. A collector with no entry still gets a correct /// message through the fallback in . /// Public so the tests can hold it to the catalog: every key must be a real collector name, and - /// every key must be a collector that is genuinely gated off SOMEWHERE, so an entry cannot outlive the - /// gate that gave it a reason to exist. + /// every key must be either a collector its OWN gate shuts out somewhere or one a shipped read asks the + /// capability question about, so an entry cannot outlive the reason it exists. The "its own gate" half + /// deliberately excludes the DIALECT gap every collector now has on every other engine — a check that + /// counted that would pass for any name anyone typed. /// public static readonly IReadOnlyDictionary CapturePathByCollector = new Dictionary(StringComparer.Ordinal) @@ -110,6 +120,21 @@ public static class CollectorEngineCapability ["database_states"] = "the sys.databases state snapshot", ["ag_replica_states"] = "the Always On availability replica states", ["ag_database_replica_states"] = "the Always On per-database replica states", + + /* The PostgreSQL side of the same table (#2532). The first two read an Aurora-only surface, so + they are permanent gaps on stock PostgreSQL — the mirror of the twelve above, which are + permanent gaps on Azure SQL Database. The other six are here because their reads ask the + capability question on the DIALECT axis: a SQL Server target asked get_pg_xmin_horizon used + to be told nothing was holding its horizon back, which is a confident all-clear about a + mechanism that engine does not have. */ + ["pg_wait_stats"] = "the aurora_stat_system_waits() cumulative wait counters", + ["pg_statement_stats"] = "the aurora_stat_statements() per-statement history", + ["pg_blocking"] = "the pg_blocking_pids() lock-wait samples", + ["pg_io_stats"] = "the pg_stat_io per-backend I/O counters", + ["pg_autovacuum_stats"] = "the pg_stat_user_tables autovacuum backlog", + ["pg_replication_slots"] = "the pg_replication_slots WAL-retention snapshot", + ["pg_wraparound_stats"] = "the per-database transaction-ID freeze headroom", + ["pg_xmin_horizon"] = "the xmin-horizon holders behind vacuum", }; /// @@ -168,12 +193,141 @@ public static IEnumerable TargetsWithEngineEdition(int engi } } + /// + /// The PostgreSQL major versions swept when asking whether ANY target of an engine KIND runs a collector + /// (#2532). Same shape and same reasoning as one engine over: 0 is + /// "unknown, assume newest" and 99 is above any floor, with the real majors carried so a gate written as + /// a RANGE is answered rather than by whichever representative value happened to be picked. + /// 13 is the floor because that is the oldest major any monitored target could plausibly be; the + /// list is deliberately generous, since a major MISSING from it can only make the derivation claim a gap + /// that a real target does not have. + /// + private static readonly int[] PostgresMajorVersionSweep = { 0, 13, 14, 15, 16, 17, 99 }; + + /// + /// The server_version_num values swept alongside the major. Carried as its OWN dimension rather + /// than derived from the major, because the gates that will read it are MINOR-version gates + /// (aurora_stat_resource_usage() needs 16.9+/17.5+ and is absent on 17.4), and a sweep that + /// derived the minor from the major would answer every such gate from one arbitrary minor. + /// The cross product includes incoherent pairs — major 16 alongside version 170005 — and + /// that is deliberate. A superset of the real target shapes can only ever find MORE shapes that pass a + /// gate, so its error direction is under-claiming: a gate satisfiable only by a combination no real + /// server has would be reported as collected, and the read keeps its unavailable vocabulary. The + /// opposite error — a shape that no sweep produces, so a gate matches nothing and the derivation + /// announces a permanent gap — is the one this whole mechanism exists to prevent, and a superset cannot + /// make it. + /// + private static readonly int[] PostgresVersionNumSweep = { 0, 160008, 160009, 170004, 170005, 999999 }; + + /// + /// Every target shape an engine KIND permits, for the exhaustive gate sweep — the engine-kind twin of + /// (#2532). + /// + /// What the kind fixes, and what it must not. On the PostgreSQL side the kind fixes exactly + /// one fact: , which is what separates the two PostgreSQL + /// tokens and is not something an operator can change — a stock PostgreSQL server does not become Aurora. + /// Everything else varies, because none of it is implied by the kind and all of it is FIXABLE: an upgrade + /// moves and + /// past a floor, and a connection to the writer + /// moves . Claiming permanence over any of those would be + /// the #2511 over-claim one engine over. + /// + /// The SQL Server arm varies the Azure flags, where + /// fixes them — because the KIND does not decide an edition. That + /// is what keeps the two axes from answering each other's question: a collector gated off on Azure SQL + /// Database still runs on some SQL Server, so it is not a kind gap, and the edition axis is left to say + /// so with the edition named. The impossible both-Azure-flags shape is generated rather than skipped, for + /// the superset reason above and because skipping it would break the full-cross-product property that + /// makes conjunctive gates answerable. + /// + /// An unknown, absent or unrecognised kind yields NOTHING. Callers must ask + /// first; an empty sweep read as "no shape runs it" would turn + /// silence into the most confident claim in the vocabulary. + /// + /// Adding a field to that a PostgreSQL gate reads means adding + /// it here too. A fact this sweep never varies sits at its CLR default in every shape, so a gate + /// written on it fails all of them and the derivation reports a permanent engine gap for a collector that + /// runs. EveryFactAPostgresGateReads_IsVariedBySweepOrFixedByKind decodes every PostgreSQL gate's + /// IL and fails the build when it happens — the twin of the #2518 guard on the edition axis. + /// + public static IEnumerable TargetsWithEngineKind(string? engineKind) + { + switch (MonitoredEngineKind.EngineOf(engineKind)) + { + case CollectorTargetEngine.PostgreSql: + var isAurora = MonitoredEngineKind.IsAurora(engineKind); + + foreach (var major in PostgresMajorVersionSweep) + { + foreach (var versionNum in PostgresVersionNumSweep) + { + foreach (var isInRecovery in new[] { false, true }) + { + yield return new CollectorTargetInfo + { + Engine = CollectorTargetEngine.PostgreSql, + IsAurora = isAurora, + PostgresMajorVersion = major, + PostgresVersionNum = versionNum, + IsInRecovery = isInRecovery, + }; + } + } + } + + break; + + case CollectorTargetEngine.SqlServer: + foreach (var major in MajorVersionSweep) + { + foreach (var hasMsdbAccess in new[] { true, false }) + { + foreach (var isAwsRds in new[] { false, true }) + { + foreach (var isAzureSqlDb in new[] { false, true }) + { + foreach (var isAzureManagedInstance in new[] { false, true }) + { + yield return new CollectorTargetInfo + { + Engine = CollectorTargetEngine.SqlServer, + IsAzureSqlDb = isAzureSqlDb, + IsAzureManagedInstance = isAzureManagedInstance, + IsAwsRds = isAwsRds, + HasMsdbAccess = hasMsdbAccess, + SqlMajorVersion = major, + }; + } + } + } + } + } + + break; + } + } + /// A bare SQL Server target, for the engine half of the dispatch gate alone. private static readonly CollectorTargetInfo SqlServerProbe = new() { Engine = CollectorTargetEngine.SqlServer }; /// The same, one engine over — for the engine-KIND axis (#2530). private static readonly CollectorTargetInfo PostgresProbe = new() { Engine = CollectorTargetEngine.PostgreSql }; + /// + /// The bare probe for an engine kind, or null for a kind this build does not recognise. Used only + /// to ask which of the two reasons a kind gap has — a foreign DIALECT, which the dispatch gate's engine + /// half stops, or the collector's own gate — so the message can name the real one. The gap itself is + /// decided by over the sweep, never + /// here. + /// + private static CollectorTargetInfo? ProbeForKind(string? engineKind) => + MonitoredEngineKind.EngineOf(engineKind) switch + { + CollectorTargetEngine.PostgreSql => PostgresProbe, + CollectorTargetEngine.SqlServer => SqlServerProbe, + _ => null, + }; + /// The closing sentence both axes end on. One copy, because the two messages have to agree /// about what a permanent gap is; a second wording would eventually say something subtly different about /// whether it is worth chasing. @@ -241,9 +395,8 @@ apply to it. Without this the sweep would report all eight PG definitions as per } /// - /// True when 's query dialect can be sent at a server of this engine - /// KIND — the second axis (#2530), and the one that separates a PostgreSQL target from a SQL Server that - /// has never connected. + /// True when SOME server of this engine KIND runs — the second axis + /// (#2530), and the one that separates a PostgreSQL target from a SQL Server that has never connected. /// An absent, unrecognised, or unclassifiable kind answers TRUE, as does an unknown collector name /// — the same true-on-miss default the edition axis carries, for the same reason: nothing to ask means /// nothing to claim. @@ -260,38 +413,51 @@ public static bool IsCollectedOnEngineKind(string collectorName, string? engineK /// reason the edition axis's does: by name this can only ever be handed a gate that ships, so nothing /// asserted through it could distinguish a derivation from a list that happens to match. /// - /// Why only the ENGINE half of the dispatch gate, and not a sweep. The edition axis sweeps - /// every target shape an edition permits and claims a gap only when NO shape runs, because the facts it - /// sweeps — msdb access, RDS, version — are fixable and a permanence claim over them would be the - /// over-claim #2511 removed. On this axis there is nothing to sweep: a target's dialect is not a fact an - /// operator can move, so — the collectors' own engine gate, which is what actually stops the dispatch - /// — is the whole answer. It is still DERIVED: flip a definition's - /// and this answer flips with it, which is what - /// CollectorEngineCapabilityMovingGateTests demonstrates rather than assumes. + /// The whole dispatch gate, over a sweep (#2532). #2530 asked only + /// — the engine + /// half — and deliberately left the collectors' own AppliesTo out of it, because a gate on a + /// FIXABLE fact must never be reported as permanent. That is still the rule; what changed is that the + /// sweep now separates the fixable facts from the one the kind fixes, so the question can be asked in + /// full: is there any target of this engine kind, under any combination of the facts the kind does not + /// decide, for which this collector runs? varies the PostgreSQL + /// version floors and the recovery state — an upgrade and a writer connection move those — and fixes + /// , which nothing moves. So pg_stat_io's PG16 floor and + /// the writer-only autovacuum read keep the unavailable vocabulary, while pg_wait_stats + /// reading aurora_stat_system_waits() is a permanent gap on stock PostgreSQL, which is what it + /// is. /// - /// What this deliberately does NOT claim. The PostgreSQL collectors' own - /// gates — Aurora-only surfaces, the PG16 floor on - /// pg_stat_io, the writer-only autovacuum read — are the fixable/variable half on this side of the - /// fence, and are left to the unavailable vocabulary exactly as msdb access is on the SQL Server - /// side. Making pg_wait_stats a permanent gap on STOCK PostgreSQL is a real and available claim - /// (Aurora-ness is fixed by the kind the way the Azure flags are fixed by the edition), and it wants the - /// PostgreSQL twin of the #2518 sweep-dimension guard before it ships — filed rather than smuggled in - /// here, because a sweep without that guard over-claims silently, which is the failure this whole - /// mechanism exists to avoid. + /// It is still DERIVED in both directions: flip a definition's + /// and the dialect answer flips with it; shut its + /// AppliesTo on Aurora-ness and the stock-PostgreSQL answer flips with that. Both are demonstrated + /// by moving a gate rather than asserted about the shipped ones + /// (CollectorEngineCapabilityMovingGateTests), and + /// EveryFactAPostgresGateReads_IsVariedBySweepOrFixedByKind is what stops the sweep quietly + /// leaving a fact at its default and manufacturing a gap for a collector that runs (#2518's twin). + /// + /// An unknown kind still claims nothing, and the check is explicit rather than falling out + /// of an empty sweep: yields no shapes for a token this build does + /// not recognise, and "no shape runs it" would otherwise read as the most confident claim in the + /// vocabulary instead of the silence it has to be. /// public static bool IsCollectedOnEngineKind(ICollectorSchemaInfo definition, string? engineKind) { - var probe = MonitoredEngineKind.EngineOf(engineKind) switch + /* Not known to be anything — the pre-#2530 state of every row, of every store an older service + wrote, and of every token a newer one writes. No claim, which is the same silence + UnknownEngineEdition keeps. */ + if (!MonitoredEngineKind.IsKnown(engineKind)) { - CollectorTargetEngine.PostgreSql => PostgresProbe, - CollectorTargetEngine.SqlServer => SqlServerProbe, - /* Not known to be anything — the pre-#2530 state of every row, and of every store an older - service wrote. No claim, which is the same silence UnknownEngineEdition keeps. */ - _ => null, - }; + return true; + } + + foreach (var target in TargetsWithEngineKind(engineKind)) + { + if (CollectorCatalog.AppliesTo(definition, target)) + { + return true; + } + } - return probe is null || CollectorCatalog.EngineMatches(definition, probe); + return false; } /// @@ -329,11 +495,30 @@ public static bool IsCollectedOnEngineKind(ICollectorSchemaInfo definition, stri starts with a vowel — an indefinite article in the template would read "a Aurora PostgreSQL". Article agreement belongs in the template or nowhere, never in a per-entry special case that the next entry gets wrong; the same reasoning the capture-path phrasing already follows. */ - return $"{serverName} runs {MonitoredEngineKind.DescribeEngineKind(engineKind)}. The " + - $"{collectorName} collector is written against " + - $"{DescribeTargetEngine(definition.TargetEngine)} and the dispatch gate's engine half never " + - $"sends it at another engine, so this server does not collect " + - $"{CapturePathOf(collectorName)}, and never will. {PermanentGapEpilogue}"; + var engine = $"{serverName} runs {MonitoredEngineKind.DescribeEngineKind(engineKind)}. "; + + /* TWO reasons a collector is a kind gap, and they must not be described with one sentence + (#2532). A DIALECT mismatch is stopped by the dispatch gate's engine half before the + collector's own gate is consulted at all; a same-dialect gap — pg_wait_stats on stock + PostgreSQL — is the collector's own AppliesTo, exactly as on the edition axis. Saying "written + against PostgreSQL and never sent at another engine" about a PostgreSQL collector on a + PostgreSQL server would be plainly false to the one operator best placed to notice. */ + if (ProbeForKind(engineKind) is { } probe && !CollectorCatalog.EngineMatches(definition, probe)) + { + return engine + + $"The {collectorName} collector is written against " + + $"{DescribeTargetEngine(definition.TargetEngine)} and the dispatch gate's engine half never " + + $"sends it at another engine, so this server does not collect " + + $"{CapturePathOf(collectorName)}, and never will. {PermanentGapEpilogue}"; + } + + /* Deliberately the edition axis's own wording, verbatim past the engine name: the two are the + same finding — the collector's own gate excludes every target of this shape — and two + spellings of it would eventually disagree about whether it is worth chasing. */ + return engine + + $"The {collectorName} collector does not run on that engine — its own AppliesTo gate " + + $"excludes it — so this server does not collect {CapturePathOf(collectorName)}, and never " + + $"will. {PermanentGapEpilogue}"; } if (IsCollectedOnEngineEdition(definition, engineEdition)) diff --git a/PerformanceMonitor.Collectors/CollectorTargetInfo.cs b/PerformanceMonitor.Collectors/CollectorTargetInfo.cs index f7ab503c58..7d563f877d 100644 --- a/PerformanceMonitor.Collectors/CollectorTargetInfo.cs +++ b/PerformanceMonitor.Collectors/CollectorTargetInfo.cs @@ -17,10 +17,14 @@ namespace PerformanceMonitor.Collectors; /// "can this collector EVER run on this engine edition" is derived, and a fact it never varies /// sits at its CLR default in every shape it produces — so a gate written on it matches none of /// them and the derivation announces a permanent, unfixable engine gap for a collector that runs -/// perfectly well. Adding a field that nothing gates on is fine ( -/// is exactly that today); adding the GATE without extending the sweep is what breaks, and -/// EveryFactASqlServerGateReads_IsVariedBySweepOrFixedByEdition fails the build when it -/// happens rather than leaving it to be noticed (#2518). +/// perfectly well. Adding a field that nothing gates on is fine; adding the GATE without extending +/// the sweep is what breaks, and EveryFactASqlServerGateReads_IsVariedBySweepOrFixedByEdition +/// fails the build when it happens rather than leaving it to be noticed (#2518). +/// The same rule holds one engine over (#2532): a field a PostgreSQL +/// gate reads must be swept by +/// , or fixed by the engine kind the +/// way is. EveryFactAPostgresGateReads_IsVariedBySweepOrFixedByKind is +/// the twin guard. /// public sealed class CollectorTargetInfo {