[Metricbeat] Better document default metricsets #12648

fearful-symmetry · 2019-06-24T13:13:25Z

This is a change to better document the idea of 'default metricsets' and to add a section to each module asciidoc saying what the default metricsets are, as the logic is buried in code.

The reason this is a draft is, obviously, that there's a doc change for only one module. I figured before I ran this change on everything we should look it over and decide if we're happy with format/everything else.

I wrote a simple bash script to extract the default modules and add the blurb you see in the draft PR. Is this how we want to do it? Do we want to create some make/mage recipe?

exekias · 2019-06-24T15:13:58Z

Thank you for starting this!

Docs are currently collected by https://github.com/elastic/beats/blob/b99ff2b20e5385ba72013b513d3e48a29d9f94c5/metricbeat/scripts/docs_collector.py, while at some point we should move that logic to mage, this may make sense there?

fearful-symmetry · 2019-06-24T15:16:00Z

Ah, that's the thing that's run by make update or whatever. @exekias isn't there a meta-issue to remove all the python2 from everything? I would be more than happy to aid in that.

fearful-symmetry · 2019-06-24T15:17:39Z

The ideal way to do this, and I don't think it'll be too hard, is to have some mage rule that generates the docs and inserts it into the docs.asciidoc that each module has.

exekias · 2019-06-24T15:41:21Z

Yes, this is one of the TODOs in #10238, feel free to create and link an specific issue for that.

The ideal way to do this, and I don't think it'll be too hard, is to have some mage rule that generates the docs and inserts it into the docs.asciidoc that each module has.

These files are edited by the user, and never modified by make/mage as of today. How about doing that on docs/modules/<module>.asscidoc? That file is generated by docs_collector.py.

fearful-symmetry · 2019-06-24T15:46:13Z

How about doing that on docs/modules/.asscidoc? That file is generated by docs_collector.py.

@exekias That makes much more sense. I wonder if this is a good opportunity to just tear out the old python2 docs scripts and replace them while we're at it.

exekias · 2019-06-24T15:52:01Z

That would be awesome! as long as it doesn't get too complex. Another approach would be adding this to the existing script and create an issue to replace it with Go

dedemorton · 2019-06-24T18:14:49Z

metricbeat/module/zookeeper/_meta/docs.asciidoc

@@ -13,3 +13,9 @@ The ZooKeeper metricsets were tested with ZooKeeper 3.4.8 and are expected to wo
 The Zookeeper module comes with a predefined dashboard:

 image::./images/metricbeat-zookeeper.png[]
+
+== Default Metricsets
+The following Metricsets are default:


metricsets should be lowercase

fearful-symmetry · 2019-06-25T14:03:57Z

Another approach would be adding this to the existing script and create an issue to replace it with Go

That seems like it would be duplicating a lot of work? I've started digging into my mage stuff and I don't think it would be adding too much complexity to just port it over.

fearful-symmetry · 2019-07-29T19:35:59Z

Finally making headway on this now that we have two more PRs merged. Plan is still to have a separate go script that actually imports the registry code and generates the list, then to have the 'main' mage doc role actually generate the asciidoc.

fearful-symmetry · 2019-07-30T18:19:43Z

Okay, so, I pushed a PR that adds an msetlist go script that uses the registry API to gather a list of default metricsets and dumps it to JSON. This is called from the mage script and Unmarshalled. As mentioned earlier, this is done so compile-time errors in metricbeat doesn't completely bomb-out mage targets. I'm not sure if there's a more elegant solution to get the data from the go script into mage, but json in go is pretty clear.

Still undecided about how to actually document the default metricsets. This just adds a conditional line to the metricset-level docs. Do we want to add something to the module docs? Or the complete module list page?

dedemorton · 2019-07-30T18:33:22Z

metricbeat/docs/modules/aerospike/namespace.asciidoc

@@ -7,6 +7,7 @@ This file is generated! See scripts/mage/docs_collector.go

 include::../../../module/aerospike/namespace/_meta/docs.asciidoc[]

+This is a default metricset.


Will users know what we mean when we say that it is a default metricset? I wonder if it might be better to say something like "This metricset is enabled by default."

Yah, I was debating about the language here. In this case, default means "This metricset will be enabled if the module is not configured" which seems...subtly different? Maybe?

I think there's still some ambiguity though. Precise and concise are always at odds. :-)

Let's just leave it as you have it for now (This is a default metricset) and see if users are confused by the language.

We could also link to the page that explains what a default module is too.

jsoriano · 2019-07-31T09:42:05Z

metricbeat/scripts/msetlists/main.go

