Add schema-level MySQL statement and I/O metrics by Areson · Pull Request #186 · block/blip

Areson · 2026-09-24T17:47:17Z

Why

Blip reports table I/O by table and query counters at server scope. Operators who share one MySQL server cannot compare statement load and table activity by schema without building their own rollups.

What

Add an opt-in stmt.schema domain for statement counts, latency, errors, and row activity grouped by db.
Add group-by: table|schema to wait.io.table; the current table output remains the default.

How

The new collector sums selected counters from events_statements_summary_by_digest by SCHEMA_NAME. The I/O collector reads either table series or schema rollups, never both in one collection. Schema rollups weight averages by event count and exclude empty tables from minimum wait times. Schema sums scan as float64 because MySQL can return values above MaxInt64.

Risk

The new domain and schema grouping are opt-in. Schema statement series omit null-schema and digest catch-all rows, so they can undercount when the digest table fills. If a table is dropped while schema grouping uses truncate-table: no, the cumulative schema total can fall and cause a delta spike; the default truncation mode avoids this.

Testing

Ran go test -count=1 ./... with a local MySQL 8.0.34 container. The MySQL integration tests for both collectors and the schema overflow regression passed. Ran go vet on the changed packages.

Generated with Codex

eduardo-landin-mestre

🤖 stmt.schema looks good: allowlisted metrics, parameterized schema filters, the float64 scan for large SUMs, and clear docs on the digest catch-all undercount.

Requesting changes for one issue in wait.io.table: with group-by: both, schema series share metric names with table series and differ only by the missing tbl tag, so Datadog/SignalFx queries filtered by db double count (details inline). The rest are smaller.

I checked the rollup SQL (weighted avg via SUM(sum_timer_x)/SUM(count_x), count_star mapping for *_wait, MIN ignoring idle tables) and it looks correct. Unit tests and go vet pass locally; I did not run the MySQL integration tests.

Nits:

stmt.schema is not listed in docs/content/about/production-readiness.md (presumably "New").

Areson · 2026-10-22T22:47:19Z

