Skip to content

Use sync.Map and atomics for lastvalue aggregations#7478

Merged
dmathieu merged 3 commits intoopen-telemetry:mainfrom
dashpole:optimize_lastvalue
Dec 16, 2025
Merged

Use sync.Map and atomics for lastvalue aggregations#7478
dmathieu merged 3 commits intoopen-telemetry:mainfrom
dashpole:optimize_lastvalue

Conversation

@dashpole
Copy link
Copy Markdown
Contributor

@dashpole dashpole commented Oct 8, 2025

Depends on #7474

This applies similar optimizations as #7427 to the last value aggregation.

Changes for last value are contained in 27e1482.

Parallel benchmarks:

                                                                   │   main.txt   │               lv.txt               │
                                                                   │    sec/op    │   sec/op     vs base               │
SyncMeasure/NoView/ExemplarsDisabled/Int64Gauge/Attributes/10-24     264.60n ± 3%   66.46n ± 1%  -74.88% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsDisabled/Float64Gauge/Attributes/10-24   270.25n ± 4%   69.69n ± 1%  -74.21% (p=0.002 n=6)
geomean                                                               267.4n        68.05n       -74.55%

@dashpole dashpole force-pushed the optimize_lastvalue branch from f6c58df to 27e1482 Compare October 8, 2025 19:23
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.2%. Comparing base (19a5a68) to head (a97f113).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #7478   +/-   ##
=====================================
  Coverage   86.2%   86.2%           
=====================================
  Files        302     302           
  Lines      21971   21985   +14     
=====================================
+ Hits       18950   18963   +13     
- Misses      2640    2641    +1     
  Partials     381     381           
Files with missing lines Coverage Δ
sdk/metric/internal/aggregate/aggregate.go 100.0% <100.0%> (ø)
sdk/metric/internal/aggregate/lastvalue.go 100.0% <100.0%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dashpole dashpole marked this pull request as ready for review December 11, 2025 18:55
@dashpole
Copy link
Copy Markdown
Contributor Author

@MrAlias @dmathieu this is ready for review. It follows the same pattern as the previous sum and histogram PRs, but should be much simpler.

@dmathieu dmathieu merged commit 68d2f72 into open-telemetry:main Dec 16, 2025
33 checks passed
@dashpole
Copy link
Copy Markdown
Contributor Author

@dmathieu I think this needed another approval prior to merging.

@MrAlias should I revert this to give you time to review?

@dmathieu
Copy link
Copy Markdown
Member

Urgh sorry, I missed the lack of second approval.

@MrAlias
Copy link
Copy Markdown
Contributor

MrAlias commented Dec 16, 2025

I was about halfway though this. I can take another look and see.

