Skip to content

Commit 9b5b007

Browse files
keremncjenkins
authored andcommitted
finagle: more comprehensive support for dimensional metrics
==== Problem Finagle doesn't have comprehensive support for dimensional metrics === Solution 1. Promote the visibility of `hierarchicalScope`, `dimensionalScope`, and `label` in `StatsReceiver` to public. This allows consumers to define their metric schemas for dual-consumption by the flat JSON & Prometheus exporters 2. Add the flag `com.twitter.finagle.stats.fullTranslation`. This allows service owners to express that raw `.scope` calls on `StatsReceiver` instances to propagate both hierarchical & dimensional metrics 3. Add the flag `com.twitter.finagle.stats.biasToFull`. This allows service owners to express that metrics with an indeterminate identity (have not been explicitly set as dimensional) to be exported through Prometheus anyways. 4. Remove the redundant dimensional scopes set on default Finagle client/service metrics 5. Remove the redundant `app` dimensional scope set on metrics exporter through `DefaultStatsReceiver` 6. Adjust, backwards-compatibly, the JVM metrics to utilize the new APIs and export the memory manager, pool, etc as a label instead of a hierarchical scope 7. Adjust, backwards-compatibly, some other internal use-cases to use labels instead of hierarchical scope Differential Revision: https://phabricator.twitter.biz/D1218090
1 parent 6d4f4cb commit 9b5b007

7 files changed

Lines changed: 36 additions & 23 deletions

File tree

CHANGELOG.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ New Features
1111
~~~~~~~~~~~~
1212

1313
* util-jvm: Add gc pause stats for all collector pools, including G1. ``PHAB_ID=D1176049``
14+
* util-stats: Expose dimensional metrics APIs and allow metrics with an indeterminate
15+
identity to be exported through the Prometheus exporter. ``PHAB_ID=D1218090``
1416

1517

1618
24.5.0
@@ -21,6 +23,8 @@ Runtime Behavior Changes
2123

2224
* util-app: When the application exits due to an error on startup, the error and
2325
and stack trace are printed to stderr in addition to the existing stdout. ``PHAB_ID=D1116753``
26+
* util-jvm: Memory manager & pool metrics are labelled dimensionally rather than hierarchically
27+
when exported as dimensional metrics. ``PHAB_ID=D1218090``
2428

2529

2630
23.11.0

