health checks?

[steve christensen]

Did you have anything in particular in mind for health checks?

I was thinking I'd start with Metrics' implementation https://dropwizard.github.io/metrics/3.1.0/manual/healthchecks/ , and follow some of the patterns you'd developed when you cleaned up the services & metrics modules.

probably something along the lines of...

• modules bind health checks as singletons
• health checks inject a HealthCheckRegistry (or wrapper around it), and register themselves w/ the registry
• a health check service that periodically runs the registry's health checks

[Jon Stevens]

please don't default them to mandatory. =)

jon

--
You received this message because you are subscribed to the Google Groups "GWizard Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to gwizard+u...@googlegroups.com.
To post to this group, send email to gwi...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/5cef871e-ea1e-41f0-9af6-3f96aacd78c3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[steve christensen]

That came together quicker than I thought it would. I've submitted a pull request.

And, I even remembered to branch this time.

[Jeff Schnitzer]

Neato! Applied. Apparently it went straight to master anyways; every time I think I have git/github fully figured out it does something to surprise me :)

How would you feel about renaming a few things? Since we've pluralized gwizard-metrics and gwizard-services, seems like gwizard-healtchecks/HealthChecksModule would be more consistent. But while it's ok I'm not totally in love - what do you think about HealthModule or DoctorModule? HealthConfig or DoctorConfig? I'm not sure. I guess this is the set of names that I'm leaning towards (but it might be different in the morning):

gwizard-health

HealthConfig

HealthModule

HealthService

HealthChecks (same as it is now)

Also - I will rename HealthChecks.register/unregister to add/remove to be consistent with Services.add().

Jeff

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/344c857b-eb4b-4bcd-ad2a-0e0f5f477672%40googlegroups.com.

[steve christensen]

I kind of like Doctor as the name.

But, I don't have a strong opinion one way or the other about these names.  I'd actually had them pluralized earlier, then renamed them to see how it looked and forgot to switch back.

I'm also realizing I probably shouldn't have made the health service a periodic service. Or, at least it should have had  a much longer interval...

If dropwizard serves as example, the registered health checks don't run until someone hits the health endpoint.

You received this message because you are subscribed to a topic in the Google Groups "GWizard Discussion" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/gwizard/RpXsQRcRuHQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to gwizard+u...@googlegroups.com.

To post to this group, send email to gwi...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/CADK-0ujTHV74xWzTJ%3DfYCdOGpHbDfw9c-TPxKBOTXLekNjPw8g%40mail.gmail.com.

[Jeff Schnitzer]

I committed this as HealthChecksModule (etc) for now. I guess it's straightforward enough.

One issue is that HealthConfig.interval is a Java8 class. Which brings up two questions:

1) We currently target Java7. Is it too early to say Java8 only?

2) Does jackson parse a java.time.Duration? [looking] Apparently if we add jackson-datatype-jsr310 we get some options, but it's not totally obvious what the format is from this:

https://github.com/FasterXML/jackson-datatype-jsr310/blob/master/src/test/java/com/fasterxml/jackson/datatype/jsr310/TestDurationSerialization.java

Jeff

[Huseyn Guliyev]

Hi guys,

I see some cool stuff being cooked here.

Just a third opinion here:

Java8 will be too limiting at the moment. Maybe best to use JodaTime for dates for now?

Best,

Huseyn

[steve christensen]

Dropwizard has a nice Duration class that'll be parsed from config. You can enter "1s", or "5min" into the yaml

You received this message because you are subscribed to a topic in the Google Groups "GWizard Discussion" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/gwizard/RpXsQRcRuHQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to gwizard+u...@googlegroups.com.

To post to this group, send email to gwi...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/b6b97ee6-7006-416c-9bb8-0a6b94331ecc%40googlegroups.com.

[Jeff Schnitzer]

I like that. I replaced it with the dropwizard Duration, which is conveniently in a util jar. It also has Size, which could be useful.

I'm not quite sure why IDEA didn't warn me about the JDK8ism even though the pom source level is set to 1.7. Mysterious.

Some thoughts:

 * I set the default interval to 10 minutes, but I'm open to any suggestion.

 * Should it run by default? And what if someone wants to disable automatic health checks? (presumably the web interface is what they want)

 * Once nice thing is that gwizard-healthcheck can register some JAXRS resources. If a RestModule (or JerseyModule) is not installed, it does nothing, so there's no hard dependency. This gives us an admin interface. But we need to establish a convention for that (everything under /admin?) and a way to secure it.

 * This calls for an AirbrakeModule or something like that that lets us automatically capture errors in logs. Probably depends on (and extends) LoggingModule.

Jeff

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/CAE6NpLStwzJhmHQg8yLiveuh9m%2BO8z7--oXenq6Leyhafea4MQ%40mail.gmail.com.

[steve christensen]

Maybe decouple the health check registration in HealthChecks from the service impl in HealthChecksService ?

Running health checks _should_ have minimal impact, but whoever is creating a gwizard app will likely have a strong opinion about how they want them triggered.

The current HealthChecksService could be renamed to "PeriodicHealthChecks", and it's the only thing that needs the health check config. We could put the binding for PeriodicHealthChecks into a separate PeriodicHealthCheckModule Guice module 

Then, only people that want that periodic checking will install the PeriodicHealthCheckModule into their injector.

