feat: remove HistogramAggregator.reset

[legendecas]

Snapshot of spec draft: open-telemetry/opentelemetry-specification#347

The Aggregator interface supports combining multiple metric events
into a single aggregated state. Different concrete aggregator types
provide different functionality and levels of concurrent performance.

Aggregators MUST support Update(), Checkpoint(), and Merge()
operations. Update() is called directly from the Meter in response
to a metric event, and may be called concurrently. Update() is also
passed the caller's distributed context, which allows it to access the
current span context and distributed correlations, however none of the
built-in aggregators use span context or distributed correlations.

The Checkpoint() operation is called to atomically save a snapshot
of the Aggregator, since Checkpoint() may be called concurrently
with Update(). The Merge() operation supports dimensionality
reduction by combining state from multiple aggregators into a single
Aggregator state.

Which problem is this PR solving?

• HistogramAggregator not been updated with merely HistogramAggregator.update(value) operation.
• HistogramAggregator is not aligned with other aggregators regarding the API semantics.

Short description of the changes

• Removed HistogramAggregator.reset() as it no longer exists in spec draft.
• HistogramAggregator.update(value) will directly update the current state and can be reflected by Histogram.toPoint.

[codecov]

Codecov Report

Merging #1292 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1292      +/-   ##
==========================================
- Coverage   92.45%   92.44%   -0.01%     
==========================================
  Files         183      183              
  Lines        4570     4565       -5     
  Branches      949      949              
==========================================
- Hits         4225     4220       -5     
  Misses        345      345              

Impacted Files	Coverage Δ
...emetry-metrics/src/export/aggregators/histogram.ts
...emetry-metrics/src/export/aggregators/Histogram.ts	100.00% <0.00%> (ø)

[legendecas]

No idea why the codecov is complaining about the coverage diff. By the first impression, this PR isn't expected to affect the coverage if I understand correctly. 🤔

[legendecas]

So previously histogram tests were not running at all for the file name not matching the *.test.ts pattern. Fixed.

[legendecas]

@open-telemetry/javascript-approvers hey, I'd like to raise my hand for this PR. This PR has been open for 8 days and been approved by two of @open-telemetry/javascript-approvers and @open-telemetry/javascript-maintainers. Do we have any guidelines on how many open days of one approved PR has to meet? Like say, a PR opened for at least two days long and been approved by @open-telemetry/javascript-approvers and @open-telemetry/javascript-maintainers can safely land.

[mayurkale22]

LGTM, sorry for the delay.

[mayurkale22]

@open-telemetry/javascript-approvers hey, I'd like to raise my hand for this PR. This PR has been open for 8 days and been approved by two of @open-telemetry/javascript-approvers and @open-telemetry/javascript-maintainers. Do we have any guidelines on how many open days of one approved PR has to meet? Like say, a PR opened for at least two days long and been approved by @open-telemetry/javascript-approvers and @open-telemetry/javascript-maintainers can safely land.

Apologies in the delay, this slipped through the cracks. Will wait for @obecny's approval before merging the PR. @dyladan WDYT?

[dyladan]

Currently the contributing guidelines state 4 approvals including "both" maintainers (now we have 3). I am currently thinking of "both" maintainers wording as "at least 2 of 3 maintainers". I would say 3 approvals is enough if those 3 are all maintainers or if the PR is small. I would prefer one more review on this.

[legendecas]

As @dyladan suggested, it will be great to have another review from @open-telemetry/javascript-maintainers and @open-telemetry/javascript-approvers. Many thanks :)

[dyladan]

@legendecas no need to tag both maintainers and approvers since maintainers is a subset of approvers :)

[obecny]

lgtm, one question

[obecny]

why is this not needed ?

[legendecas]

Aggregators are expected to update the current checkpoint on each aggregator.update call. This case asserts the original HistogramAggregator update the checkpoint on histogramAggregator.reset -- this is not compliant with the current spec draft and not aligned with other existing aggregators.