Summary
callWithAccumulatingTelemetry currently supports only counter measurements — values are summed across batched calls. This works well for counting occurrences (e.g., shown = 1, accepted = 1), but it cannot capture distribution metrics like candidate counts, latencies, or sizes without distorting the data.
Problem
When a caller sets a measurement like candidateCount = candidates.length, accumulating telemetry sums it across the batch. The resulting total doesn't represent any single event and requires dividing by a separate count to derive a meaningful average — which is fragile and error-prone.
We encountered this in PR #635 where candidateCount was removed from shell.completionList because the accumulation semantics made it unusable.
Proposed Enhancement
Extend callWithAccumulatingTelemetry to support distribution-style measurements that compute min, max, sum, count (and optionally median/p50/p95) across the batch, then emit them as separate measurement keys in the flushed event.
Example API sketch
void callWithAccumulatingTelemetry('shell.completionList', (ctx) => {
ctx.telemetry.measurements[`shown_${category}`] = 1; // counter (summed)
ctx.telemetry.distributions.candidateCount = candidates.length; // distribution (min/max/avg)
});
Flushed event would include:
candidateCount_min = 2
candidateCount_max = 15
candidateCount_sum = 84
candidateCount_count = 10
Downstream queries can then compute average (sum / count) and see the range, without the ambiguity of a raw summed total.
Scope
- Add a
distributions bag alongside existing measurements on the accumulator context
- Track min, max, sum, count per distribution key during accumulation
- Emit
{key}_min, {key}_max, {key}_sum, {key}_count on flush
- Consider p50/p95 as a future extension (requires storing all values or using approximation algorithms like t-digest)
Summary
callWithAccumulatingTelemetrycurrently supports only counter measurements — values are summed across batched calls. This works well for counting occurrences (e.g.,shown = 1,accepted = 1), but it cannot capture distribution metrics like candidate counts, latencies, or sizes without distorting the data.Problem
When a caller sets a measurement like
candidateCount = candidates.length, accumulating telemetry sums it across the batch. The resulting total doesn't represent any single event and requires dividing by a separate count to derive a meaningful average — which is fragile and error-prone.We encountered this in PR #635 where
candidateCountwas removed fromshell.completionListbecause the accumulation semantics made it unusable.Proposed Enhancement
Extend
callWithAccumulatingTelemetryto support distribution-style measurements that compute min, max, sum, count (and optionally median/p50/p95) across the batch, then emit them as separate measurement keys in the flushed event.Example API sketch
Flushed event would include:
Downstream queries can then compute average (
sum / count) and see the range, without the ambiguity of a raw summed total.Scope
distributionsbag alongside existingmeasurementson the accumulator context{key}_min,{key}_max,{key}_sum,{key}_counton flush