[Home]

Summary:ASTERISK-16094: [patch] [regression] CDRs being written where they were not before after revision 258670
Reporter:Joel Vandal (jvandal)Labels:
Date Opened:2010-05-13 11:26:06Date Closed:2010-05-26 11:00:22
Priority:BlockerRegression?No
Status:Closed/CompleteComponents:CDR/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 17334r258670-queue-cdr-fixes1-unanswered_no-ABANDON.txt
( 1) 17334r258670-queue-cdr-fixes1-unanswered_no-ANSWERED.txt
( 2) 17334r258670-queue-cdr-fixes1-unanswered_no-EXITEMPTY..txt
( 3) 17334r258670-queue-cdr-fixes1-unanswered_no-EXITFULL.txt
( 4) 17334r258670-queue-cdr-fixes1-unanswered_no-EXITUNAVAIL.txt
( 5) 17334r258670-queue-cdr-fixes1-unanswered_no-EXITWITHKEY.txt
( 6) 17334r258670-queue-cdr-fixes1-unanswered_no-EXITWITHTIMEOUT.txt
( 7) 17334r258670-reverted-ABANDONED.txt
( 8) 17334r258670-reverted-ANSWERED.txt
( 9) 17334r258670-reverted-EXITEMPTY.txt
(10) 17334r258670-reverted-EXITFULL.txt
(11) 17334r258670-reverted-EXITUNAVAIL.txt
(12) 17334r258670-reverted-EXITWITHKEY.txt
(13) 17334r258670-reverted-EXITWITHTIMEOUT.txt
(14) 17334r258670-unanswered_no-ABANDON.txt
(15) 17334r258670-unanswered_no-ANSWERED.txt
(16) 17334r258670-unanswered_no-EXITEMPTY.txt
(17) 17334r258670-unanswered_no-EXITFULL.txt
(18) 17334r258670-unanswered_no-EXITUNAVAIL.txt
(19) 17334r258670-unanswered_no-EXITWITHKEY.txt
(20) 17334r258670-unanswered_no-EXITWITHTIMEOUT.txt
(21) 17334r258670-unanswered_yes-ABANDON.txt
(22) 17334r258670-unanswered_yes-ANSWERED.txt
(23) 17334r258670-unanswered_yes-EXITEMPTY.txt
(24) 17334r258670-unanswered_yes-EXITFULL.txt
(25) 17334r258670-unanswered_yes-EXITUNAVAIL.txt
(26) 17334r258670-unanswered_yes-EXITWITHKEY.txt
(27) 17334r258670-unanswered_yes-EXITWITHTIMEOUT.txt
(28) messages_log.txt
(29) queue-cdr-fixes1.diff
Description:The following changes on 1.4 SVN cause lots of problems ...

By sample, I dial a Queue (ACD) and hangup ... I have an ABANDON event on Queue Log but no CDR records.

If I enable 'unanswered=yes' on cdr.conf, now I have CDR ...

------------------------------------------------------------------------
r258670 | mnicholson | 2010-04-22 17:49:07 -0400 (Thu, 22 Apr 2010) | 11 lines

Fix broken CDR behavior.

This change allows a CDR record previously marked with disposition ANSWERED to be set as BUSY or NO ANSWER.

Additionally this change partially reverts r235635 and does not set the AST_CDR_FLAG_ORIGINATED flag on CDRs
generated from ast_call().  To preserve proper CDR behavior, the AST_CDR_FLAG_DIALED flag is now cleared from
all brige CDRs in ast_bridge_call().

(closes issue ASTERISK-15599)
Reported by: VarnishedOtter
Tested by: mnicholson

Comments:By: David Brillert (aragon) 2010-05-13 11:30:08

Same problem here
By: David Brillert (aragon) 2010-05-13 11:45:02

I also think this is a regression
By: David Brillert (aragon) 2010-05-13 13:42:21

developer:
Here is my dilemna:
Unless I write
cdr.conf
unanswered=yes

