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

[Usage Collection] Remove unused applicationUsageTracker from Public API #92670

Conversation

afharo
Copy link
Member

@afharo afharo commented Feb 24, 2021

Summary

As part of #91611 we are planning to remove any unused APIs in the Usage Collection service to make it less confusing to other devs, and don't overwhelm them with options.

We can revert this PR in the future if we find any use cases for it.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@afharo afharo added Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes v7.13.0 labels Feb 24, 2021
@afharo afharo force-pushed the usage_collection/remove-unused-applicationUsageTracker branch from 62f0cd8 to b3d0ab2 Compare February 24, 2021 18:01
@afharo afharo force-pushed the usage_collection/remove-unused-applicationUsageTracker branch from b3d0ab2 to 42d4b1d Compare February 25, 2021 13:22
@afharo afharo added enhancement New value added to drive a business result chore and removed enhancement New value added to drive a business result labels Feb 25, 2021
@afharo afharo marked this pull request as ready for review February 25, 2021 16:17
@afharo afharo requested review from a team as code owners February 25, 2021 16:17
@afharo afharo added the auto-backport Deprecated: Automatically backport this PR after it's merged label Feb 25, 2021
@afharo
Copy link
Member Author

afharo commented Mar 1, 2021

@elasticmachine merge upstream

// This is to avoid having to mock every private property of the class
type ApplicationUsageTrackerPublic = Pick<ApplicationUsageTracker, keyof ApplicationUsageTracker>;

export const createApplicationUsageTrackerMock = (): ApplicationUsageTrackerPublic => {
Copy link
Contributor

@TinaHeiligers TinaHeiligers Mar 1, 2021

Choose a reason for hiding this comment

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

We're still using applicationUsageTracker internally so we're not strictly removing it from the code base.

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Code review only: looks fine and cleans up our public API a bit.
One nit though: Can we change the PR title or add to the PR description that we're removing applicationUsageTracker from the public API? ATM, the title can be a little misleading and some folks (like me 😊 ) might interpret it as remove applicationusageTracker from the code base.

@afharo afharo changed the title [Usage Collection] Remove unused applicationUsageTracker [Usage Collection] Remove unused applicationUsageTracker from Public API Mar 1, 2021
@afharo afharo enabled auto-merge (squash) March 1, 2021 16:22
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
triggersActionsUi 1.6MB 1.5MB -23.9KB

Page load bundle

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

id before after diff
triggersActionsUi 104.0KB 104.1KB +82.0B
usageCollection 9.8KB 9.7KB -112.0B
total -30.0B
Unknown metric groups

async chunk count

id before after diff
triggersActionsUi 41 42 +1

History

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

@afharo afharo merged commit 4ea10d9 into elastic:master Mar 1, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Mar 1, 2021
…c API (elastic#92670)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

7.x / #93076

Successful backport PRs will be merged automatically after passing CI.

@afharo afharo deleted the usage_collection/remove-unused-applicationUsageTracker branch March 1, 2021 18:07
tsullivan pushed a commit to tsullivan/kibana that referenced this pull request Mar 1, 2021
…c API (elastic#92670)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
kibanamachine added a commit that referenced this pull request Mar 1, 2021
…c API (#92670) (#93076)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

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
auto-backport Deprecated: Automatically backport this PR after it's merged chore Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes v7.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants