Skip to content

Commit

Permalink
Small cleanup: Twitter Server and Finatra
Browse files Browse the repository at this point in the history
Problem

Converting Twitter Server to use Finagle HTTPx made some code
redundant. It also introduced a bug, where stats in the
Metrics section of the admin handler were doubled.

Solution

Eliminate redundancies and fix the stat doubling bug.

RB_ID=740731
TBR=true
  • Loading branch information
luciferous authored and jenkins committed Sep 21, 2015
1 parent 514ee3a commit 45133d9
Show file tree
Hide file tree
Showing 36 changed files with 135 additions and 119 deletions.
7 changes: 7 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ Note that ``RB_ID=#`` correspond to associated messages in commits.
1.x
-----

Dependencies:
-------------

* Converted to finagle-httpx. Projects that depend transitively on
finagle-http through twitter-server will need to switch to finagle-httpx.
RB_ID=741454 RB_ID=740731

1.13.0
~~~~~~~

Expand Down
1 change: 0 additions & 1 deletion bin/travisci
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ if [ "$TWITTER_SERVER_BRANCH" != "master" ]; then
git clone https://github.com/twitter/finagle.git --branch develop
cd finagle
$TWITTER_SERVER_SBT ++$TRAVIS_SCALA_VERSION finagle-core/publishLocal
$TWITTER_SERVER_SBT ++$TRAVIS_SCALA_VERSION finagle-http/publishLocal
$TWITTER_SERVER_SBT ++$TRAVIS_SCALA_VERSION finagle-httpx/publishLocal
$TWITTER_SERVER_SBT ++$TRAVIS_SCALA_VERSION finagle-httpx-compat/publishLocal
$TWITTER_SERVER_SBT ++$TRAVIS_SCALA_VERSION finagle-thrift/publishLocal
Expand Down
21 changes: 10 additions & 11 deletions src/main/scala/com/twitter/server/AdminHttpServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package com.twitter.server
import com.twitter.app.App
import com.twitter.finagle.client.ClientRegistry
import com.twitter.finagle.httpx.{Response, Request, HttpMuxer}
import com.twitter.finagle.httpx
import com.twitter.finagle.server.ServerRegistry
import com.twitter.finagle.stats.NullStatsReceiver
import com.twitter.finagle.tracing.NullTracer
Expand Down Expand Up @@ -36,7 +35,7 @@ object AdminHttpServer {
*/
case class Route(
path: String,
handler: Service[HttpUtils.Request, HttpUtils.Response],
handler: Service[Request, Response],
alias: String,
group: Option[String],
includeInIndex: Boolean)
Expand All @@ -59,7 +58,7 @@ object AdminHttpServer {
*/
def mkRoutex(
path: String,
handler: Service[httpx.Request, httpx.Response],
handler: Service[Request, Response],
alias: String,
group: Option[String],
includeInIndex: Boolean
Expand All @@ -77,12 +76,13 @@ trait AdminHttpServer { self: App =>
def defaultHttpPort: Int = 9990
val adminPort = flag("admin.port", new InetSocketAddress(defaultHttpPort), "Admin http server port")

private[this] val adminHttpMuxer = new Service[HttpUtils.Request, HttpUtils.Response] {
override def apply(request: HttpUtils.Request): Future[HttpUtils.Response] = underlying(request)
private[this] val adminHttpMuxer = new Service[Request, Response] {
override def apply(request: Request): Future[Response] = underlying(request)

@volatile var underlying: Service[HttpUtils.Request, HttpUtils.Response] =
new Service[HttpUtils.Request, HttpUtils.Response] {
def apply(request: HttpUtils.Request): Future[HttpUtils.Response] = HttpUtils.new404("no admin server initialized")
@volatile var underlying: Service[Request, Response] =
new Service[Request, Response] {
def apply(request: Request): Future[Response] =
HttpUtils.new404("no admin server initialized")
}
}

Expand Down Expand Up @@ -111,7 +111,7 @@ trait AdminHttpServer { self: App =>
// Stat libraries join the global muxer namespace.
// Special case and group them here.
val (metricLinks, otherLinks) = {
val links = (HttpMuxer.patterns ++ httpx.HttpMuxer.patterns).map {
val links = HttpMuxer.patterns.map {
case path@"/admin/metrics.json" => IndexView.Link(path, s"$path?pretty=true")
case path => IndexView.Link(path, path)
}
Expand Down Expand Up @@ -158,14 +158,13 @@ trait AdminHttpServer { self: App =>
}

// create a service which multiplexes across all endpoints.
val httpxMuxer: Service[Request, Response] = httpx.HttpMuxer
val localMuxer = allRoutes.foldLeft(new HttpMuxer) {
case (muxer, route) =>
log.info(s"${route.path} => ${route.handler.getClass.getName}")
val service = new IndexView(route.alias, route.path, index) andThen route.handler
muxer.withHandler(route.path, service)
}
adminHttpMuxer.underlying = HttpUtils.combine(Seq(localMuxer, httpxMuxer, HttpMuxer))
adminHttpMuxer.underlying = HttpUtils.combine(Seq(localMuxer, HttpMuxer))
}

private[this] def startServer() = {
Expand Down
3 changes: 2 additions & 1 deletion src/main/scala/com/twitter/server/handler/AbortHandler.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.server.util.HttpUtils._
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.server.util.HttpUtils.newOk
import com.twitter.util.Future
import java.util.logging.Logger

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package com.twitter.server.handler

import com.twitter.finagle.{Announcer, Service}
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.util.Future
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.newOk

class AnnouncerHandler extends Service[Request, Response] {
def apply(req: Request): Future[Response] = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package com.twitter.server.handler

import com.twitter.finagle.client.ClientRegistry
import com.twitter.finagle.Service
import com.twitter.finagle.client.ClientRegistry
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.finagle.util.StackRegistry
import com.twitter.io.Buf
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{parse, new404, newResponse}
import com.twitter.server.util.MetricSource
import com.twitter.server.view.StackRegistryView
import com.twitter.util.Future
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.jvm.ContentionSnapshot
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.newOk
import com.twitter.util.Future

class ContentionHandler extends Service[Request, Response] {
Expand Down
3 changes: 2 additions & 1 deletion src/main/scala/com/twitter/server/handler/DtabHandler.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package com.twitter.server.handler

import com.twitter.finagle.{Dtab, Service}
import com.twitter.server.util.HttpUtils._
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.server.util.HttpUtils.newOk
import com.twitter.util.Future

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.io.Buf
import com.twitter.util.Future
import com.twitter.util.events.Sink
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.newResponse
import java.util.logging.Logger

private[server] object EventRecordingHandler {
Expand Down
6 changes: 3 additions & 3 deletions src/main/scala/com/twitter/server/handler/EventsHandler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package com.twitter.server.handler

import com.twitter.concurrent.exp.AsyncStream
import com.twitter.finagle.Service
import com.twitter.finagle.httpx
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.finagle.tracing.SpanId
import com.twitter.io.{Reader, Buf}
import com.twitter.server.handler.EventRecordingHandler._
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{accepts, expectsJson}
import com.twitter.server.util.{JsonSink, TraceEventSink}
import com.twitter.util.events.{Sink, Event}
import com.twitter.util.{Future, Throw, Try}
Expand All @@ -29,7 +29,7 @@ private[server] class EventsHandler(sink: Sink) extends Service[Request, Respons
else respond(Html, htmlSerialize(sink))

private[this] def respond(contentType: String, reader: Reader): Future[Response] = {
val response = httpx.Response()
val response = Response()
response.contentType = contentType
response.setChunked(true)
Reader.copy(reader, response.writer).onFailure { e =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package com.twitter.server.handler

import com.twitter.conversions.time._
import com.twitter.finagle.httpx.Status
import com.twitter.finagle.httpx.{Request, Response, Status}
import com.twitter.finagle.Service
import com.twitter.io.Buf
import com.twitter.jvm.Heapster
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{newResponse, parse}
import com.twitter.util.{Duration, Future}
import java.util.logging.Logger

Expand Down
6 changes: 3 additions & 3 deletions src/main/scala/com/twitter/server/handler/IndexHandler.scala
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.twitter.server.handler

import com.twitter.finagle.httpx.HttpMuxer
import com.twitter.finagle.{Service, httpx}
import com.twitter.finagle.httpx.{HttpMuxer, Request, Response}
import com.twitter.io.Buf
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{expectsHtml, newOk, newResponse}
import com.twitter.util.Future

/**
Expand All @@ -24,4 +24,4 @@ class IndexHandler(
content = Buf.Utf8(links.mkString("<br />\n"))
)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.io.Buf
import com.twitter.logging.{Level, Logger}
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{expectsHtml, newResponse, parse}
import com.twitter.util.Future
import java.net.URLEncoder
import java.util.{logging => javalog}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.io.Buf
import com.twitter.io.Charsets
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{newResponse, parse}
import com.twitter.server.util.{JsonConverter, MetricSource}
import com.twitter.util.{Future, Time}

Expand Down Expand Up @@ -54,4 +55,4 @@ class MetricQueryHandler(source: MetricSource = new MetricSource)
)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package com.twitter.server.handler

import com.twitter.conversions.time._
import com.twitter.finagle.httpx.Status
import com.twitter.finagle.httpx.{Request, Response, Status}
import com.twitter.finagle.Service
import com.twitter.io.Buf
import com.twitter.jvm.CpuProfile
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{newResponse, parse}
import com.twitter.util.{Duration, Future, Return, Throw}
import java.io.ByteArrayOutputStream
import java.util.logging.Logger
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.util.registry.{Formatter, GlobalRegistry}
import com.twitter.util.Future
import com.twitter.server.util.HttpUtils._
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.server.util.HttpUtils.newOk
import com.twitter.server.util.JsonConverter
import com.twitter.util.Future
import com.twitter.util.registry.{Formatter, GlobalRegistry}

/**
* A [[com.twitter.finagle.Service]] for displaying the current state of the
Expand Down
5 changes: 3 additions & 2 deletions src/main/scala/com/twitter/server/handler/ReplyHandler.scala
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.server.util.HttpUtils._
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.server.util.HttpUtils.newOk

class ReplyHandler(msg: String) extends Service[Request, Response] {
def apply(req: Request) = newOk(msg)
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.twitter.server.handler

import com.twitter.finagle.httpx.Status
import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response, Status}
import com.twitter.io.{Buf, Charsets}
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{new404, newResponse, parse}
import com.twitter.util.{Future, FuturePool, JavaSingleton}
import java.io.{File, FileInputStream, InputStream}
import java.nio.charset.Charset
Expand Down Expand Up @@ -133,4 +133,4 @@ object ResourceHandler extends JavaSingleton {
baseDirectory: String
): PartialFunction[String, InputStream] =
directoryResolver(baseDirectory) orElse jarResolver(baseResourcePath)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.finagle.stats.LoadedStatsReceiver
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.io.Buf
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.newResponse
import com.twitter.server.util.JsonConverter
import com.twitter.util.Future
import com.twitter.util.registry.GlobalRegistry
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package com.twitter.server.handler

import com.twitter.finagle.server.ServerRegistry
import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response, Status}
import com.twitter.finagle.server.ServerRegistry
import com.twitter.finagle.util.StackRegistry
import com.twitter.io.Buf
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{new404, newResponse, parse}
import com.twitter.server.util.MetricSource
import com.twitter.server.view.StackRegistryView
import com.twitter.util.Future
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package com.twitter.server.handler

import com.twitter.app.App
import com.twitter.finagle.httpx.Status
import com.twitter.finagle.httpx.{Request, Response, Status}
import com.twitter.finagle.Service
import com.twitter.io.Buf
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{parse, newOk, newResponse}
import com.twitter.util.{Duration, Future}
import java.util.logging.Logger

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.io.Buf
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{expectsHtml, newOk, newResponse}
import com.twitter.util.Future

private object SummaryHandler {
Expand Down Expand Up @@ -52,4 +53,4 @@ class SummaryHandler extends Service[Request, Response] {
content = Buf.Utf8(html)
)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package com.twitter.server.handler

import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response}
import com.twitter.io.Buf
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{expectsHtml, newOk, newResponse}
import com.twitter.server.util.JsonConverter
import com.twitter.server.view.ThreadsView
import com.twitter.util.Future
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.twitter.server.handler

import com.twitter.finagle.httpx.Status
import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response, Status}
import com.twitter.io.Buf
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{newResponse, parse}
import com.twitter.util.Future
import java.util.logging.Logger

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.twitter.server.handler

import com.twitter.finagle.httpx.Status
import com.twitter.finagle.Service
import com.twitter.finagle.httpx.{Request, Response, Status}
import com.twitter.io.Buf
import com.twitter.server.util.HttpUtils._
import com.twitter.server.util.HttpUtils.{newResponse, parse}
import com.twitter.util.Future
import java.util.logging.Logger
import scala.collection.{Map, Seq}
Expand Down
Loading

0 comments on commit 45133d9

Please sign in to comment.