[Lens] Track actions in the UI by time #47919

wylieconlon · 2019-10-11T02:29:58Z

This PR introduces:

A telemetry collector that runs on a 10 second interval while the Lens app is open. Events are tracked to local storage, and if there are any events captured in 10 seconds, they are posted to the server in a batch. Events are only posted once, they are cleared after successfully sending.
A new server endpoint to track the usage data from the browser. Each 4-tuple is written to a saved object called lens-ui-telemetry representing the name, date, type and count of the events being tracked.
A daily task which aggregates all the lens-ui-telemetry documents and saved lens documents into daily buckets. This task also deletes any events older than 90 days, as we are not reporting on events past that point.
A stats collector which reads from the daily task, using the task state as a cache of data

The full list of event names that are introduced in this PR is:

Events that track a click:

app_filters_updated
save_failed
app_date_change
app_query_change
loaded
loaded_404
chart_switch
suggestion_confirmed
suggestion_clicked
drop_total
drop_onto_workspace
drop_onto_dimension
drop_empty
drop_non_empty
indexpattern_changed
indexpattern_filters_cleared
indexpattern_type_filter_toggled
indexpattern_existence_toggled
indexpattern_dimension_removed
indexpattern_dimension_field_changed
indexpattern_dimension_operation_<type>
indexpattern_dimension_operation_from_<type>_to_<type>
indexpattern_field_info_click
xy_change_layer_display
xy_layer_removed
xy_layer_added

Events that are triggered by the suggestion panel:

back_to_current
position_<number>_of_<number>
reload

Example of telemetry collected in its full structure:

"lens": {
  "saved_overall": {
    "bar_stacked": 1
  },
  "saved_30_days": {
    "bar_stacked": 1
  },
  "saved_90_days": {
    "bar_stacked": 1
  },
  "saved_overall_total": 1,
  "saved_30_days_total": 1,
  "saved_90_days_total": 1,
  "events_30_days": {
    "loaded": 2,
    "app_date_change": 1,
    "app_query_change": 1,
    "indexpattern_changed": 1,
    "suggestion_clicked": 1,
    "suggestion_confirmed": 1,
    "xy_layer_removed": 1
  },
  "events_90_days": {
    "loaded": 2,
    "app_date_change": 1,
    "app_query_change": 1,
    "indexpattern_changed": 1,
    "suggestion_clicked": 1,
    "suggestion_confirmed": 1,
    "xy_layer_removed": 1
  },
  "suggestion_events_30_days": {
    "position_0_of_5": 1
  },
  "suggestion_events_90_days": {
    "position_0_of_5": 1
  }
},

elasticmachine · 2019-10-11T03:36:53Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: a007ec44d25a001622bca43cfe9e6c7fcad6b8fa

chrisdavies

Just a preliminary review. It looks reasonable. My comments are mostly nits. I think we discussed all of the more relevant stuff already.

x-pack/legacy/plugins/lens/public/app_plugin/app.tsx

x-pack/legacy/plugins/lens/public/lens_ui_telemetry/factory.ts

chrisdavies · 2019-10-11T20:55:36Z

x-pack/legacy/plugins/lens/public/lens_ui_telemetry/factory.ts

