Replace internal aggregate Aggregator with Measure/ComputeAggregation and a Builder

[MrAlias]

The Aggregator interface is returned from the aggregate package for all aggregate functions. The methods of this returned interface are never used by the SDK at the same place, and in some places it is deliberately split apart to avoid the generic parameter.

Instead of having this mismatch, just have the aggregate function return input and output functions to satisfy the SDKs needs.

Additionally, as this change would require a considerable update to the aggregator constructors, replace these constructors with a Builder introduced in #4220

Part of #4220

[codecov]

Codecov Report

Merging #4304 (8826a13) into main (fdbcb9a) will increase coverage by 0.0%.
The diff coverage is 97.2%.

❗ Current head 8826a13 differs from pull request most recent head dbae8f9. Consider uploading reports for the commit dbae8f9 to get more accurate results

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #4304   +/-   ##
=====================================
  Coverage   83.6%   83.7%           
=====================================
  Files        183     184    +1     
  Lines      14227   14269   +42     
=====================================
+ Hits       11903   11948   +45     
+ Misses      2098    2095    -3     
  Partials     226     226           

Impacted Files	Coverage Δ
sdk/metric/pipeline.go	92.7% <92.3%> (+1.2%)	⬆️
sdk/metric/instrument.go	91.7% <100.0%> (ø)
sdk/metric/internal/aggregate/aggregate.go	100.0% <100.0%> (ø)
sdk/metric/internal/aggregate/filter.go	100.0% <100.0%> (ø)
sdk/metric/internal/aggregate/histogram.go	100.0% <100.0%> (ø)
sdk/metric/internal/aggregate/lastvalue.go	100.0% <100.0%> (ø)
sdk/metric/internal/aggregate/sum.go	100.0% <100.0%> (ø)
sdk/metric/meter.go	85.9% <100.0%> (ø)

... and 1 file with indirect coverage changes

[dashpole]

I think the split between the interfaces is good, but I found the input/output naming confusing when reading the code that uses it, as I needed to remind myself what the input and output of the aggregate package is. Aggregate takes in measurements as inputs, and outputs an aggregation. Maybe something like InputMeasurement, and OutputAggregation would make more sense?

In particular, seeing out as a variable name on structs is much less clear than aggregator is to me.

[MrAlias]

I think the split between the interfaces is good, but I found the input/output naming confusing when reading the code that uses it, as I needed to remind myself what the input and output of the aggregate package is. Aggregate takes in measurements as inputs, and outputs an aggregation. Maybe something like InputMeasurement, and OutputAggregation would make more sense?

In particular, seeing out as a variable name on structs is much less clear than aggregator is to me.

@dashpole what do you think about Input -> Measure and Ouput -> Collect?

[dashpole]

what do you think about Input -> Measure and Ouput -> Collect?

Measure() works for me. "Collect" is better, and I'm ok with that. I do feel like the function is doing more than just collecting. Something like "Combine" seems more accurate.

[MrAlias]

Would Compute or ComputeAggregation work @dashpole?

I'm a little skeptical about aggregate.ComputeAggregation, but not entirely opposed.

Combine also sounds reasonable to me if you think that better.

[dashpole]

Ha, I like ComputeAggregation most of those :)

[MrAlias]

@dashpole / @pellared updated, PTAL

[pellared]

The names are better 👍