export DragonFlyBSD CPU time #310

stuartnelson3 · 2016-09-18T16:54:26Z

Hey,

This reports the node cpu time for machines running DragonFlyBSD. It is analogous to cpu_freebsd.go, but unfortunately DragonFly required a different implementation.

The code here is heavily influenced by the FreeBSD implementation, as well as this:

https://www.dragonflybsd.org/mailarchive/users/2010-04/msg00056.html

I have a separate implementation that uses creates a go slice from a C array of the double values, instead of how this implementation uses a string, if that is preferred: https://github.com/stuartnelson3/node_exporter/blob/dfly-cpu-doubles/collector/cpu_dragonfly.go

Note:

The units seem to be off by perhaps 3 orders of magnitude. I'm waiting for a response from the DragonFly community as to what the units for the various cpu modes.

FIXED: It seems I was using the incorrect value to find cpu time. The value I should use is the systimer_freq. The pr has been updated to use that.

Might not be lined up correctly? Weird output data in the second CPU.

Don't need it since we aren't malloc'ing

Moved to exporting via a string, which is then split and parsed. The string is sometimes duplicated, however.

Duplication was caused by malloc returning a region of memory that already had data in it.

The correct frequency is the systimer frequency, not the stathz. From one of the DragonFly developers: The bump upon each statclock is: ((cur_systimer - prev_systimer) * systimer_freq) >> 32 systimer_freq can be extracted from following sysctl in userspace: sysctl kern.cputimer.freq

This is the code that was lifted from the freebsd implementation, but was not correct.

matthiasr

In general, I think I prefer pulling values out instead of the string conversion (and associated buffer size issues).

I think in general it would be beneficial to do less in C and more in Go; this should make it easier to verify the C parts.

What about tests?

matthiasr · 2016-09-19T14:24:02Z

collector/cpu_dragonfly.go

+
+	// string needs to hold (5*ncpu)(uint64_t + char)
+	// The char is the space between values.
+	int cputime_size = (sizeof(uint64_t)+sizeof(char))*(5*ncpu);


sizeof(uint64_t) is not the size of the decimal expansion you're actually putting into the string.

Any help is welcome, I'm not a C programmer :)

sizeof is the size of the binary representation (8), but (if the number is large) the decimal representation you're putting in this array will be longer. 2^64-1 has 20 digits, so I think if you just put in 5_ncpu_21 it'll be enough?

Additionally, instead of malloc+bzero you can use calloc below – memory returned from calloc is initialized to 0: *cputime = calloc(ncpu, 5*21) should do.

I'm not a C programmer either …

matthiasr · 2016-09-19T14:30:30Z

collector/cpu_dragonfly.go

+	uint64_t user, nice, sys, intr, idle;
+	user = nice = sys = intr = idle = 0;
+	for (int i = 0; i < ncpu; ++i) {
+		user = ((double) cp_t[i].cp_user) / freq;


So this is integer seconds? This conversion/division is very lossy; can we get more granularity out? Either by putting micro- or nanoseconds in the string, or by using floats?

Or read the frequency separately, get cp_t out in to Go land as is, and do the math in Go?

Happy to read out the values separately and do the math in Go land. I would prefer to stick with exporting in a string, as my initial attempts at pulling out the struct and moving it to go didn't end well.

Export cpu mode times as original uint64_t data, and update frequency, and do the conversion to float64 and subsequent division in go.

stuartnelson3 · 2016-09-20T07:19:00Z

@matthiasr I think this latest commit might address both your concerns. The value being written to the string is now uint64_t instead of double, so I think my size calculation will be correct (maybe, still not sure, and google searches only turn up C++ stdlib conversion code), and the division to find cpu time in seconds is being executed with higher fidelity types.

Example scrape output:

# HELP node_cpu Seconds the cpus spent in each mode.
# TYPE node_cpu counter
node_cpu{cpu="cpu0",mode="idle"} 123.2041996957714
node_cpu{cpu="cpu0",mode="interrupt"} 0.05891468049710229
node_cpu{cpu="cpu0",mode="nice"} 0.01800515149271765
node_cpu{cpu="cpu0",mode="sys"} 3.9195816786770385
node_cpu{cpu="cpu0",mode="user"} 6.904898737129998
node_cpu{cpu="cpu1",mode="idle"} 122.07772426383801
node_cpu{cpu="cpu1",mode="interrupt"} 0.034394175796085814
node_cpu{cpu="cpu1",mode="nice"} 0.0070924516942795805
node_cpu{cpu="cpu1",mode="sys"} 4.034886137763319
node_cpu{cpu="cpu1",mode="user"} 7.951272577939375

Where previous values were just integers.

stuartnelson3 · 2016-09-20T07:53:59Z

This seems to produce the correct results when putting my machine under load:

stuartnelson3 · 2016-09-28T07:13:34Z

@matthiasr moved to doing all the math in go-land (exporting values as uint64_t and doing conversion to float64 in go), and I added a test.

I'm really not sure what you're looking for regarding the test. Let me know.

matthiasr

Thank you!

stuartnelson3 · 2016-10-19T16:57:31Z

any chance you wanna merge this? :)

