Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support config.d #628

Closed
RichiH opened this issue Mar 9, 2021 · 16 comments · Fixed by #970
Closed

Support config.d #628

RichiH opened this issue Mar 9, 2021 · 16 comments · Fixed by #970

Comments

@RichiH
Copy link
Member

RichiH commented Mar 9, 2021

We should allow a directory from which we build the configuration.

Open questions / debate:

  • Allow it by default? Gut is no. Require --config.directory = 'foo'.
  • Allow more than one directory? Multi-flag seems to make sense.
  • What to do if the config can not be merged? Hard fail with useful error message.
@RichiH
Copy link
Member Author

RichiH commented Mar 9, 2021

The logical consequence of this is that --config.file will also become multi-flag.

@RichiH
Copy link
Member Author

RichiH commented Mar 9, 2021

Should we only load snmp_exporter.yml and snmp_exporter_*.yml from that directory by default?

@xkilian
Copy link

xkilian commented Mar 9, 2021

I would restrict it to loading snmp_xxx.yml or snmp.yml. Makes sure no unexpected files are read.
As for the prefix, snmp_ or _snmp_exporter, my preference goes to simply: snmp_xxx as it is shorter, matches current naming and the directory structure should make it clear it is in the context of the snmp_exporter.

I personally, am not a fan of multiple directories to read from. I would rather avoid updating flags often (anti pattern) and it is common to have lots of classes of devices, so it can get unwieldy fast.

@xkilian
Copy link

xkilian commented Mar 9, 2021

Hard fail is the right approach, IMO. The fail logic should be enhanced to give an indication in which module it broke if the current error messages are not clear enough.
I believe that they should be ok. Modules names are unique, so it is easy to find them.

@xkilian
Copy link

xkilian commented Mar 9, 2021

I agree that the dir option should be optional. It is a new feature.
Principle of least surprise.

@RichiH
Copy link
Member Author

RichiH commented Mar 29, 2021

Our current thinking is at https://docs.google.com/document/d/1BK_Gc3ixoWyxr9F5qGC07HEcfDPtb6z96mfqoGyz52Y and I will open a thread on prometheus-developers to make people aware of it, as well.

@SuperQ
Copy link
Member

SuperQ commented Jul 7, 2021

How about this for the flag changes:

  • Add --config.path which supports globbing.
  • Allow --config.path to be used more than once.
  • Keep --config.file for N releases for compatibility.

@RichiH
Copy link
Member Author

RichiH commented Jul 7, 2021

JTMS, this would be in addition to https://docs.google.com/document/d/1BK_Gc3ixoWyxr9F5qGC07HEcfDPtb6z96mfqoGyz52Y/edit ?

I am wondering if we even need to remove --config.file.

What would be the default heuristic of --config.path if no globs after the final / are specified?

  • Load everything
  • Load *.yml
  • Load snmp_exporter*.yml (i.e. also snmp_exporter.yml)
  • Load snmp_exporter_*.yml and snmp_exporter.yml
  • Load *.snmp.yml

@RichiH
Copy link
Member Author

RichiH commented Jul 7, 2021

@SuperQ Implementing https://docs.google.com/document/d/1BK_Gc3ixoWyxr9F5qGC07HEcfDPtb6z96mfqoGyz52Y/edit#heading=h.jxn0lovs8rll first would make the considerations in #628 (comment) largely go away. It could pull in *.yml by default and done.

@baryluk
Copy link

baryluk commented Oct 15, 2021

Allow more than one directory? Multi-flag seems to make sense.

Not really needed. Can achieve the same using directory with symlinks.

@candlerb
Copy link
Contributor

@RichiH:

Should we only load snmp_exporter.yml and snmp_exporter_*.yml from that directory by default?

I think *.yml and *.yaml, so that you could have cisco_ncs.yml, juniper_ex.yml etc, but skip over backup files and other junk.

@baryluk:

[Allow more than one directory?] Not really needed. Can achieve the same using directory with symlinks.

