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

Deprecate prettyPrint: true #1106

Closed
mcollina opened this issue Aug 28, 2021 · 6 comments · Fixed by #1122
Closed

Deprecate prettyPrint: true #1106

mcollina opened this issue Aug 28, 2021 · 6 comments · Fixed by #1122
Milestone

Comments

@mcollina
Copy link
Member

We had too many folks complaining about how we load pino-pretty with yarn 2/3 and similar. I think we should either

  1. require the user to pass in a reference to pino-pretty
  2. make pino-pretty a full transport and let folks use it from there.
@jsumners
Copy link
Member

I'd prefer to fully deprecate it from Pino. Let people pass in a transport or whatever that does it. I'd also like to keep pino-pretty narrowed to its current focus: parsing stdin. If necessary, and possible, I'd rather see us create pino-pretty-transport to provide the transport.

@leorossi
Copy link
Contributor

Hi guys, I am willing to work on this.

Just to be clear, the intention of this is

  • remove prettyPrint option from constructor
  • remove all tests that relies on this feature (i.e: checking the output has been "prettified" and things like this)
  • remove all documentation related to that option, and redirect to pino-pretty

is that correct?

Do we want to show a warning when passing { prettyPrint: true } in the constructor?

@mcollina
Copy link
Member Author

I have some code prepared for this, wait until I upload it (might take a few days).

mcollina added a commit that referenced this issue Sep 8, 2021
Zialus added a commit to Zialus/TW-Minesweeper-Server that referenced this issue Oct 24, 2021
@aeddie-zapidhire
Copy link

For those bumping into the deprecation message and finding this issue, the solution is something like the following:

const transport = pino.transport({
  target: 'pino-pretty',
  options: { colorize: true }
})

const logger = pino({ level: env.LOG_LEVEL }, transport)

Note that transport is governed by the log level in the main options. If you only add the transport, you are restricted to the default log level (which is "info").

@zhex900
Copy link

zhex900 commented Dec 21, 2021

Hi,

I am using streams. What should I do to remove the deprecation message?


const loggerStream = () => {
  const logPath = path.resolve(
    process.platform === "win32"
      ? `${WIN_LOG_PATH}/${LOG_FILE}`
      : `${getAppDataPath()}/${LOG_FILE}`
  );
  return multistream(
    [
      {
        level: "error",
        stream: createStream(logPath, LOG_ROTATION_SETTINGS),
      },
      {
        level: "debug",
        stream: createStream(logPath, LOG_ROTATION_SETTINGS),
      },
      {
        stream: createStream(logPath, LOG_ROTATION_SETTINGS),
      },
    ],
    { dedupe: true }
  );
};

const logger = pino(
  {
    prettyPrint: {
      colorize: true,
      errorLikeObjectKeys: ["err", "error", "details"],
      levelFirst: true,
      translateTime: "SYS:hh:MM:ss TT dd-mm-yyyy",
      ignore: "pid,hostname,service",
      messageFormat: (log, messageKey) => {
        return applyServiceStyles(log, messageKey);
      },
    },
    level: process.env.LOG_LEVEL || "debug",
    serializers: {
      error: pino.stdSerializers.err,
    },
  },
  loggerStream()
);

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

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 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants