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.

Sign up for GitHub

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

Jump to bottom

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

Closed

conallob wants to merge 43 commits into prometheus:master from conallob:master

Contributor

conallob commented May 11, 2023 •

edited

Loading

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

Member

SuperQ commented May 11, 2023

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

dependabot bot and others added 16 commits

May 11, 2023 22:54


          Bump golang.org/x/sys from 0.6.0 to 0.7.0

b98f405

Bumps [golang.org/x/sys](https://github.com/golang/sys) from 0.6.0 to 0.7.0.
- [Release notes](https://github.com/golang/sys/releases)
- [Commits](golang/sys@v0.6.0...v0.7.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sys
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: conall <conall@conall.net>


          Add errors module, complete with 3 defined errors, ReadError, ParseError

b29873f

and MountError

Signed-off-by: conall <conall@conall.net>


          - Rename errors to be consistent with https://pkg.go.dev/io/fs#pkg-va…

5dce46b

…riables

- Include original err whenever raising an exception

Signed-off-by: conall <conall@conall.net>


          static analyzer issue fix

e5ce16f

issue was found by Svace static analyzer (https://www.ispras.ru/en/technologies/svace/)

Signed-off-by: Artem Seleznev <seleznyov.artyom@gmail.com>
Signed-off-by: conall <conall@conall.net>


          net class: split out attribute parser and export it

b8cf502

Can be used by other projects (eg node_exporter) to read a single
sysfs file rather than all of them at once, which can cause
lock contention in the kernel as many sysfs net files grab the
RTNL lock.

Signed-off-by: Dan Williams <dcbw@redhat.com>
Signed-off-by: conall <conall@conall.net>


          Implement mdraid sysfs parsing

56b66c5

Modernised method of fetching mdraid statistics via machine-readable
sysfs entries, instead of parsing human-readable /proc/mdstat.

Signed-off-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
Signed-off-by: conall <conall@conall.net>


          add avgRTT to nfs mountstats

e8fb89f

Signed-off-by: shankeerthan-kasilingam <shankeerthan1995@gmail.com>
Signed-off-by: conall <conall@conall.net>


          Add missing ErrFileParse and ErrFileRead vars

712525c

Signed-off-by: conall <conall@conall.net>


          Update fmt.Errorf() entries to use new errors in arp.go, buddyinfo.go…

3cb7d19

… and cpuinfo.go

Signed-off-by: conall <conall@conall.net>


          Update fmt.Errorf() entries to use new errors in crypto.go and

afe0fcc

fscache.go

Signed-off-by: conall <conall@conall.net>


          Update fmt.Errorf() entries to use new errors in ipvs.go and loadavg.go

aca9d5c

Signed-off-by: conall <conall@conall.net>


          Update fmt.Errorf() entries to use new errors in mdstat.go

bc9fdf2

Signed-off-by: conall <conall@conall.net>


          Refactor netstat parsing

d188865

Move the file hadling into the parseNetstat() function to only open one
file at a time. This eliminates keeping all files open until all are
read.

Signed-off-by: SuperQ <superq@gmail.com>
Signed-off-by: conall <conall@conall.net>


          Merge branch 'prometheus:master' into master

1642c3d


          Update fmt.Errorf() entries to use new errors in meminfo.go and

03ca4bc

mountstats.go

Signed-off-by: conall <conall@conall.net>


          Merge branch 'master' of github.com:conallob/procfs

ec21f09

conallob marked this pull request as ready for review

May 12, 2023 15:45

conallob added 8 commits

May 12, 2023 16:45


          Fix lint issues which were previously missed

262595f

Signed-off-by: conall <conall@conall.net>


          Fix lint issues in ipvs.go

f40b2fb

Signed-off-by: conall <conall@conall.net>


          Try and fix the test errors when there is no err to wrap

901ccae

Signed-off-by: conall <conall@conall.net>


          Update fmt.Errorf() entries to use new errors in net_conntrackstat.go

27514b1

and net_ip_socket.go

Signed-off-by: conall <conall@conall.net>


          Fix up minor errors inclduing phantom references to object d and remove

3e30710

fmt.Errorf() atoms when there is no err to wrap

Signed-off-by: conall <conall@conall.net>


          Fix up minor errors inclduing phantom references to object d

7a74194

Signed-off-by: conall <conall@conall.net>


          Add an unused err, so we have a better log message

e7c7efc

Signed-off-by: conall <conall@conall.net>


          Fix up loose ends, as flagged by test coverage errors on CircleCI

b96d59c

Signed-off-by: conall <conall@conall.net>

SuperQ reviewed

View reviewed changes

loadavg.go Outdated Show resolved Hide resolved

SuperQ reviewed

View reviewed changes

mdstat.go Outdated Show resolved Hide resolved

conallob added 2 commits

May 13, 2023 00:24


          Clean up loose ends flagged from make tests

9bab741

Signed-off-by: conall <conall@conall.net>


          Fix fmt.Errorf() atoms and arguments to squash test failures

64f268e

Signed-off-by: conall <conall@conall.net>

conallob added 2 commits

May 15, 2023 13:30


          Don't use multiple %w atoms in fmt.Errorf(), error wrapping via

15e0bde

fmt.Errorf() is only available in Go v1.20+ https://go.dev/doc/go1.20#errors

Signed-off-by: conall <conall@conall.net>


          Fix up buddyinfo_test.go, which is testing error string values

ebb83e8

Signed-off-by: conall <conall@conall.net>

Contributor Author

conallob commented May 15, 2023

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 #519 to track replacing these %s with %w again, once Go v1.20 is the default

SuperQ reviewed

View reviewed changes

fscache.go Outdated Show resolved Hide resolved


          Update fscache.go

68d44c9

Co-authored-by: Ben Kochie <superq@gmail.com>
Signed-off-by: Conall O'Brien <conall@conall.net>

SuperQ approved these changes

View reviewed changes

SuperQ requested a review from discordianfish

May 16, 2023 08:57

Contributor Author

conallob commented May 16, 2023

Opened #520 to try and fix up golangci-lint

conallob added 3 commits

May 16, 2023 10:56


          Merge branch 'prometheus:master' into master

635f2e4


          Clean up erroneous development artefact

0a4b436

Signed-off-by: Conall O'Brien <conall@conall.net>


          Remove erroneous development artefact

ccbfaa8

Signed-off-by: Conall O'Brien <conall@conall.net>

conallob changed the title ~~Unify error messages and adopt wrapping error messages, part 1~~ Unify error messages and prepare to adopt wrapping error messages, part 1


          Update remainder of modules to use the new ErrFileRead, ErrFileParse …

9a1c6c1

…errors

Signed-off-by: Conall O'Brien <conall@conall.net>

Contributor Author

conallob commented May 16, 2023

#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 requested a review from SuperQ

May 16, 2023 16:00

SuperQ requested changes

View reviewed changes

Member

SuperQ left a comment

There are still a lot of errors that drop the original context of what part of parsing failed. Please make sure that we keep the original details of the parsing error intact.

I've highlighted a number of them.

arp.go Outdated Show resolved Hide resolved

buddyinfo.go Outdated Show resolved Hide resolved

buddyinfo.go Outdated Show resolved Hide resolved

crypto.go Outdated Show resolved Hide resolved

fscache.go Show resolved Hide resolved

loadavg.go Outdated Show resolved Hide resolved

mdstat.go Outdated Show resolved Hide resolved

Contributor Author

conallob commented May 18, 2023

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

Contributor Author

conallob commented May 19, 2023

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

discordianfish mentioned this pull request

internal/util: Read(U)intFromFile: Return file name in errors #326

Closed

SuperQ and others added 9 commits

May 23, 2023 11:32


          Update Go version (#521)

38c890f

* Update Go to 1.20.
* Update minimum Go to 1.19.
* Update Go modules.
* Fix deprecated os/ioutil use.

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


          Add support CpusAllowedList for /proc/[pid]/status (#490)

dea253e

* add CpusAllowedList for /proc/[pid]/status

Signed-off-by: mn.albeschenko <albeschenko.work@gmail.com>

* Fix documentation comment

Co-authored-by: Ben Kochie <superq@gmail.com>
Signed-off-by: Johannes 'fish' Ziemke <github@5pi.de>

---------

Signed-off-by: mn.albeschenko <albeschenko.work@gmail.com>
Signed-off-by: Johannes 'fish' Ziemke <github@5pi.de>
Co-authored-by: Johannes 'fish' Ziemke <github@5pi.de>
Co-authored-by: Ben Kochie <superq@gmail.com>


          net_class: catch syscall.EINVAL and better handle read errors

ccf5873

#483 introduced a bug
that terminates attribute reading when the returned error is
syscall.Errno, which is what util.SysReadFile() will typically
return. Handle that case specifically.

While doing that pull the error checking into
ParseNetClassAttribute() for clarity and to allow external
callers to handle errors correctly.

Signed-off-by: Dan Williams <dcbw@redhat.com>


          Reinstate context per error message, erroneously during early develop…

72f868a

…ment

Signed-off-by: Conall O'Brien <conall@conall.net>


          Fix typo

dc01f70

Signed-off-by: Conall O'Brien <conall@conall.net>


          Revert "Fix typo"

dac514c

This reverts commit 5a7050c.

Signed-off-by: Conall O'Brien <conall@conall.net>


          Fix typo

dbb7b7d

Signed-off-by: Conall O'Brien <conall@conall.net>


          fs: Statfs_t{} doesn't have a Type field on NetBSD, OpenBSD, or Solar…

d39c95e

…is (#523)

Other projects that import procfs, like node_exporter, apparently build
for OSs that don't have Statfs_t.Type.

Signed-off-by: Dan Williams <dcbw@redhat.com>


          Fix softnet CPU Index (#517)

cb080cf

When parsing older kernel versoins, we assume the CPU is indexed based
on the parsed line, rather than an explicit index column.

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

conallob closed this by deleting the head repository

Contributor Author

conallob commented May 23, 2023

Recreated as #526 , but without all the messed up git history

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet