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

Kotlin-specific ObservationRegistry API #4772

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

Conversation

ilya40umov
Copy link

@ilya40umov ilya40umov commented Feb 26, 2024

This PR introduces a couple of extension functions on ObservationRegistry to help using Observation API from Kotlin code.

  • ObservationRegistry.observeAndGet allows to observe a non-suspending block of code:
    • this method is analogous to Observation.observe, but accepts a block of code instead of expecting a Java Supplier
    • this method will work with both nullable and non-nullable generic type (while usage of Supplier makes Kotlin compiler think the result of the nested block is always nullable)
  • ObservationRegistry.observeAndAwait allows to observe a suspending block of code:
    • this method is again similar to Observation.observe, but will accept a suspending block of code
    • by using openScope().use { observationRegistry.asContextElement() } it manages to create an instance of KotlinObservationContextElement with the new observation captured as "current"

Initial idea: Why add those to ObservationRegistry and not to Observation? Mostly for consistency. I.e. observeAndAwait needs access to ObservationRegistry anyway, so it's easy to add it to the registry as an extension. While observeAndGet could indeed be made into an extension function on Observation, however it still needs to be called something other than observe, as otherwise Intellij can't resolve which observe version the code is referring to.

Update: I realized that Observation also contains registry it was created for, so I created an alternative PR which adds both of the extension functions to Observation class: #4823

gh-4754

@pivotal-cla
Copy link

@ilya40umov Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@ilya40umov Thank you for signing the Contributor License Agreement!

@ilya40umov
Copy link
Author

@shakuzen would you be the one to ask for review on this PR? 🙏

@shakuzen
Copy link
Member

would you be the one to ask for review on this PR?

I will try to find someone to take a look. Some folks are off this week, though.

@ilya40umov ilya40umov force-pushed the feature/observation-kotlin-api branch from 20e6fd8 to 6ff6699 Compare March 2, 2024 19:39
@ilya40umov ilya40umov changed the title Kotlin-specific Observation API Kotlin-specific ObservationRegistry API Mar 2, 2024
@ilya40umov
Copy link
Author

I realized that Observation also contains the registry it was created for, so I created an alternative PR which adds both of the extension functions to Observation class as a possible alternative: #4823

Feel free to choose which one would make more sense for Micrometer project to include.

@ilya40umov ilya40umov force-pushed the feature/observation-kotlin-api branch from 6ff6699 to 93a33b7 Compare March 4, 2024 20:14
@ilya40umov ilya40umov force-pushed the feature/observation-kotlin-api branch from 93a33b7 to 0fa5845 Compare March 4, 2024 20:15
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.

None yet

3 participants