Split the exporter into multiple collectors #65

metalmatze · 2017-06-28T12:30:05Z

We want to get rid of the big maps with counters and gauges.
This is a approach I could come up with.

Put everything a metric needs (type, desc, value, labels) into a struct to have it in one place. The improvement is, that these metrics, now have value and label funcs to retrieve their values where they're declared. This way it's all combined at one place and not all over the place.

Changes to metrics:

- elasticsearch_up

We're doing multiple requests concurrently to the endpoint now. Not really sure on how this metric could be helpful now. Would need some sort of global shared state between collectors. Dropping it for now.

- elasticsearch_cluster_health_status_is_green
- elasticsearch_cluster_health_status_is_red
- elasticsearch_cluster_health_status_is_yellow

These are duplicates of elasticsearch_cluster_health_status which has the color as a label, as it should be.

- elasticsearch_filesystem_data_free_percent
- elasticsearch_filesystem_data_used_percent

Percentage has been computed in this exporter up until now. We're dropping these metrics. This should be calculated in prometheus itself. We're probably going to provide recording rules that make this optional metrics.

- elasticsearch_indices_flush_time_seconds_total
+ elasticsearch_indices_flush_time_seconds

Because we're talking about seconds.

- indices_search_fetch_time_seconds_total

This is a duplicate of indices_search_fetch_time_seconds

- elasticsearch_indices_search_query_time_seconds_total

This is a duplicate of indices_search_query_time_seconds

…se.go

Wrap promtheus.Desc to have it all in one place

…emMetrics

…etric

…or/nodes.go

dominikschulz

LGTM

Would be nice to add a proper README, example recording/alerting rules and a example Grafana dashboard, but we can merge first and add that later.

dominikschulz · 2017-06-28T14:13:31Z

collector/cluster_health.go

+		),
+		NumberOfPendingTasks: prometheus.NewDesc(
+			prometheus.BuildFQName(namespace, subsystem, "number_of_pending_tasks"),
+			"XXX WHAT DOES THIS MEAN?",


Intended or left over? If we can't figure out a proper description we may want to put an undocumented instead.

dominikschulz · 2017-06-28T14:13:46Z

collector/cluster_health.go

+		),
+		DelayedUnassignedShards: prometheus.NewDesc(
+			prometheus.BuildFQName(namespace, subsystem, "delayed_unassigned_shards"),
+			"XXX WHAT DOES THIS MEAN?",


document, please

dominikschulz · 2017-06-28T14:13:51Z

collector/cluster_health.go

+		),
+		TimedOut: prometheus.NewDesc(
+			prometheus.BuildFQName(namespace, subsystem, "timed_out"),
+			"XXX WHAT DOES THIS MEAN?",


document, please

dominikschulz · 2017-06-28T14:15:31Z

collector/cluster_health.go

+	)
+
+	var statusValue float64
+	if clusterHealthResponse.Status == "green" {


What about red and yellow? Would like to be able to distinquish between those two as well.

I currently assume that elasticsearch_cluster_health_status is 0 if red or yellow and only 1 when green.
I can only think about doing something like

red => 0

yellow => 1

green => 2
which honestly feels weird.

Yes, indeed. I see that issue. But on the other hand I'd like to differentiate between red and yellow ...

dominikschulz · 2017-06-28T14:18:40Z

collector/cluster_health.go

+			"The number of shards that are currently moving from one node to another node.",
+			[]string{"cluster"}, nil,
+		),
+		StatusIsGreen: prometheus.NewDesc(


Could we drop this, please?
We already have the status metric below.

dominikschulz · 2017-06-28T14:19:31Z

collector/cluster_health.go

+	NumberOfNodes           *prometheus.Desc
+	NumberOfPendingTasks    *prometheus.Desc
+	RelocatingShards        *prometheus.Desc
+	StatusIsGreen           *prometheus.Desc


Can we drop this in favor of Status?

dominikschulz · 2017-06-28T14:19:35Z

collector/cluster_health.go

+	RelocatingShards        *prometheus.Desc
+	StatusIsGreen           *prometheus.Desc
+	Status                  *prometheus.Desc
+	StatusIsYellow          *prometheus.Desc


Can we drop this in favor of Status?

dominikschulz · 2017-06-28T14:19:38Z

collector/cluster_health.go

+	StatusIsGreen           *prometheus.Desc
+	Status                  *prometheus.Desc
+	StatusIsYellow          *prometheus.Desc
+	StatusIsRed             *prometheus.Desc


Can we drop this in favor of Status?

… status

Since prometheus-community/elasticsearch_exporter#65 the elasticsearch_up metric is in the elasticsearch_cluster_health namespace

metalmatze added 15 commits June 19, 2017 16:44

Delete all old .go files

4fac392

Add not to use gco in .promu.yml

9a76833

Add main.go, tls.go and create collector package with cluster_health.go

76b1cb9

Vendor go-kit dependencies

17bb882

Create first draft of nodes collector

280086e

Move clusterHealthResponse into own file called cluster_health_respon…

5a153aa

…se.go

Move NodeStatsResponse into own file which is the old structs.go

6bbffdf

Add all metrics for indices per node

64342cd

Introduce nodeMetric{} that has 2 funcs to get values

1c86e32

Wrap promtheus.Desc to have it all in one place

Add jvm_memory metrics to nodes collector

e0f5fda

Add gcCollectionMetrics, breakerMetrics, threadPoolMetrics & filesyst…

4cbb4f6

…emMetrics

Add more missing counters to nodeMetrics

17f7591

Add missing process and transport node metrics

8954c64

Add indices_indexing & indices_merges subsystem metrics

85e0de6

Add indices_refresh subsystem metrics

ef9882c

metalmatze requested a review from dominikschulz June 28, 2017 12:30

metalmatze added 2 commits June 28, 2017 15:55

Add missing elasticsearch_indices_store_throttle_time_seconds_total m…

ce8a7a2

…etric

Update collector/cluster_health.go to use the same pattern as collect…

5b3d923

…or/nodes.go

dominikschulz approved these changes Jun 28, 2017

View reviewed changes

dominikschulz reviewed Jun 28, 2017

View reviewed changes

Iterate over all colors to create metric with each color as label for…

ad974b4

… status

dominikschulz merged commit d37917b into master Jun 28, 2017

dominikschulz deleted the v2 branch June 28, 2017 15:01

aveyrenc added a commit to orange-cloudfoundry/prometheus-boshrelease that referenced this pull request Sep 20, 2017

Fix Elasticsearch dashboard templating

8b5133a

Since prometheus-community/elasticsearch_exporter#65 the elasticsearch_up metric is in the elasticsearch_cluster_health namespace

aveyrenc mentioned this pull request Sep 20, 2017

Fix Elasticsearch dashboard templating cloudfoundry/prometheus-boshrelease#124

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split the exporter into multiple collectors #65

Split the exporter into multiple collectors #65

metalmatze commented Jun 28, 2017 •

edited

Loading

dominikschulz left a comment

dominikschulz Jun 28, 2017

dominikschulz Jun 28, 2017

dominikschulz Jun 28, 2017

dominikschulz Jun 28, 2017

metalmatze Jun 28, 2017

dominikschulz Jun 28, 2017

dominikschulz Jun 28, 2017

dominikschulz Jun 28, 2017

dominikschulz Jun 28, 2017

dominikschulz Jun 28, 2017

Split the exporter into multiple collectors #65

Split the exporter into multiple collectors #65

Conversation

metalmatze commented Jun 28, 2017 • edited Loading

Changes to metrics:

dominikschulz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metalmatze commented Jun 28, 2017 •

edited

Loading