Add Beats state reporting #7075

ruflin · 2018-05-11T07:59:16Z

Currently in monitoring or the API endpoint it's not visible which modules are running. This PR adds and endpoint /state which shows the following data:

{
  "module": {
    "count": 3,
    "names": [
      "system"
    ]
  }
}

The names are a unique list of module names that are running and the count is a total of all the modules that are running. In the above case it's 3 as the system module is started by default 3 times with different configs / periods. Currently this reporting only works for Metricbeat.

The same state metrics are reported to X-Pack Monitoring. The data is reported every minute by default.

Additional changes:

Introduce ArraySlice support in metrics
Improve namespace handling to prevent races
Add monitoring reporter for state endpoint
Add config options for reporting frequency. The old config option was removed. I don't consider this a breaking change as the old option was never document and not intended to be changed.

houndci-bot · 2018-05-11T07:59:36Z

libbeat/monitoring/visitor_kv.go

+func (vs *KeyValueVisitor) OnBool(b bool)            { vs.cb(vs.getName(), b) }
+func (vs *KeyValueVisitor) OnNil()                   { vs.cb(vs.getName(), nil) }
+func (vs *KeyValueVisitor) OnInt(i int64)            { vs.cb(vs.getName(), i) }
+func (vs *KeyValueVisitor) OnFloat(f float64)        { vs.cb(vs.getName(), f) }


exported method KeyValueVisitor.OnFloat should have comment or be unexported

houndci-bot · 2018-05-11T07:59:36Z

libbeat/monitoring/visitor_kv.go

+func (vs *KeyValueVisitor) OnString(s string)        { vs.cb(vs.getName(), s) }
+func (vs *KeyValueVisitor) OnBool(b bool)            { vs.cb(vs.getName(), b) }
+func (vs *KeyValueVisitor) OnNil()                   { vs.cb(vs.getName(), nil) }
+func (vs *KeyValueVisitor) OnInt(i int64)            { vs.cb(vs.getName(), i) }


exported method KeyValueVisitor.OnInt should have comment or be unexported

houndci-bot · 2018-05-11T07:59:36Z

libbeat/monitoring/visitor_kv.go

-func (vs *KeyValueVisitor) OnFloat(f float64) { vs.cb(vs.getName(), f) }
+func (vs *KeyValueVisitor) OnString(s string)        { vs.cb(vs.getName(), s) }
+func (vs *KeyValueVisitor) OnBool(b bool)            { vs.cb(vs.getName(), b) }
+func (vs *KeyValueVisitor) OnNil()                   { vs.cb(vs.getName(), nil) }


exported method KeyValueVisitor.OnNil should have comment or be unexported

houndci-bot · 2018-05-11T07:59:37Z

libbeat/monitoring/visitor_kv.go

-func (vs *KeyValueVisitor) OnInt(i int64)     { vs.cb(vs.getName(), i) }
-func (vs *KeyValueVisitor) OnFloat(f float64) { vs.cb(vs.getName(), f) }
+func (vs *KeyValueVisitor) OnString(s string)        { vs.cb(vs.getName(), s) }
+func (vs *KeyValueVisitor) OnBool(b bool)            { vs.cb(vs.getName(), b) }


exported method KeyValueVisitor.OnBool should have comment or be unexported

houndci-bot · 2018-05-11T07:59:37Z

libbeat/monitoring/visitor_kv.go

-func (vs *KeyValueVisitor) OnNil()            { vs.cb(vs.getName(), nil) }
-func (vs *KeyValueVisitor) OnInt(i int64)     { vs.cb(vs.getName(), i) }
-func (vs *KeyValueVisitor) OnFloat(f float64) { vs.cb(vs.getName(), f) }
+func (vs *KeyValueVisitor) OnString(s string)        { vs.cb(vs.getName(), s) }


exported method KeyValueVisitor.OnString should have comment or be unexported

houndci-bot · 2018-05-11T07:59:37Z

libbeat/monitoring/visitor.go

@@ -50,3 +51,8 @@ func ReportFloat(V Visitor, name string, value float64) {
 	V.OnKey(name)
 	V.OnFloat(value)
 }
+
+func ReportStringSlice(V Visitor, name string, value []string) {


exported function ReportStringSlice should have comment or be unexported

houndci-bot · 2018-05-11T07:59:37Z

libbeat/monitoring/namespace.go

+	}
+}
+
+func (n *Namespaces) Get(key string) *Namespace {


exported method Namespaces.Get should have comment or be unexported

houndci-bot · 2018-05-11T07:59:37Z

libbeat/monitoring/namespace.go

+	namespaces map[string]*Namespace
+}
+
+func NewNamespaces() *Namespaces {


exported function NewNamespaces should have comment or be unexported

houndci-bot · 2018-05-11T07:59:37Z

libbeat/monitoring/namespace.go

@@ -37,3 +35,25 @@ func (n *Namespace) GetRegistry() *Registry {
 	}
 	return n.registry
 }
+
+type Namespaces struct {


exported type Namespaces should have comment or be unexported

ruflin · 2018-05-11T08:04:50Z

@pickypg @tsullivan I think we should have a different reporting interval for the beat state. Any recommendations on the interval?

exekias

This is going to be a great source of info, It may be especially useful to debug autodiscover status at some point 🎉

exekias · 2018-05-11T09:24:26Z

metricbeat/mb/module/runner.go

+}
+
+func reportModules(m monitoring.Mode, V monitoring.Visitor) {
+


empty line here

exekias · 2018-05-11T09:25:36Z

libbeat/api/server.go

@@ -24,6 +24,7 @@ func Start(cfg *common.Config) {

 		// register handlers
 		mux.HandleFunc("/", rootHandler())
+		mux.HandleFunc("/state", stateHandler)


I wonder if it would make more sense to call this /modules, do you plan to add anything else?

I'm thinking of also adding information about the datasets, the output used. But not sure where it will evolve into. We probably need soon a more general discussion about api endpoint naming and structure.

SGTM, I guess we don't have a hard commit on these names, so we can change them later if needed (?)

yes, API endpoints are not documented so far and we can still change them.

houndci-bot · 2018-05-14T08:18:10Z

libbeat/monitoring/visitor_kv.go

+func (vs *KeyValueVisitor) OnNil()                   { vs.cb(vs.getName(), nil) }
+func (vs *KeyValueVisitor) OnInt(i int64)            { vs.cb(vs.getName(), i) }
+func (vs *KeyValueVisitor) OnFloat(f float64)        { vs.cb(vs.getName(), f) }
+func (vs *KeyValueVisitor) OnStringSlice(f []string) { vs.cb(vs.getName(), f) }


exported method KeyValueVisitor.OnStringSlice should have comment or be unexported

ruflin · 2018-05-14T08:20:03Z

libbeat/monitoring/report/elasticsearch/elasticsearch.go

@@ -217,6 +216,7 @@ func (r *reporter) snapshotLoop() {
 		r.client.Publish(beat.Event{
 			Timestamp: ts,
 			Fields:    fields,
+			Meta:      common.MapStr{"type": "beats_" + namespace, "interval_ms": period * time.Millisecond},


@pickypg I'm trying to work around the requirement that the interval has to be set as a request param and instead have it directly as part of the event. The reasons is that I try to reuse the the elasticsearch client / reporter and we set the params only once on setup and cannot pass them dynamically. I can modify the code to have a client for each reporting type but was curious if there is a work around already today in the xpack endpoint?

Unfortunately not. The parameter is required to be passed as a querystring field.

ruflin · 2018-05-14T12:21:37Z

NOTE: I'm ignoring HoundCI on the very short interface methods on purpose.

pickypg · 2018-05-16T15:04:15Z

I think we should have a different reporting interval for the beat state. Any recommendations on the interval?

Perhaps once every 5 minutes?

ruflin · 2018-05-29T06:27:49Z

libbeat/monitoring/report/elasticsearch/elasticsearch.go

+			})
+	}
+
+	statsPipeline, err := getPipeline("stats", config.StatsPeriod)


@urso This code works but is not very nice. The reason I have to define 2 different pipelines is because the interval params is different and the monitoring registry conflicts. I would prefer to have only 1 pipeline but be able to set the params on request. Are there other options?

ruflin · 2018-06-04T11:20:38Z

@urso Ready for a round of reviews.

@pickypg I selected 1m as default as I felt only 5 minutes is too low in fast changing environments. But it's a tricky one. The good thing is at the moment there is not much data. Perhaps we have to reconsider later.

ruflin · 2018-06-12T06:19:31Z

libbeat/monitoring/namespace.go


-var namespaces = struct {


@jsoriano This code change as I also already had it change in this PR. Let me now if my implementation also looks good to you.

urso · 2018-06-12T15:34:11Z

libbeat/monitoring/report/elasticsearch/client.go

+					params[k] = v
+				}
+			}
+		}


How about us copying/allocating contents if required only?

This loop rebuilds params every single time, for every event. If the batch contains multiple events, with only one event having params set, we might loose some params again.

Is c.params and params via 'meta' always configured? If not, we can reduce some copying here:

