[Telemetry] Report Kibana distro in local collectors + Usage Collectors in TS

[afharo]

Summary

• Add the missing os.distros and os.distroReleases to the local collectors in Kibana
• Add typings to the .js files in the usage_collector plugins (move them to .ts).

Fixes https://github.com/elastic/telemetry/issues/210

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

• [ ] This was checked for cross-browser compatibility, including a check against IE11
• [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• [ ] Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• [ ] This was checked for keyboard-only and screenreader accessibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately
• This includes a feature addition or change that requires a release note and was labeled appropriately

[elasticmachine]

Pinging @elastic/pulse (Team:Pulse)

[afharo]

@elasticmachine merge upstream

[Bamieh]

Honestly i'm not a fan of heavily using generics in types. The consumer of collectors (collector set) does not care about collector types since we are looping over all of them and calling their methods, so adding generic types will not be useful it only adds extra steps.

+1 for converting to TS though.

[Bamieh]

I believe callCluster should be optional

  fetch: (callCluster?: CallCluster) => Promise<T> | T;

There is no need to pass generic T at the interface level. We do not need the fetch return type when registering the collector. It could be scoped to fetch or simply the use of ..) => unknown is enough.

[afharo]

I believe callCluster should be optional

If we do it optional, every fetch will need to check if callCluster is not undefined before using it, but from the CollectorSet we are always providing it (and we should be enforced to provide it).

Declaring your fetch method without any argument if not using the callCluster parameter is still fine for TS (and even encouraged by eslint). A quick TS-safe example I just tested below:

  usageCollection.makeUsageCollector({
    type: KIBANA_REPORTING_TYPE,
    isReady,
    fetch: () => ({ count: 1 }),
  });

There is no need to pass generic T at the interface level. We do not need the fetch return type when registering the collector. It could be scoped to fetch or simply the use of ..) => unknown is enough.

The reason for being at the interface level is so formatForBulkUpload infers the type from the output of fetch. And it also will fail to compile if formatForBulkUpload is assuming an input different to the output of fetch.

An example is x-pack/legacy/plugins/reporting/server/usage/reporting_usage_collector.ts. You can see there is no need to specify the types when calling makeUsageCollector as it infers it automagically from the methods definitions.

[Bamieh]

Lets keep this typeof check. In case someone sets isReady to true the collectorSet will not throw when attempting to execute something other than a function.

[afharo]

Sure! I was relying on TS to ensure users will only provide a function here, but I'll add the safety check just in case anyone does it in a .js or as any cast

[Bamieh]

I'm not sure i understand the difference between the previous code and the current one?

[afharo]

The previous code was only mapping os.platforms[].platform and os.platformReleases[].platformRelease.

Now we are copying every element in kibanaStats.os (also including distro and distroRelease), but following the convention of os[keyName + "s"] = [ { [keyName]: value, count: 1 } ] (os[KEY_IN_PLURAL] = Array<{ [KEY]: string, count: number }>).

I can try changing it to something more strict and map the 2 new missing fields (distro and distroRelease), but that will result in the next time we add something to the os payload, we'll face this same issue again.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 79b67c2

History

• 💚 Build #22766 succeeded 153627a
• 💚 Build #22415 succeeded 578defe

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

[Bamieh]

LGTM thanks for the clarifications

[TinaHeiligers]

LGTM

[elasticmachine]

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