nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript πŸš€
https://nestjs.com
MIT License
67.16k stars 7.57k forks source link

[Question] Custom logger #247

Closed szkumorowski closed 6 years ago

szkumorowski commented 6 years ago

Is it possible to inject custom logger into nest components ?

MickL commented 6 years ago

Wouldn't you use it as normal? E.g. for Winston i would import and configure it within your root main.ts and then just import it again in whatever file you need it which will give you the configured logger.

I don't think injecting is the right way. Just use import as you would do with RxJS or Moment.js for example.

kamilmysliwiec commented 6 years ago

Hi @szkumorowski, Could you explain what do you mean?

szkumorowski commented 6 years ago

Hi,

I'm using a winston logger for application, I would like to set kind bridge for it which will resolve to use my custom logger at the end. Example with typeorm: https://github.com/typeorm/typeorm/blob/master/docs/logging.md

With above example we can implement the logger and simply pass the instance to it.

MickL commented 6 years ago

Just import it you dont need dependency injection here

szkumorowski commented 6 years ago

You don't understand me - sorry I mean set instead of inject, I would like to set my custom logger to be used in nest components in replace for default one.

cojack commented 6 years ago

@szkumorowski

Is it possible to inject custom logger into nest components ?

Not now

kamilmysliwiec commented 6 years ago

Honestly, I still don't understand. Why not create a logger component and inject it into the components?

cojack commented 6 years ago

@kamilmysliwiec he wanna to override your logger

kamilmysliwiec commented 6 years ago

@cojack but why? Nest logger is used only to show the progress of bootstrapping the application. Winson has a completely different purpose.

cojack commented 6 years ago

@kamilmysliwiec I have just explain the question ;)

szkumorowski commented 6 years ago

@kamilmysliwiec Not only, Its used to show errors which comes from nest components either.

kamilmysliwiec commented 6 years ago

@szkumorowski only when there's no custom exception filter setup. You can create your own and handle each Error in it πŸ™‚

cojack commented 6 years ago

Btw, it might be possible, if the Logger will be part of the components, than we will be able to override it by providers ;>

@kamilmysliwiec

wbhob commented 6 years ago

You should be able to substitute LoggerService like this:

import { LoggerService } from '@nestjs/common';
import { CustomLogger } from './path/to/custom_logger';

And in your components (which should be renamed to providers in v5 @kamilmysliwiec )

{ provide: LoggerService, useClass: CustomLogger } 

But if this isn't possible, it should be.

kamilmysliwiec commented 6 years ago

@wbhob Logger doesn't belong to any scope, so it won't be possible to provide it in the module metadata.

btw components won't be renamed to providers

wbhob commented 6 years ago

I should have said "could in my opinion"... I just think that either there should be explanations for why things are named as they are (that is differently from Angular, ex. @Injectable() and @Component(), or providers and components), or a review/survey of how those APIs are structured. It seems to deviate too much from Angular without good reason, but I'm sure there may be a reason that I just don't understand.

I'm sure you probably hate me for challenging you on literally everything but I honestly just want to make sure that any ambiguity is cleared up, and the transition for Angular devs to pick up Nest is clear. When writing a framework, it's critical that in addition to having well-documented APIs, that the reasoning behind them is clear. That's something Angular does very well and I'd be happy to contribute to the docs one I understand those reasonings with Nest. πŸ‘

kamilmysliwiec commented 6 years ago

@wbhob no hate at all πŸ™‚ I have explained it here https://github.com/nestjs/nest/issues/254

wbhob commented 6 years ago

Yep, I saw that! That's a good point. Does Nest have support for provider objects/substitution (like useClass, useValue, useFactory)

kamilmysliwiec commented 6 years ago

@wbhob yes for sure, all of them

kamilmysliwiec commented 6 years ago

@szkumorowski has exception filter helped with your issue?

LuukMoret commented 6 years ago

@kamilmysliwiec I'm using nestjs in combination with serverless+aws and in the logging of aws lamba it's polluting the log records with ascii characters. Is it possible to optionally disable the colorization of the bootstrap logging to prevent ascii character output in unsupported environments? (Ideally I want to capture all logging, even bootstrap logging, and write the output to a location of my choice).

