[XpackMain] Add _xpack/usage API #19232

tsullivan · 2018-05-18T21:33:58Z

Pulled from #18894

This adds the /api/_xpack/usage HTTP API endpoint, which returns data fetched from Elasticsearch about X-Pack feature usage.

It removes the integration test for stats, which is a deprecated API, and adds an integration test for the usage API.

elasticmachine · 2018-05-18T22:24:06Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

tsullivan · 2018-05-18T22:27:20Z

x-pack/test/functional/config.js

@@ -162,7 +162,6 @@ export default async function ({ readConfigFile }) {
      `--server.uuid=${env.kibana.server.uuid}`,
      `--server.port=${servers.kibana.port}`,
      `--elasticsearch.url=${formatUrl(servers.elasticsearch)}`,
-      '--xpack.monitoring.kibana.collection.enabled=false',


I had to remove this in order for const { collectorSet } = server.plugins.monitoring to work in the API handler.

Maybe it'd be better to expose the collectorSet regardless, but check for xpack.monitoring.kibana.collection.enabled in the bulk fetch method of the collector set ...

On second thought, I don't really like the idea of lazy checking a config setting in the bulk method.

The collectorSet only gets exposed on the monitoring plugin when xpack.monitoring.kibana.collection.enabled is true, and I think that's fine.

Maybe when collection is disabled, we can expose a no-op function on the plugin so other plugins don't have to think about it.

elasticmachine · 2018-05-21T20:02:55Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

tsullivan · 2018-05-21T21:05:13Z

x-pack/test/functional/config.js

@@ -164,7 +164,6 @@ export default async function ({ readConfigFile }) {
        `--server.uuid=${env.kibana.server.uuid}`,
        `--server.port=${servers.kibana.port}`,
        `--elasticsearch.url=${formatUrl(servers.elasticsearch)}`,
-        '--xpack.monitoring.kibana.collection.enabled=false',


It wasn't my first choice to remove this, but it is necessary for making the api integration test work. The HTTP endpoint relies collectorSet being exposed, which is dependent on this setting being true.

It's kind of unfortunate that all functional tests depend on the same configuration. Given the startup time of Kibana though, I get why that is the best option though.

Didn't we need to add this because it randomly broke other tests though?

Didn't we need to add this because it randomly broke other tests though?

Nope, this was just an optimization to avoid running the internal timer for the functional test server.

pickypg

Minor comments and a question, then LGTM.

pickypg · 2018-05-21T21:27:10Z

x-pack/test/functional/config.js

@@ -164,7 +164,6 @@ export default async function ({ readConfigFile }) {
        `--server.uuid=${env.kibana.server.uuid}`,
        `--server.port=${servers.kibana.port}`,
        `--elasticsearch.url=${formatUrl(servers.elasticsearch)}`,
-        '--xpack.monitoring.kibana.collection.enabled=false',


It's kind of unfortunate that all functional tests depend on the same configuration. Given the startup time of Kibana though, I get why that is the best option though.

Didn't we need to add this because it randomly broke other tests though?

pickypg · 2018-05-21T21:28:16Z

x-pack/plugins/xpack_main/server/routes/api/v1/xpack_usage.js

+
+export function xpackUsageRoute(server) {
+  server.route({
+    path: '/api/_xpack/usage',


I like the usage of _xpack here because it's consistent with ES X-Pack, but I wonder how others might feel about this.

pretty much every other API in Kibana has a convention of /api/kibana/<featurename>/<sub>/

src/core_plugins/kibana/server/routes/api/export/index.js:7:5: path: '/api/kibana/dashboards/export', src/core_plugins/kibana/server/routes/api/home/register_tutorials.js:4:5: path: '/api/kibana/home/tutorials', src/core_plugins/kibana/server/routes/api/scroll_search/index.js:5:5: path: '/api/kibana/legacy_scroll_start', src/core_plugins/kibana/server/routes/api/search/count/register_count.js:6:5: path: '/api/kibana/{id}/_count', src/core_plugins/kibana/server/routes/api/scroll_search/index.js:24:5: path: '/api/kibana/legacy_scroll_continue', src/core_plugins/kibana/server/routes/api/scripts/register_languages.js:3:5: path: '/api/kibana/scripts/languages', src/core_plugins/kibana/server/routes/api/suggestions/register_value_suggestions.js:6:5: path: '/api/kibana/suggestions/values/{index}', src/core_plugins/kibana/server/routes/api/management/saved_objects/scroll.js:19:5: path: '/api/kibana/management/saved_objects/scroll/export', src/core_plugins/kibana/server/routes/api/management/saved_objects/scroll.js:54:5: path: '/api/kibana/management/saved_objects/scroll/counts', src/core_plugins/kibana/server/routes/api/management/saved_objects/relationships.js:7:5: path: '/api/kibana/management/saved_objects/relationships/{type}/{id}',

The existing status API has a path of /api/status and the new stats API (6.3.0) has a path of /api/stats

I think it's good to be consistent with ES, as Kibana doesn't have a good foundation for conventions currently.

pickypg · 2018-05-21T21:28:47Z

x-pack/plugins/xpack_main/server/routes/api/v1/xpack_usage.js

+        });
+      } catch(err) {
+        req.log(['error'], err);
+        if (err.isBoom) {


I really dislike how Boom wrap works.

pickypg · 2018-05-21T21:30:37Z

x-pack/plugins/xpack_main/server/routes/api/v1/xpack_usage.js

+
+/*
+ * @return {Object} data from usage stats collectors registered with Monitoring CollectorSet
+ */


Probably worth marking that it throws an Error.

* @throws {Error} if the Monitoring CollectorSet is not ready

pickypg

LGTM. Agree with the convention point.

elasticmachine · 2018-05-21T23:48:30Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* [XpackMain] Add _xpack/usage API * add xpack usage http api integration test * comment * misc test describe fixes * fix integration test * fix reply called twice * enable api test * enable kibana collection for integration test to work * throw error comment

* [XpackMain] Add _xpack/usage API (#19232) * [XpackMain] Add _xpack/usage API * add xpack usage http api integration test * comment * misc test describe fixes * fix integration test * fix reply called twice * enable api test * enable kibana collection for integration test to work * throw error comment * Update config.js remove whitespace change

tsullivan added 7 commits May 18, 2018 13:20

[XpackMain] Add _xpack/usage API

a7d5837

add xpack usage http api integration test

6cf1765

comment

755274e

misc test describe fixes

fc77200

fix integration test

347dc17

fix reply called twice

303c7ae

enable api test

138a8f0

tsullivan added review v7.0.0 v6.4.0 Feature:xpack_main labels May 18, 2018

tsullivan requested a review from pickypg May 18, 2018 21:33

tsullivan mentioned this pull request May 18, 2018

[Monitoring/Reporting Stats] Move hardcoded Reporting stats type collection out of monitoring plugin #18894

Merged

4 tasks

tsullivan commented May 18, 2018

View reviewed changes

tsullivan added 2 commits May 21, 2018 11:45

Merge branch 'master' into xpack/usage-api

968e06f

enable kibana collection for integration test to work

bef7e45

tsullivan commented May 21, 2018

View reviewed changes

pickypg reviewed May 21, 2018

View reviewed changes

throw error comment

4da6e42

pickypg approved these changes May 21, 2018

View reviewed changes

tsullivan merged commit 744fc0b into elastic:master May 21, 2018

tsullivan deleted the xpack/usage-api branch May 21, 2018 23:55

tsullivan mentioned this pull request May 22, 2018

[XpackMain] 6.x Backport Add _xpack/usage API (#19232) #19287

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[XpackMain] Add _xpack/usage API #19232

[XpackMain] Add _xpack/usage API #19232

tsullivan commented May 18, 2018

elasticmachine commented May 18, 2018

tsullivan May 18, 2018

tsullivan May 21, 2018

elasticmachine commented May 21, 2018

tsullivan May 21, 2018

pickypg May 21, 2018

tsullivan May 21, 2018

pickypg left a comment

pickypg May 21, 2018

pickypg May 21, 2018

tsullivan May 21, 2018

pickypg May 21, 2018

pickypg May 21, 2018

pickypg left a comment

elasticmachine commented May 21, 2018

[XpackMain] Add _xpack/usage API #19232

[XpackMain] Add _xpack/usage API #19232

Conversation

tsullivan commented May 18, 2018

elasticmachine commented May 18, 2018

💚 Build Succeeded

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented May 21, 2018

💚 Build Succeeded

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pickypg 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

pickypg left a comment

Choose a reason for hiding this comment

elasticmachine commented May 21, 2018

💚 Build Succeeded