git sha of the commit: Based on the patch header
Author: Gaetan Rivet <[email protected]>
One line subject: ovs-exporter: Introduce an HTTP metrics server.
This commit introduces a new HTTP metrics exporter service for Open
vSwitch that exports metrics via an HTTP server using Prometheus client
library. It includes configuration files, systemd service definitions,
and integration with ovs-ctl.
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 1bfaa16e4f..17f81127d6 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -162,6 +162,7 @@ RST_MANPAGES = \
> ovs-actions.7.rst \
> ovs-appctl.8.rst \
> ovs-ctl.8.rst \
> + ovs-exporter.8.rst \
> ovs-flowviz.8.rst \
> ovs-l3ping.8.rst \
> ovs-metrics.8.rst \
[ ... ]
> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> index c65c76812e..48a52b2ee4 100644
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -259,6 +259,16 @@ start_ovs_ipsec () {
> return 0
> }
>
> +start_ovs_exporter () {
> + if daemon_is_running ovs-exporter; then
> + log_success_msg "ovs-exporter is already running"
> + else
> + set ovs-exporter --unixctl
> + set "$@" -vconsole:emer -vsyslog:err -vfile:info
> + start_daemon "-10" "" "" "$@" || return 1
> + fi
> +}
The start_ovs_exporter function is missing a return statement. Will this
function always return success even when the daemon fails to start?
> diff --git a/utilities/ovs-exporter.in b/utilities/ovs-exporter.in
> new file mode 100755
> index 0000000000..7ff2784430
> --- /dev/null
> +++ b/utilities/ovs-exporter.in
> @@ -0,0 +1,216 @@
[ ... ]
> +def push_config_into_args(config, args):
> + if not 'exporter' in config:
> + return
> +
> + exporter = config['exporter']
> +
> + if args.port is None and 'port' in exporter:
> + try:
> + args.port = uint(exporter['port'])
> + except Exception as e:
> + vlog.err("Failed to parse port: %s" % str(e))
Is this function missing error handling for the port parsing failure?
The function logs the error but continues execution, potentially leaving
args.port unset.
> + try:
> + config.read(args.config)
> + except:
> + sys.stderr.write("Unable to find configuration file %s: %s" %
> args.config)
The exception handling here appears incomplete - does this code properly
format the error message? The format string expects two arguments but
args.config provides only one. Also, should this use a bare except
clause without specifying the exception type?
> + if server is not None:
> + vlog.info("Entering run loop.")
> + poller = ovs.poller.Poller()
> + while not exiting:
> + server.run()
> + server.wait(poller)
> + if exiting:
> + poller.immediate_wake()
> + poller.block()
> + server.close()
> + else:
> + while True and not exiting:
> + time.sleep(1)
Does the condition "while True and not exiting" work as expected? Should
this be "while not exiting" instead, since "True and not exiting"
evaluates to "not exiting" anyway?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev