[Reporting/Telemetry] Include counts for each deprecated job type #108614

tsullivan · 2021-08-13T22:48:44Z

Summary

This PR adds an explicit deprecated count to each export type, which makes deprecating code in Reporting more flexible for telemetry:

you don't have to know which export types are deprecated to find usage of deprecated reporting features
you can compare overall deprecated count with overall report count.

Telemetry data can show how often deprecated export types are used. Prior to this PR, the export type itself would have to be deprecated, and we would find the usage of deprecated export types by comparing the usage of each export type.

When is merged, the printable_pdf and PNG, report jobs will be conditionally marked as deprecated, based on whether the job parameters used the 6.2 savedObjectId style, or the later relativeUrls style. Since the job type can not be used to see if the job is deprecated, this PR adds extra logic to telemetry collection in Reporting and gives an explicit flag on if the job is deprecated.

Testing:

In Kibana 7.12, import test data, create a saved search in Discover, and copy the POST URL for CSV export.
Create a script that sends the POST URL to Kibana to request a CSV report. Example: https://gist.github.com/tsullivan/0b86d5e1165f6460a876b4f63012fdd0
Create a few report jobs using the script
Export the saved objects in Stack Management > Saved Objects
Upgrade ES to 8.0, or import the test data and saved objects in ES 8.0 with running Kibana from source checked out to this PR.
Create a few more report jobs using the script
Run the script that uses the POST URL copied in 7.12.

With the reporting index set up with deprecated job types, you can then query the internal API for usage, and see that it contains info about the deprecated job types.

Run a curl command on the usage endpoint and pipe the response data to jq:

 curl 'http://elastic:changeme@localhost:5601/api/telemetry/v2/clusters/_stats' --compressed \
  -H 'Content-Type: application/json' \
  -H 'kbn-version: 8.0.0' \
  --data-raw '{"unencrypted":true}' \
  | jq '.[0].stack_stats.kibana.plugins.reporting|{l7D:.last7Days|{csv,csv_searchsource},csv,csv_searchsource}'

Example output:

{
"l7D": {
  "csv": {
    "available": true,
    "total": 4,
    "deprecated": 4
  },
  "csv_searchsource": {
    "available": true,
    "total": 5,
    "deprecated": 0
  }
},
"csv": {
  "available": true,
  "total": 4,
  "deprecated": 4
},
"csv_searchsource": {
  "available": true,
  "total": 5,
  "deprecated": 0
}
}

TODO:

Mark all count of a job type as deprecated, if the job type itself is deprecated

Checklist

Delete any items that are not applicable to this PR.

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

tsullivan · 2021-08-13T23:31:44Z

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

-  };
-  doc_count: number;
-}
-


These lines of code were mysteriously duplicated in the file

🧙🏻‍♂️

tsullivan · 2021-08-15T04:53:10Z

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

-  | 'failed'
-  | 'pending'
-  | 'processing';
+type Statuses = 'completed' | 'completed_with_warnings' | 'failed' | 'pending' | 'processing';


Removed non-existent cancelled status

tsullivan · 2021-08-17T20:26:59Z

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

  PDF_JOB_TYPE,
  PNG_JOB_TYPE,
 } from '../../common/constants';
 import { AvailableTotal, ExportType, FeatureAvailabilityMap, RangeStats } from './types';

