HealthCheck endpoint and service #630

ememmr · 2019-07-24T14:40:02Z

What this does?

It imitates gRPC's health checking protocol .
Due to some problems of accessibility at grpc health check module, a new scala version has been developed.

It contains:

module health-check-unary for unary calls
module health-check-fs2 for streaming calls with fs2 library
module health-check-monix for streaming calls with monix library
example for health check fs2 streaming case
example for health check monix streaming case

Related to #626

On one terminal: sbt health-server/run On other terminal: sbt health-client/run Fails with exception: com.google.protobuf.InvalidProtocolBufferException: While parsing a protocol message, the input ended unexpectedly in the middle of a field. This could mean either that the input has been truncated or that an embedded message misreported its own length.

- Due to there’s a bug on protobuf services, we can’t use enum types. Instead of them, we use ServerStatus as case class. - Empty.type doesn’t work on cleanAll and checkAll methods. Pending to fix -

Pending: refactor unary calls into a common part

build.sbt

codecov · 2019-07-24T15:21:23Z

Codecov Report

Merging #630 into master will increase coverage by 0.47%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master     #630      +/-   ##
==========================================
+ Coverage   82.45%   82.93%   +0.47%     
==========================================
  Files          62       69       +7     
  Lines         969      996      +27     
  Branches       11       11              
==========================================
+ Hits          799      826      +27     
  Misses        170      170

