[Home]

Summary:ASTERISK-06036: [patch] CDR not ended before 'h' extension
Reporter:Russell Bryant (russell)Labels:
Date Opened:2006-01-10 16:33:05.000-0600Date Closed:2008-01-15 17:28:07.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) cdr_before_h.patch
Description:Kenny Shumard asked me on IRC why he could not get values from ${CDR(end)} or ${CDR(billsec)} from the 'h' extension.  Sure enough, the reason is that the CDR is not ended before executing the 'h' extension.

This means that a customer would be billed for all of the time that the 'h' extension is begin executed.

This patch simply ends the CDR before starting to execute the 'h' extension.  If the 'h' extension does not exist, the cdr will be ended shortly after in ast_hangup().

The CDR is still only posted in ast_hangup, because we want the CDR to be ended, but still present in the 'h' extension so that all values can still be read from it.
Comments:By: Russell Bryant (russell) 2006-01-10 16:34:13.000-0600

This is the patch that I discussed with you over IRC.
By: Russell Bryant (russell) 2006-01-10 17:09:32.000-0600

ast_cdr_end will called on the cdr for the second time from ast_hangup.  However, this has no effect if the cdr has already been ended.

We could add an AST_CDR_FLAG_ENDED that could be checked in ast_cdr_end, but it's not going to make much of a difference since we still have to iterate the list of cdr's to check them all, anyway.
By: Olle Johansson (oej) 2006-02-02 01:27:35.000-0600

Any other opinions on this issue? Is this patch ready for commit?

/Bugmarshal oej
By: Olle Johansson (oej) 2006-03-08 02:59:58.000-0600

Reminder... What are we going to do with this issue?
By: Russell Bryant (russell) 2006-03-08 11:12:54.000-0600

I was still waiting on an opinion from someone else.  IMO, it should be ready for commit.
By: Olle Johansson (oej) 2006-03-08 11:16:08.000-0600

Guess we have to form a waiting room on IRC...
By: Olle Johansson (oej) 2006-03-08 11:20:47.000-0600

Included in test-this-branch
By: seb7 (seb7) 2006-03-10 09:25:53.000-0600

I've tested this on asterisk-1.2.5 by manually patching the code using the attached patch from 01-20-06 (patch command failed).

First impressions were great - it's fixed the dst column in the CDRs (I use the h extension to do various things after hang up and I needed a fix for h appearing in the dst column). Then I realised that it now was not executing the h extension at all!

Bit of a flaw... Any ideas?
By: seb7 (seb7) 2006-03-10 11:13:05.000-0600

I should probably have tested the behaviour before upgrading from 1.2.4 to 1.2.5. Removing the patch on asterisk-1.2.5 does not appear to restore h getting executed. I'm just compiling 1.2.4 to retest on this platform, then I'll patch 1.2.4 and see what happens. Looks like it could be a problem with 1.2.5...
By: seb7 (seb7) 2006-03-10 13:06:34.000-0600

I'm a newbie at patching asterisk, and so maybe I'm doing something wrong, but as far as I can work out it is the upgrading from 1.2.5 from 1.2.4 that fixes the CDRs but removes jumping to the h extension and not this patch(!). I've tried patching 1.2.4 and the behaviour doesn't appear to change. And I've tried a fresh 1.2.5 make clean ; make ; make install, and the behaviour changes even without the patch...

I give up for the weekend.
By: Olle Johansson (oej) 2006-03-14 01:18:37.000-0600

Consensus on the mailing list seems to be that this has to be an option, possibly in cdr.conf.
By: Russell Bryant (russell) 2006-03-14 10:50:06.000-0600

I made this an option in cdr.conf and committed it to the trunk in revision 12896
By: Russell Bryant (russell) 2006-03-14 10:50:29.000-0600

This will not be included in the 1.2 branch since it is considered a new feature
By: Digium Subversion (svnbot) 2008-01-15 17:21:34.000-0600

Repository: asterisk
Revision: 12463

U   team/oej/test-this-branch/README.test-this-branch
U   team/oej/test-this-branch/cdr.c
U   team/oej/test-this-branch/channels/chan_sip.c
U   team/oej/test-this-branch/pbx.c

------------------------------------------------------------------------
r12463 | oej | 2008-01-15 17:21:34 -0600 (Tue, 15 Jan 2008) | 3 lines

- Fix for chan_sip
- Issue ASTERISK-6036, End CDR before 'h' extension (russellb)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=12463
By: Digium Subversion (svnbot) 2008-01-15 17:28:07.000-0600

Repository: asterisk
Revision: 12896

U   trunk/cdr.c
U   trunk/configs/cdr.conf.sample
U   trunk/include/asterisk/options.h
U   trunk/pbx.c

------------------------------------------------------------------------
r12896 | russell | 2008-01-15 17:28:06 -0600 (Tue, 15 Jan 2008) | 3 lines

add an option to cdr.conf that enables ending CDRs before executing
the "h" extension as opposed to afterwards (issue ASTERISK-6036)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=12896