Hi Ben,
You made me discover that the xhttp_prom_stats modparam doesn't work
the way I thought it would work :)
I was always using all and I didn't check the scenario that you described.
I was under the impression that xhttp_prom_stats is accepting a csv
with all the stats to be collected.
So this calls for a feature request :)
We could call the modparam several times or we could pass a list.
I think it would make sense to keep a specific modparam for each
corresponding RPC command to avoid ambiguities and keep a clean
implementation.
If all is provided for stats, then all the other values should be ignored.
If a group is provided, all explicit statistics that belong to that
group should be ignored.
modparam("xhttp_prom", "stats", "stats.get_statistics sl:")
# this entry should be ignored based on the previous entry
modparam("xhttp_prom", "stats", "stats.get_statistics 1xx_replies")
modparam("xhttp_prom", "stats", "stats.get_statistics core:")
modparam("xhttp_prom", "stats", "pkg.stats")
modparam("xhttp_prom", "stats", "core.uptime")
Based on the above I think it's better (and maybe easier to implement)
to keep a specific modparam for each RPC:
# This is the modparam for stats stats
# kamcmd stats.get_statistics sl:
modparam("xhttp_prom", "stats_stats", "get_statistics sl:")
# kamcmd stats.get_statistics 1xx_replies
modparam("xhttp_prom", "stats_stats", "get_statistics 1xx_replies")
# This is the modparam for pkg stats
# kamcmd pkg.stats
modparam("xhttp_prom", "stats_pkg", "stats")
# This is the modparam for core stats
# kamcmd core.uptime
modparam("xhttp_prom", "stats_core", "uptime")
Moving the RPC module name inside the name of the modapram provides
the first level of internal indexation (inside the xhttp_prom module)
making it easier to implement.
I'm open to suggestions :)
Implementing the logic for dealing with all the statistics provided by
the stats.get_statistics RPC command should not be very complicated,
but it 's not trivial either :)
The xhttp_prom_ prefix was there before and the new
xhttp_prom_pkg_stats modparam was implemented the same way to be
consistent.
The current implementation is described here:
https://www.kamailio.org/docs/modules/devel/modules/xhttp_prom.html
SUMMARY:
We are dealing with two issues here:
1. Refactoring the modparam names
2. Implementing support for a given set of statistics provided by the
'stats.get_statistics' RPC command
Regards,
Ovidiu Sas
On Thu, Apr 4, 2024 at 6:12 PM Ben Kaufman <bkauf...@bcmone.com> wrote:
>
> Is this a variable named "xhttp_prom_stats", or the modparam key
> "xhttp_prom_stats"? There's already two separate parameters, right:
>
> - xhttp_prom_stats - corresponds to the rpc stats.get_statistics, thus it's
> argument corresponds to that RPC call
> - xhttp_prom_pkg_stats - Corresponds to the rpc pkg.stats, takes a Boolean
> argument
>
>
> A few thoughts:
>
> - The leading 'xhttp_prom_' prefix on the modparam key is unnecessary, since
> the first argument to modparam is the module itself. I point this out
> because if the issue is backwards compatibility, then my recommendation would
> be to keep 'xhttp_prom_stats' as is, and introduce a param 'stats' that can
> introduce new syntax in the third parameter. As an idea, the third parameter
> could be literally what would be passed to RPC. Not every possible RPC
> command would need to be implemented (many wouldn't make sense), but this
> would keep the syntax predictable:
> modparam("xhttp_prom", "stats", "corex.uptime"); ## Would
> provide uptime
> modparam("xhttp_prom", "stats", "stats.get_statistics all:") ## Same
> result as modparam("xhttp_prom", "xhttp_prom_stats", "all")
> modparam("xhttp_prom", "stats", "pkg.stats"); ## Same
> result as modparam("xhttp_prom", "xhttp_prom_pkg_stats", "1")
>
> - My current test env is 5.7.3, but it looks like the existing '
> xhttp_prom_stats' parameter can only be declared once, thus there's no syntax
> for selecting multiple specific items. In other words this configuration
> doesn't give counters for the sl module and the core module. It only provides
> the counters from which ever is declared last (core in this case).
>
> modparam("xhttp_prom", "xhttp_prom_stats", "sl:")
> modparam("xhttp_prom", "xhttp_prom_stats", "core:")
>
>
> Rather than changing the existing module parameters, the new "stats"
> parameter could be called multiple times, so:
> modparam("xhttp_prom", "stats", "stats.get_statistics sl:")
> modparam("xhttp_prom", "stats", "stats.get_statistics core:")
> modparam("xhttp_prom", "stats", "pkg.stats")
> modparam("xhttp_prom", "stats", "corex.uptime")
>
> Would get all sl module counters, all core module counters, the uptime and
> the package memory stats for each child process. An advantage here is that
> other counters can be implemented without any real thought to syntax. For
> example, shared memory stats would predicably be added as:
> modparam("xhttp_prom", "stats", "shm.stats")
>
>
>
> -----Original Message-----
> From: Ovidiu Sas via sr-users <sr-users@lists.kamailio.org>
> Sent: Thursday, April 4, 2024 1:40 PM
> To: Henning Westerholt <h...@gilawa.com>
> Cc: Kamailio (SER) - Users Mailing List <sr-users@lists.kamailio.org>; Ovidiu
> Sas <o...@voipembedded.com>
> Subject: [SR-Users] Re: RFC: uptime metric for xhttp_prom module
>
> CAUTION: This email originated from outside the organization. Do not click
> links or open attachments unless you recognize the sender and know the
> content is safe.
>
>
> Hello Henning,
>
> I have nothing against using a single variable, except that it is confusing.
> It is not backward compatible. If you enable all vars, then all new stats
> will be enabled by default.
> In the current stable version, the names of the stats vars and stats groups
> are inherited from the "stats.get_statistics" RPC command.
> That's why we don't have explicit documentation for them in the README file.
> For the new stats we don't have a group and the stats vars have sort of
> arbitrary names.
>
> Having dedicated modparams for all these new stats not covered by the generic
> ones require a simple and clean implementation along with an easy way to
> switch them on and off while keeping backward compatibility.
> If more kamailio admin feel that we should merge them, we could :)
>
> -ovidiu
>
>
> On Thu, Apr 4, 2024 at 1:45 PM Henning Westerholt <h...@gilawa.com> wrote:
> >
> > Hello Ovidiu,
> >
> > nothing against it from my side. I would recommend using the existing
> > variable " xhttp_prom_stats" for activating/deactivation. Just think we
> > should not introduce to many single purpose variables, especially for a
> > value which is always available without any dependencies.
> >
> > Cheers,
> >
> > Henning
> >
> > > -----Original Message-----
> > > From: Ovidiu Sas via sr-users <sr-users@lists.kamailio.org>
> > > Sent: Donnerstag, 4. April 2024 18:54
> > > To: Kamailio (SER) - Users Mailing List
> > > <sr-users@lists.kamailio.org>
> > > Cc: Ovidiu Sas <o...@voipembedded.com>
> > > Subject: [SR-Users] RFC: uptime metric for xhttp_prom module
> > >
> > > Hello all,
> > >
> > > The xhttp_module can export all stats under "stats.get_statistics"
> > > RPC command.
> > > I was thinking of adding an optional "uptime" stat that will return
> > > the kamailio server uptime (like the "core.uptime" RPC command).
> > > This will make it easier to add an uptime panel in grafana.
> > > The new stat would be controlled by a module parameter (by default
> > > disabled).
> > > Something like:
> > > modparam("xhttp_prom", "xhttp_prom_uptime_stat" 1) And this will
> > > generate something like:
> > > kam_uptime 671 1712249054631
> > >
> > > This can be already implemented in the script using the "prom_gauge_set()"
> > > and retrieving the uptime via "jsonrpc_exec()", but it would be
> > > nicer and faster to implement it in the module itself.
> > >
> > > Comments?
> > >
> > > -ovidiu
> > > __________________________________________________________
> > > Kamailio - Users Mailing List - Non Commercial Discussions To
> > > unsubscribe send an email to sr-users-le...@lists.kamailio.org
> > > Important: keep the mailing list in the recipients, do not reply
> > > only to the sender!
> > > Edit mailing list options or unsubscribe:
>
>
>
> --
> VoIP Embedded, Inc.
> http://www.voipembedded.com/
> __________________________________________________________
> Kamailio - Users Mailing List - Non Commercial Discussions To unsubscribe
> send an email to sr-users-le...@lists.kamailio.org
> Important: keep the mailing list in the recipients, do not reply only to the
> sender!
> Edit mailing list options or unsubscribe:
__________________________________________________________
Kamailio - Users Mailing List - Non Commercial Discussions
To unsubscribe send an email to sr-users-le...@lists.kamailio.org
Important: keep the mailing list in the recipients, do not reply only to the
sender!
Edit mailing list options or unsubscribe:
