diff --git a/components/client/src/main/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheck.java b/components/client/src/main/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheck.java index bdaab832d8..5a9b609227 100644 --- a/components/client/src/main/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheck.java +++ b/components/client/src/main/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheck.java @@ -16,28 +16,29 @@ package com.hotels.styx.client.healthcheck; import com.codahale.metrics.Meter; -import com.hotels.styx.client.HttpClient; import com.hotels.styx.api.FullHttpRequest; -import com.hotels.styx.api.extension.Origin; import com.hotels.styx.api.MetricRegistry; +import com.hotels.styx.api.extension.Origin; +import com.hotels.styx.client.HttpClient; import com.hotels.styx.common.SimpleCache; -import io.netty.buffer.ByteBuf; -import rx.Observer; import static com.hotels.styx.api.HttpHeaderNames.HOST; import static com.hotels.styx.api.HttpResponseStatus.OK; import static com.hotels.styx.client.healthcheck.OriginHealthCheckFunction.OriginState.HEALTHY; import static com.hotels.styx.client.healthcheck.OriginHealthCheckFunction.OriginState.UNHEALTHY; -import static io.netty.util.ReferenceCountUtil.release; +import static java.lang.String.format; import static java.util.Objects.requireNonNull; /** * Health-check that works by making a request to a URL and ensuring that it gets an HTTP 200 OK code back. */ public class UrlRequestHealthCheck implements OriginHealthCheckFunction { + private static final MeterFormat DEPRECATED_METER_FORMAT = new MeterFormat("origins.healthcheck.failure.%s"); + private static final MeterFormat CORRECTED_METER_FORMAT = new MeterFormat("origins.%s.healthcheck.failure"); + private final String healthCheckUri; private final HttpClient client; - private final SimpleCache meterCache; + private final SimpleCache meterCache; /** * Construct an instance. @@ -49,7 +50,7 @@ public class UrlRequestHealthCheck implements OriginHealthCheckFunction { public UrlRequestHealthCheck(String healthCheckUri, HttpClient client, MetricRegistry metricRegistry) { this.healthCheckUri = uriWithInitialSlash(healthCheckUri); this.client = requireNonNull(client); - this.meterCache = new SimpleCache<>(origin -> metricRegistry.meter("origins.healthcheck.failure." + origin.applicationId())); + this.meterCache = new SimpleCache<>(origin -> new FailureMeter(origin, metricRegistry)); } private static String uriWithInitialSlash(String uri) { @@ -83,19 +84,32 @@ private FullHttpRequest newHealthCheckRequestFor(Origin origin) { .build(); } - // Note: this differs from just calling Observable.subscribe with no arguments, because it ignores errors too, whereas subscribe() throws an exception - private static class DoNothingObserver implements Observer { - @Override - public void onCompleted() { + private static final class FailureMeter { + private final Meter deprecatedMeter; + private final Meter correctedMeter; + + FailureMeter(Origin origin, MetricRegistry metricRegistry) { + this.deprecatedMeter = DEPRECATED_METER_FORMAT.meter(origin, metricRegistry); + this.correctedMeter = CORRECTED_METER_FORMAT.meter(origin, metricRegistry); } - @Override - public void onError(Throwable throwable) { + void mark() { + deprecatedMeter.mark(); + correctedMeter.mark(); } + } + + private static final class MeterFormat { + private final String format; + + MeterFormat(String format) { + this.format = format; + } + + public Meter meter(Origin origin, MetricRegistry metricRegistry) { + String name = format(format, origin.applicationId()); - @Override - public void onNext(ByteBuf byteBuf) { - release(byteBuf); + return metricRegistry.meter(name); } } } diff --git a/components/client/src/test/unit/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheckTest.java b/components/client/src/test/unit/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheckTest.java index b6179da7ab..3d446d7bc0 100644 --- a/components/client/src/test/unit/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheckTest.java +++ b/components/client/src/test/unit/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheckTest.java @@ -15,23 +15,22 @@ */ package com.hotels.styx.client.healthcheck; -import com.hotels.styx.client.HttpClient; import com.hotels.styx.api.FullHttpResponse; -import com.hotels.styx.api.extension.Origin; import com.hotels.styx.api.HttpResponseStatus; import com.hotels.styx.api.MetricRegistry; +import com.hotels.styx.api.extension.Origin; import com.hotels.styx.api.metrics.codahale.CodaHaleMetricRegistry; +import com.hotels.styx.client.HttpClient; import com.hotels.styx.client.healthcheck.OriginHealthCheckFunction.OriginState; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; -import java.io.IOException; import java.util.concurrent.CompletableFuture; import static com.hotels.styx.api.FullHttpResponse.response; -import static com.hotels.styx.api.extension.Origin.newOriginBuilder; import static com.hotels.styx.api.HttpResponseStatus.NOT_FOUND; import static com.hotels.styx.api.HttpResponseStatus.OK; +import static com.hotels.styx.api.extension.Origin.newOriginBuilder; import static com.hotels.styx.client.healthcheck.OriginHealthCheckFunction.OriginState.HEALTHY; import static com.hotels.styx.client.healthcheck.OriginHealthCheckFunction.OriginState.UNHEALTHY; import static org.hamcrest.MatcherAssert.assertThat; @@ -66,7 +65,7 @@ public void sendsTheHealthCheckRequestToTheGivenUrl() { } @Test - public void declaresOriginHealthyOnOkResponseCode() throws IOException { + public void declaresOriginHealthyOnOkResponseCode() { HttpClient client = request -> respondWith(OK); new UrlRequestHealthCheck("/version.txt", client, metricRegistry) @@ -77,7 +76,7 @@ public void declaresOriginHealthyOnOkResponseCode() throws IOException { } @Test - public void declaresOriginUnhealthyOnNon200Ok() throws IOException { + public void declaresOriginUnhealthyOnNon200Ok() { HttpClient client = request -> respondWith(NOT_FOUND); new UrlRequestHealthCheck("/version.txt", client, metricRegistry) @@ -85,11 +84,12 @@ public void declaresOriginUnhealthyOnNon200Ok() throws IOException { assertThat(originState, is(UNHEALTHY)); assertThat(metricRegistry.meter("origins.healthcheck.failure.generic-app").getCount(), is(1L)); - assertThat(metricRegistry.getMeters().size(), is(1)); + assertThat(metricRegistry.meter("origins.generic-app.healthcheck.failure").getCount(), is(1L)); + assertThat(metricRegistry.getMeters().size(), is(2)); } @Test - public void declaredOriginUnhealthyOnTransportException() throws IOException { + public void declaredOriginUnhealthyOnTransportException() { HttpClient client = request -> respondWith(new RuntimeException("health check failure, as expected")); new UrlRequestHealthCheck("/version.txt", client, metricRegistry) @@ -97,7 +97,8 @@ public void declaredOriginUnhealthyOnTransportException() throws IOException { assertThat(originState, is(UNHEALTHY)); assertThat(metricRegistry.meter("origins.healthcheck.failure.generic-app").getCount(), is(1L)); - assertThat(metricRegistry.getMeters().size(), is(1)); + assertThat(metricRegistry.meter("origins.generic-app.healthcheck.failure").getCount(), is(1L)); + assertThat(metricRegistry.getMeters().size(), is(2)); } private static CompletableFuture respondWith(Throwable error) { diff --git a/docs/user-guide/configure-health-checks.md b/docs/user-guide/configure-health-checks.md index 2e12d9f21d..8ec25f9a40 100644 --- a/docs/user-guide/configure-health-checks.md +++ b/docs/user-guide/configure-health-checks.md @@ -81,8 +81,8 @@ into `BackendService`. A meter of failed health check attempts per backend service: - origins.healthcheck.failure..count - origins.healthcheck.failure..m1_rate - origins.healthcheck.failure..m5_rate - origins.healthcheck.failure..m15_rate - origins.healthcheck.failure..mean_rate + origins..healthcheck.failure.count + origins..healthcheck.failure.m1_rate + origins..healthcheck.failure.m5_rate + origins..healthcheck.failure.m15_rate + origins..healthcheck.failure.mean_rate diff --git a/docs/user-guide/metrics.md b/docs/user-guide/metrics.md index 1f1b3a224d..85fea5d4e1 100644 --- a/docs/user-guide/metrics.md +++ b/docs/user-guide/metrics.md @@ -138,6 +138,11 @@ The server side metrics scopes are illustrated in a diagram below: origins...status ``` + - Health-check metrics +``` + origins..healthcheck.failure +``` + The client side metrics scopes are illustrated in a diagram below: ![Styx Client Metrics](../assets/styx-origin-metrics.png "Styx client metrics") @@ -221,8 +226,6 @@ Styx also measures metrics from the underlying JVM: Following metrics are subject to change their names: - origins.healthcheck.failure. - origins.healthcheck.failure.. origins.response.status.