Add PoC implementation of Sum aggregator #2973
Conversation
Separate the duties of aggregation and maintaining state across aggregation periods.
Add back when filling in structures.
This is an implementation detail.
MrAlias
added
pkg:SDK
Codecov Report
@@ Coverage Diff @@
## new_sdk/main #2973 +/- ##
==============================================
+ Coverage 74.0% 74.5% +0.4%
==============================================
Files 114 125 +11
Lines 7930 8419 +489
==============================================
+ Hits 5874 6278 +404
- Misses 1921 2002 +81
- Partials 135 139 +4
|
sdk/metric/internal/sum.go
Outdated
| type sumAgg[N int64 | float64] struct { | ||
| // zero value used for the base of all new sums. | ||
| newFunc NewAtomicFunc[N] | ||
|
|
||
| // map[attribute.Set]Atomic[N] | ||
| current sync.Map | ||
| } |
There was a problem hiding this comment.
I think we are going to have better results if we just use a single lock over two atomic operations (both a load from the map, and a load from the register). I will put together a branch that would show this off.
| type sumAgg[N int64 | float64] struct { | |
| // zero value used for the base of all new sums. | |
| newFunc NewAtomicFunc[N] | |
| // map[attribute.Set]Atomic[N] | |
| current sync.Map | |
| } | |
| type sumAgg[N int64 | float64] struct { | |
| mu sync.Mutex | |
| values map[*attribute.Set]N | |
| } |
There was a problem hiding this comment.
Here is what the change looks like: https://github.com/MadVikingGod/opentelemetry-go/blob/mvg/new_sdk/impl-sum/sdk/metric/internal/sum.go
Also check out the benchmarks I added.
Results: https://gist.github.com/MadVikingGod/1d48ff07ee87e3c66acdc33b895d6b3d
Changed to a simpler sum.
|
Will recreate based on the merged form of #2954 |
Based on #2954
Resolves #2972