[asterisk-dev] [Code Review]: Preserve user defined CDR information after a Local channel is masqueraded

[Matt Jordan]

> On June 14, 2012, 5:27 p.m., rmudgett wrote:
> >

After discussing this some more, I think I'm rescinding this "Ship It!".

This behavior is odd - we're preserving some CDR information during a specific instance of a masquerade.  While I'd like to believe this would not introduce any undesired behavior for any users out there, it feels risky.

What's more, this behavior is *bad* for Asterisk 11.

With pre-dial, you can set the CDR information directly on the outbound channel, as opposed to the Local channel.  This patch would actually work against that, as it would blow out any information set on the outbound channel in the pre-dial.  What's more, pre-dial accomplishes the actual use case that this scenario is attempting to resolve: that currently, in this scenario, the only time you can apply information to the outbound channel is on the Local channel - when you really want it on the channel that is going to masquerade into the Local channel.

As it is, this may be a very nice fix for a specific user of Asterisk, but I don't think its something that should go in, at least not so long as we have a proper fix coming in the next major version.

I'm going to go ahead and close this review at this time.


- Matt


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


On June 14, 2012, 3:52 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1985/
> -----------------------------------------------------------
> 
> (Updated June 14, 2012, 3:52 p.m.)
> 
> 
> Review request for Asterisk Developers, Terry Wilson, Tilghman Lesher, and rmudgett.
> 
> 
> Summary
> -------
> 
> This patch preserves CDR information that was set on a Local channel before it is masqueraded into by another channel.  This includes accountcode, peeraccount, userfield, amaflags, and any variables that were set.
> 
> Consider a scenario in which a Local channel bridges two SIP channels (possibly the result of a channel originate Local/foo at default extension bar at default).  This would look something like the following:
> 
> SIP/A <--> Local;1 <> Local;2 <--> SIP/B
> 
> In this scenario, currently, the only opportunity for setting CDR information on the resulting call occurs on the Local channels - both SIP channels are created as the result of outbound calls.  As a result, it is desirable to have the CDR information that was set on the Local channel, i.e., Local;2, persist through the masquerade.  At the same time, most of the CDR information, e.g., caller ID information, channel information, etc., should be swapped with the channel being masqueraded away.
> 
> This patch does this by copying over the information in local_fixup - this is only an improvement that should be done with respect to local channels.  This should prevent impacting other masquerade operations (such as on SIP transfers).
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_local.c 368965 
> 
> Diff: https://reviewboard.asterisk.org/r/1985/diff
> 
> 
> Testing
> -------
> 
> Tested the following scenario:
> 
> exten => 100,1,NoOp()
>      same => n,Set(CDR(userfield)=foo)
>      same => n,Dial(SIP/A)
> 
> exten => 101,1,NoOp()
>      same => n,Set(CDR(userfield)=bar)
>      same => n,Dial(SIP/B)
> 
> Performing "channel originate Local/100 at default extension 101 at default" prior to the patch would produce the following CDR record:
> 
> "","","100","default","","Local/100 at default-5160,2","SIP/A-00000000","Dial","SIP/A","2012-06-11 19:10:40","2012-06-11 19:10:42","2012-06-11 19:10:42",2,0,"ANSWERED","DOCUMENTATION","1339441840.1","foo"
> "","100","101","default","100","SIP/A-00000000","SIP/B-00000001","Dial","SIP/B","2012-06-11 19:10:42","2012-06-11 19:10:43","2012-06-11 19:10:45",3,2,"ANSWERED","DOCUMENTATION","1339441840.2",""
> 
> Performing the same post patch produces the following CDR:
> 
> "","","100","default","","Local/100 at default-5160,2","SIP/A-00000000","Dial","SIP/A","2012-06-11 19:10:40","2012-06-11 19:10:42","2012-06-11 19:10:42",2,0,"ANSWERED","DOCUMENTATION","1339441840.1","foo"
> "","100","101","default","100","SIP/A-00000000","SIP/B-00000001","Dial","SIP/B","2012-06-11 19:10:42","2012-06-11 19:10:43","2012-06-11 19:10:45",3,2,"ANSWERED","DOCUMENTATION","1339441840.2","bar"
> 
> 
> Thanks,
> 
> Matt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120615/9c938723/attachment.htm>