Skip to content

Optimize fixedsize reservoir#7447

Merged
dashpole merged 15 commits intoopen-telemetry:mainfrom
dashpole:optimize_fixedsize_reservoir
Jan 23, 2026
Merged

Optimize fixedsize reservoir#7447
dashpole merged 15 commits intoopen-telemetry:mainfrom
dashpole:optimize_fixedsize_reservoir

Conversation

@dashpole
Copy link
Copy Markdown
Contributor

@dashpole dashpole commented Oct 2, 2025

Depends on #7441, #7443

This improves the concurrent performance of the fixed size reservoir's Offer function by 4x (i.e. 75% reduction). This improves the performance of Measure() for fixed-size reservoirs by 60% overall.

Accomplish this by:

  • using a single atomic for count and next. This assumes that both can fit in a uint32.
  • only use a lock to guard changing w and next together.

Offer benchmarks:

                           │   main.txt   │           fixedsize.txt            │
                           │    sec/op    │   sec/op     vs base               │
FixedSizeReservoirOffer-24   185.25n ± 4%   45.58n ± 1%  -75.40% (p=0.002 n=6)

Measure benchmarks:

                                                                          │   main.txt   │            fixedsize.txt            │
                                                                          │    sec/op    │    sec/op     vs base               │
SyncMeasure/NoView/ExemplarsEnabled/Int64Counter/Attributes/0-24            175.45n ± 6%   67.01n ±  9%  -61.81% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64Counter/Attributes/1-24            170.25n ± 1%   69.82n ±  6%  -58.99% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64Counter/Attributes/10-24           167.40n ± 2%   64.52n ± 10%  -61.46% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64Counter/Attributes/0-24          173.55n ± 0%   69.17n ± 12%  -60.14% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64Counter/Attributes/1-24          169.50n ± 1%   68.55n ±  5%  -59.56% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64Counter/Attributes/10-24         166.95n ± 1%   65.82n ±  6%  -60.58% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64UpDownCounter/Attributes/0-24      168.85n ± 1%   67.99n ± 11%  -59.73% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64UpDownCounter/Attributes/1-24      173.50n ± 1%   66.69n ±  2%  -61.56% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64UpDownCounter/Attributes/10-24     171.30n ± 5%   67.73n ±  8%  -60.46% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64UpDownCounter/Attributes/0-24    168.90n ± 2%   67.69n ±  9%  -59.92% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64UpDownCounter/Attributes/1-24    173.35n ± 2%   68.25n ±  9%  -60.63% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64UpDownCounter/Attributes/10-24   172.95n ± 2%   70.90n ±  7%  -59.01% (p=0.002 n=6)
geomean                                                                      171.0n        67.83n        -60.33%

@dashpole dashpole force-pushed the optimize_fixedsize_reservoir branch from bf5b8be to 7705d99 Compare October 2, 2025 16:37
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.7%. Comparing base (207b0c5) to head (198c82e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
sdk/metric/exemplar/fixed_size_reservoir.go 91.4% 2 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #7447   +/-   ##
=====================================
  Coverage   81.7%   81.7%           
=====================================
  Files        304     304           
  Lines      23235   23252   +17     
=====================================
+ Hits       18996   19014   +18     
+ Misses      3858    3857    -1     
  Partials     381     381           
Files with missing lines Coverage Δ
sdk/metric/exemplar/fixed_size_reservoir.go 92.7% <91.4%> (-2.1%) ⬇️

... and 3 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 force-pushed the optimize_fixedsize_reservoir branch 4 times, most recently from 906bb67 to a894b33 Compare October 6, 2025 14:42
@dashpole dashpole force-pushed the optimize_fixedsize_reservoir branch 3 times, most recently from 5daf244 to cb4d407 Compare December 15, 2025 18:43
@dashpole dashpole marked this pull request as ready for review December 15, 2025 18:44
@dashpole
Copy link
Copy Markdown
Contributor Author

@MrAlias @dmathieu this is ready for review!

Comment thread sdk/metric/exemplar/fixed_size_reservoir.go Outdated
Comment thread sdk/metric/exemplar/fixed_size_reservoir.go Outdated
Comment thread sdk/metric/exemplar/fixed_size_reservoir.go
Comment thread sdk/metric/exemplar/fixed_size_reservoir.go Outdated
Comment thread sdk/metric/exemplar/fixed_size_reservoir.go Outdated
Comment thread sdk/metric/exemplar/fixed_size_reservoir.go
Comment thread sdk/metric/exemplar/fixed_size_reservoir_test.go
Comment thread sdk/metric/exemplar/fixed_size_reservoir.go Outdated
Comment thread sdk/metric/exemplar/fixed_size_reservoir.go Outdated
@dashpole dashpole force-pushed the optimize_fixedsize_reservoir branch 2 times, most recently from 1f2e8e0 to 22944a9 Compare December 17, 2025 19:33
@MrAlias MrAlias added this to the v1.40.0 milestone Jan 8, 2026
@pellared pellared requested a review from MrAlias January 14, 2026 15:21
Copy link
Copy Markdown
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

I need a little more time to check if this is concurrent-safe. At quick glance it look it is

Comment thread sdk/metric/exemplar/fixed_size_reservoir.go
Comment thread CHANGELOG.md Outdated
Copy link
Copy Markdown
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

I think that there is a "logical race condition". Please double-check as I am tired and it is easy to get wrong when analyzing concurrency safety 😉

Comment thread sdk/metric/exemplar/fixed_size_reservoir.go
@dashpole dashpole force-pushed the optimize_fixedsize_reservoir branch 2 times, most recently from 5bc5bd1 to 3361643 Compare January 16, 2026 02:53
@dashpole dashpole requested a review from pellared January 16, 2026 02:56
@MrAlias MrAlias mentioned this pull request Jan 16, 2026
39 tasks
@dashpole dashpole requested a review from dmathieu January 16, 2026 18:30
@dashpole dashpole force-pushed the optimize_fixedsize_reservoir branch from c6fcdfc to c939977 Compare January 21, 2026 16:19
@dashpole dashpole closed this Jan 21, 2026
@dashpole dashpole reopened this Jan 21, 2026
@dashpole
Copy link
Copy Markdown
Contributor Author

Looks like gocovmerge is failing to merge coverage profiles...

@dashpole dashpole force-pushed the optimize_fixedsize_reservoir branch from 2ca359d to f1a8002 Compare January 21, 2026 19:57
@dashpole dashpole merged commit 714ca7c into open-telemetry:main Jan 23, 2026
33 checks passed
@dashpole dashpole deleted the optimize_fixedsize_reservoir branch January 23, 2026 14:27
@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.

4 participants