Move FreeBSD/DragonflyBSD out of meminfo add kvm. by derekmarcotte · Pull Request #547 · prometheus/node_exporter

derekmarcotte · 2017-04-05T20:38:04Z

This gives us memory_swap_used_bytes, and everything under one roof.

SuperQ · 2017-04-12T08:24:52Z

Please rebase against master to pick up the fixes for the test pipeline and fix merge conflicts.

discordianfish · 2017-06-08T15:42:25Z

typos: memory, usage

discordianfish · 2017-06-08T15:44:06Z

Looks good beside the typos. I'd try to avoid CGO where possible but overall this seems reasonable.

mdlayher

Note that I didn't closely review the C portion, but the Go seems okay except for one potential data race that I can see.

mdlayher · 2017-10-06T17:22:58Z

I think you'll also want to guard this boolean with the mutex.

Thanks for the feedback. I could do that. I think my initial reasoning was that at the moment, hasErr is a cached thing anyway. A second call into _kvm_swap_used_pages when hasErr is true doesn't really matter, yet. The _kvm_swap_used_pages function has all the kvm state self contained, and recreates it on each call. Perhaps the correct thing is just to rip it out, as it hasn't been proven to do anything helpful yet anyway - it's just intuition.

mguegan · 2017-10-22T10:09:33Z

Nice, maybe you should see #710 and modify the buffer_bytes part (adding dataType).

mguegan

see #710

mguegan · 2017-10-22T10:12:10Z

Proposal :
dataType: bsdSysctlTypeUint64,

Thanks for the heads up. Will rebase on master when #710 is solved.

discordianfish · 2017-11-02T10:59:40Z

@derekmarcotte #710 is solved. Can you rebase this? Then I think we can finally merge it.

derekmarcotte · 2017-11-02T11:30:50Z

@discordianfish Fantastic, thanks! FWIW, 6f0915f is rebased on master, and includes the changes from #710.

discordianfish

LGTM!

SuperQ

LGTM

SuperQ · 2017-11-02T14:52:25Z

It'd be nice if these had tests, but I don't know of a good way to mock these. In theory we'll be able to do end-to-end testing with buildkite.

derekmarcotte · 2017-11-02T20:06:21Z

@SuperQ: agreed on both counts. I've got a branch on my dev box where I've begun making the end-to-end test multi-platform. Working on a few other things ahead of that for now, but it's definitely on my radar.

discordianfish · 2018-01-04T09:43:30Z

I'd say let's rebase this and get it merged, we can still add tests later.

This gives us SwapUsed, and everything under one roof.

derekmarcotte · 2018-01-04T11:08:11Z

Rebased as of 6be0f84

SuperQ

LGTM

* Move FreeBSD/DragonflyBSD out of meminfo add kvm. This gives us SwapUsed, and everything under one roof. * Fix typos per review. * Update to use newer API. * Remove premature optimization per PR feedback.

Increase the /proc/stat scanner tokens size from the default of 65k to 1024k. This allows for scanning of large `intr` lines with > 65k columns. * Increase the scanner buffer from the default 4kB to 8kB bytes at a time. Fixes: prometheus/procfs#546 Signed-off-by: SuperQ <superq@gmail.com>

derekmarcotte force-pushed the dm-kvm branch from c41048e to b510fa7 Compare April 6, 2017 13:35

derekmarcotte force-pushed the dm-kvm branch 3 times, most recently from 0aab896 to 4909f01 Compare April 17, 2017 20:27

discordianfish self-requested a review April 19, 2017 16:57

derekmarcotte force-pushed the dm-kvm branch from 4909f01 to 60822f9 Compare April 24, 2017 09:52

derekmarcotte force-pushed the dm-kvm branch from 60822f9 to 2d61835 Compare June 6, 2017 14:01

discordianfish reviewed Jun 8, 2017

View reviewed changes

Comment thread collector/memory_bsd.go Outdated

discordianfish Jun 8, 2017 •

edited

Loading

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

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

typos: memory, usage

derekmarcotte force-pushed the dm-kvm branch from 79d692e to f16cf7a Compare June 19, 2017 10:28

derekmarcotte force-pushed the dm-kvm branch from f16cf7a to 3d3271f Compare October 2, 2017 10:28

mdlayher reviewed Oct 6, 2017

View reviewed changes

discordianfish requested a review from SuperQ October 7, 2017 10:11

derekmarcotte force-pushed the dm-kvm branch from 3d3271f to 8aaa955 Compare October 19, 2017 09:55

mguegan suggested changes Oct 22, 2017

View reviewed changes

derekmarcotte force-pushed the dm-kvm branch from 8aaa955 to 6f0915f Compare November 2, 2017 10:12

discordianfish approved these changes Nov 2, 2017

View reviewed changes

SuperQ approved these changes Nov 2, 2017

View reviewed changes

derekmarcotte force-pushed the dm-kvm branch from 6f0915f to 13004c2 Compare November 17, 2017 11:23

derekmarcotte force-pushed the dm-kvm branch from 13004c2 to bb9ac74 Compare January 4, 2018 10:55

derekmarcotte added 3 commits January 4, 2018 06:06

Move FreeBSD/DragonflyBSD out of meminfo add kvm.

305cf85

This gives us SwapUsed, and everything under one roof.

Fix typos per review.

d32a3d5

Update to use newer API.

5b51c52

Remove premature optimization per PR feedback.

6be0f84

derekmarcotte force-pushed the dm-kvm branch from bb9ac74 to 6be0f84 Compare January 4, 2018 11:07

SuperQ approved these changes Jan 4, 2018

View reviewed changes

SuperQ merged commit 477fe46 into prometheus:master Jan 4, 2018

derekmarcotte deleted the dm-kvm branch January 4, 2018 11:33

mguegan mentioned this pull request Jan 4, 2018

Add cpu_openbsd #727

Closed

Uh oh!

Conversation

derekmarcotte commented Apr 5, 2017

Uh oh!

SuperQ commented Apr 12, 2017

Uh oh!

discordianfish Jun 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

discordianfish commented Jun 8, 2017

Uh oh!

mdlayher left a comment

Choose a reason for hiding this comment

Uh oh!

mdlayher Oct 6, 2017

Choose a reason for hiding this comment

Uh oh!

derekmarcotte Oct 6, 2017

Choose a reason for hiding this comment

Uh oh!

mguegan commented Oct 22, 2017

Uh oh!

mguegan left a comment

Choose a reason for hiding this comment

Uh oh!

mguegan Oct 22, 2017

Choose a reason for hiding this comment

Uh oh!

derekmarcotte Oct 23, 2017

Choose a reason for hiding this comment

Uh oh!

discordianfish commented Nov 2, 2017

Uh oh!

derekmarcotte commented Nov 2, 2017

Uh oh!

discordianfish left a comment

Choose a reason for hiding this comment

Uh oh!

SuperQ left a comment

Choose a reason for hiding this comment

Uh oh!

SuperQ commented Nov 2, 2017

Uh oh!

derekmarcotte commented Nov 2, 2017

Uh oh!

discordianfish commented Jan 4, 2018

Uh oh!

derekmarcotte commented Jan 4, 2018

Uh oh!

SuperQ left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

discordianfish Jun 8, 2017 •

edited

Loading