[Asterisk-code-review] res pjsip: Update logging to show contact->uri in messages (asterisk[13])
Joshua Colp
asteriskteam at digium.com
Thu Dec 3 12:39:08 CST 2015
Joshua Colp has submitted this change and it was merged.
Change subject: res_pjsip: Update logging to show contact->uri in messages
......................................................................
res_pjsip: Update logging to show contact->uri in messages
An earlier commit changed the id of dynamic contacts to contain
a hash instead of the uri. This patch updates status change
logging to show the aor/uri instead of the id. This required
adding the aor id to contact and contact_status and adding
uri to contact_status. The aor id gets added to contact and
contact_status in their allocators and the uri gets added to
contact_status in pjsip_options when the contact_status is
created or updated.
ASTERISK-25598 #close
Reported-by: George Joseph
Tested-by: George Joseph
Change-Id: I56cbec1d2ddbe8461367dd8b6da8a6f47f6fe511
---
M include/asterisk/res_pjsip.h
M res/res_pjsip/location.c
M res/res_pjsip/pjsip_configuration.c
M res/res_pjsip/pjsip_options.c
4 files changed, 141 insertions(+), 108 deletions(-)
Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, approved
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 37b7662..6ca56bd 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -170,6 +170,8 @@
double qualify_timeout;
/*! Endpoint that added the contact, only available in observers */
struct ast_sip_endpoint *endpoint;
+ /*! The name of the aor this contact belongs to */
+ char *aor;
};
#define CONTACT_STATUS "contact_status"
@@ -201,6 +203,10 @@
int64_t rtt;
/*! Last status for a contact (default - unavailable) */
enum ast_sip_contact_status_type last_status;
+ /*! The name of the aor this contact_status belongs to */
+ char *aor;
+ /*! The original contact's URI */
+ char *uri;
};
/*!
diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index 2e0d84b..16913c2 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -92,6 +92,7 @@
struct ast_sip_contact *contact = obj;
ast_string_field_free_memory(contact);
+ ast_free(contact->aor);
ao2_cleanup(contact->endpoint);
}
@@ -99,12 +100,27 @@
static void *contact_alloc(const char *name)
{
struct ast_sip_contact *contact = ast_sorcery_generic_alloc(sizeof(*contact), contact_destroy);
+ char *id = ast_strdupa(name);
+ char *aor = id;
+ char *aor_separator = NULL;
if (!contact) {
return NULL;
}
if (ast_string_field_init(contact, 256)) {
+ ao2_cleanup(contact);
+ return NULL;
+ }
+
+ /* Dynamic contacts are delimited with ";@" and static ones with "@@" */
+ if ((aor_separator = strstr(id, ";@")) || (aor_separator = strstr(id, "@@"))) {
+ *aor_separator = '\0';
+ }
+ ast_assert(aor_separator != NULL);
+
+ contact->aor = ast_strdup(aor);
+ if (!contact->aor) {
ao2_cleanup(contact);
return NULL;
}
@@ -790,13 +806,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 - 2 - strlen(contact->aor) + 1;
- ast_str_append(&context->output_buffer, 0, "%*s: %-*.*s %-12.12s %11.3f\n",
+ ast_str_append(&context->output_buffer, 0, "%*s: %s/%-*.*s %-12.12s %11.3f\n",
indent,
"Contact",
+ contact->aor,
flexwidth, flexwidth,
- wrapper->contact_id,
+ contact->uri,
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_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 7e0c4b0..04002ef 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -56,47 +56,37 @@
return !strcmp(ast_endpoint_get_resource(persistent1->endpoint), id) ? CMP_MATCH | CMP_STOP : 0;
}
-/*! \brief Structure for communicating contact status to
- * persistent_endpoint_update_state from the contact/contact_status
- * observers.
- */
-struct sip_contact_status {
- char *uri;
- enum ast_sip_contact_status_type status;
- int64_t rtt;
-};
-
/*! \brief Callback function for changing the state of an endpoint */
-static int persistent_endpoint_update_state(void *obj, void *arg, void *data, int flags)
+static int persistent_endpoint_update_state(void *obj, void *arg, int flags)
{
struct sip_persistent_endpoint *persistent = obj;
struct ast_endpoint *endpoint = persistent->endpoint;
- char *aor = arg;
- struct sip_contact_status *status = data;
+ struct ast_sip_contact_status *status = arg;
struct ao2_container *contacts;
struct ast_json *blob;
struct ao2_iterator i;
struct ast_sip_contact *contact;
enum ast_endpoint_state state = AST_ENDPOINT_OFFLINE;
- if (!ast_strlen_zero(aor)) {
- if (!strstr(persistent->aors, aor)) {
+ if (status) {
+ char rtt[32];
+
+ /* If the status' aor isn't one of the endpoint's, we skip */
+ if (!strstr(persistent->aors, status->aor)) {
return 0;
}
- if (status) {
- char rtt[32];
- snprintf(rtt, 31, "%" PRId64, status->rtt);
- blob = ast_json_pack("{s: s, s: s, s: s, s: s, s: s}",
- "contact_status", ast_sip_get_contact_status_label(status->status),
- "aor", aor,
- "uri", status->uri,
- "roundtrip_usec", rtt,
- "endpoint_name", ast_endpoint_get_resource(endpoint));
- ast_endpoint_blob_publish(endpoint, ast_endpoint_contact_state_type(), blob);
- ast_json_unref(blob);
- }
+ snprintf(rtt, sizeof(rtt), "%" PRId64, status->rtt);
+ blob = ast_json_pack("{s: s, s: s, s: s, s: s, s: s}",
+ "contact_status", ast_sip_get_contact_status_label(status->status),
+ "aor", status->aor,
+ "uri", status->uri,
+ "roundtrip_usec", rtt,
+ "endpoint_name", ast_endpoint_get_resource(endpoint));
+ ast_endpoint_blob_publish(endpoint, ast_endpoint_contact_state_type(), blob);
+ ast_json_unref(blob);
}
+
/* Find all the contacts for this endpoint. If ANY are available,
* mark the endpoint as ONLINE.
*/
@@ -142,57 +132,56 @@
return 0;
}
-/*! \brief Function called when stuff relating to a contact happens (created/deleted) */
+/*! \brief Function called when a contact is created */
static void persistent_endpoint_contact_created_observer(const void *object)
{
const struct ast_sip_contact *contact = object;
- char *id = ast_strdupa(ast_sorcery_object_get_id(contact));
- char *aor = NULL;
- char *contact_uri = NULL;
- struct sip_contact_status status;
+ struct ast_sip_contact_status *contact_status;
- aor = id;
- /* Dynamic contacts are delimited with ";@" and static ones with "@@" */
- if ((contact_uri = strstr(id, ";@")) || (contact_uri = strstr(id, "@@"))) {
- *contact_uri = '\0';
- contact_uri += 2;
- } else {
- contact_uri = id;
+ contact_status = ast_sorcery_alloc(ast_sip_get_sorcery(), CONTACT_STATUS,
+ ast_sorcery_object_get_id(contact));
+ if (!contact_status) {
+ ast_log(LOG_ERROR, "Unable to create ast_sip_contact_status for contact %s/%s\n",
+ contact->aor, contact->uri);
+ return;
}
+ contact_status->uri = ast_strdup(contact->uri);
+ if (!contact_status->uri) {
+ ao2_cleanup(contact_status);
+ return;
+ }
+ contact_status->status = CREATED;
- status.uri = contact_uri;
- status.status = CREATED;
- status.rtt = 0;
+ ast_verb(1, "Contact %s/%s has been created\n",contact->aor, contact->uri);
- ast_verb(1, "Contact %s/%s has been created\n", aor, contact_uri);
-
- ao2_callback_data(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, aor, &status);
+ ao2_callback(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, contact_status);
+ ao2_cleanup(contact_status);
}
-/*! \brief Function called when stuff relating to a contact happens (created/deleted) */
+/*! \brief Function called when a contact is deleted */
static void persistent_endpoint_contact_deleted_observer(const void *object)
{
- char *id = ast_strdupa(ast_sorcery_object_get_id(object));
- char *aor = NULL;
- char *contact_uri = NULL;
- struct sip_contact_status status;
+ const struct ast_sip_contact *contact = object;
+ struct ast_sip_contact_status *contact_status;
- aor = id;
- /* Dynamic contacts are delimited with ";@" and static ones with "@@" */
- if ((contact_uri = strstr(id, ";@")) || (contact_uri = strstr(id, "@@"))) {
- *contact_uri = '\0';
- contact_uri += 2;
- } else {
- contact_uri = id;
+ contact_status = ast_sorcery_alloc(ast_sip_get_sorcery(), CONTACT_STATUS,
+ ast_sorcery_object_get_id(contact));
+ if (!contact_status) {
+ ast_log(LOG_ERROR, "Unable to create ast_sip_contact_status for contact %s/%s\n",
+ contact->aor, contact->uri);
+ return;
}
+ contact_status->uri = ast_strdup(contact->uri);
+ if (!contact_status->uri) {
+ ao2_cleanup(contact_status);
+ return;
+ }
+ contact_status->status = REMOVED;
- ast_verb(1, "Contact %s/%s has been deleted\n", aor, contact_uri);
+ ast_verb(1, "Contact %s/%s has been deleted\n", contact->aor, contact->uri);
- status.uri = contact_uri;
- status.status = REMOVED;
- status.rtt = 0;
-
- ao2_callback_data(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, aor, &status);
+ ao2_callback(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, contact_status);
+ ao2_cleanup(contact_status);
}
/*! \brief Observer for contacts so state can be updated on respective endpoints */
@@ -201,36 +190,23 @@
.deleted = persistent_endpoint_contact_deleted_observer,
};
-/*! \brief Function called when stuff relating to a contact status happens (updated) */
+/*! \brief Function called when a contact_status is updated */
static void persistent_endpoint_contact_status_observer(const void *object)
{
- const struct ast_sip_contact_status *contact_status = object;
- char *id = ast_strdupa(ast_sorcery_object_get_id(object));
- char *aor = NULL;
- char *contact_uri = NULL;
- struct sip_contact_status status;
+ struct ast_sip_contact_status *contact_status = (struct ast_sip_contact_status *)object;
/* If rtt_start is set (this is the outgoing OPTIONS), ignore. */
if (contact_status->rtt_start.tv_sec > 0) {
return;
}
- aor = id;
- /* Dynamic contacts are delimited with ";@" and static ones with "@@" */
- if ((contact_uri = strstr(id, ";@")) || (contact_uri = strstr(id, "@@"))) {
- *contact_uri = '\0';
- contact_uri += 2;
- } else {
- contact_uri = id;
- }
-
if (contact_status->status == contact_status->last_status) {
- ast_debug(3, "Contact %s status didn't change: %s, RTT: %.3f msec\n",
- contact_uri, ast_sip_get_contact_status_label(contact_status->status),
+ ast_debug(3, "Contact %s/%s status didn't change: %s, RTT: %.3f msec\n",
+ contact_status->aor, contact_status->uri, ast_sip_get_contact_status_label(contact_status->status),
contact_status->rtt / 1000.0);
return;
} else {
- ast_verb(1, "Contact %s/%s is now %s. RTT: %.3f msec\n", aor, contact_uri,
+ ast_verb(1, "Contact %s/%s is now %s. RTT: %.3f msec\n", contact_status->aor, contact_status->uri,
ast_sip_get_contact_status_label(contact_status->status),
contact_status->rtt / 1000.0);
}
@@ -241,11 +217,7 @@
ast_sorcery_object_get_id(contact_status),
ast_sip_get_contact_status_label(contact_status->status));
- status.uri = contact_uri;
- status.status = contact_status->status;
- status.rtt = contact_status->rtt;
-
- ao2_callback_data(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, aor, &status);
+ ao2_callback(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, contact_status);
}
/*! \brief Observer for contacts so state can be updated on respective endpoints */
@@ -1085,7 +1057,7 @@
if (ast_strlen_zero(persistent->aors)) {
ast_endpoint_set_state(persistent->endpoint, AST_ENDPOINT_UNKNOWN);
} else {
- persistent_endpoint_update_state(persistent, NULL, NULL, 0);
+ persistent_endpoint_update_state(persistent, NULL, 0);
}
ao2_link_flags(persistent_endpoints, persistent, OBJ_NOLOCK);
diff --git a/res/res_pjsip/pjsip_options.c b/res/res_pjsip/pjsip_options.c
index b172489..e055787 100644
--- a/res/res_pjsip/pjsip_options.c
+++ b/res/res_pjsip/pjsip_options.c
@@ -42,7 +42,6 @@
[UNKNOWN] = "Unknown",
[CREATED] = "Created",
[REMOVED] = "Removed",
-
};
static const char *short_status_map [] = {
@@ -65,18 +64,43 @@
/*!
* \internal
+ * \brief Destroy a ast_sip_contact_status object.
+ */
+static void contact_status_destroy(void * obj)
+{
+ struct ast_sip_contact_status *status = obj;
+
+ ast_free(status->aor);
+ ast_free(status->uri);
+}
+
+/*!
+ * \internal
* \brief Create a ast_sip_contact_status object.
*/
static void *contact_status_alloc(const char *name)
{
- struct ast_sip_contact_status *status = ast_sorcery_generic_alloc(sizeof(*status), NULL);
+ struct ast_sip_contact_status *status = ast_sorcery_generic_alloc(sizeof(*status), contact_status_destroy);
+ char *id = ast_strdupa(name);
+ char *aor = id;
+ char *aor_separator = NULL;
if (!status) {
ast_log(LOG_ERROR, "Unable to allocate ast_sip_contact_status\n");
return NULL;
}
- status->status = UNKNOWN;
+ /* Dynamic contacts are delimited with ";@" and static ones with "@@" */
+ if ((aor_separator = strstr(id, ";@")) || (aor_separator = strstr(id, "@@"))) {
+ *aor_separator = '\0';
+ }
+ ast_assert(aor_separator != NULL);
+
+ status->aor = ast_strdup(aor);
+ if (!status->aor) {
+ ao2_cleanup(status);
+ return NULL;
+ }
return status;
}
@@ -98,12 +122,17 @@
status = ast_sorcery_alloc(ast_sip_get_sorcery(), CONTACT_STATUS,
ast_sorcery_object_get_id(contact));
if (!status) {
- ast_log(LOG_ERROR, "Unable to create ast_sip_contact_status for contact %s\n",
- contact->uri);
+ ast_log(LOG_ERROR, "Unable to create ast_sip_contact_status for contact %s/%s\n",
+ contact->aor, contact->uri);
return NULL;
}
- status->status = UNKNOWN;
+ status->uri = ast_strdup(contact->uri);
+ if (!status->uri) {
+ ao2_cleanup(status);
+ return NULL;
+ }
+
status->rtt_start = ast_tv(0, 0);
status->rtt = 0;
@@ -127,8 +156,8 @@
static void update_contact_status(const struct ast_sip_contact *contact,
enum ast_sip_contact_status_type value)
{
- struct ast_sip_contact_status *status;
- struct ast_sip_contact_status *update;
+ RAII_VAR(struct ast_sip_contact_status *, status, NULL, ao2_cleanup);
+ RAII_VAR(struct ast_sip_contact_status *, update, NULL, ao2_cleanup);
status = ast_res_pjsip_find_or_create_contact_status(contact);
if (!status) {
@@ -142,6 +171,11 @@
if (!update) {
ast_log(LOG_ERROR, "Unable to allocate ast_sip_contact_status for contact %s\n",
contact->uri);
+ return;
+ }
+
+ update->uri = ast_strdup(contact->uri);
+ if (!update->uri) {
return;
}
@@ -175,9 +209,6 @@
ast_log(LOG_ERROR, "Unable to update ast_sip_contact_status for contact %s\n",
contact->uri);
}
-
- ao2_ref(status, -1);
- ao2_ref(update, -1);
}
/*!
@@ -187,8 +218,8 @@
*/
static void init_start_time(const struct ast_sip_contact *contact)
{
- struct ast_sip_contact_status *status;
- struct ast_sip_contact_status *update;
+ RAII_VAR(struct ast_sip_contact_status *, status, NULL, ao2_cleanup);
+ RAII_VAR(struct ast_sip_contact_status *, update, NULL, ao2_cleanup);
status = ast_res_pjsip_find_or_create_contact_status(contact);
if (!status) {
@@ -205,6 +236,11 @@
return;
}
+ update->uri = ast_strdup(contact->uri);
+ if (!update->uri) {
+ return;
+ }
+
update->status = status->status;
update->last_status = status->last_status;
update->rtt = status->rtt;
@@ -214,9 +250,6 @@
ast_log(LOG_ERROR, "Unable to update ast_sip_contact_status for contact %s\n",
contact->uri);
}
-
- ao2_ref(status, -1);
- ao2_ref(update, -1);
}
/*!
@@ -993,6 +1026,9 @@
return 0;
}
+static char status_value_unknown[2];
+static char status_value_created[2];
+
int ast_sip_initialize_sorcery_qualify(void)
{
struct ast_sorcery *sorcery = ast_sip_get_sorcery();
@@ -1006,10 +1042,12 @@
return -1;
}
+ snprintf(status_value_unknown, sizeof(status_value_unknown), "%u", UNKNOWN);
ast_sorcery_object_field_register_nodoc(sorcery, CONTACT_STATUS, "last_status",
- "0", OPT_UINT_T, 1, FLDSET(struct ast_sip_contact_status, last_status));
+ status_value_unknown, OPT_UINT_T, 1, FLDSET(struct ast_sip_contact_status, last_status));
+ snprintf(status_value_created, sizeof(status_value_created), "%u", CREATED);
ast_sorcery_object_field_register_nodoc(sorcery, CONTACT_STATUS, "status",
- "0", OPT_UINT_T, 1, FLDSET(struct ast_sip_contact_status, status));
+ status_value_created, OPT_UINT_T, 1, FLDSET(struct ast_sip_contact_status, status));
ast_sorcery_object_field_register_custom_nodoc(sorcery, CONTACT_STATUS, "rtt_start",
"0.0", rtt_start_handler, rtt_start_to_str, NULL, 0, 0);
ast_sorcery_object_field_register_nodoc(sorcery, CONTACT_STATUS, "rtt",
--
To view, visit https://gerrit.asterisk.org/1731
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I56cbec1d2ddbe8461367dd8b6da8a6f47f6fe511
Gerrit-PatchSet: 12
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list