szkumorowski commented 6 years ago

@LuukMoret +1 @kamilmysliwiec How to access request object in HttpFilter ? Is it possible ? I want to write some additional information which is part of request with custom logger.

export interface ExceptionFilter { catch(exception: any, response: any): any; }

ValentinFunk commented 6 years ago

I'd actually appreciate this a whole lot as well - been looking for the same thing :) If you have a standardized interface for logging everyone could inject that and when i install a library or something for nest it will all plug in nicely. Else e.g. Luuk will have a nice lib that logs using bunyan, mick will use winston and then I will do console.log. Somebody builds an app imports our modules and has a huge mess on their console with no way to control it :(

MickL commented 6 years ago

Writing a logger class that uses a logging library is simple. But you would need that class for every logger e.g. a Winston Nest Logger etc. Also you end up injecting it in every component/class. On the other hand you can just do a simple import like you do on things like rxjs or momentjs. Personally i dont see a need to have a class for that. But yeah sounds like an easy PR to have a logger service available in Nest that follows an interface and is therefor overridable.

kamilmysliwiec commented 6 years ago

Will check what we can do here πŸ™‚ thanks for your suggestions

kamilmysliwiec commented 6 years ago

Thus it might be a little bit difficult to override logger used to inform about the deprecations, for example inside decorators, but in the rest (bootstraping) should be pretty easy. What do you think about it?

szkumorowski commented 6 years ago

@kamilmysliwiec It's always better than nothing.

I don't open new issue in relation to: https://github.com/nestjs/nest/issues/247#issuecomment-346024389

Is it possible to have request in filter? And is there any reason why at this point request is not available ?

kamilmysliwiec commented 6 years ago

@szkumorowski res object contains req property, which is a request in fact

cojack commented 6 years ago

@kamilmysliwiec that's why I do PR for change your logger to debug, where they will be able to turn on/off the logs in simple way and use their own like log4js/winston.

kamilmysliwiec commented 6 years ago

@cojack now it's time when I regret this haha πŸ˜…

kamilmysliwiec commented 6 years ago

maybe we should reconsider this now @cojack :)

kamilmysliwiec commented 6 years ago

As far as I remember I was trying to make the debug enabled for Nest packages by default (it's disabled now)

cojack commented 6 years ago

@kamilmysliwiec as you wish man, I just wanna have the best fw in node ever <3

kamilmysliwiec commented 6 years ago

@cojack I have reopened your PR, would be great to find a way how to enable debug package by default πŸ€”

ValentinFunk commented 6 years ago

Glad to see this being reconsidered :) @kamilmysliwiec i'm quite amazed by how responsive you are on here. Great to see the discussion and that you're working with the community.

@MickL is correct as well, another thing is maybe you want to log somewhere you don't have DI and you'd need to inject a logger everywhere. Problem 1 is not really possible to solve if you want configurable loggers (in the sense of DI to swap them out at the root module). Problem 2 is more of a convenience thing - in the end you can always opt against it and just console.log or have some global logger.

I think the @cojack PR is a great step in the right direction, if you have a somewhat configurable logger most people would probably just stick to that and that means I can use other peoples modules nicely and nobody needs to worry about logging so much :)

This still leaves the question of which logger to use of course, we could perhaps compare some popular options? Might make sense if it's not swappable since it will likely become quite standard. E.g. winston, bunyan, debug, ...

wbhob commented 6 years ago

@kamilmysliwiec @cojack I like the idea, but I still believe the "Angular" way would be to substitute one's own LoggerService class for the built-in one if they need to change it. That way, it takes advantage of DI and the substitution features built in to Nest.

I'm super opinionated though so feel free to ignore me.

kamilmysliwiec commented 6 years ago

@wbhob but it's not possible here, I have already referred to this (btw I also think that this way would be the best πŸ™‚ )

@Kamshak @cojack debug makes possible to at least disable the bootstrapping + error handling logging.

LuukMoret commented 6 years ago

