feat(observability): ship OTel Collector config, Grafana dashboard pack and demo - #473
Conversation
- Ship a base config and a two-backend fan-out config - Validate receiver protocols and pipeline wiring in CI
- Ship two importable Grafana dashboards with a connection variable - Gate every panel query on a metric the service registers
- Chart slot key distribution, hot slots and slot IO rates - Chart anomaly counts, correlated groups, baselines and forecasts
- Compose Valkey, the monitor, the collector, Prometheus and Grafana - Provision the dashboard pack and a seed script that populates it
- Describe importing the pack into an existing Grafana - Add a PMM joint-docs skeleton for query analytics
- Make the datasource a `ds` template variable instead of a fixed uid, so the pack imports into any Grafana - Add a `job` variable and pin every selector to it, so the demo's two scrape paths no longer double-count - Give the five table panels `format: table` and a field-organizing transformation so they render rows - Read correlated groups and active anomalies through `sum by (connection)` over the gauge families - Drop the scrape config's reference to a non-existent metrics token - Mark the demo compose file as demo-only - Correct the `resource_to_telemetry_conversion` claim and say which dashboards the demo populates - Extend the asset spec with datasource, job, selector and table-format assertions
- Point the "Correlated groups" stat at `betterdb_correlated_groups_by_pattern`; the by-severity gauge is registered but never written, so it always reads empty - Describe the demo's anomaly dashboard as it actually behaves
- Remove "Correlated groups by severity"; no gauge the service writes carries a correlated-group severity breakdown, and widen "Correlated groups by pattern" to fill the row - Exclude `exported_job` and `service_name` in the table panels' organize transformations, so tables stay clean on the collector job
- fail rather than pass vacuously when a dashboard has no table panel - the datasource test now describes what it checks after the ds variable
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Warning Review limit reachedNext included review available in 24 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds OpenTelemetry Collector configurations, four Grafana dashboards, a Docker Compose observability demo, asset contract tests, and integration documentation. ChangesBetterDB observability pack
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Valkey
participant BetterDBMonitor
participant OTELCollector
participant Prometheus
participant Grafana
Valkey->>BetterDBMonitor: Provide Valkey metrics and command activity
BetterDBMonitor->>OTELCollector: Export OTLP metrics
BetterDBMonitor->>Prometheus: Expose direct Prometheus metrics
OTELCollector->>Prometheus: Expose OTLP-converted metrics
Grafana->>Prometheus: Query BetterDB dashboards
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (15 skipped: 15 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- the pinned 2.6.1 conflicted with the root yaml@2 override, so CI's frozen-lockfile install rejected the tree
- slot read rate, write rate and volatile keys read fields that CLUSTER SLOT-STATS never emits, so all three rendered flat zeros - the table-frame rule now bites at pack level, since the cluster dashboard no longer carries a table panel
KIvanow
left a comment
KIvanow
left a comment
There was a problem hiding this comment.
This is a clean, well-scoped pack - thanks for branching it off master so it doesn't tangle with the Prometheus stack. Approving.
What I like: the demo compose leads with an unmissable "DEMO ONLY" banner that names every insecure choice, so nobody copies it into prod by accident. And observability-assets.spec.ts is the right call - pinning the dashboards and collector/demo wiring to the live registry is what stops this pack from silently rotting the next time a metric is renamed.
On your open questions: deploy/ as a new top-level dir is fine by me - it's a conventional home for this and keeps it out of the app tree. Keep it.
Minor, none blocking:
- The demo pins every image except
betterdb/monitor:latest. Consider pinning it to a released tag so the demo is reproducible as the image moves. - You flagged the root
prometheus.ymlscraping the wrong endpoint (host.docker.internal:3001, nometrics_path). Since this PR is squarely about getting the scrape config right, folding a one-line fix in here would be a natural fit - but I'm happy to take it as a follow-up if you'd rather keep this pack pure additions. betterdb_correlated_groups_by_severityregistered-but-never-written: fine as tracked separately since the panel's gone.
None of that holds the merge.
2 participants
Ships the observability pack from the planning board item "[P0] Ship OTel
Collector config, Grafana dashboard pack, and side-by-side demo". Branches off
master and touches nothing the open Prometheus stack (#462/#467/#470) touches.
What's here
deploy/observability/collector/otel-collector.yamlscrapes Monitor's
/api/prometheus/metricsand re-exports on:8889;otel-collector.fanout.yamlis the same pipeline plus a secondary OTLPendpoint for users who already ship elsewhere.
deploy/observability/dashboards/: instance vitals, query patterns, clusterslots, anomalies. Each takes a
Data sourcevariable, so they import into anyGrafana without editing JSON, plus
Scrape jobandConnectionvariables.deploy/observability/demo/brings up Valkey, Monitor,the collector, Prometheus and Grafana with everything provisioned; Prometheus
scrapes both the direct path and the OTLP path so you can compare them by
flipping one variable.
deploy/observability/README.md, a newdocs/pmm-integration.md,and updates to
docs/prometheus-integration.mdanddocs/opentelemetry.md.apps/api/src/prometheus/observability-assets.spec.ts(59) checksthe assets against the live metric registry: no dashboard can reference a metric
that is not registered, every target filters by job and connection, every table
panel queries in table format, every dashboard is provisioned, and the collector
configs use every component they define.
Verified
The stack was brought up end to end: five containers healthy, both Prometheus
targets up, seeded traffic visible, all four dashboards rendering.
Known gaps
there and the anomaly detectors need a longer run to warm up. The README says so.
deploy/is a new top-level directory; the repo keeps compose files at the rootand charts in
charts/. Happy to move it if you'd rather it lived elsewhere.prometheus.ymlstill scrapeshost.docker.internal:3001with nometrics_path, which is the wrong endpoint. Pre-existing; left alone here.betterdb_correlated_groups_by_severityis registered but never written by theservice. The panel that read it is gone; the gauge itself is tracked separately.
🤖 Generated with Claude Code
Note
Low Risk
Mostly additive deploy assets, documentation, and contract tests; runtime API behavior is unchanged aside from a new dev dependency.
Overview
Adds a
deploy/observability/pack: OTLP intake Collector configs (base + optional fanout toSECONDARY_OTLP_ENDPOINT), four importable Grafana dashboards (vitals, query patterns, cluster slots, anomalies) with sharedds/job/connectionvariables, and a demo-only Docker Compose stack (Valkey, Monitor, Collector, Prometheus, Grafana) plus seed traffic and provisioning.observability-assets.spec.tslocks the pack to runtime reality by parsing shipped YAML/JSON and asserting Collector pipeline wiring, demo compose mounts/scrape jobs, and that every dashboard PromQL target uses registeredbetterdb_*metrics and consistent job/connection filters. Addsyamlas an API dev dependency for those tests.Docs gain the observability README, PMM integration guidance, Grafana pack sections in Prometheus integration, and Collector/dashboard notes in OpenTelemetry (plus table formatting touch-ups).
Reviewed by Cursor Bugbot for commit 72228c0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation