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

Add supplier variant for Observation.Context.getOrDefault() #3708

Merged
merged 2 commits into from
Mar 22, 2023

Conversation

izeye
Copy link
Contributor

@izeye izeye commented Mar 22, 2023

This PR adds a supplier variant for Observation.Context.getOrDefault() to avoid default object creation where unnecessary.

* @param <T> value type
* @return object or default if not present
*/
<T> T getOrDefault(Object key, Supplier<T> defaultObjectSupplier);
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that should be a default method of we don't want to break current users (assuming that anyone has ever created their own implementation of ObservationRegistry)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marcingrzejszczak Thanks for the feedback! I updated this based on your feedback.

@marcingrzejszczak marcingrzejszczak added the enhancement A general enhancement label Mar 22, 2023
@marcingrzejszczak
Copy link
Contributor

Ah, one more thing - can you please add a test for this?

* @param defaultObjectSupplier supplier for default object to return
* @param <T> value type
* @return object or default if not present
*/
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add @since 1.11.0?

@jonatan-ivanov jonatan-ivanov added this to the 1.11.0-RC1 milestone Mar 22, 2023
@izeye
Copy link
Contributor Author

izeye commented Mar 22, 2023

@marcingrzejszczak @jonatan-ivanov Thanks! I applied your reviews in e11b3bd.

@marcingrzejszczak marcingrzejszczak merged commit a350afa into micrometer-metrics:main Mar 22, 2023
@izeye izeye deleted the supplier branch March 22, 2023 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants