<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/24467">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, but someone else must approve
daniel: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">stat,rate_ctr: Allow setting group name and use it at report time<br><br>This patch adds a new field "name" to the rate_ctr and osmo_stat_item_group<br>structs, together with an API to set it. This new field allows for easy<br>identification of specific group instances when several of them exists,<br>rather than using a sometimes random/increasing index value.<br><br>If set, this name (string) is used instead of the index by the stats<br>reporter.<br><br>The name, if set, is also printed during "show stats" VTY commands.<br><br>It's up to the user or application to set up unique or meaningful names<br>to fullfill one's needs.<br><br>WARNING: this commit breaks ABI and possibly creates unexpected behavior<br>when run with non-rebuilt apps which use the modified structs directly<br>to get the coutners, or if use the static inline API rate_ctr_inc2().<br>Existing users of these structs should migrate to use new APIs<br>introduced in follow-up commits instead of accessing the field directly.<br><br>Related: SYS#5456<br>Change-Id: I0dc510783dd9ae8436dae8005a7b3330e80d36f3<br>---<br>M TODO-RELEASE<br>M include/osmocom/core/rate_ctr.h<br>M include/osmocom/core/stat_item.h<br>M src/rate_ctr.c<br>M src/stat_item.c<br>M src/stats_statsd.c<br>M src/vty/utils.c<br>7 files changed, 67 insertions(+), 35 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 ad644aa..0f70081 100644</span><br><span>--- a/TODO-RELEASE</span><br><span>+++ b/TODO-RELEASE</span><br><span>@@ -13,3 +13,4 @@</span><br><span> libosmocore osmo_tdef_fsm_inst_state_chg change default_timeout arg from unsigned long to long type (API breakage, not ABI)</span><br><span> libosmovty vty_read_config_filep New API</span><br><span> libosmosim osim_card_{reset,close} New API</span><br><span style="color: hsl(120, 100%, 40%);">+libosmocore struct rate_ctr_group, osmo_stat_item_group_desc ABI breakage due to new struct members</span><br><span>diff --git a/include/osmocom/core/rate_ctr.h b/include/osmocom/core/rate_ctr.h</span><br><span>index 17ee672..d944cc0 100644</span><br><span>--- a/include/osmocom/core/rate_ctr.h</span><br><span>+++ b/include/osmocom/core/rate_ctr.h</span><br><span>@@ -61,6 +61,8 @@</span><br><span> const struct rate_ctr_group_desc *desc;</span><br><span> /*! The index of this ctr_group within its class */</span><br><span> unsigned int idx;</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! Optional string-based identifier to be used instead of index at report time */</span><br><span style="color: hsl(120, 100%, 40%);">+ char *name;</span><br><span> /*! Actual counter structures below. Don't access it directly, use APIs below! */</span><br><span> struct rate_ctr ctr[0];</span><br><span> };</span><br><span>@@ -73,6 +75,7 @@</span><br><span> {</span><br><span> grp->idx = idx;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+void rate_ctr_group_set_name(struct rate_ctr_group *grp, const char *name);</span><br><span> </span><br><span> struct rate_ctr *rate_ctr_group_get_ctr(struct rate_ctr_group *grp, unsigned int idx);</span><br><span> </span><br><span>diff --git a/include/osmocom/core/stat_item.h b/include/osmocom/core/stat_item.h</span><br><span>index 3cace08..fbe0433 100644</span><br><span>--- a/include/osmocom/core/stat_item.h</span><br><span>+++ b/include/osmocom/core/stat_item.h</span><br><span>@@ -65,6 +65,8 @@</span><br><span> const struct osmo_stat_item_group_desc *desc;</span><br><span> /*! The index of this value group within its class */</span><br><span> unsigned int idx;</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! Optional string-based identifier to be used instead of index at report time */</span><br><span style="color: hsl(120, 100%, 40%);">+ char *name;</span><br><span> /*! Actual counter structures below */</span><br><span> struct osmo_stat_item *items[0];</span><br><span> };</span><br><span>@@ -80,6 +82,7 @@</span><br><span> grp->idx = idx;</span><br><span> }</span><br><span> struct osmo_stat_item *osmo_stat_item_group_get_item(struct osmo_stat_item_group *grp, unsigned int idx);</span><br><span style="color: hsl(120, 100%, 40%);">+void osmo_stat_item_group_set_name(struct osmo_stat_item_group *statg, const char *name);</span><br><span> void osmo_stat_item_group_free(struct osmo_stat_item_group *statg);</span><br><span> </span><br><span> void osmo_stat_item_inc(struct osmo_stat_item *item, int32_t value);</span><br><span>diff --git a/src/rate_ctr.c b/src/rate_ctr.c</span><br><span>index c3a5286..4d99699 100644</span><br><span>--- a/src/rate_ctr.c</span><br><span>+++ b/src/rate_ctr.c</span><br><span>@@ -273,6 +273,16 @@</span><br><span> return &grp->ctr[idx];</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! Set a name for the group of counters be used instead of index value</span><br><span style="color: hsl(120, 100%, 40%);">+ at report time.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] grp Rate counter group</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] name Name identifier to assign to the rate counter group</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+void rate_ctr_group_set_name(struct rate_ctr_group *grp, const char *name)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_talloc_replace_string(grp, &grp->name, name);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! Add a number to the counter */</span><br><span> void rate_ctr_add(struct rate_ctr *ctr, int inc)</span><br><span> {</span><br><span>diff --git a/src/stat_item.c b/src/stat_item.c</span><br><span>index a6f86cb..55aa951 100644</span><br><span>--- a/src/stat_item.c</span><br><span>+++ b/src/stat_item.c</span><br><span>@@ -177,6 +177,16 @@</span><br><span> return grp->items[idx];</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! Set a name for the statistics item group to be used instead of index value</span><br><span style="color: hsl(120, 100%, 40%);">+ at report time.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] statg Statistics item group</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] name Name identifier to assign to the statistics item group</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+void osmo_stat_item_group_set_name(struct osmo_stat_item_group *statg, const char *name)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_talloc_replace_string(statg, &statg->name, name);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! Increase the stat_item to the given value.</span><br><span> * This function adds a new value for the given stat_item at the end of</span><br><span> * the FIFO.</span><br><span>diff --git a/src/stats_statsd.c b/src/stats_statsd.c</span><br><span>index 99764e6..1acfce8 100644</span><br><span>--- a/src/stats_statsd.c</span><br><span>+++ b/src/stats_statsd.c</span><br><span>@@ -89,7 +89,7 @@</span><br><span> }</span><br><span> </span><br><span> static int osmo_stats_reporter_statsd_send(struct osmo_stats_reporter *srep,</span><br><span style="color: hsl(0, 100%, 40%);">- const char *name1, unsigned int index1, const char *name2, int64_t value,</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *name1, const char *index1, const char *name2, int64_t value,</span><br><span> const char *unit)</span><br><span> {</span><br><span> char *buf;</span><br><span>@@ -101,13 +101,13 @@</span><br><span> </span><br><span> if (prefix) {</span><br><span> if (name1)</span><br><span style="color: hsl(0, 100%, 40%);">- fmt = "%1$s.%2$s.%6$u.%3$s:%4$" PRId64 "|%5$s";</span><br><span style="color: hsl(120, 100%, 40%);">+ fmt = "%1$s.%2$s.%6$s.%3$s:%4$" PRId64 "|%5$s";</span><br><span> else</span><br><span> fmt = "%1$s.%2$0.0s%3$s:%4$" PRId64 "|%5$s";</span><br><span> } else {</span><br><span> prefix = "";</span><br><span> if (name1)</span><br><span style="color: hsl(0, 100%, 40%);">- fmt = "%1$s%2$s.%6$u.%3$s:%4$" PRId64 "|%5$s";</span><br><span style="color: hsl(120, 100%, 40%);">+ fmt = "%1$s%2$s.%6$s.%3$s:%4$" PRId64 "|%5$s";</span><br><span> else</span><br><span> fmt = "%1$s%2$0.0s%3$s:%4$" PRId64 "|%5$s";</span><br><span> }</span><br><span>@@ -162,32 +162,42 @@</span><br><span> const struct rate_ctr_desc *desc,</span><br><span> int64_t value, int64_t delta)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- if (ctrg)</span><br><span style="color: hsl(0, 100%, 40%);">- return osmo_stats_reporter_statsd_send(srep,</span><br><span style="color: hsl(0, 100%, 40%);">- ctrg->desc->group_name_prefix,</span><br><span style="color: hsl(0, 100%, 40%);">- ctrg->idx,</span><br><span style="color: hsl(0, 100%, 40%);">- desc->name, delta, "c");</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(0, 100%, 40%);">- return osmo_stats_reporter_statsd_send(srep,</span><br><span style="color: hsl(0, 100%, 40%);">- NULL, 0,</span><br><span style="color: hsl(0, 100%, 40%);">- desc->name, delta, "c");</span><br><span style="color: hsl(120, 100%, 40%);">+ char buf_idx[64];</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *idx_name = buf_idx;</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *prefix;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ctrg) {</span><br><span style="color: hsl(120, 100%, 40%);">+ prefix = ctrg->desc->group_name_prefix;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ctrg->name)</span><br><span style="color: hsl(120, 100%, 40%);">+ idx_name = ctrg->name;</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+ snprintf(buf_idx, sizeof(buf_idx), "%u", ctrg->idx);</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ prefix = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ buf_idx[0] = '0';</span><br><span style="color: hsl(120, 100%, 40%);">+ buf_idx[1] = '\n';</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ return osmo_stats_reporter_statsd_send(srep, prefix, idx_name, desc->name, delta, "c");</span><br><span> }</span><br><span> </span><br><span> static int osmo_stats_reporter_statsd_send_item(struct osmo_stats_reporter *srep,</span><br><span> const struct osmo_stat_item_group *statg,</span><br><span> const struct osmo_stat_item_desc *desc, int64_t value)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- if (value < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- return osmo_stats_reporter_statsd_send(srep,</span><br><span style="color: hsl(0, 100%, 40%);">- statg->desc->group_name_prefix,</span><br><span style="color: hsl(0, 100%, 40%);">- statg->idx,</span><br><span style="color: hsl(0, 100%, 40%);">- desc->name, 0, "g");</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- return osmo_stats_reporter_statsd_send(srep,</span><br><span style="color: hsl(0, 100%, 40%);">- statg->desc->group_name_prefix,</span><br><span style="color: hsl(0, 100%, 40%);">- statg->idx,</span><br><span style="color: hsl(0, 100%, 40%);">- desc->name, value, "g");</span><br><span style="color: hsl(120, 100%, 40%);">+ char buf_idx[64];</span><br><span style="color: hsl(120, 100%, 40%);">+ char *idx_name;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (statg->name)</span><br><span style="color: hsl(120, 100%, 40%);">+ idx_name = statg->name;</span><br><span style="color: hsl(120, 100%, 40%);">+ else {</span><br><span style="color: hsl(120, 100%, 40%);">+ snprintf(buf_idx, sizeof(buf_idx), "%u", statg->idx);</span><br><span style="color: hsl(120, 100%, 40%);">+ idx_name = buf_idx;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (value < 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ value = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return osmo_stats_reporter_statsd_send(srep, statg->desc->group_name_prefix,</span><br><span style="color: hsl(120, 100%, 40%);">+ idx_name, desc->name, value, "g");</span><br><span> }</span><br><span> #endif /* !EMBEDDED */</span><br><span> </span><br><span>diff --git a/src/vty/utils.c b/src/vty/utils.c</span><br><span>index 0358d9b..1137f2b 100644</span><br><span>--- a/src/vty/utils.c</span><br><span>+++ b/src/vty/utils.c</span><br><span>@@ -225,12 +225,10 @@</span><br><span> if (ctrg->desc->class_id > vctx->max_level)</span><br><span> return 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (ctrg->idx)</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "%s%s (%d):%s", vctx->prefix,</span><br><span style="color: hsl(0, 100%, 40%);">- ctrg->desc->group_description, ctrg->idx, VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "%s%s:%s", vctx->prefix,</span><br><span style="color: hsl(0, 100%, 40%);">- ctrg->desc->group_description, VTY_NEWLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "%s%s (%d)", vctx->prefix, ctrg->desc->group_description, ctrg->idx);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ctrg->name)</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "('%s')", ctrg->name);</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, ":%s", VTY_NEWLINE);</span><br><span> </span><br><span> rate_ctr_for_each_counter(ctrg, rate_ctr_handler, vctx);</span><br><span> </span><br><span>@@ -284,13 +282,10 @@</span><br><span> if (statg->desc->class_id > vctx->max_level)</span><br><span> return 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (statg->idx)</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "%s%s (%d):%s", vctx->prefix,</span><br><span style="color: hsl(0, 100%, 40%);">- statg->desc->group_description, statg->idx,</span><br><span style="color: hsl(0, 100%, 40%);">- VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "%s%s:%s", vctx->prefix,</span><br><span style="color: hsl(0, 100%, 40%);">- statg->desc->group_description, VTY_NEWLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "%s%s (%d)", vctx->prefix, statg->desc->group_description, statg->idx);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (statg->name)</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "('%s')", statg->name);</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, ":%s", VTY_NEWLINE);</span><br><span> </span><br><span> osmo_stat_item_for_each_item(statg, osmo_stat_item_handler, vctx);</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/24467">change 24467</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/+/24467"/><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: I0dc510783dd9ae8436dae8005a7b3330e80d36f3 </div>
<div style="display:none"> Gerrit-Change-Number: 24467 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@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: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>