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.

Sign up for GitHub

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

Jump to bottom

[Fleet] Get package info should not store the whole package #123509

Merged
merged 9 commits into from
Feb 9, 2022
Merged

[Fleet] Get package info should not store the whole package #123509

merged 9 commits into from
Feb 9, 2022

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Jan 20, 2022 •
edited
Loading

Summary

Resolve #122560

Do not fetch the whole package in /api/fleet/epm/pacakges/{pkgName}/{pkgVersion} instead just fetch the info from the registry.

For this I created a new method getPackageInfoFromRegistry as getPackageInfo is used in a lot of places and has a lot of side effect as it populate a cache in memory of the pakage (Something we should probably refactorize and address as a separate pull request)

It improve the first call to the API from ~5s to 500ms for package like APM, AWS.

@nchaulet nchaulet self-assigned this Jan 20, 2022
@nchaulet
Merge branch 'main' of github.com:elastic/kibana into feature-get-pac…
1cefc98 
…kage-info-should-not-store-the-whole-package
@nchaulet nchaulet mentioned this pull request Feb 8, 2022
Closed
@nchaulet nchaulet added Team:Fleet Team label for Observability Data Collection Fleet team v8.2.0 release_note:skip Skip the PR/issue when compiling release notes labels Feb 8, 2022
@nchaulet nchaulet marked this pull request as ready for review February 8, 2022 15:51
@nchaulet nchaulet requested a review from a team as a code owner February 8, 2022 15:51
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@nchaulet
Copy link
Member Author

nchaulet commented Feb 8, 2022

@elasticmachine merge upstream

@kpollich
Copy link
Member

kpollich commented Feb 8, 2022

It keep the existing behavior and use the local package stored in ES if the package is installed @kpollich I am wondering if we should simplify this and always fetch from the registry.

Yeah I think this would be good to refactor it so this page just uses the registry. The performance benefits for querying the package info from ES versus EPR are likely negligible at best.

@nchaulet
Copy link
Member Author

nchaulet commented Feb 8, 2022

Yeah I think this would be good to refactor it so this page just uses the registry. The performance benefits for querying the package info from ES versus EPR are likely negligible at best.

Great I am going to push a little more the refactor in this case.

@nchaulet nchaulet marked this pull request as draft February 8, 2022 20:02
@nchaulet nchaulet marked this pull request as ready for review February 8, 2022 21:49
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @nchaulet

@nchaulet nchaulet merged commit 79c8616 into elastic:main Feb 9, 2022
@nchaulet nchaulet deleted the feature-get-package-info-should-not-store-the-whole-package branch February 9, 2022 15:51
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 11, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123509 or prevent reminders by adding the backport:skip label.

2 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123509 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 123509 or prevent reminders by adding the backport:skip label.

@spalger spalger added the backport:skip This commit does not require backporting label Feb 15, 2022
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 15, 2022
@joshdover
Copy link
Contributor

This ends up interacting with #125525 which needs to be backported to 8.1 and 8.0 branches. I'm going to backport this along with the backport to that PR to make the fetch behavior consistent.

joshdover pushed a commit to kibanamachine/kibana that referenced this pull request Feb 16, 2022
@nchaulet @joshdover
[Fleet] Get package info should not store the whole package (elastic#…
ca6dcea 
…123509)
This was referenced Feb 16, 2022
Merged
Merged
joshdover pushed a commit to kibanamachine/kibana that referenced this pull request Feb 16, 2022
@nchaulet @joshdover
[Fleet] Get package info should not store the whole package (elastic#…
ddd759b 
…123509)
@joshdover
Copy link
Contributor

8.0 backport: #125672
8.1 backport: #125671

@joshdover joshdover added v8.0.1 v8.1.0 and removed backport:skip This commit does not require backporting labels Feb 16, 2022
kibanamachine added a commit that referenced this pull request Feb 16, 2022
@kibanamachine @joshdover @nchaulet
[8.0] [Fleet] Avoid breaking setup when compatible package is not ava…
226be06 
…ilable in registry (#125525) (#125672)

* [Fleet] Avoid breaking setup when compatible package is not available in registry (#125525)

(cherry picked from commit 928638e)

* [Fleet] Use registry version check on main (#125495)

* [Fleet] Get package info should not store the whole package (#123509)

Co-authored-by: Josh Dover <[email protected]>
Co-authored-by: Nicolas Chaulet <[email protected]>
kibanamachine added a commit that referenced this pull request Feb 16, 2022
@kibanamachine @joshdover @nchaulet
[8.1] [Fleet] Avoid breaking setup when compatible package is not ava…
1a3ea1b 
…ilable in registry (#125525) (#125671)

* [Fleet] Avoid breaking setup when compatible package is not available in registry (#125525)

(cherry picked from commit 928638e)

* [Fleet] Get package info should not store the whole package (#123509)

Co-authored-by: Josh Dover <[email protected]>
Co-authored-by: Nicolas Chaulet <[email protected]>
@joshdover joshdover mentioned this pull request Mar 4, 2022
Closed
2 tasks
@joshdover joshdover mentioned this pull request Apr 12, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kpollich kpollich kpollich approved these changes

Assignees

@nchaulet nchaulet

Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.0.1 v8.1.0 v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Fleet] Integration details performance - Package info loading
7 participants
@nchaulet @elasticmachine @kpollich @kibana-ci @kibanamachine @joshdover @spalger