elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.73k stars 8.14k forks source link

Update cache policy for plugins public assets and UI shared dependencies #140761

Closed tylersmalley closed 5 months ago

tylersmalley commented 2 years ago

In https://github.com/elastic/kibana/pull/64414, we updated the cache policy for plugin bundles to include the build number in the URL. This allowed us to update the cache-control policy of the assets to essentially cache indefinitely instead of requiring revalidation.

There are still static assets on page load that do not follow this policy, we should also update them to improve the end-user experience and prevent the HEAD requests.

Examples:

image

elasticmachine commented 2 years ago

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

pgayvallet commented 2 years ago

For core public assets:

FWIW, Core static files are not served the same way plugin bundles are, they are not using our custom handler but just registerStaticDir (that delegates to @hapi/inert)

https://github.com/elastic/kibana/blob/885e80a1cde3520e18aeefdbc6955d83226cfcc7/src/core/server/core_app/core_app.ts#L206-L211

and @hapi/inert does not support this out of the box: https://github.com/hapijs/inert/issues/163

We would need to manually override the header via the onPostHandler hook in http.registerStaticDir as suggested by the maintainers in the issue (and expose an option to do so, as we don't always want this behavior).

This is doable I believe, though, as only Core imports those files, so changing the paths to add the buildNum should be fine.

Also related, as I tried to migrate plugin bundles to use inert when trying to increase performance: https://github.com/elastic/kibana/pull/129871

For plugins public assets

Paths to such public assets are hardcoded in plugins code from the client-side code.

E.g

https://github.com/elastic/kibana/blob/753c86bfbefaf58930b4dbde5931df7bfc6e72f3/src/plugins/home/public/application/components/guided_onboarding/use_case_card.tsx#L109-L111

Adding the build number to the path of such assets would required to update ALL usages of these assets, which feels like a hard no-go for me, or at least totally not worth the risk and cross-team effort (but I'm of course open to discussion here @tylersmalley ).

pgayvallet commented 5 months ago

The recent CDN capabilities supersedes (and addresses) this issue, I'll go ahead and close it.