eventParams := func(meta common.MapStr) map[string]string { tmp, err := event.Content.Meta.GetValue("params") if err != nil { return nil } p, ok := tmp.(map[string]string) if !ok { return nil } return p } cpyKV := func(to map[string]string, from map[string]string) { for k, v := range from { to[k] = v } } params = c.params if userParams := eventParams(event.Content.Meta); len(userParams) > 0 { if len(c.params) > 0 { params = make(map[string]string, len(params) + len(userParams)) cpyKV(params, c.params) cpyKV(params, userParams) } else { params = userParams } }

As each event contains the interval param in meta and it's required at the moment, I think we have to reprocess it every time.

What do you mean by we loose params? As it's reprocessed every time and we don't modify c.params, all "base" params should always be there?

urso

tl'dr: this PR might break x-pack monitoring.

The HTTP parameters are global, while this PR tries to set parameters per event. As intervals of the different reporters overlap, the HTTP parameters will eventually be filled by one or the other reporter at some point in time. Plus params are reset on every event being processed in a batch.

For this to work, we either need to ensure some grouping by common event parameters. Either by:

having a client/pipeline per 'loop'
or by doing some more grouping in the client, such that multiple bulk requests will
do not use bulk API or do bulk API request per event
better, don't have monitoring enforce global parameters, while in reality we want to provide per event parameters/meta-data

urso · 2018-06-12T15:54:30Z

libbeat/_meta/config.reference.yml

@@ -982,6 +982,9 @@ logging.files:
  # never, once, and freely. Default is never.
  #ssl.renegotiation: never

+  #stats.period: 10s
+  #state.period: 1m


stats/state? I wonder how many users will get confused by this (or how many typos we will see). How about metrics.period and state.period?

ruflin · 2018-06-13T06:46:10Z

@pickypg It seems because of the interval param that we have to send each type separately and can't send it in one bulk request. I wonder if moving forward we could make the interval query param optional and instead also accept it as part of the event. Like this bulk requests would work as expected.

ruflin · 2018-06-20T10:30:57Z

@urso This should be ready for an other round of reviews.

urso · 2018-06-20T10:48:49Z

libbeat/monitoring/report/elasticsearch/client.go

-	bulk := make([]interface{}, 0, 2*len(events))
+	var failed []publisher.Event
+	var reason error
+	var params map[string]string


we still need the global 'params' variable? It's re-initialized in the loop for every bulk request.

no, this resolved also my comment above.

ruflin · 2018-06-25T10:44:09Z

heartbeat/heartbeat.reference.yml

@@ -1218,6 +1218,9 @@ logging.files:
  # never, once, and freely. Default is never.
  #ssl.renegotiation: never

+  #metrics.period: 10s


@urso I have second thoughts if we even want to add this to the config file, meaning should we even recommend users to change it?

It's the reference config. I don't really see a problem in configuring the interval. Allows users to reduce number of documents for monitoring if required.

ruflin · 2018-06-26T07:06:13Z

@urso Ready for an other round.

I will follow up with the ES team to potentially change the requirement on the xpack _bulk endpoint to simplify this implementation.

ruflin · 2018-07-03T08:45:19Z

As soon as this PR is merged, the monitoring template in Elasticsearch has to be updated.

Currently in monitoring or the API endpoint it's not visible which modules are running. This PR adds and endpoint `/state` which shows the following data: ``` { "module": { "count": 3, "names": [ "system" ] } } ``` The names are a unique list of module names that are running and the count is a total of all the modules that are running. In the above case it's 3 as the system module is started by default 3 times with different configs / periods. Currently this reporting only works for Metricbeat. The same state metrics are reported to X-Pack Monitoring. The data is reported every minute by default. Additional changes: * Introduce ArraySlice support in metrics * Improve namespace handling to prevent races * Add monitoring reporter for state endpoint * Add config options for reporting frequency. The old config option was removed. I don't consider this a breaking change as the old option was never document and not intended to be changed.

ruflin · 2018-07-05T07:42:47Z

PR for template can be found here: elastic/elasticsearch#31809

With elastic/beats#7075 Beats introduces state reporting for X-Pack Monitoring. The data sent up to Elasticsearch ends up stored in the following format. ``` "beats_state": { "timestamp": "2018-07-05T07:21:03.581Z", "state": { "module": { "count": 1, "names": [ "http" ] } }, "beat": { "uuid": "594039b5-6353-4d78-9bad-778ecc0fe83f", "type": "metricbeat", "version": "7.0.0-alpha1", "name": "ruflin", "host": "ruflin" } } ``` This PR adds the new fields to the template.

