[Asterisk-code-review] res_statsd: handle non-standard meter type safely (asterisk[master])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/16147 )

Change subject: res_statsd: handle non-standard meter type safely
......................................................................

res_statsd: handle non-standard meter type safely

Meter types are not well supported,
lacking support in telegraf, datadog and the official statsd servers.
We deprecate meters and provide a compliant fallback for any existing usages.

A flag has been introduced to allow meters to fallback to counters.


ASTERISK-29513

Change-Id: I5fcb385983a1b88f03696ff30a26b55c546a1dd7
---
M configs/samples/statsd.conf.sample
A doc/CHANGES-staging/res_statsd.txt
M include/asterisk/statsd.h
M res/res_statsd.c
4 files changed, 28 insertions(+), 2 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/configs/samples/statsd.conf.sample b/configs/samples/statsd.conf.sample
index 8060973..fd51cbd 100644
--- a/configs/samples/statsd.conf.sample
+++ b/configs/samples/statsd.conf.sample
@@ -6,3 +6,6 @@
 ;add_newline = no		; Append a newline to every event. This is
 				; useful if you want to run a fake statsd
 				; server using netcat (nc -lu 8125)
+;meter_support = yes	; Enable/disable the non-standard StatsD Meter type
+				; if disabled falls back to counter
+				; and will append a "_meter" suffix to the metric name
\ No newline at end of file
diff --git a/doc/CHANGES-staging/res_statsd.txt b/doc/CHANGES-staging/res_statsd.txt
new file mode 100644
index 0000000..317c65d
--- /dev/null
+++ b/doc/CHANGES-staging/res_statsd.txt
@@ -0,0 +1,5 @@
+Subject: Handle non-standard Meter metric type safely
+
+A meter_support flag has been introduced that defaults to true to maintain current behaviour.
+If disabled, a counter metric type will be used instead wherever a meter metric type was used,
+the counter will have a "_meter" suffix appended to the metric name.
\ No newline at end of file
diff --git a/include/asterisk/statsd.h b/include/asterisk/statsd.h
index 4dbfb77..1f8468e 100644
--- a/include/asterisk/statsd.h
+++ b/include/asterisk/statsd.h
@@ -41,9 +41,13 @@
 #define AST_STATSD_TIMER "ms"
 /*! Distribution of values over time. */
 #define AST_STATSD_HISTOGRAM "h"
-/*! Events over time. Sorta like increment-only counters. */
+/*!
+ * Meters are non-standard and poorly supported by StatsD servers
+ * \deprecated You should switch to counter or stateful counters for a similar effect.
+ */
 #define AST_STATSD_METER "m"
 
+
 /*!
  * \brief Send a stat to the configured statsd server.
  *
diff --git a/res/res_statsd.c b/res/res_statsd.c
index bdb3d6f..e250857 100644
--- a/res/res_statsd.c
+++ b/res/res_statsd.c
@@ -58,6 +58,10 @@
 					you want to fake out a server using netcat
 					(nc -lu 8125)</synopsis>
 				</configOption>
+				<configOption name="meter_support">
+					<synopsis>Enable/disable the non-standard StatsD Meter type,
+					if disabled falls back to counter and will append a "_meter" suffix to the metric name</synopsis>
+				</configOption>
 			</configObject>
 		</configFile>
 	</configInfo>
@@ -89,6 +93,8 @@
 	struct ast_sockaddr statsd_server;
 	/*! Prefix to put on every stat. */
 	char prefix[MAX_PREFIX + 1];
+	/*! Enabled support for non-standard Meter type by default, falls back to counter if disabled */
+	int meter_support;
 };
 
 /*! \brief All configuration options for statsd client. */
@@ -142,7 +148,11 @@
 		ast_str_append(&msg, 0, "%s.", cfg->global->prefix);
 	}
 
-	ast_str_append(&msg, 0, "%s:%s|%s", metric_name, value, metric_type);
+	if (!cfg->global->meter_support && strcmp(metric_type, AST_STATSD_METER)) {
+		ast_str_append(&msg, 0, "%s_meter:%s|%s", metric_name, value, AST_STATSD_COUNTER);
+	} else {
+		ast_str_append(&msg, 0, "%s:%s|%s", metric_name, value, metric_type);
+	}
 
 	if (sample_rate < 1.0) {
 		ast_str_append(&msg, 0, "|@%.2f", sample_rate);
@@ -360,6 +370,10 @@
 		"", OPT_CHAR_ARRAY_T, 0,
 		CHARFLDSET(struct conf_global_options, prefix));
 
+	aco_option_register(&cfg_info, "meter_support", ACO_EXACT, global_options,
+		"yes", OPT_BOOL_T, 1,
+		FLDSET(struct conf_global_options, meter_support));
+
 	if (aco_process_config(&cfg_info, 0) == ACO_PROCESS_ERROR) {
 		struct conf *cfg;
 

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I5fcb385983a1b88f03696ff30a26b55c546a1dd7
Gerrit-Change-Number: 16147
Gerrit-PatchSet: 10
Gerrit-Owner: Rijnhard Hessel <rijnhard at teleforge.co.za>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210803/84dadfa8/attachment.html>