Skip to content

Commit

Permalink
Core: minor performance optimizations (#170385)
Browse files Browse the repository at this point in the history
## Summary

Quick performances wins around Core's hot path
  • Loading branch information
pgayvallet authored Nov 6, 2023
1 parent 303c583 commit fd7fcdf
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ export class ExecutionContextService
// we have to use enterWith since Hapi lifecycle model is built on event emitters.
// therefore if we wrapped request handler in asyncLocalStorage.run(), we would lose context in other lifecycles.
this.contextStore.enterWith(contextContainer);
this.log.debug(JSON.stringify(contextContainer));
if (this.log.isLevelEnabled('debug')) {
this.log.debug(JSON.stringify(contextContainer));
}
}

private withContext<R>(
Expand All @@ -130,7 +132,9 @@ export class ExecutionContextService
}
const parent = this.contextStore.getStore();
const contextContainer = new ExecutionContextContainer(context, parent);
this.log.debug(JSON.stringify(contextContainer));
if (this.log.isLevelEnabled('debug')) {
this.log.debug(JSON.stringify(contextContainer));
}

return this.contextStore.run(contextContainer, fn);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('CoreKibanaRequest', () => {
});
});

describe('isSytemApi property', () => {
describe('isSystemRequest property', () => {
it('is false when no kbn-system-request header is set', () => {
const request = hapiMocks.createRequest({
headers: { custom: 'one' },
Expand Down
50 changes: 27 additions & 23 deletions packages/core/http/core-http-router-server-internal/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,18 @@ export class CoreKibanaRequest<
*/
public static from<P, Q, B>(
req: RawRequest,
routeSchemas: RouteValidator<P, Q, B> | RouteValidatorFullConfig<P, Q, B> = {},
routeSchemas:
| RouteValidator<P, Q, B>
| RouteValidatorFullConfig<P, Q, B>
| undefined = undefined,
withoutSecretHeaders: boolean = true
) {
const routeValidator = RouteValidator.from<P, Q, B>(routeSchemas);
let requestParts: { params: P; query: Q; body: B };
if (isFakeRawRequest(req)) {
if (routeSchemas === undefined || isFakeRawRequest(req)) {
requestParts = { query: {} as Q, params: {} as P, body: {} as B };
} else {
const rawParts = CoreKibanaRequest.sanitizeRequest(req);
const routeValidator = RouteValidator.from<P, Q, B>(routeSchemas);
const rawParts = sanitizeRequest(req);
requestParts = CoreKibanaRequest.validate(rawParts, routeValidator);
}
return new CoreKibanaRequest(
Expand All @@ -79,22 +82,6 @@ export class CoreKibanaRequest<
);
}

/**
* We have certain values that may be passed via query params that we want to
* exclude from further processing like validation. This method removes those
* internal values.
*/
private static sanitizeRequest<P, Q, B>(
req: Request
): { query: unknown; params: unknown; body: unknown } {
const { [ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM]: __, ...query } = req.query ?? {};
return {
query,
params: req.params,
body: req.payload,
};
}

/**
* Validates the different parts of a request based on the schemas defined for
* the route. Builds up the actual params, query and body object that will be
Expand Down Expand Up @@ -156,14 +143,16 @@ export class CoreKibanaRequest<
// KibanaRequest in conjunction with scoped Elasticsearch and SavedObjectsClient in order to pass credentials.
// In these cases, the ids default to a newly generated UUID.
const appState = request.app as KibanaRequestState | undefined;
const isRealReq = isRealRawRequest(request);

this.id = appState?.requestId ?? uuidv4();
this.uuid = appState?.requestUuid ?? uuidv4();
this.rewrittenUrl = appState?.rewrittenUrl;

this.url = request.url ?? new URL('https://fake-request/url');
this.headers = isRealRawRequest(request) ? deepFreeze({ ...request.headers }) : request.headers;
this.headers = isRealReq ? deepFreeze({ ...request.headers }) : request.headers;
this.isSystemRequest = this.headers['kbn-system-request'] === 'true';
this.isFakeRequest = isFakeRawRequest(request);
this.isFakeRequest = !isRealReq;
this.isInternalApiRequest =
X_ELASTIC_INTERNAL_ORIGIN_REQUEST in this.headers ||
Boolean(this.url?.searchParams?.has(ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM));
Expand All @@ -174,7 +163,7 @@ export class CoreKibanaRequest<
});

this.route = deepFreeze(this.getRouteInfo(request));
this.socket = isRealRawRequest(request)
this.socket = isRealReq
? new KibanaSocket(request.raw.req.socket)
: KibanaSocket.getFakeSocket();
this.events = this.getEvents(request);
Expand Down Expand Up @@ -250,6 +239,7 @@ export class CoreKibanaRequest<
options,
};
}

/** set route access to internal if not declared */
private getAccess(request: RawRequest): 'internal' | 'public' {
return (
Expand Down Expand Up @@ -333,3 +323,17 @@ export function isRealRequest(request: unknown): request is KibanaRequest | Requ
function isCompleted(request: Request) {
return request.raw.res.writableFinished;
}

/**
* We have certain values that may be passed via query params that we want to
* exclude from further processing like validation. This method removes those
* internal values.
*/
function sanitizeRequest(req: Request): { query: unknown; params: unknown; body: unknown } {
const { [ELASTIC_INTERNAL_ORIGIN_QUERY_PARAM]: __, ...query } = req.query ?? {};
return {
query,
params: req.params,
body: req.payload,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ export class KibanaSocket implements IKibanaSocket {
};
}

constructor(private readonly socket: Socket) {}

public get authorized() {
return this.socket instanceof TLSSocket ? this.socket.authorized : undefined;
}
Expand All @@ -32,12 +34,9 @@ export class KibanaSocket implements IKibanaSocket {
return this.socket.remoteAddress;
}

constructor(private readonly socket: Socket) {}

getPeerCertificate(detailed: true): DetailedPeerCertificate | null;
getPeerCertificate(detailed: false): PeerCertificate | null;
getPeerCertificate(detailed?: boolean): PeerCertificate | DetailedPeerCertificate | null;

public getPeerCertificate(detailed?: boolean) {
if (this.socket instanceof TLSSocket) {
const peerCertificate = this.socket.getPeerCertificate(detailed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,15 @@ export const createCustomHeadersPreResponseHandler = (config: HttpConfig): OnPre
csp: { header: cspHeader },
} = config;

const additionalHeaders = {
...securityResponseHeaders,
...customResponseHeaders,
'Content-Security-Policy': cspHeader,
[KIBANA_NAME_HEADER]: serverName,
};

return (request, response, toolkit) => {
const additionalHeaders = {
...securityResponseHeaders,
...customResponseHeaders,
'Content-Security-Policy': cspHeader,
[KIBANA_NAME_HEADER]: serverName,
};
return toolkit.next({ headers: additionalHeaders });
return toolkit.next({ headers: { ...additionalHeaders } });
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@ export class MetricsService

private async refreshMetrics() {
const metrics = await this.metricsCollector!.collect();
const { message, meta } = getEcsOpsMetricsLog(metrics);
this.opsMetricsLogger.debug(message!, meta);
if (this.opsMetricsLogger.isLevelEnabled('debug')) {
const { message, meta } = getEcsOpsMetricsLog(metrics);
this.opsMetricsLogger.debug(message!, meta);
}
this.metricsCollector!.reset();
this.metrics$.next(metrics);
}
Expand Down

0 comments on commit fd7fcdf

Please sign in to comment.