[Metricbeat] Migrate Ceph pool_disk to use ReporterV2 interface #11049
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? |
sayden
added
Metricbeat
|
jenkins, test this please Thank you very much for the contribution @berfinsari let's see what Jenkins says. It's awesome! |
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
| @@ -36,6 +36,8 @@ var ( | |||
| }.Build() | |||
| ) | |||
|
|
|||
| var logger = logp.NewLogger("ceph.pool_disk") | |||
There was a problem hiding this comment.
If you rebase to master, you can delete this and use base.Logger()
| if err != nil { | ||
| return nil, err | ||
| logger.Error(err) |
There was a problem hiding this comment.
If you rebase to master, you can use here m.Logger().Error(err) instead.
| @@ -22,13 +22,20 @@ import ( | |||
| "os" | |||
| "testing" | |||
|
|
|||
| "github.com/stretchr/testify/assert" | |||
|
|
|||
| mbtest "github.com/elastic/beats/metricbeat/mb/testing" | |||
| ) | |||
|
|
|||
| func TestData(t *testing.T) { | |||
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 😄 |
berfinsari
force-pushed
the
migration/mb/reportv2/ceph/pool_disk
branch
from
0b4960c to
aa09c23
Compare
|
Thank you for the review @sayden |
|
jenkins, test this |
| @@ -32,6 +32,7 @@ import ( | |||
|
|
|||
| func TestFetchEventContents(t *testing.T) { | |||
| absPath, err := filepath.Abs("../_meta/testdata/") | |||
| assert.NoError(t, err) | |||
Refer to #10774 for more info