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

Permit comma-separated list of module names in "module" parameter #949

Closed
wants to merge 1 commit into from

Conversation

candlerb
Copy link
Contributor

This is a proof-of-concept patch for discussion and review.

It allows a single scrape to specify a comma-separated list of modules, e.g. module=if_mib,mikrotik. This implements #731, and supports the breaking up of snmp.yml into independent MIB pieces as discussed in #938.

Points to note/discuss (some marked TODO in source):

  • Currently only module=foo,bar is supported. I could add module=foo&module=bar as an additional option; however, I want to keep the former, because target rewriting rules can only specify a single instance of a given parameter (i.e. __param_module in this case)
  • There's no attempt to reject duplicate modules (module=if_mib,if_mib), nor to handle distinct modules which generate overlapping metrics. In those cases, the Prometheus client library itself will barf, with an error like
    * collected metric "sysUpTime" { gauge:{value:7.102146e+08}} was collected before with the same name and label values
    
  • The modules are processed sequentially, in the order given in the request
  • Each module still has a scrape phase and an emit-metrics phase. The scrape times (and packet count / retry count) are summed, to give the same metrics
  • I have not added any additional tests - but none of the existing ones break. I believe that if you invoke this with a single module, it should behave exactly the same as today.
  • I have not updated documentation

@SuperQ
Copy link
Member

SuperQ commented Aug 16, 2023

We're already working on multiple modules in #945.

@candlerb
Copy link
Contributor Author

Closing in favour of #945 (but will leave the branch just in case)

@candlerb candlerb closed this Aug 17, 2023
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 this pull request may close these issues.

2 participants