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]

🔍 CI failure analysis for f3d5207: Flaky test failure in TestActivityHeartbeatDetailsDuringRetry, unrelated to PR changes which only modify replication metrics instrumentation.

Issue

The integration test TestIntegrationSuite/TestActivityHeartbeatDetailsDuringRetry failed with an assertion error at /cadence/host/activity_test.go:317:

• Expected value: 2
• Actual value: 1

The test failed twice during the same CI run (at 19:30:56 and 19:44:49), indicating timing-sensitive behavior.

Root Cause

This is a flaky test unrelated to the PR changes. The PR only modifies replication metrics instrumentation:

• Added exponential histogram metrics (ExponentialTaskLatency, ExponentialCacheLatency, ExponentialReplicationTasksAppliedLatency)
• Injected clock.TimeSource into replication components for testability
• No modifications to activity execution, heartbeat handling, or retry logic

The failing test validates activity heartbeat details during retries, which is completely separate from replication metrics.

Details

Evidence this is a flaky test:

1. Failed twice in same run: Different timestamps (19:30:56 and 19:44:49) with identical assertion failures
2. SQLite contention: Multiple "sqlite3: database is locked" errors throughout the logs, indicating resource contention issues common in integration tests
3. Timeout errors: Numerous "context deadline exceeded" and "Async task dispatch timed out" messages
4. PR scope: Changes are purely additive (metric instrumentation) with no business logic modifications
5. Isolated component: Injectable time sources added only for replication components, not activity execution

The test expects a heartbeat details count of 2 but receives 1, characteristic of race conditions or timing sensitivities in integration tests running with SQLite.

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.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

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}