Index metricset for elasticsearch Metricbeat module #6881

ruflin · 2018-04-17T09:38:05Z

This adds the index metricset to the Elasticsearch module. For now it only contains very basic metrics and is marked experimental.

This PR is used to create the basic for adding tests with the fetch reporter v2 interface and also generate data with it.

It also adds basic utility functions to fetch Master or Node information in the Elasticsearch module.

houndci-bot · 2018-04-17T09:38:16Z

metricbeat/module/elasticsearch/elasticsearch.go

+	return clusterStruct.MasterNode, nil
+}
+
+func GetInfo(http *helper.HTTP, uri string) (*Info, error) {


exported function GetInfo should have comment or be unexported

houndci-bot · 2018-04-17T09:38:16Z

metricbeat/module/elasticsearch/elasticsearch.go

+func GetClusterID(http *helper.HTTP, uri string, nodeID string) (string, error) {
+
+	// Check if cluster id already cached. If yes, return it.
+	if clusterId, ok := clusterIDCache[nodeID]; ok {


var clusterId should be clusterID

houndci-bot · 2018-04-17T09:38:16Z

metricbeat/module/elasticsearch/elasticsearch.go

+// Global clusterIdCache. Assumption is that the same node id never can belong to a different cluster id
+var clusterIDCache = map[string]string{}
+
+type Info struct {


exported type Info should have comment or be unexported

houndci-bot · 2018-04-17T09:38:16Z

metricbeat/mb/testing/modules.go

+	return r.events
+}
+
+func (r *CapturingReporterV2) GetErrors() []error {


exported method CapturingReporterV2.GetErrors should have comment or be unexported

houndci-bot · 2018-04-17T09:38:17Z

metricbeat/mb/testing/modules.go

 	r.errs = append(r.errs, err)
 	return true
 }

+func (r *CapturingReporterV2) GetEvents() []mb.Event {


exported method CapturingReporterV2.GetEvents should have comment or be unexported

houndci-bot · 2018-04-17T09:38:17Z

metricbeat/mb/testing/modules.go

 	events []mb.Event
 	errs   []error
 }

-func (r *capturingReporterV2) Event(event mb.Event) bool {
+func (r *CapturingReporterV2) Event(event mb.Event) bool {


exported method CapturingReporterV2.Event should have comment or be unexported

houndci-bot · 2018-04-17T09:38:17Z

metricbeat/mb/testing/modules.go

@@ -151,25 +151,33 @@ func NewReportingMetricSetV2(t testing.TB, config interface{}) mb.ReportingMetri
 	return reportingMetricSetV2
 }

-type capturingReporterV2 struct {
+type CapturingReporterV2 struct {


exported type CapturingReporterV2 should have comment or be unexported

houndci-bot · 2018-04-17T09:38:17Z

metricbeat/mb/testing/data_generator.go

@@ -57,6 +57,28 @@ func WriteEvents(f mb.EventsFetcher, t testing.TB) error {
 	return nil
 }

+// WriteEvents fetches events and writes the first event to a ./_meta/data.json


comment on exported function WriteEventsReporterV2 should be of the form "WriteEventsReporterV2 ..."

exekias

This is a great addition to the module, I left some minor comments/questions

exekias · 2018-04-17T15:26:50Z

metricbeat/metricbeat.reference.yml

+  metricsets:
+   #- "index"
+   - "node"
+   - "node_stats"


you can drop " chars here

exekias · 2018-04-17T15:31:01Z

metricbeat/module/elasticsearch/index/data.go

+}
+
+func eventMapping(node map[string]interface{}) common.MapStr {
+	event, _ := schema.Apply(node)


Perhaps we want to stop/warn on error here?

The error handling is partially broken here. We skip it in all the other modules too. We should revisit how we do error handling during parsing events.

exekias · 2018-04-17T15:33:26Z

metricbeat/module/elasticsearch/elasticsearch.go

+}
+
+func getNodeName(http *helper.HTTP, uri string) (string, error) {
+


There are a few methods with a starting empty line, it would be nice to remove them

removed all of them.

exekias · 2018-04-17T15:35:00Z

metricbeat/module/elasticsearch/index/index.go

+	}
+
+	// Not master, no event sent
+	if !isMaster {


I wonder what will be the behavior when monitoring from outside the cluster? for instance a cluster in Elastic Cloud

This will not work properly at the moment. We will have to introduce additional config options for this. See also discussion here: #6807 (comment)

I'm thinking that these will potentially be module config options which apply to all metricsets but I'm not 100% sure yet on what the exact behaviour / options will be so I ignored it for now.

houndci-bot · 2018-04-18T08:01:41Z

metricbeat/module/elasticsearch/testing.go

+	return port
+}
+
+func GetConfig24(metricset string) map[string]interface{} {


exported function GetConfig24 should have comment or be unexported

houndci-bot · 2018-04-18T08:01:41Z

metricbeat/module/elasticsearch/testing.go

+	return host
+}
+
+func GetEnvPort24() string {


exported function GetEnvPort24 should have comment or be unexported

houndci-bot · 2018-04-18T08:01:41Z

metricbeat/module/elasticsearch/testing.go

@@ -27,3 +27,29 @@ func GetConfig(metricset string) map[string]interface{} {
 		"hosts":      []string{GetEnvHost() + ":" + GetEnvPort()},
 	}
 }
+
+func GetEnvHost24() string {


exported function GetEnvHost24 should have comment or be unexported

exekias

WFG

exekias · 2018-04-19T11:08:36Z

Tests are failing, I think it's related to ELASTICSEARCH_HOST env vars

This adds the index metricset to the Elasticsearch module. For now it only contains very basic metrics and is marked experimental. This PR is used to create the basic for adding tests with the fetch reporter v2 interface and also generate data with it. It also adds basic utility functions to fetch Master or Node information in the Elasticsearch module.

ruflin · 2018-04-19T15:09:58Z

metricbeat/module/elasticsearch/node_stats/_meta/docs.asciidoc

@@ -1,5 +1,3 @@
-=== Elasticsearch node_stats metricset


This is cleanup and should not have been here. First though was this cause the doc build issue.

ph · 2018-04-19T15:13:01Z

metricbeat/docs/modules/elasticsearch/index.asciidoc

+==== Fields
+
+For a description of each field in the metricset, see the
+<<exported-fields-elasticsearch,exported fields>> section.


This ID doesn't to exist exported-fields-elasticsearch?

weird, as it is here: https://github.com/elastic/beats/pull/6881/files#diff-d43087d796b8b1d1ff48ec3561aeda60R3286

@exekias I really think the error here is related to how our tests are written, not the doc source. I tested the files offline and they build without errors. I think it's safe to ignore the error messages here.

exekias · 2018-04-19T15:42:08Z

jenkins, retest this please

… the index.asciidoc metricset doc.

exekias

Ready for merge

ruflin added in progress Pull request is currently in progress. module review Metricbeat Metricbeat labels Apr 17, 2018

houndci-bot reviewed Apr 17, 2018

View reviewed changes

exekias reviewed Apr 17, 2018

View reviewed changes

houndci-bot reviewed Apr 18, 2018

View reviewed changes

ruflin force-pushed the es-index-metricset branch from 0303367 to d826677 Compare April 19, 2018 10:17

ruflin removed the in progress Pull request is currently in progress. label Apr 19, 2018

exekias approved these changes Apr 19, 2018

View reviewed changes

ruflin force-pushed the es-index-metricset branch from 5cc0087 to 70d26a4 Compare April 19, 2018 14:10

ruflin force-pushed the es-index-metricset branch from 70d26a4 to 1581858 Compare April 19, 2018 14:26

ruflin commented Apr 19, 2018

View reviewed changes

ph reviewed Apr 19, 2018

View reviewed changes

Set max depth of index.asciidoc search to 1 as otherwise it took also…

32cb62a

… the index.asciidoc metricset doc.

exekias approved these changes Apr 20, 2018

View reviewed changes

ruflin added 2 commits April 20, 2018 15:47

Update CHANGELOG.asciidoc

ef53fba

Merge branch 'master' into es-index-metricset

cc74589

exekias merged commit 7db33f2 into elastic:master Apr 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index metricset for elasticsearch Metricbeat module #6881

Index metricset for elasticsearch Metricbeat module #6881

ruflin commented Apr 17, 2018 •

edited

Loading

houndci-bot Apr 17, 2018

houndci-bot Apr 17, 2018

houndci-bot Apr 17, 2018

houndci-bot Apr 17, 2018

houndci-bot Apr 17, 2018

houndci-bot Apr 17, 2018

houndci-bot Apr 17, 2018

houndci-bot Apr 17, 2018

exekias left a comment

exekias Apr 17, 2018

ruflin Apr 18, 2018

exekias Apr 17, 2018

ruflin Apr 18, 2018

exekias Apr 17, 2018

ruflin Apr 18, 2018

exekias Apr 17, 2018

ruflin Apr 18, 2018

houndci-bot Apr 18, 2018

houndci-bot Apr 18, 2018

houndci-bot Apr 18, 2018

exekias left a comment

exekias commented Apr 19, 2018

ruflin Apr 19, 2018

ph Apr 19, 2018

exekias Apr 19, 2018

dedemorton Apr 19, 2018

exekias commented Apr 19, 2018

exekias left a comment

		}

		func getNodeName(http *helper.HTTP, uri string) (string, error) {

Index metricset for elasticsearch Metricbeat module #6881

Index metricset for elasticsearch Metricbeat module #6881

Conversation

ruflin commented Apr 17, 2018 • edited 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

exekias 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

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

exekias left a comment

Choose a reason for hiding this comment

exekias commented Apr 19, 2018

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

exekias commented Apr 19, 2018

exekias left a comment

Choose a reason for hiding this comment

ruflin commented Apr 17, 2018 •

edited

Loading