[Asterisk-code-review] res pjsip outbound registration.c: Filter redundant statsd r... (asterisk[master])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/4543 )

Change subject: res_pjsip_outbound_registration.c: Filter redundant statsd reporting.
......................................................................


res_pjsip_outbound_registration.c: Filter redundant statsd reporting.

Increasing the testsuite shutdown timeout before forcibly killing
Asterisk allowed more events to be sent out.  Some tests failed as
a result.  The tests/channels/pjsip/statsd/registrations failed
because we now get the statsd events that a comment in the test
configuration stated couldn't be intercepted.  Unfortunately, we
get a variable number of events because of internal status state
transition races generating redundant statsd events.

We were reporting redundant statsd PJSIP.registrations.state changes
for internal state changes that equated to the same thing publicly.

* Made update_client_state_status() filter out redundant statsd
updates.

ASTERISK-26527

Change-Id: If851c7d514bb530d9226e4941ba97dcf52000646
---
M CHANGES
M res/res_pjsip_outbound_registration.c
2 files changed, 24 insertions(+), 3 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/CHANGES b/CHANGES
index 886cd00..431218f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -79,6 +79,12 @@
  * Added new pjproject.conf startup section "log_level' option to set the
    initial maximum PJPROJECT logging level.
 
+res_pjsip_outbound_registration
+------------------
+ * Statsd no longer logs redundant status PJSIP.registrations.state changes
+   for internal state transitions that don't change the reported public status
+   state.
+
 ------------------------------------------------------------------------------
 --- Functionality changes from Asterisk 14.1.0 to Asterisk 14.2.0 ------------
 ------------------------------------------------------------------------------
diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 29701a1..d486ccd 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -627,15 +627,30 @@
 
 static void update_client_state_status(struct sip_outbound_registration_client_state *client_state, enum sip_outbound_registration_status status)
 {
+	const char *status_old;
+	const char *status_new;
+
 	if (client_state->status == status) {
+		/* Status state did not change at all. */
+		return;
+	}
+
+	status_old = sip_outbound_registration_status_str(client_state->status);
+	status_new = sip_outbound_registration_status_str(status);
+	client_state->status = status;
+
+	if (!strcmp(status_old, status_new)) {
+		/*
+		 * The internal status state may have changed but the status
+		 * state we tell the world did not change at all.
+		 */
 		return;
 	}
 
 	ast_statsd_log_string_va("PJSIP.registrations.state.%s", AST_STATSD_GAUGE, "-1", 1.0,
-		sip_outbound_registration_status_str(client_state->status));
+		status_old);
 	ast_statsd_log_string_va("PJSIP.registrations.state.%s", AST_STATSD_GAUGE, "+1", 1.0,
-		sip_outbound_registration_status_str(status));
-	client_state->status = status;
+		status_new);
 }
 
 /*! \brief Callback function for unregistering (potentially) and destroying state */

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

Gerrit-MessageType: merged
Gerrit-Change-Id: If851c7d514bb530d9226e4941ba97dcf52000646
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>