only log connection pool gauges when there's a handler context value

Netflix · Nov 2, 2021 · 5b77464 · 5b77464
1 parent 2153ea5
commit 5b77464
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 6 deletions.
diff --git a/spectator-ext-aws/README.md b/spectator-ext-aws/README.md
@@ -68,7 +68,9 @@ aws.request.RequestSigningTime            | Field.RequestSigningTime,
 aws.request.responseProcessingTime        | Field.ResponseProcessingTime,
 aws.request.retryPauseTime                | Field.RetryPauseTime
 
-The following request metrics are captured as gauges:
+The following request metrics are captured as gauges, only when the handler
+context has a value for the relevant handler context key (see
+[below](#tag-via-handler-context)).
 
 metric name                              | SDK metric
 -----------------------------------------|-----------

diff --git a/...ator-ext-aws/src/main/java/com/netflix/spectator/aws/SpectatorRequestMetricCollector.java b/...ator-ext-aws/src/main/java/com/netflix/spectator/aws/SpectatorRequestMetricCollector.java
@@ -174,9 +174,15 @@ public void collectMetrics(Request<?> request, Response<?> response) {
                 .record(t.getEndTimeNano() - t.getStartTimeNano(), TimeUnit.NANOSECONDS));
       }
 
-      for (Field gauge : GAUGES) {
-        Optional.ofNullable(timing.getCounter(gauge.name()))
+      // Only include gauge metrics if there is a value in the context handler
+      // key.  Without that, it's likely that guage metrics from multiple
+      // connection pools don't make sense.  This assumes that all gauge metrics
+      // are about connection pools.
+      if (request.getHandlerContext(handlerContextKey) != null) {
+        for (Field gauge : GAUGES) {
+          Optional.ofNullable(timing.getCounter(gauge.name()))
             .ifPresent(v -> registry.gauge(metricId(gauge, allTags)).set(v.doubleValue()));
+        }
       }
 
       notEmpty(metrics.getProperty(Field.ThrottleException)).ifPresent(throttleExceptions -> {

diff --git a/...-ext-aws/src/test/java/com/netflix/spectator/aws/SpectatorRequestMetricCollectorTest.java b/...-ext-aws/src/test/java/com/netflix/spectator/aws/SpectatorRequestMetricCollectorTest.java
@@ -144,7 +144,10 @@ public void testMetricCollection() {
     List<Meter> allMetrics = new ArrayList<>();
     registry.iterator().forEachRemaining(allMetrics::add);
 
-    assertEquals(3, allMetrics.size());
+    // We didn't provide a handler context value, so don't expect any gauges.
+    // That leaves 2 metrics.
+    assertEquals(2, allMetrics.size());
+
     Optional<Timer> expectedTimer = registry.timers().findFirst();
     assertTrue(expectedTimer.isPresent());
     Timer timer = expectedTimer.get();
@@ -155,9 +158,9 @@ public void testMetricCollection() {
     assertTrue(expectedCounter.isPresent());
     assertEquals(12345L, expectedCounter.get().count());
 
+    // Again, don't expect any gauges.
     Optional<Gauge> expectedGauge = registry.gauges().findFirst();
-    assertTrue(expectedGauge.isPresent());
-    assertEquals(-5678d, expectedGauge.get().value());
+    assertFalse(expectedGauge.isPresent());
   }
 
   @Test
@@ -203,5 +206,11 @@ public void testDefaultHandlerContextKey() {
     String handlerContextValue = "some-value";
     execRequest("http://monitoring", 503, DEFAULT_HANDLER_CONTEXT_KEY, handlerContextValue);
     assertEquals(set(handlerContextValue), valueSet(DEFAULT_HANDLER_CONTEXT_KEY.getName()));
+
+    // With a value in the handler context key, make sure there is a gauge
+    // metric.
+    Optional<Gauge> expectedGauge = registry.gauges().findFirst();
+    assertTrue(expectedGauge.isPresent());
+    assertEquals(-5678d, expectedGauge.get().value());
   }
 }