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

Middleware for Microservices #1627

Open
JonathanMeyer2600 opened this issue Mar 11, 2019 · 22 comments
Open

Middleware for Microservices #1627

JonathanMeyer2600 opened this issue Mar 11, 2019 · 22 comments

Comments

@JonathanMeyer2600
Copy link

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
Copy link
Member

Middleware concept exists only for HTTP applications so far.

@skliarovartem
Copy link

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

@Ayzrian
Copy link
Contributor

Ayzrian commented Oct 14, 2021

I am on it :)

@jmcdo29
Copy link
Member

jmcdo29 commented Oct 14, 2021

@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
Copy link

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
Copy link
Contributor

Ayzrian commented Oct 21, 2021

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:

  • Making it accept context and next function, where context can be either HTTP Context that contains req and res, or Microservices context/RPC context. Also in the future, we may have Websockets context because that is another module where we missing such a concept. This will be a breaking change and potentially will break compatibility with the function middlewares that exist in the Express ecosystem. I think that it should be possible to distinguish Class middleware and function middelware and hence work around this issue to support Express style middlewares, while all Class middleware will be using new style.
  • Creating a new interface e.g. NestMicroservicesMiddleware to solve that specific problem. Again considering that we may want to add middleware to Websockets, we would also create NestWebsocketsMiddleware later.

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:

  • Add a different configure method, e.g. configureMicroservices and create dedicated ...ConfigProxy.
  • Add a way to provide something like PatternInfo and a forPatterns method in MiddlewareConfigProxy. This entails that we should handle edge cases when someone tries to mix HTTP paths and RPC patterns. Also, imho it t kinda breaks the idea of single responsibility here.

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
Copy link
Member

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
Copy link
Contributor

Ayzrian commented Oct 22, 2021

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
Copy link
Member

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
Copy link

slrv commented Oct 26, 2021

@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
Copy link
Contributor

Ayzrian commented Oct 26, 2021

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
Copy link

slrv commented Oct 26, 2021

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

@maikknebel
Copy link

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
Copy link
Contributor

Ayzrian commented Nov 17, 2021

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
Copy link

micaelparadox commented Jun 18, 2022

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
Copy link

TbotaPhantA commented Jul 27, 2022

Hope this issue will be resolved.

@gitSambhal
Copy link

@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
Copy link

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
Copy link

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
Copy link

coler-j commented Jan 21, 2023

#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;
  }
}

@bsaiuttej
Copy link

#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
Copy link

gitSambhal commented Mar 1, 2024

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.

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