Report kibana_settings to X-Pack Monitoring #7664

ycombinator · 2018-07-20T18:06:30Z

Resolves #7621.

X-Pack Monitoring of Kibana requires two types of documents in the .monitoring-kibana-* indices: kibana_stats and kibana_settings. We made Metricbeat's kibana/stats metricset index kibana_stats documents into .monitoring-kibana-* in #7525. This PR makes the same metricset index kibana_settings documents into .monitoring-kibana-*.

To test this PR

The idea is that metricbeat (specifically the kibana/stats metricset with xpack.enabled: true) will create exactly the same documents in .monitoring-kibana-* indices as Kibana's bulk uploader does today.

Start up Elasticsearch (with the default Basic license is okay)
Start up Kibana (with the default Basic license is okay), checked out from master.
Enable Monitoring in Elasticsearch (via the cluster setting xpack.monitoring.collection.enabled: true). You can do this via Elasticsearch's Cluster Update Settings API or by clicking the "Turn on Monitoring" button in the Monitoring UI in Kibana.
Let Kibana run for ~20 seconds so a few documents are indexed into .monitoring-kibana-*.
Stop Kibana
From .monitoring-kibana-*, retrieve a document for type = kibana_settings
Delete the .monitoring-kibana-* indices.
In kibana.yml, set xpack.monitoring.kibana.collection.enabled: false. This will ensure that Kibana is no longer directly shipping its monitoring metrics to Elasticsearch.
Start up Kibana again, this time checked out from [Monitoring] Kibana settings api kibana#21100.
Enable the kibana module in metricbeat: ./metricbeat modules enable kibana.
In modules.d/kibana.yml, add the stats metricset and set xpack.enabled: true. Concretely, your modules.d/kibana.yml should look something like this:
```
- module: kibana
   metricsets:
   - stats
   period: 10s
   hosts: ["localhost:5601"]
   #basepath: ""
   #username: "user"
   #password: "secret"
   xpack.enabled: true
```
Additionally, if your Kibana URLs have a basepath prefix, say foo, make sure to un-comment the basepath setting in the kibana module YAML file and set it's value to "foo".
Start metricbeat.
Let metricbeat run for ~20 seconds so a few documents are indexed into .monitoring-kibana-*.
Stop metricbeat
From .monitoring-kibana-*, retrieve a document for type = kibana_settings.
Using a tool such as http://www.jsondiff.com, compare the documents indexed by Kibana's with those indexed by metricbeat. Verify that their structures are identical (same fields, not necessarily same values), except for these known and expected differences:
1. Only Metricbeat-indexed documents are expected to contain the fields @timestamp, beat, host, and metricset. These are "standard" fields added by beats and metricbeat and don't have an adverse impact since they are additive.
2. Only Kibana-indexed documents are expected to contain the field source_node. This field is used for debugging purposes only and not actually consumed by either the Monitoring UI or Telemetry feature in Kibana.

ruflin · 2018-07-23T07:18:01Z

metricbeat/module/kibana/stats/stats.go