Then no abandoned event is written to CDR and no caller ID is captured and written to CDR.
If unanswered=yes then still no event is written to CDR for overflow events such as exitfull exitwithtimeout exitempty exitwithkeypress
queue_log captures events OK.  But queue CDR events are completely broken.
I have to downgrade to 1.4.30 to fix.


By: David Brillert (aragon) 2010-05-14 13:53:51

Fresh SVN of r263112 1.4 checkout and reverting r258670 fixes the app_queue CDR issues.


By: Paul Belanger (pabelanger) 2010-05-16 10:08:00

So, is this an issue still?
By: David Brillert (aragon) 2010-05-16 16:54:33

Yes this is a regression in current SVN release.
We must revert previous commit r258670 in order to restore correct behavior to queue cdr logs.  Else all queue CDR reports are missing CLID info and are missing unique ID per call.  Also call disposition is inaccurate with r258670 commit.  Reverting r258670 is only a workaround.
By: David Brillert (aragon) 2010-05-16 17:05:18

uploading a full messages log (messages log.txt) with verbose debug enabled.
Log is from Asterisk 1.4.31 with commit r258670 in tact (not reverted).
There is no CDR info available for the caller abandoned call.
By: Matthew Nicholson (mnicholson) 2010-05-19 11:48:49

Please post some example CDR records with what you are seeing in the CDR file and what you expect to be seeing.  Also please be sure to distinguish between the CDR log and the queue log.
By: Matthew Nicholson (mnicholson) 2010-05-19 11:55:00

Also post some queue log examples as well.
By: Matthew Nicholson (mnicholson) 2010-05-19 12:13:07

Part of this change in behavior a result of the change that allows CDRs previously marked as "ANSWERED" to now be marked as "NO ANSWER" and "BUSY".  When a call is abandoned in the queue, the CDR record for that channel is marked "NO ANSWER".  Before r258670, this was ignored and the disposition incorrectly remained "ANSWERED".

Unfortunately the disposition field in CDRs is not capable of holding all of the information someone using CDRs might want.  In this case, the incoming channel is "ANSWERED" but it's attempts to be bridged with an agent fail, thus causing that part of the call to unanswered.  It is impossible to store that data in the disposition field and neither "ANSWERED" or "NO ANSWER" is the complete correct disposition.  Some users will want to see "ANSWERED"  in that field but others will want to see "NO ANSWER".
By: David Brillert (aragon) 2010-05-20 10:27:10

I ran several tests.
Tested each scenario identically in r258670 with cdr.conf unanswered=yes and with #unanswered=yes
For each scenario a test file with cli, /var/log/asterisk/cdr-custom/MASTER.csv, and queue_log results will be uploaded.
Scenarios:
ANSWERED
ABANDON
EXITUNAVAIL
EXITEMPTY
EXITFULL
EXITWITHKEY
EXITWITHTIMEOUT

I ran the exact same scenarios with r258670 reverted with same logs attached.

During my tests I also discovered a separate issue.
There were no agents logged during EXITEMPTY tests yet any call that does EXITEMPTY does not write EXITEMPTY to queue_log.  EXITUNAVAIL is written instead.  I'll probably open a separate bug report for this issue.  But the results are logged to this ticket regardless since they are part of my tests.
By: David Brillert (aragon) 2010-05-21 10:23:15

mnicholson:  Info is uploaded as you requested.  Do you need anything else?


By: Matthew Nicholson (mnicholson) 2010-05-21 10:48:17

I'll look over this.  And I am assuming what you are expecting to see is in the pre r258670 logs?
By: David Brillert (aragon) 2010-05-21 10:52:08

I expect to see in the reverted logs is correct.
By: Rafael Prado Rocchi (prado) 2010-05-21 11:32:51

The correct disposition behavior should be "ANSWERED", independent if the queue or agent answers or not the call. CDRs should match the telco CDR, specially if you have a 800 or toll-free call service and you will have to pay that answered call.
By: Matthew Nicholson (mnicholson) 2010-05-21 15:50:26

