feat: use NODE_ENV and cds.env.features.kibana_formatter for format #2481
Conversation
ZhongpinWang
requested review from
marikaner,
FrankEssenberger and
florian-richter
|
There are following cases:
There are potentially some problems though and I need your thoughts on these:
|
[fixed] Thanks for the feedback. I removed the |
| @@ -49,7 +63,7 @@ export function unmuteLoggers(): void { | |||
| */ | |||
| export const cloudSdkExceptionLogger = container.get(exceptionLoggerId, { | |||
| defaultMeta: { logger: loggerReference, test: 'exception' }, | |||
| format, | |||
| format: container.options.format, | |||
There was a problem hiding this comment.
[q] Shouldn't the format of the SDK exception logger be the same as for other logs?
| /** | ||
| * Change the log format of a logger based on its message context. | ||
| * e.g., to set the log format for the destination accessor module of the SDK to `local`, simply call `setLogFormat(logFormat.local, 'destination-accessor')`. | ||
| * @param format - Format to set the logger to. Use `logFormat` to get the pre-defined log formats or use a custom log format. | ||
| * @param messageContextOrLogger - Message context of the logger to change the log level for or the logger itself. | ||
| */ |
There was a problem hiding this comment.
[nth] This is more than required by the original issue (scope creep) and usually we shouldn't allow for this, because this can cause some problems down the line (e.g. because the API was not aligned, did we consider that whether there are any features that could break because of this, etc.).
Anyways, I discussed this with @FrankEssenberger at length yesterday and we agreed that for this case we will keep it.
The reason I am mentioning this, is that currently users don't have the possibility to change the log format at all and with this we are opening up a new feature, that f.e. depends on types from the logform lib.
[req] We usually introduce our own compatible interfaces, we should do the same for the Format. @jjtang1985 could you show an example?
[req] If we have set global, get global, set single, I guess we also need get single?
ZhongpinWang
requested review from
florian-richter
and removed request for
florian-richter
Closes SAP/cloud-sdk-backlog#380.
We implemented
setLogFormatandsetGlobalLogFormatin this ticket similar to log level. The defaultlocalandkibanaformat is exported withlogFormat. Now, users can specify a log format both from the default two or even their customized formatters.NODE_ENVis used to determine the initial log format:production->kibana,localotherwise. This can be overwritten by the aforementioned two setters.