refactor: add replication exponential histograms

[zawadzkidiana]

Summary

• add exponential histogram metrics for replication timers alongside existing timer metrics
• wire histogram recording into replication task ack manager and store paths
• add coverage for histogram metric emission

Test plan

• Not run (not requested)

[fimanishi]

I don't think we need to test the histograms. We can remove this file

[fimanishi]

We should do either one by one or include all of the changes in one PR. Right now we are doing a change in the task_ack_manager and one in the task_store. I'm fine with either going one by one, file by file or all of them, but right now we are doing two different files and not all of them

[fimanishi]

Also, for the cache size I think we should go with gauge and not histogram

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Clean implementation of exponential histogram metrics for replication latency tracking. Follows established dual-emit migration pattern, adds proper injectable time sources for testability, and includes a sensible bug fix changing CacheSize from Timer to Gauge.

✅ 1 resolved

✅ Quality: Inconsistent time source usage in TaskStore

📄 service/history/replication/task_store.go:135
In task_store.go, the histogram latency measurement uses time.Now() directly, while task_ack_manager.go uses t.timeSource.Now() for the same purpose. This inconsistency means:

1. The TaskStore histogram cannot be properly tested with mocked time sources
2. The pattern is inconsistent with the adjacent code in task_ack_manager.go

Looking at the test file metrics_exponential_histograms_test.go, the TaskAckManager test uses a MockedTimeSource that can be advanced, allowing proper testing of the histogram emission. However, the TaskStore test relies on time.Sleep(2 * time.Millisecond) in the errHydrator to ensure a non-zero duration, which is less reliable and introduces real time delays in tests.

Suggested fix:
If TaskStore has a time source field (or can be injected with one), use it consistently:

cacheLatencyStart := m.timeSource.Now()
// ...
defer func() {
    scope.ExponentialHistogram(metrics.ExponentialCacheLatency, m.timeSource.Since(cacheLatencyStart))
}()

If no time source is available, this could be acceptable, but the pattern inconsistency is worth noting for future maintainability.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}