+const jobTypeIsDeprecated = (jobType: string) => DEPRECATED_JOB_TYPES.includes(jobType);
+
 function getForFeature(
  range: Partial<RangeStats>,
  typeKey: ExportType,
  featureAvailability: FeatureAvailabilityMap,
  additional?: any


I took a crack at fixing this any but was not able to see a clean fix for it yet

You can use generics as a clean fix.

@dokmic I merged your suggestion and there is still a lot of ambiguity in the body of this function. I reverted the commit for now, but let's keep track of this. It is a very nasty any and in a big way, it indicates the logic of the function could be improved.

tsullivan · 2021-08-17T20:29:29Z

x-pack/plugins/reporting/server/usage/reporting_usage_collector.test.ts

@@ -559,9 +722,6 @@ describe('Ready for collection observable', () => {
              },
            },
            "status": Object {
-              "cancelled": Object {


cancelled is not a valid status

elasticmachine · 2021-08-17T21:28:50Z

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

elasticmachine · 2021-08-17T21:28:52Z

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

tsullivan · 2021-08-17T22:52:03Z

@elasticmachine merge upstream

TinaHeiligers · 2021-08-17T23:34:44Z

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

@@ -4105,9 +4117,6 @@
        },
        "status": {
          "properties": {
-            "cancelled": {


Removing these from the telemetry indices will require a re-index operation. Please make sure that you follow up with infra to let them know.

TinaHeiligers

Telemetry (and therefore core) changes LGTM. Please remember to follow up with Infra to get cancelled removed from the mappings.

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

dokmic

Code changes LGTM 👍

jloleysens

Code changes look great to me! Thanks for adding this @tsullivan

I tested this locally and report generation and telemetry collection worked as expected.

I think the PR description needs to update isDeprectated -> deprecated as that what it looks like we are currently getting back from the Kibana stats API.

Co-authored-by: Michael Dokolin <[email protected]>

…tats.ts" This reverts commit 2704fe7.

kibanamachine · 2021-08-20T02:24:11Z

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`reporting`	67.2KB	67.4KB	+155.0B

History

💔 Build #146984 failed 2704fe7
💛 Build #146423 was flaky 9c47862
💚 Build #146424 succeeded 128181f
💔 Build #146393 failed 12ef82a
💚 Build #146363 succeeded 1fe51ff7a8b417789b5ccd738c3d77ef2042ffa6

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

…astic#108614) * add isDeprecated to report meta for telemetry * test clean up * update snapshots * Update x-pack/plugins/reporting/server/usage/decorate_range_stats.ts Co-authored-by: Michael Dokolin <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Michael Dokolin <[email protected]>

…08614) (#109366) * add isDeprecated to report meta for telemetry * test clean up * update snapshots * Update x-pack/plugins/reporting/server/usage/decorate_range_stats.ts Co-authored-by: Michael Dokolin <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Michael Dokolin <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Michael Dokolin <[email protected]>

tsullivan marked this pull request as draft August 13, 2021 23:01

tsullivan commented Aug 13, 2021

View reviewed changes

tsullivan changed the title ~~[Reporting/Telemetry] Include counts for each job type on the number of deprecated exports found~~ [Reporting/Telemetry] Include counts for each deprecated job type Aug 13, 2021

tsullivan added (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Feature:Telemetry Team:AppServices release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v8.0.0 labels Aug 13, 2021

tsullivan force-pushed the reporting/telemetry-for-deprecated branch 2 times, most recently from 0325f4f to 813f3df Compare August 14, 2021 16:52

tsullivan commented Aug 15, 2021

View reviewed changes

tsullivan force-pushed the reporting/telemetry-for-deprecated branch from b75313e to 1fe51ff Compare August 17, 2021 20:25

tsullivan commented Aug 17, 2021

View reviewed changes

tsullivan force-pushed the reporting/telemetry-for-deprecated branch 4 times, most recently from b9e1e74 to 74177a4 Compare August 17, 2021 21:22

tsullivan added 2 commits August 17, 2021 14:25

add isDeprecated to report meta for telemetry

40df702

test clean up

12ef82a

tsullivan force-pushed the reporting/telemetry-for-deprecated branch from 74177a4 to 12ef82a Compare August 17, 2021 21:25

tsullivan marked this pull request as ready for review August 17, 2021 21:28

tsullivan requested review from a team as code owners August 17, 2021 21:28

tsullivan requested review from dokmic and jloleysens August 17, 2021 21:29

tsullivan requested review from a team August 17, 2021 21:29

update snapshots

9c47862

Merge branch 'master' into reporting/telemetry-for-deprecated

128181f

tsullivan added v7.16.0 and removed v7.15.0 labels Aug 17, 2021

TinaHeiligers reviewed Aug 17, 2021

View reviewed changes

TinaHeiligers approved these changes Aug 17, 2021

View reviewed changes

dokmic reviewed Aug 19, 2021

View reviewed changes

x-pack/plugins/reporting/server/usage/decorate_range_stats.ts Show resolved Hide resolved

dokmic approved these changes Aug 19, 2021

View reviewed changes

jloleysens approved these changes Aug 19, 2021

View reviewed changes

Update x-pack/plugins/reporting/server/usage/decorate_range_stats.ts

2704fe7

Co-authored-by: Michael Dokolin <[email protected]>

tsullivan enabled auto-merge (squash) August 19, 2021 16:17

tsullivan added 2 commits August 19, 2021 16:51

Merge branch 'master' into reporting/telemetry-for-deprecated

d14a1f8

Revert "Update x-pack/plugins/reporting/server/usage/decorate_range_s…

51fa243

…tats.ts" This reverts commit 2704fe7.

tsullivan merged commit 91630c6 into elastic:master Aug 20, 2021

tsullivan deleted the reporting/telemetry-for-deprecated branch August 20, 2021 05:50

tsullivan mentioned this pull request Aug 20, 2021

[7.x] [Reporting/Telemetry] Include counts for each deprecated job type (#108614) #109366

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reporting/Telemetry] Include counts for each deprecated job type #108614

[Reporting/Telemetry] Include counts for each deprecated job type #108614

tsullivan commented Aug 13, 2021 •

edited

Loading

tsullivan Aug 13, 2021

jloleysens Aug 19, 2021

tsullivan Aug 15, 2021

tsullivan Aug 17, 2021

dokmic Aug 19, 2021

tsullivan Aug 20, 2021

tsullivan Aug 17, 2021

elasticmachine commented Aug 17, 2021

elasticmachine commented Aug 17, 2021

tsullivan commented Aug 17, 2021

TinaHeiligers Aug 17, 2021

TinaHeiligers left a comment

dokmic left a comment

jloleysens left a comment

kibanamachine commented Aug 20, 2021

[Reporting/Telemetry] Include counts for each deprecated job type #108614

[Reporting/Telemetry] Include counts for each deprecated job type #108614

Conversation

tsullivan commented Aug 13, 2021 • edited Loading

Summary

Testing:

TODO:

Checklist

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 Aug 17, 2021

elasticmachine commented Aug 17, 2021

tsullivan commented Aug 17, 2021

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

dokmic left a comment

Choose a reason for hiding this comment

jloleysens left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 20, 2021

💚 Build Succeeded

Metrics [docs]

Page load bundle

History

tsullivan commented Aug 13, 2021 •

edited

Loading