[asterisk-dev] [Code Review] High Resolution Call Time for Billsec and Duration

[Brad Latus]

> On 2010-01-28 11:28:19, Russell Bryant wrote:
> > The code looks ok for the most part.  I have a couple of minor comments I have posted.
> > 
> > However, my biggest concern here is the "testing done" section of this review request.  It currently says that compile testing only is what has been done so far.  While that is certainly a starting point, I am absolutely not okay with making changes to the CDR system like this without a detailed test plan, including every backend module that was modified.  Breaking CDRs in any way has a very high cost associated with it since it affects billing systems.

I have tested only with cdr_custom. I do not have databases installed on my code machine or odbc setup.

The changes to CDR do NOT affect when a CDR would be logged against any backend, the CDR records should not change at all if the fields are still 'int' based.

We made sure not to alter the underlying main/cdr.c to reduce the above risks.
But I understand your reason for wanting more complete tests of all backends.


> On 2010-01-28 11:28:19, Russell Bryant wrote:
> > /trunk/funcs/func_cdr.c, lines 221-239
> > <https://reviewboard.asterisk.org/r/461/diff/6/?file=7866#file7866line221>
> >
> >     Should checking for "billsec" or "duration" be case sensitive here?  Do you intend for "Billsec" and "Duration" to fail?

Well in main/cdr.c:281 they only compare against 'billsec' and 'duration' so why should I be different?


- Brad


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


On 2010-01-23 15:57:21, Brad Latus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/461/
> -----------------------------------------------------------
> 
> (Updated 2010-01-23 15:57:21)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This allows for the showing of the accurate times for billable time and call duration.
> Adds new flag to CDR function 'f' for the above fields.
> 
> This came about by bug #16559 and I thought i'd take a go at making it happen.
> 
> We can drop a few of the modules for ones we want to depreciate like sql-lite / odbc (in favour of sql-lite3 / adaptive_odbc)
> 
> 
> This addresses bug 16559.
>     https://issues.asterisk.org/view.php?id=16559
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 240716 
>   /trunk/UPGRADE.txt 240716 
>   /trunk/addons/cdr_mysql.c 240716 
>   /trunk/cdr/cdr_adaptive_odbc.c 240716 
>   /trunk/cdr/cdr_odbc.c 240716 
>   /trunk/cdr/cdr_pgsql.c 240716 
>   /trunk/cdr/cdr_sqlite.c 240716 
>   /trunk/cdr/cdr_tds.c 240716 
>   /trunk/configs/cdr_custom.conf.sample 240716 
>   /trunk/configs/cdr_odbc.conf.sample 240716 
>   /trunk/configs/cdr_sqlite3_custom.conf.sample 240716 
>   /trunk/configs/cdr_syslog.conf.sample 240716 
>   /trunk/configs/cdr_tds.conf.sample 240716 
>   /trunk/funcs/func_cdr.c 240716 
> 
> Diff: https://reviewboard.asterisk.org/r/461/diff
> 
> 
> Testing
> -------
> 
> Compile test only.
> 
> 
> Thanks,
> 
> Brad
> 
>