I think more than one directory would be useful particularly if you want to separate directories for modules and auths (#619)

--config.dir=/etc/snmp_exporter/modules.d --config.dir=/etc/snmp_exporter/auths.d
# OR
--config.file='/etc/snmp_exporter/modules.d/*.yml' --config.file='/etc/snmp_exporter/auths.d/*.yml'

FYI, exporter_exporter provides this feature. (Aside: it would be nice for blackbox_exporter too!)

Since in the current design for separate auth has a single YAML file with both "auths" and "modules" keys, it would then be up to the user whether they want to have combined module+auth files, or separate files in the same directory, or separate directories.

@candlerb
Copy link
Contributor

candlerb commented Jul 6, 2023

snmp_exporter 0.23(rc.0) is now out, with top level keys auths and modules, and it would be very helpful to read in multiple files and merge them. Each file could contain either modules, or auths, or both. That would avoid the need for an external YAML-merging tool when you are assembling together from drop-in pieces.

It would need to decided what to do about repeated module keys or auth keys: either a later one overrides a previous one, or an error is raised. I think as long as the ordering is deterministic (e.g. the filenames are sorted) I'd be happy with overriding.

@xkilian
Copy link

xkilian commented Jul 6, 2023

Overriding is the way to go IMO.

SuperQ added a commit that referenced this issue Aug 27, 2023
Support loading multiple configuration files by by allowing
`--config.file` to be repeateable. As well as supporting glob file
matching for `config.d` style setups.
* Conflicting auth or module keys are treated as errors.

Fixes: #628

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Aug 27, 2023
Support loading multiple configuration files by by allowing
`--config.file` to be repeateable. As well as supporting glob file
matching for `config.d` style setups.
* Conflicting auth or module keys are treated as errors.

Fixes: #628

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Aug 27, 2023
Support loading multiple configuration files by by allowing
`--config.file` to be repeateable. As well as supporting glob file
matching for `config.d` style setups.
* Conflicting auth or module keys are treated as errors.

Fixes: #628

Signed-off-by: SuperQ <[email protected]>
@SuperQ
Copy link
Member

SuperQ commented Aug 27, 2023

Ok, I've made a proposed implementation that is pretty simple.

  • Allows --config.file multiple times.
  • Allows glob matching in --config.file, for example --config.file=/etc/snmp_exporter/config.d/snmp*.yml.
  • Rejects conflicts with an error.

I tried to make override work, but ran into annoying issues with map copying. Maybe possible, but rejecting conflicts was an easier first pass at this.

SuperQ added a commit that referenced this issue Aug 28, 2023
* Support loading multiple configuration files

Support loading multiple configuration files by by allowing
`--config.file` to be repeateable. As well as supporting glob file
matching for `config.d` style setups.
* Conflicting auth or module keys are treated as errors.

Fixes: #628
---------

Signed-off-by: SuperQ <[email protected]>
Signed-off-by: Ben Kochie <[email protected]>
Co-authored-by: Richard Hartmann <[email protected]>
@nifqi18
Copy link

nifqi18 commented Aug 28, 2023

but ..

i try to set

● snmp_exporter.service - Prometheus SNMP Exporter Service
Loaded: loaded (/etc/systemd/system/snmp_exporter.service; enabled; vendor preset: enabled)
Active: failed (Result: exit-code) since Tue 2023-08-29 04:06:26 WIB; 2s ago
Process: 1885407 ExecStart=/usr/local/bin/snmp_exporter --config.file=/etc/snmp_exporter/config.d/snmp*.yml (code=exited, status=1/FAILURE)
Main PID: 1885407 (code=exited, status=1/FAILURE)
CPU: 12ms

Agu 29 04:06:26 billingtmd systemd[1]: Started Prometheus SNMP Exporter Service.
Agu 29 04:06:26 billingtmd snmp_exporter[1885407]: ts=2023-08-28T21:06:26.082Z caller=main.go:180 level=info msg="Starting snmp_exporter" version="(version=0.23.0, branch=HEAD, revision=20657d2f486a1ac237238827beed97fa67eb8611)"
Agu 29 04:06:26 billingtmd snmp_exporter[1885407]: ts=2023-08-28T21:06:26.083Z caller=main.go:181 level=info build_context="(go=go1.20.7, platform=linux/amd64, user=root@f2310b8dd92c, date=20230816-09:56:45, tags=netgo)"
Agu 29 04:06:26 billingtmd snmp_exporter[1885407]: ts=2023-08-28T21:06:26.083Z caller=main.go:188 level=error msg="Error parsing config file" err="open /etc/snmp_exporter/config.d/snmp*.yml: no such file or directory"
Agu 29 04:06:26 billingtmd snmp_exporter[1885407]: ts=2023-08-28T21:06:26.083Z caller=main.go:189 level=error msg="Possible old config file, see https://github.com/prometheus/snmp_exporter/blob/main/auth-split-migration.md"
Agu 29 04:06:26 billingtmd systemd[1]: snmp_exporter.service: Main process exited, code=exited, status=1/FAILURE
Agu 29 04:06:26 billingtmd systemd[1]: snmp_exporter.service: Failed with result 'exit-code'.
root@billingtmd:/etc/snmp_exporter# systemctl restart snmp_exporter.service
root@billingtmd:/etc/snmp_exporter# systemctl status snmp_exporter.service
● snmp_exporter.service - Prometheus SNMP Exporter Service
Loaded: loaded (/etc/systemd/system/snmp_exporter.service; enabled; vendor preset: enabled)
Active: failed (Result: exit-code) since Tue 2023-08-29 04:06:56 WIB; 1s ago
Process: 1885443 ExecStart=/usr/local/bin/snmp_exporter --config.file=/etc/snmp_exporter/config.d/snmp*.yml (code=exited, status=1/FAILURE)
Main PID: 1885443 (code=exited, status=1/FAILURE)
CPU: 12ms

Agu 29 04:06:56 billingtmd systemd[1]: Started Prometheus SNMP Exporter Service.
Agu 29 04:06:56 billingtmd snmp_exporter[1885443]: ts=2023-08-28T21:06:56.199Z caller=main.go:180 level=info msg="Starting snmp_exporter" version="(version=0.23.0, branch=HEAD, revision=20657d2f486a1ac237238827beed97fa67eb8611)"
Agu 29 04:06:56 billingtmd snmp_exporter[1885443]: ts=2023-08-28T21:06:56.199Z caller=main.go:181 level=info build_context="(go=go1.20.7, platform=linux/amd64, user=root@f2310b8dd92c, date=20230816-09:56:45, tags=netgo)"
Agu 29 04:06:56 billingtmd snmp_exporter[1885443]: ts=2023-08-28T21:06:56.199Z caller=main.go:188 level=error msg="Error parsing config file" err="open /etc/snmp_exporter/config.d/snmp*.yml: no such file or directory"
Agu 29 04:06:56 billingtmd snmp_exporter[1885443]: ts=2023-08-28T21:06:56.199Z caller=main.go:189 level=error msg="Possible old config file, see https://github.com/prometheus/snmp_exporter/blob/main/auth-split-migration.md"
Agu 29 04:06:56 billingtmd systemd[1]: snmp_exporter.service: Main process exited, code=exited, status=1/FAILURE
Agu 29 04:06:56 billingtmd systemd[1]: snmp_exporter.service: Failed with result 'exit-code'.
root@billingtmd:/etc/snmp_exporter# msg="Error parsing config file" err="open /etc/snmp_exporter/config.d/snmp*.yml: no such file or directory"
^C
root@billingtmd:/etc/snmp_exporter# cd config.d/
root@billingtmd:/etc/snmp_exporter/config.d# ls
snmp.yml

and 1'm still getting error .. how to fix that ?

@RichiH
Copy link
Member Author

RichiH commented Aug 29, 2023

You're running version 0.23.0. You need to compile from source, or wait for the next release.

See your logs:

msg="Starting snmp_exporter" version="(version=0.23.0, branch=HEAD, revision=20657d2f486a1ac237238827beed97fa67eb8611)"

stephan-windischmann-sky pushed a commit to stephan-windischmann-sky/snmp_exporter that referenced this issue Oct 27, 2023
* Support loading multiple configuration files

Support loading multiple configuration files by by allowing
`--config.file` to be repeateable. As well as supporting glob file
matching for `config.d` style setups.
* Conflicting auth or module keys are treated as errors.

Fixes: prometheus#628
---------

Signed-off-by: SuperQ <[email protected]>
Signed-off-by: Ben Kochie <[email protected]>
Co-authored-by: Richard Hartmann <[email protected]>
Signed-off-by: Stephan Windischmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants