[lens] Basic usage telemetry for total visualizations, and by type #47597

wylieconlon · 2019-10-08T16:00:02Z

This implements new telemetry that tracks saved Lens visualizations broken down by type, and also by three categories: overall, 30 day window, 90 day window.

Because we have the XY visualization type which contains sub-types, this splits that visualization into subtypes. It does not account for each of the layers.

The shape of this telemetry object is:

"lens": {
  "visualization_types_overall": {
    "area_stacked": 2,
    "lnsDatatable": 1
  },
  "visualization_types_last_30_days": {
    "area_stacked": 2,
    "lnsDatatable": 1
  },
  "visualization_types_last_90_days": {
    "area_stacked": 2,
    "lnsDatatable": 1
  },
  "saved_total": 3,
  "saved_last_30_days": 3,
  "saved_last_90_days": 3
},

elasticmachine · 2019-10-08T16:00:05Z

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

AlonaNadler

Thanks @wylieconlon this is a good start.
Can we do the same for the last 30 days? only counting the saved objects that were done in the last 30 days

elasticmachine · 2019-10-08T17:11:46Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 93b7af4

wylieconlon · 2019-10-08T17:51:07Z

@AlonaNadler Yes, I'll be adding that as soon as I understand whether there is some way of doing that automatically.

elasticmachine · 2019-10-08T23:25:15Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 6be288e

chrisdavies

LGTM w/ tweaks. This all looks fine to me, except I think we should probably optimize the querying a bit.

chrisdavies · 2019-10-09T18:17:01Z

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

