refactor(metrics): replace timers with histograms

Remove the NamedTimer helper and stop recording any durations as
tally timers. Ad-hoc durations now go through NamedDurationHistogram
(default latency buckets), and Op.Complete records its latency as a
histogram named {name}.latency (previously a {name}.latency timer
plus a redundant {name}.latency_histogram).

Migrated call sites: consumer controller/ack-nack latency, mysql
subscriber poll latency/message age, and Op.Complete.

Timers are unsafe in a distributed system: per-host percentiles
cannot be merged, so any aggregation beyond the mean is imprecise.
Bucketed histograms merge exactly across hosts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: sbalabanov-zz

platform/consumer/consumer.go

@@ -376,7 +376,7 @@ func (m *consumer) processDelivery(ctx context.Context, controller Controller, d
 		}
 	}
 
-	metrics.NamedTimer(controllerScope, opName, "controller_latency", elapsed, metrics.NewTag("success", successTag))
+	metrics.NamedDurationHistogram(controllerScope, opName, "controller_latency", elapsed, metrics.NewTag("success", successTag))
 
 	if err != nil {
 		// Single explicit classification pass through the configured
@@ -443,7 +443,7 @@ func (m *consumer) processDelivery(ctx context.Context, controller Controller, d
 			metrics.NamedCounter(controllerScope, opName, "nack_errors", 1)
 		} else {
 			metrics.NamedCounter(controllerScope, opName, "nack_count", 1)
-			metrics.NamedTimer(controllerScope, opName, "ack_nack_latency", time.Since(nackStart),
+			metrics.NamedDurationHistogram(controllerScope, opName, "ack_nack_latency", time.Since(nackStart),
 				metrics.NewTag("operation", "nack"),
 				metrics.NewTag("success", "true"),
 			)
@@ -461,7 +461,7 @@ func (m *consumer) processDelivery(ctx context.Context, controller Controller, d
 			"error", ackErr,
 		)
 		metrics.NamedCounter(controllerScope, opName, "ack_errors", 1)
-		metrics.NamedTimer(controllerScope, opName, "ack_nack_latency", time.Since(ackStart),
+		metrics.NamedDurationHistogram(controllerScope, opName, "ack_nack_latency", time.Since(ackStart),
 			metrics.NewTag("operation", "ack"),
 			metrics.NewTag("success", "false"),
 		)
@@ -470,7 +470,7 @@ func (m *consumer) processDelivery(ctx context.Context, controller Controller, d
 
 	metrics.NamedCounter(controllerScope, opName, "messages_processed", 1)
 	metrics.NamedCounter(controllerScope, opName, "ack_count", 1)
-	metrics.NamedTimer(controllerScope, opName, "ack_nack_latency", time.Since(ackStart),
+	metrics.NamedDurationHistogram(controllerScope, opName, "ack_nack_latency", time.Since(ackStart),
 		metrics.NewTag("operation", "ack"),
 		metrics.NewTag("success", "true"),
 	)

platform/consumer/consumer_test.go

@@ -470,14 +470,14 @@ func TestConsumer_ObservabilityTags(t *testing.T) {
 
 			snapshot := testScope.Snapshot()
 
-			timers := snapshot.Timers()
-			assert.NotEmpty(t, timers, "Should have timer metrics")
+			histograms := snapshot.Histograms()
+			assert.NotEmpty(t, histograms, "Should have histogram metrics")
 
 			var foundLatency bool
-			for _, timer := range timers {
-				if strings.Contains(timer.Name(), "controller_latency") {
+			for _, histogram := range histograms {
+				if strings.Contains(histogram.Name(), "controller_latency") {
 					foundLatency = true
-					tags := timer.Tags()
+					tags := histogram.Tags()
 					if tt.expectSuccess {
 						assert.Equal(t, "true", tags["success"])
 					} else {
@@ -542,7 +542,7 @@ func TestConsumer_AckNackLatencyTracking(t *testing.T) {
 	<-done
 
 	snapshot := scope.Snapshot()
-	assert.NotEmpty(t, snapshot.Timers(), "Should have timer metrics for latency tracking")
+	assert.NotEmpty(t, snapshot.Histograms(), "Should have histogram metrics for latency tracking")
 	assert.NotEmpty(t, snapshot.Counters(), "Should have counter metrics")
 
 	err = c.Stop(30000)

platform/extension/messagequeue/mysql/subscriber.go

@@ -835,7 +835,7 @@ func (w *partitionWorker) pollAndDeliver(ctx context.Context) error {
 
 		// Calculate message age for metrics
 		messageAge := time.Duration(time.Now().UnixMilli()-row.PublishedAt) * time.Millisecond
-		metrics.NamedTimer(s.scope, "poll", "message_age", messageAge,
+		metrics.NamedDurationHistogram(s.scope, "poll", "message_age", messageAge,
 			metrics.NewTag("topic", sub.topic),
 			metrics.NewTag("partition_key", partitionKey),
 		)
@@ -920,7 +920,7 @@ func (w *partitionWorker) pollAndDeliver(ctx context.Context) error {
 			metrics.NewTag("topic", sub.topic),
 			metrics.NewTag("partition_key", partitionKey),
 		)
-		metrics.NamedTimer(s.scope, "poll", "latency", elapsed,
+		metrics.NamedDurationHistogram(s.scope, "poll", "latency", elapsed,
 			metrics.NewTag("topic", sub.topic),
 			metrics.NewTag("partition_key", partitionKey),
 		)

platform/metrics/README.md

@@ -1,12 +1,12 @@
 # Metrics Utilities (`platform/metrics`)
 
-The `metrics` package provides reusable helpers for emitting counters, timers, histograms, and gauges on a `tally.Scope`. It standardizes metric names across controllers and integrates with `platform/errs` for automatic error classification tags.
+The `metrics` package provides reusable helpers for emitting counters, histograms, and gauges on a `tally.Scope`. It standardizes metric names across controllers and integrates with `platform/errs` for automatic error classification tags.
 
 ## Design
 
 **Free functions on `tally.Scope`** — no wrapper types. Existing constructors accept `tally.Scope` and don't need to change.
 
-**Operation lifecycle** — `Begin` and `Complete` tie the full metrics lifecycle together. `Begin` captures the start time and emits `{name}.called`; `Complete` emits succeeded/failed counters, a latency timer, and a latency histogram. This prevents mismatched or forgotten metrics calls.
+**Operation lifecycle** — `Begin` and `Complete` tie the full metrics lifecycle together. `Begin` captures the start time and emits `{name}.called`; `Complete` emits succeeded/failed counters and a latency histogram. This prevents mismatched or forgotten metrics calls.
 
 **Error-aware tagging** — `ErrorTags` integrates with `platform/errs` to produce `error_origin=user|infra`, `retryable=true|false`, and `dependency=true` tags automatically. `Complete` uses these to tag latency metrics on failure.
 
@@ -19,7 +19,7 @@ For any operation with a clear start/end, use `Begin`/`Complete`:
 | Function | Emits |
 |----------|-------|
 | `Begin(scope, name, ...tags)` | `{name}.called` counter +1, returns `Op` |
-| `op.Complete(err)` | `{name}.succeeded` or `{name}.failed` counter, `{name}.latency` timer, `{name}.latency_histogram` histogram — all tagged with `result=success\|error` and error classification tags on failure |
+| `op.Complete(err)` | `{name}.succeeded` or `{name}.failed` counter, `{name}.latency` histogram — tagged with `result=success\|error` and error classification tags on failure |
 
 ```go
 // RPC controller
@@ -43,13 +43,11 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
 
 On success, `Complete` emits:
 - `{name}.succeeded` counter +1
-- `{name}.latency` timer tagged `result=success`
-- `{name}.latency_histogram` histogram tagged `result=success`
+- `{name}.latency` histogram tagged `result=success`
 
 On failure, `Complete` emits:
 - `{name}.failed` counter +1
-- `{name}.latency` timer tagged `result=error`, `error_origin=user|infra`, `retryable=true|false`, and optionally `dependency=true`
-- `{name}.latency_histogram` histogram with the same tags
+- `{name}.latency` histogram tagged `result=error`, `error_origin=user|infra`, `retryable=true|false`, and optionally `dependency=true`
 
 ## Named Helpers
 
@@ -58,25 +56,29 @@ For ad-hoc metrics that don't fit the Begin/Complete lifecycle. All follow the `
 | Function | Emits | Example |
 |----------|-------|---------|
 | `NamedCounter(scope, name, counter, value, ...tags)` | `{name}.{counter}` counter | `publish.attempts` |
-| `NamedTimer(scope, name, timer, duration, ...tags)` | `{name}.{timer}` timer | `publish.queue_latency` |
+| `NamedDurationHistogram(scope, name, histogram, duration, ...tags)` | `{name}.{histogram}` histogram (default latency buckets) | `publish.queue_latency` |
 | `NamedHistogram(scope, name, histogram, buckets, ...tags)` | `{name}.{histogram}` histogram | `process.duration` |
 | `NamedGauge(scope, name, gauge, value, ...tags)` | `{name}.{gauge}` gauge | `consumer.pending_messages` |
 
 ```go
 // Count a specific sub-event
 metrics.NamedCounter(c.scope, "publish", "attempts", 1)
 
-// Record a specific sub-latency
-metrics.NamedTimer(c.scope, "publish", "queue_latency", elapsed)
+// Record a one-shot sub-latency as a histogram (default latency buckets)
+metrics.NamedDurationHistogram(c.scope, "publish", "queue_latency", elapsed)
 
 // Track current queue depth (goes up and down)
 metrics.NamedGauge(c.scope, "consumer", "pending_messages", float64(len(pending)))
 
-// Create a reusable histogram (store on struct, call RecordDuration per invocation)
+// Create a reusable histogram with custom buckets (store on struct, call RecordDuration per invocation)
 h := metrics.NamedHistogram(c.scope, "process", "duration", tally.DurationBuckets{...})
 h.RecordDuration(elapsed)
 ```
 
+### Why histograms, not timers
+
+Durations are recorded as **histograms**, never timers. In a distributed system every service instance emits its own metrics, and the monitoring backend aggregates them across hosts. Timer percentiles (p50/p99/max) are computed per host and cannot be merged — averaging two hosts' p99 values, or taking the max of their maxes, does not yield the true fleet-wide p99. Only the mean survives cross-host aggregation intact, so every percentile you actually care about becomes imprecise. Bucketed histograms merge exactly: summing per-host bucket counts reconstructs the true global distribution, so percentiles stay accurate at any aggregation level.
+
 ## Error Tags
 
 `ErrorTags` classifies errors using `platform/errs` and returns tags for dimensional filtering:

platform/metrics/metrics.go

@@ -87,19 +87,21 @@ func Begin(scope tally.Scope, name string, tags ...Tag) Op {
 }
 
 // Complete records the outcome of the operation. It emits a {name}.succeeded or
-// {name}.failed counter based on err, and records elapsed time on both
-// {name}.latency (timer) and {name}.latency_histogram (histogram with
-// defaultLatencyBuckets for percentile distributions), tagged with result=success|error.
-// On failure, error classification tags (error_origin, retryable, dependency)
-// are added to both the timer and histogram.
+// {name}.failed counter based on err, and records elapsed time on the
+// {name}.latency histogram (using defaultLatencyBuckets for percentile
+// distributions), tagged with result=success|error. On failure, error
+// classification tags (error_origin, retryable, dependency) are added to the
+// histogram.
+//
+// Latency is recorded as a histogram rather than a timer because timer
+// percentiles cannot be merged across hosts (see NamedDurationHistogram).
 func (o Op) Complete(err error) {
 	elapsed := time.Since(o.start)
 
 	if err == nil {
 		o.scope.Counter("succeeded").Inc(1)
 		s := o.scope.Tagged(map[string]string{"result": "success"})
-		s.Timer("latency").Record(elapsed)
-		s.Histogram("latency_histogram", defaultLatencyBuckets).RecordDuration(elapsed)
+		s.Histogram("latency", defaultLatencyBuckets).RecordDuration(elapsed)
 		return
 	}
 
@@ -110,18 +112,24 @@ func (o Op) Complete(err error) {
 		latencyTags[t.Key] = t.Value
 	}
 	s := o.scope.Tagged(latencyTags)
-	s.Timer("latency").Record(elapsed)
-	s.Histogram("latency_histogram", defaultLatencyBuckets).RecordDuration(elapsed)
+	s.Histogram("latency", defaultLatencyBuckets).RecordDuration(elapsed)
 }
 
 // NamedCounter increments the {name}.{counter} counter by value.
 func NamedCounter(scope tally.Scope, name string, counter string, value int64, tags ...Tag) {
 	tagged(scope, tags).SubScope(name).Counter(counter).Inc(value)
 }
 
-// NamedTimer records a duration on the {name}.{timer} timer.
-func NamedTimer(scope tally.Scope, name string, timer string, d time.Duration, tags ...Tag) {
-	tagged(scope, tags).SubScope(name).Timer(timer).Record(d)
+// NamedDurationHistogram records a duration on the {name}.{histogram} histogram
+// using defaultLatencyBuckets. Use it for one-shot latency/duration measurements
+// that don't fit the Begin/Complete lifecycle.
+//
+// Prefer histograms over timers for durations: in a distributed system, timer
+// percentiles (p50/p99/max) are computed per-host and then merged, which is
+// statistically invalid — only the mean survives aggregation. Bucketed
+// histograms merge exactly across hosts, so every percentile stays accurate.
+func NamedDurationHistogram(scope tally.Scope, name string, histogram string, d time.Duration, tags ...Tag) {
+	tagged(scope, tags).SubScope(name).Histogram(histogram, defaultLatencyBuckets).RecordDuration(d)
 }
 
 // NamedHistogram returns a tally.Histogram at {name}.{histogram} with the given

platform/metrics/metrics_test.go

@@ -105,14 +105,9 @@ func TestComplete(t *testing.T) {
 				assert.True(t, ok, "expected process.succeeded counter")
 				assert.Equal(t, int64(1), c.Value())
 
-				timers := snapshot.Timers()
-				timer, ok := timers["process.latency+result=success"]
-				assert.True(t, ok, "expected process.latency timer with result=success")
-				assert.NotEmpty(t, timer.Values())
-
 				histograms := snapshot.Histograms()
-				_, ok = histograms["process.latency_histogram+result=success"]
-				assert.True(t, ok, "expected process.latency_histogram with result=success")
+				_, ok = histograms["process.latency+result=success"]
+				assert.True(t, ok, "expected process.latency histogram with result=success")
 			} else {
 				c, ok := counters["process.failed+"]
 				assert.True(t, ok, "expected process.failed counter")
@@ -127,15 +122,7 @@ func TestComplete(t *testing.T) {
 				tagSuffix += ",result=" + tt.expectResultTag
 				tagSuffix += ",retryable=" + tt.expectRetryable
 
-				timerKey := "process.latency+" + tagSuffix
-				timers := snapshot.Timers()
-				timer, ok := timers[timerKey]
-				assert.True(t, ok, "expected timer key %s, got keys: %v", timerKey, timerKeys(timers))
-				if ok {
-					assert.NotEmpty(t, timer.Values())
-				}
-
-				histogramKey := "process.latency_histogram+" + tagSuffix
+				histogramKey := "process.latency+" + tagSuffix
 				histograms := snapshot.Histograms()
 				_, ok = histograms[histogramKey]
 				assert.True(t, ok, "expected histogram key %s, got keys: %v", histogramKey, histogramKeys(histograms))
@@ -172,15 +159,14 @@ func TestNamedCounter(t *testing.T) {
 	assert.Equal(t, int64(5), c.Value())
 }
 
-func TestNamedTimer(t *testing.T) {
+func TestNamedDurationHistogram(t *testing.T) {
 	scope := tally.NewTestScope("", nil)
-	NamedTimer(scope, "publish", "queue_latency", 42*time.Millisecond)
+	NamedDurationHistogram(scope, "publish", "queue_latency", 42*time.Millisecond)
 
 	snapshot := scope.Snapshot()
-	timers := snapshot.Timers()
-	timer, ok := timers["publish.queue_latency+"]
-	assert.True(t, ok, "expected publish.queue_latency timer")
-	assert.Equal(t, []time.Duration{42 * time.Millisecond}, timer.Values())
+	histograms := snapshot.Histograms()
+	_, ok := histograms["publish.queue_latency+"]
+	assert.True(t, ok, "expected publish.queue_latency histogram, got keys: %v", histogramKeys(histograms))
 }
 
 func TestNamedHistogram(t *testing.T) {
@@ -269,15 +255,6 @@ func TestErrorTags(t *testing.T) {
 	}
 }
 
-// timerKeys extracts map keys for error messages.
-func timerKeys(m map[string]tally.TimerSnapshot) []string {
-	keys := make([]string, 0, len(m))
-	for k := range m {
-		keys = append(keys, k)
-	}
-	return keys
-}
-
 // counterKeys extracts map keys for error messages.
 func counterKeys(m map[string]tally.CounterSnapshot) []string {
 	keys := make([]string, 0, len(m))