Refactor exporter creation with clap v4

[TheElectronWill]

This PR mainly removes code. It's a refactoring of how exporters are created.

Instead of using Arg::new and passing a Vec<clap::Args> around, one can simply define a struct with all the options and let clap do the work for us, thanks to the derive macro. This creates a cleaner separation between the exporters and the magagement of the CLI. This will be useful for #50 and #145.

Command-line arguments handling

Before

declaration:

fn get_options() -> Vec<clap::Arg> {
        let mut options = Vec::new();
        let arg = Arg::new("address")
            .default_value(DEFAULT_IP_ADDRESS)
            .help("Riemann ipv6 or ipv4 address. If mTLS is used then server fqdn must be provided")
            .long("address")
            .short('a')
            .required(false)
            .action(clap::ArgAction::Set);
        options.push(arg);

        let arg = Arg::new("port")
            .default_value(DEFAULT_PORT)
            .help("Riemann TCP port number")
            .long("port")
            .short('p')
            .required(false)
            .value_parser(value_parser!(u16))
            .action(clap::ArgAction::Set);
        options.push(arg);
    // and so on...
}

usage:

    let address = parameters.get_one::<String>("address").unwrap().to_string();
    let port: u16 = *parameters
        .get_one("port")
        .expect("Fail parsing port number");

After

declaration:

#[derive(clap::Args, Debug)]
struct ExporterArgs {
    /// Address of the Riemann server. If mTLS is used this must be the server's FQDN.
    #[arg(short, long, default_value = DEFAULT_IP_ADDRESS)]
    pub address: String,

    /// TCP port number of the Riemann server
    #[arg(short, long, default_value_t = DEFAULT_PORT)]
    pub port: u16,

    // and so on...
}

usage:

let address = args.address; // simpler :)
let port = args.port; // the parsing is done by clap, it's already a u16

The documentation of the ExporterArgs structs is both for clap and for us. It shows up nicely in the IDE.
It's also easier to use this way.

Exporter initialization from CLI

Before:

if let Some(stdout_exporter_parameters) = matches.subcommand_matches("stdout") {
    if header {
        scaphandre_header("stdout");
    }
    exporter_parameters = stdout_exporter_parameters.clone();
    let mut exporter = StdoutExporter::new(sensor_boxed);
    exporter.run(exporter_parameters);
} else if ...
// other conditions with duplicated code

After:

let exporter = match cli.exporter {
    ExporterChoice::Stdout(args) => StdoutExporter::new(sensor, args)
    // ...
}
exporter.run()

The code is now deduplicated, and it's easier to add a new exporter or sensor to Scaphandre.

Exporter initialization without CLI (new)

The exporters can now be created without a command-line interface, by creating the ExporterArgs struct.
The new function get_default_sensor allows to get the RAPL sensor on Linux, with default params, and the MSR sensor on Windows.

use scaphandre::get_default_sensor;
use exporters::json::{ExporterArgs, JsonExporter};

let sensor = get_default_sensor();
let args = ExporterArgs { timeout: 1, step: 1, /* other fields omitted */ };
let mut exporter = JsonExporter::new(&sensor, args);

All exporters are used in the same way (there were some disparities before):

1. Create the exporter with ::new(sensor, args)
2. Run it with .run()

This makes good progress towards #50.

Summary of the advantages

• Good progress towards Improve scaphandre to be usable as a library/crate #50.
• Easier to add a new exporter or sensor (even for someone new to clap or new to Scaphandre)
• Less code, better checks and better error messages, such as:

❯ scaphandre riemann --mtls
error: the following required arguments were not provided:
  --address <ADDRESS>
  --ca <CA_FILE>
  --cert <CERT_FILE>
  --key <KEY_FILE>

Usage: scaphandre riemann --address <ADDRESS> --mtls --ca <CA_FILE> --cert <CERT_FILE> --key <KEY_FILE>

For more information, try '--help'.

without any code in Scaphandre doing the check!

• More errors are now catched when the exporter is created by new() instead of when it runs.

Bonus:

• Some code have been moved from the exporters' loops to their creation (new). For example, we don't need to recreate a warp10 Client on every iteration I think (correct me if I'm wrong). It's now created in new.
• The arguments' doc have been improved

TODO

I'd like to add more integration tests before marking this as ready. done :)

[TheElectronWill]

I've added a little test and fixed a bug thanks to it. This PR is ready!

[TheElectronWill]

Note that using the derive API of clap still allows us to add new commands and new arguments dynamically (for instance for plugins), because the builder and derive APIs can be mixed.

[TheElectronWill]

rebased on dev :)

[TheElectronWill]

The remaining failure is caused by --no-default-features --features "prometheus json riemann" in windows builds. Does the warpten exporter only work on Windows?

[bpetit]

It does work since recently. Compiling only a few features should work as well.

Having a look at the tests now.

[TheElectronWill]

Compiling works but my tests require the warpten exporter :)

edit: I've modified the tests to depend on the enabled features, it should be OK

[bpetit]

Current error is this job https://github.com/hubblo-org/ansible-scaphandre/blob/master/prometheus_in_docker_test.yml

--port and --suffix seems to have become mandatory now, from the scaphandre container that failed:

error: the following required arguments were not provided:
--port
--suffix

Usage: scaphandre prometheus --port --suffix --containers

For more information, try '--help'.

I guess there is something to add to the prometheus ExporterArgs struct to make them non mandatory and add a default ?

[bpetit]

Thanks for this work.
That's a huge PR though, as it touches almost all the codebase.
We'll have to write some more tests for the different exporters and validate them prior merging.

[TheElectronWill]

all green 🎉 I guess we just have to squash the last commits with an interactive reset

[bpetit]

warp10 exporter code seems to suffered a bit from the refacto, working on it

[TheElectronWill]

😢 all those commits 😅