[asterisk-dev] [Code Review] 3962: CDRs: preserve context/extension when executing a Macro or GoSub

[rmudgett]

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



/branches/12/apps/app_macro.c
<https://reviewboard.asterisk.org/r/3962/#comment23701>

    For safety, this channel lock should be moved up to before line 332 where the code to save old info starts.  Needed to ensure that inconsistent data is not saved.



/branches/12/apps/app_stack.c
<https://reviewboard.asterisk.org/r/3962/#comment23702>

    If you want.  You could do
    new->in_subroutine = in_subroutine ? 1 : 0;
    
    and declare new->in_subroutine as
    unsigned int in_subroutine:1;
    
    This way struct gosub_stack_frame doesn't increase in size.



/branches/12/main/cdr.c
<https://reviewboard.asterisk.org/r/3962/#comment23703>

    The comment seems wrong.


- rmudgett


On Aug. 29, 2014, 9:33 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3962/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2014, 9:33 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24254
>     https://issues.asterisk.org/jira/browse/ASTERISK-24254
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The context/extension in a CDR is generally considered the destination of a call. When looking at a 2-party call CDR, users will typically be presented with the following:
> 
> context    exten      channel     dest_channel app  data
> default    1000       SIP/8675309 SIP/1000     Dial SIP/1000,,20
> 
> However, if the Dial actually takes place in a Macro, the current behaviour in 12 will result in the following CDR:
> 
> context    exten      channel     dest_channel app  data
> macro-dial s          SIP/8675309 SIP/1000     Dial SIP/1000,,20
> 
> The same is true of a GoSub:
> 
> context    exten      channel     dest_channel app  data
> subs       dial_stuff SIP/8675309 SIP/1000     Dial SIP/1000,,20
> 
> This generally makes the context/exten fields less than useful.
> 
> It isn't hard to preserve these values in the CDR state machine; however, we need to have something that informs us when a channel is executing a subroutine. Today, there isn't anything that does this.
> 
> This patch solves this problem by adding a new channel flag, AST_FLAG_SUBROUTINE_EXEC. This flag is set on a channel when it executes a Macro or a GoSub. The CDR engine looks for this value when updating a Party A snapshot; if the flag is present, we don't override the context/exten on the main CDR object. In a funny quirk, executing a hangup handler must *not* abide by this logic, as the endbeforehexten logic assumes that the user wants to see data that occurs in hangup logic, which includes those subroutines. Since those execute outside of a typical Dial operation (and will typically have their own dedicated CDR anyway), this is unlikely to cause any heartburn.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/cdr.c 422438 
>   /branches/12/include/asterisk/channel.h 422438 
>   /branches/12/apps/app_stack.c 422438 
>   /branches/12/apps/app_macro.c 422438 
> 
> Diff: https://reviewboard.asterisk.org/r/3962/diff/
> 
> 
> Testing
> -------
> 
> All existing CDR tests and the hangup handler tests pass.
> 
> In addition, a new test has been added on review 3963. This test performs several Dials within a Macro/GoSub, with at least one nested GoSub/Macro/GoSub wrapping a Dial. The CDR entry is verified to have the expected fields.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140903/9b484de2/attachment.html>