Sign in to view

 		for i := 2; i < len(cols); i++ {
+			group := map[string]string{"db": dbName, "tbl": tblName}
+			if schema {
+				group = map[string]string{"db": dbName}


🤖 👨 Agreed. We removed both. Each collection now emits either table series or schema series under these metric names, so switching to schema grouping cannot add a second set to a db aggregation. Table grouping remains the default.

eduardo-landin-mestre · 2026-10-22T22:13:15Z

 		}

 		o.query, o.params = TableIoWaitQuery(dom.Options, dom.Metrics)
+		o.groupBy = dom.Options[OPT_GROUP_BY]


🤖 Unknown values (e.g. a typo like schemas) silently fall back to table output. Could Prepare return an error for anything other than table, schema, both, or empty, like it does for an invalid truncate-timeout?

🤖 👨 Prepare now rejects values other than empty, table, or schema, including the removed both value. The collector help lists only table and schema.

eduardo-landin-mestre · 2026-10-22T22:13:15Z


-	rows, err := t.db.QueryContext(ctx, o.query, o.params...)
+	var metrics []blip.MetricValue
+	if o.groupBy != "schema" {


🤖 In both mode the two SELECTs are separate reads, so events arriving between them land in one series but not the other for that interval, and with the default truncate-table: yes the existing window before TRUNCATE (where events are lost) gets longer. One option is to roll up schema values in Go from the table rows when the needed count_*/sum_timer_* columns are selected. Otherwise it may be worth a sentence in the docs that table and schema totals can disagree slightly.

🤖 👨 Removing both also removes the second read. A collection now runs one SELECT for its chosen grouping before the optional truncate, so the two series can no longer disagree within a collection.

eduardo-landin-mestre · 2026-10-22T22:13:15Z

+		metrics = append(metrics, values...)
+	}
+
+	if o.truncate {


🤖 Nit: the move dropped the comments explaining why lock_wait_timeout is set first and why TruncateError must run even when there is no error. Worth keeping them.

🤖 👨 Restored comments explaining why we set lock_wait_timeout before truncating and why TruncateError processes the metrics even when truncation succeeds.

eduardo-landin-mestre · 2026-10-22T22:13:15Z

+			},
 		},
 		Groups: []blip.CollectorKeyValue{
 			{Key: "db", Value: "the database name for the corresponding table io, or empty string for all dbs"},


🤖 Nit: the db/tbl group help still says "or empty string for all dbs" and does not mention schema series, which have only db.

🤖 👨 Updated the group descriptions: db identifies the database in either mode, and tbl is present only for table series.

eduardo-landin-mestre · 2026-10-22T22:13:15Z

+func TestCollectMySQL80(t *testing.T) {
+	_, db, err := test.Connection("mysql80")
+	if err != nil {
+		t.Skip("mysql80 not running")


🤖 Nit: this skips on any connection error, while the new wait.io.table tests skip only on *net.OpError and fail otherwise. Using the same pattern here would keep config or auth errors from hiding as skips.

🤖 👨 The stmt.schema test now skips only for a network connection error. Configuration and authentication errors fail the test.

zyin-squareup · 2026-10-22T22:13:31Z

 		for i := 2; i < len(cols); i++ {
+			group := map[string]string{"db": dbName, "tbl": tblName}
+			if schema {
+				group = map[string]string{"db": dbName}


🤖 Non-blocking: with group-by: both, the schema series and the table series use the same metric name (e.g. wait.io.table.count_star). The only difference is that the schema series has no tbl tag. In Datadog or Prometheus, aggregating by db (e.g. sum by (db), or a Datadog sum filtered to db:app) adds up every table series and the schema series, so the total is 2x. Every dashboard would have to filter on whether tbl is present to avoid that.

Also, the existing Help().Groups text says tbl is "empty string for all tables", but these series leave tbl out entirely.

Suggestions: give schema rollups their own metric names (e.g. a schema_ prefix or a separate domain), or at least document the double-counting for both in docs/content/metrics/domains/wait.io.table/_index.md.

🤖 👨 Agreed. We removed both, so the collector no longer emits table and schema series with the same name in one collection. I also corrected the help text to say that schema series omit tbl.

zyin-squareup · 2026-10-22T22:13:34Z

+	for _, column := range columns {
+		switch {
+		case strings.HasPrefix(column, "count_"), strings.HasPrefix(column, "sum_timer_"):
+			selectColumns = append(selectColumns, fmt.Sprintf("SUM(%s) AS %s", column, column))


🤖 Non-blocking: with truncate-table: no the metric type is CUMULATIVE_COUNTER, but a schema-level SUM across tables only stays monotonic if no table rows leave table_io_waits_summary_by_table. They do leave in normal operation: DROP TABLE removes the row, which happens at the cut-over of every gh-ost/pt-osc migration (the old table is dropped), and so does eviction under performance_schema_max_table_instances. When that happens the schema sum drops. sink/delta.go:99-103 then sends the whole cumulative value as the delta, so you get one large spike per migration.

Table-level series do not have this problem because the dropped table series simply stops. The default truncate mode is not affected either. Consider documenting this limitation for group-by: schema|both with truncate-table: no, or recommending the default truncate mode for schema rollups.

🤖 👨 This remains a real limitation. Dropping a table can lower a cumulative schema total and cause a delta spike. I added a prominent warning beside group-by: schema and called it out in the PR Risk section, with a recommendation to use the default truncate-table: yes. The documentation does not eliminate the behavior.

zyin-squareup

🤖 This review was generated by an AI agent on behalf of @zyin-squareup.

Verdict: approve. I found no blocking issues, and two non-blocking items are in the inline comments.

Both new modes are opt-in, and the default wait.io.table path still does the same thing. It runs the same query and scans into int64, then applies truncation and TruncateErrorPolicy to the collected metrics as before. One small behavior change, and I think it is an improvement: collectQuery now returns rows.Err(), so an iteration error is reported instead of being dropped and then truncating anyway.

Non-blocking

With group-by: both, aggregating by db counts I/O twice, because the table and schema series share metric names (inline on metrics/wait.io.table/table.go:332).
With truncate-table: no, the schema rollups go backwards whenever a table is dropped (e.g. at a gh-ost/pt-osc cut-over), and the delta sink then reports the whole cumulative value as a spike (inline on metrics/wait.io.table/query.go:32).
Nit: moving the truncate block into Collect removed the comments that explained why lock_wait_timeout is set first and why TruncateError runs even when there is no error. It would be good to keep them.

What I checked and found sound

countSuffix maps *_timer_wait to count_star and every other *_timer_<op> to an existing count_<op> column.
The weighted average uses NULLIF/COALESCE, so it cannot divide by zero, and the minimum ignores tables with no events.
Schema SUM values come back as DECIMAL and are scanned into float64, so values above MaxInt64 do not overflow.
group-by values are validated against the option Values in Help (collector.go:92), so a typo is rejected instead of silently falling back to table.
stmt.schema checks metric names against an allowlist before interpolating them into SQL, and schema names are passed as bound parameters. The null-schema/digest catch-all row is excluded, and the docs say so.

Validation: At this commit, go build ./..., go vet, and go test -count=1 ./metrics/stmt.schema/ ./metrics/wait.io.table/ ./metrics/ all pass locally. The three MySQL 8.0 integration tests were skipped because no local MySQL was running, so I am relying on the PR description and green CI for the live-DB paths.

Areson added 2 commits October 22, 2026 15:07

Add schema-level MySQL statement and I/O metrics

c999f8e

Fix schema I/O rollup overflow and test setup

daf2762

Areson force-pushed the ioberst/schema-metrics branch from 6595bf9 to daf2762 Compare October 22, 2026 22:08

Areson marked this pull request as ready for review October 22, 2026 22:09

eduardo-landin-mestre suggested changes Oct 22, 2026

View reviewed changes

zyin-squareup reviewed Oct 22, 2026

View reviewed changes

zyin-squareup approved these changes Oct 22, 2026

View reviewed changes

Ian Oberst added 2 commits October 22, 2026 15:36

Avoid duplicate table I/O series

76fdbd8

Highlight schema rollup counter risk

94b7886

eduardo-landin-mestre approved these changes Oct 22, 2026

View reviewed changes

Conversation

Areson commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

How

Risk

Testing

Uh oh!

eduardo-landin-mestre left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zyin-squareup left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Areson commented Sep 24, 2026 •

edited

Loading