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

Logs are colorized everywhere #60

Closed
aldofunes opened this issue Oct 15, 2018 · 3 comments · May be fixed by #99
Closed

Logs are colorized everywhere #60

aldofunes opened this issue Oct 15, 2018 · 3 comments · May be fixed by #99

Comments

@aldofunes
Copy link

aldofunes commented Oct 15, 2018

I am having some trouble with the colorizing of logs. Specifically, when logging to non-tty outputs.

There is an override in colorize.js:9 that prevents the colors package to enable or disable when supported.

First of all, I would like to know why this decision was made, and if it can be removed.

I have made a hack as a workaround. Here it is, in case anyone wants to know how to do it:

    format: format.combine(
      LOGGER_COLORS === 'true' ? format.colorize({ all: true }) : format(i => i)(),
      format.printf(({
        label = 'default',
        namespace = 'default',
        level = 'debug',
        message,
        timestamp,
      }) => {
        let string = `${label}:${namespace} ${level}: ${message}`;

        if (LOGGER_CONSOLE_TIMESTAMP === 'true') {
          string = `${timestamp} ${string}`;
        }

        return string;
      }),
    ),
@DABH
Copy link
Contributor

DABH commented Oct 24, 2018

I agree with your concerns -- it would probably be better if colorizer accepted a force option which would set colors.enabled=true; if true, but otherwise winston doesn't mess with colors. That way by default colorizer will do whatever colors thinks is the right thing, which is usually correct :) . Want to make a quick PR for that change?

@maxsatula
Copy link

Have the same problem and the same question "why".
However, I prefer to do a different workaround to delegate colors module to decide rather than provide environment variables.

Here, if anyone finds my (simplified) example useful:

'use strict';
// IMPORTANT! colors.enabled value must be captured before Winston imported!
const colorsEnabled = require('colors/safe').enabled;
const winston = require('winston');

winston.add(
    new winston.transports.Console({
        format: winston.format.combine(
            colorsEnabled // the rest is pretty much the same idea as @aldofunes
                ? winston.format.colorize({ all: false })
                : winston.format(i => i)(),
            winston.format.simple()
        )
    })
);

winston.warn('Some warning');

@DABH, once I have a chance to do PR, what would be the force desirable default? Naturally I would expect false but not sure how bad that is from backwards compatibility behavior standpoint.

@maxsatula
Copy link

Maybe it's worth to note that unless colors are forced, an app running in a docker container will not produce a colorized output if -t, --tty is not specified in docker run command

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

Successfully merging a pull request may close this issue.

3 participants