This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
daniel gerrit-no-reply at lists.osmocom.orgdaniel has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/23167 ) Change subject: stats: Ensure that each osmo_stat_item only reports once per interval ...................................................................... stats: Ensure that each osmo_stat_item only reports once per interval We should never report multiple values for a metric. It is confusing for the log reporter and wrong for statsd. Statsd will record only one value, but will it be the first, last, ...? This can happen if an osmo_stat_item changes more than once within the same reporting interval. With this patch only one aggregate value is sent to the log reporters. The value reported is the maximum during this interval. Other aggregations could be possible (min, last), but reporting a (useful) average is not because the values don't include a timestamp and most osmo_stat_items change at unregular intervals. Change-Id: I366ab1c66f4ae6363111ea4e41b66b7d5bcade9c Related: SYS#4877 --- M src/stats.c M tests/stats/stats_test.c M tests/stats/stats_test.ok 3 files changed, 43 insertions(+), 18 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/67/23167/1 diff --git a/src/stats.c b/src/stats.c index a0834d2..6af77f9 100644 --- a/src/stats.c +++ b/src/stats.c @@ -699,31 +699,36 @@ int have_value; have_value = osmo_stat_item_get_next(item, &idx, &value) > 0; - if (!have_value) + if (!have_value) { /* Send the last value in case a flush is requested */ value = osmo_stat_item_get_last(item); + } else { + int32_t next_val; + int got_next = 0; + // If we have multiple values only send the max + do { + got_next = osmo_stat_item_get_next(item, &idx, &next_val) > 0; + if (got_next) { + value = OSMO_MAX(value, next_val); + } + } while (got_next); + } - do { - llist_for_each_entry(srep, &osmo_stats_reporter_list, list) { - if (!srep->running) - continue; + llist_for_each_entry(srep, &osmo_stats_reporter_list, list) { + if (!srep->running) + continue; - if (!have_value && !srep->force_single_flush) - continue; + if (!have_value && !srep->force_single_flush) + continue; - if (!osmo_stats_reporter_check_config(srep, - statg->idx, statg->desc->class_id)) - continue; + if (!osmo_stats_reporter_check_config(srep, + statg->idx, statg->desc->class_id)) + continue; - osmo_stats_reporter_send_item(srep, statg, - item->desc, value); - } + osmo_stats_reporter_send_item(srep, statg, + item->desc, value); - if (!have_value) - break; - - have_value = osmo_stat_item_get_next(item, &idx, &value) > 0; - } while (have_value); + } return 0; } diff --git a/tests/stats/stats_test.c b/tests/stats/stats_test.c index 71f710a..05caf98 100644 --- a/tests/stats/stats_test.c +++ b/tests/stats/stats_test.c @@ -440,6 +440,20 @@ osmo_stats_report(); OSMO_ASSERT(send_count == 2); + printf("report (group 1, item 1 update twice):\n"); + osmo_stat_item_set(statg1->items[TEST_A_ITEM], 10); + osmo_stat_item_set(statg1->items[TEST_A_ITEM], 10); + send_count = 0; + osmo_stats_report(); + OSMO_ASSERT(send_count == 2); + + printf("report (group 1, item 1 update twice, check max):\n"); + osmo_stat_item_set(statg1->items[TEST_A_ITEM], 20); + osmo_stat_item_set(statg1->items[TEST_A_ITEM], 10); + send_count = 0; + osmo_stats_report(); + OSMO_ASSERT(send_count == 2); + printf("report (remove statg1, ctrg1):\n"); /* force single flush */ srep1->force_single_flush = 1; diff --git a/tests/stats/stats_test.ok b/tests/stats/stats_test.ok index 8628adb..26d9e38 100644 --- a/tests/stats/stats_test.ok +++ b/tests/stats/stats_test.ok @@ -100,6 +100,12 @@ report (group 1, item 1 update): test2: item p= g=test.one i=1 n=item.a v=10 u=ma test1: item p= g=test.one i=1 n=item.a v=10 u=ma +report (group 1, item 1 update twice): + test2: item p= g=test.one i=1 n=item.a v=10 u=ma + test1: item p= g=test.one i=1 n=item.a v=10 u=ma +report (group 1, item 1 update twice, check max): + test2: item p= g=test.one i=1 n=item.a v=20 u=ma + test1: item p= g=test.one i=1 n=item.a v=20 u=ma report (remove statg1, ctrg1): test2: counter p= g=ctr-test:one_dot i=3 n=ctr:a v=0 d=0 test1: counter p= g=ctr-test:one_dot i=3 n=ctr:a v=0 d=0 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/23167 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I366ab1c66f4ae6363111ea4e41b66b7d5bcade9c Gerrit-Change-Number: 23167 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann at sysmocom.de> Gerrit-MessageType: newchange -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210301/4a2f661c/attachment.htm>