Skip to content

Emit query latency metrics#46

Merged
grodowski merged 2 commits into
masterfrom
grodowski/issue-5458-query-latency-metrics
Jun 8, 2026
Merged

Emit query latency metrics#46
grodowski merged 2 commits into
masterfrom
grodowski/issue-5458-query-latency-metrics

Conversation

@grodowski

@grodowski grodowski commented Jun 2, 2026

Copy link
Copy Markdown
Member

Closes https://github.com/Shopify/schema-migrations/issues/5458

Add a query duration helper and instrument source/target query paths with
millisecond latency histograms.

Metrics emitted:

  • query.duration_milliseconds tagged with side, kind, and outcome

Initial coverage includes chunk copy, binlog apply, range select, and exact
row count queries. This lets dashboards compare source and target query
latency side-by-side and break down latency by query kind.

heartbeat_read is intentionally left out for now: in this codebase heartbeat lag is derived from a lightweight changelog-table lookup in the throttler, and existing lag gauges cover the operational signal. We can add that query kind later if dashboards show inspector read latency is needed

Add unit coverage for the query duration metric helper.

🎩

Cherry-picked this commit plus a few others onto a separate branch, ran a dev tophat run to add name to fake_data:

Screenshot 2026-06-02 at 9 57 42 PM The histogram prometheus query needs to be updated, but we have data

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

@grodowski grodowski added the #gsd:50633 Data Storage: gh-ost Observability Instrumentation label Jun 2, 2026
@grodowski grodowski changed the base branch from grodowski/metrics-client-histogram to pb/refactor_metrics_emitter June 2, 2026 09:52
@grodowski grodowski force-pushed the grodowski/issue-5458-query-latency-metrics branch from 130c72e to 2b5388c Compare June 2, 2026 13:03
@grodowski grodowski changed the base branch from pb/refactor_metrics_emitter to pb/consolidate_metrics_emitters June 2, 2026 13:04
@grodowski grodowski force-pushed the grodowski/issue-5458-query-latency-metrics branch from 2b5388c to 36da844 Compare June 2, 2026 13:18
@grodowski grodowski marked this pull request as ready for review June 2, 2026 13:42
Comment thread go/metrics/emit.go Outdated
}()
}

type queryHistogramEmitter interface {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was attempting to keep all the emitter calls in one interface so we didn't end up with urban sprawl

@forge33 forge33 mentioned this pull request Jun 3, 2026
2 tasks
@forge33 forge33 force-pushed the pb/consolidate_metrics_emitters branch 2 times, most recently from bd9bdb1 to c4e2b06 Compare June 3, 2026 14:41
@forge33 forge33 mentioned this pull request Jun 3, 2026
2 tasks
@grodowski grodowski force-pushed the grodowski/issue-5458-query-latency-metrics branch from 36da844 to 545b558 Compare June 8, 2026 10:19
@grodowski grodowski changed the base branch from pb/consolidate_metrics_emitters to master June 8, 2026 14:27
   Add a query duration helper and instrument source/target query paths with
   millisecond latency histograms.

   Metrics emitted:
   - query.duration_milliseconds tagged with side, kind, and outcome

   Initial coverage includes chunk copy, binlog apply, range select, and exact
   row count queries. This lets dashboards compare source and target query
   latency side-by-side and break down latency by query kind.

   Add unit coverage for the query duration metric helper.
@grodowski grodowski force-pushed the grodowski/issue-5458-query-latency-metrics branch from a09b5e4 to 2a72155 Compare June 8, 2026 14:49
@grodowski grodowski merged commit 8015c79 into master Jun 8, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

#gsd:50633 Data Storage: gh-ost Observability Instrumentation to-upstream

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants