Adding cluster_stats metricset for elasticsearch module #7638

ycombinator · 2018-07-18T23:06:46Z

This PR adds a cluster_stats metricset for the elasticsearch Metricbeat module. Only an initial set of fields are added by this PR.

For follow up PRs

Adding event generation for X-Pack Monitoring indices.
Adding more fields, as necessary.

houndci-bot · 2018-07-18T23:06:56Z

metricbeat/module/elasticsearch/cluster_stats/data.go

don't use an underscore in package name

houndci-bot · 2018-07-18T23:06:56Z

metricbeat/module/elasticsearch/cluster_stats/cluster_stats.go

don't use an underscore in package name

houndci-bot · 2018-07-18T23:06:56Z

metricbeat/module/elasticsearch/cluster_stats/data.go

don't use an underscore in package name

houndci-bot · 2018-07-18T23:06:57Z

metricbeat/module/elasticsearch/cluster_stats/cluster_stats.go

don't use an underscore in package name

houndci-bot · 2018-07-18T23:17:11Z

metricbeat/module/elasticsearch/cluster_stats/data_test.go

don't use an underscore in package name

ruflin · 2018-07-19T07:12:26Z

metricbeat/module/elasticsearch/cluster_stats/_meta/data.json

How does the cluster index data overlap with the index and node_stats metricset data?

index metricset has docs, segments, and store which are also present in this metricset here.

node_stats metricset has docs, segments, and store, which are also present in this metricset here.

node_stats metricset also has fs and jvm stats, which are currently not present in this metricset here but are available from the Elasticsearch GET _cluster/stats API.

This metricset here has shards which is absent in the index metricset and also not available from the Elasticsearch GET _stats API.

This metricset here has shards which is absent in the node_stats metricset and also not available from the Elasticsearch GET _nodes/_local API or the GET _nodex/_local/stats API.

This metricset here has fielddata which is absent in the index metricset but is available from the Elasticsearch GET _stats API.

This metricset here has fielddata which is absent in the node_stats metricset and also not available from the Elasticsearch GET _nodes/_local API. However it is available from the Elasticsearch GET _nodes/_local/stats API. Of course, the latter API is missing a bunch of other fields that are present in the former API so either we call both or we ask the ES team to enhance the latter API's response to include all the fields we use from the former API's response as well.

I do need to fix the field names in this metricset here so they match up with the ones in other elasticsearch metricsets (e.g. I need to use count instead of total, etc.)

Field names fixed in 6662787.

Should we report all the data we have in the index and node_stats metricset here too? Or should we focus on the data points that other metricsets don't have?

I think to start off we can only report the stats that are not already covered by other metricsets. Concretely, those would be fielddata and shards fields.

Fixed in 27ef3fb.

ruflin · 2018-07-19T07:13:42Z

metricbeat/module/elasticsearch/cluster_stats/data.go

++

I think both values are a must to be returned y cluster stats API

Created elastic/elasticsearch#32205 to request cluster UUID from cluster stats API itself.

... and put up PR as well: elastic/elasticsearch#32206

Keep in mind that older versions of ES will not have it in, so in these cases we need to skip it.

Good point! Should we skip it or make the extra API call in that case (I realize that complicates the logic/organization of the code a fair bit)?

Lets skip it to not add more complexity. New versions, new features :-)

Fixed in ed74319b4.

ruflin

Code LGTM. Could you add a CHANGELOG entry?

ruflin · 2018-07-23T21:06:11Z

metricbeat/module/elasticsearch/cluster_stats/_meta/fields.yml

Apply to all lines: I think normally we put a dot at the end for the docs.

ruflin · 2018-07-23T21:06:24Z

metricbeat/module/elasticsearch/cluster_stats/_meta/test/cluster_stats.700.json

Can we also have one for 630?

Added in a5953bd.

ruflin · 2018-07-24T06:19:25Z

@ycombinator Looks like this could be merged before the ES PR is merged as we do check if the field exists?

Could you rebase and directly squash the commits?

ycombinator · 2018-07-24T11:39:39Z

Yes, good point @ruflin. I will fix up this PR now so it can be reviewed/merged soon.

ycombinator · 2018-07-24T12:02:02Z

Alright, all squashed up into 766b7f3 and ready for review/merge. Thanks @ruflin!

ycombinator added in progress Pull request is currently in progress. Metricbeat Metricbeat v7.0.0-alpha1 labels Jul 18, 2018

houndci-bot reviewed Jul 18, 2018

View reviewed changes

metricbeat/module/elasticsearch/cluster_stats/data_test.go Outdated

Copy link

houndci-bot Jul 18, 2018

Choose a reason for hiding this comment

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

don't use an underscore in package name

ycombinator requested review from jsoriano and ruflin July 18, 2018 23:34

ycombinator added review and removed in progress Pull request is currently in progress. labels Jul 18, 2018

ruflin reviewed Jul 19, 2018

View reviewed changes

ruflin mentioned this pull request Jul 23, 2018

Monitor the Elastic Stack with Metricbeat #7035

Closed

39 tasks

ruflin reviewed Jul 23, 2018

View reviewed changes

ycombinator added the v6.4.0 label Jul 24, 2018

Adding cluster_stats metricset for elasticsearch module

766b7f3

ruflin merged commit beada5f into elastic:master Jul 24, 2018

ycombinator deleted the metricbeat/elasticsearch/cluster-stats branch July 30, 2018 22:24

Adding cluster_stats metricset for elasticsearch module #7638

Adding cluster_stats metricset for elasticsearch module #7638

Uh oh!

Conversation

ycombinator commented Jul 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

For follow up PRs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ycombinator Jul 19, 2018 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruflin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruflin commented Jul 24, 2018

Uh oh!

ycombinator commented Jul 24, 2018

Uh oh!

ycombinator commented Jul 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ycombinator commented Jul 18, 2018 •

edited

Loading

ycombinator Jul 19, 2018 •

edited

Loading

ycombinator commented Jul 24, 2018 •

edited

Loading