prometheus-junkyard / mesos_exporter

Prometheus exporter for Mesos, deprecated.
Apache License 2.0
42 stars 22 forks source link

Scrape a Mesos Master process #13

Closed wndhydrnt closed 9 years ago

wndhydrnt commented 9 years ago

This PR adds support for scraping metrics exposed by a Mesos Master process.

It also introduces a change to the CLI flags by specifying exporter.scrape-mode to distinguish between the different modes that mesos_exporter supports. Also the different flags that included the term url have been unified into the flag exporter.url.

For now, I have added only those metrics also exposed by wndhydrnt/mesos-task-exporter (which I'd like to deprecate in favour of this repo) as these are the metrics I use every day.

Together with prometheus/prometheus#1027, these changes make it possible to monitor a Mesos cluster by running an instance of mesos_exporter next to a Mesos process.

wndhydrnt commented 9 years ago

PTAL @antonlindstrom @juliusv @tommyulfsparre

wndhydrnt commented 9 years ago

I forgot one feature in the initial push. The name of the task and the name of the framework that spawned the task are now added as labels to each metric generated from the /monitor/statistics.json endpoint. The names are the same that are visible in the Mesos UI. This simplifies writing expressions that target several tasks of the same app as the names usually do not change while IDs do.

StephanErb commented 9 years ago

What's the status of this pull request? Would love to get to use those new metrics.

juliusv commented 9 years ago

Good question. /cc @wndhydrnt @brian-brazil

wndhydrnt commented 9 years ago

@juliusv thanks for the review. I've updated the code according to your suggestions.

juliusv commented 9 years ago

@wndhydrnt Awesome, thanks! Taking another look!

juliusv commented 9 years ago

Looks :+1: to me otherwise.

wndhydrnt commented 9 years ago

@juliusv addressed your comment regarding _bytes. As the /metrics/snapshot endpoint also exposes megabytes instead of bytes, I've added the functionality to convert the value of a metric by passing a function to the snapshotMetric struct.

Let me know if this is fine with you and I'll squash the commits.

juliusv commented 9 years ago

@wndhydrnt Thanks!

Hmm, I'm wondering if adding two extra constructor functions isn't a bit of over-engineering for that. How about still keeping all of these metric definitions as struct literals instead of function calls and simply have an optional convertFn field that can be nil (when omitted), and it's only applied if it's not nil?

Also, the convertMegabytesToBytes function is so small that you could consider inlining it, though that's up to taste:

  // ...
  convertFn: func(v float64) { return v * 1024 * 1024 },
  // ...

If you keep it, I'd just name it megabytesToBytes. Seems to be more common to omit prefixes like get, compute, convert, etc. from method names if they are already clear without it.

wndhydrnt commented 9 years ago

@juliusv addressed your last comment

juliusv commented 9 years ago

:+1: Awesome, please squash and I'll merge!

wndhydrnt commented 9 years ago

Squashed

juliusv commented 9 years ago

Thanks!