Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Darling/Darling.Tests/CollectorRunnerConnectionEngineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ namespace Darling.Tests;
///
/// <para><b>What this exists to catch.</b> The runner had two paths: the per-database branch resolved
/// <c>TargetProviders.For(target)</c> correctly, and the branch serving everything else did
/// <c>new SqlConnection(server.ConnectionString)</c> literally. Six of the seven PostgreSQL collectors take
/// the second path — only <c>pg_autovacuum_stats</c> fans out — so they were handed a SQL Server connection.
/// <c>new SqlConnection(server.ConnectionString)</c> literally. Six of the seven PostgreSQL collectors then
/// shipping took the second path — only <c>pg_autovacuum_stats</c> fanned out — so they were handed a SQL Server connection.
/// SqlClient rejects Npgsql's keywords while PARSING the connection string, before any query runs
/// ("Keyword not supported: 'host'"), and the resulting <c>ArgumentException</c> is neither
/// <c>SqlException</c> nor <c>PostgresException</c>, so it missed both fault-classification arms and recorded
Expand Down
72 changes: 72 additions & 0 deletions Darling/Darling.Tests/ReadmeDerivedCountPinTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,78 @@ public void TestConnectionTranscript_DenominatorsMatchThePostgresCollectorCount(
Assert.All(denominators, d => Assert.Equal(PostgresCollectors, d));
}

/// <summary>
/// The "What gets collected" table names EVERY PostgreSQL collector, in both directions (#3261). It sat
/// at 12 of 27 across five collector waves with every check green, because nothing failed when a
/// collector shipped without a row — the exact defect this file exists for, one resolution finer than
/// a count.
///
/// <para>A bespoke fact rather than a <see cref="SetPins"/> entry, and the reason is the shared
/// mutation proof: <see cref="EveryPin_ReportsAnInjectedDrift"/> drops the FIRST occurrence of each
/// member's backticked name inside the span, and several collector names legitimately appear inside
/// OTHER rows before their own — <c>pg_replication_slots</c> is a SOURCE of <c>pg_xmin_horizon</c>,
/// whose row sits above its own. Dropping that occurrence would leave extraction intact and the shared
/// proof would report this pin as vacuous when it is not. So this fact carries its own drift proof,
/// one that deletes a whole ROW instead of a token.</para>
/// </summary>
[Fact]
public void WhatGetsCollectedTable_NamesEveryPostgresCollector()
{
var readme = ReadReadme();
var span = WhatGetsCollectedSpan(readme);

var expected = CollectorCatalog.All
.Where(c => c.TargetEngine == CollectorTargetEngine.PostgreSql)
.Select(c => c.Name)
.ToHashSet(StringComparer.Ordinal);
var actual = RowLeadingNames(span);

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,
"the What-gets-collected table: the collector catalog ships " + string.Join(", ", missing)
+ ", which Darling/README.md's table has no row for. Add the row — a collector the inventory "
+ "documentation does not name is one nobody installing this product knows they are running.");

Assert.True(extra.Length == 0,
"the What-gets-collected table: Darling/README.md has a row for " + string.Join(", ", extra)
+ ", which is not a PostgreSQL collector in the catalog. Remove the row or fix its name.");

/* The drift proof: deleting one collector's whole row must surface as a missing member. The victim
is a name that appears in no other row's cells, so the proof stays about the ROW being read. */
var victimLine = span.Split('\n')
.First(l => l.StartsWith("| `pg_wraparound_stats`", StringComparison.Ordinal));
var mutated = span.Replace(victimLine, string.Empty, StringComparison.Ordinal);
Assert.NotEqual(span, mutated);
Assert.DoesNotContain("pg_wraparound_stats", RowLeadingNames(mutated));
}

/// <summary>The table's rows: everything between the header row and the paragraph after the table.</summary>
private static string WhatGetsCollectedSpan(string readme)
{
var match = Regex.Match(
readme,
@"\| Collector \| Source \| Cadence / retention \| Why it exists \|(.+?)\r?\n\r?\nThe two Aurora-only collectors",
RegexOptions.Singleline);

Assert.True(match.Success,
"Darling/README.md no longer contains the What-gets-collected table in the pinned shape "
+ "(its header row, then rows, then a blank line, then the 'The two Aurora-only collectors' "
+ "paragraph). It restates the catalog's PostgreSQL roster, so keep it parseable — a reworded "
+ "header or a moved table has to fail here rather than quietly stop being checked.");

return match.Groups[1].Value;
}

/// <summary>Each row's FIRST cell — the collector the row is about, never a name inside its prose.</summary>
private static IReadOnlySet<string> RowLeadingNames(string span) =>
Regex.Matches(span, @"(?m)^\| `([a-z0-9_]+)`")
.Select(m => m.Groups[1].Value)
.ToHashSet(StringComparer.Ordinal);

