Update origin health-check metric name (fixes #269) (#289)

ExpediaGroup · Sep 18, 2018 · 2ee1e6a · 2ee1e6a
1 parent 4f42065
commit 2ee1e6a
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 32 deletions.
diff --git a/...onents/client/src/main/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheck.java b/...onents/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<Origin, Meter> meterCache;
+    private final SimpleCache<Origin, FailureMeter> 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<ByteBuf> {
-        @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/...ient/src/test/unit/java/com/hotels/styx/client/healthcheck/UrlRequestHealthCheckTest.java b/...ient/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,27 +76,29 @@ 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)
                 .check(someOrigin, state -> this.originState = state);
 
         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)
                 .check(someOrigin, state -> this.originState = state);
 
         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<FullHttpResponse> respondWith(Throwable error) {

diff --git 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.<BACKEND-ID>.count
-    origins.healthcheck.failure.<BACKEND-ID>.m1_rate
-    origins.healthcheck.failure.<BACKEND-ID>.m5_rate
-    origins.healthcheck.failure.<BACKEND-ID>.m15_rate
-    origins.healthcheck.failure.<BACKEND-ID>.mean_rate
+    origins.<BACKEND-ID>.healthcheck.failure.count
+    origins.<BACKEND-ID>.healthcheck.failure.m1_rate
+    origins.<BACKEND-ID>.healthcheck.failure.m5_rate
+    origins.<BACKEND-ID>.healthcheck.failure.m15_rate
+    origins.<BACKEND-ID>.healthcheck.failure.mean_rate
diff --git 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.<backend>.<origin>.status
 ```
 
+ - Health-check metrics
+```
+    origins.<backend>.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.<app>
-     origins.healthcheck.failure.<app>.<instance>
      origins.response.status.<code>