Review Board 1.7.16

Track On/Off- Hold events in queue_log

Review Request #2417 - Created March 28, 2013 and updated

Steve Murphy
This patch introduces some new events in the queue_log, to track on-hold, and off-hold events,
which QueueMetrics (and anyone else) can use to measure hold times, a traditionally import
metric for Call Centers.

It uses the ast_event interface to generate the events and execute call-back funcs in app_queue.c.

The result are CALLERONHOLD and CALLEROFFHOLD events in queue_log:


The CALLERONHOLD event also records the musicclass associated with the MOH.

By using hashtabs in app_queue, the code is optimized for the large-queue cases.
Every MOH call will generate a callback into the app_queue module; it is important
that the code quickly determine whether the MOH event is associated with a queue call.
No Linked-list searches allowed!

This code also drags a few events back into app_queue, namely the code in chan_agent that
recorded agent log-ins and log-outs.

I've tested this code on 1.8 and trunk, and because of the asynchronous nature of 
the queue logging, included a few lines of code to restrict ONHOLD and OFFHOLD events
such that OFFHOLD events will only be generated after an ONHOLD event, and vice-versa.

Others have tested the code and the results are so far positive. We hope to get some
"smaller" production boxes online with this code, and see how it performs "under load".

On trunk, with small queues; on 1.8 (backported) in at least 2 sites so far (at least, 
according to feedback.
Posted (April 4, 2013, 2:28 p.m.)
Was there any particular reason you chose to use ast_hashtab instead of an ao2_container?  Only pbx and sched use ast_hashtab.  The rest of the system uses the ao2_container which by default is a hash table.

At this time, the event system is being replaced with stasis in trunk.  For the MOH events, you would only need to subscribe to the existing events like AMI will do.
/trunk/apps/app_queue.c (Diff revision 1)
You should make the doxygen comments here actually state what the subscription is for rather than copy-pasta from the device state subscription. :)
/trunk/apps/app_queue.c (Diff revision 1)
You don't need to test for NULL before calling ast_free().  ast_free() is NULL tolerant just like free().
/trunk/apps/app_queue.c (Diff revision 1)
This is an inlined version of connected_member_destroy().
/trunk/apps/app_queue.c (Diff revision 1) and onhold event...
/trunk/apps/app_queue.c (Diff revision 1)
Red blob.
/trunk/channels/chan_agent.c (Diff revision 1)
You need to use AST_EVENT_Q_AGENT_LOGOFF here instead.
/trunk/include/asterisk/event_defs.h (Diff revision 1)
These should be added to the event_names[] in events.c
/trunk/include/asterisk/event_defs.h (Diff revision 1)
These event ies need to be added to the ie_maps table in events.c.
/trunk/include/asterisk/event_defs.h (Diff revision 1)
There already is an AST_EVENT_IE_CEL_CHANNAME just like the AST_EVENT_IE_CEL_UNIQUEID that you borrowed. runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to