[OpenSIPS-Devel] Accounting enhancement proposal

[Dan Pascu]

On 10 Apr 2019, at 13:31, Liviu Chircu <liviu at opensips.org> wrote:

> Hi Dan,
> 
> A very interesting proposal - too bad it arrived 4 days before the beta release, so the discussion will likely have to wait until 3.1.  Below are my comments to some of your key ideas:
> 
>> In the past, the process was fully automatic. There were module parameters for the acc module that specified which were the extra fields and where to take the values from them when accounting happened. Accounting was triggered by setting a flag and later when accounting was actually generated, it would automatically fetch the extra accounting field values and send the accounting request.
> 
> What do you mean by "fully automatic”?

I mean that the generation of the accounting record didn’t need manual input from the script writer, every time an accounting record was generated. The script writer would only need to specify in a modparam in the script from where the extra accounting values to be taken and the code that created the accounting record would build it by automatically fetching the data from the right avps.

The main idea of my point was that creating the accounting record and filling in extra field happened in the same place at the same time.

>  Because the so-called "where to take the values from" sources were often a bunch of AVPs, which you would inevitably spread all over the script, almost identically to the way $acc_extra() is used now.

There is an important difference. Before I could declare some avps to hold values for extra accounting and calculate those avps when appropriate. But those avps were no exclusively used by accounting. They had internal function to my script and were used for other purposes, so the fact that they were computed all over the script bares no relevance. The mapping between those avps and extra accounting fields was specified only in 1 place and fetching the values for them was automatic.

Now I need to declare the extra fields in modparam (but no association with avps). Then I need to compute the values and then I also need to assign manually to $acc_extra.

My point is mostly about filling in $acc_extra in a single unified place.

> However, with the freedom that AVPs gave you came an additional problem, which the current $acc_extra() system eliminates:
> 
>         +----+----+----+
> LEG #5: | x5 | ?? | ?? |
>         +----+----+----+
> LEG #4: | x4 | y4 | ?? |
>         +----+----+----+
> LEG #3: | x3 | y3 | z3 |
> +----+----+----+
> LEG #2: | x2 | y2 | z2 |
> +----+----+----+
> LEG #1: | x1 | y1 | z1 |
> +----+----+----+
> AVP   AVP  AVP
> #1    #2   #3
> 
> Notice how, in more complex accounting scenarios involving multiple legs (e.g. call forwarding, VoIP-VoIP hairpinned calls, etc.), you had to precisely manage the size of each AVP, taking EXTRA care to NOT mistakenly push too many values into it after both GW #1 and GW #2 failed and the call terminated via GW #3. Desynchronizing the old AVP accounting fields (see diagram above, where AVP #1 has too many values pushed into it, so legs #5 and #4 are not recorded) was a nightmare, leading to missing accounting records and lots of pain.  $acc_extra() fixes this problem, making life easier for script writers, since they don't have to micro-manage AVPs as they had to before.

I’m not advocating to return to the old model. All I’d like is to have a means to fill in $acc_extra in a single place, as late as possible, preferably right before the accounting record is generated.

> 
>> However as it is now, accounting is scheduled for later, while filling in extra values needs to happen on the spot.
> Not true at all.

It is true. When I said on the spot, I meant during the execution of the route. The main point is that it still happens in a different place and time than the actual creation of the accounting record happens.

>  You can fill in all $acc_extra values on BYE processing, provided that you can still extract all data that is required by the record.

I insert records on INVITE and update them on BYE. Your model wouldn’t work for me. Plus it’s not the point.

To give and example. I compute a canonical URI in the script. This is basically the request URI, but if it’s a PSTN number, then it will be normalised to 00<E164>. I need this for routing, but it’s also used for an extra field in accounting.

Now when I call do_accounting() I do not have yet computed this, so I cannot yet fill in $acc_extra() (at least not for this field). But that early is the only place where my routing logic have not yet split. After that I start to compute the destination by doing ENUM lookups, considering diversions, etc. At every step the canonical URI will change, which means I might have to set $acc_extra for it every time I compute a new one, or I need to wait until late when I call t_relay to make sure the final value was calculated and can be assigned to $acc_extra only 1 time. But now I have another problem, because my routing logic has split, I ended up in some route that calls t_relay, so I have to assign $acc_extra there, but I also have to assign it in other routes where I can end up and have their own t_relay.

My main request is about having 1 place to set this, as opposed to setting it before each t_relay or every time after canonical URI is computed.

Another advantage of having this, is that it will avoid unnecessarily setting $acc_extra when it might not be used (consider if I decide accounting is not needed anymore and call drop_accounting. All the $acc_extra I assigned was for nothing). A route like I proposed is only called like a callback when the accounting record is generated. If I called drop_accounting is not called and I avoid wasting time in setting extra values for nothing.

> But I doubt setting $acc_extra() 4 times is as worse as the old "de-synchronized accounting legs due to inconsistent AVP stack sizes" problem.

Not the argument. I’m not debating the benefits of the new model, I’m just asking for a change that would bring back some benefit the old model had that was lost in transition.

>> We create a new route type let’s call it accounting_route (or if possible we reuse the event routes by adding an accounting event). The idea is that this accounting_route (or the event route with the accounting event) is called right before the accounting record is generated. Inside this route we can then manually fill in the values for the extra accounting, right before accounting happens, making sure the values are up to date.
> 
> Making the accounting take place in a singular place at script level sounds like an excellent idea to me.

The we agree.

>  The only problem we need to solve is to make sure script writers have all the data they need at that point.  What if they require some metadata that they can only derive through a custom INVITE header?  The INVITE is well gone now, and they didn't do any special handling for it (e.g. store it as a $dlg_val), so what to do next?

That’s why I submitted this proposal. I do not have the in-depth knowledge of the system as you do, so I’ll leave arguing on the technical details of the implementation to the more knowledgeable people.