[Metricbeat] Migrate Ceph pool_disk to use ReporterV2 interface#11049
[Metricbeat] Migrate Ceph pool_disk to use ReporterV2 interface#11049sayden merged 2 commits intoelastic:masterfrom
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
jenkins, test this please Thank you very much for the contribution @berfinsari let's see what Jenkins says. It's awesome! |
sayden
left a comment
There was a problem hiding this comment.
Hi @berfinsari sorry for the delay but we have had some issues with Ceph in previous migrations and I wanted to update with helpful info here. Guide to migration needed an update too.
In the guide we have introduced a new interface that looks like Fetch(reporter mb.ReporterV2) error (with error at the end) that is the preferred way now. You can omit this here however
There was a problem hiding this comment.
If you rebase to master, you can delete this and use base.Logger()
There was a problem hiding this comment.
If you rebase to master, you can use here m.Logger().Error(err) instead.
There was a problem hiding this comment.
We have realized that this call mbtest.ReportingFetchV2(f) must be omitted here because this test must not be launched in CI and check is happening in mbtest.WriteEventsReporterV2 so it should end up looking more or less like this:
func TestData(t *testing.T) {
f := mbtest.NewReportingMetricSetV2(t, getConfig())
if err := mbtest.WriteEventsReporterV2(f, t, ""); err != nil {
t.Fatal("write", err)
}
}This is the only important change here to avoid a flaky test that was happening
|
You can also check this Ceph PR #10990 as reference. Sorry about that, this module was one of the trickiest 😄 |
0b4960c to
aa09c23
Compare
|
Thank you for the review @sayden |
|
jenkins, test this |
|
|
||
| func TestFetchEventContents(t *testing.T) { | ||
| absPath, err := filepath.Abs("../_meta/testdata/") | ||
| assert.NoError(t, err) |
Refer to #10774 for more info