From 822dff76667830a6a06e0a6aac0a21da646f0f3b Mon Sep 17 00:00:00 2001 From: Camilo Sierra Date: Fri, 25 Sep 2026 11:33:10 +0200 Subject: [PATCH 1/4] dashboard: collapse stack traces and extra instances in the alert panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An alert message is the rule's template with one row's values substituted, and those values carry whatever the server said. A ClickHouse exception embeds its stack trace in the message text: one measured in a real bundle ran 1725 characters over 15 lines, of which the first 216 were the error. The panel rendered every row in full with white-space:pre-wrap, so replication_queue_errors at its 50-row limit produced 750 lines of stack frames and pushed the rest of the page out of sight. Three things now collapse behind native
, each only when there is something to hide: - a message is shown up to the stack trace and capped at 260 characters, with the trailing "(version X (official build))" dropped β€” it is the same on every row and the header already states the version; the complete text sits under "full message and stack trace" - instances past the fifth move under "N more instances" - a rule description shows its first paragraph, the "what to check" half under "more about this rule" Nothing leaves the page: DATA.alerts still carries every row and every frame, and bundle-layout.md tells the skill to read it rather than the rendered page. A short single-line message gets no toggle at all. Worst case measured with the real exception: 750 lines become 6. make dashboard-preview now also writes bin/alerts_preview.html, a fixture covering every case β€” stack trace, over-cap instances, a message needing no disclosure, a rule whose query failed, a rule that was not applicable. Co-Authored-By: Claude Opus 5 (1M context) --- Makefile | 3 +- README.md | 6 +- internal/dashboard/alerts_panel_test.go | 76 ++++++++ internal/dashboard/alerts_preview_test.go | 79 ++++++++ internal/dashboard/generator.go | 170 +++++++++++++++--- internal/dashboard/keeper_preview_test.go | 26 +++ .../references/bundle-layout.md | 2 +- 7 files changed, 335 insertions(+), 27 deletions(-) create mode 100644 internal/dashboard/alerts_panel_test.go create mode 100644 internal/dashboard/alerts_preview_test.go diff --git a/Makefile b/Makefile index 03c9b33..c213296 100644 --- a/Makefile +++ b/Makefile @@ -133,4 +133,5 @@ help: .PHONY: dashboard-preview dashboard-preview: mkdir -p bin - DASHBOARD_PREVIEW_DIR=$(CURDIR)/bin go test ./internal/dashboard -run TestBuildHTML_KeeperIncidentPreview -count=1 >/dev/null && echo "bin/keeper_incident_preview.html" + DASHBOARD_PREVIEW_DIR=$(CURDIR)/bin go test ./internal/dashboard -run 'TestBuildHTML_(KeeperIncidentPreview|AlertsPreview)' -count=1 >/dev/null \ + && echo "bin/keeper_incident_preview.html" && echo "bin/alerts_preview.html" diff --git a/README.md b/README.md index af2ebba..17eba92 100644 --- a/README.md +++ b/README.md @@ -896,7 +896,7 @@ When `-skip-dashboard` is not set, the tool generates a single self-contained `d | # | Section | What it shows | |---|---|---| -| 1 | 🚨 **Alert Summary** | Fired alerts grouped by severity (critical / warning / info), with the row-level message template expanded per instance. Rules that **could not run** appear with a ⚠ marker and a separate "Could not run" chip β€” they are never counted in the severity badge. Rules that are **not applicable** here are listed in a muted footnote. A green "no issues" banner appears when nothing fired. | +| 1 | 🚨 **Alert Summary** | Fired alerts grouped by severity (critical / warning / info), with the row-level message template expanded per instance. Long content collapses: a message is shown up to the ClickHouse stack trace (a real one runs 1700+ characters over 15 lines, of which ~200 are the error) and capped at 260 characters, with the complete text one click away under *full message and stack trace*; only the first 5 instances are listed, the rest behind *N more instances*; and a rule description shows its first paragraph, the rest under *more about this rule*. Nothing is dropped from the page β€” `DATA.alerts` still carries every row in full. Rules that **could not run** appear with a ⚠ marker and a separate "Could not run" chip β€” they are never counted in the severity badge. Rules that are **not applicable** here are listed in a muted footnote. A green "no issues" banner appears when nothing fired. | | 2 | πŸ“ˆ **Overview** | Top-level counters: server version, uptime, total databases, total tables, active parts, total size | | 3 | πŸ“¦ **Storage** | Size by database (horizontal bar), table-engine distribution (doughnut), and a top-20-by-size table list | | 4 | πŸ“‹ **Tables Explorer** | Searchable / paginated table of every user table with engine, parts, rows, size, partition / sorting keys, and storage policy | @@ -921,11 +921,11 @@ A sticky top nav at the page header lets you jump straight to any section. Secti ### Previewing the Keeper Health panel without an outage -`make dashboard-preview` renders `bin/keeper_incident_preview.html` from an anonymised fixture shaped like a real Keeper outage on a shared-storage cluster: 48 hours of Keeper counters (a blip on day one, quorum lost for eight hours on day two), the `keeper_health`, `keeper_connection_blips`, `merges_stalled`, `background_operation_failures`, `high_exception_rate` and `too_many_parts` alerts as they would fire, the error codes per hour and a re-established Keeper session. Use it to see what the panel and the alerts look like before an incident, or to review a theme or wording change. +`make dashboard-preview` renders two pages from anonymised fixtures. `bin/alerts_preview.html` exercises every Alert Summary collapse case β€” a message carrying a stack trace, more instances than the inline cap, a short message that needs no disclosure, a rule whose query failed, a rule that was not applicable. `bin/keeper_incident_preview.html` is shaped like a real Keeper outage on a shared-storage cluster: 48 hours of Keeper counters (a blip on day one, quorum lost for eight hours on day two), the `keeper_health`, `keeper_connection_blips`, `merges_stalled`, `background_operation_failures`, `high_exception_rate` and `too_many_parts` alerts as they would fire, the error codes per hour and a re-established Keeper session. Use it to see what the panel and the alerts look like before an incident, or to review a theme or wording change. ### What's interactive vs static -- **Interactive**: Tables Explorer (full text search, database/engine filters, pagination); all charts (hover tooltips, legend toggling). +- **Interactive**: Tables Explorer (full text search, database/engine filters, pagination); all charts (hover tooltips, legend toggling); the Alert Summary disclosures (native `
`, so they work without JavaScript and survive `Ctrl-F` only when open). - **Static**: every other table β€” they render in a fixed order, but their underlying JSON is embedded in the page so you can `grep DATA dashboard.html | head` if you want raw values. ## Configuration Collection diff --git a/internal/dashboard/alerts_panel_test.go b/internal/dashboard/alerts_panel_test.go new file mode 100644 index 0000000..a264c82 --- /dev/null +++ b/internal/dashboard/alerts_panel_test.go @@ -0,0 +1,76 @@ +package dashboard + +import ( + "strings" + "testing" +) + +// An alert message is the rule's template with one row's values substituted, +// and those values routinely carry a full ClickHouse exception: one measured +// in a real bundle was 1764 characters over 15 lines, of which the first 216 +// were the error itself. replication_queue_errors returns up to 50 such rows +// and keeper_health up to 168, so rendering every message in full turned the +// panel into hundreds of lines of stack frames. These are the bounds that +// keep the panel readable; a refactor must not drop them. +func TestTemplate_AlertMessagesCollapseVerboseParts(t *testing.T) { + for _, want := range []string{ + "const ALERT_HEAD_CHARS=260;", // inline length of one instance + "const ALERT_ROWS_SHOWN=5;", // instances before the rest collapse + "const ALERT_STACK_RE=", // the stack trace is the cut point + `Stack trace \(when copying this message`, // ClickHouse's own marker, matched literally + "function alertMessageParts(msg){", // head / full split + "function alertDisclosure(parts){", // the
toggle + "function alertRowLine(a,row){", // one instance line + `'
`, // native disclosure, no JS needed + `'
'+esc(parts.full)+`, // full text is kept, and escaped
+		"more instance",                               // the extra-rows toggle
+		"more about this rule",                        // the description's second half
+	} {
+		if !strings.Contains(htmlTemplate, want) {
+			t.Errorf("alert panel lost %q", want)
+		}
+	}
+
+	// The head must never be emitted raw: every branch that prints customer
+	// text goes through esc().
+	for _, banned := range []string{
+		"+parts.head+", "+p.head+", "+ep.head+", "+msg+'", "+a.error+'",
+	} {
+		if strings.Contains(htmlTemplate, banned) {
+			t.Errorf("alert panel interpolates customer text unescaped: %q", banned)
+		}
+	}
+
+	// A disclosure that is always open helps nobody; the toggle must be
+	// closed by default (no `open` attribute on the element we emit).
+	if strings.Contains(htmlTemplate, `
const&, ...) @ 0x000000001a2b9f10\n" + + "3. ./src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp:1188: DB::ReplicatedMergeTreeQueue::processEntry(...) @ 0x000000001c0d4a22\n" + + "4. ./src/Storages/StorageReplicatedMergeTree.cpp:3702: DB::StorageReplicatedMergeTree::processQueueEntry(...) @ 0x000000001bf51188\n" + + "5. ./src/Storages/MergeTree/MergeTreeBackgroundExecutor.cpp:288: DB::MergeTreeBackgroundExecutor<...>::threadFunction() @ 0x000000001c113d90\n" + + "6. ./base/poco/Foundation/src/ThreadPool.cpp:205:14: Poco::PooledThread::run() @ 0x000000002334b505\n" + + "7. ./base/poco/Foundation/src/Thread_POSIX.cpp:335:5: Poco::ThreadImpl::runnableEntry(void*) @ 0x0000000023348f01\n" + + queue := []map[string]interface{}{} + for i, tbl := range []string{"events_local", "device_log", "sensor_raw", "job_history", "audit_trail", "shift_report", "line_state"} { + queue = append(queue, map[string]interface{}{ + "database": "demo_app", "table": tbl, "replica_name": "r-0" + string(rune('1'+i)), + "type": "GET_PART", "num_tries": 40 + i*17, "last_exception": trace, + }) + } + keeper := []map[string]interface{}{} + for h := 13; h <= 20; h++ { + keeper = append(keeper, map[string]interface{}{ + "hour": "2026-09-10 " + string(rune('0'+h/10)) + string(rune('0'+h%10)) + ":00:00", "hostname": "node-07", + "hw_exceptions": 10647545 - h*1000, "transactions": 11033023, "usual_transactions": 28010220, "pct_of_usual": 39 - h, + }) + } + + data := map[string]interface{}{ + "generated_at": "2026-09-25 10:00:00 UTC", "mode": "onprem", "version": "26.2.1.390", + "uptime": "1 hours 51 minutes", "total_databases": 410, "total_tables": 33882, "active_parts": 50000, "total_size": "28.40 TiB", + "alerts": []alert.Result{ + // 1. long message (stack trace) AND more instances than the cap + {Name: "replication_queue_errors", Title: "Replication queue entries have exceptions", Severity: "critical", File: "replication_queue_errors.yaml", + Description: "A replication queue entry has a non-empty last_exception: the replica tried to execute it and the server refused.\n\nCheck: the exception text and num_tries β€” a high count with the same error is a stuck entry, not a slow one. Then system.replicas for the same table, and the source replica named in the message.", + Message: "{database}.{table} (replica {replica_name}): {type} failed after {num_tries} tries β€” {last_exception}", + Rows: queue}, + // 2. short messages, many instances β†’ only the row cap applies + {Name: "keeper_health", Title: "Keeper unavailable: session loss storm while Keeper traffic collapsed", Severity: "critical", File: "keeper_health.yaml", + Description: "The two-signal Keeper health test over the last 7 days, per hour.\n\nCheck next: system.zookeeper_connection (session age), part_log errors in the same hours, then the Keeper nodes themselves.", + Message: "hour starting {hour} on {hostname}: {hw_exceptions} Keeper hardware exceptions, {transactions} Keeper transactions = {pct_of_usual}% of usual ({usual_transactions}/h median) β€” Keeper effectively unavailable to this server", + Rows: keeper}, + // 3. the control case: one short instance, single-paragraph description β†’ no toggles at all + {Name: "too_many_parts", Title: "Table partition approaching too-many-parts limit", Severity: "warning", File: "too_many_parts.yaml", + Description: "A table partition has more than 300 active parts.", + Message: "{database}.{table} partition '{partition_id}' has {parts_count} active parts β€” inserts are delayed from 1000 parts (parts_to_delay_insert) and rejected with code 252 TOO_MANY_PARTS at 3000 (parts_to_throw_insert)", + Rows: []map[string]interface{}{{"database": "demo_app", "table": "events_summary", "partition_id": "202609", "parts_count": 3469}}}, + // 4. a rule whose own query failed, with a trace in the error text + {Name: "detached_parts_exist", Title: "Detached parts present", Severity: "info", File: "detached_parts_exist.yaml", + Description: "Parts exist in the detached/ folder.", + Error: "error executing query: non-OK status: 500, body: " + trace}, + // 5. not applicable here + {Name: "crash_log_entries", Title: "Server crash detected", Severity: "critical", File: "crash_log_entries.yaml", Skipped: true, Reason: "table not present"}, + }, + } + if err := os.WriteFile(filepath.Join(dir, "alerts_preview.html"), []byte(buildHTML(data)), 0o644); err != nil { + t.Fatal(err) + } + t.Logf("written: %s", filepath.Join(dir, "alerts_preview.html")) +} diff --git a/internal/dashboard/generator.go b/internal/dashboard/generator.go index d56d857..92c9407 100644 --- a/internal/dashboard/generator.go +++ b/internal/dashboard/generator.go @@ -1529,7 +1529,10 @@ header .meta{margin-left:auto;text-align:right;font-size:var(--click-font-size-1 #theme-toggle:hover{background:rgba(255,255,255,.12)} nav{background:var(--surface-card);border-bottom:var(--click-border-width-1) solid var(--stroke);padding:0 var(--click-space-6);display:flex;overflow-x:auto} nav a{padding:var(--click-space-3) var(--click-space-4);color:var(--ink-muted);text-decoration:none;font-size:var(--click-font-size-1);font-weight:var(--click-font-weight-2);white-space:nowrap;border-bottom:2px solid transparent;display:block} -nav a:hover,nav a.active{color:var(--ink);border-bottom-color:var(--ink)} +nav a:hover{color:var(--ink);border-bottom-color:var(--stroke)} +/* The section you are in: accent underline + weight, so it reads as state + rather than as the link the pointer happens to be over. */ +nav a.active{color:var(--ink);border-bottom-color:var(--status-info);font-weight:var(--click-font-weight-3)} .badge{display:inline-block;padding:2px var(--click-space-2);border-radius:var(--click-radii-full);font-size:var(--click-font-size-0);font-weight:var(--click-font-weight-3);text-transform:uppercase;letter-spacing:.5px;margin-top:2px} .badge-cloud{background:var(--status-info);color:#fff} .badge-onprem{background:var(--status-good);color:#fff} @@ -1603,6 +1606,19 @@ footer{text-align:center;color:var(--ink-muted);font-size:var(--click-font-size- .alert-messages{padding-left:18px;margin:0} .alert-messages li{font-size:var(--click-font-size-1);color:var(--ink);margin:3px 0;font-family:var(--click-font-mono);word-break:break-word;white-space:pre-wrap} .alert-err-msg{font-size:var(--click-font-size-1);color:var(--ink-muted);margin-top:var(--click-space-1);font-style:italic} +/* Disclosures inside an alert. A ClickHouse exception carries its stack trace + inline β€” a measured one was 1764 chars over 15 lines, of which 216 were the + message β€” and a rule may return dozens of rows, so the verbose part sits + behind a
and the alert list stays scannable. */ +.alert-more{margin:2px 0 0} +.alert-more>summary{cursor:pointer;color:var(--ink-muted);font-size:var(--click-font-size-0);font-family:var(--click-font-regular);font-style:normal;list-style:none;display:inline-flex;align-items:center;gap:4px;user-select:none} +.alert-more>summary::-webkit-details-marker{display:none} +.alert-more>summary::before{content:'\25B8';display:inline-block;transition:transform .12s ease} +.alert-more[open]>summary::before{transform:rotate(90deg)} +.alert-more>summary:hover{color:var(--ink);text-decoration:underline} +.alert-more>summary:focus-visible{outline:2px solid var(--status-info);outline-offset:2px;border-radius:var(--click-radii-1)} +.alert-full{background:var(--surface-sunken);border:var(--click-border-width-1) solid var(--stroke);border-radius:var(--click-radii-1);padding:var(--click-space-2);margin:var(--click-space-1) 0 var(--click-space-2);font-family:var(--click-font-mono);font-size:var(--click-font-size-0);line-height:var(--click-line-height-1);color:var(--ink-muted);white-space:pre-wrap;word-break:break-word;max-height:360px;overflow:auto} +.alert-desc-rest{margin-top:var(--click-space-1)} .alert-tags{display:flex;gap:var(--click-space-1);flex-wrap:wrap;margin-top:var(--click-space-2)} .alert-tag{background:var(--surface-sunken);border:var(--click-border-width-1) solid var(--stroke);color:var(--ink-muted);border-radius:var(--click-radii-full);padding:1px var(--click-space-2);font-size:var(--click-font-size-0)} .alert-summary-bar{display:flex;gap:var(--click-space-2);flex-wrap:wrap;margin-bottom:var(--click-space-3)} @@ -2412,6 +2428,61 @@ function dictStatusBadge(status){ })(); // ── alerts renderer ─────────────────────────────────────────────────────────── +// +// How much of one alert is shown before the reader has to ask for more. A +// ClickHouse exception embeds its stack trace in the message text: one +// measured in a real bundle ran 1764 characters over 15 lines, of which the +// first 216 were the actual error. replication_queue_errors returns up to 50 +// such rows and keeper_health up to 168, so rendering every message in full +// buried the rest of the page under stack frames. +const ALERT_HEAD_CHARS=260; // inline length of one instance line +const ALERT_ROWS_SHOWN=5; // instances listed before the rest collapse + +const ALERT_STACK_RE=/\s*Stack trace \(when copying this message[\s\S]*$/; +// Every ClickHouse exception ends with the build it came from. It is the same +// string on every row and the dashboard header already states the version, so +// it is dropped from the inline line and kept in the full text. +const ALERT_VERSION_RE=/\s*\(version [0-9][^)]*\([^)]*\)\)[\s,.;]*$/; + +// alertMessageParts splits a substituted message into the line shown inline +// and the complete text kept for the disclosure. Returns truncated=false when +// nothing was actually dropped, so a short message gets no useless toggle. +function alertMessageParts(msg){ + const full=String(msg==null?'':msg); + const flat=full.replace(/\s+/g,' ').trim(); + let head=full.replace(ALERT_STACK_RE,'').replace(/\s+/g,' ').trim() + .replace(ALERT_VERSION_RE,'').replace(/[\s,;]+$/,''); + if(head.length>ALERT_HEAD_CHARS){ + const cut=head.slice(0,ALERT_HEAD_CHARS); + const sp=cut.lastIndexOf(' '); + head=(sp>ALERT_HEAD_CHARS*0.6?cut.slice(0,sp):cut)+'\u2026'; + } + if(!head) head=flat; + // truncated drives the toggle: true only when the reader would otherwise + // lose content, so a short single-line message gets no useless disclosure. + const flatTrimmed=flat.replace(ALERT_VERSION_RE,'').replace(/[\s,;]+$/,''); + return {head:head, full:full, truncated:head!==flatTrimmed||flat!==flatTrimmed, hasStack:ALERT_STACK_RE.test(full)}; +} + +// alertDisclosure renders the "show the rest" toggle for one message. +function alertDisclosure(parts){ + if(!parts.truncated) return ''; + const label=parts.hasStack?'full message and stack trace':'full message'; + return '
'+label+'' + +'
'+esc(parts.full)+'
'; +} + +// alertRowLine renders one instance: the rule's message template with this +// row's values substituted. The template is ours, the values are customer +// data (table names, partition ids, raw server text), so the whole line is +// escaped. +function alertRowLine(a,row){ + let msg=a.message; + Object.entries(row).forEach(([k,v])=>{msg=msg.split('{'+k+'}').join(String(v==null?'':v));}); + const parts=alertMessageParts(msg); + return '
  • \u25B8 '+esc(parts.head)+alertDisclosure(parts)+'
  • '; +} + function renderAlerts(){ const alerts=DATA.alerts||[]; const fired=alerts.filter(a=>(a.rows&&a.rows.length>0)||a.error); @@ -2482,22 +2553,37 @@ function renderAlerts(){ if((a.tags||[]).length) html+=''+a.tags.map(t=>''+esc(t)+'').join('')+''; html+=''; // header - if(a.description) html+='
    '+esc(a.description.trim()).replace(/\n/g,'
    ')+'
    '; + if(a.description){ + // The rule descriptions are deliberately long β€” an explanation followed + // by what to check next. The first paragraph is the explanation; the + // rest is guidance the reader wants only once the alert is worth + // following, so it collapses. + const d=a.description.trim(); + const brk=d.indexOf('\n\n'); + const first=(brk>0?d.slice(0,brk):d).trim(); + html+='
    '+esc(first).replace(/\n/g,'
    '); + if(brk>0){ + html+='
    more about this rule' + +'
    '+esc(d.slice(brk).trim()).replace(/\n/g,'
    ')+'
    '; + } + html+='
    '; + } if(a.error){ - // a.error is raw server exception text β€” customer-influenced. - html+='
    ⚠ '+esc(a.error)+'
    '; + // a.error is raw server exception text β€” customer-influenced, and it + // carries a stack trace as often as a row message does. + const ep=alertMessageParts(a.error); + html+='
    ⚠ '+esc(ep.head)+alertDisclosure(ep)+'
    '; } else if(a.message&&(a.rows||[]).length){ - html+='
      '; - (a.rows||[]).forEach(row=>{ - let msg=a.message; - // Row values are customer data (table names, partition ids, raw - // messages) substituted into the rule's message template β€” the - // template is ours, the values are not. Escape the whole line. - Object.entries(row).forEach(([k,v])=>{msg=msg.split('{'+k+'}').join(String(v??''));}); - html+='
    • β–Έ '+esc(msg)+'
    • '; - }); - html+='
    '; + // keeper_health can return one row per hour of a 7-day window; only the + // first few are listed, the rest stay one click away. + const rows=a.rows||[]; + const shown=rows.slice(0,ALERT_ROWS_SHOWN), hidden=rows.slice(ALERT_ROWS_SHOWN); + html+='
      '+shown.map(r=>alertRowLine(a,r)).join('')+'
    '; + if(hidden.length){ + html+='
    '+hidden.length+' more instance'+(hidden.length===1?'':'s')+'' + +'
      '+hidden.map(r=>alertRowLine(a,r)).join('')+'
    '; + } } html+=''; // item @@ -2807,19 +2893,59 @@ document.addEventListener('DOMContentLoaded',function(){ window.addEventListener('resize', measureTopbar, {passive:true}); } - // nav active highlight on scroll - const secs=[...document.querySelectorAll('section[id]')]; + // nav active highlight on scroll β€” "you are here" in the sticky band. + // + // Only RENDERED sections may win. Every optional panel starts at + // display:none, and a non-rendered element reports + // getBoundingClientRect().top = 0, which passes the "its top is above the + // line" test on every scroll. The last such section in the document + // therefore won every pass β€” on a typical bundle that is sec-async-inserts, + // whose nav link is hidden too, so the band showed no highlight at all. + // getClientRects() is empty for anything not rendered, and the list is + // rebuilt each pass because panels un-hide after their data renders. const navLinks=[...document.querySelectorAll('nav a')]; - window.addEventListener('scroll',function(){ - let cur=''; + const navFor={}; + navLinks.forEach(a=>{navFor[a.getAttribute('href')]=a;}); + function spySections(){ + return [...document.querySelectorAll('section[id]')] + .filter(s=>s.getClientRects().length && navFor['#'+s.id]); + } + function syncNav(){ + const secs=spySections(); + if(!secs.length) return; // A section counts as current once its top reaches the underside of the - // band, so the highlight matches what the reader can actually see. - const line=topbarH+8; + // band, so the highlight matches what the reader can actually see. The + // tolerance clears the subpixel gap an anchor jump leaves, which lands a + // heading at exactly scroll-margin-top. + const line=topbarH+16; + // Default to the first section: at the very top of the page nothing has + // crossed the line yet, and a blank band is what this replaced. + let cur=secs[0].id; secs.forEach(s=>{if(s.getBoundingClientRect().top<=line)cur=s.id;}); + // At the end of the page a short final section can never reach the line, + // so the last one wins once the scroll cannot go further. + if(Math.ceil(window.innerHeight+window.scrollY)>=document.documentElement.scrollHeight-2){ + cur=secs[secs.length-1].id; + } navLinks.forEach(a=>{ - a.classList.toggle('active',a.getAttribute('href')==='#'+cur); + const on=a.getAttribute('href')==='#'+cur; + a.classList.toggle('active',on); + if(on) a.setAttribute('aria-current','true'); else a.removeAttribute('aria-current'); }); - },{passive:true}); + } + // Called straight from the listener rather than coalesced through + // requestAnimationFrame. The pass is ~20 getBoundingClientRect reads with + // no writes, the browser already caps scroll events at the frame rate, and + // rAF would make the highlight depend on a repaint β€” which never happens + // under a headless --virtual-time-budget, so the behaviour could not be + // tested. Reading live also keeps it correct when a disclosure expands and + // shifts every section below it; a cached offset table would not. + window.addEventListener('scroll',syncNav,{passive:true}); + window.addEventListener('resize',syncNav,{passive:true}); + // Once now for the initial highlight, once after load β€” optional panels + // un-hide as their data renders, which changes which sections exist. + syncNav(); + window.addEventListener('load',syncNav); // header document.getElementById('hdr-badge').innerHTML= diff --git a/internal/dashboard/keeper_preview_test.go b/internal/dashboard/keeper_preview_test.go index 737fab1..9a25e6d 100644 --- a/internal/dashboard/keeper_preview_test.go +++ b/internal/dashboard/keeper_preview_test.go @@ -84,7 +84,33 @@ func keeperIncidentFixture() map[string]interface{} { "pct_of_usual": x.tx * 100 / 28010220, }) } + // A real ClickHouse exception, anonymised: the frames are public source + // paths, the table and replica are invented. This is what made the alert + // panel unreadable before the messages collapsed β€” 1764 characters over + // 15 lines per row, of which the first 216 are the error. + trace := "Code: 999. Coordination::Exception: Session expired. (KEEPER_EXCEPTION) (version 26.2.1.390 (official build)), " + + "Stack trace (when copying this message, always include the lines below):\n\n" + + "0. ./ci/tmp/build/./src/Common/Exception.cpp:141:1: DB::Exception::Exception(DB::Exception::MessageMasked&&, int, bool) @ 0x000000001373469f\n" + + "1. ./src/Common/ZooKeeper/ZooKeeperImpl.cpp:1071: Coordination::ZooKeeper::pushRequest(Coordination::ZooKeeper::RequestInfo&&) @ 0x000000001a2b3c4d\n" + + "2. ./src/Common/ZooKeeper/ZooKeeper.cpp:412: zkutil::ZooKeeper::multiImpl(...) @ 0x000000001a2b9f10\n" + + "3. ./src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp:1188: DB::ReplicatedMergeTreeQueue::processEntry(...) @ 0x000000001c0d4a22\n" + + "4. ./src/Storages/StorageReplicatedMergeTree.cpp:3702: DB::StorageReplicatedMergeTree::processQueueEntry(...) @ 0x000000001bf51188\n" + + "5. ./src/Storages/MergeTree/MergeTreeBackgroundExecutor.cpp:288: DB::MergeTreeBackgroundExecutor<...>::threadFunction() @ 0x000000001c113d90\n" + + "6. ./base/poco/Foundation/src/ThreadPool.cpp:205:14: Poco::PooledThread::run() @ 0x000000002334b505\n" + + "7. ./base/poco/Foundation/src/Thread_POSIX.cpp:335:5: Poco::ThreadImpl::runnableEntry(void*) @ 0x0000000023348f01\n" + queueRows := []map[string]interface{}{} + for i, tbl := range []string{"events_local", "device_log", "sensor_raw", "job_history", "audit_trail", "shift_report", "line_state"} { + queueRows = append(queueRows, map[string]interface{}{ + "database": "demo_app", "table": tbl, "replica_name": "r-0" + string(rune('1'+i)), + "type": "GET_PART", "create_time": "2026-09-10 15:0" + string(rune('1'+i)) + ":22", + "num_tries": 40 + i*17, "last_exception": trace, + }) + } + alerts := []alert.Result{ + {Name: "replication_queue_errors", Title: "Replication queue entries have exceptions", Severity: "critical", File: "replication_queue_errors.yaml", FiredAt: "2026-09-10T22:58:57Z", + Message: "{database}.{table} (replica {replica_name}): {type} failed after {num_tries} tries β€” {last_exception}", + Rows: queueRows}, {Name: "keeper_health", Title: "Keeper unavailable: session loss storm while Keeper traffic collapsed", Severity: "critical", File: "keeper_health.yaml", FiredAt: "2026-09-10T22:58:57Z", Message: "hour starting {hour}: {hw_exceptions} Keeper hardware exceptions, {transactions} Keeper transactions = {pct_of_usual}% of usual ({usual_transactions}/h median) β€” Keeper effectively unavailable to this server", Rows: keeperHealthRows}, diff --git a/skills/clickhouse-diagnostic/references/bundle-layout.md b/skills/clickhouse-diagnostic/references/bundle-layout.md index 0275bcc..5dab1c2 100644 --- a/skills/clickhouse-diagnostic/references/bundle-layout.md +++ b/skills/clickhouse-diagnostic/references/bundle-layout.md @@ -150,7 +150,7 @@ Mirror of the config directory (`config.d/…`, `users.d/…`, sometimes `config ## 8. `dashboard.html` -Self-contained; all panel data is embedded as one JSON literal on a line starting `const DATA = {`. Keys: `alerts` (full rule results **including matched rows**), `keeper_metric_hourly` (per hour: `transactions, hw_exceptions, user_exceptions, wait_us, sessions_min, sessions_max` β€” the Keeper health test inputs, 7 days; **check `keeper_wait_available` / `keeper_session_available` first** β€” when either is `false` the server's version does not export that counter and the generator substituted `0`, which is indistinguishable from a real zero, so never read `sessions_min = 0` as a lost session or `wait_us = 0` as zero latency without them), `keeper_errors_hourly` (`time, code_name, count` for 999/242/319/571/252; `keeper_errors_source` says whether it came from `error_log` or `query_log`), `keeper_connection` (live `zookeeper_connection` rows), `version, uptime, total_databases, total_tables, active_parts, total_size`, `host_info`, `host_checks`, `storage_by_db`, `engines_dist`, `tables_list`, `query_by_time`, `query_by_kind`, `query_slow`, `query_heavy`, `query_by_user`, `exceptions`, `part_log_by_time`, `part_log_by_type`, `dictionaries`, `crash_log`, `mutations`, `detached`, `replication_queue`, `clusters`, `replicas`, `disks`, `text_log`, `bundle_files`, `server_errors`, `high_part_count`, `ttl_activity`, `async_inserts`, and `qa_*` when query analysis ran. Extraction recipe in `reading-recipes.md`. +Self-contained; all panel data is embedded as one JSON literal on a line starting `const DATA = {`. The Alert Summary panel collapses long content (stack traces, instances past the fifth, the tail of a rule description) behind `
    ` toggles β€” read `DATA.alerts` rather than the rendered page when you want every instance and the full exception text. Keys: `alerts` (full rule results **including matched rows**), `keeper_metric_hourly` (per hour: `transactions, hw_exceptions, user_exceptions, wait_us, sessions_min, sessions_max` β€” the Keeper health test inputs, 7 days; **check `keeper_wait_available` / `keeper_session_available` first** β€” when either is `false` the server's version does not export that counter and the generator substituted `0`, which is indistinguishable from a real zero, so never read `sessions_min = 0` as a lost session or `wait_us = 0` as zero latency without them), `keeper_errors_hourly` (`time, code_name, count` for 999/242/319/571/252; `keeper_errors_source` says whether it came from `error_log` or `query_log`), `keeper_connection` (live `zookeeper_connection` rows), `version, uptime, total_databases, total_tables, active_parts, total_size`, `host_info`, `host_checks`, `storage_by_db`, `engines_dist`, `tables_list`, `query_by_time`, `query_by_kind`, `query_slow`, `query_heavy`, `query_by_user`, `exceptions`, `part_log_by_time`, `part_log_by_type`, `dictionaries`, `crash_log`, `mutations`, `detached`, `replication_queue`, `clusters`, `replicas`, `disks`, `text_log`, `bundle_files`, `server_errors`, `high_part_count`, `ttl_activity`, `async_inserts`, and `qa_*` when query analysis ran. Extraction recipe in `reading-recipes.md`. ## 8a. `execution_log.txt` From 8d99e640db1c2317ec767465d94ae834187fcb78 Mon Sep 17 00:00:00 2001 From: Camilo Sierra Date: Fri, 25 Sep 2026 11:33:10 +0200 Subject: [PATCH 2/4] dashboard: fix the nav scroll spy, which never highlighted anything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sticky nav was supposed to mark the section you are reading. It never did, on any bundle. Every optional panel starts at display:none, and a non-rendered element reports getBoundingClientRect().top = 0. Zero always passes the "its top is above the line" test, so the last such section in the document won every pass β€” on a typical bundle sec-async-inserts, whose nav link is hidden too. The class landed on an invisible link and the band looked dead. Confirmed in a browser before the fix: after scrolling to Storage the active link was #sec-async-inserts. The section list is now rebuilt on each pass and filtered to those actually rendered that also have a nav link. It syncs on load and resize, not only on scroll, because panels un-hide after their data renders β€” the same lateness that makes a snapshot wrong. The first section is current at the top of the page and the last at the bottom, where a short final section can never reach the threshold, and the active link carries aria-current. Hover and active shared one CSS rule, so the current section was indistinguishable from whatever the pointer was over; active now gets an accent underline and heavier weight. The rAF coalescing this first used is gone: requestAnimationFrame never fires under a headless --virtual-time-budget, which made the behaviour untestable, and the pass is ~20 rect reads with no writes against scroll events the browser already caps at the frame rate. Reading live also stays correct when a disclosure expands and shifts everything below it. Verified in a browser across all 15 rendered sections of the preview page and 15 scroll positions of a live dashboard; every one highlights its own link. theme_test's threshold assertion now checks that the line derives from the measured band height rather than pinning the tolerance value, which widened from 8 to 16 px so a subpixel rounding after an anchor jump cannot flip the comparison. Co-Authored-By: Claude Opus 5 (1M context) --- internal/dashboard/nav_spy_test.go | 56 ++++++++++++++++++++++++++++++ internal/dashboard/theme_test.go | 6 +++- 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 internal/dashboard/nav_spy_test.go diff --git a/internal/dashboard/nav_spy_test.go b/internal/dashboard/nav_spy_test.go new file mode 100644 index 0000000..097b639 --- /dev/null +++ b/internal/dashboard/nav_spy_test.go @@ -0,0 +1,56 @@ +package dashboard + +import ( + "strings" + "testing" +) + +// The sticky nav highlights the section you are reading. It silently stopped +// doing that: every optional panel starts at display:none, a non-rendered +// element reports getBoundingClientRect().top = 0, and 0 is always above the +// threshold β€” so the LAST section in the document won every pass. On a +// typical bundle that is sec-async-inserts, whose nav link is hidden too, so +// nothing appeared highlighted at any scroll position. These are the parts +// of the fix that must not regress. +func TestTemplate_NavScrollSpy(t *testing.T) { + for _, want := range []string{ + "function spySections(){", + "s.getClientRects().length", // only rendered sections may win… + "navFor['#'+s.id]", // …and only those with a nav link + "function syncNav(){", + "const line=topbarH+16;", // threshold follows the measured band + "let cur=secs[0].id;", // never blank at the top of the page + "secs[secs.length-1].id", // last section wins at the bottom + `window.addEventListener('scroll',syncNav,{passive:true});`, + `window.addEventListener('resize',syncNav,{passive:true});`, + `window.addEventListener('load',syncNav);`, // panels un-hide after render + `a.setAttribute('aria-current','true')`, // state, not just colour + } { + if !strings.Contains(htmlTemplate, want) { + t.Errorf("nav scroll spy lost %q", want) + } + } + + // The list must be rebuilt per pass: a panel that un-hides later, or a + // disclosure that expands and shifts everything below it, changes the + // answer. A snapshot taken once at init is the bug in a new shape. + if strings.Contains(htmlTemplate, "const secs=[...document.querySelectorAll('section[id]')];") { + t.Error("sections are snapshotted once again β€” rebuild them inside syncNav") + } + + // requestAnimationFrame never fires under a headless --virtual-time-budget, + // which made this behaviour impossible to test. Keep the read direct. + spy := htmlTemplate[strings.Index(htmlTemplate, "function spySections(){"):] + spy = spy[:strings.Index(spy, "// header")] + if strings.Contains(spy, "requestAnimationFrame(") { // the call, not the comment explaining its absence + t.Error("the scroll spy must not depend on a repaint to update") + } + + // Current section and hovered link must not look identical. + if !strings.Contains(htmlTemplate, "nav a.active{color:var(--ink);border-bottom-color:var(--status-info)") { + t.Error("the active nav link needs its own accent, distinct from :hover") + } + if strings.Contains(htmlTemplate, "nav a:hover,nav a.active{") { + t.Error("hover and active share one rule again β€” the current section stops being legible") + } +} diff --git a/internal/dashboard/theme_test.go b/internal/dashboard/theme_test.go index 4520782..dfbf2ba 100644 --- a/internal/dashboard/theme_test.go +++ b/internal/dashboard/theme_test.go @@ -184,7 +184,11 @@ func TestTemplate_TopbarSticksAsOneBand(t *testing.T) { if !strings.Contains(htmlTemplate, "new ResizeObserver(measureTopbar)") { t.Error("--topbar-h must be re-measured via ResizeObserver, not set once") } - if !strings.Contains(htmlTemplate, "const line=topbarH+8;") { + // The threshold is the measured height plus a small tolerance: an anchor + // jump lands a heading at exactly scroll-margin-top, and a subpixel + // rounding there must not flip the comparison. The slack value is free to + // change; deriving it from topbarH is not. + if !strings.Contains(htmlTemplate, "const line=topbarH+") { t.Error("the scroll-spy threshold must follow the measured band height") } } From 5e700b0572b677bad526162475bf59cf173687a1 Mon Sep 17 00:00:00 2001 From: Camilo Sierra Date: Fri, 25 Sep 2026 17:33:28 +0200 Subject: [PATCH 3/4] dashboard: fix two off-by-ones in the alert message truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found by review on #32. The 260-character cap was exclusive of the ellipsis. The word-boundary branch stayed inside it, but the no-space fallback β€” reachable with a long path or any 260-character run without a space after position 156 β€” emitted slice(0,260) plus the ellipsis, so 261. Slicing one short makes the documented cap inclusive. The disclosure toggle compared the inline head against a version-stripped copy of the message. For a message that is NOTHING BUT a version suffix, stripping empties head, the `if(!head) head=flat` fallback restores it, and the comparison against the stripped (empty) copy reported a difference β€” so the panel rendered a
    whose body repeated the line above it. Comparing head against the flat message instead states the actual question, "is the inline line already the whole message", and drops flatTrimmed entirely. Unreachable with the shipped rules, since every template in alerts/ wraps its substitutions in literal prose and the version regex is $-anchored, so the prose always survives. Fixed anyway: the predicate is simpler than the one it replaces, and the next rule that interpolates a bare server string would hit it. Verified by extracting the function out of the template and running it over eight message shapes β€” version-only, short with a version suffix, short plain, long with and without spaces, a stack trace, empty and null. Before: the two failures above. After: none, with every other shape unchanged. alerts_panel_test.go now pins both forms and bans the pre-fix ones. Co-Authored-By: Claude Opus 5 (1M context) --- internal/dashboard/alerts_panel_test.go | 12 ++++++++++++ internal/dashboard/generator.go | 16 +++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/internal/dashboard/alerts_panel_test.go b/internal/dashboard/alerts_panel_test.go index a264c82..44c8cf2 100644 --- a/internal/dashboard/alerts_panel_test.go +++ b/internal/dashboard/alerts_panel_test.go @@ -25,6 +25,16 @@ func TestTemplate_AlertMessagesCollapseVerboseParts(t *testing.T) { `'
    '+esc(parts.full)+`, // full text is kept, and escaped
     		"more instance",                               // the extra-rows toggle
     		"more about this rule",                        // the description's second half
    +
    +		// Two off-by-one traps found in review. The cap is INCLUSIVE of the
    +		// ellipsis, so the slice has to be one short β€” otherwise the no-space
    +		// fallback emits 261 characters. And the toggle compares the inline
    +		// head against the whole flat message, not a version-stripped copy:
    +		// with the stripped copy, a message that is NOTHING BUT a version
    +		// suffix strips to empty, the fallback restores it, and the mismatch
    +		// renders a disclosure whose body repeats the line above it.
    +		"head.slice(0,ALERT_HEAD_CHARS-1)",
    +		"truncated:head!==flat,",
     	} {
     		if !strings.Contains(htmlTemplate, want) {
     			t.Errorf("alert panel lost %q", want)
    @@ -35,6 +45,8 @@ func TestTemplate_AlertMessagesCollapseVerboseParts(t *testing.T) {
     	// text goes through esc().
     	for _, banned := range []string{
     		"+parts.head+", "+p.head+", "+ep.head+", "+msg+'", "+a.error+'",
    +		// The pre-fix forms of the two traps above.
    +		"head.slice(0,ALERT_HEAD_CHARS)+", "truncated:head!==flatTrimmed",
     	} {
     		if strings.Contains(htmlTemplate, banned) {
     			t.Errorf("alert panel interpolates customer text unescaped: %q", banned)
    diff --git a/internal/dashboard/generator.go b/internal/dashboard/generator.go
    index 92c9407..4c17347 100644
    --- a/internal/dashboard/generator.go
    +++ b/internal/dashboard/generator.go
    @@ -2453,15 +2453,21 @@ function alertMessageParts(msg){
       let head=full.replace(ALERT_STACK_RE,'').replace(/\s+/g,' ').trim()
                    .replace(ALERT_VERSION_RE,'').replace(/[\s,;]+$/,'');
       if(head.length>ALERT_HEAD_CHARS){
    -    const cut=head.slice(0,ALERT_HEAD_CHARS);
    +    // ALERT_HEAD_CHARS is inclusive of the ellipsis: slice one short so the
    +    // no-space fallback below yields 260 characters, not 261.
    +    const cut=head.slice(0,ALERT_HEAD_CHARS-1);
         const sp=cut.lastIndexOf(' ');
         head=(sp>ALERT_HEAD_CHARS*0.6?cut.slice(0,sp):cut)+'\u2026';
       }
       if(!head) head=flat;
    -  // truncated drives the toggle: true only when the reader would otherwise
    -  // lose content, so a short single-line message gets no useless disclosure.
    -  const flatTrimmed=flat.replace(ALERT_VERSION_RE,'').replace(/[\s,;]+$/,'');
    -  return {head:head, full:full, truncated:head!==flatTrimmed||flat!==flatTrimmed, hasStack:ALERT_STACK_RE.test(full)};
    +  // truncated drives the toggle: offer it only when the inline line is not
    +  // already the whole message, so a short single-line message gets no useless
    +  // disclosure. Comparing against the flat message rather than a
    +  // version-stripped copy also covers the degenerate case where the message
    +  // is NOTHING BUT a version suffix: stripping empties head, the fallback
    +  // above restores it, and head===flat then correctly reports that there is
    +  // nothing to reveal.
    +  return {head:head, full:full, truncated:head!==flat, hasStack:ALERT_STACK_RE.test(full)};
     }
     
     // alertDisclosure renders the "show the rest" toggle for one message.
    
    From 117537cdbffb0749cab9026d86417299caa247a7 Mon Sep 17 00:00:00 2001
    From: Camilo Sierra 
    Date: Mon, 28 Sep 2026 16:05:56 +0200
    Subject: [PATCH 4/4] dashboard: re-sync the nav highlight when the page's
     content height changes
    MIME-Version: 1.0
    Content-Type: text/plain; charset=UTF-8
    Content-Transfer-Encoding: 8bit
    
    Review finding on #32, and my own comment overclaimed: reading rects live
    keeps the scroll-spy's MATH right when a disclosure expands, but nothing
    triggered the pass β€” syncNav ran on scroll, resize and load only, and
    opening a 
    is none of those. Every section below the disclosure shifts while the highlight names the pre-expansion section. Why it was hard to see: Chrome and Firefox scroll-anchor. When content above the viewport grows, they adjust scrollY to keep the view stable, and that adjustment fires a scroll event, which ran syncNav. Safari has no scroll anchoring, and the same holds anywhere with overflow-anchor: none. Driven in headless Chromium with anchoring disabled: opening the twelve disclosures in the Alerts section grew it by 1687 px and left Overview highlighted 1667 px below the line. With this change the highlight moves to Alerts. The fix observes the page's own height β€” a ResizeObserver on
    re-runs the pass for any content change that moves a section: a disclosure, a table filter, a panel un-hiding after its data renders β€” plus a capturing toggle listener at the document as the explicit hook and the fallback where ResizeObserver is missing (toggle does not bubble; capture still sees it). The comment now says what is true, and nav_spy_test pins both hooks. Co-Authored-By: Claude Fable 5.1 --- internal/dashboard/generator.go | 17 +++++++++++++++-- internal/dashboard/nav_spy_test.go | 6 +++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/internal/dashboard/generator.go b/internal/dashboard/generator.go index 4c17347..5d5af6d 100644 --- a/internal/dashboard/generator.go +++ b/internal/dashboard/generator.go @@ -2944,10 +2944,23 @@ document.addEventListener('DOMContentLoaded',function(){ // no writes, the browser already caps scroll events at the frame rate, and // rAF would make the highlight depend on a repaint β€” which never happens // under a headless --virtual-time-budget, so the behaviour could not be - // tested. Reading live also keeps it correct when a disclosure expands and - // shifts every section below it; a cached offset table would not. + // tested. Reading live keeps the MATH right when a disclosure expands and + // shifts every section below it (a cached offset table would not) β€” but + // the pass still has to be triggered, and opening a
    is neither + // a scroll nor a resize. Left alone, the highlight named the pre-expansion + // section until the reader scrolled. So the page's own height is observed: + // any content change that moves a section β€” a disclosure, a table filter, + // a panel un-hiding after its data renders β€” re-runs the pass. window.addEventListener('scroll',syncNav,{passive:true}); window.addEventListener('resize',syncNav,{passive:true}); + const mainEl=document.querySelector('main'); + if(window.ResizeObserver && mainEl){ + new ResizeObserver(syncNav).observe(mainEl); + } + // toggle does not bubble, but a capturing listener at the document still + // sees it β€” the explicit hook for the disclosure case, and the fallback + // where ResizeObserver is missing. + document.addEventListener('toggle',syncNav,true); // Once now for the initial highlight, once after load β€” optional panels // un-hide as their data renders, which changes which sections exist. syncNav(); diff --git a/internal/dashboard/nav_spy_test.go b/internal/dashboard/nav_spy_test.go index 097b639..ba64de8 100644 --- a/internal/dashboard/nav_spy_test.go +++ b/internal/dashboard/nav_spy_test.go @@ -24,7 +24,11 @@ func TestTemplate_NavScrollSpy(t *testing.T) { `window.addEventListener('scroll',syncNav,{passive:true});`, `window.addEventListener('resize',syncNav,{passive:true});`, `window.addEventListener('load',syncNav);`, // panels un-hide after render - `a.setAttribute('aria-current','true')`, // state, not just colour + // Opening a
    moves every section below it without a scroll or + // a resize; the pass must be re-triggered by the content height itself. + `new ResizeObserver(syncNav).observe(mainEl);`, + `document.addEventListener('toggle',syncNav,true);`, + `a.setAttribute('aria-current','true')`, // state, not just colour } { if !strings.Contains(htmlTemplate, want) { t.Errorf("nav scroll spy lost %q", want)