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

[Feature Request] @nestjs/platform-fastify support Custom Log Level #14137

Open
1 task done
daimalou opened this issue Nov 13, 2024 · 12 comments
Open
1 task done

[Feature Request] @nestjs/platform-fastify support Custom Log Level #14137

daimalou opened this issue Nov 13, 2024 · 12 comments
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@daimalou
Copy link

daimalou commented Nov 13, 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

I use fastify buildin logger, this logger is awesome, has builtin traceId feature and can access by request.log.info()

https://fastify.dev/docs/v4.28.x/Reference/Logging/

It works.

  const fastifyInstance = fastify({
    logger: true,
  });

  const app = await NestFactory.create<NestFastifyApplication>(
    AppModule,
    new FastifyAdapter(fastifyInstance),
    {
      logger: false,
    },
  );

But this logger log every request and response. I find a solution to close some route. But In Nest I can't find place to config.

solution:
fastify/fastify#2120
fastify/help#140
https://fastify.dev/docs/latest/Reference/Routes/#custom-log-level

Describe the solution you'd like

Give a config.

Teachability, documentation, adoption, migration strategy

None

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

None

@daimalou daimalou added needs triage This issue has not been looked into type: enhancement 🐺 labels Nov 13, 2024
@Tony133
Copy link
Contributor

Tony133 commented Nov 18, 2024

If I understand you correctly, you want to know how to use request.log.info(), with @nestjs/platform-fastify package ?

@daimalou
Copy link
Author

@Tony133, I need a configuration in nest to close some route log.

like this fastify.register(require('./routes/your-route'), { logLevel: 'silent' })

@daimalou
Copy link
Author

daimalou commented Nov 19, 2024

I found the key code. I'll try modifying it.
https://github.com/nestjs/nest/tree/master/packages/platform-fastify/decorators
https://github.com/nestjs/nest/blob/master/packages/platform-fastify/adapters/fastify-adapter.ts#L726

@daimalou
Copy link
Author

daimalou commented Nov 26, 2024

I have already completed this work, but I encountered an issue that is preventing me from submitting the PR.
I installed the source code for platform-fastify, but it reported many type errors in my environment that I couldn't fix, so I had to use a lot of //@ts-ignore.
Here are my modification methods.

  1. add a new constant.
export const FASTIFY_ROUTE_LOG_METADATA = '__fastify_route_log__';
  1. add a new decorator
import { SetMetadata } from '@nestjs/common';
import { FASTIFY_ROUTE_LOG_METADATA } from '../constants';
import { LogLevel } from 'fastify';

/**
 * @publicApi
 *
 * @param config See {@link https://fastify.dev/docs/latest/Reference/Routes/#custom-log-level}
 */
export const RouteLog = (config: { logLevel: LogLevel }) =>
  SetMetadata(FASTIFY_ROUTE_LOG_METADATA, config);
  1. add new logic in fastify-adapter.ts
    const routeConfig = Reflect.getMetadata(
      FASTIFY_ROUTE_CONFIG_METADATA,
      handlerRef,
    );

    // Here
    const routeLog = Reflect.getMetadata(
      FASTIFY_ROUTE_LOG_METADATA,
      handlerRef,
    );

    const routeConstraints = Reflect.getMetadata(
      FASTIFY_ROUTE_CONSTRAINTS_METADATA,
      handlerRef,
    );

    const hasConfig = !isUndefined(routeConfig);
    // Here
    const hasLog = !isUndefined(routeLog);
    const hasConstraints = !isUndefined(routeConstraints);
 
   // Here
    if (isVersioned || hasConstraints || hasConfig || hasLog) {
      const isPathAndRouteTuple = args.length === 2;
      if (isPathAndRouteTuple) {
        const constraints = {
          ...(hasConstraints && routeConstraints),
          ...(isVersioned && {
            version: handlerRef.version,
          }),
        };

        const options = {
          constraints,
          ...(hasConfig && {
            config: {
              ...routeConfig,
            },
          }),
          // Here
          ...(hasLog && {
            ...routeLog,
          }),
        };

        const routeToInjectWithOptions = { ...routeToInject, ...options };

        return this.instance.route(routeToInjectWithOptions);
      }
    }

usage:

  @Get('')
  @RouteLog({ logLevel: 'silent' })
  public getCredit(
    @Req() req: FastifyRequest,
    @User('userId') userId: string,
  ): Promise<any> {
    req.log.info('This log will never take effect.');
    return this.creditService.getCredit(userId);
  }

@kamilmysliwiec Hi, Could you help me finish the remaining work?

@iiAku
Copy link

iiAku commented Dec 1, 2024