+
+  private async postToServer() {
+    try {
+      await this.http.post(`${this.basePath}${BASE_API_URL}/telemetry`, {


I wonder if we should track this.request or something and then just exit here, if the previous request is still running for some reason. 10 seconds should be enough, but...

x-pack/legacy/plugins/lens/server/routes/telemetry.ts

x-pack/legacy/plugins/lens/server/usage/collectors.ts

chrisdavies · 2019-10-11T20:59:16Z

x-pack/legacy/plugins/lens/server/usage/collectors.ts

+      task manager 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.includes('NotInitialized')) {


Is there not a code or something systematic that we could use to do this check?

chrisdavies · 2019-10-11T20:59:46Z

x-pack/legacy/plugins/lens/server/usage/task.ts

+    [TELEMETRY_TASK_TYPE]: {
+      title: 'Lens telemetry fetch task',
+      type: TELEMETRY_TASK_TYPE,
+      timeout: '1m',


I wonder if we should have a longer timeout, just in case

elasticmachine · 2019-10-11T21:13:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: a832b1e

elasticmachine · 2019-10-11T21:38:11Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 8c818e4

elasticmachine · 2019-10-11T23:36:41Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 5281cbd

elasticmachine · 2019-10-12T04:27:41Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: bf26faa

elasticmachine · 2019-10-13T17:45:14Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 5c7fdb7

elasticmachine · 2019-10-14T05:29:39Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: e8e2b70

elasticmachine · 2019-10-14T22:47:19Z

Pinging @elastic/kibana-app (Team:KibanaApp)

chrisdavies

LGTM w/ tweaks.

chrisdavies · 2019-10-15T15:25:37Z

x-pack/legacy/plugins/apm/typings/elasticsearch.ts

+    buckets: SubAggregationMap extends {
+      filters: { filters: Record<string, unknown> };
+    }
+      ? {


I'm unfamiliar with this TypeScript signature. TIL

chrisdavies · 2019-10-15T15:27:59Z

x-pack/legacy/plugins/lens/public/app_plugin/app.tsx

+                ) {
+                  trackUiEvent('app_date_change');
+                } else {
+                  trackUiEvent('app_query_change');


Is this actually interesting? I'm not sure it's worth collecting, as I expect it to be just super noisy.

I could combine this with filter changes, but I do think it's interesting

x-pack/legacy/plugins/lens/public/app_plugin/plugin.tsx

chrisdavies · 2019-10-15T15:31:24Z

x-pack/legacy/plugins/lens/public/app_plugin/plugin.tsx

    const renderEditor = (routeProps: RouteComponentProps<{ id?: string }>) => {
+      trackUiEvent('loaded');


Is this worth tracking? Maybe. 404 certainly is.

If we don't track some kind of loaded event it won't be possible to create any kind of funnel, because these metrics are in a window and no other metrics are

chrisdavies · 2019-10-15T15:33:43Z

x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_panel.tsx

                if (lastSelectedSuggestion === index) {
                  rollbackToCurrentVisualization();
                } else {
+                  trackSuggestionEvent(`index_${index}_of_${suggestions.length}`);


Maybe suggestion_${index}_of_${suggestions.length}? I had to stare at this for a bit to realize that index was not an Elasticsearch index name.

That's why it's a different function call- Raya wanted to track the position of suggestions. I'll rename to position

chrisdavies · 2019-10-15T16:17:06Z

x-pack/legacy/plugins/lens/server/usage/task.ts

+  metrics.aggregations!.daily.buckets.forEach(daily => {
+    const byType: Record<string, number> = byDateByType[daily.key] || {};
+    daily.groups.buckets.regularEvents.names.buckets.forEach(bucket => {
+      byType[bucket.key] = bucket.sums.value || 0 + (byType[daily.key] || 0);


It's hard for me to follow this, so am just commenting here for you to double-check that the use of || is really what you want here, same for below.

chrisdavies · 2019-10-15T16:17:34Z

x-pack/legacy/plugins/lens/server/usage/task.ts

+  const byDateByType: Record<string, Record<string, number>> = {};
+  const suggestionsByDate: Record<string, Record<string, number>> = {};
+
+  metrics.aggregations!.daily.buckets.forEach(daily => {


Are we sure this can never be undefined? Same for the deeper properties.

As long as the .kibana index exists, the aggregations key will be returned. And if not, this entire task will fail but that's also handled

chrisdavies · 2019-10-15T16:18:11Z

x-pack/legacy/plugins/lens/server/usage/task.ts

+      query: {
+        bool: {
+          filter: [
+            { term: { type: 'lens-ui-telemetry' } },


I wonder if we should pull 'lens-ui-telemetry' out into a const?

chrisdavies · 2019-10-15T16:18:49Z

x-pack/legacy/plugins/lens/server/usage/task.ts

+  };
+}
+
+function getNextMidnight() {


You're using moment everywhere else, why not here?

This was copied

chrisdavies · 2019-10-15T16:20:48Z

x-pack/legacy/plugins/lens/server/usage/types.ts

 }
+
+export interface LensVisualizationUsage {
+  saved_overall: Record<string, number>;


Just to be sure this can't explode in size, we should double-check that our generated event names have a realistically small potential set of values.

These aren't generated names, they're all strings in code

AlonaNadler

Great work @wylieconlon I can't wait to start using it !!

how does the suggestions fields work e.g. "index_0_of_5": 1 ?
Using workspace_drop_non_empty: 1 I want to know how common users drag more than 1 fields, does it make sense to do AVG(workspace_drop_non_empty/workspace_drop_empty)?
suggestion_confirmed: 1 is it suggestion selected & saved?
What is the definition of loaded?
What's the definition of app_query_change is it a change from the configuration panel?

`

wylieconlon · 2019-10-15T18:19:43Z

@AlonaNadler

how does the suggestions fields work e.g. "index_0_of_5": 1 ?

Renamed to position_0_of_5. All the suggestion events are also added to something like suggestion_clicked in the events list

Using workspace_drop_non_empty: 1 I want to know how common users drag more than 1 fields, does it make sense to do AVG(workspace_drop_non_empty/workspace_drop_empty)?

No, that would not make sense. I've changed it so that there is now a number that tracks all drops regardless of their destination, and then breaks that out into:

All drops that were empty, vs all that were non-empty
All drops onto workspace vs all onto a dimension

Both of those should add up to 100%

suggestion_confirmed: 1 is it suggestion selected & saved?

This is number of clicks on the "Reload" button

What is the definition of loaded?

This tracks any page load, including ones that happen because the user saved. I can change this definition.

What's the definition of app_query_change is it a change from the configuration panel?

For example, a KQL filter was typed into the query bar.

elasticmachine · 2019-10-15T19:59:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: fd1f6d6

chrisdavies

LGTM w/ tweaks. I think you need to correct the operator precedence stuff I commented on in this review and my previous one. But other than that, I think this is fine.

chrisdavies · 2019-10-15T20:06:16Z

x-pack/legacy/plugins/lens/public/drag_drop/drag_drop.tsx

@@ -121,7 +122,8 @@ export function DragDrop(props: Props) {
    setState({ ...state, isActive: false });
    setDragging(undefined);

-    if (onDrop) {
+    if (onDrop && droppable) {
+      trackUiEvent('drop_total');


This is probably fine, but if we ever want to make this drag / drop thing more general, this would be something that requires yanking out.

chrisdavies · 2019-10-15T20:09:48Z

x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/dimension_panel.tsx

@@ -137,6 +138,10 @@ export const IndexPatternDimensionPanel = memo(function IndexPatternDimensionPan
                field: droppedItem.field,
              });

+          trackUiEvent('drop_onto_dimension');
+          const hasData = Object.values(props.state.layers).some(({ columns }) => columns.length);
+          trackUiEvent(hasData ? 'drop_non_empty' : 'drop_empty');


This is fine, but also, do we need three different events? drop_onto_dimension would be a sum of drop_non_empty and drop_empty

It's actually five events:

drop_total is every drop

Two pairs:

drop_onto_workspace + drop_onto_dimension = drop_total
drop_empty + drop_non_empty = drop_total

chrisdavies · 2019-10-15T20:14:15Z

x-pack/legacy/plugins/lens/public/lens_ui_telemetry/factory.ts

+  }
+
+  private async postToServer() {
+    this.readFromStorage();


This is probably fine, but it does introduce a disk read for a whole bunch of user-driven events. I wonder if this'll have any impact on performance on low-end devices... Probably not.

Anyway, an alternative would be to track all the stuff in memory, but when you flush it to storage, read and merge then, and reset your in-memory state.

There are probably some alternatives here, but this is the simplest implementation of it

chrisdavies · 2019-10-15T20:18:15Z

x-pack/legacy/plugins/lens/server/usage/collectors.ts

+
+function addEvents(prevEvents: Record<string, number>, newEvents: Record<string, number>) {
+  Object.keys(newEvents).forEach(key => {
+    prevEvents[key] = prevEvents[key] || 0 + newEvents[key];


I'm not sure why lint won't let you put parenthesis there, but I think you should, and should add a lint exception comment.

elasticmachine · 2019-10-15T23:58:46Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 16ebf6c

elasticmachine · 2019-10-16T00:45:44Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 4da50b3

elasticmachine · 2019-10-16T01:08:39Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: b5d54a7

Bamieh

This collection method almost duplicates @kbn/analytics code just for providing a new collection method. This UI metric can be easily baked into the @kbn/analytics library by introducing it as a new METRIC_TYPE. Doing so it makes it easier to manage telemetry, refactor code and have others utilize these metric types.

I have opened an issue for this as a follow up to this PR: #48352

While it is nice to have this tracking as soon as possible for lens. Ideally implementing this would be approached differently; I've opened an issue so we can revisit the use of this metric type so we can learn its usefulness for the consumers of this usage data and then introduce it as METRIC_TYPE in @kbn/analytics for others to be able to use it as well.

I have opened an issue for this as a follow up to this PR: #48353

While not completely thrilled about introducing new telemetry code floating outside our telemetry packages (ui_metric / telemetry / and @kbn/analytics); I believe we can merge this and revisit these concerns afterwards.

elasticmachine · 2019-10-16T02:42:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 1836ed1

wylieconlon · 2019-10-16T03:01:37Z

@Bamieh You are right that it is heavily based on the existing code, and I am looking forward to integrating some of these changes for the rest of Kibana to use in releases.

* [Lens] Track actions in the UI by time * Switch collector to use task data * Report summarized version of task data when requested * Track a more complete set of metrics * Collect suggestion events separately * Pre-aggregate by date in localStorage * Add integration tests * Fix test linter * Fix telemetry naming and run at midnight instead of every minute * Improve cleanup at app level * Fix lint errors * Remove unused mock * Fix tests * Fix types * Update event names and fix local tracking * Respond to review comments * Fix task time * Fix test

[Lens] Track actions in the UI by time

a832b1e

wylieconlon force-pushed the lens/click-telemetry branch from a007ec4 to a832b1e Compare October 11, 2019 20:04

wylieconlon added the Feature:Lens label Oct 11, 2019

Switch collector to use task data

8c818e4

chrisdavies reviewed Oct 11, 2019

View reviewed changes

Report summarized version of task data when requested

5281cbd

Track a more complete set of metrics

bf26faa

Collect suggestion events separately

5c7fdb7

Pre-aggregate by date in localStorage

e8e2b70

wylieconlon added 4 commits October 14, 2019 18:17

Add integration tests

1fcce38

Merge remote-tracking branch 'origin/master' into lens/click-telemetry

8a2dce4

Fix test linter

c4c5639

Fix telemetry naming and run at midnight instead of every minute

e9590e8

wylieconlon marked this pull request as ready for review October 14, 2019 22:47

wylieconlon requested a review from a team as a code owner October 14, 2019 22:47

wylieconlon requested a review from a team October 14, 2019 22:47

wylieconlon added release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.5.0 v8.0.0 labels Oct 14, 2019

wylieconlon requested a review from chrisdavies October 14, 2019 22:47

wylieconlon requested a review from Bamieh October 14, 2019 22:47

chrisdavies approved these changes Oct 15, 2019

View reviewed changes

AlonaNadler reviewed Oct 15, 2019

View reviewed changes

wylieconlon added 2 commits October 15, 2019 13:03

Merge remote-tracking branch 'origin/master' into lens/click-telemetry

f1a1399

Update event names and fix local tracking

fd1f6d6

wylieconlon requested a review from chrisdavies October 15, 2019 18:17

chrisdavies approved these changes Oct 15, 2019

View reviewed changes

wylieconlon added 4 commits October 15, 2019 18:24

Merge remote-tracking branch 'origin/master' into lens/click-telemetry

2a28500

Respond to review comments

16ebf6c

Fix task time

4da50b3

Merge remote-tracking branch 'origin/master' into lens/click-telemetry

b5d54a7

wylieconlon added 2 commits October 15, 2019 21:15

Fix test

5ef055f

Merge remote-tracking branch 'origin/master' into lens/click-telemetry

1836ed1

This was referenced Oct 16, 2019

[Lens] Bake lens ui metric type into @kbn/analytics #48352

Closed

[Lens] UI Metric requirements #48353

Closed

Bamieh reviewed Oct 16, 2019

View reviewed changes

wylieconlon merged commit fba41f6 into elastic:master Oct 16, 2019

wylieconlon deleted the lens/click-telemetry branch October 16, 2019 03:01

wylieconlon mentioned this pull request Oct 16, 2019

[7.x] [Lens] Track actions in the UI by time (#47919) #48354

Merged

rayafratkina mentioned this pull request Oct 22, 2019

[Lens] User interaction telemetry for Lens #46599

Closed

		const renderEditor = (routeProps: RouteComponentProps<{ id?: string }>) => {
		trackUiEvent('loaded');

[Lens] Track actions in the UI by time #47919

[Lens] Track actions in the UI by time #47919

Conversation

wylieconlon commented Oct 11, 2019 • edited Loading

elasticmachine commented Oct 11, 2019

💔 Build Failed

chrisdavies 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

elasticmachine commented Oct 11, 2019

💔 Build Failed

elasticmachine commented Oct 11, 2019

💔 Build Failed

elasticmachine commented Oct 11, 2019

💔 Build Failed

elasticmachine commented Oct 12, 2019

💔 Build Failed

elasticmachine commented Oct 13, 2019

💔 Build Failed

elasticmachine commented Oct 14, 2019

💔 Build Failed

elasticmachine commented Oct 14, 2019

chrisdavies 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

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

AlonaNadler left a comment • edited Loading

Choose a reason for hiding this comment

wylieconlon commented Oct 15, 2019

elasticmachine commented Oct 15, 2019

💚 Build Succeeded

chrisdavies 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

elasticmachine commented Oct 15, 2019

💔 Build Failed

elasticmachine commented Oct 16, 2019

💔 Build Failed

elasticmachine commented Oct 16, 2019

💔 Build Failed

Bamieh left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 16, 2019

💚 Build Succeeded

wylieconlon commented Oct 16, 2019

wylieconlon commented Oct 11, 2019 •

edited

Loading

AlonaNadler left a comment •

edited

Loading