+import { getVisualizationCounts } from './visualization_counts';
+import { LensUsage } from './types';
+
+export function getSavedObjectsClient(


This seems to only be used internally. Maybe don't export it?

chrisdavies · 2019-10-09T18:18:37Z

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

+        return getVisualizationCounts(savedObjectsClient);
+      } catch (err) {
+        return {
+          saved_total: 0,


Is this the right behavior? It seems like it'd skew our data if errors are happening. Not sure what else we should do, and I know nothing about the usage collector system, so this may be the right thing. It just feels wrong, and makes me doubt our metrics, if this is the recommended approach.

chrisdavies · 2019-10-09T18:19:14Z

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

+ */
+
+export interface LensUsage {
+  visualization_types_overall: Record<string, number>;


Is there a reason you're using snake_case instead of camelCase? JavaScript is generally camel cased.

This is the pattern being used in the rest of the metrics we collect

chrisdavies · 2019-10-09T18:23:02Z

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

+function groupByType(response: SavedObjectsFindResponse) {
+  const byVisType: Record<string, number> = {};
+
+  response.saved_objects.forEach(({ attributes }) => {


Nit: this feels like it should be a reduce, since that's basically what you're doing in the forEach.

return response.saved_objects.reduce((acc, { attributes }) => { const type = getTypeName(attributes as Document); byVisType[type] = (byVisType[type] || 0) + 1; return acc; }, {} as Record<string, number>);

chrisdavies · 2019-10-09T18:30:51Z

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

+export async function getVisualizationCounts(
+  savedObjectsClient: SavedObjectsClient
+): Promise<LensUsage> {
+  const [overall, last30, last90] = await Promise.all([


You're doing three fetches, when you only need one: the first one. You can change your groupByType function to return overall, 30_days, and 90_days. That may be premature optimization, but it seems worth it. To me, telemetry should be very light-weight and have as low an impact on the system as possible, since it is not a primary feature.

I also think we should filter this down to only fetch the subset of fields that we care about, rather than fetching the entire saved object. I think all that we care about is type, visualizationType, and updated_at, right?

elasticmachine · 2019-10-09T18:48:24Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 0dc6e23

elasticmachine · 2019-10-10T03:25:33Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 6e055b1

chrisdavies

LGTM I know next to nothing about painless, so can't speak to the accuracy of it, but it looks right based on a read-through and glancing at the docs.

chrisdavies · 2019-10-10T18:02:08Z

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

+      // without this kind of mapping we would not be able to access `lens.state` in painless
+      map_script: `
+        String visType = doc['lens.visualizationType'].value;
+        String niceType = visType == 'lnsXY' ? doc['lens.state.visualization.preferredSeriesType'].value : visType;


This is fine for now, but I really don't like special casing like this. Eventually, we should solve this more generally so that if other visualizations / Lens plugins have sub-types, we still get metrics about that.

elasticmachine · 2019-10-10T18:56:23Z

💚 Build Succeeded

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

…lastic#47597) * [lens] Basic usage telemetry for total visualizations, and by type * Implement overall, 30 day, 90 day tracking for saved visualizations * Add forgotten file * Implement collection using scripted metrics * Add functional test to ensure painless script is working

…47597) (#47899) * [lens] Basic usage telemetry for total visualizations, and by type * Implement overall, 30 day, 90 day tracking for saved visualizations * Add forgotten file * Implement collection using scripted metrics * Add functional test to ensure painless script is working

[lens] Basic usage telemetry for total visualizations, and by type

93b7af4

wylieconlon added Feature:Telemetry Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 Feature:Lens v7.5.0 labels Oct 8, 2019

wylieconlon requested a review from a team October 8, 2019 16:00

wylieconlon requested review from chrisdavies, rayafratkina, AlonaNadler and Bamieh October 8, 2019 16:00

wylieconlon added the release_note:skip Skip the PR/issue when compiling release notes label Oct 8, 2019

AlonaNadler reviewed Oct 8, 2019

View reviewed changes

wylieconlon added 2 commits October 8, 2019 16:23

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

48a5ccf

Implement overall, 30 day, 90 day tracking for saved visualizations

6be288e

wylieconlon requested a review from tsullivan October 8, 2019 22:15

wylieconlon requested a review from AlonaNadler October 9, 2019 17:24

wylieconlon added 2 commits October 9, 2019 13:25

Add forgotten file

60ec92a

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

0dc6e23

chrisdavies approved these changes Oct 9, 2019

View reviewed changes

wylieconlon added 2 commits October 9, 2019 17:15

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

5b6546c

Implement collection using scripted metrics

6e055b1

wylieconlon requested a review from chrisdavies October 10, 2019 15:43

Add functional test to ensure painless script is working

8a0ad4d

chrisdavies approved these changes Oct 10, 2019

View reviewed changes

wylieconlon merged commit 976cf47 into elastic:master Oct 10, 2019

wylieconlon deleted the lens/telemetry branch October 10, 2019 20:36

wylieconlon mentioned this pull request Oct 10, 2019

[7.x] [lens] Basic usage telemetry for total visualizations, and by type (#47597) #47899

Merged

rayafratkina mentioned this pull request Oct 22, 2019

[Lens] User interaction telemetry for Lens #46599

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lens] Basic usage telemetry for total visualizations, and by type #47597

[lens] Basic usage telemetry for total visualizations, and by type #47597

wylieconlon commented Oct 8, 2019 •

edited

Loading

elasticmachine commented Oct 8, 2019

AlonaNadler left a comment

elasticmachine commented Oct 8, 2019

wylieconlon commented Oct 8, 2019

elasticmachine commented Oct 8, 2019

chrisdavies left a comment

chrisdavies Oct 9, 2019

chrisdavies Oct 9, 2019

chrisdavies Oct 9, 2019

wylieconlon Oct 10, 2019

chrisdavies Oct 9, 2019

chrisdavies Oct 9, 2019

elasticmachine commented Oct 9, 2019

elasticmachine commented Oct 10, 2019

chrisdavies left a comment

chrisdavies Oct 10, 2019

elasticmachine commented Oct 10, 2019

[lens] Basic usage telemetry for total visualizations, and by type #47597

[lens] Basic usage telemetry for total visualizations, and by type #47597

Conversation

wylieconlon commented Oct 8, 2019 • edited Loading

elasticmachine commented Oct 8, 2019

AlonaNadler left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 8, 2019

💔 Build Failed

wylieconlon commented Oct 8, 2019

elasticmachine commented Oct 8, 2019

💔 Build Failed

chrisdavies left a comment

Choose a reason for hiding this comment

chrisdavies Oct 9, 2019

Choose a reason for hiding this comment

chrisdavies Oct 9, 2019

Choose a reason for hiding this comment

chrisdavies Oct 9, 2019

Choose a reason for hiding this comment

wylieconlon Oct 10, 2019

Choose a reason for hiding this comment

chrisdavies Oct 9, 2019

Choose a reason for hiding this comment

chrisdavies Oct 9, 2019

Choose a reason for hiding this comment

elasticmachine commented Oct 9, 2019

💚 Build Succeeded

elasticmachine commented Oct 10, 2019

💚 Build Succeeded

chrisdavies left a comment

Choose a reason for hiding this comment

chrisdavies Oct 10, 2019

Choose a reason for hiding this comment

elasticmachine commented Oct 10, 2019

💚 Build Succeeded

wylieconlon commented Oct 8, 2019 •

edited

Loading