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

Allow custom logger implementation for health check service #2068

Closed
1 task done
colematthew4 opened this issue Oct 12, 2022 · 10 comments
Closed
1 task done

Allow custom logger implementation for health check service #2068

colematthew4 opened this issue Oct 12, 2022 · 10 comments

Comments

@colematthew4
Copy link

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

My team uses a logger implementation with a custom winston transport to our logs host provider for all of our services. When a health check fails, the HealthCheckService logs using the built-in nest logger.

Describe the solution you'd like

We would like to be able to have the HealthCheckService log using a custom logger implementation, ideally something similar to how custom loggers are already used for regular controller modules.

Teachability, documentation, adoption, migration strategy

Custom logger documentation already exists.

What is the motivation / use case for changing the behavior?

Detailed above

@BrunnerLivio
Copy link
Member

Sorry for the late response. It is possible to overwrite the logger, by simply adding your custom implementation within NestFactory.create:

const app = await NestFactory.create(AppModule, {
    logger: {
      ...console,
      log: (msg: string) => {
        console.log('LOG:');
        console.log(msg);
      },
      error: (msg: string) => {
        console.error('ERROR:');
        console.error(msg);
      },
    },
  });

Example log with Terminus:

...
LOG:
TerminusModule dependencies initialized
LOG:
AppModule dependencies initialized
LOG:
HealthController {/health}:
LOG:
Mapped {/health, GET} route
LOG:
Nest application successfully started
ERROR:
Health Check has failed! {"asdf":{"status":"down","message":"getaddrinfo ENOTFOUND doesnotexistforsure.io"}}

Or were you for looking for something else?

@shmam
Copy link

shmam commented Nov 9, 2022

@BrunnerLivio hello! Is there any way to apply a custom logger just to the health check service? I wanted to reduce our logs to the health check service to just one line but leave the logs the same for the rest of my application.

@colematthew4
Copy link
Author

@BrunnerLivio we are using a module/service extending the ConsoleLogger and injecting it throughout our application, so this wouldn't work for us

@jrista
Copy link

jrista commented Jan 25, 2023

@BrunnerLivio As with the others, including from some previously closed issues, I would like to control where Terminus logging goes. Its not just a matter of changing how it logs to the main nest.js log...I want to prevent Terminus logging from going to the main log entirely, which pretty much means being able to supply an alternative logger.

The main problem we are facing is, Terminus hard-logs (we can't control it), every few seconds (frequency of our health checks in AWS), and that completely fills up 97% of our logs with stuff that is of no value. We have a real hard time finding error or warning logs, or even just debug logging, when we actually have to troubleshoot something, because Terminus offers no way to even disable its logging, let alone redirect it.

Currently, as far as I can tell, the TerminusModuleOptions object only supports one option, errorLogStyle, which seems to only control whether error logs are pretty or json. There is no way to disable the logging though, which even that, would be of use to us to get around this issue we have of our logs being 97% or more terminus logging (and 99% of that, is "health ok" basically.)

BrunnerLivio added a commit that referenced this issue Jan 27, 2023
@BrunnerLivio BrunnerLivio mentioned this issue Jan 27, 2023
12 tasks
BrunnerLivio added a commit that referenced this issue Jan 27, 2023
BrunnerLivio added a commit that referenced this issue Jan 27, 2023
@BrunnerLivio
Copy link
Member

@jrista I published a beta with this feature. npm i @nestjs/terminus@beta.

Docs are at the moment found in the PR #2202. Could you verify whether this is what you expected?

@jrista
Copy link

jrista commented Jan 27, 2023

Thanks, @BrunnerLivio! Will check it out soon here.

@jrista
Copy link

jrista commented Jan 30, 2023

@BrunnerLivio Had a chance to look at the updates. To make sure I understand, this adds a publicly configurable logger: boolean flag? I did see that internally, there were some changes to separate out error logging from other logging. That is intriguing, and it makes me wonder if there might be any way to expose two configurable flags, one for general logging, and a separate one for error logging.

The main issue we have run into lately is that our logs get filled up with benign logging that has no value: Health check succeeded. We do want to know when health checks fail, though... I am not sure if that is actually what your error logging is used for, I kind of suspect it may in fact be used for internal errors in terminus, not really failed health checks. So, the new flag is definitely welcome, but I am wondering if we could make this better, by allowing...oh, maybe it could just be a log level configuration, where we can either turn logging off completely, only log when one or more health checks fail, or log everything, on the general logger (and always allow the error logger to log internal errors for terminus itself)?

@BrunnerLivio
Copy link
Member

BrunnerLivio commented Jan 30, 2023

adds a publicly configurable logger: boolean flag

Yes, but also you can add your own logger. Hence you can filter out any messages if you'd like

Custom Logger

app.module.ts

@Module({
imports: [
  TerminusModule.forRoot({
    logger: TerminusLogger,
  }),
],
...
})
export class AppModule {}

terminus-logger.service.ts

import { Injectable, Scope, ConsoleLogger } from '@nestjs/common';

@Injectable({ scope: Scope.TRANSIENT })
export class TerminusLogger extends ConsoleLogger {
  error(message: any, stack?: string, context?: string): void;
  error(message: any, ...optionalParams: any[]): void;
  error(
    message: unknown,
    stack?: unknown,
    context?: unknown,
    ...rest: unknown[]
  ): void {
    //noop
  }
}

filled up with benign logging that has no value: Health check succeeded

What do you mean by that? Terminus does not log this message. Are you sure this isn't something from your side? Terminus only logs error messages
I was thinking of adding log level support but for now since only errors are being logged a boolean would suffice

@jrista
Copy link

jrista commented Jan 30, 2023

@BrunnerLivio Hmm, well, its possible its another logger... I'll have to dig around a bit. If its not actually Terminus, then that's good. ;) We may have injected some other logger, or something that is logging...

A customizable logger should do for our needs though! Thanks!

@BrunnerLivio
Copy link
Member

Released with v9.2.0

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

4 participants