CDRs: fix a variety of dial status problems, h/hangup handler creating CDRs
authorMatthew Jordan <mjordan@digium.com>
Fri, 31 Jan 2014 23:40:51 +0000 (23:40 +0000)
committerMatthew Jordan <mjordan@digium.com>
Fri, 31 Jan 2014 23:40:51 +0000 (23:40 +0000)
This patch fixes a number of small-ish problems that were noticed when
witnessing the records that the FreePBX dialplan produces:
(1) Mid-call events (as well as privacy options) have the ability to change the
    overall state of the Dial operation after the called party answers. This
    means that publishing the DialEnd event when the called party is premature;
    we have to wait for the execution of these subroutines to complete before
    we can signal the overall status of the DialEnd. This patch moves that
    publication and adds handlers for the mid-call events.
(2) The AST_FLAG_OUTGOING channel flag is cleared if an after bridge goto
    datastore is detected. This flag was preventing CDRs from being recorded
    for all outbound channels that had a 'continue' option enabled on them by
    the Dial application.
(3) The CDR engine now locks the 'Dial' application as being the CDR
    application if it detects that the current CDR has entered that app. This
    is similar to the logic that is done for Parking. In general, if we entered
    into Dial, then we want that CDR to record the application as such - this
    prevents pre-dial handlers, mid-call handlers, and other shenaniganry
    from changing the application value.
(4) The CDR engine now checks for the AST_SOFTHANGUP_HANGUP_EXEC in more places
    to determine if the channel is in hangup logic or dead. In either case, we
    don't want to record changes in the channel.
(5) The default option for "endbeforehexten" has been changed to "yes". In
    general, you don't want to see CDRs in the 'h' exten or in hangup logic.
    Since the semantics of that option changed in 12, it made sense to update
    the default value as well.
(6) Finally, because we now have the ability to synchronize on the messages
    published to the CDR topic, on shutdown the CDR engine will now synchronize
    to the messages currently in flight. This helps to ensure that all
    in-flight CDRs are written before shutting down.

(closes issue ASTERISK-23164)
Reported by: Matt Jordan

Review: https://reviewboard.asterisk.org/r/3154
........

Merged revisions 407084 from http://svn.asterisk.org/svn/asterisk/branches/12

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@407085 65c4cc65-6c06-0410-ace0-fbb531ad65f3

UPGRADE.txt
apps/app_dial.c
main/bridge_after.c
main/cdr.c
main/manager_channels.c
main/pbx.c

index 4948ba4..d0460fb 100644 (file)
@@ -22,7 +22,8 @@
 ===========================================================
 
 From 12 to 13:
-* The per console verbose level feature as previously implemented caused a
+
+- The per console verbose level feature as previously implemented caused a
   large performance penalty.  The fix required some minor incompatibilities
   if the new rasterisk is used to connect to an earlier version.  If the new
   rasterisk connects to an older Asterisk version then the root console verbose
@@ -31,6 +32,13 @@ From 12 to 13:
   an older version of rasterisk connects to the new version then the
   "core set verbose" command will have no effect.
 
+CDRs:
+ - The "endbeforehexten" setting now defaults to "yes", instead of "no".
+   When set to "no", yhis setting will cause a new CDR to be generated when a
+   channel enters into hangup logic (either the 'h' extension or a hangup
+   handler subroutine). In general, this is not the preferred default: this
+   causes extra CDRs to be generated for a channel in many common dialplans.
+
 CLI commands:
  - "core show settings" now lists the current console verbosity in addition
    to the root console verbosity.
@@ -51,25 +59,25 @@ Configuration Files:
    instead of a "basic" one.
 
 Realtime Configuration:
- * New columns have been added to realtime tables for 'support_path' on
+ - New columns have been added to realtime tables for 'support_path' on
    ps_registrations and ps_aors and for 'path' on ps_contacts for the new
    SIP Path support in chan_pjsip.
- * The following new tables have been added for pjsip realtime: 'ps_systems',
+ - The following new tables have been added for pjsip realtime: 'ps_systems',
    'ps_globals', 'ps_tranports', 'ps_registrations'.
- * The following columns were added to the 'ps_aors' realtime table:
+ - The following columns were added to the 'ps_aors' realtime table:
    'maximum_expiration', 'outbound_proxy', and 'support_path'.
- * The following columns were added to the 'ps_contacts' realtime table:
+ - The following columns were added to the 'ps_contacts' realtime table:
    'outbound_proxy' and 'path'.
