Permalink
Browse files

finagle-stats: Remove dependency on commons metrics

Summary: Problem

finagle-stats has a dependency on commons which inflates its footprint and makes
it more difficult to manage.  However, this is complicated by the fact that some
tooling relies upon the shared dependency where they assume they can use the
commons API and the finagle one interchangeably.

Solution

Reverse the dependency, so that you can still use the two APIs interchangeably,
but finagle-stats backs commons metrics, instead of the other way around.

Result

finagle-stats no longer pulls in commons metrics.

JIRA Issues: CSL-2447

Differential Revision: https://phabricator.twitter.biz/D73497
  • Loading branch information...
mosesn authored and jenkins committed Aug 2, 2017
1 parent 4f47e6b commit a647fb9cb051ddccaf8efff1533844264cdcc1d1
Showing with 605 additions and 349 deletions.
  1. +2 −0 CHANGES
  2. +0 −1 build.sbt
  3. +0 −3 finagle-stats/src/main/scala/BUILD
  4. +9 −6 finagle-stats/src/main/scala/com/twitter/finagle/stats/BucketedHistogram.scala
  5. +41 −0 finagle-stats/src/main/scala/com/twitter/finagle/stats/ImmediateMetricsHistogram.scala
  6. +0 −53 finagle-stats/src/main/scala/com/twitter/finagle/stats/ImmediateMetricsStatsReceiver.scala
  7. +6 −7 finagle-stats/src/main/scala/com/twitter/finagle/stats/JsonExporter.scala
  8. +197 −0 finagle-stats/src/main/scala/com/twitter/finagle/stats/Metrics.scala
  9. +17 −15 finagle-stats/src/main/scala/com/twitter/finagle/stats/MetricsBucketedHistogram.scala
  10. +29 −0 finagle-stats/src/main/scala/com/twitter/finagle/stats/MetricsHistogram.scala
  11. +0 −1 finagle-stats/src/main/scala/com/twitter/finagle/stats/MetricsHostStatsReceiver.scala
  12. +2 −3 finagle-stats/src/main/scala/com/twitter/finagle/stats/MetricsRegistry.scala
  13. +17 −80 finagle-stats/src/main/scala/com/twitter/finagle/stats/MetricsStatsReceiver.scala
  14. +93 −0 finagle-stats/src/main/scala/com/twitter/finagle/stats/MetricsStore.scala
  15. +24 −0 finagle-stats/src/main/scala/com/twitter/finagle/stats/MetricsView.scala
  16. +1 −2 finagle-stats/src/main/scala/com/twitter/finagle/stats/SampledValues.scala
  17. +42 −0 finagle-stats/src/main/scala/com/twitter/finagle/stats/Snapshot.scala
  18. +5 −5 finagle-stats/src/main/scala/com/twitter/finagle/stats/StatsFormatter.scala
  19. +1 −2 finagle-stats/src/test/scala/BUILD
  20. +3 −4 finagle-stats/src/test/scala/com/twitter/finagle/stats/CounterDeltasTest.scala
  21. +9 −40 finagle-stats/src/test/scala/com/twitter/finagle/stats/ImmediateStatsReceiverTest.scala
  22. +27 −31 finagle-stats/src/test/scala/com/twitter/finagle/stats/JsonExporterTest.scala
  23. +44 −44 finagle-stats/src/test/scala/com/twitter/finagle/stats/MetricsBucketedHistogramTest.scala
  24. +14 −8 finagle-stats/src/test/scala/com/twitter/finagle/stats/MetricsHostStatsReceiverTest.scala
  25. +13 −16 finagle-stats/src/test/scala/com/twitter/finagle/stats/MetricsStatsReceiverTest.scala
  26. +9 −28 finagle-stats/src/test/scala/com/twitter/finagle/stats/StatsFormatterTest.scala
