Skip to content

chore(selfhost): validate Grafana dashboard JSON and Prometheus alert-rule syntax#2659

Merged
JSONbored merged 4 commits into
mainfrom
chore/validate-observability-configs
Jul 3, 2026
Merged

chore(selfhost): validate Grafana dashboard JSON and Prometheus alert-rule syntax#2659
JSONbored merged 4 commits into
mainfrom
chore/validate-observability-configs

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Closes #1943

Summary

PR #2638 (this session) audited every deliverable in #1943 against the actual code and found dashboards (6), alert rules (13 groups, ~21 rules), and metrics already comprehensive — the genuine gap was 5 missing troubleshooting runbooks, which shipped there. The one deliverable left unverified was "add validation for dashboard JSON / alert rule syntax if available."

scripts/validate-observability-configs.mjs closes that gap: Grafana and Prometheus both fail OPEN on a malformed dashboard/rule file (skip it, log a warning), so nothing else catches a broken file until an operator notices a panel or alert is simply missing.

  • Checks every grafana/dashboards/*.json file parses as JSON and has a non-empty title + a panels array.
  • Checks prometheus/rules/alerts.yml parses as YAML and every rule has alert, expr, labels.severity, and annotations.summary.
  • Wired into test:ci as selfhost:validate-observability.

Test plan

  • npm run selfhost:validate-observability — passes against the real dashboards/rules
  • npm run typecheck, npm run test:coverage — clean; 12 new unit tests cover both validators' happy paths and failure modes (invalid JSON/YAML, missing fields, empty directory, unreadable path)
  • npm run test:ci (full local gate, unsharded) — green
  • npm audit --audit-level=moderate — 0 vulnerabilities

…-rule syntax (#1943)

Completes the last unverified deliverable from #1943 -- the rest (dashboard
panels, alert rules, troubleshooting runbooks) was audited and found already
comprehensive or shipped in #2638. Grafana and Prometheus both fail OPEN on a
malformed dashboard/rule file (skip it, log a warning), so nothing else catches
this until an operator notices a panel or alert is simply missing.

scripts/validate-observability-configs.mjs checks every grafana/dashboards/*.json
file parses and has a title + panels array, and prometheus/rules/alerts.yml
parses and every rule has alert/expr/labels.severity/annotations.summary. Wired
into test:ci. 12 unit tests cover both validators' happy paths and failure modes.
@dosubot dosubot Bot added the size:M label Jul 3, 2026
@loopover-orb

loopover-orb Bot commented Jul 3, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-03 01:53:02 UTC

4 files · 1 AI reviewer · no blockers · readiness 82/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change adds a small observability-config validator, wires it into `test:ci`, and covers the exported validators with direct unit tests plus real repository fixtures. The dashboard and rule-shape checks are correctly guarded against non-object YAML/JSON values, and the PromQL sanity check now catches the explicit dangling-operator and bracket-mismatch cases shown in the tests. The notable limitation is that this is not full PromQL validation, but the script documents that boundary and still closes the basic fail-open coverage gap.

Nits — 6 non-blocking
  • nit: `scripts/validate-observability-configs.mjs:58` builds child paths with string concatenation, so using `join(dir, file)` would be more robust if this helper is ever called with a trailing slash or platform-specific path.
  • nit: `test/unit/validate-observability-configs-script.test.ts:7` creates temporary files but never removes them, so repeated local runs leave `gt-dash-*` and `gt-alerts-*` directories behind.
  • Use `node:path`'s `join` in `validateDashboards` for `const path = join(dir, file)` to match the test helper style and avoid path-format assumptions.
  • Consider adding one explicit test that a syntactically invalid but bracket-balanced PromQL expression remains out of scope, or rename the PromQL helper/test wording to make the boundary impossible to misread later.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #1943
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (size label size:L; 1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 61 registered-repo PR(s), 52 merged, 506 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 61 PR(s), 506 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • No action.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 3, 2026
…rashing the validator

Gate review caught a real bug: valid JSON/YAML that parses to a non-object (a
dashboard file containing literally "null", or a YAML sequence entry like
"- null") crashed the validator with a TypeError instead of producing the
structured validation error it exists to provide -- the one failure mode it's
explicitly meant to catch cleanly.

Added an isObject() guard before every dereference (dashboard, group, rule) and
4 new regression tests covering null/array/string/primitive-at-each-level.
@dosubot dosubot Bot added size:L and removed size:M labels Jul 3, 2026
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.10%. Comparing base (52f770f) to head (0da7a3e).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2659   +/-   ##
=======================================
  Coverage   96.10%   96.10%           
=======================================
  Files         237      237           
  Lines       26505    26505           
  Branches     9614     9614           
=======================================
  Hits        25472    25472           
  Misses        424      424           
  Partials      609      609           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JSONbored added 2 commits July 2, 2026 18:46
…t import

Gate review correctly noted the validator claimed to check "alert rule syntax"
but only verified expr was a non-empty string -- malformed PromQL like
"up ==" passed silently. Added a lightweight (not a real parser -- no
promtool/PromQL-grammar dependency, out of scope for this "if available"
deliverable) sanity check: balanced brackets and no dangling trailing binary
operator. Verified zero false positives against all 21 real rules in
prometheus/rules/alerts.yml. Also dropped an unused mkdirSync import the
gate flagged as a nit.
…L sanity check

Gate review caught a real bug: a plain depth counter only checks net nesting
count, so "sum(foo[5m))" -- opened with "(" and "[", closed with ")" and ")" --
nets to depth 0 and wrongly passes as balanced, even though the second ")"
doesn't match the "[" it's supposed to close. Switched to a stack that checks
each closer against the delimiter it actually needs to match. Verified against
the real alerts.yml (still zero false positives) and added a regression test
for the exact mismatched-type case the gate identified.
@JSONbored
JSONbored merged commit cfbdc9f into main Jul 3, 2026
9 checks passed
@JSONbored
JSONbored deleted the chore/validate-observability-configs branch July 3, 2026 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs(ops): finish self-host rate-limit dashboards and runbooks

1 participant