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

Logger Enhancements #2547

Open
1 task done
ssilve1989 opened this issue May 1, 2024 · 1 comment · May be fixed by #2552
Open
1 task done

Logger Enhancements #2547

ssilve1989 opened this issue May 1, 2024 · 1 comment · May be fixed by #2552

Comments

@ssilve1989
Copy link

ssilve1989 commented May 1, 2024

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

There is no way to provide a custom logger that has injectable dependencies since you can't provide those providers (other modules, etc) to Terminus when calling TerminusModule.forRoot()

Terminus is just taking the class-type and passing it as a useClass provider.

Describe the solution you'd like

It would be useful if the terminus module could default to using the Nest provided logger, which can be overridden when doing app.useLogger. This would allow for custom loggers an application is using to just be re-used by Terminus without any additional configuration.

Alternatively, the options to TerminusModule.forRoot could be enhanced to be able to provide the dependencies needed to create an instance of the custom logger.

Teachability, documentation, adoption, migration strategy

If the first suggestion is adopted, users wouldn't have to change anything about how they declare TerminusModule.

If a different approach is chosen, maybe it could look like

TerminusModule.forRootAsync({
  imports: [SomeDependentModule],
  inject: [...dependencies],
  useFactory: (...dependencies) => ({
    logger: new CustomLogger(...dependencies),
    ...otherTerminusOptions
  })
})

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

I have a custom LoggerModule that exports a LoggerService that re-uses a single Pino logger instance. This LoggerService's dependencies, in addition to the pino logger, also has some stuff in it for AsyncContext storage management to enhance logs, but the main priority is that the entire log needs to be in JSON format to be ingested by Humio in the format expected for structured logging, which my custom logger handles.

Currently Terminus's errorLogStyle doesn't change the entire log to JSON but only the details

[Nest] 64381  - 05/01/2024, 11:48:10 AM   ERROR [HealthCheckService] Health Check has failed! {"rabbitmq":{"status":"up","uri":"amqp://localhost:5672"},"postgres":{"status":"down"}}

but that should really look like

{
  "context": "HealthCheckService",
  "@timestamp": "2024-01-05T11:48:10.000Z",
  "level": "ERROR",
  "message": "Health Check has failed!",
  "details": {
    "rabbitmq": { "status": "up", "uri": "amqp://localhost:5672" },
    "postgres": { "status": "down" }
  }
}

in my case.

BrunnerLivio added a commit that referenced this issue Jul 16, 2024
It will now use the configured NestJS application logger
that has been configured with `app.useLogger(...)`

resolves #2547
@BrunnerLivio BrunnerLivio linked a pull request Jul 16, 2024 that will close this issue
12 tasks
@BrunnerLivio
Copy link
Member

Sorry for the late response. I've implemented a version where we'd use the default logger, hence you can overwrite it with app.useLogger(...). Would you like to verify my implementation by installing the latest beta? npm -i --save @nestjs/terminus@beta?

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

Successfully merging a pull request may close this issue.

2 participants