Skip to content

[Metricbeat] Migrate Ceph cluster_disk to use ReporterV2 interface#10990

Merged
sayden merged 8 commits intoelastic:masterfrom
sayden:migration/mb/reporterv2/ceph/cluster_disk
Mar 7, 2019
Merged

[Metricbeat] Migrate Ceph cluster_disk to use ReporterV2 interface#10990
sayden merged 8 commits intoelastic:masterfrom
sayden:migration/mb/reporterv2/ceph/cluster_disk

Conversation

@sayden
Copy link
Copy Markdown
Contributor

@sayden sayden commented Feb 28, 2019

Refer to #10774 for more info

@sayden sayden added Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Feb 28, 2019
@sayden sayden self-assigned this Feb 28, 2019
@sayden sayden requested a review from a team as a code owner February 28, 2019 14:59
@sayden sayden changed the title [Metricbeat] Migrate ETCD leader to use ReporterV2 interface [Metricbeat] Migrate Ceph cluster_disk to use ReporterV2 interface Feb 28, 2019
@sayden
Copy link
Copy Markdown
Contributor Author

sayden commented Mar 1, 2019

Error in Kibana/status module seems unrelated

@sayden sayden added the review label Mar 1, 2019
@sayden sayden force-pushed the migration/mb/reporterv2/ceph/cluster_disk branch from a8df0bb to d13577f Compare March 1, 2019 12:33
Copy link
Copy Markdown
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

It LGTM in principle, could you regenerate the data.json file?

@sayden
Copy link
Copy Markdown
Contributor Author

sayden commented Mar 5, 2019

https://github.com/elastic/beats/blob/master/metricbeat/module/ceph/cluster_disk/_meta/data.json does not need to be regenerated (in other words, when you regenerate it, nothing changes)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do you skip it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What a nice timing 😄 #10990 (comment) and more info here #10993 (comment)

Copy link
Copy Markdown
Contributor Author

@sayden sayden Mar 5, 2019

Choose a reason for hiding this comment

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

CI test aren't launched with -data option but this is checked in mbtest.WriteEventsReporterV2 in line 42. An error can still be raised in line 36 if ceph is not started by compose, which was producing flaky tests.

The problem is that Ceph start with Compose is buggy and it was omitted consistently in all Metricsets

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see. As soon as #10648 is merged we should switch over Ceph to generate the data file based on an http server as the current tests already use a http server.

Let's skip it for now but open a follow up issue to track:

@sayden sayden force-pushed the migration/mb/reporterv2/ceph/cluster_disk branch from d86447c to 2401d09 Compare March 6, 2019 14:55
@sayden sayden force-pushed the migration/mb/reporterv2/ceph/cluster_disk branch from ede70fe to 11031ce Compare March 7, 2019 10:02
@sayden
Copy link
Copy Markdown
Contributor Author

sayden commented Mar 7, 2019

Pushed few things I saw on new commits, mainly 30f809b and 45074a3 : New logger interface and reporting interface with errors

@sayden sayden merged commit a78c552 into elastic:master Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Metricbeat Metricbeat review Team:Integrations Label for the Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants