Add support to hide metadata in log line #445
Conversation
…ition and property key and add further unit tests for parseMessageFormat function
Co-authored-by: Matteo Collina <[email protected]>
…teral unit test scenario, refactored interpretConditionals logic to be more readable and renamed internal unit tests to be more fitting
Co-authored-by: James Sumners <[email protected]>
timlohse1104
changed the title
timlohse1104
changed the title
| } else if (prettifiedTime) { | ||
| line = `${line} ${prettifiedTime}` | ||
| } | ||
| if (!opts.hideMetadata) { |
There was a problem hiding this comment.
Can this be done in some other way than continually nesting the core code of this function further and further to the right?
There was a problem hiding this comment.
Do you have something in mind. I could see some options. One with higher refactoring effort like using strategy pattern (https://refactoring.guru/design-patterns/strategy) and some with lower effort like using switch statements/ lookup tables.
There was a problem hiding this comment.
I think a simple approach would be to move the now optional code path into a function. Note, such a function should return a string, not mutate one through a side effect.
There was a problem hiding this comment.
Added an approach
| @@ -206,6 +178,46 @@ function prettyFactory (options) { | |||
| } | |||
|
|
|||
| return line | |||
|
|
|||
| function createLineMetadata () { | |||
There was a problem hiding this comment.
Does this function need to be defined as an inner function of prettyFactory? Or can we put it in lib/utils?
There was a problem hiding this comment.
We could put it in lib/utils but would have to pass many parameters like log, colorizer, levelKey etc. Think i would prefer to leave it in index.js. What do you think about the naming?
There was a problem hiding this comment.
My concern is legibility and maintainability. The inlined function is easier to read than the nested conditional version, but it is still a lot of code deeply nested within another function.
I think the function name is fine.
There was a problem hiding this comment.
@timlohse1104 I think you'll find it much easier to accommodate the ask now that #453 has been merged. Unfortunately, it'll be easier to start with a fresh PR. Rebasing this one may be too difficult.
If you have any questions, don't hesitate to ask.
|
I'm closing this due to lack of response. If you'd like to finish this work, please open a new PR. |
See #444
With option
--hideMetadatait is possible to transform a basic log line like this:[25.7.2023, 15:57:32] INFO: [Nest] 25.7.2023, 15:57:32 [RoutesResolver] | OcrController {/ocr}: {"context":"RoutesResolver"}into a simulated NestJS log line like this (provided you use the
messageFormatoption):[Nest] 25.7.2023, 15:57:32 [RoutesResolver] | OcrController {/ocr}: {"context":"RoutesResolver"}--hideMetadataoption effectively hidesepoch,leveland other metadata likereqIdfrom beeing printed in front of the actual log message.