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.89k stars 7.66k forks source link

Middleware for Microservices #1627

Open JonathanMeyer2600 opened 5 years ago

JonathanMeyer2600 commented 5 years ago

I'm submitting a...


[ ] Regression 
[ ] Bug report
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Middleware exists for HTTP but not for Microservices.

Expected behavior

I would like to be able to create middleware functions for Microservices to introduce something like a request context for logging.

Example:


export function MicroserviceMiddleware(context: ExecutionContext, next) {
// .. do some stuff with the context
next()
}

This is different to interceptors because it spans over all guards, interceptors and exception filters. Or ist there already another way to do this? Thanks in advance

Environment


Nest version: 5.7.3


kamilmysliwiec commented 5 years ago

Middleware concept exists only for HTTP applications so far.

skliarovartem commented 3 years ago

Did someone find solution how to implement logger with context in microservice?

Ayzrian commented 3 years ago

I am on it :)

jmcdo29 commented 3 years ago

@skliarovartem just saw this message, you can use an interceptor to do some request logging in interceptors. That's what I've done with Ogma. It's also got optional request scoping so that you can have the correlationId in each log if you want that too.

skliarovartem commented 3 years ago

I made my tracing by using nestjs-pino + nestjs-steroids/async-context. and yes, I use interceptors to set reqId to context. Thank you for the answer anyway!

Ayzrian commented 3 years ago

I was looking into the code, and I would like to discuss the solution I see at the moment, with the core team, to make sure that I won't spend effort needlessly.

I will split the questions into sections.

Interfaces

Let's start from the fact that the current NestMiddleware interface is very specific to HTTP, so there is two options:

Configuration

Currently, we expose configure method in the module to do configuration. There is a bit of a difference between HTTP routes and RPC patterns. And hence I see two options:

And there are the same concerns regarding the extendability of both approaches if we consider adding Websockets middleware in the future.

Application

Current HTTP middleware relies on the underlying Adapter to mount middleware, due to that fact it is pretty easy to just path the path and handler to the adaptor, and handle exclude config in our custom logic for the handler.

Microservices transports don't have such built-in functions as middleware, so the only way to apply them will be to manually perform the mapping when we create the handler, in the same way, we do with Guards, Interceptors, and so on. This will require the creation of something like MiddlewareContextCreator and MiddlewareConsumer. Our custom handler will always come as the last "middleware".

Other Thoughts

I wonder whether the outcome is worth the effort. The only reasonable way to use middleware in Microservices that I see right now is to use things like AsyncLocalStorage, everything else can be built with Nest.js enhancers. I wonder if we should just expose something like GlobalMiddleware for Microservices, which will wrap every call if someone does need Middleware for something.

kamilmysliwiec commented 3 years ago

everything else can be built with Nest.js enhancers. I wonder if we should just expose something like GlobalMiddleware for Microservices, which will wrap every call if someone does need Middleware for something.

Agree. Maybe we could just allow registering a "preRequest" (where request = event/message) hook so that you can register it for all handlers?

Ayzrian commented 3 years ago

Maybe we could just allow registering a "preRequest" (where request = event/message) hook so that you can register it for all handlers?

Yep, I think that makes sense. That is very close to what I thought when was speaking about GlobalMiddleware.

One note that such preRequest ideally should still receive next or handler function, that will be our handler with enhancers applied, so that something like this would be possible

        asyncLocalStorage.run(context, () => {
            next();
        });

What do you think?

Also while we on that, I think it makes sense to implement similar preRequest for WebSockets, because currently, it is impossible to wrap the Nest.js message handler into AsyncLocalStorage context.

kamilmysliwiec commented 3 years ago

One note that such preRequest ideally should still receive next or handler function, that will be our handler with enhancers applied, so that something like this would be possible

Sounds good. next() should be fine

Also while we on that, I think it makes sense to implement similar preRequest for WebSockets,

We could implement this in a subsequent PR

slrv commented 3 years ago

@Ayzrian If it helps you, I have solved same problem with interceptor:

@Injectable()
export class AsyncContextMsInterceptor implements NestInterceptor {

  constructor(private _asyncStorage: AppAsyncLocalStorage) {
  }

  intercept(context: ExecutionContext, next: CallHandler<any>): Observable<any> | Promise<Observable<any>> {
    const input = context.switchToRpc().getData();
    const { ...some data from headers } = input.headers;

    this._asyncStorage.enterWith(new AsyncContextStorage(...some data from headers));
    return next.handle();
  }
}

AppAsyncLocalStorage is just a typed AsyncLocalStorage

Ayzrian commented 3 years ago

Hey, @slrv , the problem here is that Guards are called before Interceptors, so you won't have that async storage available in the guards. Though if you don't actually care about Guards, then yes the Interceptor solution will work.

slrv commented 3 years ago

Hey, @Ayzrian, totally understand you. Anyway, preRequest or something like middleware in http processing will be great feature.

maikknebel commented 3 years ago

Hello,

is it possible to get an "onFinish" hook too? It should be triggered after the ExceptionFilters right before the response is send.

I am trying to log microservices with an interceptor, which works, as long as i don't throw an exception. In that case the ExceptionFilter is the last instance (and not the interceptor) before the response is send and will modify it. I would like to use this data for my logging. In http-logging there is a hook called "finish" which grants access to the response send to the client - i guess this hook comes from express. I use that hook for a Logging-Middleware, but that only works for http. A similar hook for microservices would be great.

Ayzrian commented 3 years ago

Hello,

is it possible to get an "onFinish" hook too? It should be triggered after the ExceptionFilters right before the response is send.

I am trying to log microservices with an interceptor, which works, as long as i don't throw an exception. In that case the ExceptionFilter is the last instance (and not the interceptor) before the response is send and will modify it. I would like to use this data for my logging. In http-logging there is a hook called "finish" which grants access to the response send to the client - i guess this hook comes from express. I use that hook for a Logging-Middleware, but that only works for http. A similar hook for microservices would be great.

I think that you can use rxjs catchError operator, to catch an error in interceptor and do the logging you need.

micaelparadox commented 2 years ago

Did someone find solution how to implement logger with context in microservice?

You can try use the adapter pattern in order to make it agnostic to any outside service such as Sentry and/or Elastic or Datadog for example.

TbotaPhantA commented 2 years ago

Hope this issue will be resolved.

gitSambhal commented 2 years ago

@Ayzrian If it helps you, I have solved same problem with interceptor:

@Injectable()
export class AsyncContextMsInterceptor implements NestInterceptor {

  constructor(private _asyncStorage: AppAsyncLocalStorage) {
  }

  intercept(context: ExecutionContext, next: CallHandler<any>): Observable<any> | Promise<Observable<any>> {
    const input = context.switchToRpc().getData();
    const { ...some data from headers } = input.headers;

    this._asyncStorage.enterWith(new AsyncContextStorage(...some data from headers));
    return next.handle();
  }
}

AppAsyncLocalStorage is just a typed AsyncLocalStorage

Can you pls share full code as I am unable to find the AppAsyncLocalStorage and AsyncContextStorage?

bsaiuttej commented 1 year ago

One note that such preRequest ideally should still receive next or handler function, that will be our handler with enhancers applied, so that something like this would be possible

Sounds good. next() should be fine

Also while we on that, I think it makes sense to implement similar preRequest for WebSockets,

We could implement this in a subsequent PR

Is it included in the nestjs.

pasha-vuiko commented 1 year ago

Hey! What's the state of this? I think it would be great have such feature for example to be able to log RPC messages trace IDs via AsyncLocalStorage

coler-j commented 1 year ago

https://github.com/nestjs/nest/issues/1627#issuecomment-951775191

Can't you just put this logic into a Guard then?

import { Injectable, CanActivate, ExecutionContext } from '@nestjs/common';
import { Observable } from 'rxjs';

@Injectable()
export class RolesGuard implements CanActivate {
    constructor(private _asyncStorage: AppAsyncLocalStorage) {
  }

  canActivate(
    context: ExecutionContext,
  ): boolean | Promise<boolean> | Observable<boolean> {
    const input = context.switchToRpc().getData();
    const { ...some data from headers } = input.headers;

    this._asyncStorage.enterWith(new AsyncContextStorage(...some data from headers));

    return true;
  }
}
bsaiuttej commented 1 year ago

#1627 (comment)

Can't you just put this logic into a Guard then?

import { Injectable, CanActivate, ExecutionContext } from '@nestjs/common';
import { Observable } from 'rxjs';

@Injectable()
export class RolesGuard implements CanActivate {
    constructor(private _asyncStorage: AppAsyncLocalStorage) {
  }

  canActivate(
    context: ExecutionContext,
  ): boolean | Promise<boolean> | Observable<boolean> {
    const input = context.switchToRpc().getData();
    const { ...some data from headers } = input.headers;

    this._asyncStorage.enterWith(new AsyncContextStorage(...some data from headers));

    return true;
  }
}

Can you share, from where AppAsyncLocalStorage dependency come from, if it is written locally in the application itself, can you share it

gitSambhal commented 8 months ago

This is how I am using it in the pubsub microservice.

import { PinoLogger } from 'nestjs-pino';
import { storage, Store } from 'nestjs-pino/storage';

export const useLoggerAsyncStorage = (callback): unknown => {
  return storage.run(new Store(PinoLogger.root), callback);
};

/**
 * Pubsub Microservice
 */
@EventPattern('my-subscription')
public handlePubsubReq(
  @Payload('payload')
  reqMessage: any,
) {
  useLoggerAsyncStorage(() => {
    this.handleReq(reqMessage);
  });
}

I was unable to use the assign function in non http transports but using useLoggerAsyncStorage, I can use pino assign method.