[Reporting] Remove `any`, improve telemetry schema for completeness #111212

tsullivan · 2021-09-03T20:11:13Z

Summary

Removes type ambiguity when the Reporting usage collector crafts the data for counting by exportType-per-app and layout-per-app.

Makes app and layout fields of the usage data applicable to all the export types, not just PDF
Theoretically, CSV could be created in different apps, not just search. Or, CSV/PNG could have different layouts. Those use cases aren't supported in Kibana today but there's nothing blocking anyone from working on them.
Adds canvas workpad and search to the set of app names used as indexes in the of per-app usage data.
Adds canvas to the set of layout names used as indexes in the per-layout usage data. (This should have been done in #84959(
Renamed decorateRangeStats to getExportStats to use a better name

Previously, only the PDF export type was the only one to count the app and layout fields in the usage data. This is hard to maintain from a types point of view. Also, CSV exports could theoretically have been created from different apps, not just Discover.

Notes

the Discover app is referred to as search in fields that count the number of CSV reports created.
the terms "exportType" and "jobType" are interchangeable in this part of the code. A "jobType" comes from a registered export type's jobType field and is used to uniquely identify the export type for telemetry.
the mappings in the telemetry cluster do not need to be updated as a result of this cleanup.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2021-09-03T20:11:51Z

Pinging @elastic/kibana-app-services (Team:AppServices)

elasticmachine · 2021-09-03T20:11:51Z

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

tsullivan · 2021-09-03T20:28:01Z

x-pack/plugins/reporting/server/usage/get_export_stats.ts

+ * 2.0.
+ */
+
+import { DEPRECATED_JOB_TYPES } from '../../common/constants';


This file was rewritten and renamed: decorate_range_stats.ts → get_export_stats.ts

Previously, this code had a tangle of type ambiguity in the getAvailableTotalForFeature function

tsullivan · 2021-09-03T20:37:58Z

x-pack/plugins/reporting/server/usage/types.ts

@@ -117,8 +119,7 @@ export type ReportingUsageType = RangeStats & {
  last7Days: RangeStats;
 };

-export type ExportType = 'csv' | 'csv_searchsource' | 'printable_pdf' | 'PNG';
-export type FeatureAvailabilityMap = { [F in ExportType]: boolean };
+export type FeatureAvailabilityMap = Record<string, boolean>;


Changed this to be indexed by string instead ExportType for easier maintenance.

Instead of keeping a hardcoded list of all the known export types, we can use exportTypesHandler.getJobTypes(): it returns the jobTypes of all registered export types.

tsullivan · 2021-09-03T20:38:47Z

x-pack/plugins/reporting/server/usage/get_export_stats.ts

+  } = rangeStatsInput;
+
+  // combine the known types with any unknown type found in reporting data
+  const statsForExportType = exportTypesHandler.getJobTypes().reduce((accum, exportType) => {


exportTypesHandler.getJobTypes() replaces the need for a hardcoded list of all the job types. See https://github.com/elastic/kibana/pull/111212/files#r702149771

jloleysens

Thanks for cleaning up the types and clearly structured commits and comments 👍🏻

It looks like we need to add csv_searchsource_immediate as another export type to the telemetry schema. I'll approve because everything else looks good.

jloleysens · 2021-09-06T09:34:09Z

x-pack/plugins/reporting/server/usage/types.ts

 }

+// FIXME: find a way to get this from exportTypesHandler or common/constants


jloleysens · 2021-09-06T09:53:56Z

@elasticmachine merge upstream

pgayvallet · 2021-09-07T09:07:01Z

x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json

+            },
+            "app": {
+              "properties": {
+                "search": {
+                  "type": "long"
+                },


@Bamieh that's a lot of additions, is that fine with you?

Thanks for raising this @pgayvallet . The main reason why there are so many additions is that adding new export types multiplies what we track with telemetry:

PDF

PNG

CSV

PDFv2

PNGv2

CSVv2

CSV Immediate Download

For every export type, there is telemetry tracking for which apps were used, and what the layout options were.

This PR adds all of the V2 types to the schema

I was on holidays this week. It is fine. better than the alternatives!

tsullivan · 2021-09-07T23:43:54Z

@elasticmachine merge upstream

kibanamachine · 2021-09-08T01:55:23Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…lastic#111212) * [Reporting] Remove `any`, improve telemetry schema for completeness * remove another any * rename file per exported function * test variable name * use variable for DRY * update reporting telemetry contract * added csv_searchsource_immediate to telemetry * fix types * update jest snapshots * remove tests on large literal objects Co-authored-by: Jean-Louis Leysens <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

…111212) (#111665) * [Reporting] Remove `any`, improve telemetry schema for completeness * remove another any * rename file per exported function * test variable name * use variable for DRY * update reporting telemetry contract * added csv_searchsource_immediate to telemetry * fix types * update jest snapshots * remove tests on large literal objects Co-authored-by: Jean-Louis Leysens <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Jean-Louis Leysens <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

tsullivan added 2 commits September 3, 2021 13:08

[Reporting] Remove any, improve telemetry schema for completeness

b0d5566

remove another any

bfb4056

tsullivan requested review from a team as code owners September 3, 2021 20:11

rename file per exported function

4fea726

tsullivan force-pushed the reporting/fix-telemetry-types branch from 17ac8ca to 4fea726 Compare September 3, 2021 20:16

test variable name

6f178a1

tsullivan commented Sep 3, 2021

View reviewed changes

use variable for DRY

771a191

tsullivan commented Sep 3, 2021

View reviewed changes

update reporting telemetry contract

06b021d

tsullivan requested review from a team as code owners September 3, 2021 20:40

elastic deleted a comment from kibanamachine Sep 3, 2021

added csv_searchsource_immediate to telemetry

4d6b6a9

jloleysens approved these changes Sep 6, 2021

View reviewed changes

Merge branch 'master' into reporting/fix-telemetry-types

17bad33

pgayvallet reviewed Sep 7, 2021

View reviewed changes

jloleysens and others added 3 commits September 7, 2021 11:36

fix types

663e413

update jest snapshots

206678c

remove tests on large literal objects

69a45f0

Merge branch 'master' into reporting/fix-telemetry-types

6642010

pgayvallet approved these changes Sep 8, 2021

View reviewed changes

tsullivan merged commit c4c6536 into elastic:master Sep 8, 2021

tsullivan mentioned this pull request Sep 8, 2021

[7.x] [Reporting] Remove any, improve telemetry schema for completeness (#111212) #111665

Merged

tsullivan deleted the reporting/fix-telemetry-types branch September 9, 2021 00:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reporting] Remove `any`, improve telemetry schema for completeness #111212

[Reporting] Remove `any`, improve telemetry schema for completeness #111212

tsullivan commented Sep 3, 2021 •

edited

Loading

elasticmachine commented Sep 3, 2021

elasticmachine commented Sep 3, 2021

tsullivan Sep 3, 2021 •

edited

Loading

tsullivan Sep 3, 2021

tsullivan Sep 3, 2021

jloleysens left a comment

jloleysens Sep 6, 2021

jloleysens commented Sep 6, 2021

pgayvallet Sep 7, 2021

tsullivan Sep 7, 2021 •

edited

Loading

Bamieh Sep 9, 2021

tsullivan commented Sep 7, 2021

kibanamachine commented Sep 8, 2021

		}

		// FIXME: find a way to get this from exportTypesHandler or common/constants

[Reporting] Remove any, improve telemetry schema for completeness #111212

[Reporting] Remove any, improve telemetry schema for completeness #111212

Conversation

tsullivan commented Sep 3, 2021 • edited Loading

Summary

Notes

Checklist

elasticmachine commented Sep 3, 2021

elasticmachine commented Sep 3, 2021

tsullivan Sep 3, 2021 • edited Loading

Choose a reason for hiding this comment

tsullivan Sep 3, 2021

Choose a reason for hiding this comment

tsullivan Sep 3, 2021

Choose a reason for hiding this comment

jloleysens left a comment

Choose a reason for hiding this comment

jloleysens Sep 6, 2021

Choose a reason for hiding this comment

jloleysens commented Sep 6, 2021

pgayvallet Sep 7, 2021

Choose a reason for hiding this comment

tsullivan Sep 7, 2021 • edited Loading

Choose a reason for hiding this comment

Bamieh Sep 9, 2021

Choose a reason for hiding this comment

tsullivan commented Sep 7, 2021

kibanamachine commented Sep 8, 2021

💚 Build Succeeded

Metrics [docs]

History

[Reporting] Remove `any`, improve telemetry schema for completeness #111212

[Reporting] Remove `any`, improve telemetry schema for completeness #111212

tsullivan commented Sep 3, 2021 •

edited

Loading

tsullivan Sep 3, 2021 •

edited

Loading

tsullivan Sep 7, 2021 •

edited

Loading