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.

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

[Logging] Add callback interface to logging methods for message serialization #129110

Closed
Tracked by #134169
simianhacker opened this issue Mar 31, 2022 · 4 comments
Closed
Tracked by #134169
Labels
enhancement New value added to drive a business result Feature:Logging Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@simianhacker
Copy link
Member

For the Logger interfaces it would be nice if we could use a callback for the message creation so when we need to serialized data, it only happens if the logging level is enabled.

Imagine you're setting up trace logging for request/response for queries against Elasticsearch but you only want to serialized the data when trace logging is enabled (scoped to a very specific part of the application). If the response is large enough, the following code will cause a delay because regardless of the current logging level, it always runs JSON.stringify.

logger.trace(`Request: ${JSON.stringify(request)}`);
const body = await esClient.search<undefined, ResponseAggregations>(request);
logger.trace(`Response: ${JSON.stringify(body)}`);

But if if the trace method accepted string | () => string for message then we could do this:

logger.trace(() => `Request: ${JSON.stringify(request)}`);
const body = await esClient.search<undefined, ResponseAggregations>(request);
logger.trace(() => `Response: ${JSON.stringify(body)}`);

CC: @dgieselaar

@botelastic botelastic bot added the needs-team Issues missing a team label label Mar 31, 2022
@kertal kertal added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Jun 6, 2022
@elasticmachine
Copy link
Contributor

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

@botelastic botelastic bot removed the needs-team Issues missing a team label label Jun 6, 2022
@kertal kertal added triage_needed needs-team Issues missing a team label labels Jun 6, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jun 6, 2022
@pgayvallet
Copy link
Contributor

pgayvallet commented Jun 7, 2022

This kind of feature (disabling expensive logging operations when the proper level is not reached) is usually achieved via distinct isLevelEnabled (that aren't implemented atm either) methods, e.g

if(logger.isInfoEnabled()) {
  logger.info(myExpensiveLogOperation())
}

But given we're using a dynamic language, I kinda like your proposal of allowing to provide a string callback instead. I think it would be acceptable to state that we don't want/need to 'log' callable objects, so it should be fine.

@pgayvallet pgayvallet added enhancement New value added to drive a business result and removed triage_needed labels Jun 7, 2022
@pgayvallet
Copy link
Contributor

Related to #144002

@pgayvallet
Copy link
Contributor

In the end, we went with the more traditional approach described in #144002, which serves the same purpose.

I'll close this as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Logging Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

4 participants