Skip to content

Emit throttle metrics#42

Open
forge33 wants to merge 6 commits into
pb/consolidate_metrics_emittersfrom
pb/emit_throttle_metrics
Open

Emit throttle metrics#42
forge33 wants to merge 6 commits into
pb/consolidate_metrics_emittersfrom
pb/emit_throttle_metrics

Conversation

@forge33
Copy link
Copy Markdown

@forge33 forge33 commented Jun 1, 2026

Related issue: github#1672
Closes: https://github.com/Shopify/schema-migrations/issues/5456

Description

  • Emit gh_ost.throttle.active on each throttler check tick.
  • Emit gh_ost.throttle.duration_seconds and gh_ost.throttle.events_total once per completed throttle interval.
  • Tag interval metrics with the throttler reason, using the consolidated metrics emitter helper file from the parent PR.

🎩

Screenshot 2026-06-02 at 9 59 10 PM

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.

forge33 added 4 commits June 1, 2026 13:29
Use a single metrics emitter abstraction for gauge, count, and histogram samples so metric helpers share one testable client contract.
@forge33 forge33 force-pushed the pb/emit_throttle_metrics branch from c148501 to fab54df Compare June 1, 2026 18:00
@forge33 forge33 changed the base branch from pb/refactor_metrics_emitter to pb/consolidate_metrics_emitters June 1, 2026 18:01
@forge33 forge33 force-pushed the pb/consolidate_metrics_emitters branch from 8aff79e to aa3774f Compare June 1, 2026 18:03
@forge33 forge33 force-pushed the pb/emit_throttle_metrics branch 2 times, most recently from aec0681 to 4528c12 Compare June 1, 2026 18:04
Keep the small metric emission helpers, runtime reporter, and their tests together so the metrics package has fewer one-function files.
@forge33 forge33 force-pushed the pb/consolidate_metrics_emitters branch from aa3774f to 63e4992 Compare June 1, 2026 18:30
@forge33 forge33 force-pushed the pb/emit_throttle_metrics branch from 4528c12 to 2beb2ce Compare June 1, 2026 18:30
@forge33 forge33 added the #gsd:50633 Data Storage: gh-ost Observability Instrumentation label Jun 1, 2026
Comment thread go/logic/throttler.go Outdated
}

func (thlr *Throttler) recordThrottleMetrics(alreadyThrottling bool, shouldThrottle bool, throttleReason string, now time.Time) {
metrics.EmitThrottleActiveGauge(thlr.migrationContext.Metrics, shouldThrottle)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might want to add debounce here so we don't call this every 100ms?

Record throttle active state at a debounced cadence and emit duration plus event metrics when a throttled interval completes.
@forge33 forge33 force-pushed the pb/emit_throttle_metrics branch from 2beb2ce to a201553 Compare June 2, 2026 20:45
@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
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