- * New columns have been added to the ps_endpoints realtime table for the
+ - New columns have been added to the ps_endpoints realtime table for the
    'media_address', 'redirect_method' and 'set_var' options.  Also the
    'mwi_fromuser' column was renamed to 'mwi_from_user'.
- * WARNING: The database migration script that adds the 'extensions' table for
+ - WARNING: The database migration script that adds the 'extensions' table for
    realtime had to be modified due to an error when installing for MySQL.  The
    'extensions' table's 'id' column was changed to be a primary key.  This could
    potentially cause a migration problem.  If so, it may be necessary to
    manually alter the affected table/column to bring it back in line with the
    migration scripts.
- * A new column was added to the 'ps_globals' realtime table for the 'debug'
+ - A new column was added to the 'ps_globals' realtime table for the 'debug'
    option.
 
 
index 6d2437a..ce73e9a 100644 (file)
@@ -1285,7 +1285,10 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in,
                                                        }
                                                }
                                                peer = c;
-                                               ast_channel_publish_dial(in, peer, NULL, "ANSWER");
+                                               /* Inform everyone else that they've been canceled.
+                                                * The dial end event for the peer will be sent out after
+                                                * other Dial options have been handled.
+                                                */
                                                publish_dial_end_event(in, out_chans, peer, "CANCEL");
                                                ast_copy_flags64(peerflags, o,
                                                        OPT_CALLEE_TRANSFER | OPT_CALLER_TRANSFER |
@@ -2739,6 +2742,7 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
                }
                if ( (ast_test_flag64(&opts, OPT_PRIVACY) || ast_test_flag64(&opts, OPT_SCREENING)) && pa.privdb_val == AST_PRIVACY_UNKNOWN) {
                        if (do_privacy(chan, peer, &opts, opt_args, &pa)) {
+                               ast_channel_publish_dial(chan, peer, NULL, pa.status);
                                res = 0;
                                goto out;
                        }
@@ -2836,6 +2840,7 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
                        if (continue_exec)
                                *continue_exec = 1;
                        res = 0;
+                       ast_channel_publish_dial(chan, peer, NULL, "ANSWER");
                        goto done;
                }
 
@@ -2848,7 +2853,6 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
                        ast_channel_exten_set(peer, ast_channel_exten(chan));
                        ast_channel_unlock(peer);
                        ast_channel_unlock(chan);
-
                        ast_replace_subargument_delimiter(opt_args[OPT_ARG_CALLEE_MACRO]);
                        res = ast_app_exec_macro(chan, peer, opt_args[OPT_ARG_CALLEE_MACRO]);
 
@@ -2889,6 +2893,7 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
                                                ast_set_flag64(peerflags, OPT_GO_ON);
                                        }
                                }
+                               ast_channel_publish_dial(chan, peer, NULL, macro_result);
                        } else {
                                ast_channel_unlock(peer);
                        }
@@ -2971,6 +2976,7 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
                                                ast_set_flag64(peerflags, OPT_GO_ON);
                                        }
                                }
