[Asterisk-code-review] res pjsip: Use a SHA1 hash for the statsd label for contacts (asterisk[13])

[Matt Jordan]

Matt Jordan has uploaded a new change for review.

  https://gerrit.asterisk.org/1727

Change subject: res_pjsip: Use a SHA1 hash for the statsd label for contacts
......................................................................

res_pjsip: Use a SHA1 hash for the statsd label for contacts

When 90d9a70789 was merged, it mostly tested dynamic contacts created as
a result of registering a PJSIP endpoint. Contacts generated in this
fashion typically have a long alphanumeric string as their object identifier,
which maps reasonably well for StatsD. Unfortunately, this doesn't work in the
general case. StatsD treats both '.' and ':' characters as special characters.
In particular, having a ':' appear in the middle of a StatsD metric will
result in the metric being rejected.

This causes some obvious issues with SIP URIs.

The StatsD API should not be responsible for escaping the metric name passed
to it. The metric is treated as a single long string, and it would be
challenging to know what to escape in the string passed to the function.
Likewise, we don't want to escape the metric in PJSIP, as that involves
overhead that is wasted when either res_statsd isn't loaded or enabled.

This patch takes an alternative approach. Rather than use the Contact ID
as-is, a SHA1 hash is produced of the ID. This (a) won't contain any of the
aforementioned special characters, and (b) can be done on Contact creation,
which has minimal impact on run-time performance.

The downside of this is that StatsD users will have to map SHA1 hashes back to
the Contacts that are emitting the statistics. To that end, the CLI commands
have been updated to include the first 10 characters of the SHA1 hash, which
should be enough to match what is shown in Graphite (or some other StatsD
backend).

ASTERISK-25595 #close

Change-Id: I2cf2d4e5b5987bda5823a0d984d15f022d5a835c
---
M include/asterisk/res_pjsip.h
M res/res_pjsip/location.c
M res/res_pjsip/pjsip_options.c
3 files changed, 12 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/27/1727/1

diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 37b7662..933fed0 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -201,6 +201,8 @@
 	int64_t rtt;
 	/*! Last status for a contact (default - unavailable) */
 	enum ast_sip_contact_status_type last_status;
+	/*! SHA1 hash of this object's sorcery ID */
+	char hash[41];
 };
 
 /*!
diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index 2e0d84b..3c414d9 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -763,12 +763,12 @@
 {
 	struct ast_sip_cli_context *context = arg;
 	int indent = CLI_INDENT_TO_SPACES(context->indent_level);
-	int filler = CLI_LAST_TABSTOP - indent - 18;
+	int filler = CLI_LAST_TABSTOP - indent - 29;
 
 	ast_assert(context->output_buffer != NULL);
 
 	ast_str_append(&context->output_buffer, 0,
-		"%*s:  <Aor/ContactUri%*.*s>  <Status....>  <RTT(ms)..>\n",
+		"%*s:  <Aor/ContactUri%*.*s> <Hash......> <Status....>  <RTT(ms)..>\n",
 		indent, "Contact", filler, filler, CLI_HEADER_FILLER);
 
 	return 0;
@@ -790,13 +790,14 @@
 	ast_assert(context->output_buffer != NULL);
 
 	indent = CLI_INDENT_TO_SPACES(context->indent_level);
-	flexwidth = CLI_LAST_TABSTOP - indent - 2;
+	flexwidth = CLI_LAST_TABSTOP - indent - 14;
 
-	ast_str_append(&context->output_buffer, 0, "%*s:  %-*.*s  %-12.12s  %11.3f\n",
+	ast_str_append(&context->output_buffer, 0, "%*s:  %-*.*s   %10.10s   %-11.11s %11.3f\n",
 		indent,
 		"Contact",
 		flexwidth, flexwidth,
 		wrapper->contact_id,
+		status->hash,
 		ast_sip_get_contact_short_status_label(status ? status->status : UNKNOWN),
 		(status && (status->status != UNKNOWN) ? ((long long) status->rtt) / 1000.0 : NAN));
 
diff --git a/res/res_pjsip/pjsip_options.c b/res/res_pjsip/pjsip_options.c
index b172489..465aa6f 100644
--- a/res/res_pjsip/pjsip_options.c
+++ b/res/res_pjsip/pjsip_options.c
@@ -75,7 +75,7 @@
 		ast_log(LOG_ERROR, "Unable to allocate ast_sip_contact_status\n");
 		return NULL;
 	}
-
+	ast_sha1_hash(status->hash, name);
 	status->status = UNKNOWN;
 
 	return status;
@@ -102,7 +102,7 @@
 			contact->uri);
 		return NULL;
 	}
-
+	ast_sha1_hash(status->hash, ast_sorcery_object_get_id(contact));
 	status->status = UNKNOWN;
 	status->rtt_start = ast_tv(0, 0);
 	status->rtt = 0;
@@ -162,7 +162,7 @@
 	update->rtt_start = ast_tv(0, 0);
 
 	ast_statsd_log_full_va("PJSIP.contacts.%s.rtt", AST_STATSD_TIMER,
-		update->rtt / 1000, 1.0, ast_sorcery_object_get_id(update));
+		update->rtt / 1000, 1.0, update->hash);
 	ast_test_suite_event_notify("AOR_CONTACT_QUALIFY_RESULT",
 		"Contact: %s\r\n"
 		"Status: %s\r\n"
@@ -1014,6 +1014,8 @@
 		"0.0", rtt_start_handler, rtt_start_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register_nodoc(sorcery, CONTACT_STATUS, "rtt",
 		"0", OPT_UINT_T, 1, FLDSET(struct ast_sip_contact_status, rtt));
+	ast_sorcery_object_field_register_nodoc(sorcery, CONTACT_STATUS, "hash",
+		"", OPT_CHAR_ARRAY_T, 0, CHARFLDSET(struct ast_sip_contact_status, hash));
 
 	return 0;
 }

-- 
To view, visit https://gerrit.asterisk.org/1727
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2cf2d4e5b5987bda5823a0d984d15f022d5a835c
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>