<p>neels <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/25465">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, but someone else must approve
neels: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">refactor stat_item: report only changed values<br><br>Change the functionality of skipping unchanged values: instead of<br>looking up whether new values have been set on a stat item, rather<br>remember the last reported value and skip reporting identical values.<br><br>stats_test.c shows that previously, a stat item reported a value of 10<br>again, even though the previous report had already sent a value of 10.<br>That's just because the value 10 was explicitly set again, internally.<br><br>From a perspective of preserving all data points, it could make sense to<br>send consecutive identical values. But since we already collapse all<br>data points per reporting period into a max, that is pointless.<br><br>Related: SYS#5542<br>Change-Id: I8f4cf34dfed17e0879716fa2cbeee137c158978b<br>---<br>M TODO-RELEASE<br>M src/stats.c<br>M tests/stats/stats_test.c<br>M tests/stats/stats_test.err<br>4 files changed, 9 insertions(+), 14 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/TODO-RELEASE b/TODO-RELEASE</span><br><span>index 0f202e3..ead2c50 100644</span><br><span>--- a/TODO-RELEASE</span><br><span>+++ b/TODO-RELEASE</span><br><span>@@ -20,3 +20,4 @@</span><br><span> libosmocore osmo_stat_item_value API breakage: struct definition removed, because no known users exist / no users should exist.</span><br><span> libosmocore osmo_stat_item ABI breakage: struct osmo_stat_item made opaque.</span><br><span> libosmocore osmo_stat_item No FIFO buffer of values used anymore, the "skipped values" error is no longer possible.</span><br><span style="color: hsl(120, 100%, 40%);">+libosmocore stats reporting for osmo_stat_item, values are now never repeated from one stats report to the next.</span><br><span>diff --git a/src/stats.c b/src/stats.c</span><br><span>index 5dac242..28a3ab3 100644</span><br><span>--- a/src/stats.c</span><br><span>+++ b/src/stats.c</span><br><span>@@ -700,15 +700,11 @@</span><br><span> if (!srep->running)</span><br><span> continue;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* If no new stat values have been set in the current reporting period, skip resending the value.</span><br><span style="color: hsl(0, 100%, 40%);">- * However, if the previously sent value is not the same as the current value, then do send the changed</span><br><span style="color: hsl(0, 100%, 40%);">- * value (while n == 0, the last value from the previous reporting period is in item->value.max == .last</span><br><span style="color: hsl(0, 100%, 40%);">- * == .min, which was put in new_value above).</span><br><span style="color: hsl(0, 100%, 40%);">- * Also if the stats reporter is set to resend all values, also do resend the current value regardless</span><br><span style="color: hsl(0, 100%, 40%);">- * of repetitions.</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If the previously reported value is the same as the current value, skip resending the value.</span><br><span style="color: hsl(120, 100%, 40%);">+ * However, if the stats reporter is set to resend all values, do resend the current value regardless of</span><br><span style="color: hsl(120, 100%, 40%);">+ * repetitions.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- if ((!item->value.n && new_value == prev_reported_value)</span><br><span style="color: hsl(0, 100%, 40%);">- && !srep->force_single_flush)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (new_value == prev_reported_value && !srep->force_single_flush)</span><br><span> continue;</span><br><span> </span><br><span> if (!osmo_stats_reporter_check_config(srep,</span><br><span>diff --git a/tests/stats/stats_test.c b/tests/stats/stats_test.c</span><br><span>index dea2bf7..3aa1f52 100644</span><br><span>--- a/tests/stats/stats_test.c</span><br><span>+++ b/tests/stats/stats_test.c</span><br><span>@@ -443,10 +443,10 @@</span><br><span> osmo_stat_item_set(osmo_stat_item_group_get_item(statg1, TEST_A_ITEM), 10);</span><br><span> do_report(0, 2);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- fprintf(stderr, "report (group 1, item 1 update twice):\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ fprintf(stderr, "report (group 1, item 1 update twice, with same value):\n");</span><br><span> osmo_stat_item_set(osmo_stat_item_group_get_item(statg1, TEST_A_ITEM), 10);</span><br><span> osmo_stat_item_set(osmo_stat_item_group_get_item(statg1, TEST_A_ITEM), 10);</span><br><span style="color: hsl(0, 100%, 40%);">- do_report(0, 2);</span><br><span style="color: hsl(120, 100%, 40%);">+ do_report(0, 0);</span><br><span> </span><br><span> fprintf(stderr, "report (group 1, item 1 update twice, check max):\n");</span><br><span> osmo_stat_item_set(osmo_stat_item_group_get_item(statg1, TEST_A_ITEM), 20);</span><br><span>diff --git a/tests/stats/stats_test.err b/tests/stats/stats_test.err</span><br><span>index a890e0f..1e604d1 100644</span><br><span>--- a/tests/stats/stats_test.err</span><br><span>+++ b/tests/stats/stats_test.err</span><br><span>@@ -114,10 +114,8 @@</span><br><span> test2: item p= g=test.one i=1 n=item.a v=10 u=ma</span><br><span> test1: item p= g=test.one i=1 n=item.a v=10 u=ma</span><br><span> reported: 0 counter vals, 2 stat item vals</span><br><span style="color: hsl(0, 100%, 40%);">-report (group 1, item 1 update twice):</span><br><span style="color: hsl(0, 100%, 40%);">- test2: item p= g=test.one i=1 n=item.a v=10 u=ma</span><br><span style="color: hsl(0, 100%, 40%);">- test1: item p= g=test.one i=1 n=item.a v=10 u=ma</span><br><span style="color: hsl(0, 100%, 40%);">-reported: 0 counter vals, 2 stat item vals</span><br><span style="color: hsl(120, 100%, 40%);">+report (group 1, item 1 update twice, with same value):</span><br><span style="color: hsl(120, 100%, 40%);">+reported: 0 counter vals, 0 stat item vals</span><br><span> report (group 1, item 1 update twice, check max):</span><br><span> test2: item p= g=test.one i=1 n=item.a v=20 u=ma</span><br><span> test1: item p= g=test.one i=1 n=item.a v=20 u=ma</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/25465">change 25465</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/25465"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I8f4cf34dfed17e0879716fa2cbeee137c158978b </div>
<div style="display:none"> Gerrit-Change-Number: 25465 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: daniel <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>