prometheus / procfs

procfs provides functions to retrieve system, kernel and process metrics from the pseudo-filesystem proc.
Apache License 2.0
769 stars 319 forks source link

Unify error messages and prepare to adopt wrapping error messages, part 1 #518

Closed conallob closed 1 year ago

conallob commented 1 year ago

Unify error messages by using the errors module. Prepare for wrapping errors once Go v1.20 is the default.

This is part 1 of an implementation for #358

SuperQ commented 1 year ago

This needs a DCO sign-off. You can use git commit -s --amend to add it.

SuperQ commented 1 year ago

Yes, I guess we need to either string some errors, or wait a bit. We typically try and support the currently supported Go versions. So we need to stay compatible with 1.19 for now.

conallob commented 1 year ago

I could adapt this PR to use https://github.com/hashicorp/go-multierror instead and file a new issue to migrate to the errors package once Go v1.20 is the default

SuperQ commented 1 year ago

No, I think just using some %s is fine for now. For cases where there is multi-error, prefer to pass the underlying error, not the new error type.

conallob commented 1 year ago

No, I think just using some %s is fine for now. For cases where there is multi-error, prefer to pass the underlying error, not the new error type.

Done.

I've also filed https://github.com/prometheus/procfs/issues/519 to track replacing these %s with %w again, once Go v1.20 is the default

conallob commented 1 year ago

Opened https://github.com/prometheus/procfs/pull/520 to try and fix up golangci-lint

conallob commented 1 year ago

https://github.com/prometheus/procfs/pull/522 was intended to be a complimentary PR, but I managed to goof the git branches, so all changes are now in this PR

PTAL

conallob commented 1 year ago

Thanks, I have been fixing up entries that didn't give specifics, but I may have missed a few early on

conallob commented 1 year ago

Fixed up all the vague, non specific errors (grep "fmt.Errorf" *.go | grep -e "%s: %.: %w" confirms they're all gone)

I do have 2 commits which aren't DCO signed (should work out how to config that per repository) and I'm not sure how to amend those commits

conallob commented 1 year ago

Recreated as https://github.com/prometheus/procfs/pull/526 , but without all the messed up git history