Please test the queue-cdr-fixes1.diff patch that I uploaded and see if it fixes your issue.
By: David Brillert (aragon) 2010-05-21 15:51:58

Thanks for the patch.
Its a long weekend here in Canada so I will test the patch on Tuesday when I get back to work.
By: David Brillert (aragon) 2010-05-21 16:02:00

mnicholson: Quick question?
I should test with #unanswered=yes in cdr.conf?
By: Matthew Nicholson (mnicholson) 2010-05-21 16:18:46

Test with both, but you shouldn't need that to get what you are expecting.
By: David Brillert (aragon) 2010-05-25 08:55:59

mnicholson:
Patch succeeded.
I can't get Asterisk to make on fresh svn checkout Revision 265569

make
Generating embedded module rules ...
  [CC] chan_dahdi.c -> chan_dahdi.o
chan_dahdi.c: In function âparse_buffers_policyâ:
chan_dahdi.c:3472: error: âDAHDI_POLICY_HALF_FULLâ undeclared (first use in this function)
chan_dahdi.c:3472: error: (Each undeclared identifier is reported only once
chan_dahdi.c:3472: error: for each function it appears in.)
make[1]: *** [chan_dahdi.o] Error 1

I'm testing the patch on 1.4.32rc1 which patches, makes, and installs OK.
Should have results in a few hours.
By: Joel Vandal (jvandal) 2010-05-25 08:58:28

Dave, send me a message on MSN before I take plane. The problem is that you haven't installed the "zaptel-devel" package on this build server, do a "yum install zaptel-devel".
By: Joel Vandal (jvandal) 2010-05-25 09:12:29

Dave, maybe open a new ticket related to the chan_dahdi compilation error, but it look that r263292 break Zaptel compatibility ... I've do the following on your server to be able to compile Asterisk :

svn diff -r 263291:263292 channels/chan_dahdi.c > 263292.diff

patch -R -p0 < 263292.diff
By: David Brillert (aragon) 2010-05-25 09:27:44

I opened a new ticket for the make regression r263292 breaks zaptel compatibility.  Ticket ASTERISK-16145
By: Leif Madsen (lmadsen) 2010-05-25 10:19:09

The ticket and regression in ASTERISK-16145 is being acted upon today and should be put back into the latest 1.4 later today I hope.
By: Leif Madsen (lmadsen) 2010-05-25 10:22:17

Patches now ready in issue ASTERISK-16145 for anyone who may be affected by that here.
By: David Brillert (aragon) 2010-05-25 10:24:17

Testing is complete on mnicholson's app_queue patch but I have not yet reviewed the text outputs I have uploaded to this ticket.
By: David Brillert (aragon) 2010-05-25 10:32:03

For the EXITEMPTY events not written to queue_log bug I have opened a separate ticket ASTERISK-16147
By: David Brillert (aragon) 2010-05-25 10:36:12

Since jvandal writes all my cdr code and queue reports backends, I must defer comments to him regarding the uploaded txt logs from my testing.
By: Joel Vandal (jvandal) 2010-05-25 10:43:06

aragon, I have check logs and this look to be working correctly, we now have entries on CDR for Abandoned calls. IMHO this patch can be commited.
By: David Brillert (aragon) 2010-05-25 10:45:48

I agree with jvandal's assessment.
By: Digium Subversion (svnbot) 2010-05-25 11:48:19

Repository: asterisk
Revision: 265610

U   branches/1.4/apps/app_queue.c

------------------------------------------------------------------------
r265610 | mnicholson | 2010-05-25 11:48:19 -0500 (Tue, 25 May 2010) | 8 lines

Don't mark the cdr records of unanswered queue calls with "NOANSWER".  This restores the behavior prior to r258670.

(closes issue ASTERISK-16094)
Reported by: jvandal
Patches:
     queue-cdr-fixes1.diff uploaded by mnicholson (license 96)
Tested by: aragon, jvandal

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

http://svn.digium.com/view/asterisk?view=rev&revision=265610
By: Digium Subversion (svnbot) 2010-05-25 12:00:11

Repository: asterisk
Revision: 265611

_U  trunk/
U   trunk/apps/app_queue.c

------------------------------------------------------------------------
r265611 | mnicholson | 2010-05-25 12:00:10 -0500 (Tue, 25 May 2010) | 15 lines

Merged revisions 265610 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r265610 | mnicholson | 2010-05-25 11:48:19 -0500 (Tue, 25 May 2010) | 8 lines
 
 Don't mark the cdr records of unanswered queue calls with "NOANSWER".  This restores the behavior prior to r258670.
 
 (closes issue ASTERISK-16094)
 Reported by: jvandal
 Patches:
       queue-cdr-fixes1.diff uploaded by mnicholson (license 96)
 Tested by: aragon, jvandal
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=265611
By: Digium Subversion (svnbot) 2010-05-25 12:06:04

Repository: asterisk
Revision: 265612

_U  branches/1.6.2/
U   branches/1.6.2/apps/app_queue.c

------------------------------------------------------------------------
r265612 | mnicholson | 2010-05-25 12:06:04 -0500 (Tue, 25 May 2010) | 22 lines

Merged revisions 265611 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r265611 | mnicholson | 2010-05-25 12:00:11 -0500 (Tue, 25 May 2010) | 15 lines
 
 Merged revisions 265610 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r265610 | mnicholson | 2010-05-25 11:48:19 -0500 (Tue, 25 May 2010) | 8 lines
   
   Don't mark the cdr records of unanswered queue calls with "NOANSWER".  This restores the behavior prior to r258670.
   
   (closes issue ASTERISK-16094)
   Reported by: jvandal
   Patches:
         queue-cdr-fixes1.diff uploaded by mnicholson (license 96)
   Tested by: aragon, jvandal
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=265612
By: Digium Subversion (svnbot) 2010-05-26 10:56:44

Repository: asterisk
Revision: 265891

_U  tags/1.4.32-rc2/
U   tags/1.4.32-rc2/apps/app_queue.c

------------------------------------------------------------------------
r265891 | mnicholson | 2010-05-26 10:56:43 -0500 (Wed, 26 May 2010) | 12 lines

Merged r265610 from 1.4:

Don't mark the cdr records of unanswered queue calls with "NOANSWER".  This rest
ores the behavior prior to r258670.

(closes issue ASTERISK-16094)
Reported by: jvandal
Patches:
     queue-cdr-fixes1.diff uploaded by mnicholson (license 96)
Tested by: aragon, jvandal


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

http://svn.digium.com/view/asterisk?view=rev&revision=265891
By: Digium Subversion (svnbot) 2010-05-26 11:00:20

Repository: asterisk
Revision: 265892

_U  tags/1.6.2.8-rc2/
U   tags/1.6.2.8-rc2/apps/app_queue.c

------------------------------------------------------------------------
r265892 | mnicholson | 2010-05-26 11:00:20 -0500 (Wed, 26 May 2010) | 25 lines

Merged r265612 from 1.6.2:

Merged revisions 265611 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r265611 | mnicholson | 2010-05-25 12:00:11 -0500 (Tue, 25 May 2010) | 15 lines
 
 Merged revisions 265610 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r265610 | mnicholson | 2010-05-25 11:48:19 -0500 (Tue, 25 May 2010) | 8 lines
   
   Don't mark the cdr records of unanswered queue calls with "NOANSWER".  This restores the behavior prior to r258670.
   
   (closes issue ASTERISK-16094)
   Reported by: jvandal
   Patches:
         queue-cdr-fixes1.diff uploaded by mnicholson (license 96)
   Tested by: aragon, jvandal
 ........
................


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

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