@@ -34,18 +37,21 @@ func init() {
 }

 var (
-	hostParser = parse.URLHostParserBuilder{
+	statsPath    = "api/stats"


What about making these 2 a const?

ruflin · 2018-07-23T07:21:14Z

metricbeat/module/kibana/stats/stats.go

+	now := time.Now()
+
+	m.fetchStats(r, intervalMs, now)
+	m.fetchSettings(r, intervalMs, now)


I would prefer to have the XPack check directly here instead of in the fetchSettings method below. The reason is that for a read it would make it directly clear here that it is only called if XPack is enabled.

ruflin · 2018-07-23T07:23:53Z

metricbeat/module/kibana/stats/_meta/test/settings.700.json

@@ -0,0 +1,18 @@
+{


Could you also add a test file for 6.3 or 6.4?

The settings API in Kibana is not expected to exist until 6.5. Once it has been implemented for 6.5, I can add a settings.650.json file here. Should I do that?

Is your concern about how Metricbeat >= 6.5.0 will react when running with Kibana < 6.5.0?

Yes. Seems like we should get the version number of KB on the first request and make decisions based on it.

Actually I have a question about this. Prior to 6.4.0 there is no GET /api/stats in Kibana either. Does that mean we should be checking the version of Kibana before making that call too? [EDIT] This would require yet another Kibana API call, to GET /api/status first.

Yes, I think we will need such a check or at least provide a good error message.

I think we could check the version once during New and just assume it stays the same over the lifetime of the metricset for now. So we would only do it once.

I'm going to add this version check in New in a separate PR since it affects code that has already been merged into master (#7525). I will rebase this PR on top of my new PR.

Version check PR: #7697

Implemented version check for Kibana settings API in 85b2438.

ycombinator · 2018-08-07T20:47:59Z

@ruflin The Kibana PR that this PR was dependent upon has now been merged. So this PR is ready for a final review/merge.

ruflin · 2018-08-08T08:31:18Z

NOTICE.txt

@@ -171,6 +171,21 @@ COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
 IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
 CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

+--------------------------------------------------------------------
+Dependency: github.com/coreos/go-semver


Is this package still somewhere used?

It shouldn't be any more. This section probably got re-introduced during a rebase or something. I will remove it. Thanks for catching!

ruflin · 2018-08-08T08:33:22Z

metricbeat/module/kibana/stats/data_xpack.go

+	var data map[string]interface{}
+	err := json.Unmarshal(content, &data)
+	if err != nil {
+		r.Error(err)


Should we report an error in x-pack or just ignore it? I would opt for ignoring for now as it ends up in an other index and we will not make use of it.

ruflin · 2018-08-08T08:36:01Z

metricbeat/module/kibana/stats/stats.go

 	if err != nil {
 		return nil, err
 	}

-	kibanaVersion, err := kibana.GetVersion(http, statsPath)
+	kibanaVersion, err := kibana.GetVersion(statsHTTP, statsPath)


One thing I just realised that bugs me a bit with this implementation is that it requires Kibana to be up and running normally when Metricbeat is started. Assuming Kibana is flaky during the moment the check happens, the metricset will not be running but error out even though Kibana could be totally fine again 1 sec later.

Not for this PR but thought worth mentioning.

Good point. I suppose we could move this check (and the code dependent on it) into Fetch() but that might be too expensive to run every fetch cycle?

Maybe we need another metricset lifecycle method in addition to New() and Fetch(), say Ping() (I know, not a great name), which runs periodically but less frequently than Fetch(). The frequency can be defined per-metricset in that metricset's New() function.

By default this Ping() method would do nothing. A metricset could, however, override it. The code in Ping() could then set fields in the metricset's struct so the code in Fetch() can use the data in those fields.

In the case of this metricset, we would move the Kibana version checking (and dependent) code into Ping().

Thoughts? I agree that this would be beyond the scope of this PR but, if you like it, I can make a separate issue/followup PR for it.

What about using fetch but do it only once? I remember we do something like that in other cases.

Merged as it's something we should discuss as a follow up.

ruflin · 2018-08-08T08:38:00Z

metricbeat/module/kibana/stats/stats.go

@@ -101,17 +131,35 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 // It returns the event which is then forward to the output. In case of an error, a
 // descriptive error must be returned.
 func (m *MetricSet) Fetch(r mb.ReporterV2) {
-	content, err := m.http.FetchContent()
+	intervalMs := m.Module().Config().Period.Nanoseconds() / 1000 / 1000


Could we have this code inside the x-pack part as it's only used there I think? Perhaps we can find a way to only if that xPackEnabled check once and run instead also in fetchStats. Why not move this to line 151 with the fetchSettings?

I'm not sure I fully understood your comment but I took a stab at it in 3afa1a2. Take a look and let me know what you think. Thanks!

ruflin · 2018-08-08T08:39:54Z

metricbeat/module/kibana/stats/stats.go

+func (m *MetricSet) fetchSettings(r mb.ReporterV2, intervalMs int64, now time.Time) {
+	content, err := m.settingsHTTP.FetchContent()
+	if err != nil {
+		r.Error(err)


Should we report an error? I think we will not make use of it.

Resolves #7621. Depends on elastic/kibana#21100. X-Pack Monitoring of Kibana requires two types of documents in the `.monitoring-kibana-*` indices: `kibana_stats` and `kibana_settings`. We made Metricbeat's `kibana/stats` metricset index `kibana_stats` documents into `.monitoring-kibana-*` in #7525. This PR makes the same metricset index `kibana_settings` documents into `.monitoring-kibana-*`. (cherry picked from commit 2af5ab9)

ycombinator added in progress Pull request is currently in progress. Metricbeat Metricbeat v7.0.0-alpha1 monitoring labels Jul 20, 2018

ruflin mentioned this pull request Jul 20, 2018

Monitor the Elastic Stack with Metricbeat #7035

Closed

39 tasks

ruflin reviewed Jul 23, 2018

View reviewed changes

ycombinator force-pushed the metricbeat/kibana/settings branch 5 times, most recently from 7febb95 to 8c8bf88 Compare July 24, 2018 17:53

ycombinator added the v6.5.0 label Jul 26, 2018

ycombinator force-pushed the metricbeat/kibana/settings branch from 8c8bf88 to d1f5765 Compare July 26, 2018 21:20

ycombinator mentioned this pull request Jul 26, 2018

[Monitoring] Kibana settings api elastic/kibana#21100

Merged

ycombinator force-pushed the metricbeat/kibana/settings branch from 233cd27 to b8240e0 Compare August 6, 2018 18:28

ycombinator added 2 commits August 7, 2018 10:14

Report kibana_settings to X-Pack Monitoring

86cfe01

Using common helper method

a59d59d

ycombinator force-pushed the metricbeat/kibana/settings branch from b8240e0 to a59d59d Compare August 7, 2018 17:14

Account for cluster_uuid vs. clusterUuid in Kibana API responses

828ba1d

ycombinator added review and removed in progress Pull request is currently in progress. labels Aug 7, 2018

ycombinator added the needs_backport PR is waiting to be backported to other branches. label Aug 8, 2018

ruflin reviewed Aug 8, 2018

View reviewed changes

ycombinator added 3 commits August 8, 2018 10:41

Removing unused semver package

9101c9d

Don't report error

011b7cf

Calculate interval only in the x-pack case

3afa1a2

ruflin merged commit 2af5ab9 into elastic:master Aug 9, 2018

ycombinator removed the needs_backport PR is waiting to be backported to other branches. label Aug 13, 2018

ycombinator deleted the metricbeat/kibana/settings branch December 25, 2019 11:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Report kibana_settings to X-Pack Monitoring #7664

Report kibana_settings to X-Pack Monitoring #7664

ycombinator commented Jul 20, 2018 •

edited

Loading

ruflin Jul 23, 2018

ruflin Jul 23, 2018

ruflin Jul 23, 2018

ycombinator Jul 23, 2018

ruflin Jul 24, 2018

ycombinator Jul 24, 2018 •

edited

Loading

ruflin Jul 24, 2018

ycombinator Jul 24, 2018

ycombinator Jul 24, 2018

ycombinator Jul 24, 2018 •

edited

Loading

ycombinator commented Aug 7, 2018

ruflin Aug 8, 2018

ycombinator Aug 8, 2018 •

edited

Loading

ruflin Aug 8, 2018

ruflin Aug 8, 2018

ycombinator Aug 8, 2018 •

edited

Loading

ycombinator Aug 8, 2018 •

edited

Loading

ruflin Aug 9, 2018

ruflin Aug 9, 2018

ruflin Aug 8, 2018

ycombinator Aug 8, 2018

ruflin Aug 9, 2018

ruflin Aug 8, 2018

Report kibana_settings to X-Pack Monitoring #7664

Report kibana_settings to X-Pack Monitoring #7664

Conversation

ycombinator commented Jul 20, 2018 • edited Loading

To test this PR

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

ycombinator Jul 24, 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

ycombinator Jul 24, 2018 • edited Loading

Choose a reason for hiding this comment

ycombinator commented Aug 7, 2018

Choose a reason for hiding this comment

ycombinator Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ycombinator Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

ycombinator Aug 8, 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

ycombinator commented Jul 20, 2018 •

edited

Loading

ycombinator Jul 24, 2018 •

edited

Loading

ycombinator Jul 24, 2018 •

edited

Loading

ycombinator Aug 8, 2018 •

edited

Loading

ycombinator Aug 8, 2018 •

edited

Loading

ycombinator Aug 8, 2018 •

edited

Loading