Stats API by stacey-gammon · Pull Request #11992 · elastic/kibana

stacey-gammon · 2017-05-24T20:58:28Z

Release Note:
We introduced a new api endpoint at api/stats which displays counts of saved objects in your kibana index:

{"dashboard":{"total":1},"visualization":{"total":3},"search":{"total":1},"index_pattern":{"total":1},"index":".kibana"}

Currently tracking total counts of:

saved searches
index patterns
visualizations
dashboards

stacey-gammon · 2017-06-01T11:15:47Z

Modifying the .kibana index turned out to be more of a challenge than originally thought. I looked into changing the mapping to allow aggregations on visualization sub type and it sounds like this can't be done without a manual migration step (operations team tagged below can confirm). Filed #12117 which is a blocker for that portion of the stats.

@tylersmalley @jbudz -- Given where the operations team is at and the extra complexity involved with modifying the index to allow for visualization sub type tracking, (in addition more efficient type tracking if we made type a searchable and aggregatable field), how likely do you think it is that this can happen in time for 6.0, with some time left for me to modify the stats API to take advantage of the new fields? If we don't get it in to 6.0, can we make these index changes in 6.1? I don't think it will be a BWC change, but do we write migration steps for minor releases? Or will we have to wait till 7.0 to add the additional metrics?

@alexfrancoeur @pickypg @tsullivan -- based on the response from my questions above, I'm wondering if we should check this in as is so we at least have some stats to track for 6.0, and push the additional stats till 6.x, 7.0 at worst.

alexfrancoeur · 2017-06-01T13:30:49Z

@stacey-gammon I'm comfortable with treating visualization sub-type as a secondary priority. The first comment of this PR is a bit unclear but based on our previous conversations I'd assume that we can track the total number of saved objects for each type of saved object. Is that correct? Total number of dashboards, visualizations, index patterns, saved searches, etc. Each one of these being a separate ES query for now.

If there is no bandwidth from the ops team (which it doesn't sound like there is) I'd be comfortable with introducing these metrics for 6.0 and enhancing with visualization sub-types in a 6.x release

stacey-gammon · 2017-06-01T13:38:21Z

I'd assume that we can track the total number of saved objects for each type of saved object. Is that correct? Total number of dashboards, visualizations, index patterns, saved searches, etc. Each one of these being a separate ES query for now.

@alexfrancoeur - Yep exactly, and that is solved by the current state of this PR (I updated the description). I'll go ahead and bump this into the review process so we can at least have that ready to go for 6.0, and will wait on the additional stats/efficiency enhancements for more bandwidth from the ops team.

tsullivan

This is great, but I had a few minor suggestions

tsullivan · 2017-06-01T23:03:57Z

src/server/stats/stats_mixin.js

The ) looks like it should go 1 line down :)

tsullivan · 2017-06-01T23:06:15Z

src/server/stats/stats.js

This can be simply:

const requests = types.map(type => getStatsForType(savedObjectsClient, type));

tsullivan · 2017-06-01T23:07:50Z

src/server/stats/stats.js

I think this can be moved down closer to where it is first referenced

tsullivan · 2017-06-01T23:13:21Z

src/server/stats/stats.js

const { total } = await savedObjectsClient.find({ type, perPage: 0 }); return { total };

stacey-gammon · 2017-06-06T14:51:58Z

Any additional comments @tsullivan, @pickypg?

tsullivan

LGTM!

stacey-gammon · 2017-06-09T13:37:27Z

ping @pickypg :)

tylersmalley · 2017-06-09T17:04:00Z

src/server/stats/stats_mixin.js

+        callAdminCluster
+      );
+
+      return reply(stats);


You can reply with the promise instead of awaiting it. Just a nitpick, doesn't need to be changed.

And then the handler doesn't need to be marked as async.

@stacey-gammon

return Promise.resolve(getStats(server.config().get('kibana.index'), callAdminCluster));

Unless not modifying the return actually works? I would be surprised if that were the case, but JS has done crazier things.

@pickypg, It worked when I tested it, but using the above doesn't seem to work (I don't see an error, it just hangs). Do I need to wrap your return with the reply?

async / await is all just sugar on top of Promise, so the method is already returning a Promise that you choose to await against to get the value (or throw the exception). So I'm guessing if it worked, then you simply can stick with what you've got and can avoid my extra fluff.

Oh doh! Yes, my example was missing reply wrapping the Promise.

You shouldn't need Promise.resolve since getStats returns a promise. Just need to return the getStats call.

tylersmalley

LGTM

pickypg

LGTM

pickypg · 2017-06-09T18:34:31Z

src/server/stats/stats_mixin.js

+        callAdminCluster
+      );
+
+      return reply(stats);


And then the handler doesn't need to be marked as async.

…tats-api

jimgoodwin · 2017-07-25T19:43:52Z

@stacey-gammon could we get a Release Note: paragraph in the description that summarizes the effect of the changes

stacey-gammon · 2017-07-25T19:54:05Z

Done @jimgoodwin - not sure how the ``` will come out though. I can get rid of them if it messes up the formatting.

tsullivan · 2017-09-12T20:56:01Z

I'm rolling back this PR for 6.0, but parts of it will be available in 6.1+. There will be no HTTP API. The mixin provides a server method in 6.1+, and that functionality will remain.

@jimgoodwin @stacey-gammon sorry for the churn, but let's make sure there's no mention of this change in the release notes.

stacey-gammon force-pushed the new-stats-api branch from 17d9d17 to 6d9a6aa Compare May 24, 2017 21:08

stacey-gammon added the :Sharing label May 30, 2017

Simple stats api POC

8ee2960

stacey-gammon mentioned this pull request May 31, 2017

Move kibana index under a single type #10831

Closed

stacey-gammon requested review from pickypg and tylersmalley June 1, 2017 13:38

stacey-gammon added review v6.0.0 labels Jun 1, 2017

tsullivan requested changes Jun 1, 2017

View reviewed changes

Code review clean up

b571688

stacey-gammon force-pushed the new-stats-api branch from 6d9a6aa to b571688 Compare June 2, 2017 12:43

stacey-gammon removed the request for review from tylersmalley June 6, 2017 14:52

tsullivan approved these changes Jun 6, 2017

View reviewed changes

tylersmalley reviewed Jun 9, 2017

View reviewed changes

tylersmalley approved these changes Jun 9, 2017

View reviewed changes

pickypg approved these changes Jun 9, 2017

View reviewed changes

Stacey Gammon added 2 commits June 9, 2017 15:36

reply with promise, avoids async function

4488f1b

Merge branch 'master' of https://github.com/elastic/kibana into new-s…

e1145d3

…tats-api

stacey-gammon merged commit 415bf15 into elastic:master Jun 12, 2017

jimgoodwin added the release_note:enhancement label Jul 25, 2017

tsullivan mentioned this pull request Sep 7, 2017

Remove Stats API #13896

Merged

epixa removed the release_note:enhancement label Sep 7, 2017

tsullivan mentioned this pull request Sep 12, 2017

Revert Stats API for 6.0 #13969

Merged

jimgoodwin added the v6.0.0-rc1 label Sep 28, 2017

stacey-gammon deleted the new-stats-api branch October 24, 2017 13:53

epixa added the reverted label Nov 13, 2017

elasticmachine mentioned this pull request Apr 24, 2018

Read-only support for short URL generation #18006

Closed

Conversation

stacey-gammon commented May 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stacey-gammon commented Jun 1, 2017

Uh oh!

alexfrancoeur commented Jun 1, 2017

Uh oh!

stacey-gammon commented Jun 1, 2017

Uh oh!

tsullivan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stacey-gammon commented Jun 6, 2017

Uh oh!

tsullivan left a comment

Choose a reason for hiding this comment

Uh oh!

stacey-gammon commented Jun 9, 2017

Uh oh!

tylersmalley Jun 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pickypg Jun 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stacey-gammon Jun 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tylersmalley Jun 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tylersmalley left a comment

Choose a reason for hiding this comment

Uh oh!

pickypg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimgoodwin commented Jul 25, 2017

Uh oh!

stacey-gammon commented Jul 25, 2017

Uh oh!

tsullivan commented Sep 12, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

stacey-gammon commented May 24, 2017 •

edited

Loading

tylersmalley Jun 9, 2017 •

edited

Loading

pickypg Jun 9, 2017 •

edited

Loading

stacey-gammon Jun 9, 2017 •

edited

Loading

tylersmalley Jun 9, 2017 •

edited

Loading