/// <summary>
/// Non-vacuity, the way <see cref="CiClusterWorkerSizingTests.ParsedSettings_Comparison_FailsOnAnInjectedDrift"/>
/// does it: mutate a COPY of the README so each pinned number is wrong, and require the identical
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1096,8 +1096,8 @@ items WERE found and merely some of their probes failed. Lite's twin is _lastCol
/* The engine's provider, resolved ONCE for both branches. It used to be resolved only inside the
per-database branch, and the branch below opened a hardcoded SqlConnection — so every collector
that does NOT fan out per database was handed a SQL Server connection whatever the target was.
Six of the seven PostgreSQL collectors take that path (only pg_autovacuum_stats fans out), and
SqlClient rejects Npgsql's keywords while parsing the connection string, before any query runs:
Six of the seven PostgreSQL collectors then shipping took that path (only pg_autovacuum_stats
fanned out), and SqlClient rejects Npgsql's keywords while parsing the connection string, before any query runs:
"Keyword not supported: 'host'". Worse, an ArgumentException is neither SqlException nor
PostgresException, so it missed BOTH classification arms in DarlingWorker and recorded a raw
ERROR every sweep forever — including for all three Tier 0 outage predictors. */
Expand Down Expand Up @@ -4627,8 +4627,8 @@ internal async Task<SqlConnection> OpenAzureDatabaseConnectionAsync(ServerRuntim
/// The connection for a collector that reads the server as a whole — engine-resolved from the probed
/// target, never constructed directly.
/// <para>Extracted so it can be PINNED by test. This is the exact seam that broke: the non-per-database
/// branch built a <c>SqlConnection</c> literally, so six of the seven PostgreSQL collectors got a SQL
/// Server connection and failed in the connection-string parser before running a query. Both engines'
/// branch built a <c>SqlConnection</c> literally, so six of the seven PostgreSQL collectors then
/// shipping got a SQL Server connection and failed in the connection-string parser before running a query. Both engines'
/// providers were already correct and individually tested — nothing asserted that the RUNNER asked them.
/// A test that opens nothing and only checks the returned TYPE is enough to catch it, which is why it is
/// worth having.</para>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace PerformanceMonitor.Darling.Service.Mcp;
[McpServerToolType]
public sealed class DarlingMcpPgStatementTools
{
[McpServerTool(Name = "get_pg_top_queries"), Description("Gets the top PostgreSQL query shapes by total execution time over a time period, for Amazon Aurora PostgreSQL targets. Includes Aurora's I/O source breakdown, which stock PostgreSQL cannot provide: a block 'read' may have come from the distributed storage volume or from the local NVMe Optimized Reads cache, and the two have very different costs. Also reports peak memory per statement, the closest PostgreSQL equivalent of a memory grant, and WAL bytes generated, which has no SQL Server DMV counterpart. Returns query_text for each statement, captured hourly and keyed on queryid, or null when none has been captured yet (a statement first seen minutes ago, or a queryid minted by a major-version upgrade). queryid itself is stable within a major version but changes across a major upgrade — which is exactly why the text is STORED rather than fetched live: after an upgrade the live view no longer holds the old ids, so their text would otherwise be unrecoverable and the history would read as a list of integers. queryid is returned as a STRING, not a number: it is a signed 64-bit value spread over the whole int8 range, so most ids exceed what a JSON number survives and a numeric wire form would be silently rounded by any parser that decodes numbers as IEEE-754 doubles — after which it matches nothing. Compare it and join on it as text. This is a separate tool from get_top_queries_by_cpu, which covers SQL Server.")]
[McpServerTool(Name = "get_pg_top_queries"), Description("Gets the top PostgreSQL query shapes by total execution time over a time period, wherever pg_stat_statements is installed: on Amazon Aurora the collector reads aurora_stat_statements(), and on any other PostgreSQL the vanilla view, with the Aurora-only columns null rather than zero. On Aurora that includes the I/O source breakdown, which stock PostgreSQL cannot provide: a block 'read' may have come from the distributed storage volume or from the local NVMe Optimized Reads cache, and the two have very different costs. Also reports peak memory per statement (Aurora only), the closest PostgreSQL equivalent of a memory grant, and WAL bytes generated, which has no SQL Server DMV counterpart. Returns query_text for each statement, captured hourly and keyed on queryid, or null when none has been captured yet (a statement first seen minutes ago, or a queryid minted by a major-version upgrade). queryid itself is stable within a major version but changes across a major upgrade — which is exactly why the text is STORED rather than fetched live: after an upgrade the live view no longer holds the old ids, so their text would otherwise be unrecoverable and the history would read as a list of integers. queryid is returned as a STRING, not a number: it is a signed 64-bit value spread over the whole int8 range, so most ids exceed what a JSON number survives and a numeric wire form would be silently rounded by any parser that decodes numbers as IEEE-754 doubles — after which it matches nothing. Compare it and join on it as text. This is a separate tool from get_top_queries_by_cpu, which covers SQL Server.")]
public static async Task<string> GetPgTopQueries(
NpgsqlDataSource postgres,
[Description("Server name or display name.")] string? server_name = null,
Expand Down
Loading
Loading