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

Reply via email to