-
Notifications
You must be signed in to change notification settings - Fork 639
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
chore!: adopt log/slog, drop go-kit/log #1249
Conversation
cc: @SuperQ |
a43f4e1
to
13b3318
Compare
@@ -365,7 +364,7 @@ func (c Collector) collect(ch chan<- prometheus.Metric, logger log.Logger, clien | |||
results, err := ScrapeTarget(client, c.target, c.auth, module.Module, logger, c.metrics) | |||
c.metrics.SNMPInflight.Dec() | |||
if err != nil { | |||
level.Info(logger).Log("msg", "Error scraping target", "err", err) | |||
logger.Info("Error scraping target", "err", err) |
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.
There are several instances like this that are logging errors at levels other than error
-- is this intentional?
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.
I will need to check the history, but these probably should be Debug()
or Warn()
.
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.
I went through the history of the Info logging in collector.go
and it seems like these were recent logging level mistakes.
For now let's preserve the existing logging levels and we can revisit them later in a separate PR.
I've merged the prometheus/common update. |
This PR includes: - Go version updates, so that `log/slog` can be used - linter updates to remove configs for `go-kit/log` that are no longer needed, and enable `sloglint` linter - Go dep updates for prometheus/{client_golang,common,exporter-toolkit} libs The bulk of this PR was automated by the following script which is being used to aid in converting the various exporters/projects to use slog: https://gist.github.com/tjhop/49f96fb7ebbe55b12deee0b0312d8434 Builds and passes tests. Signed-off-by: TJ Hoplock <[email protected]>
762f587
to
fc5dbd4
Compare
The generator CI error is unrelated. |
BREAKING CHANGES: This version of the exporter introduces a cleaned up default snmp.yml that moved all ucd-snmp-mib oids into a separate module. If you used one of the following modules: * synology * ddwrt * kemp_loadmaster you will need to change your scrape config to also include the ucd_la_table module as well. See https://github.com/prometheus/snmp_exporter/tree/main?tab=readme-ov-file#multi-module-handling for further instructions. * [CHANGE] generator: Update generator default MIBOPTS #1231 * [CHANGE] adopt log/slog, drop go-kit/log #1249 * [ENHANCEMENT] generator: Improve config error message #1274 * [FEATURE] add ParseDateAndTime type #1234 * [FEATURE] Set UseUnconnectedUDPSocket option if one of the modules has if set #1247 * [FEATURE] add NTPTimeStamp type #1315 * [BUGFIX] fixed dashboard mixins #1319 snmp.yml changes: * cleanup ucd-snmp-mibs #1200 * moved oids from synology,ddwrt and kemp_loadmaster to new module ucd_la_table * Added support for Sophos XG Series #1239 * Added support for HPE #1267 * Added support for powercom #1275 * Added support for Cisco IMC #1293 * Updated mib for apc #1303 * Added support for TPLink DDM #1304 --------- Signed-off-by: Sebastian Schubert <[email protected]> Signed-off-by: Sebastian Schubert <[email protected]> Co-authored-by: Ben Kochie <[email protected]>
This PR includes:
log/slog
can be usedgo-kit/log
that are no longer needed, and enablesloglint
linterThe bulk of this PR was automated by the following script which is being used to aid in converting the various exporters/projects to use slog:
https://gist.github.com/tjhop/49f96fb7ebbe55b12deee0b0312d8434
Builds and passes tests.