Skip to content

prometheus: use a sync.Pool instead of allocating metricdata.ResourceMetrics in Collect #6472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Mar 20, 2025

Conversation

yumosx
Copy link
Contributor

@yumosx yumosx commented Mar 19, 2025

Fix #3047

Use a sync.Pool instead of allocating metricdata.ResourceMetrics in go.opentelemetry.io/otel/exporters/prometheus

goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/exporters/prometheus
cpu: Apple M3
               │ before.txt  │             after.txt             │
               │   sec/op    │   sec/op     vs base              │
Collect1-8       8.273µ ± 0%   8.013µ ± 5%       ~ (p=0.065 n=6)
Collect10-8      23.84µ ± 0%   22.40µ ± 3%  -6.02% (p=0.002 n=6)
Collect100-8     148.0µ ± 0%   139.4µ ± 0%  -5.84% (p=0.002 n=6)
Collect1000-8    1.326m ± 0%   1.244m ± 1%  -6.14% (p=0.002 n=6)
Collect10000-8   15.90m ± 1%   14.51m ± 1%  -8.77% (p=0.002 n=6)
geomean          227.9µ        214.3µ       -6.00%

               │  before.txt  │              after.txt              │
               │     B/op     │     B/op      vs base               │
Collect1-8       35.65Ki ± 0%   34.49Ki ± 0%   -3.24% (p=0.002 n=6)
Collect10-8      55.78Ki ± 0%   45.57Ki ± 0%  -18.31% (p=0.002 n=6)
Collect100-8     260.8Ki ± 0%   174.1Ki ± 0%  -33.23% (p=0.002 n=6)
Collect1000-8    2.307Mi ± 0%   1.537Mi ± 1%  -33.38% (p=0.002 n=6)
Collect10000-8   22.47Mi ± 0%   14.75Mi ± 4%  -34.36% (p=0.002 n=6)
geomean          489.8Ki        365.3Ki       -25.42%

               │ before.txt  │             after.txt             │
               │  allocs/op  │  allocs/op   vs base              │
Collect1-8        72.00 ± 0%    66.00 ± 0%  -8.33% (p=0.002 n=6)
Collect10-8       398.0 ± 0%    375.0 ± 0%  -5.78% (p=0.002 n=6)
Collect100-8     3.662k ± 0%   3.491k ± 0%  -4.67% (p=0.002 n=6)
Collect1000-8    36.15k ± 0%   34.62k ± 0%  -4.24% (p=0.002 n=6)
Collect10000-8   361.0k ± 0%   345.7k ± 0%  -4.25% (p=0.002 n=6)
geomean          4.239k        4.008k       -5.47%

Copy link

codecov bot commented Mar 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (1b8fe16) to head (5704970).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #6472   +/-   ##
=====================================
  Coverage   81.8%   81.8%           
=====================================
  Files        283     283           
  Lines      24910   24913    +3     
=====================================
+ Hits       20384   20387    +3     
  Misses      4121    4121           
  Partials     405     405           

see 1 file with indirect coverage changes

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

@yumosx yumosx changed the title exporters: prometheus: use a sync.Pool instead of allocating metrics in collect. Fix(#3047): use a sync.Pool instead of allocating metricdata.ResourceMetrics in collect. Mar 19, 2025
@pellared pellared changed the title Fix(#3047): use a sync.Pool instead of allocating metricdata.ResourceMetrics in collect. prometheus: use a sync.Pool instead of allocating metricdata.ResourceMetrics in Collect Mar 19, 2025
@pellared pellared requested a review from MrAlias March 20, 2025 14:12
Copy link
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.

Thanks for the contribution.

@MrAlias MrAlias added this to the v1.36.0 milestone Mar 20, 2025
@pellared pellared merged commit f556bd3 into open-telemetry:main Mar 20, 2025
32 checks passed
dmathieu added a commit that referenced this pull request May 21, 2025
# Overview

Closes #6786

### Added

- Add exponential histogram support in
`go.opentelemetry.io/otel/exporters/prometheus`. (#6421)
- The `go.opentelemetry.io/otel/semconv/v1.31.0` package.
The package contains semantic conventions from the `v1.31.0` version of
the OpenTelemetry Semantic Conventions.
See the [migration documentation](./semconv/v1.31.0/MIGRATION.md) for
information on how to upgrade from
`go.opentelemetry.io/otel/semconv/v1.30.0`. (#6479)
- Add `Recording`, `Scope`, and `Record` types in
`go.opentelemetry.io/otel/log/logtest`. (#6507)
- Add `WithHTTPClient` option to configure the `http.Client` used by
`go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`.
(#6751)
- Add `WithHTTPClient` option to configure the `http.Client` used by
`go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`.
(#6752)
- Add `WithHTTPClient` option to configure the `http.Client` used by
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#6688)
- Add `ValuesGetter` in `go.opentelemetry.io/otel/propagation`, a
`TextMapCarrier` that supports retrieving multiple values for a single
key. (#5973)
- Add `Values` method to `HeaderCarrier` to implement the new
`ValuesGetter` interface in `go.opentelemetry.io/otel/propagation`.
(#5973)
- Update `Baggage` in `go.opentelemetry.io/otel/propagation` to retrieve
multiple values for a key when the carrier implements `ValuesGetter`.
(#5973)
- Add `AssertEqual` function in `go.opentelemetry.io/otel/log/logtest`.
(#6662)
- The `go.opentelemetry.io/otel/semconv/v1.32.0` package.
The package contains semantic conventions from the `v1.32.0` version of
the OpenTelemetry Semantic Conventions.
See the [migration documentation](./semconv/v1.32.0/MIGRATION.md) for
information on how to upgrade from
`go.opentelemetry.io/otel/semconv/v1.31.0`(#6782)
- Add `Transform` option in `go.opentelemetry.io/otel/log/logtest`.
(#6794)
- Add `Desc` option in `go.opentelemetry.io/otel/log/logtest`. (#6796)

### Removed

- Drop support for [Go 1.22]. (#6381, #6418)
- Remove `Resource` field from `EnabledParameters` in
`go.opentelemetry.io/otel/sdk/log`. (#6494)
- Remove `RecordFactory` type from
`go.opentelemetry.io/otel/log/logtest`. (#6492)
- Remove `ScopeRecords`, `EmittedRecord`, and `RecordFactory` types from
`go.opentelemetry.io/otel/log/logtest`. (#6507)
- Remove `AssertRecordEqual` function in
`go.opentelemetry.io/otel/log/logtest`, use `AssertEqual` instead.
(#6662)

### Changed

- ⚠️ Update `github.com/prometheus/client_golang` to `v1.21.1`, which
changes the `NameValidationScheme` to `UTF8Validation`.
This allows metrics names to keep original delimiters (e.g. `.`), rather
than replacing with underscores.
This can be reverted by setting
`github.com/prometheus/common/model.NameValidationScheme` to
`LegacyValidation` in `github.com/prometheus/common/model`. (#6433)
- Initialize map with `len(keys)` in `NewAllowKeysFilter` and
`NewDenyKeysFilter` to avoid unnecessary allocations in
`go.opentelemetry.io/otel/attribute`. (#6455)
- `go.opentelemetry.io/otel/log/logtest` is now a separate Go module.
(#6465)
- `go.opentelemetry.io/otel/sdk/log/logtest` is now a separate Go
module. (#6466)
- `Recorder` in `go.opentelemetry.io/otel/log/logtest` no longer
separately stores records emitted by loggers with the same
instrumentation scope. (#6507)
- Improve performance of `BatchProcessor` in
`go.opentelemetry.io/otel/sdk/log` by not exporting when exporter cannot
accept more. (#6569, #6641)

### Deprecated

- Deprecate support for `model.LegacyValidation` for
`go.opentelemetry.io/otel/exporters/prometheus`. (#6449)

### Fixes

- Stop percent encoding header environment variables in
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` and
`go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#6392)
- Ensure the `noopSpan.tracerProvider` method is not inlined in
`go.opentelemetry.io/otel/trace` so the `go.opentelemetry.io/auto`
instrumentation can instrument non-recording spans. (#6456)
- Use a `sync.Pool` instead of allocating `metricdata.ResourceMetrics`
in `go.opentelemetry.io/otel/exporters/prometheus`. (#6472)

---------

Co-authored-by: Tyler Yahn <[email protected]>
Co-authored-by: Robert Pająk <[email protected]>
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.

Optimize the memory allocations of the cumulative histogram
4 participants