Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API DOCS] Telemetry #98610

Merged
merged 3 commits into from
Apr 29, 2021
Merged

[API DOCS] Telemetry #98610

merged 3 commits into from
Apr 29, 2021

Conversation

afharo
Copy link
Member

@afharo afharo commented Apr 28, 2021

Summary

Resolves #98516: This PR cleans up all the unused APIs exposed by the telemetry plugin and documents the exposed ones.

When running node scripts/build_api_docs --plugin telemetry --stats any --stats comments --stats exports we still get the following warnings.

 info --- Plugin 'telemetry' ----`
 info 0 API items with ANY
┌─────────┐
│ (index) │
├─────────┤
└─────────┘
 info 4 API items missing comments
┌─────────┬─────────────────────────────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ (index) │               id                │                                                            link                                                            │
├─────────┼─────────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│    0    │ 'def-server.getClusterUuids.$1' │ 'https://github.com/elastic/kibana/tree/master/src/plugins/telemetry/server/telemetry_collection/get_cluster_stats.ts#L26' │
│    1    │  'def-server.getLocalStats.$1'  │  'https://github.com/elastic/kibana/tree/master/src/plugins/telemetry/server/telemetry_collection/get_local_stats.ts#L63'  │
│    2    │  'def-server.getLocalStats.$2'  │  'https://github.com/elastic/kibana/tree/master/src/plugins/telemetry/server/telemetry_collection/get_local_stats.ts#L64'  │
│    3    │  'def-server.getLocalStats.$3'  │  'https://github.com/elastic/kibana/tree/master/src/plugins/telemetry/server/telemetry_collection/get_local_stats.ts#L65'  │
└─────────┴─────────────────────────────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

This is because the current tool is not able to link the JSDocs and the methods declared as const myFn = ({ destructVar }) => arrowFnResult. However, hopefully, when we complete #93185, we'll be able to stop exposing those APIs.

UPDATE: Setting @internal to those APIs solves the issue. Since they are only used internally by the X-Pack Telemetry extension, I think that's a good enough approach in this case 👍. Running the same command again, we get:

 info --- Plugin 'telemetry'  passes all checks ----

Checklist

  • Documentation was added for features that require explanation or tutorials

For maintainers

@afharo afharo added Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes v7.14.0 APIDocs Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:KibanaTelemetry labels Apr 28, 2021
@afharo afharo marked this pull request as ready for review April 28, 2021 16:39
@afharo afharo requested a review from a team as a code owner April 28, 2021 16:39
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@afharo afharo requested a review from stacey-gammon April 28, 2021 16:39
});

it('returns true if lastReported is undefined', () => {
const telemetryService = mockTelemetryService();
telemetryService.getIsOptedIn = jest.fn().mockReturnValue(true);
const telemetrySender = new TelemetrySender(telemetryService);
const shouldSendRerpot = telemetrySender['shouldSendReport']();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure that was me with this faulty mac keyboard 😓

Copy link
Member Author

@afharo afharo Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or me 😅 I have a spellchecker in my IDE because I'm always mixing up keystrokes... In mind, I think that I know what I'm doing, but my fingers think otherwise... 🤦

Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :elasticheart:

@afharo afharo force-pushed the api_docs/telemetry branch from 9c4e799 to 5b0472a Compare April 29, 2021 08:51
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
telemetry 78 0 -78

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
telemetry 1 0 -1

Page load bundle

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

id before after diff
telemetry 35.9KB 36.4KB +495.0B
Unknown metric groups

API count

id before after diff
telemetry 83 40 -43

History

  • 💚 Build #123060 succeeded 9c4e7996ef3e5627d1dd1f25f6ec6c58860cbcaa
  • 💚 Build #122964 succeeded acc2b721e1ca8dfacc2649c5da8073c15781f313

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

@afharo afharo added the auto-backport Deprecated - use backport:version if exact versions are needed label Apr 29, 2021
@afharo afharo merged commit ed2e544 into elastic:master Apr 29, 2021
@afharo afharo deleted the api_docs/telemetry branch April 29, 2021 13:38
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 29, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Apr 29, 2021
Co-authored-by: Alejandro Fernández Haro <alejandro.haro@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APIDocs auto-backport Deprecated - use backport:version if exact versions are needed Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Telemetry] Complete the API docs
5 participants