Add basic index recovery metricset by ruflin · Pull Request #7225 · elastic/beats

ruflin · 2018-05-31T14:01:18Z

Add basic metricset for index recovery. By default only data about indices is fetched that are currently recovering.

Further changes:

Fix NO_COMPOSE flag for Golang integration tests
Clean up elasticsearch.GetInfo to also use fetchPath function
Enhance testing to also support settings metricset configs.

houndci-bot · 2018-05-31T14:01:40Z

metricbeat/module/elasticsearch/index_recovery/index_recovery.go

don't use an underscore in package name

houndci-bot · 2018-05-31T14:01:40Z

metricbeat/module/elasticsearch/index_recovery/data_test.go

don't use an underscore in package name

houndci-bot · 2018-05-31T14:01:40Z

metricbeat/module/elasticsearch/index_recovery/data.go

don't use an underscore in package name

houndci-bot · 2018-05-31T14:01:41Z

metricbeat/module/elasticsearch/index_recovery/index_recovery.go

don't use an underscore in package name

houndci-bot · 2018-05-31T14:01:41Z

metricbeat/module/elasticsearch/index_recovery/data_test.go

don't use an underscore in package name

houndci-bot · 2018-05-31T14:01:41Z

metricbeat/module/elasticsearch/index_recovery/data.go

don't use an underscore in package name

jsoriano · 2018-07-11T08:47:15Z

metricbeat/module/elasticsearch/index_recovery/data.go

err is not used

I added an if err check and send an the error event. I wonder if this is the right thing to do here.

I now skiped the error completely. I initially reported it as an event. The good thing about it is that I detected a problem I had in the schema. The bad thing was that now instead of in the logs the user would have tons of documents in his index with errors which I think is not good.

Maybe we could log the errors instead of sending them, or collect them in a multierror and send them all together as an only event.

jsoriano · 2018-07-11T08:47:52Z

metricbeat/module/elasticsearch/index_recovery/data.go

Would we want to explicitely check if shards exists?

jsoriano · 2018-07-11T08:56:09Z

metricbeat/tests/system/config/metricbeat.yml.j2

I wouldn't add more variables to this template, for this kind of settings, the extra option is used In system and munin modules, this could be also used here (or we could also rename the extra to metricset_options if this is the use case for extra as it seems to be used for similar things).

removed, didn't reliase extra is there.

jsoriano · 2018-07-11T09:05:53Z

metricbeat/module/elasticsearch/index_recovery/index_recovery.go

Is recoveryPath used to check master state?

The full url must be passed as this is the url that is revert to afterwards again. If not passed parts of the path will go missing. This is because of a hack to reuse the http client.

ycombinator · 2018-07-12T01:50:02Z

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

Should this be shard_id?

Kind of, see my comment in the code: https://github.com/elastic/beats/pull/7225/files#diff-d198e8deee0000763245926eefe134eaR32

TBH I'm still struggling a bit with what shard.id is as I would expect it to be a unique identifier but it's a counting integer related to an index as far as I can see.

ycombinator · 2018-07-12T01:52:16Z

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

What about other fields such as translog.*, start_time, stop_time, etc.? Are you planning on adding them later?

Add them later.

ycombinator · 2018-07-12T02:02:21Z

metricbeat/module/elasticsearch/index_recovery/data.go

Maybe I'm missing something but at first glance it looks like info isn't actually being used here?

You are right, I added it out of habit as the plan is to add the xpack part where it will be need I think. Should I remove it?

Yeah, my preference would be to remove it until we need it. Otherwise it might be confusing as to why it's there.

Ok, removed and new version pushed.

ycombinator · 2018-07-12T02:03:05Z

metricbeat/module/elasticsearch/index_recovery/index_recovery.go

See my other comment about info not being used.

Add basic metricset for index recovery. By default only data about indices is fetched that are currently recovering. Further changes: * Fix `NO_COMPOSE` flag for Golang integration tests * Clean up elasticsearch.GetInfo to also use `fetchPath` function * Enhance testing to also support settings metricset configs.

ruflin added in progress Pull request is currently in progress. module Metricbeat Metricbeat labels May 31, 2018

houndci-bot reviewed May 31, 2018

View reviewed changes

ruflin force-pushed the index-recovery branch 2 times, most recently from 0173924 to 6de0631 Compare July 10, 2018 07:49

ruflin added review and removed in progress Pull request is currently in progress. labels Jul 10, 2018

ruflin force-pushed the index-recovery branch 2 times, most recently from 52e9a1d to e82ad36 Compare July 10, 2018 11:56

ruflin changed the title ~~add basic index recovery metricset~~ Add basic index recovery metricset Jul 10, 2018

jsoriano reviewed Jul 11, 2018

View reviewed changes

ruflin force-pushed the index-recovery branch from a439d23 to 1091b6b Compare July 11, 2018 13:18

ycombinator reviewed Jul 12, 2018

View reviewed changes

ruflin mentioned this pull request Jul 12, 2018

Monitor the Elastic Stack with Metricbeat #7035

Closed

39 tasks

ruflin added 7 commits July 13, 2018 15:48

improve testing

cc9b504

remove template part

988b562

update generated file

f23eb14

skip mapping errors

e04a187

rebase

3b2b331

remove info as not used yet

0f84366

ruflin force-pushed the index-recovery branch from 1091b6b to 0f84366 Compare July 13, 2018 14:04

jsoriano approved these changes Jul 13, 2018

View reviewed changes

ycombinator merged commit 14888de into elastic:master Jul 14, 2018

ruflin deleted the index-recovery branch July 20, 2018 13:47

Conversation

ruflin commented May 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ruflin commented May 31, 2018 •

edited

Loading