[Home]

Summary:ASTERISK-16303: [patch] cdr->src variable is not set anymore in destination channels
Reporter:Theo Belder (tbelder)Labels:
Date Opened:2010-06-30 08:07:21Date Closed:2010-09-29 12:08:57
Priority:MinorRegression?No
Status:Closed/CompleteComponents:CDR/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 17569.diff
Description:This bug is introduced by a part in revision 127663 (version 1.4.22).

You can see it by using the cli command: 'show channel SIP/bla-bla' or by requesting the value of ${CDR(src)} after connect using the 'G' option in Dial ().

Comments:By: Theo Belder (tbelder) 2010-06-30 08:09:07

Attached is the patch to fix this problem, as it was removed in revision 127663.



By: Leif Madsen (lmadsen) 2010-06-30 13:38:59

I'd like to know if re-introducing this causes the issue that was resolved by removing it to show back up or whether just "too much" was removed/changed.

By: Leif Madsen (lmadsen) 2010-06-30 13:39:17

Any chance you can weigh in on this?

By: Steve Murphy (murf) 2010-06-30 17:51:54

Hmmm. It's been a quite a while, and I just read the log entry for that version, and it's not one of my ultra-verbose-explain-every-change entries. It was made almost exactly two years ago.

I have some hazy memory of situations where, if I set the CDR src val where it used to be set, it would get reset over and over in other situations, and by then it would be quite wrong. I thought I had it covered by setting the val at a different point in the code, but alas, perhaps later fixes got rid of *that*.. but hey, if the user has a fix, I'd advise the standard set of tests:

1. A calls B, they tallk, one/both hang(s) up. If it makes any difference to a CDR, then both ways.

2. A calls B, A transfers B to C. (blind transfer via feature)

3. A calls B, B transfers A to C.(blind transfer via feature)

and variants of 2 and 3 where dahdi hookflash is used to do a transfer
and then sip phone buttons to do xfers (the xfer is done in the channel driver); make sure to throw in as many chanel drivers as allow xfers.

4. Call file connects A to B.

5. All the variants of Assisted transfers.

6. All the variants of 3-ways during Assisted transfers, with various parties hanging up first. (this results in different CDRs!)

7 Caller dials in, gets connected to Application.

8. Caller dials in, gets connected to AGI

9. All the parking scenarios. (A calls B, A parks B, picks back up; B parks A, picks back up; Same sets, but C picks up instead; No-one picks up, etc)

10. SIP phone forwarding.

And who knows what other common, everyday scenarios are involved....

Check each CDR carefully, and determine whether the mod makes things better, or worse.

If everything says the change is for the better, then commit it!

If not, maybe you have to settle in favor of the "most common cases", whatever that might mean.

And now, here's my real advise: CDR's are fundamentally, seriously flawed, complicated, and undocumented. (OK, seriously under-documented, but due to the complexity, it might be doing people a favor). Every moment you re-arrange the deck furniture is a waste of time. Instead, pull out the CEL stuff, and determine how you will extract what you need from the CEL event streams. Do not store the CDR related data on channels. The only successful billing systems that can handle transfers,  keep track of events outside the channels, if not outside asterisk entirely. I've even heard of AMI being used from another process. Whatever.

By: Matthew Nicholson (mnicholson) 2010-09-15 10:41:29

Can you still reproduce this bug with the latest version or SVN version of asterisk 1.4?



By: Matthew Nicholson (mnicholson) 2010-09-28 15:00:35

Have you had a chance to test this with the SVN version of 1.4?

By: Theo Belder (tbelder) 2010-09-29 04:37:30

I have tested it on SVN-branch-1.4-r289094M.

I can still reproduce it on this branch. We have now running asterisk at 20 customersites with the patch and have no problems encoutered.

By: Digium Subversion (svnbot) 2010-09-29 10:03:29

Repository: asterisk
Revision: 289177

U   branches/1.4/main/channel.c

------------------------------------------------------------------------
r289177 | mnicholson | 2010-09-29 10:03:28 -0500 (Wed, 29 Sep 2010) | 8 lines

Set the caller id on CDRs when it is set on the parent channel.

(closes issue ASTERISK-16303)
Reported by: tbelder
Patches:
     17569.diff uploaded by tbelder (license 618)
Tested by: tbelder

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

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

By: Digium Subversion (svnbot) 2010-09-29 10:04:12

Repository: asterisk
Revision: 289178

_U  branches/1.6.2/
U   branches/1.6.2/main/channel.c

------------------------------------------------------------------------
r289178 | mnicholson | 2010-09-29 10:04:12 -0500 (Wed, 29 Sep 2010) | 15 lines

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

........
 r289177 | mnicholson | 2010-09-29 10:03:27 -0500 (Wed, 29 Sep 2010) | 8 lines
 
 Set the caller id on CDRs when it is set on the parent channel.
 
 (closes issue ASTERISK-16303)
 Reported by: tbelder
 Patches:
       17569.diff uploaded by tbelder (license 618)
 Tested by: tbelder
........

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

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

By: Digium Subversion (svnbot) 2010-09-29 10:04:57

Repository: asterisk
Revision: 289179

_U  branches/1.8/
U   branches/1.8/main/channel.c

------------------------------------------------------------------------
r289179 | mnicholson | 2010-09-29 10:04:57 -0500 (Wed, 29 Sep 2010) | 22 lines

Merged revisions 289178 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

................
 r289178 | mnicholson | 2010-09-29 10:04:11 -0500 (Wed, 29 Sep 2010) | 15 lines
 
 Merged revisions 289177 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r289177 | mnicholson | 2010-09-29 10:03:27 -0500 (Wed, 29 Sep 2010) | 8 lines
   
   Set the caller id on CDRs when it is set on the parent channel.
   
   (closes issue ASTERISK-16303)
   Reported by: tbelder
   Patches:
         17569.diff uploaded by tbelder (license 618)
   Tested by: tbelder
 ........
................

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

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

By: Digium Subversion (svnbot) 2010-09-29 10:07:58

Repository: asterisk
Revision: 289180

_U  trunk/
U   trunk/main/channel.c

------------------------------------------------------------------------
r289180 | mnicholson | 2010-09-29 10:07:57 -0500 (Wed, 29 Sep 2010) | 29 lines

Merged revisions 289179 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r289179 | mnicholson | 2010-09-29 10:04:56 -0500 (Wed, 29 Sep 2010) | 22 lines
 
 Merged revisions 289178 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ................
   r289178 | mnicholson | 2010-09-29 10:04:11 -0500 (Wed, 29 Sep 2010) | 15 lines
   
   Merged revisions 289177 via svnmerge from
   https://origsvn.digium.com/svn/asterisk/branches/1.4
   
   ........
     r289177 | mnicholson | 2010-09-29 10:03:27 -0500 (Wed, 29 Sep 2010) | 8 lines
     
     Set the caller id on CDRs when it is set on the parent channel.
     
     (closes issue ASTERISK-16303)
     Reported by: tbelder
     Patches:
           17569.diff uploaded by tbelder (license 618)
     Tested by: tbelder
   ........
 ................
................

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

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

By: Digium Subversion (svnbot) 2010-09-29 12:08:21

Repository: asterisk
Revision: 289268

U   branches/1.8/main/channel.c

------------------------------------------------------------------------
r289268 | mnicholson | 2010-09-29 12:08:21 -0500 (Wed, 29 Sep 2010) | 5 lines

Update the CDR record when ast_channel_set_caller_event() is called

(related to issue ASTERISK-16303)
Reported by: tbelder

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

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

By: Digium Subversion (svnbot) 2010-09-29 12:08:57

Repository: asterisk
Revision: 289269

_U  trunk/
U   trunk/main/channel.c

------------------------------------------------------------------------
r289269 | mnicholson | 2010-09-29 12:08:56 -0500 (Wed, 29 Sep 2010) | 12 lines

Merged revisions 289268 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
 r289268 | mnicholson | 2010-09-29 12:08:20 -0500 (Wed, 29 Sep 2010) | 5 lines
 
 Update the CDR record when ast_channel_set_caller_event() is called
 
 (related to issue ASTERISK-16303)
 Reported by: tbelder
........

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

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