Filter non measurable latencies

* Ensures that stats without a sensible measured min/max latency value, are filtered. Avoiding the use of synthetic 0 latency measurements - which taints the min-metric. * fixes #23
olle · May 25, 2020 · eaf19e2 · eaf19e2
1 parent 3040c76
commit eaf19e2
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 30 deletions.
diff --git a/src/main/java/com/studiomediatech/queryresponse/Statistics.java b/src/main/java/com/studiomediatech/queryresponse/Statistics.java
@@ -29,6 +29,7 @@
 import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Objects;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
@@ -66,7 +67,7 @@ class Statistics implements Logging {
  private AtomicLong consumedResponsesCount = new AtomicLong(0);
  private AtomicLong publishedResponsesCount = new AtomicLong(0);
  private AtomicLong fallbacksCount = new AtomicLong(0);
- private List<Long> latencies = new LinkedList<>(Arrays.asList(0L));
+ private List<Long> latencies = new LinkedList<>();
  private AtomicLong lastPublishedQueriesCount = new AtomicLong(0);
  private AtomicLong lastPublishedResponsesCount = new AtomicLong(0);
 
@@ -97,22 +98,23 @@ void respond() {
 
  protected Collection<Stat> getStats() {
 
- return List.of( // NOSONAR
- getPublishedQueriesCountStat(), // NOSONAR
- getConsumedResponsesCountStat(), // NOSONAR
- getPublishedResponsesCountStat(), // NOSONAR
- getFallbacksCountStat(), // NOSONAR
- getMeta(META_NAME, nameSupplier.get()), // NOSONAR
- getMeta(META_HOSTNAME, hostSupplier.get()), // NOSONAR
- getMeta(META_PID, pidSupplier.get()), // NOSONAR
- getMeta(META_UPTIME, uptimeSupplier.get()), // NOSONAR
- getMeta(META_RESPONSES, onlyResponses.get()), // NOSONAR
- getMaxLatencyStat(), // NOSONAR
- getMinLatencyStat(), // NOSONAR
- getAvgLatencyStat(), // NOSONAR
- getThroughputQueriesStat(), // NOSONAR
- getThroughputResponsesStat() // NOSONAR
- );
+ return Arrays.asList( // NOSONAR
+ getPublishedQueriesCountStat(), // NOSONAR
+ getConsumedResponsesCountStat(), // NOSONAR
+ getPublishedResponsesCountStat(), // NOSONAR
+ getFallbacksCountStat(), // NOSONAR
+ getMeta(META_NAME, nameSupplier.get()), // NOSONAR
+ getMeta(META_HOSTNAME, hostSupplier.get()), // NOSONAR
+ getMeta(META_PID, pidSupplier.get()), // NOSONAR
+ getMeta(META_UPTIME, uptimeSupplier.get()), // NOSONAR
+ getMeta(META_RESPONSES, onlyResponses.get()), // NOSONAR
+ getMaxLatencyStat(), // NOSONAR
+ getMinLatencyStat(), // NOSONAR
+ getAvgLatencyStat(), // NOSONAR
+ getThroughputQueriesStat(), // NOSONAR
+ getThroughputResponsesStat() // NOSONAR
+ )
+ .stream().filter(Objects::nonNull).collect(Collectors.toList());
  }
 
 
@@ -124,13 +126,17 @@ private Stat getAvgLatencyStat() {
 
  private Stat getMinLatencyStat() {
 
- return Stat.from(STAT_LATENCY_MIN, getMinLatency(), this.uuid);
+ Long minLatency = getMinLatency();
+
+ return minLatency != null ? Stat.from(STAT_LATENCY_MIN, minLatency, this.uuid) : null;
  }
 
 
  private Stat getMaxLatencyStat() {
 
- return Stat.from(STAT_LATENCY_MAX, getMaxLatency(), this.uuid);
+ Long maxLatency = getMaxLatency();
+
+ return maxLatency != null ? Stat.from(STAT_LATENCY_MAX, maxLatency, this.uuid) : null;
  }
 
 
@@ -182,17 +188,29 @@ protected Stat getThroughputResponsesStat() {
 
  protected double getAvgLatency() {
 
+ if (latencies.isEmpty()) {
+ return 0.0;
+ }
+
  return latencies.stream().collect(Collectors.summarizingLong(Long::valueOf)).getAverage();
  }
 
 
- protected long getMinLatency() {
+ protected Long getMinLatency() {
+
+ if (latencies.isEmpty()) {
+ return null;
+ }
 
  return latencies.stream().collect(Collectors.summarizingLong(Long::valueOf)).getMin();
  }
 
 
- protected long getMaxLatency() {
+ protected Long getMaxLatency() {
+
+ if (latencies.isEmpty()) {
+ return null;
+ }
 
  return latencies.stream().collect(Collectors.summarizingLong(Long::valueOf)).getMax();
  }

diff --git a/src/test/java/com/studiomediatech/queryresponse/StatisticsTest.java b/src/test/java/com/studiomediatech/queryresponse/StatisticsTest.java
@@ -325,17 +325,17 @@ void ensureMeasuresAndRetainsLatencyStatisticsInBoundedCollection() throws Excep
  // NOOP
  sut.measureLatency(null, 43L);
 
- assertThat(sut.getMinLatency()).isEqualTo(0);
- assertThat(sut.getMaxLatency()).isEqualTo(0);
+ assertThat(sut.getMinLatency()).isNull();
+ assertThat(sut.getMaxLatency()).isNull();
  assertThat(sut.getAvgLatency()).isEqualTo(0.0, within(1.0));
 
  for (long i = 1; i < 1000; i++) {
  // NOOP
  sut.measureLatency(1L, 1L);
  }
 
- assertThat(sut.getMinLatency()).isEqualTo(0);
- assertThat(sut.getMaxLatency()).isEqualTo(0);
+ assertThat(sut.getMinLatency()).isNull();
+ assertThat(sut.getMaxLatency()).isNull();
  assertThat(sut.getAvgLatency()).isEqualTo(0.0, within(1.0));
 
  // Given that we have measured 1000 20ms latencies

diff --git a/ui/src/main/java/com/studiomediatech/QueryResponseUI.java b/ui/src/main/java/com/studiomediatech/QueryResponseUI.java
@@ -155,19 +155,19 @@ void query() {
  handler.handleCountQueriesAndResponses(countQueriesSum, countResponsesSum, countFallbacksSum, successRate,
  successRates);
 
- long minLatency = stats
+ Long minLatency = stats
  .stream()
  .filter(stat -> "min_latency".equals(stat.key))
  .mapToLong(statToLong)
  .min()
- .orElse(0);
+ .orElse(-1);
 
  long maxLatency = stats
  .stream()
  .filter(stat -> "max_latency".equals(stat.key))
  .mapToLong(statToLong)
  .max()
- .orElse(0);
+ .orElse(-1);
 
  double avgLatency = stats
  .stream()
@@ -370,8 +370,8 @@ public void handleLatency(long minLatency, long maxLatency, double avgLatency, L
 
  var json = String.format(Locale.US,
  "{\"metrics\": {"
- + "\"min_latency\": %d,"
- + "\"max_latency\": %d,"
+ + (minLatency != -1 ? "\"min_latency\": %d," : "")
+ + (maxLatency != -1 ? "\"max_latency\": %d," : "")
  + "\"avg_latency\": %f,"
  + "\"avg_latencies\": %s"
  + "}}", minLatency, maxLatency, avgLatency, latencies);