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

logformat assumes innerVal is not an Object. #1

Open
ghost opened this issue Apr 8, 2016 · 2 comments
Open

logformat assumes innerVal is not an Object. #1

ghost opened this issue Apr 8, 2016 · 2 comments

Comments

@ghost
Copy link

ghost commented Apr 8, 2016

logformat assumes innerVal is not an Object. Does not process innerVal if innerVal is an Object. This doesn't provide a deep reformat of JSON to key-value pairs
Take for example: {"date":"2015-11-19","client":{"agent":"firefox","ip":"10.1.32.1"},"server":{"windows":{"ip":"192.168.2.222"}}}
Parse JSON to the last primitive/simple value

@tcort
Copy link
Member

tcort commented Apr 8, 2016

This is somewhat by design...

  • The implementation is simpler because it doesn't have to worry about detecting circular references.
  • It is safer because there isn't a chance of blowing the stack like there would be with a recursive function.
  • It is faster and uses less memory when transforming large/complex objects because it doesn't traverse very deep.

That said, I think traversing deeper into the object could be a useful feature if done right. I think the right way here would be to follow the lead of util.inspect(): add an optional options parameter that can set a depth which defaults to 2. If depth is null the function will recurse indefinitely. Some mechanism for detecting circular references would need to be implemented as well. Above all, the changes shouldn't change existing behaviour.

I'm labelling this as an 'enhancement' and not a 'bug' as the current implementation does what it is designed to do. I don't have an immediate need for this functionality, so it may be some time before I implement it. In the meantime, feel free to submit a pull request.

@ankon
Copy link

ankon commented Jul 30, 2021

I guess this was actually solved with ea85596 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants