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.

Sign up for GitHub

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

HealthCheckRoutes Provider, so user can add it to their own server #320

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

HealthCheckRoutes Provider, so user can add it to their own server #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fd336ab
- Created an extension for HealthCheckRoutes, so users can get the ro…
shakeeb-upstart Oct 2, 2024
3be732a
- optimized imports
shakeeb-upstart Oct 2, 2024
c2bb457
- updated scaladoc
shakeeb-upstart Oct 2, 2024
3cad941
- updated scaladoc
shakeeb-upstart Oct 2, 2024
95d1b44
- updated scaladoc
shakeeb-upstart Oct 2, 2024
6a2f13c
- added unit test
shakeeb-upstart Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions management/src/main/scala/org/apache/pekko/management/HealthCheckRoutes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
package org.apache.pekko.management

import org.apache.pekko
import pekko.actor.ExtendedActorSystem
import pekko.actor.{ ActorSystem, ClassicActorSystemProvider, ExtendedActorSystem, ExtensionId, ExtensionIdProvider }
import pekko.annotation.InternalApi
import pekko.http.scaladsl.model._
import pekko.http.scaladsl.server.Directives._
Expand Down Expand Up @@ -47,7 +47,12 @@ private[pekko] class HealthCheckRoutes(system: ExtendedActorSystem) extends Mana
StatusCodes.InternalServerError -> s"Health Check Failed: ${t.getMessage}")
}

override def routes(mrps: ManagementRouteProviderSettings): Route = {
override def routes(mrps: ManagementRouteProviderSettings): Route = routes()

/**
* @since 1.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

1.1.0 was already released so we need to decide whether this can go in 1.1.1 or if we need a 1.2.0 before this can be released

Copy link
Contributor

@pjfanning pjfanning Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

not much point changing this yet - but I will block the merge until we can discuss how to eventually release this (which affects which branch we can use)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes sure, let me know if I am needed for any part in the discussion.

*/
def routes(): Route = {
concat(
path(PathMatchers.separateOnSlashes(settings.startupPath)) {
get {
Expand All @@ -66,3 +71,17 @@ private[pekko] class HealthCheckRoutes(system: ExtendedActorSystem) extends Mana
})
}
}

/**
* Providing an extension, so users can get the routes and add it to their own server
* @since 1.1.0
*/
object HealthCheckRoutes extends ExtensionId[HealthCheckRoutes] with ExtensionIdProvider {
override def get(system: ActorSystem): HealthCheckRoutes = super.get(system)

override def get(system: ClassicActorSystemProvider): HealthCheckRoutes = super.get(system)

override def lookup: HealthCheckRoutes.type = HealthCheckRoutes

override def createExtension(system: ExtendedActorSystem): HealthCheckRoutes = new HealthCheckRoutes(system)
}
shakeeb-upstart marked this conversation as resolved.
Show resolved Hide resolved
27 changes: 24 additions & 3 deletions management/src/test/scala/org/apache/pekko/management/HealthCheckRoutesSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class HealthCheckRoutesSpec extends AnyWordSpec with Matchers with ScalatestRout

private val eas = system.asInstanceOf[ExtendedActorSystem]

private def testRoute(
private def testRouteWithProviderSettings(
startupResultValue: Future[Either[String, Unit]] = Future.successful(Right(())),
readyResultValue: Future[Either[String, Unit]] = Future.successful(Right(())),
aliveResultValue: Future[Either[String, Unit]] = Future.successful(Right(()))): Route = {
Expand All @@ -44,12 +44,33 @@ class HealthCheckRoutesSpec extends AnyWordSpec with Matchers with ScalatestRout
}.routes(ManagementRouteProviderSettings(Uri("http://whocares"), readOnly = false))
}

private def testRoute(
startupResultValue: Future[Either[String, Unit]] = Future.successful(Right(())),
readyResultValue: Future[Either[String, Unit]] = Future.successful(Right(())),
aliveResultValue: Future[Either[String, Unit]] = Future.successful(Right(()))): Route = {
new HealthCheckRoutes(eas) {
override protected val healthChecks: HealthChecks = new HealthChecks {
override def startupResult(): Future[Either[String, Unit]] = startupResultValue
override def startup(): Future[Boolean] = startupResultValue.map(_.isRight)
override def readyResult(): Future[Either[String, Unit]] = readyResultValue
override def ready(): Future[Boolean] = readyResultValue.map(_.isRight)
override def aliveResult(): Future[Either[String, Unit]] = aliveResultValue
override def alive(): Future[Boolean] = aliveResultValue.map(_.isRight)
}
}.routes()
}

tests("/startup", result => testRoute(startupResultValue = result))
tests("/ready", result => testRoute(readyResultValue = result))
tests("/alive", result => testRoute(aliveResultValue = result))

def tests(endpoint: String, route: Future[Either[String, Unit]] => Route) = {
s"Health check $endpoint endpoint" should {
// testRoutes with provider settings
tests("/startup", result => testRouteWithProviderSettings(startupResultValue = result), withSettings = true)
tests("/ready", result => testRouteWithProviderSettings(readyResultValue = result), withSettings = true)
tests("/alive", result => testRouteWithProviderSettings(aliveResultValue = result), withSettings = true)

def tests(endpoint: String, route: Future[Either[String, Unit]] => Route, withSettings: Boolean = false) = {
s"Health check $endpoint endpoint - withSettingsProvided: $withSettings" should {
"return 200 for Right" in {
Get(endpoint) ~> route(Future.successful(Right(()))) ~> check {
status shouldEqual StatusCodes.OK
Expand Down
Loading