segmentio / stats

Go package for abstracting stats collection
https://godoc.org/github.com/segmentio/stats
MIT License
208 stars 32 forks source link

fix procstats #63

Closed achille-roussel closed 7 years ago

achille-roussel commented 7 years ago

There are three main changes in the PR:

Those things are pretty hard to test because we can never predict how much resources a process is using (plus it's very dependent on the environment), here's a trace of the tests ran on OSX:

    proc_test.go:26: { cpu(counter:usage.seconds=46µs, gauge:usage.percent=73.76050285421076) [type=user] }
    proc_test.go:26: { cpu(counter:usage.seconds=16µs, gauge:usage.percent=25.655827079725483) [type=system] }
    proc_test.go:26: { memory(gauge:usage.bytes=2707456, gauge:usage.percent=0.03151893615722656) [type=resident] }
    proc_test.go:26: { memory(gauge:usage.bytes=0) [type=shared] }
    proc_test.go:26: { memory(gauge:usage.bytes=0) [type=text] }
    proc_test.go:26: { memory(gauge:usage.bytes=0) [type=data] }
    proc_test.go:26: { memory.pagefault(counter:count=0) [type=major] }
    proc_test.go:26: { memory.pagefault(counter:count=6) [type=minor] }
    proc_test.go:26: { memory(gauge:available.bytes=8589934592, gauge:total.bytes=2707456) [] }
    proc_test.go:26: { files(gauge:open.count=45, gauge:open.max=7168) [] }
    proc_test.go:26: { threads.switch(counter:count=0) [type=voluntary] }
    proc_test.go:26: { threads.switch(counter:count=0) [type=involuntary] }
    proc_test.go:26: { threads(gauge:count=5) [] }

The values seem to make sense, let me know if anything should be changed!

abraithwaite commented 7 years ago

Why do we care about darwin? Is there a darwin host which needs to read these?

If we don't need to support it, we should just fill the functions with mocks and state that it's not supported.

achille-roussel commented 7 years ago

We don't need it... but it's nice to have, especially when testing programs locally that depend on this package (think something that exposes its metrics to a local prometheus or similar).

Sorry I mistakenly hit merge, feel free to leave more comments and I'll address them.

abraithwaite commented 7 years ago

No biggie. You're right it's nice to have. I just dislike cgo. We can always remove it later if it's too much to bear.

achille-roussel commented 7 years ago

Agree, CGO is only used of OSX tho, the linux version of the code doesn't depend on any C code.