If I'm not mistaken fastify is using pino logger under the hood. If you do pass pino as custom logger within your nest application you'll have full control over your logger rather than trying to get the encapsulated one within the fastify platform.
Then you are also free to pilot the level of your logger on a controller/route basis.

resources:
https://docs.nestjs.com/techniques/logger#injecting-a-custom-logger
https://github.com/pinojs/pino/tree/main
https://github.com/pinojs/pino/blob/main/docs/api.md#logger-level

@daimalou
Copy link
Author

daimalou commented Dec 2, 2024

@iiAku This custom-logger is very simple.I want to log everything from request to response for an API, but it can't be done because it lacks trace ID or request ID functionality. When using tools like Grafana, you can't filter by trace ID or request ID.

The built-in Pino logger in Fastify automatically adds a request ID and allows you to call log methods directly from the request object, such as request.log.info() and request.log.error(). It also allows you to directly set the log level. { logger: true } or { logger: { level: 'info' } }. in

 const fastifyInstance = fastify({
    logger: true,
  });

When you use the request.log.* methods in both an API's controller and its service, they will print logs with the same request ID. Additionally, there's no need to inject a custom logger. Injecting custom logger in every methods is cumbersome.

I have audit requirements, and I need to avoid logging unimportant routes. Instead of writing a log method for each desired route, I prefer to enable a global log method and only disable it for a few routes.

I am already using my modified version of the @nestjs/platform-fastify package. I have installed its source code in my project.

@daimalou
Copy link
Author

daimalou commented Dec 2, 2024

If anyone else wants this feature, please help by submitting a PR. I encountered unsolvable type issues in my environment and cannot resolve them at the moment.

@kamilmysliwiec
Copy link
Member

When you use the request.log.* methods in both an API's controller and its service, they will print logs with the same request ID. Additionally, there's no need to inject a custom logger. Injecting custom logger in every methods is cumbersome.
I have audit requirements, and I need to avoid logging unimportant routes. Instead of writing a log method for each desired route, I prefer to enable a global log method and only disable it for a few routes.

@daimalou why don't you create a middleware and use the als package https://docs.nestjs.com/recipes/async-local-storage?

@daimalou
Copy link
Author

daimalou commented Dec 2, 2024

@kamilmysliwiec I previously discovered an problem. I have a requirement to log the request body and response body. When I used Nest's middleware or interceptor, I can access request body, but I couldn't access the response body or raw body when using fastify adaptor.

To access the response body in Fastify, need to use its built-in hooks. It's fastify part.

https://fastify.dev/docs/latest/Reference/Hooks/#onsend

This is like a black-box to Nest.

  const fastifyInstance = fastify({
    logger: true,
  });

  fastifyInstance.addHook('onSend', function (request, reply, payload, next) {
    const responseInfo = {
      body: payload,
    };
    // It can also use request.log.*
    request.log.info(responseInfo, 'reply body');
    next();
  });

  const app = await NestFactory.create<NestFastifyApplication>(
    AppModule,
    new FastifyAdapter(fastifyInstance),
    {
      logger: false,
    },
  );

It can also access request body in Fastify.

https://fastify.dev/docs/latest/Reference/Hooks/#prehandler

  fastifyInstance.addHook('preHandler', function (request, reply, next) {
    if (request.body) {
      request.log.info({ body: request.body }, 'parsed body');
    }
    next();
  });

Sometimes, Nest,I think it's like an abstraction layer. Some features cannot be configured in Nest. But compared to Fastify, I prefer Nest, as its philosophy is more similar to OOP.

Introducing AsyncLocalStorage with nestjs-cls is invasive and complex for me. The request.log.* method is built into Fastify's features, and it can be used even within hooks, so I don't need to import any additional packages. I can achieve my use case using Fastify's built-in features.

If I use Express, I would definitely use these two libraries. It's not have build-in logger. However, Express's performance is poor. Fastify is four times faster than Express.

By the way, I’m also using Nestia, which makes Nest + Fastify incredibly fast.

https://github.com/samchon/nestia

https://github.com/samchon/nestia/tree/master/benchmark/results/11th%20Gen%20Intel(R)%20Core(TM)%20i5-1135G7%20%40%202.40GHz

@0x0bit
Copy link

0x0bit commented Dec 16, 2024

@daimalou

By the way, I’m also using Nestia, which makes Nest + Fastify incredibly fast.

hello, Can you provide a demo of how you use it? thanks

@daimalou
Copy link
Author

@0x0bit https://nestia.io/docs/core/TypedRoute/
just change decorators and use interface instead of dto class.

@0x0bit
Copy link

0x0bit commented Dec 18, 2024

https://nestia.io/docs/core/TypedRoute/ just change decorators and use interface instead of dto class.

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

5 participants