ricoberger / script_exporter

Prometheus exporter to execute scripts and collect metrics from the output or the exit status.
MIT License
354 stars 82 forks source link

Define script parameters in config as array #55

Closed Nick-Triller closed 2 years ago

Nick-Triller commented 2 years ago

Hi there!

Currently, the script string is split on spaces to generate the program name and any fixed arguments. This approach means that the program name and fixed arguments can't contain spaces.

I propose that fixed arguments in the exporter config should be defined as an array instead of being defined in the script property, e.g. something like this:

Example 1:

scripts:
  - name: "example1"
    command: "./examples/connectivity-check.sh"
    args:
    - "google.com"
    - "bing.com"

Example 2:

scripts:
  - name: "example2"
    command: "netcat"
    args:
    - "-vzw"
    - "2"
    - "example.com"

The change can be implemented in a backwards compatible way:

  1. Keep script property with current behaviour.
  2. Require either script or command to be defined.
  3. Error if script is combined with args or command. args is always optional.

This is also how arguments are passed to a process in a Kubernetes pod manifest which means it will feel natural to many users of script_exporter:

apiVersion: v1
kind: Pod
metadata:
  name: command-demo
spec:
  containers:
  - name: command-demo-container
    image: debian
    command:
    - "printenv"
    args:
    - "HOSTNAME"
    - "KUBERNETES_PORT"

@ricoberger let me know what you think, I will be happy to implement this change.

ricoberger commented 2 years ago

Hi @Nick-Triller, this sounds like a useful change 🙂.

If you create a PR, please keep the script field for now as you already suggested. But maybe we can mark is as deprecated in the readme and remove it with the next major version bump.