View
@@ -111,6 +111,8 @@ Runtime Behavior Changes
* finagle-http: `serverErrorsAsFailuresV2` toggle is turned into a flag `serverErrorsAsFailures`.
``PHAB_ID=D73265``
* finagle-stats: No longer backed by commons metrics, now its own thing. ``PHAB_ID=D73497``
6.45.0
------
View
@@ -318,7 +318,6 @@ lazy val finagleStats = Project(
).settings(
name := "finagle-stats",
libraryDependencies ++= Seq(
"com.twitter.common" % "metrics" % "0.0.39",
util("app"),
util("core"),
util("events"),
@@ -13,9 +13,6 @@ scala_library(
'finagle/finagle-core/src/main/scala:scala',
'finagle/finagle-http/src/main/scala:scala',
'finagle/finagle-stats/src/main/resources',
'src/java/com/twitter/common/metrics:metrics',
'src/java/com/twitter/common/quantity:quantity',
'src/java/com/twitter/common/stats:util',
'util/util-app/src/main/scala:scala',
'util/util-core/src/main/scala:scala',
'util/util-lint/src/main/scala:scala',
@@ -1,10 +1,13 @@
package com.twitter.finagle.stats
import com.twitter.common.stats
import java.util
private[twitter] object BucketedHistogram {
private[stats] val DefaultQuantiles = IndexedSeq(
0.5, 0.9, 0.95, 0.99, 0.999, 0.9999
)
/**
* Given an error, compute all the bucket values from 1 until we run out of positive
* 32-bit ints. The error should be in percent, between 0.0 and 1.0.
@@ -75,7 +78,7 @@ private[twitter] object BucketedHistogram {
* This is ''not'' internally thread-safe and thread-safety must be applied
* externally. Typically, this is done via [[MetricsBucketedHistogram]].
*
* ''Note:'' while the interface for [[BucketedHistogram.add(Long)]] takes a `Long`,
* ''Note:'' while the interface for [[add(Long)]] takes a `Long`,
* internally the maximum value we will observe is `Int.MaxValue`. This is subject
* to change and should be considered the minimum upper bound. Also, the smallest
* value we will record is `0`.
@@ -100,7 +103,7 @@ private[twitter] object BucketedHistogram {
*
* @see [[BucketedHistogram.apply()]] for creation.
*/
private[stats] class BucketedHistogram(limits: Array[Int]) extends stats.Histogram {
private[stats] class BucketedHistogram(limits: Array[Int]) {
BucketedHistogram.assertLimits(limits)
private[this] def countsLength: Int = limits.length + 1
@@ -134,7 +137,7 @@ private[stats] class BucketedHistogram(limits: Array[Int]) extends stats.Histogr
num += 1
}
override def clear(): Unit = {
def clear(): Unit = {
var i = 0
while (i < countsLength) {
counts(i) = 0
@@ -221,10 +224,10 @@ private[stats] class BucketedHistogram(limits: Array[Int]) extends stats.Histogr
}
}
override def getQuantile(quantile: Double): Long =
def getQuantile(quantile: Double): Long =
percentile(quantile)
override def getQuantiles(quantiles: Array[Double]): Array[Long] = {
def getQuantiles(quantiles: IndexedSeq[Double]): Array[Long] = {
val ps = new Array[Long](quantiles.length)
var i = 0
while (i < ps.length) {
@@ -0,0 +1,41 @@
package com.twitter.finagle.stats
object ImmediateMetricsHistogram {
/**
* Constructs a [[MetricsHistogram]] that has no buffering, windowing, or latching.
*
* Any value added is immediately aggregated in the result.
* Useful for tests or short-lived jobs.
*/
def apply(name: String, quantiles: IndexedSeq[Double]): MetricsHistogram = {
new MetricsHistogram {
private[this] val stats = BucketedHistogram()
def snapshot(): Snapshot = synchronized {
new Snapshot {
def average: Double = stats.average
def count: Long = stats.count
def min: Long = stats.minimum
def max: Long = stats.maximum
def sum: Long = stats.sum
def percentiles: IndexedSeq[Snapshot.Percentile] = {
stats.getQuantiles(quantiles).zip(quantiles).map {
case (q, p) =>
Snapshot.Percentile(p, q)
}
}
}
}
def clear(): Unit = synchronized {
stats.clear()
}
def add(n: Long): Unit = synchronized {
stats.add(n)
}
}
}
}

This file was deleted.

Oops, something went wrong.
@@ -4,7 +4,6 @@ import com.fasterxml.jackson.core.util.DefaultPrettyPrinter
import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import com.twitter.app.GlobalFlag
import com.twitter.common.metrics.Metrics
import com.twitter.conversions.time._
import com.twitter.finagle.Service
import com.twitter.finagle.http.{MediaType, RequestParamMap, Response, Request}
@@ -60,11 +59,11 @@ object JsonExporter {
private val log = Logger.get()
}
class JsonExporter(registry: Metrics, timer: Timer) extends Service[Request, Response] { self =>
class JsonExporter(metrics: MetricsView, timer: Timer) extends Service[Request, Response] { self =>
import JsonExporter._
def this(registry: Metrics) = this(registry, DefaultTimer)
def this(registry: MetricsView) = this(registry, DefaultTimer)
private[this] val mapper = new ObjectMapper
mapper.registerModule(DefaultScalaModule)
@@ -147,7 +146,7 @@ class JsonExporter(registry: Metrics, timer: Timer) extends Service[Request, Res
deltas = Some(new CounterDeltas())
timer.schedule(startOfNextMinute, 1.minute) {
val ds = self.synchronized { deltas.get }
ds.update(registry.sampleCounters())
ds.update(metrics.counters)
}
deltas.get
}
@@ -158,7 +157,7 @@ class JsonExporter(registry: Metrics, timer: Timer) extends Service[Request, Res
filtered: Boolean,
counterDeltasOn: Boolean = false
): String = {
val gauges = try registry.sampleGauges().asScala
val gauges = try metrics.gauges.asScala
catch {
case NonFatal(e) =>
// because gauges run arbitrary user code, we want to protect ourselves here.
@@ -167,11 +166,11 @@ class JsonExporter(registry: Metrics, timer: Timer) extends Service[Request, Res
log.error(e, "exception while collecting gauges")
Map.empty[String, Number]
}
val histos = registry.sampleHistograms().asScala
val histos = metrics.histograms.asScala
val counters = if (counterDeltasOn && useCounterDeltas()) {
getOrRegisterLatchedStats().deltas
} else {
registry.sampleCounters().asScala
metrics.counters.asScala
}
val values = SampledValues(gauges, counters, histos)
Oops, something went wrong.

0 comments on commit a647fb9

Please sign in to comment.