util-jvm/src/main/scala/com/twitter/jvm/Allocations.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ private[jvm] class Allocations(statsReceiver: StatsReceiver) {
4141
private[this] val beanAndListeners =
4242
new LinkedBlockingQueue[(NotificationEmitter, NotificationListener)]()
4343

44-
private[this] val edenGcPauses = statsReceiver.scope("eden").stat("pause_msec")
44+
private[this] val edenGcPauses = statsReceiver
45+
.hierarchicalScope("eden").label("jmm", "eden").stat("pause_msec")
4546

4647
private[jvm] def start(): Unit = {
4748
edenPool

util-jvm/src/main/scala/com/twitter/jvm/JvmStats.scala

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,15 +113,17 @@ object JvmStats {
113113
val postGCStats = memStats.scope("postGC")
114114
memPool.foreach { pool =>
115115
val name = pool.getName.regexSub("""[^\w]""".r) { m => "_" }
116+
val currentPoolScope = currentMem.hierarchicalScope(name).label("pool", name)
117+
val postGcPoolScope = postGCStats.hierarchicalScope(name).label("pool", name)
116118
if (pool.getCollectionUsage != null) {
117119
def usage = pool.getCollectionUsage // this is a snapshot, we can't reuse the value
118-
gauges.add(postGCStats.addGauge(name, "used") { usage.getUsed.toFloat })
120+
gauges.add(postGcPoolScope.addGauge("used") { usage.getUsed.toFloat })
119121
}
120122
if (pool.getUsage != null) {
121123
def usage = pool.getUsage // this is a snapshot, we can't reuse the value
122-
val usageGauge = currentMem.addGauge(name, "used") { usage.getUsed.toFloat }
124+
val usageGauge = currentPoolScope.addGauge("used") { usage.getUsed.toFloat }
123125
gauges.add(usageGauge)
124-
gauges.add(currentMem.addGauge(name, "max") { usage.getMax.toFloat })
126+
gauges.add(currentPoolScope.addGauge("max") { usage.getMax.toFloat })
125127

126128
// register memory usage expression
127129
currentMem.registerExpression(
@@ -171,27 +173,29 @@ object JvmStats {
171173
val bufferPoolStats = memStats.scope("buffer")
172174
jBufferPool.asScala.foreach { bp =>
173175
val name = bp.getName
174-
gauges.add(bufferPoolStats.addGauge(name, "count") { bp.getCount.toFloat })
175-
gauges.add(bufferPoolStats.addGauge(name, "used") { bp.getMemoryUsed.toFloat })
176-
gauges.add(bufferPoolStats.addGauge(name, "max") { bp.getTotalCapacity.toFloat })
176+
val bufferPoolScope = bufferPoolStats.hierarchicalScope(name).label("pool", name)
177+
gauges.add(bufferPoolScope.addGauge("count") { bp.getCount.toFloat })
178+
gauges.add(bufferPoolScope.addGauge("used") { bp.getMemoryUsed.toFloat })
179+
gauges.add(bufferPoolScope.addGauge("max") { bp.getTotalCapacity.toFloat })
177180
}
178181
}
179182

180183
val gcPool = ManagementFactory.getGarbageCollectorMXBeans.asScala
181184
val gcStats = stats.scope("gc")
182185
gcPool.foreach { gc =>
183186
val name = gc.getName.regexSub("""[^\w]""".r) { m => "_" }
187+
val poolScope = gcStats.hierarchicalScope(name).label("jmm", name)
184188
val poolCycles =
185-
gcStats.addGauge(
186-
gcStats.metricBuilder(GaugeType).withCounterishGauge.withName(name, "cycles")) {
189+
poolScope.addGauge(
190+
poolScope.metricBuilder(GaugeType).withCounterishGauge.withName("cycles")) {
187191
gc.getCollectionCount.toFloat
188192
}
189-
val poolMsec = gcStats.addGauge(
190-
gcStats.metricBuilder(GaugeType).withCounterishGauge.withName(name, "msec")) {
193+
val poolMsec = poolScope.addGauge(
194+
poolScope.metricBuilder(GaugeType).withCounterishGauge.withName("msec")) {
191195
gc.getCollectionTime.toFloat
192196
}
193197

194-
val gcPauseStat = gcStats.stat(name, "pause_msec")
198+
val gcPauseStat = poolScope.stat("pause_msec")
195199
gc.asInstanceOf[NotificationEmitter].addNotificationListener(
196200
new NotificationListener {
197201
override def handleNotification(
@@ -244,7 +248,7 @@ object JvmStats {
244248
allocations.start()
245249
if (allocations.trackingEden) {
246250
val allocationStats = memStats.scope("allocations")
247-
val eden = allocationStats.scope("eden")
251+
val eden = allocationStats.hierarchicalScope("eden").label("space", "eden")
248252
gauges.add(eden.addGauge("bytes") { allocations.eden.toFloat })
249253
}
250254

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,7 @@ object LoadedStatsReceiver extends StatsReceiverProxy {
2323
* [[com.twitter.finagle.util.LoadService]] mechanism.
2424
*/
2525
object DefaultStatsReceiver extends StatsReceiverProxy {
26-
val self: StatsReceiver =
27-
LoadedStatsReceiver
28-
.dimensionalScope("app")
29-
.label("implementation", "app")
26+
val self: StatsReceiver = LoadedStatsReceiver.label("implementation", "app")
3027

3128
override def repr: DefaultStatsReceiver.type = this
3229

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.twitter.finagle.stats
22

3+
import com.twitter.app.GlobalFlag
34
import com.twitter.finagle.stats.MetricBuilder.CounterType
45
import com.twitter.finagle.stats.MetricBuilder.CounterishGaugeType
56
import com.twitter.finagle.stats.MetricBuilder.GaugeType
@@ -10,6 +11,12 @@ import com.twitter.finagle.stats.MetricBuilder.MetricType
1011
import com.twitter.finagle.stats.MetricBuilder.UnlatchedCounter
1112
import scala.annotation.varargs
1213

14+
object biasToFull
15+
extends GlobalFlag[Boolean](
16+
false,
17+
"Bias metrics with an indeterminate identity to be exported through all available exporters"
18+
)
19+
1320
/**
1421
* Represents the "role" this service plays with respect to this metric.
1522
*
@@ -225,7 +232,7 @@ object MetricBuilder {
225232
* The current behavior is to default to [[IdentityType.HierarchicalOnly]]
226233
*/
227234
private[twitter] def toResolvedIdentityType(identityType: IdentityType): ResolvedIdentityType =
228-
identityType.bias(IdentityType.HierarchicalOnly)
235+
identityType.bias(if (biasToFull()) IdentityType.Full else IdentityType.HierarchicalOnly)
229236

230237
/** An [[IdentityType]] that cannot be [[NonDeterminate]] */
231238
sealed abstract class ResolvedIdentityType extends IdentityType

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ trait StatsReceiver {
401401
* Create a new `StatsReceiver` that will add a scope that is only used when the metric is
402402
* emitted in hierarchical form.
403403
*/
404-
private[finagle] final def hierarchicalScope(namespace: String): StatsReceiver = {
404+
def hierarchicalScope(namespace: String): StatsReceiver = {
405405
if (namespace == "") this
406406
else
407407
new ScopeTranslatingStatsReceiver(
@@ -414,7 +414,7 @@ trait StatsReceiver {
414414
* Create a new `StatsReceiver` that will add a scope that is only used when the metric is
415415
* emitted in dimensional form.
416416
*/
417-
private[finagle] final def dimensionalScope(namespace: String): StatsReceiver = {
417+
def dimensionalScope(namespace: String): StatsReceiver = {
418418
if (namespace == "") this
419419
else
420420
new ScopeTranslatingStatsReceiver(
@@ -426,7 +426,7 @@ trait StatsReceiver {
426426
/**
427427
* Create a new `StatsReceiver` that will add the specified label to all created metrics.
428428
*/
429-
private[finagle] final def label(labelName: String, labelValue: String): StatsReceiver = {
429+
def label(labelName: String, labelValue: String): StatsReceiver = {
430430
require(labelName.nonEmpty)
431431
if (labelValue == "") this
432432
else new LabelTranslatingStatsReceiver(this, labelName, labelValue)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ package com.twitter.finagle.stats
33
import org.scalatest.funsuite.AnyFunSuite
44

55
class DefaultStatsReceiverTest extends AnyFunSuite {
6-
test("DefaultStatsReceiver has app in dimensional scope and label") {
6+
test("DefaultStatsReceiver initializes dimensional scope and label") {
77
LoadedStatsReceiver.self = new InMemoryStatsReceiver
88
val fooCounter = DefaultStatsReceiver.counter("foo")
99
val identity = fooCounter.metadata.toMetricBuilder.get.identity
1010

11-
assert(identity.dimensionalName == Seq("app", "foo"))
11+
assert(identity.dimensionalName == Seq("foo"))
1212
assert(identity.hierarchicalName == Seq("foo"))
1313
assert(identity.labels == Map("implementation" -> "app"))
1414
}

0 commit comments

Comments
 (0)