-
Notifications
You must be signed in to change notification settings - Fork 257
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
Bump snmp_exporter to 0.27.0. Add support for multi-module handling #2523
Conversation
…y comma separation for `prometheus.exporter.snmp`
💻 Deploy preview deleted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this feature and updating the dependency :)
Do you have a setup to test it? It would be nice to make sure that it works as expected + that the logging still works (it should follow the log level set via the logging block (https://grafana.com/docs/alloy/latest/reference/config-blocks/logging/)). When checking the log level, please ensure that it can also be updated at runtime (change the log level from error to debug, reload the config via "curl localhost:12345/-/reload" and ensure that alloy logs debug lines from the snmp exporter)
We also have integration tests for the snmp exporter: https://github.com/grafana/alloy/tree/main/internal/cmd/integration-tests/tests/snmp |
Tested logging, it works now, including reloading: Here , changed debug to warn:
|
…e= coming from snmp_exporter logs
Also changed "module" in logging to "module_param" to avoid conflicts with module from snmp_exporter logs:
|
Signed-off-by: Vitaly Zhuravlev <[email protected]>
About the test, it's a tricky one but I actually found out that the defaults are not applied because it's using a custom UnmarshalAlloy function. One way to solve this is to call SetToDefault at the start of this function. It might need to also be called in the config converter if the tests fail there too after the fix |
Signed-off-by: Vitaly Zhuravlev <[email protected]>
Signed-off-by: Vitaly Zhuravlev <[email protected]>
Signed-off-by: Vitaly Zhuravlev <[email protected]>
Signed-off-by: Vitaly Zhuravlev <[email protected]>
thank you , fixed. |
PR Description
Bump snmp_exporter to 0.27.0 and adds support for multi-module handling by comma separation:
https://github.com/prometheus/snmp_exporter?tab=readme-ov-file#multi-module-handling
Which issue(s) this PR fixes
Fixes #2438 .
Notes to the Reviewer
I think there should be a better way to define logger that require slog instead of github.com/go-kit/log .(prometheus/snmp_exporter#1249)
PR Checklist
Example output: