Fix metrics in new routing model

components/api/src/main/java/com/hotels/styx/api/LiveHttpRequest.java

@@ -533,6 +533,9 @@ private interface BuilderTransformer {
         LiveHttpRequest build();
     }
 
+    /**
+     * Transforms a LiveHttpRequest.
+     */
     public static final class Transformer implements BuilderTransformer {
         private final Builder builder;
 

components/api/src/main/java/com/hotels/styx/api/LiveHttpResponse.java

@@ -423,6 +423,9 @@ private interface BuilderTransformer {
 
     }
 
+    /**
+     * Transforms a LiveHttpResponse.
+     */
     public static final class Transformer implements BuilderTransformer {
         private final Builder builder;
 

components/client/src/main/java/com/hotels/styx/client/OriginStatsFactory.java

@@ -23,6 +23,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
+import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
 
 
@@ -58,7 +59,7 @@ public CachingOriginStatsFactory(MetricRegistry metricRegistry) {
          * @return the {@link OriginStats}
          */
         public OriginStats originStats(Origin origin) {
-            return metricsByOrigin.computeIfAbsent(origin, theOrigin -> OriginMetrics.create(theOrigin, metricRegistry));
+            return metricsByOrigin.computeIfAbsent(origin, theOrigin -> OriginMetrics.create(theOrigin.applicationId(), theOrigin.id().toString(), metricRegistry));
         }
     }
 }

components/client/src/main/java/com/hotels/styx/client/applications/metrics/OriginMetrics.java

@@ -18,7 +18,7 @@
 import com.codahale.metrics.Counter;
 import com.codahale.metrics.Meter;
 import com.codahale.metrics.Timer;
-import com.hotels.styx.api.extension.Origin;
+import com.hotels.styx.api.Id;
 import com.hotels.styx.api.MetricRegistry;
 import com.hotels.styx.client.applications.AggregateTimer;
 import com.hotels.styx.client.applications.OriginStats;
@@ -62,11 +62,12 @@ public class OriginMetrics implements OriginStats {
      * @param applicationMetrics application metrics
      * @param origin             an origin
      */
-    public OriginMetrics(ApplicationMetrics applicationMetrics, Origin origin) {
+    public OriginMetrics(ApplicationMetrics applicationMetrics, String originId) {
         this.applicationMetrics = requireNonNull(applicationMetrics);
+        requireNonNull(originId);
 
         this.registry = this.applicationMetrics.metricRegistry();
-        this.requestMetricPrefix = requestMetricPrefix(requireNonNull(origin));
+        this.requestMetricPrefix = name(originId, "requests");
 
         this.requestSuccessMeter = this.registry.meter(name(this.requestMetricPrefix, "success-rate"));
         this.requestErrorMeter = this.registry.meter(name(this.requestMetricPrefix, "error-rate"));
@@ -84,9 +85,9 @@ public OriginMetrics(ApplicationMetrics applicationMetrics, Origin origin) {
      * @param metricRegistry a metrics registry
      * @return a new OriginMetrics
      */
-    public static OriginMetrics create(Origin origin, MetricRegistry metricRegistry) {
-        ApplicationMetrics appMetrics = new ApplicationMetrics(origin.applicationId(), metricRegistry);
-        return new OriginMetrics(appMetrics, origin);
+    public static OriginMetrics create(Id appId, String originId, MetricRegistry metricRegistry) {
+        ApplicationMetrics appMetrics = new ApplicationMetrics(appId, metricRegistry);
+        return new OriginMetrics(appMetrics, originId);
     }
 
     @Override
@@ -141,11 +142,4 @@ private int httpStatusCodeClass(int code) {
         return code / 100;
     }
 
-    private static String originName(Origin origin) {
-        return name(origin.id().toString());
-    }
-
-    private String requestMetricPrefix(Origin origin) {
-        return originName(origin) + ".requests";
-    }
 }

components/client/src/test/unit/java/com/hotels/styx/client/applications/metrics/OriginMetricsTest.java

@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2013-2018 Expedia Inc.
+  Copyright (C) 2013-2019 Expedia Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@
 import static com.hotels.styx.client.applications.OriginStats.REQUEST_SUCCESS;
 import static com.hotels.styx.client.netty.MetricsSupport.IsNotUpdated.hasNotReceivedUpdatesExcept;
 import static com.hotels.styx.client.netty.MetricsSupport.name;
+import static java.lang.String.format;
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static java.util.concurrent.TimeUnit.SECONDS;
@@ -66,7 +67,7 @@ public OriginMetricsTest() {
     private void setUp() {
         rootMetricRegistry = new StubClockMeterMetricRegistry(clock);
         appMetrics = new ApplicationMetrics(appId, rootMetricRegistry);
-        originMetrics = new OriginMetrics(appMetrics, origin);
+        originMetrics = new OriginMetrics(appMetrics, originPrefix(origin));
     }
 
     @AfterMethod
@@ -82,7 +83,7 @@ private void clearMetricsRegistry() {
 
     @Test(expectedExceptions = NullPointerException.class)
     public void failsIfCreatedWithNullApplicationMetrics() {
-        new OriginMetrics(null, origin);
+        new OriginMetrics(null, originPrefix(origin));
     }
 
     @Test(expectedExceptions = NullPointerException.class)
@@ -92,7 +93,7 @@ public void failsIfCreatedWithNullOrigin() {
 
     @Test
     public void successfullyCreated() {
-        assertThat(new OriginMetrics(appMetrics, origin), is(notNullValue()));
+        assertThat(new OriginMetrics(appMetrics, originPrefix(origin)), is(notNullValue()));
     }
 
     @Test
@@ -120,8 +121,8 @@ public void requestWithSuccessGetsAggregatedToApplication() {
                 .id("h2")
                 .build();
 
-        OriginMetrics originMetricsA = new OriginMetrics(appMetrics, originA);
-        OriginMetrics originMetricsB = new OriginMetrics(appMetrics, originB);
+        OriginMetrics originMetricsA = new OriginMetrics(appMetrics, originPrefix(originA));
+        OriginMetrics originMetricsB = new OriginMetrics(appMetrics, originPrefix(originB));
 
         originMetricsA.requestSuccess();
         originMetricsA.requestSuccess();
@@ -164,8 +165,8 @@ public void requestWithFailureGetsAggregatedToApplication() {
                 .applicationId(this.appId)
                 .build();
 
-        OriginMetrics originMetricsA = new OriginMetrics(appMetrics, originA);
-        OriginMetrics originMetricsB = new OriginMetrics(appMetrics, originB);
+        OriginMetrics originMetricsA = new OriginMetrics(appMetrics, originPrefix(originA));
+        OriginMetrics originMetricsB = new OriginMetrics(appMetrics, originPrefix(originB));
 
         originMetricsA.requestError();
         originMetricsA.requestError();
@@ -489,4 +490,9 @@ private Meter newMeter() {
             return new Meter(clock);
         }
     }
+
+    private static String originPrefix(Origin origin) {
+        return origin.id().toString();
+    }
+
 }

components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/RequestsToOriginMetricsCollectorTest.java

@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2013-2018 Expedia Inc.
+  Copyright (C) 2013-2019 Expedia Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -56,6 +56,7 @@
 import static io.netty.handler.codec.http.HttpResponseStatus.MOVED_PERMANENTLY;
 import static io.netty.handler.codec.http.HttpResponseStatus.NOT_IMPLEMENTED;
 import static io.netty.handler.codec.http.HttpResponseStatus.OK;
+import static java.lang.String.format;
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.hamcrest.CoreMatchers.is;
@@ -97,7 +98,7 @@ public RequestsToOriginMetricsCollectorTest() {
     private void setUp() {
         this.metricRegistry = new CodaHaleMetricRegistry();
         ApplicationMetrics appMetrics = new ApplicationMetrics(this.appId, this.metricRegistry);
-        this.originMetrics = new OriginMetrics(appMetrics, this.origin);
+        this.originMetrics = new OriginMetrics(appMetrics, originPrefix(this.origin));
     }
 
     @AfterMethod
@@ -113,7 +114,7 @@ private void clearMetricsRegistry() {
 
     private EmbeddedChannel buildEmbeddedChannel() {
         ApplicationMetrics appMetrics = new ApplicationMetrics(this.origin.applicationId(), this.metricRegistry);
-        OriginMetrics originMetrics = new OriginMetrics(appMetrics, this.origin);
+        OriginMetrics originMetrics = new OriginMetrics(appMetrics, originPrefix(this.origin));
 
         return new EmbeddedChannel(
                 new HttpClientCodec(),
@@ -499,4 +500,8 @@ private static HttpResponse mockHttpResponseWithCode(int code) {
         return msg;
     }
 
+    private static String originPrefix(Origin origin) {
+        return origin.id().toString();
+    }
+
 }

components/proxy/src/main/java/com/hotels/styx/admin/handlers/RoutingObjectHandler.java

@@ -49,7 +49,6 @@
 import static com.hotels.styx.api.HttpResponseStatus.OK;
 import static com.hotels.styx.infrastructure.configuration.json.ObjectMappers.addStyxMixins;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Collections.emptyList;
 import static java.util.stream.Collectors.joining;
 import static org.slf4j.LoggerFactory.getLogger;
 
@@ -97,7 +96,7 @@ public RoutingObjectHandler(StyxObjectStore<RoutingObjectRecord> routeDatabase,
 
                     try {
                         StyxObjectDefinition payload = YAML_MAPPER.readValue(body, StyxObjectDefinition.class);
-                        RoutingMetadataDecorator decorator = new RoutingMetadataDecorator(Builtins.build(emptyList(), routingObjectFactoryContext, payload));
+                        RoutingMetadataDecorator decorator = new RoutingMetadataDecorator(Builtins.build(ImmutableList.of(name), routingObjectFactoryContext, payload));
 
                         routeDatabase.insert(name, new RoutingObjectRecord(payload.type(), ImmutableSet.copyOf(payload.tags()), payload.config(), decorator))
                                 .ifPresent(previous -> previous.getRoutingObject().stop());

components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java

@@ -26,7 +26,6 @@
 import com.hotels.styx.api.extension.service.BackendService;
 import com.hotels.styx.api.extension.service.ConnectionPoolSettings;
 import com.hotels.styx.api.extension.service.HealthCheckConfig;
-import com.hotels.styx.api.extension.service.TlsSettings;
 import com.hotels.styx.api.extension.service.spi.Registry;
 import com.hotels.styx.client.BackendServiceClient;
 import com.hotels.styx.client.Connection;
@@ -43,7 +42,6 @@
 import com.hotels.styx.client.healthcheck.OriginHealthStatusMonitorFactory;
 import com.hotels.styx.client.healthcheck.UrlRequestHealthCheck;
 import com.hotels.styx.client.netty.connectionpool.NettyConnectionFactory;
-import com.hotels.styx.common.format.HttpMessageFormatter;
 import com.hotels.styx.server.HttpRouter;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.socket.SocketChannel;
@@ -124,15 +122,11 @@ public void onChange(Registry.Changes<BackendService> changes) {
             ConnectionPoolSettings poolSettings = backendService.connectionPoolConfig();
 
             Connection.Factory connectionFactory = connectionFactory(
-                    nettyEventLoopGroup,
-                    socketChannelClass,
-                    backendService.responseTimeoutMillis(),
-                    backendService.tlsSettings().orElse(null),
+                    backendService,
                     requestLoggingEnabled,
                     longFormat,
                     originStatsFactory,
-                    poolSettings.connectionExpirationSeconds(),
-                    environment.httpMessageFormatter());
+                    poolSettings.connectionExpirationSeconds());
 
             ConnectionPool.Factory connectionPoolFactory = new SimpleConnectionPoolFactory.Builder()
                     .connectionFactory(connectionFactory)
@@ -183,29 +177,25 @@ private StyxHttpClient healthCheckClient(BackendService backendService) {
     }
 
     private Connection.Factory connectionFactory(
-            EventLoopGroup nettyEventLoopGroup,
-            Class<? extends SocketChannel> socketChannelClass,
-            int responseTimeoutMillis,
-            TlsSettings tlsSettings,
+            BackendService backendService,
             boolean requestLoggingEnabled,
             boolean longFormat,
             OriginStatsFactory originStatsFactory,
-            long connectionExpiration,
-            HttpMessageFormatter httpMessageFormatter) {
+            long connectionExpiration) {
 
         Connection.Factory factory = new NettyConnectionFactory.Builder()
                 .nettyEventLoop(nettyEventLoopGroup, socketChannelClass)
                 .httpRequestOperationFactory(
                         httpRequestOperationFactoryBuilder()
                                 .flowControlEnabled(true)
                                 .originStatsFactory(originStatsFactory)
-                                .responseTimeoutMillis(responseTimeoutMillis)
+                                .responseTimeoutMillis(backendService.responseTimeoutMillis())
                                 .requestLoggingEnabled(requestLoggingEnabled)
                                 .longFormat(longFormat)
-                                .httpMessageFormatter(httpMessageFormatter)
+                                .httpMessageFormatter(environment.httpMessageFormatter())
                                 .build()
                 )
-                .tlsSettings(tlsSettings)
+                .tlsSettings(backendService.tlsSettings().orElse(null))
                 .build();
 
         if (connectionExpiration > 0) {

components/proxy/src/main/java/com/hotels/styx/routing/handlers/HostProxy.java

@@ -46,6 +46,7 @@
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 
+import static com.hotels.styx.api.Id.id;
 import static com.hotels.styx.api.extension.Origin.newOriginBuilder;
 import static com.hotels.styx.api.extension.service.ConnectionPoolSettings.defaultConnectionPoolSettings;
 import static com.hotels.styx.client.HttpRequestOperationFactory.Builder.httpRequestOperationFactoryBuilder;
@@ -211,21 +212,23 @@ public RoutingObject build(List<String> fullName, Context context, StyxObjectDef
                     .orElse(DEFAULT_REQUEST_TIMEOUT);
 
             String metricPrefix = config.get("metricPrefix", String.class)
-                    .orElse("routing.objects.hostProxy");
+                    .orElse("routing.objects");
 
             HostAndPort hostAndPort = config.get("host")
                     .map(HostAndPort::fromString)
                     .map(it -> addDefaultPort(it, tlsSettings))
                     .orElseThrow(() -> missingAttributeError(configBlock, join(".", fullName), "host"));
 
+            String objectName = fullName.get(fullName.size() - 1);
+
             return createHostProxyHandler(
                     context.environment().metricRegistry(),
-                    hostAndPort.getHostText(),
-                    hostAndPort.getPort(),
+                    hostAndPort,
                     poolSettings,
                     tlsSettings,
                     responseTimeoutMillis,
-                    metricPrefix);
+                    metricPrefix,
+                    objectName);
         }
 
         private static HostAndPort addDefaultPort(HostAndPort hostAndPort, TlsSettings tlsSettings) {
@@ -243,21 +246,22 @@ private static HostAndPort addDefaultPort(HostAndPort hostAndPort, TlsSettings t
         @NotNull
         public static HostProxy createHostProxyHandler(
                 MetricRegistry metricRegistry,
-                String host,
-                int port,
+                HostAndPort hostAndPort,
                 ConnectionPoolSettings poolSettings,
                 TlsSettings tlsSettings,
                 int responseTimeoutMillis,
-                String metricPrefix) {
+                String metricPrefix,
+                String objectName) {
+
+            String host = hostAndPort.getHostText();
+            int port = hostAndPort.getPort();
 
             Origin origin = newOriginBuilder(host, port)
                     .applicationId(metricPrefix)
-                    .id(format("%s:%s", host, port))
+                    .id(objectName)
                     .build();
 
-            OriginMetrics originMetrics = OriginMetrics.create(
-                    origin,
-                    metricRegistry);
+            OriginMetrics originMetrics = OriginMetrics.create(id(metricPrefix), objectName, metricRegistry);
 
             ConnectionPool.Factory connectionPoolFactory = new SimpleConnectionPoolFactory.Builder()
                     .connectionFactory(

components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt

@@ -163,7 +163,8 @@ internal class OriginsConfigConverter(
                         app.connectionPoolConfig(),
                         app.tlsSettings().orElse(null),
                         app.responseTimeoutMillis(),
-                        origin))
+                        origin,
+                        "origins.${app.id()}.${origin.id()}"))
 
         private fun loadBalancingGroupConfig(origins: String,
                                              originRestrictionCookie: String?,
@@ -221,12 +222,13 @@ internal class OriginsConfigConverter(
         private fun hostProxyConfig(poolSettings: ConnectionPoolSettings,
                                     tlsSettings: TlsSettings?,
                                     responseTimeout: Int,
-                                    origin: Origin): JsonNode = MAPPER.valueToTree(
+                                    origin: Origin,
+                                    metricsPrefix: String): JsonNode = MAPPER.valueToTree(
                 HostProxyConfiguration(
                         "${origin.host()}:${origin.port()}",
                         poolSettings,
                         tlsSettings,
                         responseTimeout,
-                        "origins.${origin.id()}.${origin.id()}"))
+                        metricsPrefix))
     }
 }

pom.xml

@@ -176,7 +176,7 @@
     <jacoco.excludes />
 
     <!-- Checkstyle -->
-    <checkstyle.maxAllowedViolations>2</checkstyle.maxAllowedViolations>
+    <checkstyle.maxAllowedViolations>0</checkstyle.maxAllowedViolations>
     <checkstyle.basedir>codequality</checkstyle.basedir>
     <checkstyle.violationSeverity>error</checkstyle.violationSeverity>
     <checkstyle.skip>false</checkstyle.skip>