fix(metrics): use RecordHistogramValue for task attempt count histograms

[zawadzkidiana]

What changed

Switch ExponentialTaskAttemptCounts and ExponentialTaskAttemptCountsPerDomain histogram definitions from intExponentialBuckets: Mid1To16k + IntExponentialHistogram to buckets: ResponseRowSizeBuckets + RecordHistogramValue, aligning with the pattern used by ReplicationTasksFetchedSize.

Why

The previous implementation used IntExponentialHistogram with intExponentialBuckets, but the correct pattern in this codebase for recording histogram values is to use RecordHistogramValue with standard buckets. This aligns the task attempt count metrics with the established pattern used by other histogram metrics.

How did you test it

Existing unit tests cover the task attempt recording paths. The change is a straightforward API alignment with no behavioral change in the recorded values.

Potential risks

Low. The bucket boundaries change from Mid1To16k (integer exponential buckets) to ResponseRowSizeBuckets. Existing dashboards or alerts that rely on the old bucket boundaries may need updating.

Release notes

No user-facing changes. Internal metrics instrumentation improvement.

Documentation changes

None required.

[Shaddoll]

Do you mind renaming it to TaskAttemptCounts?

[gitar-bot]

The PR description needs a couple of improvements:

Details

[How did you test it?] — Please replace the vague statement with specific, copy-pasteable test commands so reviewers can reproduce the test steps. For example:

go test -v ./service/history/task/... -run TestTaskAttempt

[Why?] — Please explain what was functionally wrong or incorrect with the previous IntExponentialHistogram + intExponentialBuckets: Mid1To16k approach, and why ResponseRowSizeBuckets is an appropriate bucket set for task attempt counts (or acknowledge it as a known trade-off if that's the case).

[gitar-bot]

⚠️ Bug: ResponseRowSizeBuckets is a poor fit for task attempt counts

ResponseRowSizeBuckets has boundaries at {0, 1, 2, 4, 8, 16, 32, 64, ..., 65536} — power-of-2 steps designed for response payload sizes. For task attempt counts, which are typically small integers (1–20 for most tasks), these buckets lose significant resolution: you can't distinguish 2 vs 3 attempts, or 5 vs 7 attempts. The old Mid1To16k had 4 sub-buckets per octave giving much finer granularity.

Consider defining dedicated buckets for attempt counts with better low-end resolution, e.g. linear buckets from 1–10 then exponential beyond that. Alternatively, use tally.MustMakeLinearValueBuckets(1, 1, 20) (buckets at 1,2,3,...,20) appended with a few exponential buckets for the tail, which would capture the full distribution of attempt counts where it matters most.

_{Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion}

[gitar-bot]

Code Review ⚠️ Changes requested 0 resolved / 1 findings

Switches task attempt count histograms to use RecordHistogramValue, but ResponseRowSizeBuckets is a poor fit for this metric—its boundaries {0, 1, 2, 4, 8, 16, 32, 64...} don't align with typical task attempt distributions.

⚠️ Bug: ResponseRowSizeBuckets is a poor fit for task attempt counts

📄 common/metrics/defs.go:3336 📄 common/metrics/defs.go:3359

ResponseRowSizeBuckets has boundaries at {0, 1, 2, 4, 8, 16, 32, 64, ..., 65536} — power-of-2 steps designed for response payload sizes. For task attempt counts, which are typically small integers (1–20 for most tasks), these buckets lose significant resolution: you can't distinguish 2 vs 3 attempts, or 5 vs 7 attempts. The old Mid1To16k had 4 sub-buckets per octave giving much finer granularity.

Consider defining dedicated buckets for attempt counts with better low-end resolution, e.g. linear buckets from 1–10 then exponential beyond that. Alternatively, use tally.MustMakeLinearValueBuckets(1, 1, 20) (buckets at 1,2,3,...,20) appended with a few exponential buckets for the tail, which would capture the full distribution of attempt counts where it matters most.

🤖 Prompt for agents

Code Review: Switches task attempt count histograms to use RecordHistogramValue, but ResponseRowSizeBuckets is a poor fit for this metric—its boundaries {0, 1, 2, 4, 8, 16, 32, 64...} don't align with typical task attempt distributions.

1. ⚠️ Bug: ResponseRowSizeBuckets is a poor fit for task attempt counts
   Files: common/metrics/defs.go:3336, common/metrics/defs.go:3359

   `ResponseRowSizeBuckets` has boundaries at `{0, 1, 2, 4, 8, 16, 32, 64, ..., 65536}` — power-of-2 steps designed for response payload sizes. For task attempt counts, which are typically small integers (1–20 for most tasks), these buckets lose significant resolution: you can't distinguish 2 vs 3 attempts, or 5 vs 7 attempts. The old `Mid1To16k` had 4 sub-buckets per octave giving much finer granularity.
   
   Consider defining dedicated buckets for attempt counts with better low-end resolution, e.g. linear buckets from 1–10 then exponential beyond that. Alternatively, use `tally.MustMakeLinearValueBuckets(1, 1, 20)` (buckets at 1,2,3,...,20) appended with a few exponential buckets for the tail, which would capture the full distribution of attempt counts where it matters most.

Rules ❌ No requirements met

Repository Rules

❌ PR Description Quality Standards: The [How did you test it?] section lacks specific, copy-pasteable test commands, and the [Why?] section does not explain what was functionally wrong with the previous IntExponentialHistogram approach or why ResponseRowSizeBuckets is appropriate for task attempt counts.

_{1 rule not applicable. Show all rules by commenting gitar display:verbose.}

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}