@kamilmysliwiec optionally disable all bootstrapping logging would be nice, but I actually like the logging :) I just want to disable colors in a non local environment.

wbhob commented 6 years ago

Oh that’s right, I forgot! Sorry

allanharris commented 6 years ago

Custom logging is essential for any real production use case. As example you may have hundreds of identical application pods running in a cluster and sending logs as JSON to Elasticsearch. Or app just printing logs as JSON lines to STDOUT so Filebeat can grab it from Docker container logs and forward to a log collector. Another example – I may need to turn off any logs except Errors for automated tests.

But of course having nice green logs are awesome for local dev environment.

MickL commented 6 years ago

Yes but you can build any of those custom loggers within minutes. I donβ€˜t think this is Nest related and can be integrated super easy.

allanharris commented 6 years ago

Agree, but I need a switch to turn off Nest logging too or better reproduce it in my custom logging somehow because logs from Nest engine could be useful.

LuukMoret commented 6 years ago

Any update on this? We would really benefit from optionally disabling the coloring of logging on deployed nestjs apps.

wbhob commented 6 years ago

@LuukMoret no update. You still may not override the logger.

chadjaros commented 6 years ago

The way this is accomplished in Spring apps, I believe, is that the spring underpinnings use a logging facade that handwavey magics your logging framework of choice into place before bootstrapping your app. SLF4J perhaps?

Would it be possible to have a similar thing here where it's less magical, but the Nest framework lets you configure a logging implementation object matching an interface that the framework requires before bootstrapping?

cojack commented 6 years ago

Guys, because it's not going anywhere, I will give you just a simple solution for all of us, just to turn off the nest logs before you call NestFactory.create: add this line:

Logger.setMode(NestEnvironment.TEST);

use your own logger, like winston, log4js or whatever you wanna in code.

Regards.

chadjaros commented 6 years ago

@cojack Your suggestion is fine for turning off bootstrap logging. It's not fine if you're building a production application and need to see the bootstrapping logs so you can debug issues on a remote host; to send them to a greylog or spelunk using a custom logger for example.

If this library is to have adoption in production systems, this is not a feature that should be blithely overlooked.

cojack commented 6 years ago

@chadjaros I know, so I follow to new idea:

app.logger.ts

import {Component, Inject} from '@nestjs/common';
import * as winston from 'winston';
import {DateTime} from 'luxon';

@Component()
export class AppLogger extends winston.Logger {
    constructor(@Inject('LoggerLabelToken') label: string) {
        super({
            transports: [
                new winston.transports.Console({
                    label,
                    timestamp: () => DateTime.local().toString(),
                    formatter: options => `${options.timestamp()} [${options.level.toUpperCase()}] ${options.label} - ${options.message}`
                })
            ]
        });
    }
}

Usage:

const loggerLabelProvider = { provide: 'LoggerLabelToken', useValue: 'AppModule' };

@Module({
    modules: [AuthModule],
    components: [AppLogger, loggerLabelProvider]
})
export class AppModule {
    constructor(private logger: AppLogger) {
        this.logger.log('info', 'test message %s', 'my string');
        setTimeout(() => {
            this.logger.info('after 5 sec');
        }, 5000);
    }
}
const loggerLabelProvider = { provide: 'LoggerLabelToken', useValue: 'AuthModule' };

@Module({
    components: [AuthService, JwtStrategy, AppLogger, loggerLabelProvider],
    controllers: [AuthController],
})
export class AuthModule implements NestModule {
    constructor(private logger: AppLogger) {
        this.logger.info(`what's uuuuuuuup?`);
    }
}

And fe:

@Controller('auth')
export class AuthController {
    constructor(private logger: AppLogger, private readonly authService: AuthService) {
        this.logger.info('hello from the other side');
    }
}

Output:

