Do not initialise RequestHandlerContext object when serve bundle assets

[mshustov]

Kibana uses the standard RequestHandler interface to send static assets (js bundles) to the browser.

kibana/src/core/server/core_app/bundle_routes/dynamic_asset_response.ts

Line 51 in 8f63225

}): RequestHandler<{ path: string }, {}, {}> => {

The problem with this approach is that the RequestHandler creates a RequestHandlerContext object on every invocation, even though the route handler does not use the initialized context object. Thus, Kibana performs unnecessary work since most properties extending the RequestHandlerContext object are instantiated eagerly.

Proposed Solution

Kibana provides API to serve static assets

kibana/src/core/server/core_app/core_app.ts

Lines 201 to 204 in 8f63225

	core.http.registerStaticDir(
	'/node_modules/@kbn/ui-framework/dist/{path*}',
	fromRoot('node_modules/@kbn/ui-framework/dist')
	);

CoreApp service can be refactored to use it, or Core can provide an alternative implementation that doesn't require RequestHandlerContext initialization on every call.

Another option could be to refactor Context service to build context objects lazily on demand

kibana/src/core/server/context/container/context.ts

Line 227 in 8f63225

const context = await this.buildContext(source, ...args);

[elasticmachine]

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

[pgayvallet]

CoreApp service can be refactored to use it

I though @hapi/inert was not performing compressed version lookup on the disk, but looking at

kibana/src/core/server/http/http_server.ts

Lines 487 to 491 in d5416ed

	directory: {
	path: dirPath,
	listing: false,
	lookupCompressed: true,
	},

it does.

So the only difference between registerStaticDir and createDynamicAssetHandler I see is the cache control/policy:

registerStaticDir always uses:

kibana/src/core/server/http/http_server.ts

Lines 495 to 498 in d5416ed

	cache: {
	privacy: 'public',
	otherwise: 'must-revalidate',
	},

where createDynamicAssetHandler uses a strategy depending on the environment:

kibana/src/core/server/core_app/bundle_routes/dynamic_asset_response.ts

Lines 77 to 86 in 25e586a

	if (isDist) {
	headers = { 'cache-control': `max-age=${365 * DAY}` };
	} else {
	const stat = await fstat(fd);
	const hash = await getFileHash(fileHashCache, path, stat, fd);
	headers = {
	etag: `${hash}-${publicPath}`,
	'cache-control': 'must-revalidate',
	};
	}

If we were to change registerStaticDir to accept the cache strategy as an option, I think we could stop using createDynamicAssetHandler. Not 100% sure how to use an etag with hapi/inert, but I assume this is possible given its a very basic need for static assets (it may even be the default when using privacy: 'public',)

Another option could be to refactor Context service to build context objects lazily on demand

More work, but that's ihmo the end goal here, as it will improve performance for all routes and not only the bundles ones.

We could even think of an option to allow route registration to totally opt-out of context handling/wrapping. This may be type-hell though, as the handlers would change from handler(ctx, req, res) to handler(req, res) depending on that option.

[pgayvallet]

So I took a shot as using registerStaticDir (inert) instead of our custom file serving handler (#129871), and two observations:

1. I couldn't find a way to force HAPI to not add the must-revalidate directive to cache-control for distribution mode

Expected: max-age=31536000
Received: max-age=31536000, must-revalidate

Note that looking at https://developer.mozilla.org/en-US/docs/Web/HTTP/Caching#freshness, revalidation should not be performed until the max-age based freshness expires, so it may not, in fact be an issue? Could anyone with better http-caching experience than I have confirm (or infirm) this?

2. We were using a internal file hash cache (FileHashCache) in our custom handler to avoid unnecessarily recomputing each file's etag in dev mode

This will go away if using registerStaticDir (but given it's only for dev mode, this doesn't look like a real problem)

[pgayvallet]

Also linking to #84763 which is the opened issue about

Another option could be to refactor Context service to build context objects lazily on demand

[pgayvallet]

#84763 was closed by #129896, should we consider this issue resolved too?

[lukeelmers]

Yeah I think we can close as resolved by #129896