Skip to content
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.

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

Dragonfly devstat #323

Merged
merged 3 commits into from
Nov 17, 2016
Merged

Dragonfly devstat #323

merged 3 commits into from
Nov 17, 2016

Conversation

stuartnelson3
Copy link
Contributor

Similar to the FreeBSD implementation, this exports data available from libdevstat. It's unfortunately less granular, but potentially still helpful.

https://www.dragonflybsd.org/cgi/web-man?command=devstat&section=3

Question:
A few of the values returned from compute_stats are gauges whose values I believe could be computed with the counter values already exported. Would it be preferred to drop these rates exported by Dragonfly in favor of calculating them with Prometheus?

@matthiasr

device := fmt.Sprintf("%s%d", C.GoString(&stats.device[0]), stats.unit)

c.bytes_total.With(prometheus.Labels{"device": device}).Set(float64(stats.bytes))
c.transfers_total.With(prometheus.Labels{"device": device}).Set(float64(stats.transfers))
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use ConstMetrics here

prometheus.GaugeOpts{
Namespace: Namespace,
Subsystem: devstatSubsystem,
Name: "blocks_per_second",
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to removing all the unnecessary gauges

@stuartnelson3 stuartnelson3 force-pushed the dfly-devstat branch 2 times, most recently from 292e53f to 189128e Compare October 19, 2016 16:59
@stuartnelson3
Copy link
Contributor Author

@sdurrheimer any idea what's going on with the failures?

# linux-ppc64

>> building binaries

 >   node_exporter

# runtime/cgo

In file included from $WORK/runtime/cgo/_obj/_cgo_export.c:3:0:

/tmp/go-build724044605/runtime/cgo/_obj/_cgo_export.h:37:14: error: size of array '_check_for_64_bit_pointer_matching_GoInt' is negative

 typedef char _check_for_64_bit_pointer_matching_GoInt[sizeof(void*)==64/8 ? 1:-1];

I'm compiling and running this locally so that seems to work, I'm not sure what's going on with this powerpc stuff?

@sdurrheimer
Copy link
Contributor

sdurrheimer commented Oct 20, 2016

@stuartnelson3 Good question, this is pretty low level for me to really help, but the reason is maybe that all platforms doesn't support all GoInt size.

For example, in the uname_linux.go collector is associated to 2 other files uname_linux_int8.go and uname_linux_uint8.go depending on the built platform.

Maybe your new dragonfly collector is built during the powerpc crossbuild, which, it seems, doesn't support full size Int ?

Shouldn't this collector only be taken when building on/for dragonfly ?

@stuartnelson3
Copy link
Contributor Author

Yeah exactly, based on the file name devstat_dragonfly.go I thought it would only build for dragonfly.

This is also past my realm of experience/understanding. I believe dragonfly only runs on 64bit architecture, too.

I'll see if I can figure out what's going on.

@stuartnelson3
Copy link
Contributor Author

I'm getting the same failure when I run promu crossbuild locally against master, and also against this branch when I remove the devstat_dragonfly.go file :/

Is there something that could have changed recently in the build container (in the last 4 days, since the last successful build)? Can you confirm whether building for linux-ppc64 on master works for you currently?

@sdurrheimer
Copy link
Contributor

sdurrheimer commented Oct 20, 2016

@stuartnelson3 You're right, the latest release 0.13.0-rc.1 (and other commit on master) also failed for linux/ppc64 and linux/ppc64le and we didn't even see it, probably due to a mistake in promu.

Need to find the commit responsible of this.

EDIT: Looks like the problem is coming from #249
@SuperQ Any idea ?

Copy link
Contributor

@matthiasr matthiasr left a comment

Choose a reason for hiding this comment

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

Since the intersection between Prometheus users and DragonFly users is approximately @stuartnelson3 and me, I vote to merge this (but don't have the permissions to do so anymore).

@stuartnelson3
Copy link
Contributor Author

ex parvo magnus :p

@SuperQ
Copy link
Member

SuperQ commented Nov 14, 2016

Sorry for not looking into this earlier. I did some digging into the build history. It looks like this commit enabled cgo. This seems to not work for ppc64 because it requires a gcc configured to cross-compile, which is not setup. Not sure exactly how we want to fix this.

@stuartnelson3
Copy link
Contributor Author

Are there people wanting ppc support??

@grobie
Copy link
Member

grobie commented Nov 15, 2016

We can lookup download numbers via the GitHub API.

On Mon, Nov 14, 2016, 19:23 stuart nelson notifications@github.com wrote:

Are there people wanting ppc support??


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#323 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAANaMK1bexl3W18VaJbPxArateGAFQzks5q-QmigaJpZM4KRPdM
.

@stuartnelson3
Copy link
Contributor Author

stuartnelson3 commented Nov 15, 2016

Here's all the data I get:

curl -s https://api.github.com/repos/prometheus/node_exporter/releases | jq '.[].assets[] | {downloads: .download_count, name: .name, created_at: .created_at} | select(contains({name: "ppc"}))'

{
  "downloads": 1,
  "name": "node_exporter-0.13.0-rc.1.linux-ppc64.tar.gz",
  "created_at": "2016-10-16T12:42:50Z"
}
{
  "downloads": 0,
  "name": "node_exporter-0.13.0-rc.1.linux-ppc64le.tar.gz",
  "created_at": "2016-10-16T12:42:50Z"
}
{
  "downloads": 7,
  "name": "node_exporter-0.12.0.linux-ppc64.tar.gz",
  "created_at": "2016-05-05T22:23:25Z"
}
{
  "downloads": 5,
  "name": "node_exporter-0.12.0.linux-ppc64le.tar.gz",
  "created_at": "2016-05-05T22:23:26Z"
}
{
  "downloads": 5,
  "name": "node_exporter-0.12.0rc1.linux-ppc64.tar.gz",
  "created_at": "2015-10-06T17:40:11Z"
}
{
  "downloads": 3,
  "name": "node_exporter-0.12.0rc1.linux-ppc64le.tar.gz",
  "created_at": "2015-10-06T17:40:11Z"
}

EDIT: The release data starts at release 0.8.0

@SuperQ
Copy link
Member

SuperQ commented Nov 15, 2016

Looking at the data for just 0.12 (jq '.[].assets | map([.name, .download_count | tostring] | join(" "))')

[
  "node_exporter-0.12.0.darwin-386.tar.gz 32",
  "node_exporter-0.12.0.darwin-amd64.tar.gz 315",
  "node_exporter-0.12.0.linux-386.tar.gz 334",
  "node_exporter-0.12.0.linux-amd64.tar.gz 1805580",
  "node_exporter-0.12.0.linux-arm64.tar.gz 84",
  "node_exporter-0.12.0.linux-armv5.tar.gz 20",
  "node_exporter-0.12.0.linux-armv6.tar.gz 53",
  "node_exporter-0.12.0.linux-armv7.tar.gz 273",
  "node_exporter-0.12.0.linux-ppc64.tar.gz 7",
  "node_exporter-0.12.0.linux-ppc64le.tar.gz 5",
  "node_exporter-0.12.0.netbsd-386.tar.gz 9",
  "node_exporter-0.12.0.netbsd-amd64.tar.gz 35",
  "node_exporter-0.12.0.netbsd-armv5.tar.gz 5",
  "node_exporter-0.12.0.netbsd-armv6.tar.gz 6",
  "node_exporter-0.12.0.netbsd-armv7.tar.gz 4",
  "node_exporter-0.12.0.windows-386.tar.gz 31",
  "node_exporter-0.12.0.windows-amd64.tar.gz 539"
]

There are not so many ppc users compared to the rest.. While it would be nice to have these builds available, I think we need to get more frequent releases out the door.

@SuperQ
Copy link
Member

SuperQ commented Nov 16, 2016

We've fixed master, please rebase to make circleci happy.

@SuperQ SuperQ merged commit 4fd03c3 into prometheus:master Nov 17, 2016
@SuperQ SuperQ mentioned this pull request Nov 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants