Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Telemetry for Visualizations by type #28793

Merged
merged 16 commits into from
Jan 30, 2019

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Jan 15, 2019

Closing #22010

Summary

This PR adds a visualization_types property to Kibana usage stats.

Parts of the solution

Nightly Task to parse visState JSON

This schedules a nightly task to parse the visState JSON strings from every visualization saved object in the .kibana index. Therefore, it depends on the x-pack/task_manager plugin, which I believe makes it suitable for creating as a new X-Pack plugin that can have elasticsearch, xpack_main and task_manager as dependencies.

At first, the task is scheduled to run immediately, and from that point, it runs nightly at midnight local server time.

Collector to return the parsed / summarized stats

The actual usage collector gets the scheduled task by _id, finds the state data that contains the stats and simply returns it.

Screenshot of API response

image

Explanation: Looking at the area stats, my default space has 10 area charts and my custom space has 1 area chart. Hence, min, max and avg are anonymized breakdowns of how that visualization type is distributed across spaces.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

@tsullivan tsullivan force-pushed the telemetry/visualization-types branch 8 times, most recently from d98e8b9 to 126d065 Compare January 17, 2019 22:38
@tsullivan tsullivan changed the title [WIP] Telemetry for Visualizations by type Telemetry for Visualizations by type Jan 17, 2019
@tsullivan tsullivan requested review from pickypg, epixa, njd5475, chrisdavies and Bargs and removed request for epixa January 17, 2019 22:39
@tsullivan tsullivan added review and removed WIP Work in progress labels Jan 17, 2019
@elastic elastic deleted a comment from elasticmachine Jan 17, 2019
@elastic elastic deleted a comment from elasticmachine Jan 17, 2019
@elastic elastic deleted a comment from elasticmachine Jan 17, 2019
@elastic elastic deleted a comment from elasticmachine Jan 17, 2019
@tsullivan tsullivan force-pushed the telemetry/visualization-types branch from 126d065 to c64cba9 Compare January 17, 2019 22:50
@tsullivan tsullivan requested a review from timroes January 17, 2019 22:51
@tsullivan
Copy link
Member Author

cc @epixa @njd5475 this is an example of defining task types and scheduling tasks in the Task Manager directly. Need to keep you in the loop since work on Alerting could change the Task Manager API.

@elastic elastic deleted a comment from elasticmachine Jan 27, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@tsullivan tsullivan added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Jan 27, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@tsullivan
Copy link
Member Author

CI is green, and looking to get 1 more review

* has to wait for all plugins to initialize first.
* It's fine to ignore it as next time around it will be initialized (or it will throw a different type of error)
*/
if (errMessage.indexOf('NotInitialized') >= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errMessage.includes('NotInitialized') might read a bit better here (might also be sub-ms faster as well?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also might want a test for this since it's interesting behavior (not sure how difficult that is)

Copy link
Contributor

@joelgriffith joelgriffith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one case that I think would be good to test

* has to wait for all plugins to initialize first.
* It's fine to ignore it as next time around it will be initialized (or it will throw a different type of error)
*/
if (errMessage.indexOf('NotInitialized') >= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also might want a test for this since it's interesting behavior (not sure how difficult that is)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@tsullivan tsullivan merged commit b379751 into elastic:master Jan 30, 2019
@tsullivan tsullivan deleted the telemetry/visualization-types branch January 30, 2019 16:40
tsullivan added a commit to tsullivan/kibana that referenced this pull request Jan 30, 2019
* task runner and usage collector for visualizations by type

* type is always just "visualization"

* drop the I- prefix for interfaces

* bug fixes

* ts fix

* comment perfection

* just usage.

* const for task numworkers

* use mapValues

* get next midnight module

* move to oss_telemtry

* test fix

* errMessage.includes(NotInitialized)
@tsullivan
Copy link
Member Author

6.x/6.7: #29625

@alexfrancoeur
Copy link

Just want to say how excited I am about this one, huge thank you 🙇 to everyone who had a hand in this. This PR is amazing!

tsullivan added a commit that referenced this pull request Jan 31, 2019
* task runner and usage collector for visualizations by type

* type is always just "visualization"

* drop the I- prefix for interfaces

* bug fixes

* ts fix

* comment perfection

* just usage.

* const for task numworkers

* use mapValues

* get next midnight module

* move to oss_telemtry

* test fix

* errMessage.includes(NotInitialized)
@AlonaNadler
Copy link

any idea why TSVB is not in this telemetry pr? I know @tsullivan worked on it and it was supposed to be included

@lukeelmers
Copy link
Member

@AlonaNadler In the codebase, TSVB currently lives inside a legacy plugin called metrics (not to be confused with the metric vis type).

I haven't looked closely at this PR, but based on Tim's summary above it looks like the API response includes data for metrics, which I would expect to be the TSVB data.

@AlonaNadler
Copy link

AlonaNadler commented Jun 11, 2019

Thanks @lukeelmers that is very helpful !
few other questions:

  • any idea where is goal and single metric visualizations?
  • Tile map, does it include coordinate map and region map visualizations combined?
  • Which visualizations included within histogram? is it for bar chart and horizontal bars combined?
  • Do you see vega in there hidden somewhere ? (wishful thinking :) )

@timroes
Copy link
Contributor

timroes commented Jun 11, 2019

@AlonaNadler I have not yet confirmed

  • Goal should be tracked as goal.
  • Single Metric visualization is tracked as metric, while TSVB is tracked as metrics!
  • tile_map should only be the coordinate map (tile map was the old name), Region map should be tracked as region_map.
  • histogram tracks vertical bar charts, horizontal_bar horizontal bar charts. Though histogram, area. horizontal_bar and line always needs to be treated with caution. They are only indicating the type the user initially selected in the wizard. You can create a bar chart, then switch it's style over to line, and add one series that is an area, so the final chart would include one line and one area; it would still be tracked as histogram since that was what the user initially selected. The same is true if you start over with either area, line and switch them over, so basically those three types are not tracked exactly.
  • Vega should be tracked as vega.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Telemetry review Team:Visualizations Visualization editors, elastic-charts and infrastructure v6.7.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.