Summary:	ASTERISK-25458: Unable to set CDR variable in h extension or hangup_handler
Reporter:	Ross Beer (rossbeer)	Labels:
Date Opened:	2015-10-09 09:09:36	Date Closed:	2016-09-23 14:17:03
Priority:	Minor	Regression?
Status:	Closed/Complete	Components:	CDR/General
Versions:	13.6.0 13.7.0	Frequency of Occurrence	Constant
Related Issues:	is duplicated by ASTERISK-25789 cdr_adaptive_odbc: Not storing custom fields in all cases
Environment:		Attachments:	( 0) ASTERISK-25458-13.diff
Description:	It is not possible to add CDR variables in the h extension or the hangup_handler. The CDR should only be closed once the hangup block has been processed. This would allow for calls that dial more than one extension (or fail-over to a secondary route) to store the hangup cause etc for each dial attempt which is the same way CDRs are created in Asterisk 13. Matthew Jordan via the dev mailing list commented: "The short answer is that it is working as designed; the long answer is that I think the design is wrong. (As the guy responsible, I feel like I'm only throwing stones at myself.) Keeping a single CDR running after channels have left a bridge was one of those things that was relatively "easy" in 11 and earlier versions, and is a bit more challenging in 13. I think in this case that we probably should revisit the rules around it. I do know this has come up a few times on the mailing lists, but I don't think anyone has made an issue yet. Please do made one, referencing this discussion. Just to summarize what I think should happen: endbeforehexten=no should cause no new CDR to be created in the 'h' extension, and the 'end' time on the CDR should not be set. The CDR should allow manipulation of its properties via the CDR function."
Comments:	By: Asterisk Team (asteriskteam) 2015-10-09 09:09:37.943-0500 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. By: Matt Jordan (mjordan) 2016-01-02 10:20:27.499-0600 I've spent some time digging into this, and while I believe we can fix 90% of what the issue describes, there is one part of the current behaviour that isn't worth changing. When a CDR accounts for a bridged party - that is, when there is both a Party A and a Party B in the CDR - we *have* to finalize the CDR when the bridge is broken. That rule is pretty much impossible to change: the ripple effects of not doing that would be rather difficult to even begin to account. If we take a fairly simple scenario, where Alice calls Bob, that means that when either party leaves the bridge, we lock down the CDR that accounts for Alice's time with Bob. If we have a hangup handler or an 'h' extension, we now have two options: # We don't generate a new CDR. This leaves the CDR that is on Alice in a 'locked' state, but accessible in the hangup logic. # We do generate a new CDR. This allows systems to account for the time that is spent processing things during hangup logic. That behaviour is rightly governed by the {{endbeforehexten}} option. Because most people will not want a new CDR generated, this defaults to {{yes}}. All of that really can't be easily or correctly changed, and I wouldn't want to try. There are two bugs that exist in the code today with respect to this behaviour though: # There are ways in which, even when {{endbeforehexten}} is set, we will not actually set the end time on the CDR. This would happen in our "Alice calls Bob" scenario, as that CDR gets finalized no matter what we want. However, if you merely have Alice executing dialplan, a glitch in the existing checks prevents this from occurring. That's wrong; {{endbeforehexten}} should always set the End time on the CDR if set to {{yes}}. # The real crux of this issue: we can't set properties on a CDR in the {{h}} extension or a hangup handler, particularly if {{endbeforehexten}} is {{yes}}. Most people don't want another CDR that accounts for the time spent in hangup logic; they do, however, want to set some property on the CDR that accounts for the actual call. Those two bugs should be fixed by the patch I've attached to this issue. The patch does the following: # First, we are a bit more aggressive about setting the end time. We previously called a method that inspected the snapshot on the CDR to see if we needed to set the end time; that always failed with {{endbeforehexten}} as we are explicitly not updating the state of the channels on the CDR while it executes in hangup logic. Explicitly setting the end time via {{cdr_object_finalize}} suffices. # Second, we now allow setting properties on a finalized CDR if there are no other CDRs left in the chain. This should preserve the 'locking' of attributes that can occur with ForkCDR, as well as allowing setting multiple CDRs if the Forked CDRs are not finalized when forked. By: Matt Jordan (mjordan) 2016-01-02 10:21:51.448-0600 [~rossbeer]: If you can try out the patch and see if it works well enough, that'd be appreciated. I suspect you'll want to run with {{endbeforehexten=True}}. By: Ross Beer (rossbeer) 2016-01-05 11:14:26.850-0600 I can confirm that variables are now being stored correctly from the hangup handler. The variable 'endbeforehexten' is set to 'no', is this correct as you say it should be set to 'True'? By: Ross Beer (rossbeer) 2016-01-06 11:39:34.619-0600 I have a related question here, if a call fails due to a response code should a hangup_handler be called? At the moment I am unable to set CDR variables via a hangup_handler in these situations. By: Ross Beer (rossbeer) 2016-01-07 04:37:20.964-0600 On further investigation, it looks like on a failed dial with an error response code setting variables after the dial command does not cause any information to be saved in the CDR. The only time data is saved is when the h or hangup_handler is called. Ideally we should be able to set the hangup cause within the CDR for each call attempt, where as at the moment it's only possible to record the last call attempts hangup cause. By: Ross Beer (rossbeer) 2016-01-07 10:34:13.513-0600 Custom CDR variables are not being saved when using the following code: exten => d-NOANSWER,1,ResetCDR(w) exten => d-NOANSWER,n,NoCDR() exten => d-NOANSWER,n,Answer() exten => d-NOANSWER,n,Wait(1) exten => d-NOANSWER,n,Playback(number-not-answering) exten => d-NOANSWER,n,Hangup() Variables are set before dial, however do not appear to be set in the initial CDR record as expected. I've tried ForkCDR(e) and CDR_Prop disable instead of ResetCDR and NoCDR, however with the CDR_PROP a second CDR is still created and the same issue occurs where by no variables are populated into the CDR if they are set before a dial or in the h extension. By: Ross Beer (rossbeer) 2016-01-08 07:55:12.823-0600 I believe there are currently the following issues with CDRs: 1) If there is a negative response from the Dial command, no custom variables are saved even if they are set before the dial. 2) ResetCDR does not reset all fields, namely duration and billsec. Billsec should only be set if the next call is answered as with a normal call flow. The disposition is however correct. 3) Setting custom variables after each dial attempt is not possible. By: David Burrows (davidb) 2016-01-27 05:22:26.924-0600 I've spent some time trying to work around this issue, with no success.  The patch attached to this issue seems to make no difference for me.  I have a very simple way to reproduce the issue: {noformat} [mytrunk-outgoing] exten => _X.,1,DIAL(SIP/mytrunk/${EXTEN}) exten => h,1,Set(CDR(userfield)=TEST) {noformat} So from a registered extension, I dial my mobile phone via mytrunk.  Party A -> mytrunk -> Party B.  Call is Bridged.  Party B hangs up.  With endbeforehexten=yes, I see the following CDR: {noformat} "","1000","04XXXXXXXX","mytrunk-outgoing",""""" <1000>","SIP/1000-00000000","SIP/mytrunk-00000001","Dial","SIP/mytrunk/04XXXXXXXX","2016-01-27 10:44:02","2016-01-27 10:44:10","2016-01-27 10:44:36",34,25,"ANSWERED","DOCUMENTATION","1453891442.0","" {noformat} With endbeforehexten=no, I see the following: {noformat} "","1000","04XXXXXXXX","mytrunk-outgoing",""""" <1000>","SIP/1000-00000002","SIP/mytrunk-00000003","Dial","SIP/mytrunk/04XXXXXXXX","2016-01-27 10:45:56","2016-01-27 10:46:01","2016-01-27 10:46:06",10,4,"ANSWERED","DOCUMENTATION","1453891556.3","" "","1000","h","mytrunk-outgoing",""""" <1000>","SIP/1000-00000002","","Set","CDR(userfield)=TEST","2016-01-27 10:46:06","2016-01-27 10:46:06","2016-01-27 10:46:06",0,0,"ANSWERED","DOCUMENTATION","1453891556.3","TEST" {noformat} What I expect to see is something like the first CDR, except with "TEST" correctly placed in the userfield. It looks like this really is a show stopper for some people who depend on being able to set custom CDR variables in either the h extension or a hangup handler.  I believe the following (now closed) issue is related: ASTERISK-25596 It looks like Pedro has opted to stay with Asterisk 11, rather than follow up with his submission. By: Ross Beer (rossbeer) 2016-06-22 05:11:09.846-0500 The fix that was put in place for this ticket worked its way into a release, however, if a call is rejected due to busy or a 404 the current code will create a new CDR for the 'hangup_handler_push' function. The correct functionality needs to be that even on failed calls the CDR records the variables set in the hangup_handler. Would this be possible to change the logic so that multiple CDRs are not created?