[Nest] 10919   - 2018-2-10 10:14:18   [NestFactory] Starting Nest application...
2018-02-10T10:14:18.937+01:00 [INFO] AppModule - test message my string
[Nest] 10919   - 2018-2-10 10:14:18   [InstanceLoader] AppModule dependencies initialized +23ms
2018-02-10T10:14:18.946+01:00 [INFO] AuthModule - what's uuuuuuuup?
2018-02-10T10:14:18.955+01:00 [INFO] AuthModule - hello from the other side
[Nest] 10919   - 2018-2-10 10:14:18   [InstanceLoader] AuthModule dependencies initialized +15ms
[Nest] 10919   - 2018-2-10 10:14:18   [RoutesResolver] AuthController {/auth}: +30ms
[Nest] 10919   - 2018-2-10 10:14:18   [RouterExplorer] Mapped {/token, POST} route +2ms
[Nest] 10919   - 2018-2-10 10:14:18   [RouterExplorer] Mapped {/authorized, GET} route +0ms
[Nest] 10919   - 2018-2-10 10:14:18   [NestApplication] Nest application successfully started +1ms
[Nest] 10919   - 2018-2-10 10:14:18   [Index] Everything up
2018-02-10T10:14:23.948+01:00 [INFO] AppModule - after 5 sec

And then, seriously turn off the nest logs when production.

Regards.

sqrter commented 6 years ago

Hi guys,

I've been watching this thread for a little while and trying to solve a related issue.. I need the logger to be configurable, depending on environment. I also need some context to be passed along with the logger messages.

I really liked the solution @cojack suggested above, however the downside is that you need to provide LoggerLabelToken for every single component where you want to pass the context, which is a little verbose imo. I came up with a little different solution - a decorator that passes context automatically by proxying the injected logger component. Here's an example of usage:

@Module({
    imports: [CommonModule]
})
@LoggerContext(AppLogger)
export class ApplicationModule implements OnModuleInit {
    constructor(@Inject(AppLogger) private logger: ILogger) {
        this.logger.debug("APP MODULE CONSTRUCTOR...");
    }

    onModuleInit() {
        this.logger.debug("APP MODULE INIT...");
    }
}
@Component()
export class AppLogger extends winston.Logger implements ILogger {
    constructor(@Inject(AppConfigService) private appConfig: IAppConfig) {
        super({
            exitOnError: false
        });
        if (appConfig.environment === "dev") {
            const {formatter, timestamp} = wcf();
            this.add(winston.transports.Console, {
                level: "silly",
                humanReadableUnhandledException: true,
                formatter: formatter,
                timestamp: timestamp,
                handleExceptions: true
            });
        }
    }
}

Output:

[Nest] 16064   - 2018-2-12 11:27:07   [NestFactory] Starting Nest application...
[2018-02-12 09:27:07.827] [DEBUG] ApplicationModule - APP MODULE CONSTRUCTOR...
[Nest] 16064   - 2018-2-12 11:27:07   [InstanceLoader] ApplicationModule dependencies initialized +65ms
[2018-02-12 09:27:07.832] [DEBUG] CommonModule - COMMON MODULE CONSTRUCTOR...
[Nest] 16064   - 2018-2-12 11:27:07   [InstanceLoader] CommonModule dependencies initialized +3ms
[2018-02-12 09:27:07.877] [DEBUG] ApplicationModule - APP MODULE INIT...
[2018-02-12 09:27:07.878] [DEBUG] CommonModule - COMMON MODULE INIT...
[Nest] 16064   - 2018-2-12 11:27:07   [NestApplication] Nest application successfully started +45ms

The decorator itself is here

The advantage is that the context is passed automatically(constructor name), plus you can extend it to pass whatever you need (e.g. hostname, some processingId). The disadvantage is that if you need to instantiate you logger manually in component's constructor (not DI) the current decorator can't pass the context to the logger invocations from withing this constructor (not yet sure if it's possible).

What do you think?

chadjaros commented 6 years ago

These are progress, but are still workarounds. There should be no reason that a production grade library has to turn of a specific set of loggers, or should tolerate log messages that are otherwise uncontrolled.

I'm not clear on why the internal Nest logger can't be A) configurable and usable anywhere in the app, or B) overridden before bootstrapping.

Boostrap logging, if it exists, should be treated no differently than any other log message. It should have a log level that can be disabled through configuration, and a consistent format with "normal" logs.