+	_ "github.com/elastic/beats/metricbeat/include"
+	"github.com/elastic/beats/metricbeat/mb"
+	_ "github.com/elastic/beats/x-pack/metricbeat/include"
+	xpackmb "github.com/elastic/beats/x-pack/metricbeat/mb"


Not sure if we can import code from x-pack in OSS code 🤔

Oh bah, I didn't even think of that. You're probably right. We need that to make sure we can add the light modules to the list. Is there a reason that code is in the x-pack dir? Are light modules going to be a basic-and-up thing?

It is not only about light modules, native modules in x-pack won't be listed if we don't include github.com/elastic/beats/x-pack/metricbeat/include.

fearful-symmetry · 2019-08-06T01:36:15Z

Okay, I put in a quick commit to demonstrate two scripts each for OSS and x-pack. Yes, the scripts are nearly identical, but they're also < 30 LoC before the license header.

metricbeat/scripts/msetlists/main.go

jsoriano · 2019-08-06T10:55:36Z

metricbeat/scripts/mage/docs_collector.go

+	}
+
+	return masterMap, nil
+}


Ok, let's go with this approach, it'd be similar to other x-pack scripts being called from OSS build scripts.

fearful-symmetry · 2019-08-06T19:12:27Z

Okay, this moves the bulk of the logic to a new little library. The actual main functions are just little 5-line shims now.

jsoriano

It seems this needs a mage update fmt.

metricbeat/scripts/mage/docs_collector.go

x-pack/metricbeat/scripts/msetlists/main.go

fearful-symmetry · 2019-08-14T17:03:10Z

Changed the wording a bit, since I liked @Titch990 's version. Lets see if CI is green now.

fearful-symmetry · 2019-08-14T17:04:40Z

Also, @Titch990 , Yah, there's tons of manually added metricset lists in the modules doc, and we need to figure out how to address those. We'd probably have to go through and remove the hand-written ones, then add the template code to do the lists in the generator. That's a big enough undertaking on its own, and we might wanna do it as a separate PR.

fearful-symmetry · 2019-08-15T01:10:16Z

So, I had to make some changes so this is bypassed if we're running inside a generator. I'm not sure if a metricbeat-custombeat can have a concept of "default metricsets" but that'll...probably have to be its own PR.

fearful-symmetry · 2019-08-15T01:20:07Z

jenkins, test this

jsoriano · 2019-08-15T14:53:46Z

metricbeat/scripts/mage/template/metricsetDoc.tmpl

@@ -11,6 +11,7 @@ This file is generated! See scripts/mage/docs_collector.go
 {{end -}}
 include::../../../{{.Mod.Path | basePath}}/{{.Mod.Base}}/{{.Metricset.Title}}/_meta/docs.asciidoc[]

+{{if .Metricset.IsDefault}}This is a default metricset. If the host module is unconfigured, this metricset is enabled by default.{{end}}


Nit. It should be possible to avoid the empty lines added when metricset is not default with something like this:

Suggested change

{{if .Metricset.IsDefault}}This is a default metricset. If the host module is unconfigured, this metricset is enabled by default.{{end}}

{{if .Metricset.IsDefault -}}

This is a default metricset. If the host module is unconfigured, this metricset is enabled by default.

{{end -}}

Uh, those newlines get me every time.

So, all this seems to do is remove the newline when the metricset is default.

Nevermind, it somehow removes two newlines as opposed to one.

fearful-symmetry · 2019-08-15T16:40:09Z

Okay, that should fix the newline issues.

fearful-symmetry · 2019-08-16T01:16:07Z

jenkins, test this

fearful-symmetry added docs Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Jun 24, 2019

fearful-symmetry requested review from dedemorton and a team June 24, 2019 13:13

fearful-symmetry self-assigned this Jun 24, 2019

andresrc added [zube]: Ready [zube]: In Review [zube]: In Progress and removed [zube]: Ready [zube]: In Review labels Jun 24, 2019

dedemorton reviewed Jun 24, 2019

View reviewed changes

fearful-symmetry mentioned this pull request Jun 28, 2019

[Packaging][Metricbeat] Port the docs_collector.py script to mage #12722

Merged

5 tasks

fearful-symmetry mentioned this pull request Jul 18, 2019

[metricbeat] Change behavior of Registry.Modules() to make it more consistant. #12972

Merged

fearful-symmetry force-pushed the document-default-metricsets branch from 0cdc11f to 5e15220 Compare July 30, 2019 18:06

dedemorton reviewed Jul 30, 2019

View reviewed changes

jsoriano reviewed Jul 31, 2019

View reviewed changes

jsoriano reviewed Aug 6, 2019

View reviewed changes

jsoriano reviewed Aug 7, 2019

View reviewed changes

metricbeat/scripts/mage/docs_collector.go Show resolved Hide resolved

x-pack/metricbeat/scripts/msetlists/main.go Show resolved Hide resolved

x-pack/metricbeat/scripts/msetlists/main.go Outdated Show resolved Hide resolved

fearful-symmetry added 13 commits August 14, 2019 11:56

add example of default metricset docs, and blurb on default metricsets

c18324e

start adding in default metricsets

a80ab7c

remove old zookeeper example

d3f6d1b

add lic leader

84e4c2c

create second mage script for x-pack

2073f4c

move bulk of code to a new lib

0bc6790

add header

a026cd7

move files around, fmt, cleanup

ca26c7b

change template language

0081429

change grammar and wording

9caf109

error handling, caps

672b2d0

update all docs

e2f76fb

update doc wording

9334a67

fearful-symmetry force-pushed the document-default-metricsets branch from 9668f21 to 9334a67 Compare August 14, 2019 17:02

bypass if we're not on metricbeat

3eadafb

jsoriano approved these changes Aug 15, 2019

View reviewed changes

fix newlines

347c965

jsoriano approved these changes Aug 15, 2019

View reviewed changes

fearful-symmetry merged commit 781453f into elastic:master Aug 16, 2019

zube bot added [zube]: Done and removed [zube]: In Progress labels Aug 16, 2019

jsoriano removed the [zube]: Done label Aug 19, 2019

This was referenced Nov 12, 2019

Add light modules of oss as source in docs collector #14483

Merged

Cherry-pick #14483 to 7.x: Add light modules of oss as source in docs collector #14502

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Metricbeat] Better document default metricsets #12648

[Metricbeat] Better document default metricsets #12648

fearful-symmetry commented Jun 24, 2019 •

edited by andresrc

Loading

exekias commented Jun 24, 2019

fearful-symmetry commented Jun 24, 2019

fearful-symmetry commented Jun 24, 2019

exekias commented Jun 24, 2019

fearful-symmetry commented Jun 24, 2019

exekias commented Jun 24, 2019

dedemorton Jun 24, 2019

fearful-symmetry commented Jun 25, 2019

fearful-symmetry commented Jul 29, 2019

fearful-symmetry commented Jul 30, 2019

dedemorton Jul 30, 2019

fearful-symmetry Jul 30, 2019

dedemorton Jul 30, 2019

fearful-symmetry Jul 31, 2019

jsoriano Jul 31, 2019 •

edited

Loading

fearful-symmetry Jul 31, 2019

jsoriano Aug 1, 2019

fearful-symmetry commented Aug 6, 2019

jsoriano Aug 6, 2019

fearful-symmetry commented Aug 6, 2019

jsoriano left a comment

fearful-symmetry commented Aug 14, 2019

fearful-symmetry commented Aug 14, 2019

fearful-symmetry commented Aug 15, 2019

fearful-symmetry commented Aug 15, 2019

jsoriano Aug 15, 2019

fearful-symmetry Aug 15, 2019

fearful-symmetry Aug 15, 2019

fearful-symmetry Aug 15, 2019

fearful-symmetry commented Aug 15, 2019

fearful-symmetry commented Aug 16, 2019

		@@ -7,6 +7,7 @@ This file is generated! See scripts/mage/docs_collector.go

		include::../../../module/aerospike/namespace/_meta/docs.asciidoc[]

		This is a default metricset.

[Metricbeat] Better document default metricsets #12648

[Metricbeat] Better document default metricsets #12648

Conversation

fearful-symmetry commented Jun 24, 2019 • edited by andresrc Loading

exekias commented Jun 24, 2019

fearful-symmetry commented Jun 24, 2019

fearful-symmetry commented Jun 24, 2019

exekias commented Jun 24, 2019

fearful-symmetry commented Jun 24, 2019

exekias commented Jun 24, 2019

Choose a reason for hiding this comment

fearful-symmetry commented Jun 25, 2019

fearful-symmetry commented Jul 29, 2019

fearful-symmetry commented Jul 30, 2019

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

jsoriano Jul 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fearful-symmetry commented Aug 6, 2019

Choose a reason for hiding this comment

fearful-symmetry commented Aug 6, 2019

jsoriano left a comment

Choose a reason for hiding this comment

fearful-symmetry commented Aug 14, 2019

fearful-symmetry commented Aug 14, 2019

fearful-symmetry commented Aug 15, 2019

fearful-symmetry commented Aug 15, 2019

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

fearful-symmetry commented Aug 15, 2019

fearful-symmetry commented Aug 16, 2019

fearful-symmetry commented Jun 24, 2019 •

edited by andresrc

Loading

jsoriano Jul 31, 2019 •

edited

Loading