Skip to content

Commit 52c72c4

Browse files
keremncjenkins
authored and
jenkins
committed
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 11ca7e8 commit 52c72c4

File tree

6 files changed

+77
-28
lines changed

6 files changed

+77
-28
lines changed

finagle-core/src/main/scala/com/twitter/finagle/factory/StatsFactoryWrapper.scala

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
package com.twitter.finagle.factory
22

33
import com.twitter.finagle._
4+
import com.twitter.finagle.stats.RollupStatsReceiver
5+
import com.twitter.finagle.stats.StatsReceiver
6+
import com.twitter.util.Future
7+
import com.twitter.util.Return
8+
import com.twitter.util.Stopwatch
9+
import com.twitter.util.Throw
410
import com.twitter.util.Throwables
5-
import com.twitter.finagle.stats.{StatsReceiver, RollupStatsReceiver}
6-
import com.twitter.util.{Future, Stopwatch, Return, Throw}
711

812
private[finagle] object StatsFactoryWrapper {
913
val role = Stack.Role("ServiceCreationStats")
@@ -22,7 +26,10 @@ private[finagle] object StatsFactoryWrapper {
2226
else
2327
new StatsFactoryWrapper(
2428
next,
25-
new RollupStatsReceiver(statsReceiver.scope("service_creation"))
29+
new RollupStatsReceiver(
30+
statsReceiver.scope("service_creation"),
31+
hierarchicalOnly = true
32+
)
2633
)
2734
}
2835
}
@@ -41,7 +48,10 @@ class StatsFactoryWrapper[Req, Rep](self: ServiceFactory[Req, Rep], statsReceive
4148
val elapsed = Stopwatch.start()
4249
super.apply(conn) respond {
4350
case Throw(t) =>
44-
failureStats.counter(Throwables.mkString(t): _*).incr()
51+
failureStats
52+
.label("exception", Throwables.RootCause.nested(t).getClass.getName)
53+
.counter(Throwables.mkString(t): _*)
54+
.incr()
4555
latencyStat.add(elapsed().inMilliseconds)
4656
case Return(_) =>
4757
latencyStat.add(elapsed().inMilliseconds)

finagle-http/src/main/scala/com/twitter/finagle/http/filter/StatsFilter.scala

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package com.twitter.finagle.http.filter
22

33
import com.twitter.finagle._
4-
import com.twitter.finagle.http.{Request, Response, Status}
5-
import com.twitter.finagle.stats.{Counter, Stat, StatsReceiver}
4+
import com.twitter.finagle.http.Request
5+
import com.twitter.finagle.http.Response
6+
import com.twitter.finagle.http.Status
7+
import com.twitter.finagle.stats.Counter
8+
import com.twitter.finagle.stats.Stat
9+
import com.twitter.finagle.stats.StatsReceiver
610
import com.twitter.util._
711
import java.util.concurrent.ConcurrentHashMap
812
import java.util.concurrent.atomic.AtomicReferenceArray
@@ -77,12 +81,17 @@ class StatsFilter[REQUEST <: Request] private[filter] (stats: StatsReceiver, now
7781
private[this] val statusRange: AtomicReferenceArray[StatusStats] =
7882
new AtomicReferenceArray[StatusStats](6)
7983

84+
private[this] def mkStatusStats(code: String): StatusStats = StatusStats(
85+
statusReceiver.hierarchicalScope(code).label("code", code).counter(),
86+
timeReceiver.hierarchicalScope(code).label("code", code).stat(),
87+
)
88+
8089
private[this] def getStatusRange(code: Int): StatusStats = {
8190
val index = getStatusRangeIndex(code)
8291
val statsPair = statusRange.get(index)
8392
if (statsPair == null) {
8493
val codeRange = statusCodeRange(code)
85-
val initPair = StatusStats(statusReceiver.counter(codeRange), timeReceiver.stat(codeRange))
94+
val initPair = mkStatusStats(codeRange)
8695
statusRange.compareAndSet(index, null, initPair)
8796
initPair
8897
} else {
@@ -94,7 +103,7 @@ class StatsFilter[REQUEST <: Request] private[filter] (stats: StatsReceiver, now
94103
private[this] val statusCacheFn = new java.util.function.Function[Int, StatusStats] {
95104
def apply(t: Int): StatusStats = {
96105
val statusCode = t.toString
97-
StatusStats(statusReceiver.counter(statusCode), timeReceiver.stat(statusCode))
106+
mkStatusStats(statusCode)
98107
}
99108
}
100109

finagle-http/src/test/scala/com/twitter/finagle/http/filter/StatsFilterTest.scala

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package com.twitter.finagle.http.filter
33
import com.twitter.finagle.Service
44
import com.twitter.finagle.http.Request
55
import com.twitter.finagle.http.Response
6-
import com.twitter.finagle.stats.MetricBuilder.CounterType
7-
import com.twitter.finagle.stats.MetricBuilder.HistogramType
86
import com.twitter.finagle.stats.InMemoryStatsReceiver
97
import com.twitter.finagle.stats.MetricBuilder
108
import com.twitter.finagle.stats.NameTranslatingStatsReceiver
@@ -13,6 +11,7 @@ import com.twitter.util.Duration
1311
import com.twitter.util.Future
1412
import com.twitter.util.Stopwatch
1513
import com.twitter.util.Time
14+
import org.mockito.ArgumentMatchers.argThat
1615
import org.mockito.Mockito.spy
1716
import org.mockito.Mockito.verify
1817
import org.scalatest.funsuite.AnyFunSuite
@@ -56,9 +55,9 @@ class StatsFilterTest extends AnyFunSuite {
5655
}
5756

5857
// Verify that the counters and stats were only created once
59-
verify(receiver).counter(MetricBuilder(name = Seq("status", "404"), metricType = CounterType))
60-
verify(receiver).counter(MetricBuilder(name = Seq("status", "4XX"), metricType = CounterType))
61-
verify(receiver).stat(MetricBuilder(name = Seq("time", "404"), metricType = HistogramType))
62-
verify(receiver).stat(MetricBuilder(name = Seq("time", "4XX"), metricType = HistogramType))
58+
verify(receiver).counter(argThat((mb: MetricBuilder) => mb.name == Seq("status", "404")))
59+
verify(receiver).counter(argThat((mb: MetricBuilder) => mb.name == Seq("status", "4XX")))
60+
verify(receiver).stat(argThat((mb: MetricBuilder) => mb.name == Seq("time", "404")))
61+
verify(receiver).stat(argThat((mb: MetricBuilder) => mb.name == Seq("time", "4XX")))
6362
}
6463
}

finagle-mysql/src/main/scala/com/twitter/finagle/mysql/ClientDispatcher.scala

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,25 @@
11
package com.twitter.finagle.mysql
22

3-
import com.github.benmanes.caffeine.cache.{Caffeine, RemovalCause, RemovalListener}
3+
import com.github.benmanes.caffeine.cache.Caffeine
4+
import com.github.benmanes.caffeine.cache.RemovalCause
5+
import com.github.benmanes.caffeine.cache.RemovalListener
46
import com.twitter.cache.caffeine.CaffeineCache
57
import com.twitter.finagle.dispatch.GenSerialClientDispatcher
68
import com.twitter.finagle.dispatch.ClientDispatcher.wrapWriteException
79
import com.twitter.finagle.mysql.LostSyncException.const
8-
import com.twitter.finagle.mysql.param.{MaxConcurrentPrepareStatements, UnsignedColumns}
9-
import com.twitter.finagle.mysql.transport.{MysqlBuf, MysqlBufReader, Packet}
10+
import com.twitter.finagle.mysql.param.MaxConcurrentPrepareStatements
11+
import com.twitter.finagle.mysql.param.UnsignedColumns
12+
import com.twitter.finagle.mysql.transport.MysqlBuf
13+
import com.twitter.finagle.mysql.transport.MysqlBufReader
14+
import com.twitter.finagle.mysql.transport.Packet
1015
import com.twitter.finagle.param.Stats
11-
import com.twitter.finagle.stats.{Counter, LazyStatsReceiver, StatsReceiver}
16+
import com.twitter.finagle.stats.Counter
17+
import com.twitter.finagle.stats.LazyStatsReceiver
18+
import com.twitter.finagle.stats.StatsReceiver
1219
import com.twitter.finagle.transport.Transport
13-
import com.twitter.finagle.{Service, ServiceProxy, Stack}
20+
import com.twitter.finagle.Service
21+
import com.twitter.finagle.ServiceProxy
22+
import com.twitter.finagle.Stack
1423
import com.twitter.util._
1524

1625
/**
@@ -38,8 +47,10 @@ private[mysql] class PrepareCache(
3847
private[this] val evictionCounters = {
3948
val counters = new Array[Counter](RemovalCause.values().length)
4049
for (value <- RemovalCause.values()) {
41-
counters(value.ordinal()) =
42-
scopedStatsReceiver.counter(s"evicted_${value.name().toLowerCase}")
50+
counters(value.ordinal()) = scopedStatsReceiver
51+
.hierarchicalScope(s"evicted_${value.name().toLowerCase}")
52+
.dimensionalScope("evicted").label("cause", value.name().toLowerCase())
53+
.counter()
4354
}
4455
counters
4556
}

finagle-thrift/src/main/scala/com/twitter/finagle/thrift/service/ThriftReqRepServicePerEndpoint.scala

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,19 @@ package com.twitter.finagle.thrift.service
33
import com.twitter.finagle.context.Contexts
44
import com.twitter.finagle.service.ResponseClassifier
55
import com.twitter.finagle.stats.StatsReceiver
6-
import com.twitter.finagle.thrift.{Headers, RichClientParam, ThriftClientRequest, ThriftMethodStats}
7-
import com.twitter.finagle.{Filter, Service, SimpleFilter}
6+
import com.twitter.finagle.thrift.Headers
7+
import com.twitter.finagle.thrift.RichClientParam
8+
import com.twitter.finagle.thrift.ThriftClientRequest
9+
import com.twitter.finagle.thrift.ThriftMethodStats
10+
import com.twitter.finagle.Filter
11+
import com.twitter.finagle.Service
12+
import com.twitter.finagle.SimpleFilter
813
import com.twitter.scrooge
914
import com.twitter.scrooge.ThriftMethod
10-
import com.twitter.util.{Future, Return, Throw, Try}
15+
import com.twitter.util.Future
16+
import com.twitter.util.Return
17+
import com.twitter.util.Throw
18+
import com.twitter.util.Try
1119

1220
/**
1321
* Construct `Service[scrooge.Request[method.Args],scrooge.Response[method.SuccessType]]` interface for a [[ThriftMethod]].
@@ -99,7 +107,11 @@ object ThriftReqRepServicePerEndpoint {
99107
stats: StatsReceiver,
100108
responseClassifier: ResponseClassifier
101109
): SimpleFilter[scrooge.Request[method.Args], scrooge.Response[method.SuccessType]] = {
102-
val methodStats = ThriftMethodStats(stats.scope(method.serviceName).scope(method.name))
110+
val methodStats = ThriftMethodStats(
111+
stats
112+
.hierarchicalScope(method.serviceName).hierarchicalScope(method.name)
113+
.label("service", method.serviceName).label("method", method.name)
114+
)
103115
val methodStatsResponseHandler = ThriftMethodStatsHandler(method) _
104116
new SimpleFilter[scrooge.Request[method.Args], scrooge.Response[method.SuccessType]] {
105117
def apply(

finagle-thrift/src/main/scala/com/twitter/finagle/thrift/service/ThriftServicePerEndpoint.scala

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
package com.twitter.finagle.thrift.service
22

3-
import com.twitter.finagle.{Filter, Service, SimpleFilter}
3+
import com.twitter.finagle.Filter
4+
import com.twitter.finagle.Service
5+
import com.twitter.finagle.SimpleFilter
46
import com.twitter.finagle.service.ResponseClassifier
57
import com.twitter.finagle.stats.StatsReceiver
6-
import com.twitter.finagle.thrift.{RichClientParam, ThriftClientRequest, ThriftMethodStats}
8+
import com.twitter.finagle.thrift.RichClientParam
9+
import com.twitter.finagle.thrift.ThriftClientRequest
10+
import com.twitter.finagle.thrift.ThriftMethodStats
711
import com.twitter.scrooge.ThriftMethod
812
import com.twitter.util.Future
913
import org.apache.thrift.TApplicationException
@@ -82,7 +86,11 @@ object ThriftServicePerEndpoint {
8286
stats: StatsReceiver,
8387
responseClassifier: ResponseClassifier
8488
): SimpleFilter[method.Args, method.SuccessType] = {
85-
val methodStats = ThriftMethodStats(stats.scope(method.serviceName).scope(method.name))
89+
val methodStats = ThriftMethodStats(
90+
stats
91+
.hierarchicalScope(method.serviceName).hierarchicalScope(method.name)
92+
.label("service", method.serviceName).label("method", method.name)
93+
)
8694
val methodStatsResponseHandler = ThriftMethodStatsHandler(method) _
8795
new SimpleFilter[method.Args, method.SuccessType] {
8896
def apply(

0 commit comments

Comments
 (0)