Sam Xie
3429e15b9a
Revert Cleanup interaction of exemplar and aggregation ( #5913 )
...
Topic: #5249
This reverts commit 8041156518ee2f31ec9b36852fdc29f093d0f468 (PR: #5899 )
due to the performance degradation found by Benchmarks CI
https://github.com/open-telemetry/opentelemetry-go/actions/runs/11447364022/job/31848519243
Here is the benchmark test on my machine:
```
goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/sdk/metric
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
Instrument/instrumentImpl/aggregate-10 3.378µ ± 3% 49.366µ ± 1% +1361.40% (p=0.000 n=10)
Instrument/observable/observe-10 2.288µ ± 2% 37.791µ ± 1% +1551.73% (p=0.000 n=10)
geomean 2.780µ 43.19µ +1453.65%
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
Instrument/instrumentImpl/aggregate-10 1.245Ki ± 1% 22.363Ki ± 0% +1696.08% (p=0.000 n=10)
Instrument/observable/observe-10 823.0 ± 1% 17432.5 ± 0% +2018.17% (p=0.000 n=10)
geomean 1.000Ki 19.51Ki +1850.48%
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
Instrument/instrumentImpl/aggregate-10 1.000 ± 0% 21.000 ± 0% +2000.00% (p=0.000 n=10)
Instrument/observable/observe-10 1.000 ± 0% 16.000 ± 0% +1500.00% (p=0.000 n=10)
```
2024-10-23 10:48:07 -07:00
David Ashpole
8041156518
Cleanup interaction of exemplar and aggregation ( #5899 )
...
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`.
2024-10-21 15:37:32 -04:00
Matthieu MOREL
063239fa37
[chore]: enable len and empty rules from testifylint ( #5832 )
...
#### Description
Testifylint is a linter that provides best practices with the use of
testify.
This PR enables
[empty](https://github.com/Antonboom/testifylint?tab=readme-ov-file#empty )
and
[len](https://github.com/Antonboom/testifylint?tab=readme-ov-file#len )
rules from [testifylint](https://github.com/Antonboom/testifylint )
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
2024-09-21 08:04:28 -07:00
Nathan Baulch
506a9baf5e
Fix typos ( #5763 )
2024-09-09 08:53:15 +02:00
David Ashpole
0485de287e
Move time.Now call into exemplar reservoir to improve performance ( #5545 )
...
Part of addressing
https://github.com/open-telemetry/opentelemetry-go/issues/5542 .
### Motivation
This removes the `time.Now()` call from filtered-out Exemplars by only
invoking `time.Now()` after the filtering decision is made. This
improvement is especially noticeable for measurements without any
attributes.
```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/metric
cpu: AMD EPYC 7B12
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
SyncMeasure/NoView/Int64Counter/Attributes/0-24 158.20n ± 4% 99.83n ± 1% -36.90% (p=0.000 n=10)
SyncMeasure/NoView/Int64Counter/Attributes/1-24 333.3n ± 4% 274.8n ± 1% -17.55% (p=0.000 n=10)
SyncMeasure/NoView/Int64Counter/Attributes/10-24 1.640µ ± 1% 1.600µ ± 1% -2.41% (p=0.000 n=10)
SyncMeasure/NoView/Float64Counter/Attributes/0-24 159.0n ± 3% 101.3n ± 0% -36.27% (p=0.000 n=10)
SyncMeasure/NoView/Float64Counter/Attributes/1-24 340.0n ± 2% 272.0n ± 1% -20.00% (p=0.000 n=10)
SyncMeasure/NoView/Float64Counter/Attributes/10-24 1.661µ ± 1% 1.597µ ± 0% -3.85% (p=0.000 n=10)
SyncMeasure/NoView/Int64UpDownCounter/Attributes/0-24 159.8n ± 1% 103.1n ± 0% -35.50% (p=0.000 n=10)
SyncMeasure/NoView/Int64UpDownCounter/Attributes/1-24 339.5n ± 1% 273.1n ± 0% -19.57% (p=0.000 n=10)
SyncMeasure/NoView/Int64UpDownCounter/Attributes/10-24 1.656µ ± 0% 1.589µ ± 0% -4.05% (p=0.000 n=10)
SyncMeasure/NoView/Float64UpDownCounter/Attributes/0-24 159.3n ± 2% 100.8n ± 0% -36.74% (p=0.000 n=10)
SyncMeasure/NoView/Float64UpDownCounter/Attributes/1-24 337.9n ± 2% 271.8n ± 1% -19.55% (p=0.000 n=10)
SyncMeasure/NoView/Float64UpDownCounter/Attributes/10-24 1.657µ ± 0% 1.593µ ± 1% -3.83% (p=0.000 n=10)
SyncMeasure/NoView/Int64Histogram/Attributes/0-24 144.65n ± 4% 89.38n ± 0% -38.21% (p=0.000 n=10)
SyncMeasure/NoView/Int64Histogram/Attributes/1-24 235.7n ± 2% 183.5n ± 0% -22.15% (p=0.000 n=10)
SyncMeasure/NoView/Int64Histogram/Attributes/10-24 900.8n ± 1% 836.8n ± 0% -7.10% (p=0.000 n=10)
SyncMeasure/NoView/Float64Histogram/Attributes/0-24 145.60n ± 5% 93.48n ± 1% -35.80% (p=0.000 n=10)
SyncMeasure/NoView/Float64Histogram/Attributes/1-24 240.9n ± 1% 183.0n ± 0% -24.06% (p=0.000 n=10)
SyncMeasure/NoView/Float64Histogram/Attributes/10-24 905.6n ± 1% 826.3n ± 0% -8.76% (p=0.000 n=10)
SyncMeasure/DropView/Int64Counter/Attributes/0-24 20.33n ± 0% 20.35n ± 0% ~ (p=0.302 n=10)
SyncMeasure/DropView/Int64Counter/Attributes/1-24 26.46n ± 0% 26.45n ± 1% ~ (p=0.868 n=10)
SyncMeasure/DropView/Int64Counter/Attributes/10-24 26.50n ± 0% 26.47n ± 0% ~ (p=0.208 n=10)
SyncMeasure/DropView/Float64Counter/Attributes/0-24 20.34n ± 1% 20.27n ± 0% -0.34% (p=0.009 n=10)
SyncMeasure/DropView/Float64Counter/Attributes/1-24 26.55n ± 0% 26.60n ± 1% ~ (p=0.109 n=10)
SyncMeasure/DropView/Float64Counter/Attributes/10-24 26.59n ± 1% 26.57n ± 1% ~ (p=0.926 n=10)
SyncMeasure/DropView/Int64UpDownCounter/Attributes/0-24 20.38n ± 1% 20.38n ± 0% ~ (p=0.725 n=10)
SyncMeasure/DropView/Int64UpDownCounter/Attributes/1-24 26.39n ± 0% 26.44n ± 0% ~ (p=0.238 n=10)
SyncMeasure/DropView/Int64UpDownCounter/Attributes/10-24 26.52n ± 0% 26.42n ± 0% -0.36% (p=0.049 n=10)
SyncMeasure/DropView/Float64UpDownCounter/Attributes/0-24 20.30n ± 0% 20.25n ± 0% ~ (p=0.196 n=10)
SyncMeasure/DropView/Float64UpDownCounter/Attributes/1-24 26.57n ± 0% 26.54n ± 1% ~ (p=0.540 n=10)
SyncMeasure/DropView/Float64UpDownCounter/Attributes/10-24 26.57n ± 0% 26.51n ± 1% ~ (p=0.643 n=10)
SyncMeasure/DropView/Int64Histogram/Attributes/0-24 20.37n ± 0% 20.36n ± 1% ~ (p=1.000 n=10)
SyncMeasure/DropView/Int64Histogram/Attributes/1-24 26.41n ± 0% 26.50n ± 0% +0.32% (p=0.007 n=10)
SyncMeasure/DropView/Int64Histogram/Attributes/10-24 26.44n ± 0% 26.55n ± 1% +0.42% (p=0.012 n=10)
SyncMeasure/DropView/Float64Histogram/Attributes/0-24 20.30n ± 0% 20.45n ± 0% +0.74% (p=0.000 n=10)
SyncMeasure/DropView/Float64Histogram/Attributes/1-24 26.52n ± 0% 26.48n ± 0% ~ (p=0.127 n=10)
SyncMeasure/DropView/Float64Histogram/Attributes/10-24 26.55n ± 0% 26.48n ± 0% -0.26% (p=0.002 n=10)
SyncMeasure/AttrFilterView/Int64Counter/Attributes/0-24 170.5n ± 2% 110.8n ± 0% -35.03% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Int64Counter/Attributes/1-24 402.5n ± 1% 331.5n ± 1% -17.64% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Int64Counter/Attributes/10-24 1.363µ ± 1% 1.281µ ± 1% -6.02% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Float64Counter/Attributes/0-24 170.6n ± 1% 111.5n ± 1% -34.64% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Float64Counter/Attributes/1-24 397.1n ± 1% 335.9n ± 0% -15.41% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Float64Counter/Attributes/10-24 1.371µ ± 1% 1.279µ ± 1% -6.71% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Int64UpDownCounter/Attributes/0-24 170.1n ± 1% 112.2n ± 0% -34.09% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Int64UpDownCounter/Attributes/1-24 397.5n ± 1% 330.2n ± 0% -16.93% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Int64UpDownCounter/Attributes/10-24 1.371µ ± 1% 1.289µ ± 1% -5.95% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Float64UpDownCounter/Attributes/0-24 171.4n ± 2% 112.9n ± 0% -34.13% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Float64UpDownCounter/Attributes/1-24 397.0n ± 3% 336.4n ± 0% -15.24% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Float64UpDownCounter/Attributes/10-24 1.383µ ± 1% 1.305µ ± 1% -5.61% (p=0.000 n=10)
SyncMeasure/AttrFilterView/Int64Histogram/Attributes/0-24 157.30n ± 2% 98.58n ± 1% -37.33% (p=0.000 n=6+10)
```
### Changes
* Introduce `exemplar.Filter`, which is a filter function based on the
context. It will not be user-facing, so we can always add other
parameters later if needed.
* Introduce `exemplar.FilteredReservoir`, which is similar to a
reservoir, except it does not receive a timestamp. It gets the current
time after the filter decision has been made. It uses generics to avoid
the call to exemplar.NewValue(), since it is internal-only.
* The `exemplar.Reservoir` is left as-is, so that it can be made public
when exemplars are stable. It still includes a timestamp argument.
* Unit tests are updated to expect a much lower number of calls to
time.Now
* `exemplar.Drop` is now an `exemplar.FilteredReservoir` instead of a
`Reservoir`, since we don't need a Reservoir to store things in if the
measurement is always dropped.
Co-authored-by: Sam Xie <sam@samxie.me>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
2024-07-01 09:36:11 -07:00
Tyler Yahn
9e7d7446c3
Test metric aggregate times ( #5323 )
2024-05-09 20:11:32 +02:00
Tyler Yahn
fe9bab54b7
Use Distinct instead of Set for map keys ( #5027 )
2024-03-06 11:11:16 +01:00
Robert Pająk
7dea232a46
[chore] Simplify the license header ( #4987 )
2024-02-29 07:05:28 +01:00
Tyler Yahn
fecb92e366
Add the experimental exemplar feature ( #4871 )
...
* 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
2024-01-31 13:15:35 -08:00
Tyler Yahn
e3bf787c21
Add cardinality limiting to the metric SDK as an experimental feature ( #4457 )
...
* 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>
2023-12-19 07:53:01 -08:00
Tyler Yahn
3904523917
Flatten sdk/metric/aggregation into sdk/metric ( #4435 )
...
* 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>
2023-08-14 08:15:15 -07:00
Tyler Yahn
859a87098c
Simplify the histogram implementation ( #4370 )
2023-07-27 14:06:22 -07:00
Tyler Yahn
f194fb0c6c
Allow histogram for all instruments ( #4332 )
...
* 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
2023-07-19 07:12:00 -07:00
Tyler Yahn
d18f20179e
Replace internal aggregate Aggregator with Measure/ComputeAggregation and a Builder ( #4304 )
2023-07-17 16:15:50 +02:00
Tyler Yahn
10c3445543
Move aggs to internal/aggregate ( #4283 )
2023-07-03 10:53:00 +02:00