The HealthChecks.run() method could change return signatures, and just do : 

return healthCheckRegistry.runHealthChecks().entrySet();

Then, if it's the dumb PeriodicHealthCheckService that's running, it'll just use slf4j to log the results. If it's a JAXRS resource, maybe it sticks all the results into JSON

For my possible usage, I'd probably just expose a Gauge metric that runs my health check (like the dumb HealthCheckInJmxToo class in HealthChecksModuleExample.java). Then, whenever jmxtrans or other JMX-scraper queries the Metrics mbeans, it'll trigger a run of the health check. 

[steve christensen]

I've implemented the changes I'd suggested below. Will have a pull request later tonight

[Jeff Schnitzer]

I merged this but I actually like having just the single HeathChecksModule and letting the HealthChecksConfig determine whether or not the service starts up. Two modules here feels a bit much for something so simple.

Since the HealthChecksService can inspect the HeathChecksConfig before it calls Services.add(), we can eliminate all the service overhead if we detect that the service shouldn't start. Either checking for null interval or an explicit on/off switch in the config.

Sound good?

I'll try this out in a few mins.

Jeff

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/fb1f2591-b908-4052-ae21-dc1aa03809a1%40googlegroups.com.

[Jeff Schnitzer]

Hmmm, no easy way to add a 'null' for the interval Duration in yml.  Some offhand ideas:

---- Option 1: Default to not running

---- Option 2: Separate flag

healthChecks:

    enabled: false   # anything else to call the flag?

---- Option 3: Custom Duration implementation

healthChecks:

    interval: never

[steve christensen]

Null check in the config sounds good

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/CADK-0uhYDkTN0ezbO%2BMkk9AGG29QXyQT_%2BU%2Bb2o0u%3D0HpiTROA%40mail.gmail.com.

[Jeff Schnitzer]

The problem with a null check in the config is that I can't seem to put a null in yaml. From the YAML spec it looks like 'null' or '~' should work but jackson is failing - 'null' just produces the default value and '~' is read as a literal. Maybe this is a bug in jackson or snakeyaml.

I tried '!!null' too and it didn't work.

I will ask about this on the jackson mailing list. In the mean time we can default to null or just punt on the problem until it gets fixed.

Jeff

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/CAE6NpLS%2BOi7DVWs37N0R40zTPCB12QDwT1Lxhrt%2B08zQ5ZaS-w%40mail.gmail.com.

[steve christensen]

You could default to to a ludicrously long Duration (10 years?). And, in the PeriodicHealthCheckService.schedule() don't use a zero initialDelay.

Oh. Hmm. Just thought of a weird interaction. Due to the zero-length initial delay, the health checks might run prior to all services finishing their startup. 

HealthChecks can be added/removed dynamically, throughout the life of the application. Should it be up to each service to register its HealthCheck at an appropriate point in the service's lifecycle?

Or, should the HealthChecks class be a ServiceManager.Listener, and only allow health checks to run once the services have completed their startup?

[Jeff Schnitzer]

Any compelling reason not to use the interval as a delay?

If we really want healthchecks to run immediately after startup, we can add an addStartHook() method to Run which lets the healthcheck system do some work after the awaitHealthy() is done. But the simple solution is just to leave a delay.

Jeff

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/c582ce26-38af-4bf8-9476-f148e49bc4f2%40googlegroups.com.

[Jon Stevens]

I vote for only running on services that have finished starting. 

Although, it might be useful to health check services through the whole lifecycle to be able to report how long it takes to start.

Jon

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/c582ce26-38af-4bf8-9476-f148e49bc4f2%40googlegroups.com.

[steve christensen]

Nope, I think just using the interval as the delay makes sense. 

In my mind... any 'real' health issues during initial startup should probably mean a service fails to startup, and the application barfs the errors all over the log

[steve christensen]

Timing startup could also be done by adding a @Timed annotation to your service's startup method (and adding gwizard-metrics' MetricsModule() to your injector)

[Jeff Schnitzer]

This is confusing to me. This test fails:

https://github.com/stickfigure/gwizard/blob/master/gwizard-config/src/test/java/com/voodoodyne/gwizard/config/YamlTest.java

Here's the input:

https://github.com/stickfigure/gwizard/blob/master/gwizard-config/src/test/resources/has_null.yml

The YAML spec and the snakeyaml docs suggest this should work, so I'm guessing jackson is to blame. Will know more soon.

Jeff

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/78a9637b-3c6c-460a-9712-e0f4d073d5d9%40googlegroups.com.

[steve christensen]

Do you need to represent null in the config file?

If the field's default is null in the POJO, just not supplying that field in the YAML input will mean it's null when deserialized from the config.

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/CADK-0uiuVFoKBxQj5%2BNeqnvT5pRp9MCm9_G%2BkECNkWDWrpQ6Rg%40mail.gmail.com.

[Jeff Schnitzer]

Right - that just means it's off by default. Which I'm fine with for now, although it seems like there's some merit to DW's "production ready out of the box" mantra.

I'll make that change.

Jeff

To view this discussion on the web visit https://groups.google.com/d/msgid/gwizard/CAE6NpLQacFOPboC2fZjZmC2sm02F-fOt4LWhAsy5oZ9ZJMV4WQ%40mail.gmail.com.