Skip to content

Performance improvements for the metricstarttime processor#40998

Merged
atoulme merged 3 commits into
open-telemetry:mainfrom
dashpole:metricstarttime_removeif
Jul 13, 2025
Merged

Performance improvements for the metricstarttime processor#40998
atoulme merged 3 commits into
open-telemetry:mainfrom
dashpole:metricstarttime_removeif

Conversation

@dashpole

Copy link
Copy Markdown
Contributor

Description

The existing implementation copied everything from the previous resourcemetrics to a completely new resourcemetrics. However, the component is marked as mutating data, so this PR just changes it to mutate the batch of metrics in-place. This avoids a lot of copying.

From previous experience caching start times and values, storing attributes and exemplars in a cache is very expensive unless you need them. The second commit adds "minimal copy to" methods to avoid storing attributes and exemplars.

Link to tracking issue

Fixes #39400

Testing

Passes all existing unit tests.

@dashpole dashpole requested a review from a team as a code owner June 30, 2025 18:56
@dashpole dashpole requested a review from ArthurSens June 30, 2025 18:56
@github-actions github-actions Bot requested a review from ridwanmsharif June 30, 2025 18:57
@dashpole dashpole added Skip Changelog PRs that do not require a CHANGELOG.md entry enhancement New feature or request labels Jun 30, 2025
@dashpole dashpole force-pushed the metricstarttime_removeif branch from 0a0396a to c1671ea Compare June 30, 2025 18:58
@dashpole

Copy link
Copy Markdown
Contributor Author

cc @ridwanmsharif

@dashpole dashpole force-pushed the metricstarttime_removeif branch 2 times, most recently from 0296360 to d2e29e7 Compare July 1, 2025 18:01

@ridwanmsharif ridwanmsharif left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome change!

@dashpole dashpole force-pushed the metricstarttime_removeif branch from 994a514 to d653622 Compare July 7, 2025 20:20

@ridwanmsharif ridwanmsharif left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@dashpole dashpole added the ready to merge Code review completed; ready to merge by maintainers label Jul 11, 2025
@atoulme atoulme merged commit 55dade9 into open-telemetry:main Jul 13, 2025
187 checks passed
@github-actions github-actions Bot added this to the next release milestone Jul 13, 2025
Dylan-M pushed a commit to Dylan-M/opentelemetry-collector-contrib that referenced this pull request Aug 5, 2025
…metry#40998)

#### Description

The existing implementation copied everything from the previous
resourcemetrics to a completely new resourcemetrics. However, the
component is marked as mutating data, so this PR just changes it to
mutate the batch of metrics in-place. This avoids a lot of copying.

From previous experience caching start times and values, storing
attributes and exemplars in a cache is very expensive unless you need
them. The second commit adds "minimal copy to" methods to avoid storing
attributes and exemplars.

#### Link to tracking issue
Fixes
open-telemetry#39400

#### Testing

Passes all existing unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request processor/metricstarttime ready to merge Code review completed; ready to merge by maintainers Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

metricstarttimeprocessor: avoid copy for gauges when adjusting metrics

4 participants