…1809) (#31893) With elastic/beats#7075 Beats introduces state reporting for X-Pack Monitoring. The data sent up to Elasticsearch ends up stored in the following format. ``` "beats_state": { "timestamp": "2018-07-05T07:21:03.581Z", "state": { "module": { "count": 1, "names": [ "http" ] } }, "beat": { "uuid": "594039b5-6353-4d78-9bad-778ecc0fe83f", "type": "metricbeat", "version": "7.0.0-alpha1", "name": "ruflin", "host": "ruflin" } } ``` This PR adds the new fields to the template.

With elastic/beats#7075 Beats introduces state reporting for X-Pack Monitoring. The data sent up to Elasticsearch ends up stored in the following format. ``` "beats_state": { "timestamp": "2018-07-05T07:21:03.581Z", "state": { "module": { "count": 1, "names": [ "http" ] } }, "beat": { "uuid": "594039b5-6353-4d78-9bad-778ecc0fe83f", "type": "metricbeat", "version": "7.0.0-alpha1", "name": "ruflin", "host": "ruflin" } } ``` This PR adds the new fields to the template.

…1809) With elastic/beats#7075 Beats introduces state reporting for X-Pack Monitoring. The data sent up to Elasticsearch ends up stored in the following format. ``` "beats_state": { "timestamp": "2018-07-05T07:21:03.581Z", "state": { "module": { "count": 1, "names": [ "http" ] } }, "beat": { "uuid": "594039b5-6353-4d78-9bad-778ecc0fe83f", "type": "metricbeat", "version": "7.0.0-alpha1", "name": "ruflin", "host": "ruflin" } } ``` This PR adds the new fields to the template.

ruflin added in progress Pull request is currently in progress. review libbeat Metricbeat Metricbeat labels May 11, 2018

houndci-bot reviewed May 11, 2018

View reviewed changes

exekias reviewed May 11, 2018

View reviewed changes

elastic deleted a comment from houndci-bot May 14, 2018

houndci-bot reviewed May 14, 2018

View reviewed changes

ruflin commented May 14, 2018

View reviewed changes

ruflin force-pushed the stringarray branch from 882725d to 44260cf Compare May 29, 2018 06:25

ruflin commented May 29, 2018

View reviewed changes

ruflin force-pushed the stringarray branch 3 times, most recently from ab6edff to 1c27307 Compare June 4, 2018 11:18

ruflin removed the in progress Pull request is currently in progress. label Jun 4, 2018

ruflin force-pushed the stringarray branch from 1c27307 to bf56e11 Compare June 12, 2018 06:18

ruflin commented Jun 12, 2018

View reviewed changes

urso reviewed Jun 12, 2018

View reviewed changes

urso suggested changes Jun 12, 2018

View reviewed changes

urso reviewed Jun 12, 2018

View reviewed changes

ruflin force-pushed the stringarray branch 2 times, most recently from 1e4d3e7 to e76ecc2 Compare June 20, 2018 07:36

urso reviewed Jun 20, 2018

View reviewed changes

ruflin force-pushed the stringarray branch from e76ecc2 to 7693a1e Compare June 25, 2018 10:06

ruflin commented Jun 25, 2018

View reviewed changes

ruflin force-pushed the stringarray branch from c85e990 to 8daf678 Compare June 26, 2018 07:05

ruflin force-pushed the stringarray branch from 8daf678 to 25d2e75 Compare July 3, 2018 08:54

ruflin added 5 commits July 3, 2018 11:43

rename to metrics.period and implement one bulk per event.

ee4fbbf

rename to metrics

6e920e1

local params

334ec1b

fix invalid units

7f5b6ff

ruflin force-pushed the stringarray branch from 25d2e75 to 7f5b6ff Compare July 3, 2018 09:43

urso approved these changes Jul 4, 2018

View reviewed changes

urso merged commit 0db2369 into elastic:master Jul 4, 2018

ruflin mentioned this pull request Jul 5, 2018

Add template config for Beat state to X-Pack Monitoring elastic/elasticsearch#31809

Merged

ruflin deleted the stringarray branch July 5, 2018 07:42

ruflin mentioned this pull request Jul 5, 2018

Implement state reporting for Beats #7521

Closed

9 tasks

ruflin mentioned this pull request Jul 9, 2018

Backport: Add template config for Beat state to X-Pack Monitoring (#31809) elastic/elasticsearch#31893

Merged

		}

		func reportModules(m monitoring.Mode, V monitoring.Visitor) {

Add Beats state reporting #7075

Add Beats state reporting #7075

Conversation

ruflin commented May 11, 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

Choose a reason for hiding this comment

ruflin commented May 11, 2018

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

ruflin commented May 14, 2018

pickypg commented May 16, 2018

Choose a reason for hiding this comment

ruflin commented Jun 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

urso Jun 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

urso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Jun 13, 2018

ruflin commented Jun 20, 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

ruflin commented Jun 26, 2018

ruflin commented Jul 3, 2018

ruflin commented Jul 5, 2018

ruflin commented May 11, 2018 •

edited

Loading

urso Jun 12, 2018 •

edited

Loading