Use sync.Map and atomics for fixed bucket histograms#7474
Use sync.Map and atomics for fixed bucket histograms#7474dashpole merged 20 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7474 +/- ##
======================================
Coverage 86.1% 86.2%
======================================
Files 298 298
Lines 21726 21829 +103
======================================
+ Hits 18722 18818 +96
- Misses 2627 2635 +8
+ Partials 377 376 -1
🚀 New features to boost your workflow:
|
ac02811 to
389d0bd
Compare
34d7502 to
f0b28ca
Compare
45effea to
b18ff3d
Compare
bwplotka
left a comment
There was a problem hiding this comment.
Trying to help a bit with review of this.
Generally looks good, just small things, but also I'm not a maintainer and a bit new to the codebase (I maintain Prometheus client_golang SDK though).
Great work, great to see amazing results! Do you mid benchmarking with allocs too?
Yeah, zero allocs on the Measure() path before and after, so I didn't post it. |
Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
e3c2257 to
219e89e
Compare
Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
|
Not sure why coverage is failing |
I think the changes are just exposing an area where we have had coverage overlap. I think this has more to do with how we merge coverage instead of the changes here. |
|
Likely worth looking into using Go 1.20 coverage directory feature to replace our current approach. |
|
Seems to have resolved itself. Now just links are failing (503s from github due to outage). |
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% ``` Co-authored-by: Damien Mathieu <42@dmathieu.com>
### 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>
Implement a lockless histogram using atomics, and use a sync.Map for attribute access. This improves performance by ~2x.
The design is very similar to #7427, but with one additional change to make the histogram data point itself atomic:
Parallel benchmarks:
zero memory allocations before and after this change for Measure(). Omitted for brevity
Benchmarks for collect:
Collect does get substantially worse, but Measure is expected to be called significantly more often than collect.