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

[http] Add log for route path access #152621

Merged
merged 5 commits into from
Mar 6, 2023
Merged

[http] Add log for route path access #152621

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bf7a3c3
Logs route path access
TinaHeiligers Mar 2, 2023
2962355
Merge branch 'main' into kbn-152269-followup-log-route-access
kibanamachine Mar 2, 2023
ed281fe
Merge branch 'main' into kbn-152269-followup-log-route-access
kibanamachine Mar 5, 2023
4ccb36c
address PR comments
TinaHeiligers Mar 6, 2023
fb86412
Adds route method to options logger logs, implement PR nit
TinaHeiligers Mar 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/core/http/core-http-server-internal/src/http_server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,7 @@ export class HttpServer {
}

private configureRoute(route: RouterRoute) {
const optionsLogger = this.log.get('options');
this.log.debug(`registering route handler for [${route.path}]`);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if extending this log entry with the values suggested in https://github.com/elastic/kibana/pull/152621/files/bf7a3c36cb4394b06a3b3fa09a0553c3f958bfed#r1126120651 would suffice. Both entries look too similar and are logged at the same time. If the only reason is so that we can use a new logger, I wonder if we can use it here (as long as it's nested, it should still log with the same config)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about that but went a new logger route to be able to silence the extra info or to later remove the logger completely without losing the original log

// Hapi does not allow payload validation to be specified for 'head' or 'get' requests
const validate = isSafeMethod(route.method) ? undefined : { payload: true };
Expand All @@ -526,6 +527,12 @@ export class HttpServer {
xsrfRequired: route.options.xsrfRequired ?? !isSafeMethod(route.method),
access: route.options.access ?? (route.path.startsWith('/internal') ? 'internal' : 'public'),
};
// Log HTTP API target consumer.
optionsLogger.debug(
`access [${kibanaRouteOptions.access}] [${route.method.toUpperCase()}] for path [${
route.path
}]`
);

this.server!.route({
handler: route.handler,
Expand Down