[asterisk-dev] [Code Review] Asterisk Unique ID for call logging Phase II

Tue Mar 20 15:18:42 CDT 2012

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1823/#review5841
-----------------------------------------------------------


/trunk/include/asterisk/logger.h
<https://reviewboard.asterisk.org/r/1823/#comment10680>

    already have the read blob. It was copied from the above function unfortunately.


- jrose


On March 20, 2012, 3:14 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1823/
> -----------------------------------------------------------
> 
> (Updated March 20, 2012, 3:14 p.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson, junky, and Matt Jordan.
> 
> 
> Summary
> -------
> 
> https://wiki.asterisk.org/wiki/display/AST/Unique+Call-ID+Logging
> 
> Since none of this patch has appeared on the public reviewboard yet, I'll go ahead and fill everyone in on phase I's objectives which have already been met.
> Phase I
>     Implement <call-id> structure and factory. Bind <call-id> pointers to newly created channel threads.
>     Modify logging to read thread storage so that channel thread log messages include the <call-id> stamp.
> 
> 
> As for the current objective...
> Phase II
>     1. Spread references to ast_callid structs to threads created by a pbx/channel thread already bound to a <call-id>.
>     2. Add CLI command to display <call-id> with verbose messages on CLI.
> 
> The primary focus of this review is a sanity check for some of the thread associations that are being worked on for this feature and also to get some community feedback on the feature in general since it hasn't seen a great deal of that yet.
> 
> What I'm looking to learn from this review is if I missed any obvious places where this callid should be passed to a new thread and if the various places that I am passing it around right now are all appropriate.  It could also be that there is a better approach to handling this passing of callids that I'm just not aware of right now.
> 
> This code can be found in the following repository:
> https://origsvn.digium.com/svn/asterisk/team/jrose/call_identifiers
> 
> 
> Diffs
> -----
> 
>   /trunk/main/pbx.c 360031 
>   /trunk/apps/app_mixmonitor.c 360031 
>   /trunk/include/asterisk/bridging.h 360031 
>   /trunk/include/asterisk/ccss.h 360031 
>   /trunk/include/asterisk/logger.h 360031 
>   /trunk/main/bridging.c 360031 
>   /trunk/main/ccss.c 360031 
>   /trunk/main/dial.c 360031 
>   /trunk/main/features.c 360031 
>   /trunk/main/logger.c 360031 
> 
> Diff: https://reviewboard.asterisk.org/r/1823/diff
> 
> 
> Testing
> -------
> 
> Honestly, I'm not sure if all of these thread associations are appropriate or not.  The one I understand best right now is mixmonitor, which handles thread associations like you would expect... it inherits them from the parent thread if the parent thread is associated with a call-id.  In other words, if it's activated from the dialplan, it will be a part of the call.  If it's activated from AMI or CLI, then the mixmonitor won't have a callid associated with it. This doesn't necessarily seem inappropriate, though it wouldn't necessarily be inappropriate to go ahead and bind mixmonitor to the callid when created in those ways either... but nothing can be done about that until the additon to handle callids in channels goes through in phase III.
> 
> For a little demonstration of this feature in action, the following is a call where the following happens:
> 1. SIP/gold dials an extension which mixmonitors a dial to SIP/101
> 2. SIP/gold holds on SIP/101 and then unholds
> 3. SIP/gold blind transfers SIP/101 to SIP/123
> 4. SIP/101 hangs up the line
> 
> [2012-03-2015:01:30] VERBOSE[28696] netsock2.c:   == Using SIP RTP CoS mark 5
> [2012-03-2015:01:30] DEBUG[28729] logger.c: CALL_ID [C000000] created by thread.
> [2012-03-2015:01:30] DEBUG[28729][C000000] logger.c: CALL_ID [C000000] bound to thread.
> [2012-03-2015:01:30] VERBOSE[28729][C000000] pbx.c:     -- Executing [006 at sipphones:1] MixMonitor("SIP/gold-00000000", "recordingx.wav,m(1111 at default)") in new stack
> [2012-03-2015:01:30] VERBOSE[28729][C000000] pbx.c:     -- Executing [006 at sipphones:2] Dial("SIP/gold-00000000", "SIP/101") in new stack
> [2012-03-2015:01:30] DEBUG[28730][C000000] logger.c: CALL_ID [C000000] bound to thread.
> [2012-03-2015:01:30] VERBOSE[28730][C000000] app_mixmonitor.c:   == Begin MixMonitor Recording SIP/gold-00000000
> [2012-03-2015:01:30] VERBOSE[28729][C000000] netsock2.c:   == Using SIP RTP CoS mark 5
> [2012-03-2015:01:30] VERBOSE[28729][C000000] app_dial.c:     -- Called SIP/101
> [2012-03-2015:01:30] VERBOSE[28729][C000000] app_dial.c:     -- SIP/101-00000001 is ringing
> [2012-03-2015:01:33] VERBOSE[28729][C000000] app_dial.c:     -- SIP/101-00000001 answered SIP/gold-00000000
> [2012-03-2015:02:17] VERBOSE[28729][C000000] res_musiconhold.c:     -- Started music on hold, class 'default', on SIP/101-00000001
> [2012-03-2015:02:22] VERBOSE[28729][C000000] res_musiconhold.c:     -- Stopped music on hold on SIP/101-00000001
> [2012-03-2015:02:22] DEBUG[28733] logger.c: CALL_ID [C000001] created by thread.
> [2012-03-2015:02:22] DEBUG[28733][C000001] logger.c: CALL_ID [C000001] bound to thread.
> [2012-03-2015:02:22] VERBOSE[28733][C000001] pbx.c:     -- Executing [123*@sipphones:1] Dial("SIP/101-00000001", "SIP/123") in new stack
> [2012-03-2015:02:22] VERBOSE[28733][C000001] netsock2.c:   == Using SIP RTP CoS mark 5
> [2012-03-2015:02:22] VERBOSE[28733][C000001] app_dial.c:     -- Called SIP/123
> [2012-03-2015:02:22] NOTICE[28696] chan_sip.c: Got OK on REFER Notify message
> [2012-03-2015:02:22] VERBOSE[28729][C000000] pbx.c:   == Spawn extension (sipphones, 006, 2) exited non-zero on 'SIP/gold-00000000'
> [2012-03-2015:02:22] VERBOSE[28730][C000000] app_mixmonitor.c:   == MixMonitor close filestream (mixed)
> [2012-03-2015:02:22] VERBOSE[28730][C000000] app_mixmonitor.c:   == End MixMonitor Recording SIP/gold-00000000
> [2012-03-2015:02:22] VERBOSE[28733][C000001] app_dial.c:     -- SIP/123-00000002 is ringing
> [2012-03-2015:02:23] VERBOSE[28733][C000001] app_dial.c:     -- SIP/123-00000002 answered SIP/101-00000001
> [2012-03-2015:02:23] VERBOSE[28733][C000001] rtp_engine.c:     -- Locally bridging SIP/101-00000001 and SIP/123-00000002
> [2012-03-2015:02:35] VERBOSE[28733][C000001] pbx.c:   == Spawn extension (sipphones, 123*, 1) exited non-zero on 'SIP/101-00000001'
> 
> I'm still working on test cases for the other associations I'm performing.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120320/a5b68119/attachment-0001.htm>