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

Clarify prettyPrint documentation #835

Closed
AleksandrSl opened this issue May 11, 2020 · 13 comments
Closed

Clarify prettyPrint documentation #835

AleksandrSl opened this issue May 11, 2020 · 13 comments

Comments

@AleksandrSl
Copy link
Contributor

I think that there are some old "code" in the docs about pretty print.

For example this

  function isPinoLog (log) {
    return log && (log.hasOwnProperty('v') && log.v === 1)
  }

function. From this #623 I understood that v was removed some time ago.

Also there is no infromation about why type of inputData could be different. Based on pino code it seems to me that object is always passed. If there are acutally another cases, maybe it will be helpfull to clarify when they are possible.

@mcollina
Copy link
Member

Would you like to send a PR?

@AleksandrSl
Copy link
Contributor Author

Yaeh, I can remove mentions of v in docs, but I'm not sure about prettifier inputData type clarification.

@mcollina
Copy link
Member

I think it should be changed by the field
used by pino-pretty

@AleksandrSl
Copy link
Contributor Author

Sorry, I don't fully understand what

it should be changed by the field
used by pino-pretty

means

@AleksandrSl
Copy link
Contributor Author

Maybe string is passed when prettifier is used with pipe (as stated in docs - cat app.log | pino-pretty), and if prettifier is registered in app it will always get objects? Then this can be mentioned in docs and we can write less boiler plate in custom prettifiers, if they are not supposed to be used with pipes.

@mcollina
Copy link
Member

I don't understand :/

@AleksandrSl
Copy link
Contributor Author

In pretty print docs

  return function prettifier (inputData) {
    let logObject
    if (typeof inputData === 'string') {
      logObject = someJsonParser(inputData)
    } else if (isObject(inputData)) {
      logObject = inputData
    }
    if (!logObject) return inputData
    // implement prettification
  }

it's stated that prettifier function for some reason could get from pino two types of inputData - string and Object. While in the pino code, prettifier function is only called with Object. So I don't understand why string option exist.
Maybe this example was taken from pino-pretty, which has an option to parse logs from stream (i.e. cat app.log | pino-pretty) and in that case prettifier function could recieve string?

I think that prettifier example could be simplified, because you could write prettifier that works only from code and doesn't support piping logs into it.

@jsumners
Copy link
Member

Please read the text in that section of the docs prior to that code sample. It very clearly explains the example.

@AleksandrSl
Copy link
Contributor Author

AleksandrSl commented May 12, 2020

@jsumners

Pino prettifier modules are extra modules that provide a CLI for parsing NDJSON log lines piped via stdin and expose an API which conforms to the Pino metadata streams API.

The API requires modules provide a factory function which returns a prettifier function. This prettifier function must accept either a string of NDJSON or a Pino log object.

There is no info when string can be passed. I wrote prettifier myself, that works with pino, and doesn't work with string. From my point of view prettifier needn't to be a CLI, simple function would suffice for most usages.
(Stupid reason why I can't just include this string check as in example - test coverage. I couldn't come up with test case, when string is passed to prettifier, so this statement is uncovered)

@jsumners
Copy link
Member

Literally the first sentence in your quote describes when a string will be passed in. If you want to write a prettifier that doesn't support CLI usage, that's your prerogative. But we do not advocate doing so. Our docs outline what we expect in a prettifier to be considered a compliant prettifier.

@AleksandrSl
Copy link
Contributor Author

Ok, so supplying simple function as prettifier is not how it's supposed to be used?

Literally the first sentence in your quote describes when a string will be passed in.
After our discussion I understood, it wasn't so transparent.

I used prettifier as simple function to easily migrate from bunyan where 'raw' stream was used to prettify output. Just thought it may be useful to mention in docs.

@AleksandrSl
Copy link
Contributor Author

#510

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants