Skip to content

Commit 9602a29

Browse files
keremncjenkins
authored andcommitted
finagle: more integration with dimensional metrics
=== Problem 1. Finagle & Finatra exceptions are reported only as hierarchical metrics. 2. Finatra route metrics are reported only as hierarchical metrics. 3. Certain Thrift client metrics are reported only as hierarchical metrics. === Solution 1. Continues to add first-class dimensional metrics support to Finagle by translating some hierarchically-scoped exception stats to labelled metrics. 2. Add dimensional equivalents to Finatra per-route metrics. 3. Report `ServicePerEndpoint` Thrift metrics dimensionally. Exceptions here are still hierarchical, for now. Differential Revision: https://phabricator.twitter.biz/D1218341
1 parent 9b5b007 commit 9602a29

File tree

5 files changed

+108
-19
lines changed

5 files changed

+108
-19
lines changed

util-core/src/main/scala/com/twitter/util/Throwables.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.twitter.util
22

33
import scala.annotation.tailrec
4+
import scala.collection.mutable
45

56
object Throwables {
67

@@ -38,5 +39,15 @@ object Throwables {
3839

3940
object RootCause {
4041
def unapply(e: Throwable): Option[Throwable] = Option(e.getCause)
42+
43+
def nested(ex: Throwable): Throwable = {
44+
var rootException = ex
45+
val exceptions = mutable.HashSet[Throwable]()
46+
while (rootException.getCause != null && !exceptions.contains(rootException)) {
47+
exceptions.add(rootException)
48+
rootException = rootException.getCause
49+
}
50+
rootException
51+
}
4152
}
4253
}

util-stats/src/main/scala/com/twitter/finagle/stats/ExceptionStatsHandler.scala

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,18 @@ class CategorizingExceptionStatsHandler(
111111
* failures/restartable/com.twitter.finagle.Failure/com.twitter.finagle.CancelledConnectionException: 1
112112
* failures/restartable/com.twitter.finagle.Failure/com.twitter.finagle.CancelledConnectionException/java.net.ConnectException: 1
113113
*
114+
* `f1` is also recorded dimensionally as:
115+
* failures{interrupted=true, restartable=true, exception=java.net.ConnectException}
116+
*
114117
* `f2` is recorded as:
115118
* failures
116119
* failures/com.twitter.finagle.IndividualRequestTimeoutException
117120
* sourcedfailures/myservice
118121
* sourcedfailures/myservice/com.twitter.finagle.IndividualRequestTimeoutException
119122
*
123+
* `f2` is also recorded dimensionally as:
124+
* failures{interrupted=true, source=myservice, restartable=true, exception=com.twitter.finagle.IndividualRequestTimeoutException}
125+
*
120126
* @param mkLabel label prefix, default to 'failures'
121127
* @param mkFlags extracting flags if the Throwable is a Failure
122128
* @param mkSource extracting source when configured
@@ -139,14 +145,32 @@ private[finagle] class MultiCategorizingExceptionStatsHandler(
139145
if (flags.isEmpty) Seq(Seq(parentLabel))
140146
else flags.toSeq.map(Seq(parentLabel, _))
141147

142-
val labels: Seq[Seq[String]] = mkSource(t) match {
148+
val maybeSource = mkSource(t)
149+
val labels: Seq[Seq[String]] = maybeSource match {
143150
case Some(service) => flagLabels :+ Seq(SourcedFailures, service)
144151
case None => flagLabels
145152
}
146153

147154
val paths: Seq[Seq[String]] = statPaths(t, labels, rollup)
148155

149-
if (flags.nonEmpty) statsReceiver.counter(parentLabel).incr()
150-
paths.foreach { path => statsReceiver.counter(path: _*).incr() }
156+
val dimensionalLabels = Map("exception" -> Throwables.RootCause.nested(t).getClass.getName) ++
157+
flags.map { flag => flag -> "true" } ++
158+
maybeSource.map("source" -> _).toSeq
159+
160+
val dimensionalMetric = MetricBuilder.forCounter
161+
.withName(parentLabel)
162+
.withLabels(dimensionalLabels)
163+
.withDimensionalSupport
164+
165+
statsReceiver.counter(dimensionalMetric).incr()
166+
paths.foreach {
167+
case Seq(comp) if comp == parentLabel =>
168+
// The "flat" counter is used to build up the rollup hierarchies but since we also use it
169+
// to hold the dimensional metric, we skip it when recording hierarchical metrics.
170+
case path =>
171+
statsReceiver
172+
.counter(MetricBuilder.forCounter.withHierarchicalOnly.withName(path: _*))
173+
.incr()
174+
}
151175
}
152176
}

util-stats/src/main/scala/com/twitter/finagle/stats/RollupStatsReceiver.scala

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,37 +9,45 @@ package com.twitter.finagle.stats
99
* - "/errors"
1010
* - "/errors/clientErrors"
1111
* - "/errors/clientErrors/java_net_ConnectException"
12+
*
13+
* @param self the [[StatsReceiver]] to proxy metrics creation to
14+
* @param hierarchicalOnly whether non-root scopes should be created with the HierarchicalOnly metrics identity
1215
*/
13-
class RollupStatsReceiver(protected val self: StatsReceiver) extends StatsReceiverProxy {
16+
class RollupStatsReceiver(protected val self: StatsReceiver, hierarchicalOnly: Boolean = false)
17+
extends StatsReceiverProxy {
1418

15-
private[this] def tails[A](s: Seq[A]): Seq[Seq[A]] = {
19+
/**
20+
* @return Seq(metrics namespace -> whether the namespace is the "parent" scope)
21+
*/
22+
private[this] def tails[A](s: Seq[A]): Seq[(Seq[A], Boolean)] = {
1623
s match {
1724
case s @ Seq(_) =>
18-
Seq(s)
25+
Seq(s -> true)
1926

2027
case Seq(hd, tl @ _*) =>
21-
Seq(Seq(hd)) ++ (tails(tl) map { t => Seq(hd) ++ t })
28+
Seq(Seq(hd) -> true) ++ (tails(tl) map { case (t, _) => (Seq(hd) ++ t) -> false })
2229
}
2330
}
2431

25-
override def counter(metricBuilder: MetricBuilder) = new Counter {
26-
private[this] val allCounters = BroadcastCounter(
27-
tails(metricBuilder.name).map(n => self.counter(metricBuilder.withName(n: _*)))
28-
)
32+
private[this] def metrics(parent: MetricBuilder): Seq[MetricBuilder] = tails(parent.name).map {
33+
case (name, isRoot) =>
34+
val builder = parent.withName(name: _*)
35+
if (isRoot || !hierarchicalOnly) builder else builder.withHierarchicalOnly
36+
}
37+
38+
override def counter(metricBuilder: MetricBuilder): Counter = new Counter {
39+
private[this] val allCounters = BroadcastCounter(metrics(metricBuilder).map(self.counter))
2940
def incr(delta: Long): Unit = allCounters.incr(delta)
3041
def metadata: Metadata = allCounters.metadata
3142
}
32-
override def stat(metricBuilder: MetricBuilder) = new Stat {
33-
private[this] val allStats = BroadcastStat(
34-
tails(metricBuilder.name).map(n => self.stat(metricBuilder.withName(n: _*)))
35-
)
43+
override def stat(metricBuilder: MetricBuilder): Stat = new Stat {
44+
private[this] val allStats = BroadcastStat(metrics(metricBuilder).map(self.stat))
3645
def add(value: Float): Unit = allStats.add(value)
3746
def metadata: Metadata = allStats.metadata
3847
}
3948

40-
override def addGauge(metricBuilder: MetricBuilder)(f: => Float) = new Gauge {
41-
private[this] val underlying =
42-
tails(metricBuilder.name).map(n => self.addGauge(metricBuilder.withName(n: _*))(f))
49+
override def addGauge(metricBuilder: MetricBuilder)(f: => Float): Gauge = new Gauge {
50+
private[this] val underlying = metrics(metricBuilder).map { self.addGauge(_)(f) }
4351
def remove(): Unit = underlying.foreach(_.remove())
4452
def metadata: Metadata = MultiMetadata(underlying.map(_.metadata))
4553
}

util-stats/src/test/scala/com/twitter/finagle/stats/MultiCategorizingExceptionStatsHandlerTest.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,16 @@ class MultiCategorizingExceptionStatsHandlerTest extends AnyFunSuite {
2020

2121
val keys = receiver.counters.keys.map(_.mkString("/")).toSeq.sorted
2222

23+
val identity = receiver.schemas(Seq("clienterrors")).toMetricBuilder.get.identity
24+
assert(
25+
identity.labels == Map(
26+
"interrupted" -> "true",
27+
"restartable" -> "true",
28+
"source" -> "service",
29+
"exception" -> "java.lang.Exception"
30+
)
31+
)
32+
2333
assert(receiver.counters(Seq("clienterrors")) == 1)
2434
assert(receiver.counters(Seq("clienterrors", "interrupted")) == 1)
2535
assert(receiver.counters(Seq("clienterrors", "restartable")) == 1)
@@ -99,6 +109,9 @@ class MultiCategorizingExceptionStatsHandlerTest extends AnyFunSuite {
99109

100110
val keys = receiver.counters.keys.map(_.mkString("/")).toSeq.sorted
101111

112+
val identity = receiver.schemas(Seq("clienterrors")).toMetricBuilder.get.identity
113+
assert(identity.labels == Map("source" -> "service", "exception" -> "java.lang.Exception"))
114+
102115
assert(receiver.counters(Seq("clienterrors")) == 1)
103116
assert(receiver.counters(Seq("clienterrors", classOf[RuntimeException].getName)) == 1)
104117
assert(
@@ -142,6 +155,9 @@ class MultiCategorizingExceptionStatsHandlerTest extends AnyFunSuite {
142155

143156
val keys = receiver.counters.keys.map(_.mkString("/")).toSeq.sorted
144157

158+
val identity = receiver.schemas(Seq("failures")).toMetricBuilder.get.identity
159+
assert(identity.labels == Map("exception" -> "java.lang.Exception"))
160+
145161
assert(receiver.counters.view.filterKeys(_.contains("failures")).size == 3)
146162
assert(receiver.counters.view.filterKeys(_.contains("sourcedfailures")).size == 0)
147163

util-stats/src/test/scala/com/twitter/finagle/stats/StatsReceiverTest.scala

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import scala.collection.mutable.ArrayBuffer
1616

1717
class StatsReceiverTest extends AnyFunSuite {
1818

19-
test("RollupStatsReceiver counter/stats") {
19+
test("RollupStatsReceiver counter/stats - full translation") {
2020
val mem = new InMemoryStatsReceiver
2121
val receiver = new RollupStatsReceiver(mem)
2222

@@ -30,6 +30,36 @@ class StatsReceiverTest extends AnyFunSuite {
3030
assert(mem.counters(Seq("toto", "titi")) == 2)
3131
assert(mem.counters(Seq("toto", "titi", "tata")) == 1)
3232
assert(mem.counters(Seq("toto", "titi", "tutu")) == 1)
33+
34+
def identity(path: String*): IdentityType = mem.schemas(path).identity.identityType
35+
36+
assert(identity("toto") == IdentityType.NonDeterminate)
37+
assert(identity("toto", "titi") == IdentityType.NonDeterminate)
38+
assert(identity("toto", "titi", "tata") == IdentityType.NonDeterminate)
39+
assert(identity("toto", "titi", "tutu") == IdentityType.NonDeterminate)
40+
}
41+
42+
test("RollupStatsReceiver counter/stats - hierarchical only") {
43+
val mem = new InMemoryStatsReceiver
44+
val receiver = new RollupStatsReceiver(mem, hierarchicalOnly = true)
45+
46+
receiver.counter("toto", "titi", "tata").incr()
47+
assert(mem.counters(Seq("toto")) == 1)
48+
assert(mem.counters(Seq("toto", "titi")) == 1)
49+
assert(mem.counters(Seq("toto", "titi", "tata")) == 1)
50+
51+
receiver.counter("toto", "titi", "tutu").incr()
52+
assert(mem.counters(Seq("toto")) == 2)
53+
assert(mem.counters(Seq("toto", "titi")) == 2)
54+
assert(mem.counters(Seq("toto", "titi", "tata")) == 1)
55+
assert(mem.counters(Seq("toto", "titi", "tutu")) == 1)
56+
57+
def identity(path: String*): IdentityType = mem.schemas(path).identity.identityType
58+
59+
assert(identity("toto") == IdentityType.NonDeterminate)
60+
assert(identity("toto", "titi") == IdentityType.HierarchicalOnly)
61+
assert(identity("toto", "titi", "tata") == IdentityType.HierarchicalOnly)
62+
assert(identity("toto", "titi", "tutu") == IdentityType.HierarchicalOnly)
3363
}
3464

3565
test("Broadcast Counter/Stat") {

0 commit comments

Comments
 (0)