Skip to content

[metricbeat] migrate kvm to ReporterV2 with new error handling #11814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fearful-symmetry merged 4 commits into from
Apr 15, 2019
Merged

[metricbeat] migrate kvm to ReporterV2 with new error handling #11814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
97bd26a
migrate kvm to reporterv2 with error handling
fearful-symmetry Apr 15, 2019
aa59de3
update logging on defer
fearful-symmetry Apr 15, 2019
c7d52cf
remove whitespace from Fetch
fearful-symmetry Apr 15, 2019
de623e7
change reported check, to return
fearful-symmetry Apr 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions metricbeat/module/kvm/dommemstat/dommemstat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
// Fetch methods implements the data gathering and data conversion to the right
// format. It publishes the event which is then forwarded to the output. In case
// of an error set the Error field of mb.Event or simply call report.Error().
func (m *MetricSet) Fetch(report mb.ReporterV2) {

func (m *MetricSet) Fetch(report mb.ReporterV2) error {
var (
c net.Conn
err error
Expand All @@ -102,44 +101,47 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) {

c, err = net.DialTimeout(u.Scheme, address, m.Timeout)
if err != nil {
report.Error(errors.Wrapf(err, "cannot connect to %v", u))
return
return errors.Wrapf(err, "cannot connect to %v", u)
}
}

defer c.Close()

l := libvirt.New(c)
if err = l.Connect(); err != nil {
report.Error(errors.Wrap(err, "error connecting to libvirtd"))
return
return errors.Wrap(err, "error connecting to libvirtd")
}
defer func() {
if err = l.Disconnect(); err != nil {
report.Error(errors.Wrap(err, "failed to disconnect"))
msg := errors.Wrap(err, "failed to disconnect")
report.Error(msg)
m.Logger().Error(msg)
Copy link
Copy Markdown
Contributor

@exekias exekias Apr 15, 2019

Choose a reason for hiding this comment

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

sorry I missed previous discussions and refactor. Was report.Error logging this message before?

Copy link
Copy Markdown
Contributor Author

@fearful-symmetry fearful-symmetry Apr 15, 2019

Choose a reason for hiding this comment

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

@ruflin 's thinking was that report-ed error messages and logged error messages get sent to different places & are consumed in different ways, so we should sent errors to both.

Copy link
Copy Markdown
Contributor

@exekias exekias Apr 15, 2019

Choose a reason for hiding this comment

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

Thanks for explaining, LGTM

}
}()

domains, err := l.Domains()
if err != nil {
report.Error(errors.Wrap(err, "error listing domains"))
return
return errors.Wrap(err, "error listing domains")
}

for _, d := range domains {
gotDomainMemoryStats, err := l.DomainMemoryStats(d, maximumStats, flags)
if err != nil {
report.Error(errors.Wrapf(err, "error fetching memory stats for domain %s", d.Name))
msg := errors.Wrapf(err, "error fetching memory stats for domain %s", d.Name)
report.Error(msg)
m.Logger().Error(msg)
continue
}

if len(gotDomainMemoryStats) == 0 {
report.Error(errors.Errorf("no memory stats for domain %s", d.Name))
msg := errors.Errorf("no memory stats for domain %s", d.Name)
report.Error(msg)
m.Logger().Error(msg)
continue
}

for i := range gotDomainMemoryStats {
report.Event(mb.Event{
reported := report.Event(mb.Event{
MetricSetFields: common.MapStr{
"id": d.ID,
"name": d.Name,
Expand All @@ -149,8 +151,13 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) {
},
},
})
if !reported {
return errors.New("metricset has closed")
}
}
}

return nil
}

func getDomainMemoryStatName(tag int32) string {
Expand Down
4 changes: 2 additions & 2 deletions metricbeat/module/kvm/dommemstat/dommemstat_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ import (
func TestFetchEventContents(t *testing.T) {
conn := libvirttest.New()

f := mbtest.NewReportingMetricSetV2(t, getConfig(conn))
f := mbtest.NewReportingMetricSetV2Error(t, getConfig(conn))

events, errs := mbtest.ReportingFetchV2(f)
events, errs := mbtest.ReportingFetchV2Error(f)
if len(errs) > 0 {
t.Fatal(errs)
}
Expand Down