stuartnelson3 · 2016-10-26T08:24:20Z

@fabxc any love?

stuartnelson3 added 11 commits September 10, 2016 19:22

wip

cda2f49

successfully exporting one of 2 cpus

03da1ea

Creating slice from C-array

1b7a18c

Might not be lined up correctly? Weird output data in the second CPU.

checkpoint

1d75b37

Appears to be working? Still need to cleanup after self

a9d27ea

Remove some unnecessary includes

cc5142a

Remove free

4b4385b

Don't need it since we aren't malloc'ing

Correctly exporting values

3e4a154

Moved to exporting via a string, which is then split and parsed. The string is sometimes duplicated, however.

Remove unused comment.

c02dcde

Remember to bzero string

9f7822c

Duplication was caused by malloc returning a region of memory that already had data in it.

Remove unneeded ncpu variable

8cc06aa

stuartnelson3 changed the title ~~Dfly cpu~~ export DragonFlyBSD CPU time Sep 18, 2016

stuartnelson3 added 3 commits September 19, 2016 09:35

Remove old freq finding code

78c84b1

This is the code that was lifted from the freebsd implementation, but was not correct.

Update comment

57f88ac

matthiasr requested changes Sep 19, 2016

View reviewed changes

Maintain granularity in cpu data

e942d7e

Export cpu mode times as original uint64_t data, and update frequency, and do the conversion to float64 and subsequent division in go.

stuartnelson3 added 2 commits September 20, 2016 23:27

Export values as uint64_t

ee37a27

Add test

450fe0f

matthiasr approved these changes Oct 18, 2016

View reviewed changes

matthiasr approved these changes Oct 21, 2016

View reviewed changes

SuperQ approved these changes Nov 17, 2016

View reviewed changes

SuperQ merged commit 7a9aad0 into prometheus:master Nov 17, 2016

SuperQ mentioned this pull request Nov 25, 2016

Release v0.13.0 #336

Merged

discordianfish mentioned this pull request Oct 30, 2018

cpu_dragonfly.go's cpu time doesn't seem to be in second against its name (node_cpu_seconds_total) #1129

Closed

stuartnelson3 deleted the dfly-cpu branch October 30, 2018 14:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

export DragonFlyBSD CPU time #310

export DragonFlyBSD CPU time #310

stuartnelson3 commented Sep 18, 2016 •

edited

Loading

matthiasr left a comment

matthiasr Sep 19, 2016

stuartnelson3 Sep 19, 2016

matthiasr Sep 20, 2016

matthiasr Sep 19, 2016

stuartnelson3 Sep 19, 2016

stuartnelson3 commented Sep 20, 2016

stuartnelson3 commented Sep 20, 2016

stuartnelson3 commented Sep 28, 2016

matthiasr left a comment

stuartnelson3 commented Oct 19, 2016

stuartnelson3 commented Oct 26, 2016

export DragonFlyBSD CPU time #310

export DragonFlyBSD CPU time #310

Conversation

stuartnelson3 commented Sep 18, 2016 • edited Loading

matthiasr left a comment

Choose a reason for hiding this comment

matthiasr Sep 19, 2016

Choose a reason for hiding this comment

stuartnelson3 Sep 19, 2016

Choose a reason for hiding this comment

matthiasr Sep 20, 2016

Choose a reason for hiding this comment

matthiasr Sep 19, 2016

Choose a reason for hiding this comment

stuartnelson3 Sep 19, 2016

Choose a reason for hiding this comment

stuartnelson3 commented Sep 20, 2016

stuartnelson3 commented Sep 20, 2016

stuartnelson3 commented Sep 28, 2016

matthiasr left a comment

Choose a reason for hiding this comment

stuartnelson3 commented Oct 19, 2016

stuartnelson3 commented Oct 26, 2016

stuartnelson3 commented Sep 18, 2016 •

edited

Loading