Cut back excessive logging. (#559)

ExpediaGroup · Dec 10, 2019 · d7d0fee · d7d0fee
1 parent 9ae5478
commit d7d0fee
Show file tree

Hide file tree

Showing 51 changed files with 263 additions and 130 deletions.
diff --git a/components/api/src/test/java/com/hotels/styx/api/UrlTest.java b/components/api/src/test/java/com/hotels/styx/api/UrlTest.java
@@ -93,8 +93,6 @@ public void decodesQueryParams() {
         Url url = url("http://example.com/?foo=bar")
                 .build();
 
-        System.out.println(url.query());
-
         assertThat(url.queryParam("foo"), isValue("bar"));
     }
 

diff --git a/components/api/src/test/resources/logback.xml b/components/api/src/test/resources/logback.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<configuration>
+
+  <root level="OFF">
+  </root>
+
+  <logger name="Styx-Tests" level="INFO"/>
+
+</configuration>
diff --git a/components/client/src/test/integration/resources/logback.xml b/components/client/src/test/integration/resources/logback.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<configuration>
+
+  <root level="OFF">
+  </root>
+
+  <logger name="Styx-Tests" level="INFO"/>
+  <logger name="com.hotels.styx.client.OriginsInventory" level="INFO"/>
+  <logger name="com.hotels.styx.http-messages.inbound" level="INFO"/>
+  <logger name="com.hotels.styx.http-messages.outbound" level="INFO"/>
+  <logger name="com.hotels.styx.client.OriginRestrictionLoadBalancingStrategy" level="INFO"/>
+  <logger name="com.hotels.styx.client.netty.connectionpool.FlowControllingHttpContentProducer" level="INFO"/>
+
+</configuration>
diff --git a/components/client/src/test/integration/scala/com/hotels/styx/client/RetryHandlingSpec.scala b/components/client/src/test/integration/scala/com/hotels/styx/client/RetryHandlingSpec.scala
@@ -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.
@@ -41,10 +41,13 @@ import io.netty.handler.codec.http.HttpHeaders.Names._
 import io.netty.handler.codec.http.HttpHeaders.Values._
 import io.netty.handler.codec.http.LastHttpContent
 import org.scalatest.{BeforeAndAfterAll, FunSuite, Matchers}
+import org.slf4j.LoggerFactory
 import reactor.core.publisher.Mono
 
 class RetryHandlingSpec extends FunSuite with BeforeAndAfterAll with Matchers with OriginSupport {
 
+  val LOGGER = LoggerFactory.getLogger(classOf[RetryHandlingSpec])
+
   val response = "Response From localhost"
 
   val server1 = new FakeHttpServer(0, "app", "HEALTHY_ORIGIN_ONE")
@@ -178,7 +181,7 @@ class RetryHandlingSpec extends FunSuite with BeforeAndAfterAll with Matchers wi
   private def doNotRespond(retryCount: AtomicInteger): (ChannelHandlerContext, Any) => Unit = {
     (ctx: ChannelHandlerContext, msg: scala.Any) => {
       if (msg.isInstanceOf[LastHttpContent]) {
-        println("Origin received request, but not responding.")
+        LOGGER.warn("Origin received request, but not responding.")
         retryCount.incrementAndGet
       }
     }
@@ -187,7 +190,7 @@ class RetryHandlingSpec extends FunSuite with BeforeAndAfterAll with Matchers wi
   private def doNotRespond(latch: CountDownLatch, responseCount: AtomicInteger): (ChannelHandlerContext, Any) => Unit = {
     (ctx: ChannelHandlerContext, msg: scala.Any) => {
       if (msg.isInstanceOf[LastHttpContent]) {
-        println("Origin received request, but not responding.")
+        LOGGER.warn("Origin received request, but not responding.")
         responseCount.incrementAndGet
         latch.countDown()
       }

diff --git a/components/client/src/test/unit/resources/logback.xml b/components/client/src/test/unit/resources/logback.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<configuration>
+
+  <root level="OFF">
+  </root>
+
+  <logger name="Styx-Tests" level="INFO"/>
+  <logger name="com.hotels.styx.client.OriginsInventory" level="INFO"/>
+  <logger name="com.hotels.styx.http-messages.inbound" level="INFO"/>
+  <logger name="com.hotels.styx.http-messages.outbound" level="INFO"/>
+  <logger name="com.hotels.styx.client.OriginRestrictionLoadBalancingStrategy" level="INFO"/>
+  <logger name="com.hotels.styx.client.netty.connectionpool.FlowControllingHttpContentProducer" level="INFO"/>
+
+</configuration>
diff --git a/components/common/src/test/java/com/hotels/styx/common/QueueDrainingEventProcessorTest.java b/components/common/src/test/java/com/hotels/styx/common/QueueDrainingEventProcessorTest.java
@@ -120,7 +120,7 @@ private void await(CyclicBarrier barrier) {
         } catch (InterruptedException e) {
             e.printStackTrace();
             currentThread().interrupt();
-            ;
+
             throw new RuntimeException(e);
         } catch (BrokenBarrierException e) {
             throw new RuntimeException(e);

diff --git a/components/common/src/test/resources/logback.xml b/components/common/src/test/resources/logback.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<configuration>
+
+  <root level="OFF">
+  </root>
+
+  <logger name="Styx-Tests" level="INFO"/>
+  <logger name="com.hotels.styx.common.FsmEventProcessor" level="INFO"/>
+
+</configuration>
diff --git a/components/proxy/src/test/java/com/hotels/styx/StyxServerTest.java b/components/proxy/src/test/java/com/hotels/styx/StyxServerTest.java
@@ -40,12 +40,9 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInstance;
 
-import java.lang.reflect.Field;
-import java.nio.file.Paths;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 

diff --git a/...s/proxy/src/test/java/com/hotels/styx/applications/yaml/YamlApplicationsProviderTest.java b/...s/proxy/src/test/java/com/hotels/styx/applications/yaml/YamlApplicationsProviderTest.java
@@ -20,12 +20,13 @@
 import com.hotels.styx.api.extension.service.RewriteConfig;
 import com.hotels.styx.api.extension.service.TlsSettings;
 import com.hotels.styx.applications.BackendServices;
-import com.hotels.styx.server.routing.antlr.DslFunctionResolutionError;
 import org.hamcrest.CoreMatchers;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.List;
@@ -52,10 +53,10 @@
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.matchesPattern;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class YamlApplicationsProviderTest {
+    private static final Logger LOGGER = LoggerFactory.getLogger(YamlApplicationsProviderTest.class);
 
     private static final String ORIGINS_FILE = fixturesHome() + "conf/origins/origins-for-https-test.yml";
     private static final String OLD_ORIGINS_FILE = fixturesHome() + "conf/origins/origins-for-https-test-ssl-settings.yml";
@@ -119,7 +120,7 @@ public void canGetApplications(String path) {
         YamlApplicationsProvider config = loadFromPath(path);
 
         StreamSupport.stream(config.get().spliterator(), false)
-                .forEach(service -> System.out.println(service.id() + " - " + service.healthCheckConfig()));
+                .forEach(service -> LOGGER.info(service.id() + " - " + service.healthCheckConfig()));
 
         assertThat(config.get(), containsInAnyOrder(
                 anApplication()

diff --git a/components/proxy/src/test/java/com/hotels/styx/infrastructure/FileBackedRegistryTest.java b/components/proxy/src/test/java/com/hotels/styx/infrastructure/FileBackedRegistryTest.java
@@ -52,7 +52,8 @@ public void setUp() {
         originalContent = "... origins file ...".getBytes(UTF_8);
         newContent = "... new file ...".getBytes(UTF_8);
         backendService = new BackendService.Builder().build();
-        listener = mock(Registry.ChangeListener.class, withSettings().verboseLogging());
+        listener = mock(Registry.ChangeListener.class);
+        listener = mock(Registry.ChangeListener.class);
     }
 
     @Test

diff --git a/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java b/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java
@@ -180,7 +180,6 @@ public void doesNotMatchRequestIfFinalSlashIsMissing() {
 
         LiveHttpRequest request = get("/ba/").build();
         Optional<HttpHandler> route = router.route(request, context);
-        System.out.println("route: " + route);
 
         assertThat(route, is(Optional.empty()));
     }

diff --git a/components/proxy/src/test/java/com/hotels/styx/proxy/StyxProxyTest.java b/components/proxy/src/test/java/com/hotels/styx/proxy/StyxProxyTest.java
@@ -36,6 +36,8 @@
 import com.hotels.styx.server.netty.WebServerConnectorFactory;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 
@@ -47,6 +49,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 
 public class StyxProxyTest extends SSLSetup {
+    private static final Logger LOGGER = LoggerFactory.getLogger(StyxProxyTest.class);
     final HttpClient client = new StyxHttpClient.Builder()
             .build();
 
@@ -121,7 +124,7 @@ public void startsServerWithBothHttpAndHttpsConnectors() throws IOException {
         server.startAsync().awaitRunning();
         assertThat("Server should be running", server.isRunning());
 
-        System.out.println("server is running: " + server.isRunning());
+        LOGGER.info("server is running: " + server.isRunning());
 
         HttpResponse clearResponse = get("http://localhost:8080/search?q=fanta");
         assertThat(clearResponse.bodyAs(UTF_8), containsString("Response from http Connector"));

diff --git a/...s/proxy/src/test/java/com/hotels/styx/startup/extensions/PluginLoadingForStartupTest.java b/...s/proxy/src/test/java/com/hotels/styx/startup/extensions/PluginLoadingForStartupTest.java
@@ -45,7 +45,6 @@
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.matchesPattern;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class PluginLoadingForStartupTest {

diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/Responses.kt b/components/proxy/src/test/kotlin/com/hotels/styx/Responses.kt
@@ -17,15 +17,18 @@ package com.hotels.styx
 
 import com.hotels.styx.api.Eventual
 import com.hotels.styx.api.LiveHttpResponse
+import org.slf4j.LoggerFactory
 import reactor.core.publisher.toMono
 import java.nio.charset.StandardCharsets
 
 
+private val LOGGER = LoggerFactory.getLogger("com.hotels.styx.Responses")
+
 fun Eventual<LiveHttpResponse>.wait(maxBytes: Int = 100*1024, debug: Boolean = false) = this.toMono()
         .flatMap { it.aggregate(maxBytes).toMono() }
         .doOnNext {
             if (debug) {
-                println("${it.status()} - ${it.headers()} - ${it.bodyAs(StandardCharsets.UTF_8)}")
+                LOGGER.info("${it.status()} - ${it.headers()} - ${it.bodyAs(StandardCharsets.UTF_8)}")
             }
         }
         .block()
diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/admin/OriginsPageRendererTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/admin/OriginsPageRendererTest.kt
@@ -122,9 +122,6 @@ class OriginsPageRendererTest : FunSpec({
 
         val page = OriginsPageRenderer(assetsRoot, "dcProvider", routeDb).render()
 
-        println("page: ")
-        println(page)
-
         test("Provider name appears in title") {
             page.shouldContain("<h3 class=\"title grey-text text-darken-3 left-align\">Configured Services</h3>")
             page.shouldContain("<h6 class=\"title grey-text text-darken-3 left-align\">Provider: dcProvider</h6>")
@@ -204,9 +201,6 @@ class OriginsPageRendererTest : FunSpec({
 
         val page = OriginsPageRenderer(assetsRoot, "dcProvider", routeDb).render()
 
-        println("page: ")
-        println(page)
-
         test("It detects them wrapped LoadBalancingGroup objects as configured applications") {
             page.shouldContain("<h6>/rewrites/ -&gt; appWithRewrites</h6>")
         }

diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/admin/handlers/RoutingObjectHandlerTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/admin/handlers/RoutingObjectHandlerTest.kt
@@ -123,7 +123,6 @@ class RoutingObjectHandlerTest : FeatureSpec({
                     .toMono()
                     .block()
                     .let {
-                        println(it!!.bodyAs(UTF_8))
                         it.status() shouldBe CREATED
                     }
 

diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/routing/handlers/LoadBalancingGroupTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/routing/handlers/LoadBalancingGroupTest.kt
@@ -39,6 +39,7 @@ import io.kotlintest.specs.FeatureSpec
 import io.mockk.mockk
 import io.mockk.verify
 import org.reactivestreams.Publisher
+import org.slf4j.LoggerFactory
 import reactor.core.Disposable
 import reactor.core.publisher.toFlux
 import reactor.core.publisher.toMono
@@ -47,6 +48,8 @@ import java.time.Duration
 
 class LoadBalancingGroupTest : FeatureSpec() {
 
+    private val LOGGER = LoggerFactory.getLogger(LoadBalancingGroupTest::class.java)
+
     // Tests depend on each other. So run the tests sequentially:
     override fun isolationMode(): IsolationMode = IsolationMode.SingleInstance
 
@@ -177,7 +180,7 @@ class LoadBalancingGroupTest : FeatureSpec() {
 
                 invocations.forEach {
                     val response = it.block()
-                    println("response: ${response.bodyAs(UTF_8)}")
+                    LOGGER.debug("response: ${response.bodyAs(UTF_8)}")
                 }
             }
 

diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/services/FileMonitoringServiceTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/services/FileMonitoringServiceTest.kt
@@ -21,18 +21,20 @@ import io.kotlintest.eventually
 import io.kotlintest.seconds
 import io.kotlintest.shouldBe
 import io.kotlintest.specs.StringSpec
+import org.slf4j.LoggerFactory
 import java.io.File
 import java.nio.charset.StandardCharsets.UTF_8
 import java.util.concurrent.atomic.AtomicInteger
 import java.util.concurrent.atomic.AtomicReference
 
 class FileMonitoringServiceTest : StringSpec() {
+    val LOGGER = LoggerFactory.getLogger(FileMonitoringServiceTest::class.java)
 
     val tempDir = createTempDir(suffix = "-${this.javaClass.simpleName}")
     val monitoredFile = File("${tempDir.absolutePath}/config.yml")
 
     override fun beforeSpec(spec: Spec) {
-        println("Temp directory: " + tempDir.absolutePath)
+        LOGGER.info("Temp directory: " + tempDir.absolutePath)
     }
 
     override fun beforeTest(testCase: TestCase) {

diff --git a/...onents/proxy/src/test/kotlin/com/hotels/styx/services/HealthCheckMonitoringServiceTest.kt b/...onents/proxy/src/test/kotlin/com/hotels/styx/services/HealthCheckMonitoringServiceTest.kt
@@ -33,11 +33,13 @@ import io.kotlintest.specs.FeatureSpec
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
+import org.slf4j.LoggerFactory
 import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.ScheduledFuture
 import java.util.concurrent.TimeUnit.MILLISECONDS
 
 class HealthCheckMonitoringServiceTest : FeatureSpec({
+    val LOGGER = LoggerFactory.getLogger(HealthCheckMonitoringServiceTest::class.java)
 
     fun createdTag(tag: String) = tag.matches("created:.*".toRegex())
 
@@ -217,7 +219,7 @@ class HealthCheckMonitoringServiceTest : FeatureSpec({
         scenario("Re-tag an active object") {
             reTag(setOf(lbGroupTag("aaa"), "state=active", "healthCheck=on;probes-FAIL:0"), ObjectActive(1))
                     .let {
-                        println("new tags: " + it)
+                        LOGGER.info("new tags: " + it)
 
                         it.shouldContainExactlyInAnyOrder(
                                 lbGroupTag("aaa"),
@@ -229,7 +231,7 @@ class HealthCheckMonitoringServiceTest : FeatureSpec({
         scenario("Re-tag an unreachable object") {
             reTag(setOf(lbGroupTag("aaa"), "state=unreachable", "healthCheck=on;probes-OK:0"), ObjectActive(1))
                     .let {
-                        println("new tags: " + it)
+                        LOGGER.info("new tags: " + it)
 
                         it.shouldContainExactlyInAnyOrder(
                                 lbGroupTag("aaa"),
@@ -241,7 +243,7 @@ class HealthCheckMonitoringServiceTest : FeatureSpec({
         scenario("Re-tag a failed active object as unreachable") {
             reTag(setOf(lbGroupTag("aaa"), "state=active", "healthCheck=on;probes-FAIL:1"), ObjectUnreachable(0))
                     .let {
-                        println("new tags: " + it)
+                        LOGGER.info("new tags: " + it)
 
                         it.shouldContainExactlyInAnyOrder(
                                 lbGroupTag("aaa"),
@@ -253,7 +255,7 @@ class HealthCheckMonitoringServiceTest : FeatureSpec({
         scenario("Re-tag a successful unreachable object as active") {
             reTag(setOf(lbGroupTag("aaa"), "state=unreachable", "healthCheck=on;probes-OK:1"), ObjectActive(0))
                     .let {
-                        println("new tags: " + it)
+                        LOGGER.info("new tags: " + it)
 
                         it.shouldContainExactlyInAnyOrder(
                                 lbGroupTag("aaa"),

diff --git a/...t/kotlin/com/hotels/styx/services/YamlFileConfigurationServiceDuplicateIdentifiersTest.kt b/...t/kotlin/com/hotels/styx/services/YamlFileConfigurationServiceDuplicateIdentifiersTest.kt
@@ -81,7 +81,7 @@ class YamlFileConfigurationServiceDuplicateIdentifiersTest : FunSpec() {
                 Thread.sleep(500)
 
                 objectStore.entrySet().forEach {
-                    println("entry: ${it.key} -> ${it.value.type} - ${it.value.tags}")
+                    LOGGER.debug("entry: ${it.key} -> ${it.value.type} - ${it.value.tags}")
                 }
 
                 objectStore.entrySet().size.shouldBe(4)
@@ -114,10 +114,10 @@ class YamlFileConfigurationServiceDuplicateIdentifiersTest : FunSpec() {
                 Thread.sleep(500)
 
                 objectStore.entrySet().forEach {
-                    println("routing entry: ${it.key} -> ${it.value.type} - ${it.value.tags}")
+                    LOGGER.debug("routing entry: ${it.key} -> ${it.value.type} - ${it.value.tags}")
                 }
                 serviceDb.entrySet().forEach {
-                    println("service entry: ${it.key} -> ${it.value.type} - ${it.value.tags}")
+                    LOGGER.debug("service entry: ${it.key} -> ${it.value.type} - ${it.value.tags}")
                 }
 
                 serviceDb.entrySet().size.shouldBe(1)