fix(monitoring): five panels answered an absent measurement with a confident zero (#424) - #452
Merged
Conversation
…nfident zero (#424) #448 settled a rule for one panel and a live browser drive of that same release found the neighbours still breaking it. Connected to Apache Cassandra 5.0.9 in Chrome, Monitoring -> Tables read `Tables 0 / "0 rows"`, `Size "0 B" / "Total"` and `Vacuum 0 / "OK"` while the list underneath correctly said "No tables found" - and the Overview tab of the same session, same connection, read `Tables 6 / 2 indexes`. The product answered one question two ways, republished the sentinel #448 had just deleted, and issued a clean bill of health for an operation Apache Cassandra does not have. An audit of every tab found the same shape in five of six. The cause is never engine-specific: a `reduce` over `data?.tables ?? []`, or a `?? 0` on a field its own type marks optional, turns a refusal into a number. The rule applied throughout, unchanged from #448: ABSENCE and ZERO are different inputs. A provider reporting nothing is saying the figure is not knowable; a provider reporting 0 has measured one. Only the first changes, and every panel now pins the second with a test of its own so the two cannot be collapsed again. - **TablesTab** - the panel the browser proved. Statistics are absent, rather than the database being empty, exactly when the engine knows of tables and reports none for them, which the required `overview.tableCount` can say and `tables` alone cannot. The Vacuum card is now a capability question rather than a data one: `supportsMaintenance: false` means the engine has no such operation, so the card states no verdict for it, and `lastVacuum` no longer renders the word "Never" - a measurement on PostgreSQL, a claimed history elsewhere - for an engine that cannot vacuum. - **PerformanceTab** - `bufferPoolUsage` and `deadlocks` are optional and genuinely omitted: Trino's provider records in prose that it "holds no buffer pool" and "takes no locks, so there are no deadlocks to count", Cassandra and Druid omit both, and SQLite sets the first undefined outright. The panel was rating that absence "Poor" from a fabricated 0, and badging the second "Healthy". It already did the right thing three lines above for `cacheHitRatio`; the two now agree, trend charts included, which stops the charts plotting a zero floor nobody measured. - **OverviewTab** - the same two fields on the compact rows. - **PoolTab** - the zeros were not even the provider's: the route answers a literal `{total: 0, idle: 0, active: 0, waiting: 0}` with a "not available" message for engines that pool nothing, and the panel rendered "Max pool size 0" and "No queue" from it. - **QueriesTab** - same shape, same fix. - **SessionsTab** - examined and REFUTED, no change: its counts are of a list the user can see, which is honest, and its `?? 0` fallbacks read fields their types mark required. Nineteen tests added across the five panels, each pinning both inputs. Provider docs synced where they described a panel's rendering: cassandra, druid, trino, elasticsearch, opensearch and libredb. Found by driving the product rather than by a gate - all six gates were green over the frame that showed `Vacuum 0 - OK`.
…nstead of a nested ternary The absence-versus-zero work put four of these components over Sonar's cognitive complexity threshold (S3776: Overview 16, Pool 16, Tables 20, Performance 21) and left four nested ternaries (S3358). The quality gate passed anyway, so this is legibility rather than a defect - but the nested ternaries are the exact places a reader is most likely to misread, because each encodes THREE states in a shape that looks like two. - `vacuumIconClass` and `VacuumNote` in TablesTab, `deadlockIconClass` in PerformanceTab. Each replaces `absent ? grey : positive ? warn : green`. Written as early returns the third state is visible: an engine that publishes no statistics at all is not an engine that looked and found nothing, and it must not borrow the green of one that did. - `bloatBadgeVariant` for the >20 / >10 / else thresholds. - `formatBytes`, `formatNumber` and `formatVacuumDate` hoisted out of the TablesTab body to module scope. They are pure and were being re-created on every render; moving them also leaves the component's own complexity to the absence decisions it exists to make. - `MetricTrendCard` and `metricSeries` in PerformanceTab replace three near-identical chart cards and three near-identical `flatMap` builders. `heading` is a separate prop because the deadlock card is labelled "Deadlock Trend" while its chart is titled "Deadlocks" - deriving one from the other would have silently changed that copy. - `PoolStatsGrid` in PoolTab, `PerformanceSummaryCard` and `QuickStatsCard` in OverviewTab: presentation over a decision already made, moved to where it reads as one. No rendered output changes, which is what the existing tests assert: all 30 component groups and all 330 core files pass untouched, and every extracted branch was already exercised, so the four files stay at 100% line coverage (Tables 214 -> 232 lines, Overview 213 -> 231, Performance 244, Pool 118 -> 125, all fully hit). One extraction was caught by the typechecker rather than by a test, and it is worth recording: moving the Performance card out of `OverviewTab` left `performance?.checkpointWriteTime` resolving to the DOM `performance` global, not the prop. It failed only because `Performance` has no `checkpointWriteTime`. A colliding property name would have compiled.
Sonar measured the previous commit and the four cognitive-complexity findings and four nested ternaries are gone, but the five new components it could now see each raised S6759: a React component must not mutate its props, so the type should say so. The rule had never fired in this file before because no prop signature here had been new code since the rule was enabled - the surrounding components are unflagged rather than exempt. Type-level only. Nothing rendered changes and no test moved: 30 component groups pass, and all four files stay fully covered (Overview 231/231, Performance 244/244, Pool 125/125, Tables 232/232).
|
This was referenced Aug 21, 2026
cevheri
added a commit
that referenced
this pull request
Aug 21, 2026
…ose it (#453) U14 asked that a metric an engine cannot report render as unavailable rather than as zero, on every provider whose health payload omits it. PR #452 (408a1e9) implemented exactly that across the Overview, Performance, Tables, Pool and Queries tabs, so by this file's own convention the entry goes rather than being marked done. #452 did not touch this file, which is the only reason it was still here. Deleting it alone would read as the whole problem being solved, and it is not. D7 gains a paragraph saying which half is closed: #452 fixed the PANEL half, where a field the provider omits is now rendered as unavailable. D7 is the PROVIDER half - mysql.ts and mongodb.ts return `cacheHitRatio: 99` from a catch and libredb.ts returns 100 unconditionally - and a fabricated number is not an absence, so there is nothing for a panel to detect. The OceanBase reading recorded in D7, 99 percent cache hit on every refresh forever against a tenant with no performance_schema at all, is exactly as true after #452 as before it. U12 is untouched too: QueriesTab still hardcodes "pg_stat_statements required" as the empty state for every engine. Both are named in the #424 comment of 2026-08-21 alongside this deletion so the record is in one place. No entry other than U14 is removed, and no B-numbered entry is touched, so the M2 deferral assertions in tests/unit/agent-documentation.test.ts (which match `### (B\d+)\.` only) are unaffected. All six gates green.
cevheri
pushed a commit
that referenced
this pull request
Aug 22, 2026
…read as 99% (#424) (#458) Two defects found while connecting the product to MariaDB 12.3 and MySQL 26.7 side by side, both in the MySQL provider rather than in either engine. The overview named the wrong vendor. `getOverview()` returned `MySQL ${version}` unconditionally, so a MariaDB server whose `VERSION()` answers `12.3.2-MariaDB-ubu2404` was published as "MySQL 12.3.2-MariaDB-ubu2404" - an assertion the server never made. The same statement was wrong for every relative whose version string names itself: TiDB `8.0.11-TiDB-v8.5.1`, Vitess `8.0.43-Vitess` and OceanBase `5.7.25-OceanBase_CE-v4.4.2.1`, all four being the strings WIRE_COMPATIBLE_ENGINES already records from a live probe. A version string that names its own vendor is now passed through unchanged, and only a bare number gets the `MySQL ` prefix. StarRocks and SingleStore are deliberately not on that list: both answer with a plain MySQL number and give nothing to key on, which the compatibility table already records as their behaviour. `getPerformanceMetrics()` manufactured measurements. MariaDB ships `performance_schema` OFF by default (measured on `mariadb:12.3`: `@@performance_schema` = 0) and the tables still exist, so the metric queries do not fail - each is a bare `SELECT (subquery)` that answers one row of NULLs. `parseFloat(row?.hit_ratio || "99")` turned that NULL into a confident 99% cache hit ratio, and the buffer pool and QPS defaults turned theirs into 0. Connected in the browser, a stock MariaDB server therefore reported a healthy 99% cache for a figure nobody had taken. This is the fault #448 and #452 removed from the panels, still live in the provider that feeds them: ABSENCE and ZERO are different inputs, and only the first must stay invisible. Every reading now goes through `measuredNumber()` and a field with nothing behind it is omitted; if `performance_schema` is absent outright the method returns `{}` rather than a set of defaults. `getHealth()` had the same fabricated 99, and now uses the shared `formatCacheHitRatio()`, which is the one place that already turns an absent ratio into "N/A". One thing the fix uncovered: `Innodb_deadlocks` is MariaDB's status variable and MySQL does not publish it. `SHOW STATUS LIKE 'Innodb_deadlocks'` is empty on both MySQL 8.0.46 and 26.7.0, so `parseInt(row?.Value || "0")` had been reporting a deadlock count MySQL never gave. It is now absent there and present on MariaDB, which makes it the one performance figure a default MariaDB reports and a MySQL server does not. Measured after the change, same two connections, same session: MySQL 26.7 version "MySQL 26.7.0" cacheHitRatio 96.49, queriesPerSecond 0.03, bufferPoolUsage 14.89 MariaDB 12.3 version "12.3.2-MariaDB-ubu2404" deadlocks 0, and nothing else claimed Docs and tests move with the code. `docs/providers/mysql.md` gains a section on the MySQL-protocol engines and rewrites the monitoring degradation notes; the compatibility registry and its published table record the `performance_schema` default as measured rather than assumed. Six new provider tests cover the vendor label for all four probed strings, the plain-MySQL case, `performance_schema` off, `performance_schema` absent, and a server that publishes no deadlock counter.
cevheri
added a commit
that referenced
this pull request
Aug 23, 2026
…t had done, plus five more (#469) * fix(providers): every MySQL DDL and DML reported a failure for work it had done, plus five more Six defects, each measured against a live server before and after. Four were BACKLOG entries; two were found while closing them and are the most serious of the six. Every non-SELECT statement on MySQL threw. `query()` called `.map` on mysql2's first return value, which is a `ResultSetHeader` object rather than an array for DDL and DML, so `CREATE TABLE`, `INSERT`, `UPDATE` and `DELETE` each answered `result.rows.map is not a function` AFTER the server had applied the statement - the answer that makes a user retry and double-apply it. Measured through the provider with an interleaved SELECT proving each statement had landed, and again in the browser: a `CREATE TABLE` from the editor now reports "0 rows" with the table created, and a two-row `INSERT` reports "2 rows". The tests could not see it because they mocked `execute` as returning an array; the mock type is now wide enough to express the real shape, which is what hid it. X7 - the DDL export typed a numeric or a timestamp column as TEXT, because the wire hands it a string. The inference was never at fault: it never saw a type. `pg`, `mysql2`, `oracledb` and `mssql` now fill `QueryResult.columnTypes` with the engine's own spelling, joining the six provider families that already did. The mappings were measured, not remembered: Postgres resolves its OIDs through a generated `pg_type` table (149 built-ins; verified over dvdrental as 128 columns, 125 named, 0 wrong), and MySQL's shared type codes are separated by charset 63 and the length tiers rather than by assumption (38 of 39 match `information_schema.DATA_TYPE`; `POINT` and `GEOMETRY` are indistinguishable on the wire). In the browser a Postgres result now reads `price numeric`, `ts timestamp without time zone`, `b bytea` in the grid, and its exported DDL says the same instead of TEXT. That change had a consequence which is fixed in the same PR rather than left to be discovered: a declared type is a BARE base name, because a length cannot be recovered from the wire, and a bare name is not legal DDL everywhere. Measured by replay - MySQL `ERROR 1064`, Oracle `ORA-00906`, and SQL Server accepting it while silently narrowing `nvarchar` to length 1. The export now completes a bare name to its target dialect's unbounded form, and re-spells a foreign engine's declaration rather than writing a `VARCHAR2` into Postgres DDL, which the shells make reachable by keeping the connection type beside the tab's own result. Every arm replayed into its own engine and read back out of its catalog. X8 - the SQL INSERT and DDL exports wrote a binary value as its JSON shape, so a replayed INSERT stored 46 bytes of `{"type":"Buffer","data":[...]}` where six bytes belonged, successfully and silently. Each dialect now writes a literal its own engine accepts, every spelling measured rather than assumed: Postgres's `X'...'` is a bit string and cannot be cast to bytea, MySQL has no `0x` spelling for an empty value, SQL Server rejects the standard form, and Oracle parses neither so the conversion function is the literal. Replayed into seven engines, six bytes each. D7 - three providers answered the performance panel with a fabricated cache hit ratio, and the sweep found five more. All eight are honest now: MySQL's health read no longer dies on a tenant with no `performance_schema` DATABASE (measured on OceanBase as ERROR 1049, where the entry's own three sites had already been fixed by #458), and SQLite, LibreDB, MongoDB, Couchbase, Postgres, Oracle and SQL Server report an unmeasurable ratio as absent, which #452's tabs already render as "Not measured". Two of those fabrications were pinned by tests - one named "reports zero rather than a perfect score when denied" - and a denial is the ordinary case, so the panel was inventing a red critical cache fault. `bufferPoolUsage` went with them in three providers: it was the cache hit ratio wearing a second name, drawn and rated as an independent gauge. In the browser: MySQL 98.8%, Oracle 99.5%, SQLite "Not measured" where it used to claim a fixed 95%. D12 - a pasted `rediss://` URL landed on SSL mode `disable`, so the scheme was silently dropped. It now arrives as `require`, and so does `couchbases://`, whose excuse for being left out ("the provider re-reads the scheme") was untrue: that transport picks http or https from `config.ssl` alone and was posting plaintext to 18091. `mongodb+srv://` is deliberately left unset - the driver turns TLS on itself WITH chain verification, and carrying our `require` there would have stopped an Atlas certificate being checked. Verified in the browser and end to end against a TLS-only Redis, both arms. Follow-ups recorded rather than smuggled in: D13 (Oracle answers every non-SELECT with `rowCount: 0`, including rows it just wrote), D14 (what a real buffer-pool gauge would cost on each engine), D15 (SQLite's table size is `rowCount * 100`), D16 (the TLS a connection string carries in its query string is still dropped), X9 (what `columnTypes` still cannot name) and X10 (MySQL and Cassandra stringify their bytes before the export can see them, which bounds what X8 closed). * docs(export): the reason a dialect is absent from STANDS_ALONE covered only half the cases An external review of #469 approved it and read the absence of the other dialects as settled. It is not, and the module's own comment is why: it justified having no row for clickhouse/trino/sqlite/cassandra by saying those engines spell their types out in full, which is true of the names THEY declare and says nothing about a name arriving from somewhere else. Measured under each target for one Oracle result: postgres writes TEXT, mysql TEXT, mssql NVARCHAR(MAX) - and clickhouse, plus the no-connection default, write VARCHAR2, BINARY_DOUBLE and CLOB, which replay nowhere. The no-connection case is a regression: a value-shaped guess used to write portable standard SQL there. Widening the rule is not the fix - Trino's own bare varchar is legal and unbounded and would become a TEXT it does not have - so each remaining dialect needs a measured row, recorded as BACKLOG X11. Comment and entry only, no behaviour change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(monitoring): five panels answered an absent measurement with a confident zero (#424)
#448 settled a rule for one panel and a live browser drive of that same release found the
neighbours still breaking it. Connected to Apache Cassandra 5.0.9 in Chrome, Monitoring ->
Tables read
Tables 0 / "0 rows",Size "0 B" / "Total"andVacuum 0 / "OK"while thelist underneath correctly said "No tables found" - and the Overview tab of the same
session, same connection, read
Tables 6 / 2 indexes. The product answered one questiontwo ways, republished the sentinel #448 had just deleted, and issued a clean bill of health
for an operation Apache Cassandra does not have.
An audit of every tab found the same shape in five of six. The cause is never
engine-specific: a
reduceoverdata?.tables ?? [], or a?? 0on a field its own typemarks optional, turns a refusal into a number.
The rule applied throughout, unchanged from #448: ABSENCE and ZERO are different inputs. A
provider reporting nothing is saying the figure is not knowable; a provider reporting 0 has
measured one. Only the first changes, and every panel now pins the second with a test of
its own so the two cannot be collapsed again.
database being empty, exactly when the engine knows of tables and reports none for them,
which the required
overview.tableCountcan say andtablesalone cannot. The Vacuumcard is now a capability question rather than a data one:
supportsMaintenance: falsemeans the engine has no such operation, so the card states no verdict for it, and
lastVacuumno longer renders the word "Never" - a measurement on PostgreSQL, a claimedhistory elsewhere - for an engine that cannot vacuum.
bufferPoolUsageanddeadlocksare optional and genuinelyomitted: Trino's provider records in prose that it "holds no buffer pool" and "takes no
locks, so there are no deadlocks to count", Cassandra and Druid omit both, and SQLite
sets the first undefined outright. The panel was rating that absence "Poor" from a
fabricated 0, and badging the second "Healthy". It already did the right thing three
lines above for
cacheHitRatio; the two now agree, trend charts included, which stopsthe charts plotting a zero floor nobody measured.
{total: 0, idle: 0, active: 0, waiting: 0}with a "not available" message for enginesthat pool nothing, and the panel rendered "Max pool size 0" and "No queue" from it.
see, which is honest, and its
?? 0fallbacks read fields their types mark required.Nineteen tests added across the five panels, each pinning both inputs. Provider docs synced
where they described a panel's rendering: cassandra, druid, trino, elasticsearch, opensearch
and libredb.
Found by driving the product rather than by a gate - all six gates were green over the
frame that showed
Vacuum 0 - OK.Follow-up commit — the absence branching had put four of these files over Sonar's
cognitive complexity threshold. The quality gate passed regardless (0 blocking issues,
100% coverage on new code), so this is legibility, not a defect; but the four nested
ternaries it flagged sit at exactly the places a reader is most likely to misread, because
each encodes THREE states in a shape that looks like two.
vacuumIconClass/VacuumNote(TablesTab) anddeadlockIconClass(PerformanceTab) eachreplace
absent ? grey : positive ? warn : green. As early returns the third state isvisible: an engine that published no statistics at all is not an engine that looked and
found nothing, and must not borrow the green of one that did.
bloatBadgeVariantdoes thesame for the >20 / >10 thresholds.
formatBytes,formatNumberandformatVacuumDatemoved to module scope - pure, and previously re-created on every render.
MetricTrendCardand
metricSeriesreplace three near-identical chart cards and three near-identicalflatMapbuilders.PoolStatsGrid,PerformanceSummaryCardandQuickStatsCardmovepresentation to where it reads as presentation.
Nothing rendered changes, which is what the existing tests assert: 330 core files and all
30 component groups pass untouched, no test added or edited, and every extracted branch was
already exercised - the four files stay at 100% line coverage.
Two notes worth recording.
MetricTrendCardtakesheadingas a separate prop rather thanderiving it from
title, because the deadlock card is labelled "Deadlock Trend" while itschart is titled "Deadlocks" - deriving one from the other would have silently changed that
copy. And moving the Performance card out of
OverviewTableftperformance?.checkpointWriteTimeresolving to the DOM
performanceglobal rather than the prop; it failed typecheck onlybecause
Performancehas no such property, and a colliding name would have compiled.