Change in libosmocore[master]: stat, rate_ctr: Allow setting group name and use it at report time

laforge gerrit-no-reply at lists.osmocom.org
Sat Jun 5 15:46:27 UTC 2021


laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/24467 )

Change subject: stat,rate_ctr: Allow setting group name and use it at report time
......................................................................

stat,rate_ctr: Allow setting group name and use it at report time

This patch adds a new field "name" to the rate_ctr and osmo_stat_item_group
structs, together with an API to set it. This new field allows for easy
identification of specific group instances when several of them exists,
rather than using a sometimes random/increasing index value.

If set, this name (string) is used instead of the index by the stats
reporter.

The name, if set, is also printed during "show stats" VTY commands.

It's up to the user or application to set up unique or meaningful names
to fullfill one's needs.

WARNING: this commit breaks ABI and possibly creates unexpected behavior
when run with non-rebuilt apps which use the modified structs directly
to get the coutners, or if use the static inline API rate_ctr_inc2().
Existing users of these structs should migrate to use new APIs
introduced in follow-up commits instead of accessing the field directly.

Related: SYS#5456
Change-Id: I0dc510783dd9ae8436dae8005a7b3330e80d36f3
---
M TODO-RELEASE
M include/osmocom/core/rate_ctr.h
M include/osmocom/core/stat_item.h
M src/rate_ctr.c
M src/stat_item.c
M src/stats_statsd.c
M src/vty/utils.c
7 files changed, 67 insertions(+), 35 deletions(-)

Approvals:
  laforge: Looks good to me, but someone else must approve
  daniel: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/TODO-RELEASE b/TODO-RELEASE
index ad644aa..0f70081 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -13,3 +13,4 @@
 libosmocore	osmo_tdef_fsm_inst_state_chg	change default_timeout arg from unsigned long to long type (API breakage, not ABI)
 libosmovty      vty_read_config_filep   New API
 libosmosim	osim_card_{reset,close}	New API
+libosmocore     struct rate_ctr_group, osmo_stat_item_group_desc ABI breakage due to new struct members
diff --git a/include/osmocom/core/rate_ctr.h b/include/osmocom/core/rate_ctr.h
index 17ee672..d944cc0 100644
--- a/include/osmocom/core/rate_ctr.h
+++ b/include/osmocom/core/rate_ctr.h
@@ -61,6 +61,8 @@
 	const struct rate_ctr_group_desc *desc;
 	/*! The index of this ctr_group within its class */
 	unsigned int idx;
+	/*! Optional string-based identifier to be used instead of index at report time */
+	char *name;
 	/*! Actual counter structures below. Don't access it directly, use APIs below! */
 	struct rate_ctr ctr[0];
 };
@@ -73,6 +75,7 @@
 {
 	grp->idx = idx;
 }
+void rate_ctr_group_set_name(struct rate_ctr_group *grp, const char *name);
 
 struct rate_ctr *rate_ctr_group_get_ctr(struct rate_ctr_group *grp, unsigned int idx);
 
diff --git a/include/osmocom/core/stat_item.h b/include/osmocom/core/stat_item.h
index 3cace08..fbe0433 100644
--- a/include/osmocom/core/stat_item.h
+++ b/include/osmocom/core/stat_item.h
@@ -65,6 +65,8 @@
 	const struct osmo_stat_item_group_desc *desc;
 	/*! The index of this value group within its class */
 	unsigned int idx;
+	/*! Optional string-based identifier to be used instead of index at report time */
+	char *name;
 	/*! Actual counter structures below */
 	struct osmo_stat_item *items[0];
 };
@@ -80,6 +82,7 @@
 	grp->idx = idx;
 }
 struct osmo_stat_item *osmo_stat_item_group_get_item(struct osmo_stat_item_group *grp, unsigned int idx);
+void osmo_stat_item_group_set_name(struct osmo_stat_item_group *statg, const char *name);
 void osmo_stat_item_group_free(struct osmo_stat_item_group *statg);
 
 void osmo_stat_item_inc(struct osmo_stat_item *item, int32_t value);
diff --git a/src/rate_ctr.c b/src/rate_ctr.c
index c3a5286..4d99699 100644
--- a/src/rate_ctr.c
+++ b/src/rate_ctr.c
@@ -273,6 +273,16 @@
 	return &grp->ctr[idx];
 }
 
+/*! Set a name for the group of counters be used instead of index value
+  at report time.
+ *  \param[in] grp Rate counter group
+ *  \param[in] name Name identifier to assign to the rate counter group
+ */
+void rate_ctr_group_set_name(struct rate_ctr_group *grp, const char *name)
+{
+	osmo_talloc_replace_string(grp, &grp->name, name);
+}
+
 /*! Add a number to the counter */
 void rate_ctr_add(struct rate_ctr *ctr, int inc)
 {
diff --git a/src/stat_item.c b/src/stat_item.c
index a6f86cb..55aa951 100644
--- a/src/stat_item.c
+++ b/src/stat_item.c
@@ -177,6 +177,16 @@
 	return grp->items[idx];
 }
 
+/*! Set a name for the statistics item group to be used instead of index value
+  at report time.
+ *  \param[in] statg Statistics item group
+ *  \param[in] name Name identifier to assign to the statistics item group
+ */
+void osmo_stat_item_group_set_name(struct osmo_stat_item_group *statg, const char *name)
+{
+	osmo_talloc_replace_string(statg, &statg->name, name);
+}
+
 /*! Increase the stat_item to the given value.
  *  This function adds a new value for the given stat_item at the end of
  *  the FIFO.
diff --git a/src/stats_statsd.c b/src/stats_statsd.c
index 99764e6..1acfce8 100644
--- a/src/stats_statsd.c
+++ b/src/stats_statsd.c
@@ -89,7 +89,7 @@
 }
 
 static int osmo_stats_reporter_statsd_send(struct osmo_stats_reporter *srep,
-	const char *name1, unsigned int index1, const char *name2, int64_t value,
+	const char *name1, const char *index1, const char *name2, int64_t value,
 	const char *unit)
 {
 	char *buf;
@@ -101,13 +101,13 @@
 
 	if (prefix) {
 		if (name1)
-			fmt = "%1$s.%2$s.%6$u.%3$s:%4$" PRId64 "|%5$s";
+			fmt = "%1$s.%2$s.%6$s.%3$s:%4$" PRId64 "|%5$s";
 		else
 			fmt = "%1$s.%2$0.0s%3$s:%4$" PRId64 "|%5$s";
 	} else {
 		prefix = "";
 		if (name1)
-			fmt = "%1$s%2$s.%6$u.%3$s:%4$" PRId64 "|%5$s";
+			fmt = "%1$s%2$s.%6$s.%3$s:%4$" PRId64 "|%5$s";
 		else
 			fmt = "%1$s%2$0.0s%3$s:%4$" PRId64 "|%5$s";
 	}
@@ -162,32 +162,42 @@
 	const struct rate_ctr_desc *desc,
 	int64_t value, int64_t delta)
 {
-	if (ctrg)
-		return osmo_stats_reporter_statsd_send(srep,
-			ctrg->desc->group_name_prefix,
-			ctrg->idx,
-			desc->name, delta, "c");
-	else
-		return osmo_stats_reporter_statsd_send(srep,
-			NULL, 0,
-			desc->name, delta, "c");
+	char buf_idx[64];
+	const char *idx_name = buf_idx;
+	const char *prefix;
+
+	if (ctrg) {
+		prefix = ctrg->desc->group_name_prefix;
+		if (ctrg->name)
+			idx_name = ctrg->name;
+		else
+			snprintf(buf_idx, sizeof(buf_idx), "%u", ctrg->idx);
+	} else {
+		prefix = NULL;
+		buf_idx[0] = '0';
+		buf_idx[1] = '\n';
+	}
+	return osmo_stats_reporter_statsd_send(srep, prefix, idx_name, desc->name, delta, "c");
 }
 
 static int osmo_stats_reporter_statsd_send_item(struct osmo_stats_reporter *srep,
 	const struct osmo_stat_item_group *statg,
 	const struct osmo_stat_item_desc *desc, int64_t value)
 {
-	if (value < 0) {
-		return osmo_stats_reporter_statsd_send(srep,
-				statg->desc->group_name_prefix,
-				statg->idx,
-				desc->name, 0, "g");
-	} else {
-		return osmo_stats_reporter_statsd_send(srep,
-			statg->desc->group_name_prefix,
-			statg->idx,
-			desc->name, value, "g");
+	char buf_idx[64];
+	char *idx_name;
+	if (statg->name)
+		idx_name = statg->name;
+	else {
+		snprintf(buf_idx, sizeof(buf_idx), "%u", statg->idx);
+		idx_name = buf_idx;
 	}
+
+	if (value < 0)
+		value = 0;
+
+	return osmo_stats_reporter_statsd_send(srep, statg->desc->group_name_prefix,
+					       idx_name, desc->name, value, "g");
 }
 #endif /* !EMBEDDED */
 
diff --git a/src/vty/utils.c b/src/vty/utils.c
index 0358d9b..1137f2b 100644
--- a/src/vty/utils.c
+++ b/src/vty/utils.c
@@ -225,12 +225,10 @@
 	if (ctrg->desc->class_id > vctx->max_level)
 		return 0;
 
-	if (ctrg->idx)
-		vty_out(vty, "%s%s (%d):%s", vctx->prefix,
-			ctrg->desc->group_description, ctrg->idx, VTY_NEWLINE);
-	else
-		vty_out(vty, "%s%s:%s", vctx->prefix,
-			ctrg->desc->group_description, VTY_NEWLINE);
+	vty_out(vty, "%s%s (%d)", vctx->prefix, ctrg->desc->group_description, ctrg->idx);
+	if (ctrg->name)
+		vty_out(vty, "('%s')", ctrg->name);
+	vty_out(vty, ":%s", VTY_NEWLINE);
 
 	rate_ctr_for_each_counter(ctrg, rate_ctr_handler, vctx);
 
@@ -284,13 +282,10 @@
 	if (statg->desc->class_id > vctx->max_level)
 		return 0;
 
-	if (statg->idx)
-		vty_out(vty, "%s%s (%d):%s", vctx->prefix,
-			statg->desc->group_description, statg->idx,
-			VTY_NEWLINE);
-	else
-		vty_out(vty, "%s%s:%s", vctx->prefix,
-			statg->desc->group_description, VTY_NEWLINE);
+	vty_out(vty, "%s%s (%d)", vctx->prefix, statg->desc->group_description, statg->idx);
+	if (statg->name)
+		vty_out(vty, "('%s')", statg->name);
+	vty_out(vty, ":%s", VTY_NEWLINE);
 
 	osmo_stat_item_for_each_item(statg, osmo_stat_item_handler, vctx);
 

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/24467
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0dc510783dd9ae8436dae8005a7b3330e80d36f3
Gerrit-Change-Number: 24467
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210605/a1d93a6d/attachment.htm>


More information about the gerrit-log mailing list