[Usage collection] Make `schema` mandatory by afharo · Pull Request #79999 · elastic/kibana

afharo · 2020-10-08T13:36:19Z

Summary

Now that all the existing collectors have a schema, let's change the TS definition to enforce every collector to include a schema definition (only for UsageCollectors).

In addition to that:

Add a bit of TS documentation to each property in CollectorOptions
Add schema to the still-in-legacy collector localization in the legacy file src/plugins/telemetry/schema/legacy_plugins.json (we might need a follow up issue to move that plugin out of the legacy dir)
Fix TS not allowing to provide additional properties in the CollectorOptions to extend the Collector class.
Remove use of any due to the issue fixed in the previous point

Checklist

Delete any items that are not applicable to this PR.

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

afharo · 2020-10-08T13:40:39Z

x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts

+        default_admin_email: { type: 'text' },
+      },
+    },
+    async fetch(this: Collector<EmailSettingData | undefined>) {


TS's fancy way of specifying the expected type for this. It doesn't affect the arguments.

afharo · 2020-10-08T15:25:04Z

src/plugins/usage_collection/server/collector/collector_set.ts

+  public makeStatsCollector = <
+    T,
+    U,
+    O extends CollectorOptions<T, U> = CollectorOptions<T, U> // Used to allow extra properties (the Collector constructor extends the class with the additional options provided)


This is to allow x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts to send the additional method getEmailValueStructure without TS complaining about it.

The logic inside the Collector class actually allows doing that.

In a follow-up PR I'll update the types inside the Collector class to better define the this context in the isReady, fetch and formatForBulkUpload methods. It requires some mocks to be updated in tests (production logic is good as it is for now), and I don't want to make the scope of this PR explode 🙂

elasticmachine · 2020-10-08T17:48:34Z

Pinging @elastic/kibana-telemetry (Team:KibanaTelemetry)

chrisronline

LGTM for stack monitoring!

lukeelmers

App arch docs change LGTM (not sure why the generated markdown changed, but I'm not seeing anything concerning)

TinaHeiligers · 2020-10-12T23:53:49Z

@Bamieh we'll need to take care of this PR for @afharo.

Bamieh · 2020-10-18T11:11:02Z

@elasticmachine merge upstream

kibanamachine · 2020-10-18T11:11:05Z

merge conflict between base and head

TinaHeiligers

We were missing the import for UsageCollectorOptions https://github.com/elastic/kibana/pull/79999/files#diff-43996794a7bdd31a9969b85c6512affa813db1959068e18f9921273ac86a473bR23.

Adds `UsageCollectorOptions` to `collector_set.test.ts`

TinaHeiligers

LGTM

Bamieh · 2020-10-21T14:41:13Z

@elasticmachine merge upstream

Bamieh · 2020-10-26T09:09:32Z

@elasticmachine merge upstream

kibanamachine · 2020-10-26T10:46:50Z

💚 Build Succeeded

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

Metrics [docs]

distributable file count

id	before	after	diff
`default`	48076	48077	+1
`oss`	28590	28591	+1

History

💚 Build #83080 succeeded 5e5d757
💚 Build #82586 succeeded fb9176e
💔 Build #82324 failed 38f7a83
💚 Build #80794 succeeded 0309340
💔 Build #80778 failed ef8188e

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

Co-authored-by: Ahmad Bamieh <ahmadbamieh@gmail.com> Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/usage_collection/server/collector/collector_set.test.ts # x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts

…arm-phase-to-formlib * 'master' of github.com:elastic/kibana: [Trigger Actions UI] Properly unmount app (elastic#81436) skip flaky suite (elastic#81576) skip flaky suite (elastic#78373) [Security Solution] Fix styling of EditDataProvider content (elastic#81456) [Search] Error Alignment 2 (elastic#80965) [APM] Unskip test (elastic#81574) [ML] Fix partition value selection on the Single Metric Viewer (elastic#81585) cleaning up expression service types (elastic#80643) Fix suggestions dropdown for query input (elastic#80990) [Usage collection] Make `schema` mandatory (elastic#79999) [ILM] Update show/hide data tier logic on cloud (elastic#81455) added brace import to advanced settings (elastic#81458) chore(NA): add spec-to-console and plugin-helpers as devOnly dependencies

* [Usage collection] Make `schema` mandatory (#79999) Co-authored-by: Ahmad Bamieh <ahmadbamieh@gmail.com> Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # x-pack/plugins/monitoring/server/kibana_monitoring/collectors/get_settings_collector.ts * eslint fix Co-authored-by: Alejandro Fernández Haro <alejandro.haro@elastic.co>

elasticmachine · 2021-10-01T21:30:11Z

Pinging @elastic/kibana-core (Team:Core)

[Usage collection] Make schema mandatory

6b7fe71

afharo added Feature:Telemetry v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:KibanaTelemetry v7.11.0 labels Oct 8, 2020

afharo commented Oct 8, 2020

View reviewed changes

Make schema optional for StatsCollectors

ef8188e

afharo force-pushed the usage_collection/make-schema-mandatory branch from b00f7c8 to ef8188e Compare October 8, 2020 15:20

afharo commented Oct 8, 2020

View reviewed changes

Accept Doc API changes 🤷

0309340

afharo marked this pull request as ready for review October 8, 2020 17:48

afharo requested review from a team as code owners October 8, 2020 17:48

afharo requested a review from a team October 8, 2020 17:48

chrisronline approved these changes Oct 8, 2020

View reviewed changes

lukeelmers approved these changes Oct 12, 2020

View reviewed changes

Merge branch 'master' into usage_collection/make-schema-mandatory

38f7a83

TinaHeiligers requested changes Oct 19, 2020

View reviewed changes

Adds missing type import

fb9176e

Adds `UsageCollectorOptions` to `collector_set.test.ts`

TinaHeiligers self-requested a review October 19, 2020 19:25

TinaHeiligers approved these changes Oct 19, 2020

View reviewed changes

TinaHeiligers requested a review from Bamieh October 19, 2020 21:09

Merge branch 'master' into usage_collection/make-schema-mandatory

5e5d757

Merge branch 'master' into usage_collection/make-schema-mandatory

e333426

Bamieh merged commit 38c6d1f into elastic:master Oct 26, 2020

This was referenced Oct 26, 2020

[7.x] [Usage collection] Make schema mandatory (#79999) #81595

Merged

[7.10] [Usage collection] Make schema mandatory (#79999) #81596

Closed

afharo deleted the usage_collection/make-schema-mandatory branch November 2, 2020 11:29

lukeelmers added the Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// label Oct 1, 2021

lukeelmers removed the Team:KibanaTelemetry label Oct 1, 2021

Conversation

afharo commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

afharo Oct 8, 2020

Choose a reason for hiding this comment

Uh oh!

afharo Oct 8, 2020

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Oct 8, 2020

Uh oh!

chrisronline left a comment

Choose a reason for hiding this comment

Uh oh!

lukeelmers left a comment

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers commented Oct 12, 2020

Uh oh!

Bamieh commented Oct 18, 2020

Uh oh!

kibanamachine commented Oct 18, 2020

Uh oh!

TinaHeiligers left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers left a comment

Choose a reason for hiding this comment

Uh oh!

Bamieh commented Oct 21, 2020

Uh oh!

Bamieh commented Oct 26, 2020

Uh oh!

kibanamachine commented Oct 26, 2020

💚 Build Succeeded

Metrics [docs]

distributable file count

History

Uh oh!

elasticmachine commented Oct 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

afharo commented Oct 8, 2020 •

edited

Loading

TinaHeiligers left a comment •

edited

Loading