Copy link
Copy Markdown
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good. Minor feedback on comments and the return value.

) int {
// Ignore if dest is not a metricdata.Gauge. The chance for memory reuse of
// the DataPoints is missed (better luck next time).
// the lastValuePoints is missed (better luck next time).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// the lastValuePoints is missed (better luck next time).
// the DataPoints is missed (better luck next time).

This is still referring to the dest field.

s.hotColdValMap[readIdx].values.Clear()
*dest = gData

return n
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return n
return i

If range for some reason does not iterate over n values, this will be incorrect.


func newLastValue[N int64 | float64](limit int, r func(attribute.Set) FilteredExemplarReservoir[N]) *lastValue[N] {
return &lastValue[N]{
// lastValue summarizes a set of measurements as the last one made.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// lastValue summarizes a set of measurements as the last one made.
// lastValueMap summarizes a set of measurements as the last one made.

@dashpole
Copy link
Copy Markdown
Contributor Author

@MrAlias are you OK with a follow-up PR?

@MrAlias
Copy link
Copy Markdown
Contributor

MrAlias commented Dec 16, 2025

@MrAlias are you OK with a follow-up PR?

Yeah 👍

dashpole added a commit to dashpole/opentelemetry-go that referenced this pull request Dec 16, 2025
@dashpole dashpole deleted the optimize_lastvalue branch December 16, 2025 16:48
@dashpole
Copy link
Copy Markdown
Contributor Author

#7718

dashpole added a commit that referenced this pull request Dec 16, 2025
Addresses
#7478 (review),
which was left after the PR merged.
dashpole added a commit to dashpole/opentelemetry-go that referenced this pull request Dec 17, 2025
@MrAlias MrAlias mentioned this pull request Jan 16, 2026
39 tasks
@MrAlias MrAlias mentioned this pull request Feb 2, 2026
MrAlias added a commit that referenced this pull request Feb 2, 2026
### Added

- Add `Enabled` method to all synchronous instrument interfaces
(`Float64Counter`, `Float64UpDownCounter`, `Float64Histogram`,
`Float64Gauge`, `Int64Counter`, `Int64UpDownCounter`, `Int64Histogram`,
`Int64Gauge`,) in `go.opentelemetry.io/otel/metric`. This stabilizes the
synchronous instrument enabled feature, allowing users to check if an
instrument will process measurements before performing computationally
expensive operations. (#7763)
- Add `AlwaysRecord` sampler in `go.opentelemetry.io/otel/sdk/trace`.
(#7724)
- Add `go.opentelemetry.io/otel/semconv/v1.39.0` package. The package
contains semantic conventions from the `v1.39.0` version of the
OpenTelemetry Semantic Conventions. See the [migration
documentation](https://github.com/open-telemetry/opentelemetry-go/blob/298cbedf256b7a9ab3c21e41fc5e3e6d6e4e94aa/semconv/v1.39.0/MIGRATION.md)
for information on how to upgrade from
`go.opentelemetry.io/otel/semconv/v1.38.0.` (#7783, #7789)

### Changed

- `Exporter` in `go.opentelemetry.io/otel/exporter/prometheus` ignores
metrics with the scope `go.opentelemetry.io/contrib/bridges/prometheus`.
This prevents scrape failures when the Prometheus exporter is
misconfigured to get data from the Prometheus bridge. (#7688)
- Improve performance of concurrent histogram measurements in
`go.opentelemetry.io/otel/sdk/metric`. (#7474)
- Add experimental observability metrics in
`go.opentelemetry.io/otel/exporters/stdout/stdoutmetric`. (#7492)
- Improve the concurrent performance of `HistogramReservoir` in
`go.opentelemetry.io/otel/sdk/metric/exemplar` by 4x. (#7443)
- Improve performance of concurrent synchronous gauge measurements in
`go.opentelemetry.io/otel/sdk/metric`. (#7478)
- Improve performance of concurrent exponential histogram measurements
in `go.opentelemetry.io/otel/sdk/metric`. (#7702)
- Improve the concurrent performance of `FixedSizeReservoir` in
`go.opentelemetry.io/otel/sdk/metric/exemplar`. (#7447)
- The `rpc.grpc.status_code` attribute in the experimental metrics
emitted from
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc` is
replaced with the `rpc.response.status_code` attribute to align with the
semantic conventions. (#7854)
- The `rpc.grpc.status_code` attribute in the experimental metrics
emitted from
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` is
replaced with the `rpc.response.status_code` attribute to align with the
semantic conventions. (#7854)

### Fixed

- Fix bad log message when key-value pairs are dropped because of key
duplication in `go.opentelemetry.io/otel/sdk/log`. (#7662)
- Fix `DroppedAttributes` on `Record` in
`go.opentelemetry.io/otel/sdk/log` to not count the non-attribute
key-value pairs dropped because of key duplication. (#7662)
- Fix `SetAttributes` on `Record` in `go.opentelemetry.io/otel/sdk/log`
to not log that attributes are dropped when they are actually not
dropped. (#7662)
- `WithHostID` detector in `go.opentelemetry.io/otel/sdk/resource` to
use full path for `ioreg` command on Darwin (macOS). (#7818)
- Fix missing `request.GetBody` in
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp` to
correctly handle HTTP2 GOAWAY frame. (#7794)

### Deprecated

- Deprecate `go.opentelemetry.io/otel/exporters/zipkin`. For more
information, see the [OTel blog post deprecating the Zipkin
exporter](https://opentelemetry.io/blog/2025/deprecating-zipkin-exporters/).
(#7670)

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants