Follow-up to
https://github.com/open-telemetry/opentelemetry-go/pull/5861. This is an
attempt to:
* Limit the API surface of the aggregate package
* Try to use predefined types (e.g. exemplar.Filter) over custom
functions where possible.
* Avoid using nil, and use No-Ops where it makes sense
This makes `aggregate.NewFilteredExemplarReservoir` no longer exported,
removes the `aggregate.FilteredExemplarReservoir` interface, and removes
the `aggregate.dropReservoir`.
Resolve https://github.com/open-telemetry/opentelemetry-go/issues/5249
### Spec
> exemplar_reservoir: A functional type that generates an exemplar
reservoir a MeterProvider will use when storing exemplars. This
functional type needs to be a factory or callback similar to aggregation
selection functionality which allows different reservoirs to be chosen
by the aggregation.
> Users can provide an exemplar_reservoir, but it is up to their
discretion. Therefore, the stream configuration parameter needs to be
structured to accept an exemplar_reservoir, but MUST NOT obligate a user
to provide one. If the user does not provide an exemplar_reservoir
value, the MeterProvider MUST apply a [default exemplar
reservoir](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exemplar-defaults).
Also,
> the reservoir MUST be given the Attributes associated with its
timeseries point either at construction so that additional sampling
performed by the reservoir has access to all attributes from a
measurement in the "offer" method.
### Changes
In sdk/metric/exemplar, add:
* `exemplar.ReservoirProvider`
* `exemplar.HistogramReservoirProvider`
* `exemplar.FixedSizeReservoirProvider`
In sdk/metric, add:
* `metric.ExemplarReservoirProviderSelector` (func Aggregation ->
ReservoirProvider)
* `metric.DefaultExemplarReservoirProviderSelector` (our default
implementation)
* `ExemplarReservoirProviderSelector` added to `metric.Stream`
Note: the only required public types are
`metric.ExemplarReservoirProviderSelector` and
`ExemplarReservoirProviderSelector` in `metric.Stream`. Others are for
convenience and readability.
### Alternatives considered
#### Add ExemplarReservoirProvider directly to metric.Stream, instead of
ExemplarReservoirProviderSelector
This would mean users can configure a `func() exemplar.Reservoir`
instead of a `func(Aggregation) func() exemplar.Reservoir`.
I don't think this complies with the statement: `This functional type
needs to be a factory or callback similar to aggregation selection
functionality which allows different reservoirs to be chosen by the
aggregation.`. I'm interpreting "allows different reservoirs to be
chosen by the aggregation" as meaning "allows different reservoirs to be
chosen **based on the** aggregation", rather than meaning that the
aggregation is somehow choosing the reservoir.
### Future work
There is some refactoring I plan to do after this to simplify the
interaction between the internal/aggregate and exemplar package. I've
omitted that from this PR to keep the diff smaller.
---------
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Part of https://github.com/open-telemetry/opentelemetry-go/issues/5249
This makes all existing types designed to implement the public Exemplar
API public by moving most of `internal/exemplar` to `exemplar`. The only
types that are not being made public are `exemplar.Drop`, and
`exemplar.FilteredReservoir`. Those types are moved to
`internal/aggregate`, and are renamed to `DropReservoir` and
`FilteredExemplarReservoir`.
The following types are made public:
* `exemplar.Exemplar`
* `exemplar.Filter`
* `exemplar.SampledFilter`
* `exemplar.AlwaysOnFilter`
* `exemplar.HistogramReservoir`
* `exemplar.FixedSizeReservoir`
* `exemplar.Reservoir`
* `exemplar.Value`
* `exemplar.ValueType`
* Refactor exemplars to not use generic argument
* Update internal/aggregate
* Update metric SDK
* Test exemplar value type
* Add TestCollectExemplars
* Fix lint
---------
Co-authored-by: Sam Xie <sam@samxie.me>
* Add the experimental exemplar feature
* Add exemplars to EXPERIMENTAL.md
* Add changelog entry
* Fix hist buckets > 1 detection
* Collect instead of Flush res about to be deleted
* Add e2e test
* Do not pre-alloc ResourceMetrics
This only has a single use.
* Fix grammatical error in comment
* Add test cases
Default and invalid OTEL_METRICS_EXEMPLAR_FILTER.
Test sampled and non-sampled context for trace_based.
* Comment nCPU
* Doc OTEL_METRICS_EXEMPLAR_FILTER
* Add agg limiting func
* Add unit test for limitAttr
* Add limiting to aggregate types
* Add internal x pkg for experimental feature-flagging
* Connect cardinality limit to metric SDK
* Replace limitAttr fn with limiter type
The Attribute method is still inlinable.
* Use x.CardinalityLimit directly
* Simplify limiter test
* Add limiter benchmark
* Document the AggregationLimit field
* Test sum limits
* Test limit for last value
* Test histogram limit
* Refactor expo hist test to use existing fixtures
The tests for the exponential histogram create their own testing
fixtures. There is nothing these new fixtures do that cannot already be
done with the existing testing fixtures used by all the other aggregate
functions. Unify the exponential histogram testing to use the existing
fixtures.
* Test the ExponentialHistogram limit
* Fix lint
* Add docs
* Rename aggregation field to aggLimit
---------
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Deprecate the aggregation pkg
* Decouple the internal/aggregate from aggregation pkg
* Add Aggregation to the metric pkg
* Do not use sdk/metric/aggregation in stdoutmetric exporter
* Update all generated templates
* Update prom exporter
* Fix view example
* Add changes to changelog
* Update CHANGELOG.md
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Rename Sum to AggregationSum
* Fix comments
* Centralize validation of aggregation in pipeline
* Remove validation of agg in manual_reader selector opt
* Fix merge
---------
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Allow histogram for all instruments
Any instrument that can record negative values, do not include a sum in
the produced aggregation (like the specification recommends).
Resolves#4161
* Add changes to changelog
* Fix TestBucketsSum
