Commit Graph

10 Commits

Author SHA1 Message Date
Vadim Yanitskiy 4e92472281 stats: use llist_add_tail() in osmo_stats_reporter_alloc()
This allows printing reporters in the exact order as they were configired.

Change-Id: I904cd0ed53510dbe26c15cd287ba2707ca04cd6e
Related: SYS#5713
2021-11-09 04:34:46 +03:00
Neels Hofmeyr 6a5940740a refactor stat_item: report only changed values
Change the functionality of skipping unchanged values: instead of
looking up whether new values have been set on a stat item, rather
remember the last reported value and skip reporting identical values.

stats_test.c shows that previously, a stat item reported a value of 10
again, even though the previous report had already sent a value of 10.
That's just because the value 10 was explicitly set again, internally.

From a perspective of preserving all data points, it could make sense to
send consecutive identical values. But since we already collapse all
data points per reporting period into a max, that is pointless.

Related: SYS#5542
Change-Id: I8f4cf34dfed17e0879716fa2cbeee137c158978b
2021-09-30 18:33:43 +00:00
Neels Hofmeyr e90c7176be refactor stat_item: get rid of FIFO and "skipped" error
Intead of attempting to store all distinct values of a reporting period,
just store min, max, last as well as a sum and N of each reporting
period.

This gets rid of error messages like

  DLSTATS ERROR stat_item.c:285 num_bts:oml_connected: 44 stats values skipped

while at the same time more accurately reporting the max value for each
reporting period. (So far stats_item only reports the max value; keep
that part unchanged, as shown in stats_test.c.)

With the other so far unused values (min, sum), we are ready to also
report the minimum value as well as an average value per reporting
period in the future, if/when our stats reporter allows for it.

Store the complete record of the previous reporting period. So far we
only compare the 'max' value, but like this we are ready to also see
changes in min, last and average value between reporting periods.

This patch breaks API by removing:
- struct members osmo_stats_item.stats_next_id, .last_offs and .values[]
- struct osmo_stats_item_value
- osmo_stat_item_get_next()
- osmo_stat_item_discard()
- osmo_stat_item_discard_all()
and by making struct osmo_stats_item opaque.
In libosmocore, we do have a policy of never breaking API. But since the
above should never be accessed by users of the osmo_stats_item API -- or
if they are, would no longer yield useful results, we decided to make an
exception in this case. The alternative would be to introduce a new
osmo_stats_item2 API and maintaining an unused legacy osmo_stats_item
forever, but we decided that the effort is not worth it. There are no
known users of the removed items.

Related: SYS#5542
Change-Id: I137992a5479fc39bbceb6c6c2af9c227bd33b39b
2021-09-30 18:33:43 +00:00
Neels Hofmeyr 599601e12c stats_test: assert counter and stat item val counts separately
Instead of just a send_count, keep one such count for the counter
updates, and a separate one for the stat item updates.
Print those numbers in the test output.

An upcoming patch will tweak stat_item reporting so that only an
actually changed value results in sending a new stat value. This patch
allows illustrating that change clearly.

Related: SYS#5542
Change-Id: I2da003ee6ec15f1c3959efe69e01b4ee24af82bb
2021-09-20 13:05:32 +00:00
Oliver Smith 11da4a4abd stats: send real last value if no new values come
Background:
* Individual values can be added to osmo_stat_item.values at any time.
* Stats are reported at a fixed interval (see vty 'stats interval'),
  e.g. every 10 seconds.
* In order to report a new stat value, we use the maximum of all
  osmo_stat_item.values added since the last report.
* By default, we do not send new stat values if they did not change
  (see vty 'config-stats' -> 'flush-period' default of 0).

Fix the following bug:
* If 'flush-period' is 0, and no new osmo_stat_item.values are coming
  in, the last value that gets reported is not necessarily the last
  entry in osmo_stat_item.values.
* For attached reporters (statsd), it could then be that the given stat
  stays at the wrong value for a long stretch of time (think of several
  hours/days/forever).

Explanation of how the test shows that it is fixed:
* stats get reported (value is irrelevant)
* osmo_stat_item gets a new value: 20
* osmo_stat_item gets a new value: 10
* stats get reported (value: 20, the maximum of both new values)
* osmo_stat_item gets no new values
* stats get reported (value: 10, this is new because of the bug fix,
  the real last value in osmo_stat_item, different from the 20 sent
  earlier, without the fix it would not send anything here and the last
  sent value would be 20)
* osmo_stat_item gets no new values
* stats get reported (nothing gets sent, since the real last value was
  already sent and 'flush-period' is 0)

Fixes: OS#5215
Change-Id: Ibeefd0e3d1dbe4be454ff05a21df4848b2abfabe
2021-08-20 14:04:54 +00:00
Oliver Smith a79a549273 tests/stats: show how last item sent may be wrong
Extend the test to illustrate the bug described in the related issue,
which will be fixed with the next patch.

Related: OS#5215
Change-Id: I1d26867ac1b837bea6a9754a3203e53c147e7a5f
2021-08-20 14:04:54 +00:00
Oliver Smith 2623fca8ad stats: log error when missing stats values (v2)
Related: SYS#4877
Change-Id: I5140d967c2f1d36dadf93b03e52b9bbd42e2a3a6
2021-04-07 18:38:54 +00:00
Oliver Smith a7eb735b8d Revert "stats: log error when missing stats values"
This reverts commit d290439b4a, which
caused "stats values skipped" messages to appear even if they were not
skipped. Revert for now, replace with a proper version in the future.

Related: SYS#4877
Change-Id: Ib43bd53188a4d31d771feb921ea14abe1a3ec877
2021-03-19 16:47:52 +01:00
Oliver Smith d290439b4a stats: log error when missing stats values
Let the user know when the stats were not consumed fast enough for the
given FIFO length.

Related: SYS#4877
Change-Id: If0e8ab55103007693101538fb6ea310075217774
2021-03-17 17:52:37 +01:00
Oliver Smith d89d35e933 tests/stats: enable logging in test output
Move test output from stdout to stderr and enable logging to stderr.
This is in preparation for the next patch, which will add a new log
message when osmo_stat_item_get_next() skips a value.

Related: SYS#4877
Change-Id: Ie0eaec2f93ac6859397a6bfca45039fdcc27cb9e
2021-03-17 16:39:35 +01:00