Quellcode durchsuchen

Web: fix packets series emitting a spurious zero point at the start

The packets series is a delta series: each point represents the change in
packets_recv/packets_sent between two consecutive samples.  The oldest
point (index 0) has no predecessor, so no meaningful delta can be computed
for it.

Previously, buildSeriesJson() started the loop at i = 0 and guarded the
delta computation with have_previous, emitting [uptime, 0, 0] for the
first sample. This spurious zero point caused several visible problems:

 - Hovering over the leftmost column of the stacked packets graph showed
   a zero-height bar with a zero tooltip.

 - The min/max range display always showed min 0 because the zero point
   was included in the range calculation.

 - oldest_age_secs (computed via buildPointValue in the first loop) was
   anchored to the zero point rather than the oldest valid delta sample.

Fix by restructuring the packets branch in buildSeriesJson: read sample 0
into previous before the loop, then start the loop at i = step. This
eliminates the zero point at the source and removes the per-iteration
have_previous check from the inner loop.

Also change buildPointValue for the packets series to return false
when previous == nullptr, consistent with error_rate.  This ensures the
first loop (which computes oldest_age_secs) no longer treats the first
sample as a valid data point.  The current-value calculation is unaffected
because it already passes an explicit previous pointer for delta series.

As a side effect, the original loop incremented emitted for the wasted
first iteration, counting it against the emitted < points limit and
causing one fewer actual point to be serialized than requested.  The
new loop only increments emitted for real iterations, correcting this
off-by-one.
Valentin V. Bartenev vor 2 Monaten
Ursprung
Commit
1018404b82
1 geänderte Dateien mit 20 neuen und 20 gelöschten Zeilen
  1. 20 20
      src/helpers/StatsHistory.cpp

+ 20 - 20
src/helpers/StatsHistory.cpp

@@ -229,8 +229,7 @@ bool buildPointValue(const HistorySample& sample, const HistorySample* previous,
   }
   if (strcmp(series, "packets") == 0) {
     if (previous == nullptr) {
-      value = 0;
-      return true;
+      return false;
     }
     const uint32_t curr_total = sample.packets_sent + sample.packets_recv;
     const uint32_t prev_total = previous->packets_sent + previous->packets_recv;
@@ -1345,25 +1344,26 @@ bool StatsHistory::buildSeriesJson(const char* series, char* buffer, size_t buff
   size_t valid_emitted = 0;
   have_previous = false;
   if (strcmp(series, "packets") == 0) {
-    for (size_t i = 0; i < _sample_count && emitted < points; i += step, ++emitted) {
-      if (!getSampleFromOldest(i, sample)) {
-        break;
-      }
-      const int rx = (have_previous && sample.packets_recv >= previous.packets_recv)
-        ? (int)(sample.packets_recv - previous.packets_recv) : 0;
-      const int tx = (have_previous && sample.packets_sent >= previous.packets_sent)
-        ? (int)(sample.packets_sent - previous.packets_sent) : 0;
-      offset += snprintf(&buffer[offset], buffer_size - offset,
-                         "%s[%lu,%d,%d]",
-                         valid_emitted == 0 ? "" : ",",
-                         static_cast<unsigned long>(sample.uptime_secs),
-                         rx, tx);
-      valid_emitted++;
-      if (offset + 40 >= buffer_size) {
-        break;
+    if (_sample_count >= 1 && getSampleFromOldest(0, previous)) {
+      for (size_t i = step; i < _sample_count && emitted < points; i += step, ++emitted) {
+        if (!getSampleFromOldest(i, sample)) {
+          break;
+        }
+        const int rx = sample.packets_recv >= previous.packets_recv
+          ? (int)(sample.packets_recv - previous.packets_recv) : 0;
+        const int tx = sample.packets_sent >= previous.packets_sent
+          ? (int)(sample.packets_sent - previous.packets_sent) : 0;
+        offset += snprintf(&buffer[offset], buffer_size - offset,
+                          "%s[%lu,%d,%d]",
+                          valid_emitted == 0 ? "" : ",",
+                          static_cast<unsigned long>(sample.uptime_secs),
+                          rx, tx);
+        valid_emitted++;
+        if (offset + 40 >= buffer_size) {
+          break;
+        }
+        previous = sample;
       }
-      previous = sample;
-      have_previous = true;
     }
   } else {
     for (size_t i = 0; i < _sample_count && emitted < points; i += step, ++emitted) {