Add system/raid metricset #5642

ruflin · 2017-11-20T03:54:34Z

Implements #5600.

houndci-bot · 2017-11-20T03:54:44Z

metricbeat/module/system/raid/raid.go

+	return m, nil
+}
+
+func (m *MetricSet) Fetch() ([]common.MapStr, error) {


exported method MetricSet.Fetch should have comment or be unexported

houndci-bot · 2017-11-20T03:54:44Z

metricbeat/module/system/raid/raid.go

+	fs procfs.FS
+}
+
+func New(base mb.BaseMetricSet) (mb.MetricSet, error) {


exported function New should have comment or be unexported

houndci-bot · 2017-11-20T03:54:44Z

metricbeat/module/system/raid/raid.go

+	}
+}
+
+type MetricSet struct {


exported type MetricSet should have comment or be unexported

houndci-bot · 2017-11-20T03:54:45Z

metricbeat/module/system/raid/raid.go

+	return m, nil
+}
+
+func (m *MetricSet) Fetch() ([]common.MapStr, error) {


exported method MetricSet.Fetch should have comment or be unexported

houndci-bot · 2017-11-20T03:54:45Z

metricbeat/module/system/raid/raid.go

+	fs procfs.FS
+}
+
+func New(base mb.BaseMetricSet) (mb.MetricSet, error) {


exported function New should have comment or be unexported

houndci-bot · 2017-11-20T03:54:45Z

metricbeat/module/system/raid/raid.go

+	}
+}
+
+type MetricSet struct {


exported type MetricSet should have comment or be unexported

I guess it would be worth modifying the generator to add comments for all these.

@tsg We actually have pretty extensive comments in the generator which tell you what to do: https://github.com/elastic/beats/blob/master/metricbeat/scripts/module/metricset/metricset.go.tmpl#L17 I removed all of them manually to not have the boiler plate ...

tsg · 2017-11-20T09:21:43Z

LGTM, but it would be good to have some sort of testing on it.

andrewkroh · 2017-11-20T17:27:55Z

metricbeat/module/system/raid/raid.go

+			"activity_state": stat.ActivityState,
+			"disks": common.MapStr{
+				"active": stat.DisksActive,
+				"total":  stat.DisksTotal,


Usage question: How would I figure out how many available spares there are? Is spares = total - active? Or does total include the number of failed disks still in the array?

@andrewkroh Based on https://raid.wiki.kernel.org/index.php/Mdstat I think what you state is correct.

andrewkroh · 2017-11-20T17:28:39Z

metricbeat/module/system/raid/raid.go

+	}
+
+	mountPoint := filepath.Join(systemModule.HostFS, procfs.DefaultMountPoint)
+	fs, err := procfs.NewFS(mountPoint)


For testing purposes I'd make it possible to fully define the proc dir location so that you can create testdata/proc/mdstat and read it.

Here's some sample data for testing: https://github.com/elastic/procfs/blob/master/fixtures/mdstat

andrewkroh · 2017-11-20T17:32:13Z

metricbeat/module/system/raid/raid_test.go

+)
+
+func TestData(t *testing.T) {
+


Consider removing this newline for me 😄 . (BTW I have a tool for this at nonewlines)

@andrewkroh You should add this tool to our make fmt part so CI can bug us in the future instead of requiring the krohcompiler. Like this you can also go on vacation :-D

It will be part of gofmt in Go 1.10. I'm just trying to prepare you in advance.

I need to check if it was you that contributed it to Go 1.10 :-)

BTW: Probably worth adding the script it will take some time until we are on 1.10 (lots of newlines to come until then)

ruflin · 2017-11-20T20:38:59Z

@andrewkroh I added some basic tests (still WIP). It works with the file from the procfs repo but not the one I got from our servers. The error I get:

        Error:          Received unexpected error "error parsing _meta/test/proc/mdstat: unexpected statusline:       497754112 blocks super 1.2 512k chunks"

The content of the file looks as following:

Personalities : [raid0] [raid1] [linear] [multipath] [raid6] [raid5] [raid4] [raid10]
md0 : active raid1 sdb1[1] sda1[0]
      1047552 blocks super 1.2 [2/2] [UU]

md1 : active raid0 sdb2[1] sda2[0]
      497754112 blocks super 1.2 512k chunks

unused devices: <none>

Probably an issue we have to solve in procfs (didn't look into it yet).

andrewkroh · 2017-11-20T23:00:27Z

metricbeat/module/system/raid/_meta/test/mdstat1

@@ -0,0 +1,8 @@
+Personalities : [raid0] [raid1] [linear] [multipath] [raid6] [raid5] [raid4] [raid10]


I recommend storing this under system/raid/testdata/[testname]/proc/mdstat. Using testdata seems to be conventional to Go; we have it in a few places. And keeping the proc/mdstat ensures that we don't have to mess with temporary files which eliminates code and potential problems. To switch between test cases you just change the raid.mount_point between testdata/scenarioA and testdata/scenarioB.

The "[testname]" could be anything "hostA", "scenarioA", "ubuntu16".

I changed it to just /proc/mdstats inside testdata as I realised one mdstat file should be enough and we can just add more lines. I kept the testdata directory inside _meta as we do for other metricsets too to not conflict with package and and "automatic logic to gather files" we have around metricbeat. Like this only the _meta directory has to be ignored from all these logics.

ruflin · 2017-11-27T03:01:21Z

@andrewkroh I for now skipped the special raid data I have pasted above as this has to be fixed in procfs. I wanted to open an issue there for it but it seems there is no issue tab? https://github.com/elastic/procfs Is that intentional?

andrewkroh · 2017-11-27T13:15:15Z

I think issues are turned off by default for forks to encourage using the parent's issue tracker. But you can turn issues on in the project's settings IIRC.

ruflin · 2017-11-27T23:27:10Z

@andrewkroh How close are we still to the "parent project". I actually don't have access rights to turn on the issue tracker :-)

ruflin · 2017-11-28T01:07:20Z

@plinde @andrewkroh I suggest we get this in as is and start to iterate on top of it?

ruflin · 2017-11-28T02:27:04Z

jenkins, retest it

Closes elastic#5600

ruflin added in progress Pull request is currently in progress. Metricbeat Metricbeat review labels Nov 20, 2017

ruflin requested a review from andrewkroh November 20, 2017 03:54

houndci-bot reviewed Nov 20, 2017

View reviewed changes

ruflin mentioned this pull request Nov 20, 2017

Module/metricset for Metricbeat: RAID Metrics #5600

Closed

ruflin force-pushed the raid-metricset branch 5 times, most recently from 377cef5 to ae80d6d Compare November 20, 2017 05:24

ruflin force-pushed the raid-metricset branch from ae80d6d to e8e7aa2 Compare November 20, 2017 11:02

andrewkroh reviewed Nov 20, 2017

View reviewed changes

ruflin force-pushed the raid-metricset branch from 37c9c7a to bafa4cf Compare November 27, 2017 02:58

ruflin force-pushed the raid-metricset branch from 8ef1b88 to 31950e5 Compare November 27, 2017 03:08

ruflin force-pushed the raid-metricset branch from 31950e5 to 2257685 Compare November 28, 2017 01:06

ruflin removed the in progress Pull request is currently in progress. label Nov 28, 2017

ruflin force-pushed the raid-metricset branch 2 times, most recently from 2c37869 to caf62d4 Compare November 30, 2017 05:30

Add system/raid metricset to Metricbeat

46c2691

Closes elastic#5600

ruflin force-pushed the raid-metricset branch from caf62d4 to 46c2691 Compare December 4, 2017 03:05

andrewkroh merged commit 6536115 into elastic:master Dec 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add system/raid metricset #5642

Add system/raid metricset #5642

ruflin commented Nov 20, 2017 •

edited by andrewkroh

Loading

houndci-bot Nov 20, 2017

houndci-bot Nov 20, 2017

houndci-bot Nov 20, 2017

houndci-bot Nov 20, 2017

houndci-bot Nov 20, 2017

houndci-bot Nov 20, 2017

tsg Nov 20, 2017

ruflin Nov 27, 2017

tsg commented Nov 20, 2017

andrewkroh Nov 20, 2017

ruflin Nov 27, 2017

andrewkroh Nov 20, 2017

andrewkroh Nov 20, 2017

andrewkroh Nov 20, 2017

ruflin Nov 27, 2017

andrewkroh Nov 27, 2017

ruflin Nov 27, 2017

ruflin Nov 27, 2017

ruflin commented Nov 20, 2017

andrewkroh Nov 20, 2017

ruflin Nov 27, 2017

ruflin commented Nov 27, 2017

andrewkroh commented Nov 27, 2017

ruflin commented Nov 27, 2017

ruflin commented Nov 28, 2017

ruflin commented Nov 28, 2017

		@@ -0,0 +1,8 @@
		Personalities : [raid0] [raid1] [linear] [multipath] [raid6] [raid5] [raid4] [raid10]

Add system/raid metricset #5642

Add system/raid metricset #5642

Conversation

ruflin commented Nov 20, 2017 • edited by andrewkroh Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsg commented Nov 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Nov 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Nov 27, 2017

andrewkroh commented Nov 27, 2017

ruflin commented Nov 27, 2017

ruflin commented Nov 28, 2017

ruflin commented Nov 28, 2017

ruflin commented Nov 20, 2017 •

edited by andrewkroh

Loading