My initial response upon seeing the changes to the accounting in 2.x was
disappointment, as it created a lot of extra work for me. As Dan mentioned, the
key here is that I was already storing all of the accounting values in
variables for my own use in the script. So in the old method, I only had to
tell the module once which variables I would be using for this and then
everything was handled automatically. This was very convenient. However, the
argument about multi-leg accounting being more complicated in the old module is
something I had not considered, as we do not use it. So I will have to take
Liviu's word on that. (
We have employed 2 methods to decrease the pain of setting all of these
variables:
* For as many values as possible, we removed the script variable and use only
the acc_extra variable. It is a r/w variable after all. This means that for
these variables we do not have to worry about copying the value from an avp or
dlg_val into acc_extra as it is just always there.
* We use wrapper functions extensively. In fact, we have had to patch OpenSIPS
to increase the number of allowed routes. For Dan's example with t_relay, our
approach would be to wrap the t_relay call in another route, which always sets
the acc_extra before calling t_relay.
As for the idea about adding an event, I think this is something that applies
beyond just accounting. The availability of events in OpenSIPS is quite low;
only a handful of modules have events, usually only one or two, and the core
only exports a couple. I would like to see more triggers exposed via events
across the board, to give script writers more flexibility for handling complex
requirements and scenarios. This is one area where OpenSIPS is very lacking as
compared to Kamailio. Almost every module for Kamailio exposes multiple events.
As for concerns about having data available in the event routes, as long as the
transaction and/or dialog can be loaded in the event route then it is up to the
script writer to insure any necessary data will be persisted until it is
needed, just as it is today.
Another thing I'm not sure I understand is why using one method precludes use
of the other. Would it not be possible to define accounting values in the
modparam that map to variables directly as in the old style, and also define
ones that have no variable mapping and would then be expected to be placed in
acc_extra? Something like this:
modparam("acc", "extra_fields",
"db:did=$dlg_val(did);dir;from;to;from_prefix=$avp(from_prefix")
In this example, "did" and "from_prefix" would be pulled from the specified
variables, while "dir", "from", and "to" would be expected to be in
acc_extra(dir), acc_extra(from), and acc_extra(to) respectively. This would
allow accounting to be much simpler for those who do not have complicated
multi-leg requirements, while providing flexibility to those who do.
Ben Newlin
On 4/10/19, 7:30 AM, "Devel on behalf of Dan Pascu"
<[email protected] on behalf of [email protected]> wrote:
On 10 Apr 2019, at 13:31, Liviu Chircu <[email protected]> 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.
_______________________________________________
Devel mailing list
[email protected]
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
_______________________________________________
Devel mailing list
[email protected]
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
