Skip to content
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

Improve performance: Don't request current context when creating a measurement #4253

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fcollonval
Copy link
Contributor

Description

Fixes #4243

Make Measurement.context optional and stop requesting the current context when creating a measurement

Type of change

Improve code performance following degradation with #4094

How Has This Been Tested?

I run locally the benchmarks to check the improvement.
I also tried the metrics benchmark with exemplar deactivate (aka setting exemplar filter to always off); it also show slight performance boost

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@fcollonval
Copy link
Contributor Author

@emdneto here is my attempt to restore code performance to its level prior to the introduction of the exemplar in #4094

As the project has been released with the new API for measurement, aka it has a context attribute, instead of dropping it as discussed, I made it optional and drop all call to get_current().

Do you think it would be better to drop the context attribute altogether?

Would it make sense to add benchmark with exemplar deactivated?

If so what is the best way of doing so?

I guess using pytest parametrize to change the type of exemplar filter is not a great idea as it will change the name of the benchmark tests and prevent comparison with the old results.

@fcollonval fcollonval changed the title Don't request current context when creating a measurement Improve performance: Don't request current context when creating a measurement Nov 9, 2024
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.

sdk/metrics: performance impact of exemplars support even when configured as always_off
1 participant