+                               ast_channel_publish_dial(chan, peer, NULL, gosub_result);
                        } else {
                                ast_channel_unlock(peer);
                                ast_channel_unlock(chan);
@@ -2978,6 +2984,12 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
                }
 
                if (!res) {
+
+                       /* None of the Dial options changed our status; inform
+                        * everyone that this channel answered
+                        */
+                       ast_channel_publish_dial(chan, peer, NULL, "ANSWER");
+
                        if (!ast_tvzero(calldurationlimit)) {
                                struct timeval whentohangup = ast_tvadd(ast_tvnow(), calldurationlimit);
                                ast_channel_lock(peer);
index 7a69395..3d1d2f3 100644 (file)
@@ -477,6 +477,9 @@ int ast_bridge_setup_after_goto(struct ast_channel *chan)
                        ast_pbx_h_exten_run(chan, after_bridge->context);
                }
        } else if (!ast_check_hangup(chan)) {
+               /* Clear the outgoing flag */
+               ast_clear_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
+
                if (after_bridge->specific) {
                        goto_failed = ast_explicit_goto(chan, after_bridge->context,
                                after_bridge->exten, after_bridge->priority);
index d83d506..2b67ba9 100644 (file)
@@ -129,7 +129,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
                                                channel. Any statistics are gathered from this new CDR. By enabling
                                                this option, no new CDR is created for the dialplan logic that is
                                                executed in <literal>h</literal> extensions or attached hangup handler
-                                               subroutines. The default value is <literal>no</literal>, indicating
+                                               subroutines. The default value is <literal>yes</literal>, indicating
                                                that a CDR will be generated during hangup logic.</para>
                                        </description>
                                </configOption>
@@ -203,7 +203,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #define DEFAULT_BATCHMODE "0"
 #define DEFAULT_UNANSWERED "0"
 #define DEFAULT_CONGESTION "0"
-#define DEFAULT_END_BEFORE_H_EXTEN "0"
+#define DEFAULT_END_BEFORE_H_EXTEN "1"
 #define DEFAULT_INITIATED_SECONDS "0"
 
 #define DEFAULT_BATCH_SIZE "100"
@@ -347,6 +347,9 @@ static struct stasis_forward *parking_subscription;
 /*! \brief The parent topic for all topics we want to aggregate for CDRs */
 static struct stasis_topic *cdr_topic;
 
+/*! \brief A message type used to synchronize with the CDR topic */
+STASIS_MESSAGE_TYPE_DEFN_LOCAL(cdr_sync_message_type);
+
 struct cdr_object;
 
 /*! \brief Return types for \ref process_bridge_enter functions */
@@ -1089,7 +1092,7 @@ static struct ast_cdr *cdr_object_create_public_records(struct cdr_object *cdr)
 
                /* Don't create records for CDRs where the party A was a dialed channel */
                if (snapshot_is_dialed(it_cdr->party_a.snapshot) && !it_cdr->party_b.snapshot) {
-                       CDR_DEBUG(mod_cfg, "%p - %s is dialed and has no Party B; discarding\n", it_cdr,
+                       ast_debug(1, "CDR for %s is dialed and has no Party B; discarding\n",
                                it_cdr->party_a.snapshot->name);
                        continue;
                }
@@ -1347,7 +1350,14 @@ static int base_process_party_a(struct cdr_object *cdr, struct ast_channel_snaps
        RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);
 
        ast_assert(strcasecmp(snapshot->name, cdr->party_a.snapshot->name) == 0);
+
        cdr_object_swap_snapshot(&cdr->party_a, snapshot);
+       /* Ignore any snapshots from a dead or dying channel */
+       if (ast_test_flag(&snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)
+                       && ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN)) {
+               cdr_object_check_party_a_hangup(cdr);
+               return 0;
+       }
 
        /* When Party A is originated to an application and the application exits, the stack
         * will attempt to clear the application and restore the dummy originate application
@@ -1359,6 +1369,15 @@ static int base_process_party_a(struct cdr_object *cdr, struct ast_channel_snaps
                        && !ast_test_flag(&cdr->flags, AST_CDR_LOCK_APP)) {
                ast_string_field_set(cdr, appl, snapshot->appl);
                ast_string_field_set(cdr, data, snapshot->data);
+
+               /* Dial (app_dial) is a special case. Because pre-dial handlers, which
+                * execute before the dial begins, will alter the application/data to
+                * something people typically don't want to see, if we see a channel enter
+                * into Dial here, we set the appl/data accordingly and lock it.
+                */
+               if (!strcmp(snapshot->appl, "Dial")) {
+                       ast_set_flag(&cdr->flags, AST_CDR_LOCK_APP);
+               }
        }
 
        ast_string_field_set(cdr, linkedid, snapshot->linkedid);
@@ -1434,6 +1453,12 @@ static int single_state_process_dial_begin(struct cdr_object *cdr, struct ast_ch
                cdr_object_swap_snapshot(&cdr->party_b, peer);
                CDR_DEBUG(mod_cfg, "%p - Updated Party B %s snapshot\n", cdr,
                                cdr->party_b.snapshot->name);
+
+               /* If we have two parties, lock the application that caused the
+                * two parties to be associated. This prevents mid-call event
+                * macros/gosubs from perturbing the CDR application/data
+                */
+               ast_set_flag(&cdr->flags, AST_CDR_LOCK_APP);
        } else if (!strcasecmp(cdr->party_a.snapshot->name, peer->name)) {
                /* We're the entity being dialed, i.e., outbound origination */
                cdr_object_swap_snapshot(&cdr->party_a, peer);
@@ -1822,9 +1847,8 @@ static int finalized_state_process_party_a(struct cdr_object *cdr, struct ast_ch
        RAII_VAR(struct module_config *, mod_cfg,
                ao2_global_obj_ref(module_configs), ao2_cleanup);
 
-       /* If we ignore hangup logic, indicate that we don't need a new CDR */
-       if (ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN)
-               && ast_test_flag(&snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)) {
+       if (ast_test_flag(&snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)
+                       && ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN)) {
                return 0;
        }
 
@@ -1922,7 +1946,7 @@ static void handle_dial_message(void *data, struct stasis_subscription *sub, str
                                continue;
                        }
                        CDR_DEBUG(mod_cfg, "%p - Processing Dial Begin message for channel %s, peer %s\n",
-                                       cdr,
+                                       it_cdr,
                                        caller ? caller->name : "(none)",
                                        peer ? peer->name : "(none)");
                        res &= it_cdr->fn_table->process_dial_begin(it_cdr,
@@ -1933,7 +1957,7 @@ static void handle_dial_message(void *data, struct stasis_subscription *sub, str
                                continue;
                        }
                        CDR_DEBUG(mod_cfg, "%p - Processing Dial End message for channel %s, peer %s\n",
-                                       cdr,
+                                       it_cdr,
                                        caller ? caller->name : "(none)",
                                        peer ? peer->name : "(none)");
                        it_cdr->fn_table->process_dial_end(it_cdr,
@@ -1999,11 +2023,10 @@ static int check_new_cdr_needed(struct ast_channel_snapshot *old_snapshot,
        RAII_VAR(struct module_config *, mod_cfg,
                        ao2_global_obj_ref(module_configs), ao2_cleanup);
 
-       if (!new_snapshot) {
-               return 0;
-       }
-
-       if (ast_test_flag(&new_snapshot->flags, AST_FLAG_DEAD)) {
+       /* If we're dead, we don't need a new CDR */
+       if (!new_snapshot
+               || (ast_test_flag(&new_snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)
+                       && ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN))) {
                return 0;
        }
 
@@ -2194,6 +2217,7 @@ static void handle_bridge_leave_message(void *data, struct stasis_subscription *
        /* Party A */
        ao2_lock(cdr);
        for (it_cdr = cdr; it_cdr; it_cdr = it_cdr->next) {
+
                if (!it_cdr->fn_table->process_bridge_leave) {
                        continue;
                }
@@ -2562,6 +2586,19 @@ static void handle_parked_call_message(void *data, struct stasis_subscription *s
 
 }
 
+/*!
+ * \brief Handler for a synchronization message
+ * \param data Passed on
+ * \param sub The stasis subscription for this message callback
+ * \param topic The topic this message was published for
+ * \param message A blank ao2 object
+ * */
+static void handle_cdr_sync_message(void *data, struct stasis_subscription *sub,
+               struct stasis_message *message)
+{
+       return;
+}
+
 struct ast_cdr_config *ast_cdr_get_config(void)
 {
        RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);
@@ -3337,6 +3374,11 @@ int ast_cdr_fork(const char *channel_name, struct ast_flags *options)
                new_cdr->fn_table = cdr_obj->fn_table;
                ast_string_field_set(new_cdr, bridge, cdr->bridge);
                new_cdr->flags = cdr->flags;
+               /* Explicitly clear the AST_CDR_LOCK_APP flag - we want
+                * the application to be changed on the new CDR if the
+                * dialplan demands it
+                */
+               ast_clear_flag(&new_cdr->flags, AST_CDR_LOCK_APP);
 
                /* If there's a Party B, copy it over as well */
                if (cdr_obj->party_b.snapshot) {
@@ -4017,6 +4059,8 @@ static void cdr_engine_shutdown(void)
        ao2_cleanup(cdr_topic);
        cdr_topic = NULL;
 
+       STASIS_MESSAGE_TYPE_CLEANUP(cdr_sync_message_type);
+
        ao2_callback(active_cdrs_by_channel, OBJ_NODATA, cdr_object_dispatch_all_cb,
                NULL);
        finalize_batch_mode();
@@ -4120,11 +4164,17 @@ int ast_cdr_engine_init(void)
        if (!stasis_router) {
                return -1;
        }
+
+       if (STASIS_MESSAGE_TYPE_INIT(cdr_sync_message_type)) {
+               return -1;
+       }
+
        stasis_message_router_add_cache_update(stasis_router, ast_channel_snapshot_type(), handle_channel_cache_message, NULL);
        stasis_message_router_add(stasis_router, ast_channel_dial_type(), handle_dial_message, NULL);
        stasis_message_router_add(stasis_router, ast_channel_entered_bridge_type(), handle_bridge_enter_message, NULL);
        stasis_message_router_add(stasis_router, ast_channel_left_bridge_type(), handle_bridge_leave_message, NULL);
        stasis_message_router_add(stasis_router, ast_parked_call_type(), handle_parked_call_message, NULL);
+       stasis_message_router_add(stasis_router, cdr_sync_message_type(), handle_cdr_sync_message, NULL);
 
        active_cdrs_by_channel = ao2_container_alloc(NUM_CDR_BUCKETS,
                cdr_object_channel_hash_fn, cdr_object_channel_cmp_fn);
@@ -4149,6 +4199,8 @@ int ast_cdr_engine_init(void)
 void ast_cdr_engine_term(void)
 {
        RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);
+       RAII_VAR(void *, payload, ao2_alloc(sizeof(*payload), NULL), ao2_cleanup);
+       RAII_VAR(struct stasis_message *, message, NULL, ao2_cleanup);
 
        /* Since this is called explicitly during process shutdown, we might not have ever
         * been initialized. If so, the config object will be NULL.
@@ -4156,10 +4208,22 @@ void ast_cdr_engine_term(void)
        if (!mod_cfg) {
                return;
        }
-       if (!ast_test_flag(&mod_cfg->general->settings, CDR_BATCHMODE)) {
+
+       /* Make sure we have the needed items */
+       if (!stasis_router || !payload) {
                return;
        }
-       cdr_submit_batch(ast_test_flag(&mod_cfg->general->batch_settings.settings, BATCH_MODE_SAFE_SHUTDOWN));
+
+       ast_debug(1, "CDR Engine termination request received; waiting on messages...\n");
+
+       message = stasis_message_create(cdr_sync_message_type(), payload);
+       if (message) {
+               stasis_message_router_publish_sync(stasis_router, message);
+       }
+
+       if (ast_test_flag(&mod_cfg->general->settings, CDR_BATCHMODE)) {
+               cdr_submit_batch(ast_test_flag(&mod_cfg->general->batch_settings.settings, BATCH_MODE_SAFE_SHUTDOWN));
+       }
 }
 
 int ast_cdr_engine_reload(void)
index 0bebb21..3fdcc96 100644 (file)
@@ -151,12 +151,38 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
                                <parameter name="DialStatus">
                                        <para>The result of the dial operation.</para>
                                        <enumlist>
-                                               <enum name="ANSWER" />
-                                               <enum name="BUSY" />
-                                               <enum name="CANCEL" />
-                                               <enum name="CHANUNAVAIL" />
-                                               <enum name="CONGESTION" />
-                                               <enum name="NOANSWER" />
+                                               <enum name="ABORT">
+                                                       <para>The call was aborted.</para>
+                                               </enum>
+                                               <enum name="ANSWER">
+                                                       <para>The caller answered.</para>
+                                               </enum>
+                                               <enum name="BUSY">
+                                                       <para>The caller was busy.</para>
+                                               </enum>
+                                               <enum name="CANCEL">
+                                                       <para>The caller cancelled the call.</para>
+                                               </enum>
+                                               <enum name="CHANUNAVAIL">
+                                                       <para>The requested channel is unavailable.</para>
+                                               </enum>
+                                               <enum name="CONGESTION">
+                                                       <para>The called party is congested.</para>
+                                               </enum>
+                                               <enum name="CONTINUE">
+                                                       <para>The dial completed, but the caller elected
+                                                       to continue in the dialplan.</para>
+                                               </enum>
+                                               <enum name="GOTO">
+                                                       <para>The dial completed, but the caller jumped to
+                                                       a dialplan location.</para>
+                                                       <para>If known, the location the caller is jumping
+                                                       to will be appended to the result following a
+                                                       ":".</para>
+                                               </enum>
+                                               <enum name="NOANSWER">
+                                                       <para>The called party failed to answer.</para>
+                                               </enum>
                                        </enumlist>
                                </parameter>
                        </syntax>
index 19db8b2..65865ff 100644 (file)
@@ -5914,6 +5914,12 @@ void ast_pbx_h_exten_run(struct ast_channel *chan, const char *context)
 
        ast_channel_lock(chan);
 
+       /*
+        * Make sure that the channel is marked as hungup since we are
+        * going to run the h exten on it.
+        */
+       ast_softhangup_nolock(chan, AST_SOFTHANGUP_HANGUP_EXEC);
+
        /* Set h exten location */
        if (context != ast_channel_context(chan)) {
                ast_channel_context_set(chan, context);
@@ -5921,12 +5927,6 @@ void ast_pbx_h_exten_run(struct ast_channel *chan, const char *context)
        ast_channel_exten_set(chan, "h");
        ast_channel_priority_set(chan, 1);
 
-       /*
-        * Make sure that the channel is marked as hungup since we are
-        * going to run the h exten on it.
-        */
-       ast_softhangup_nolock(chan, AST_SOFTHANGUP_HANGUP_EXEC);
-
        /* Save autoloop flag */
        autoloopflag = ast_test_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP);
        ast_set_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP);