Metricbeat: MongoDB collstats metricset #5852

MerlinDMC · 2017-12-10T10:36:14Z

This adds a metricset for individual collection statistics.
It will allow us to graph "hot" collections for read, write and other spikes in usage.

elasticmachine · 2017-12-10T10:36:16Z

Can one of the admins verify this patch?

houndci-bot · 2017-12-10T10:36:21Z

metricbeat/module/mongodb/collstats/collstats.go

+	if _, ok := result["totals"]; !ok {
+		logp.Err("Error accessing collection totals in returned data")
+		return events, err
+	} else {


if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)

houndci-bot · 2017-12-10T10:36:22Z

metricbeat/module/mongodb/collstats/collstats.go

+	if _, ok := result["totals"]; !ok {
+		logp.Err("Error accessing collection totals in returned data")
+		return events, err
+	} else {


if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)

MerlinDMC · 2017-12-10T13:55:21Z

@ruflin There you go. Happy reviewing 🎉

ruflin

Left a few comments. Can you please also add a CHANGELOG entry?

ruflin · 2017-12-11T23:09:44Z

metricbeat/module/mongodb/collstats/_meta/fields.yml

+          description: >
+            Total number of lock wait events.
+
+        - name: readLock.time


To better follow our naming conventions, this should be lock.read.time, lock.read.count, lock.write.time, ....

https://www.elastic.co/guide/en/beats/devguide/current/event-conventions.html

ruflin · 2017-12-11T23:12:51Z

metricbeat/module/mongodb/collstats/collstats.go

+		return events, err
+	}
+
+	totals := result["totals"].(common.MapStr)


We should check there that this type assertion works by checking for the second return value: totals, ok := result["totals"].(common.MapStr) ...

ruflin · 2017-12-11T23:13:51Z

metricbeat/module/mongodb/collstats/collstats.go

+			continue
+		}
+
+		debugf("input=%v", info.(common.MapStr))


Was this mainly intended for development? Still needed? If yes, probably add some more text what this debug message is about.

ruflin · 2017-12-11T23:14:46Z

metricbeat/module/mongodb/collstats/collstats.go

+
+		debugf("input=%v", info.(common.MapStr))
+
+		events = append(events, eventMapping(group, info.(common.MapStr)))


One more type assertion we should check to guarantee we don't have a panic.

ruflin · 2017-12-11T23:15:34Z

metricbeat/module/mongodb/collstats/data.go

+	names := strings.SplitN(key, ".", 2)
+
+	event := common.MapStr{
+		"db":         names[0],


Let make sure the len of names is actually at least 0 so 0 and 1 exist.

ruflin · 2017-12-11T23:17:02Z

metricbeat/module/mongodb/collstats/data.go

+		"db":         names[0],
+		"collection": names[1],
+		"name":       key,
+		"statistics": common.MapStr{


Do we need here the statistics namespace? As this is already under mongodb.collstats I think we can put all the stats below directly on the same level.

andrewkroh

Should this metricset be called top? It looks like it uses the [top[(https://docs.mongodb.com/manual/reference/command/top/) command.

Based on ruflin's comments this needs a few field renames.

andrewkroh · 2017-12-15T16:49:34Z

metricbeat/module/mongodb/collstats/_meta/docs.asciidoc

@@ -0,0 +1 @@
+This is the `collstats` metricset of the module mongodb.


Could you add a bit of information about what information or commands that it uses to collect the data from mongodb. This might help someone familiar with mongodb to quickly understand what the metricset does.

andrewkroh · 2017-12-15T16:51:03Z

jenkins, test it

MerlinDMC · 2017-12-21T10:26:00Z

Sorry it took a while longer to get back to this.
I've changed the field naming a little further to include the microseconds hint as well.

@andrewkroh I would not rename it into top - I think it's easier to have a name of what you can expect as an output. Where here it will give you back collection level statistics.
If you want me to rename it so tell me and I will but this at least in my mind is easier to handle.

andrewkroh · 2017-12-28T03:59:31Z

metricbeat/module/mongodb/collstats/_meta/docs.asciidoc

@@ -0,0 +1,13 @@
+This is the `collstats` metricset of the module mongodb.
+
+It is using the `top` adminitrative command to return usage statistics for each collection. It provides the amount of time, in microseconds, used and acount of operations for the following types:


Thanks, I think this will be helpful.

"acount" looks like a typo. Can you fix that?

andrewkroh · 2018-01-02T16:17:25Z

jenkins, test it

ruflin · 2018-01-04T06:55:10Z

@MerlinDMC Any chance you could rebase this on master. We made some fixes for CI today.

MerlinDMC · 2018-01-04T09:48:44Z

@ruflin rebase done

ruflin · 2018-01-05T05:41:42Z

jenkins, test it

MerlinDMC · 2018-01-07T13:34:18Z

I've rebased again to resolve a merge conflict in the CHANGELOG.asciidoc file.

MerlinDMC · 2018-01-07T16:30:56Z

Looks like the master changes also fixed the CI runs 😄

houndci-bot reviewed Dec 10, 2017

View reviewed changes

MerlinDMC force-pushed the mongodb-collstats branch 4 times, most recently from 20970b0 to 36a04c4 Compare December 10, 2017 12:14

ruflin added Metricbeat Metricbeat review labels Dec 11, 2017

ruflin reviewed Dec 11, 2017

View reviewed changes

andrewkroh reviewed Dec 15, 2017

View reviewed changes

MerlinDMC force-pushed the mongodb-collstats branch from 36a04c4 to 93b3fa7 Compare December 21, 2017 10:22

andrewkroh reviewed Dec 28, 2017

View reviewed changes

MerlinDMC force-pushed the mongodb-collstats branch from 93b3fa7 to 317a525 Compare December 28, 2017 08:37

andrewkroh approved these changes Jan 2, 2018

View reviewed changes

ruflin added the module label Jan 4, 2018

MerlinDMC force-pushed the mongodb-collstats branch from 317a525 to 30eabfa Compare January 4, 2018 09:48

MerlinDMC added 3 commits January 7, 2018 13:33

Add mongodb collstats metricset

c458e0f

Include new collstats metricset

42fab32

Add CHANGELOG entry

a19022e

MerlinDMC force-pushed the mongodb-collstats branch from 30eabfa to a19022e Compare January 7, 2018 13:33

exekias added 2 commits January 8, 2018 08:47

Merge branch 'master' into mongodb-collstats

e16da7a

Merge branch 'master' into mongodb-collstats

4e266ce

exekias merged commit 1821662 into elastic:master Jan 8, 2018

MerlinDMC deleted the mongodb-collstats branch January 8, 2018 10:53

MerlinDMC mentioned this pull request Jan 14, 2018

Fix metricbeat mongodb collstats schema #6072

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metricbeat: MongoDB collstats metricset #5852

Metricbeat: MongoDB collstats metricset #5852

MerlinDMC commented Dec 10, 2017

elasticmachine commented Dec 10, 2017

houndci-bot Dec 10, 2017

houndci-bot Dec 10, 2017

MerlinDMC commented Dec 10, 2017

ruflin left a comment

ruflin Dec 11, 2017

ruflin Dec 11, 2017

ruflin Dec 11, 2017

ruflin Dec 11, 2017

ruflin Dec 11, 2017

ruflin Dec 11, 2017

andrewkroh left a comment

andrewkroh Dec 15, 2017

andrewkroh commented Dec 15, 2017

MerlinDMC commented Dec 21, 2017

andrewkroh Dec 28, 2017

andrewkroh commented Jan 2, 2018

ruflin commented Jan 4, 2018

MerlinDMC commented Jan 4, 2018

ruflin commented Jan 5, 2018

MerlinDMC commented Jan 7, 2018

MerlinDMC commented Jan 7, 2018


		debugf("input=%v", info.(common.MapStr))

		events = append(events, eventMapping(group, info.(common.MapStr)))

		@@ -0,0 +1 @@
		This is the `collstats` metricset of the module mongodb.

		@@ -0,0 +1,13 @@
		This is the `collstats` metricset of the module mongodb.

		It is using the `top` adminitrative command to return usage statistics for each collection. It provides the amount of time, in microseconds, used and acount of operations for the following types:

Metricbeat: MongoDB collstats metricset #5852

Metricbeat: MongoDB collstats metricset #5852

Conversation

MerlinDMC commented Dec 10, 2017

elasticmachine commented Dec 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MerlinDMC commented Dec 10, 2017

ruflin left a 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

andrewkroh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh commented Dec 15, 2017

MerlinDMC commented Dec 21, 2017

Choose a reason for hiding this comment

andrewkroh commented Jan 2, 2018

ruflin commented Jan 4, 2018

MerlinDMC commented Jan 4, 2018

ruflin commented Jan 5, 2018

MerlinDMC commented Jan 7, 2018

MerlinDMC commented Jan 7, 2018