Skip to content

Conversation

SuperQ
Copy link
Contributor

@SuperQ SuperQ commented Oct 20, 2024

This PR includes:

  • linter updates to enable sloglint linter
  • Go dep updates for prometheus/{client_golang,common,exporter-toolkit} libs
  • refactorings to adopt log/slog in favor of go-kit/log

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

This PR includes:
- linter updates to enable `sloglint` linter
- Go dep updates for prometheus/{client_golang,common,exporter-toolkit} libs
- refactorings to adopt log/slog in favor of go-kit/log

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

Signed-off-by: SuperQ <superq@gmail.com>
@SuperQ
Copy link
Contributor Author

SuperQ commented Oct 20, 2024

👓 @tjhop

@SuperQ SuperQ requested a review from dswarbrick October 20, 2024 08:30
Copy link
Member

@dswarbrick dswarbrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall mostly ok, just a few nits here and there. Some of the logging calls that pass an error as a KV should perhaps actually be logger.Error(...) instead of Warn or Info. I think some of the Info logging calls are also actually more like Debug.

I'm not sure how necessary it is to log an error immediately before returning a wrapped error, e.g.

		c.logger.Error("Error polling:", "err", err)
		return fmt.Errorf("error polling: %w", err)

In cases like that, it probably makes more sense that the caller chooses whether to log the returned error.

Lastly, there seems to be some inconsistency between logging errors with the key err and error. It would be good to standardize on one or the other.

@SuperQ
Copy link
Contributor Author

SuperQ commented Oct 21, 2024

Decisions about what to log when, as well as changing a lot of the logging strings, is scope creep. Additional improvements in logging are something that will be done in separate PRs.

Signed-off-by: Ben Kochie <superq@gmail.com>

Co-authored-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
Signed-off-by: Ben Kochie <superq@gmail.com>
@SuperQ SuperQ merged commit 62f3cfb into master Oct 21, 2024
5 checks passed
@SuperQ SuperQ deleted the superq/slog branch October 21, 2024 06:50
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