[ostrich]: ServerConfig: Prefer configuring loggers with LoggerFactor…

…y when available over LoggerConfig

RB_ID=591744

src/main/scala/com/twitter/ostrich/admin/config/ServerConfig.scala

@@ -18,24 +18,33 @@ package com.twitter.ostrich
 package admin
 package config
 
-import com.twitter.logging.Logger
+import com.twitter.logging.{Logger, LoggerFactory}
 import com.twitter.logging.config.LoggerConfig
 import com.twitter.util.Config
 
 @deprecated("no direct replacement")
 abstract class ServerConfig[T <: Service] extends Config[RuntimeEnvironment => T] {
   var loggers: List[LoggerConfig] = Nil
+  protected def loggerFactories: List[LoggerFactory] = Nil
   var admin = new AdminServiceConfig()
 
   protected var httpServer: Option[AdminHttpService] = None
 
+  private[config] def configureLogging(): Unit = {
+    if (loggerFactories.isEmpty) {
+      Logger.configure(loggers)
+    } else {
+      Logger.configure(loggerFactories)
+    }
+  }
+
   def apply() = { (runtime: RuntimeEnvironment) =>
-    Logger.configure(loggers)
+    configureLogging()
     httpServer = admin()(runtime)
     val service = apply(runtime)
     ServiceTracker.register(service)
     service
   }
 
   def apply(runtime: RuntimeEnvironment): T
-}
+}

src/test/scala/com/twitter/ostrich/admin/config/ServerConfigSpec.scala

@@ -0,0 +1,76 @@
+package com.twitter.ostrich.admin.config
+
+import com.twitter.ostrich.admin.Service
+import com.twitter.logging._
+import com.twitter.logging.config.{FileHandlerConfig, LoggerConfig}
+import com.twitter.ostrich.admin.RuntimeEnvironment
+import org.junit.runner.RunWith
+import org.scalatest.{BeforeAndAfter, FunSuite}
+import org.scalatest.junit.JUnitRunner
+import java.util.{logging => javalog}
+
+@RunWith(classOf[JUnitRunner])
+class ServerConfigSpec extends FunSuite with BeforeAndAfter {
+  val logLevel = Logger.levelNames(Option[String](System.getenv("log")).getOrElse("FATAL").toUpperCase)
+
+  private val logger = Logger.get("")
+  private var oldLevel: javalog.Level = _
+
+  before {
+    oldLevel = logger.getLevel()
+    logger.setLevel(logLevel)
+    logger.addHandler(new ConsoleHandler(new Formatter(), None))
+  }
+
+  after {
+    logger.clearHandlers()
+    logger.setLevel(oldLevel)
+  }
+
+  class SampleService extends Service {
+    def start() {}
+    def shutdown() {}
+  }
+
+  class TestServerConfig extends ServerConfig[SampleService] {
+    override def apply(runtime: RuntimeEnvironment): SampleService = new SampleService()
+  }
+
+  val sampleFactory = List(LoggerFactory(
+    node = "fromFactory",
+    level = Some(Level.INFO),
+    handlers = List(ScribeHandler(
+      category = "fromFactory"
+    ))
+  ))
+
+  val sampleConfig = List(new LoggerConfig {
+    node = "fromConfig"
+    level = Some(Level.INFO)
+    handlers = new FileHandlerConfig {
+      filename = "fromConfig"
+    }
+  })
+
+  test("configure Logger with loggerConfig when LoggerFactories not specified") {
+    val serverConfig = new TestServerConfig {
+      override def loggerFactories = Nil
+      loggers = sampleConfig
+    }
+    Logger.clearHandlers()
+    serverConfig.configureLogging()
+    assert(Logger.get("fromFactory").getHandlers().size == 0)
+    assert(Logger.get("fromConfig").getHandlers().size == 1)
+  }
+
+  test("configure Logger with loggerFactories when LoggerFactories specified") {
+    val serverConfig = new TestServerConfig {
+      override def loggerFactories = sampleFactory
+      loggers = sampleConfig
+    }
+    Logger.clearHandlers()
+    serverConfig.configureLogging()
+    assert(Logger.get("fromFactory").getHandlers().size == 1)
+    assert(Logger.get("fromConfig").getHandlers().size == 0)
+  }
+}