Impacted Files	Coverage Δ
...herkindness/mu/rpc/healthcheck/unary/service.scala	`0% <0%> (ø)`
...ndness/mu/rpc/healthcheck/monix/serviceMonix.scala	`0% <0%> (ø)`
...erkindness/mu/rpc/healthcheck/fs2/serviceFS2.scala	`0% <0%> (ø)`
...pc/healthcheck/fs2/handler/HealthServiceImpl.scala	`100% <100%> (ø)`
...erkindness/mu/rpc/healthcheck/unary/ordering.scala	`100% <100%> (ø)`
.../healthcheck/monix/handler/HealthServiceImpl.scala	`100% <100%> (ø)`
.../healthcheck/unary/handler/HealthServiceImpl.scala	`100% <100%> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dce6b79...ec14467. Read the comment docs.

modules/examples/todolist/server/src/main/resources/application.conf

...alth-check/health-check-unary/src/main/scala/higherkindness/mu/rpc/healthcheck/service.scala

+ application.conf of todolist example restored

juanpedromoreno

Thanks @mrtmmr! This is looking neat, I left some comments though.

...ck-fs2/health-client/src/main/scala/higherkindness/mu/rpc/healthcheck/client/ClientApp.scala

...lient/src/main/scala/higherkindness/mu/rpc/healthcheck/client/HealthCheckClientHandler.scala

...-monix/health-client/src/main/scala/higherkindness/mu/rpc/healthcheck/client/ClientApp.scala

...eck-monix/health-server/src/main/scala/higherkindness/mu/rpc/healthcheck/CommonRuntime.scala

...h-check-fs2/src/main/scala/higherkindness/mu/rpc/healthcheck/handler/HealthServiceImpl.scala

juanpedromoreno · 2019-07-25T19:24:49Z

...h-check-fs2/src/main/scala/higherkindness/mu/rpc/healthcheck/handler/HealthServiceImpl.scala

+      Stream.eval(watchTopic.publish1(newStatus)).compile.drain
+
+  def watch(service: HealthCheck): Stream[F, HealthStatus] =
+    watchTopic.subscribe(20).filter(hs => hs.hc == service)


I thought 20 was enough elements to enqueue before blocking the subscription to the stream. I guess it depends on the project in which is used

Does it make sense if we can provide it as a new function argument?

More or less, there is only one stream providing status information to everyone who ask for it. It just filters depending on the service you ask, so the number you pass will apply to every status (of every service added). The user needs beware of this to choose the correct number.

...check-monix/src/main/scala/higherkindness/mu/rpc/healthcheck/handler/HealthServiceImpl.scala

...check/health-check-unary/src/main/scala/higherkindness/mu/rpc/healthcheck/ServerStatus.scala

juanpedromoreno · 2019-07-25T19:28:35Z

...alth-check/health-check-unary/src/main/scala/higherkindness/mu/rpc/healthcheck/service.scala

+import higherkindness.mu.rpc.protocol.{service, Empty, Protobuf}
+object service {
+
+  @service(Protobuf)


I was thinking about providing this protocols as IDL files (.proto files), however, what we are providing here is also the implementation, so at the end, you will be adding the binary dependencies in any case. Am I right?

Yes, but I think is better to provide it

- Monix: Debug messages dropped ExecutionContext dropped Effects fixed - FS2: Debug messages dropped Effects fixed Pending: refactor health-check modules into existing

Service: - unary service: server module - fs2 service: fs2 module - monix service: monix module Example: - health-client: all clients - health-server-fs2: fs2 server - health-server-monix: monix server

fedefernandez

This is looking good, thanks @mrtmmr

build.sbt

...-check/health-client/src/main/scala/higherkindness/mu/rpc/healthcheck/client/ClientApp.scala

...alth-client/src/main/scala/higherkindness/mu/rpc/healthcheck/client/monix/gclientMonix.scala

build.sbt

juanpedromoreno · 2019-07-30T08:17:05Z

build.sbt

-  .dependsOn(fs2 % "test->test")
+  .dependsOn(`internal-monix`% "test->test")
+  .dependsOn(`internal-fs2`% "test->test")
+  .dependsOn(channel)


Could it be .dependsOn(channel % "test->test")?

I need it for the unary service creation :/

fedefernandez

Left some more comments, LGTM once addressed or answered

fedefernandez · 2019-07-31T09:24:58Z

modules/channel/src/main/scala/higherkindness/mu/rpc/healthcheck/HealthServiceImpl.scala

+      Ref.of[F, Map[String, ServerStatus]](Map.empty[String, ServerStatus])
+
+    checkRef.map(c => new HealthCheckServiceUnaryImpl[F](c))
+  }


def buildInstance[F[_]: Sync](implicit s: Scheduler): F[HealthCheckServiceUnary[F]] = Ref.of[F, Map[String, ServerStatus]](Map.empty[String, ServerStatus]) .map(new HealthCheckServiceUnaryImpl[F](_))

modules/channel/src/main/scala/higherkindness/mu/rpc/healthcheck/HealthServiceImpl.scala

fedefernandez · 2019-07-31T09:53:41Z

...aming/monix/src/main/scala/higherkindness/mu/rpc/healthcheck/handler/HealthServiceImpl.scala

+
+class HealthCheckServiceMonixImpl[F[_]: Sync](
+    checkStatus: Ref[F, Map[String, ServerStatus]],
+    pipe: (Observer.Sync[HealthStatus], Observable[HealthStatus]))(implicit s: Scheduler)


I'd change this to receive the types in two params:

object HealthServiceMonix { def buildInstance[F[_]: Sync](implicit s: Scheduler): F[HealthCheckServiceMonix[F]] = { val (observer, observable) = Pipe( MulticastStrategy.behavior( HealthStatus(new HealthCheck("FirstStatus"), ServerStatus("UNKNOWN")))) .concurrent(s) Ref.of[F, Map[String, ServerStatus]](Map.empty[String, ServerStatus]) .map(new HealthCheckServiceMonixImpl[F](_, observer, observable)) } } class HealthCheckServiceMonixImpl[F[_]: Sync]( checkStatus: Ref[F, Map[String, ServerStatus]], observer: Observer.Sync[HealthStatus], observable: Observable[HealthStatus])(implicit s: Scheduler) extends AbstractHealthService[F](checkStatus) with HealthCheckServiceMonix[F] { override def setStatus(newStatus: HealthStatus): F[Unit] = checkStatus.update(_ + (newStatus.hc.nameService -> newStatus.status)) <* Sync[F].delay(observer.onNext(newStatus)) override def watch(service: HealthCheck): Observable[HealthStatus] = observable.filter(_.hc.nameService == service.nameService) }

fedefernandez · 2019-07-31T09:55:53Z

...aming/monix/src/main/scala/higherkindness/mu/rpc/healthcheck/handler/HealthServiceImpl.scala

+      Sync[F].delay(pipe._1.onNext(newStatus))
+
+  override def watch(service: HealthCheck): Observable[HealthStatus] =
+    pipe._2.filter(_.hc == service)


I'm thinking about creating an implicit instance of cats.kernel.Order for the type HealthCheck, for example in a package object (mu/modules/channel/src/main/scala/higherkindness/mu/rpc/healthcheck/package.scala)

package higherkindness.mu.rpc import cats.kernel.Order import cats.instances.string._ package object healthcheck { implicit val orderForHealthCheck: Order[HealthCheck] = new HealthCheckOrder class HealthCheckOrder(implicit O: Order[String]) extends Order[HealthCheck] { override def eqv(x: HealthCheck, y: HealthCheck): Boolean = O.eqv(x.nameService, y.nameService) def compare(x: HealthCheck, y: HealthCheck): Int = O.compare(x.nameService, y.nameService) } }

An then use the triple equals here:

pipe._2.filter(_.hc === service)

What would this prevent? I'm not sure...

Using the triple equals will check the types at compilation time, avoiding some bugs when this code is evolved. i.e. if in the future the param service is not of type HealthCheck, the compilation will fail.

fedefernandez · 2019-07-31T10:28:38Z

Now I'm seeing the errors in the build it looks we need to provide the protocol in a different module.

We could have the following modules:

health-check-protocol-unary: With the current unary @service
health-check-service-unary: Depending on channel and health-check-protocol-unary-scala with the % provided

We could also include the avdl and proto files in the channel resources, providing instructions about how to add the sbt settings for generating your protocol definition.

We'd need something similar for monix and fs2. @mrtmmr @juanpedromoreno thoughts?

juanpedromoreno · 2019-07-31T11:01:22Z

Sounds good to me @fedefernandez @mrtmmr :)

fedefernandez · 2019-07-31T11:06:55Z

@juanpedromoreno after a talk with @mrtmmr we could create the unary service in its own single module, right now only with Protobuf and create an issue for giving support for both, avro and proto.

fedefernandez · 2019-07-31T16:39:40Z

build.sbt

+lazy val `health-check-unary` = project
+  .in(file("modules/health-check-unary"))
+  .dependsOn(channel)
+  .dependsOn(server)


I think you don't need server

That's right, I need it just on the examples

fedefernandez · 2019-07-31T16:40:45Z

modules/health-check-unary/health-check-unary.iml

@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="UTF-8"?>


What happen?

fedefernandez · 2019-07-31T16:41:38Z

build.sbt

+  .dependsOn(channel)
+  .dependsOn(server)
+  .settings(healthCheckSettings)
+  .settings(moduleName := "mu-rpc-example-health-check-unary")


Suggested change

.settings(moduleName := "mu-rpc-example-health-check-unary")

.settings(moduleName := "mu-rpc-health-check-unary")

ops, too much changes on build.sbt

juanpedromoreno · 2019-08-01T06:12:10Z

modules/examples/health-check/health-server-monix/hc-server.iml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>


This file seems to be related to your IDE. Could we remove it and ignore it from the .gitignore file?

Sure, no problem. I can't understand why it didn't happen before.

What should I do with code coverage test? It fails

juanpedromoreno · 2019-08-01T06:14:15Z

modules/health-check-unary/health-check-unary.iml

@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="UTF-8"?>


Same for this file

juanpedromoreno

Thanks @mrtmmr ! Outstanding job 👍

fedefernandez · 2019-08-02T18:17:24Z

...ck-unary/src/test/scala/higherkindness/mu/rpc/healthcheck/unary/HealthServiceUnaryTest.scala

+
+  }
+
+}


fedefernandez

👏 👏 👏

fedefernandez · 2019-08-02T18:18:11Z

...health-check-unary/src/test/scala/higherkindness/mu/rpc/healthcheck/unary/OrderingTest.scala

+
+class OrderingTest extends WordSpec with Matchers {
+  "Ordering" should {
+    "works" in {


Suggested change

"works" in {

"work" in {

fedefernandez · 2019-08-02T18:18:24Z

...health-check-unary/src/test/scala/higherkindness/mu/rpc/healthcheck/unary/OrderingTest.scala

+      assert(new HealthCheck("example") === new HealthCheck("example"))
+      assert(new HealthCheck("example") !== new HealthCheck("not example"))
+    }
+    "works with boolean comparison" in {


Suggested change

"works with boolean comparison" in {

"work with boolean comparison" in {

mrtmmr added 5 commits July 17, 2019 15:50

Unary calls

2717fed

- Due to there’s a bug on protobuf services, we can’t use enum types. Instead of them, we use ServerStatus as case class. - Empty.type doesn’t work on cleanAll and checkAll methods. Pending to fix -

fs2 watch part

12f6daa

Streaming monix part

3477089

Pending: refactor unary calls into a common part

Unary part refactored

962189c

ememmr requested review from fedefernandez and juanpedromoreno July 24, 2019 14:40

fedefernandez reviewed Jul 24, 2019

View reviewed changes

build.sbt Outdated Show resolved Hide resolved

fedefernandez reviewed Jul 25, 2019

View reviewed changes

modules/examples/todolist/server/src/main/resources/application.conf Outdated Show resolved Hide resolved

...alth-check/health-check-unary/src/main/scala/higherkindness/mu/rpc/healthcheck/service.scala Outdated Show resolved Hide resolved

Some changes in service definition

3d88d35

+ application.conf of todolist example restored

juanpedromoreno reviewed Jul 25, 2019

View reviewed changes

mrtmmr added 3 commits July 26, 2019 12:04

Some fixes 1/2

fc318de

- Monix: Debug messages dropped ExecutionContext dropped Effects fixed - FS2: Debug messages dropped Effects fixed Pending: refactor health-check modules into existing

Some fixes 2/2 - modules refactored

ed0530b

Service: - unary service: server module - fs2 service: fs2 module - monix service: monix module Example: - health-client: all clients - health-server-fs2: fs2 server - health-server-monix: monix server

Fixed some dependencies for continuous integration

8689ea6

fedefernandez reviewed Jul 30, 2019

View reviewed changes

juanpedromoreno reviewed Jul 30, 2019

View reviewed changes

Unary service move into channel module + log4cats added

cfd73b9

fedefernandez reviewed Jul 31, 2019

View reviewed changes

mrtmmr added 2 commits July 31, 2019 16:18

Helath-check-unary module recovered + some little changes

4320553

Continuous integration fail fixed

af708dd

fedefernandez reviewed Jul 31, 2019

View reviewed changes

Modules dependencies fixed - 'server' dropped

418d5c7

juanpedromoreno reviewed Aug 1, 2019

View reviewed changes

Tests added to increase code coverage + iml added to gitignore

fbeae3f

juanpedromoreno approved these changes Aug 2, 2019

View reviewed changes

fedefernandez reviewed Aug 2, 2019

View reviewed changes

...ck-unary/src/test/scala/higherkindness/mu/rpc/healthcheck/unary/HealthServiceUnaryTest.scala

}

}

Copy link

Contributor

fedefernandez Aug 2, 2019

Choose a reason for hiding this comment

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

Nice

fedefernandez approved these changes Aug 2, 2019

View reviewed changes

Little grammar thing

ec14467

ememmr merged commit 0a0bac5 into master Aug 5, 2019

This was referenced Aug 6, 2019

Support gRPC health check endpoint by default #626

Closed

Unary Health Check Service should support not only protobuf but also avro #633

Open

ememmr deleted the mmm/626-healthcheckpoint branch August 6, 2019 13:20

fedefernandez changed the title ~~mmm/626 healthcheckpoint~~ HealthCheck endpoint and service Aug 19, 2019

AdrianRaFo mentioned this pull request Oct 17, 2019

Missing docs for new added modules #682

Closed

eugkhp mentioned this pull request Nov 20, 2020

Health check doesn't implement grpc health protocol #1054

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HealthCheck endpoint and service #630

HealthCheck endpoint and service #630

ememmr commented Jul 24, 2019 •

edited

Loading

codecov bot commented Jul 24, 2019 •

edited

Loading

juanpedromoreno left a comment

juanpedromoreno Jul 25, 2019

ememmr Jul 26, 2019

juanpedromoreno Jul 26, 2019

ememmr Jul 26, 2019

juanpedromoreno Jul 25, 2019

ememmr Jul 26, 2019

fedefernandez left a comment

juanpedromoreno Jul 30, 2019

ememmr Jul 30, 2019

fedefernandez left a comment

fedefernandez Jul 31, 2019

fedefernandez Jul 31, 2019

fedefernandez Jul 31, 2019

ememmr Jul 31, 2019

fedefernandez Jul 31, 2019 •

edited

Loading

fedefernandez commented Jul 31, 2019 •

edited

Loading

juanpedromoreno commented Jul 31, 2019

fedefernandez commented Jul 31, 2019

fedefernandez Jul 31, 2019

ememmr Jul 31, 2019

fedefernandez Jul 31, 2019

ememmr Jul 31, 2019

fedefernandez Jul 31, 2019

ememmr Jul 31, 2019

juanpedromoreno Aug 1, 2019

ememmr Aug 1, 2019

juanpedromoreno Aug 1, 2019

juanpedromoreno left a comment

fedefernandez Aug 2, 2019

fedefernandez left a comment

fedefernandez Aug 2, 2019

fedefernandez Aug 2, 2019

	.settings(moduleName := "mu-rpc-example-health-check-unary")
	.settings(moduleName := "mu-rpc-health-check-unary")

	"works with boolean comparison" in {
	"work with boolean comparison" in {

HealthCheck endpoint and service #630

HealthCheck endpoint and service #630

Conversation

ememmr commented Jul 24, 2019 • edited Loading

What this does?

codecov bot commented Jul 24, 2019 • edited Loading

Codecov Report

juanpedromoreno left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedefernandez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedefernandez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedefernandez Jul 31, 2019 • edited Loading

Choose a reason for hiding this comment

fedefernandez commented Jul 31, 2019 • edited Loading

juanpedromoreno commented Jul 31, 2019

fedefernandez commented Jul 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juanpedromoreno left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedefernandez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ememmr commented Jul 24, 2019 •

edited

Loading

codecov bot commented Jul 24, 2019 •

edited

Loading

fedefernandez Jul 31, 2019 •

edited

Loading

fedefernandez commented Jul 31, 2019 •

edited

Loading