Refactor to use scopes more cleanly (request-scoped provider?)

[eropple]

There are cases where one may want to send additional events to Sentry besides an exception--it's a pretty common pattern. However, right now, the scope is generated on-demand when an exception is captured and there's no way to get that data from the interceptor.

So, two suggestions:

• Add a factory-scoped provider in Scope.REQUEST scope that creates, and returns, a Sentry.Hub (exposing it for import in user modules). This hub's default scope (Sentry scope, not NestJS scope) should be populated with request information at the start of the request and be available through its lifecycle.
• Refactor the interceptor to use this hub.

I may be able to help with this if it's of interest. Thoughts?

[mentos1386]

I would love to add support for breadcrumbs and messages. I was giving this a try before but couldn't find a good solution.

Injection Scopes look like a good solution [1]. Will look in to it over the weekend. If you want you can create some POC.

One thing that we need to be careful is the performance impact.

[1] https://docs.nestjs.com/fundamentals/injection-scopes

[eropple]

Cool--this should make breadcrumbs and messages easier too, for sure.

Performance impact of request-scoped stuff is negligible, in my experience. I wrote @eropple/nestjs-bunyan, which effectively makes your entire application, request-scoped and it has no meaningful performance impact. And a lot of other frameworks, like .NET's ASP.NET, are request-scoped by default.

I can try to get to this one this week, if you don't get there first.

[farzadso]

Any updates on this? How's the milestone going? @mentos1386

[mentos1386]

@farzadso moved to v7.0.0. I'm sadly not involved in NestJS world anymore, so it's hard for me to work on something that i don't use. I will try to find a new maintainer to push this project forward.

v6.0.0 will be released without this changes as it has some other improvements that should be released a while ago.

[farzadso]

@farzadso moved to v7.0.0. I'm sadly not involved in NestJS world anymore, so it's hard for me to work on something that i don't use. I will try to find a new maintainer to push this project forward.

v6.0.0 will be released without this changes as it has some other improvements that should be released a while ago.

Thanks for the update @mentos1386

I hope you can find a maintainer as we are considering using your library in our new project.

[mentos1386]

@9renpoto would you be willing to work on this?

[kdubb]

I'm not using this library (as I just discovered it) but I think adding breadcrumb support is pretty easy with the current interceptor the library already implements and without requiring request scoping. We changed our custom Sentry integration to look like this:

  intercept(context: ExecutionContext, next: CallHandler): Observable<any> {
    return of(null)
      .pipe(
        tap(() => {

          const scope = Sentry.getCurrentHub().pushScope();

          scope.setTag('controller', context.getClass().name);
          scope.setTag('handler', context.getHandler().name);

          if (context.getType() === 'http') {
            const http = context.switchToHttp();
            scope.setExtra('req', serializeRequest((http.getRequest() as FastifyRequest).req));
          }

        }),

        switchMapTo(next.handle()),

        tap(
          () => {
            Sentry.getCurrentHub().popScope();
          },
          (error) => {
            Sentry.getCurrentHub().captureException(error);
            Sentry.getCurrentHub().popScope();
          },
        ),
      );
}

It appears to be working properly and capturing breadcrumbs properly for specific requests

[mentos1386]

Looking at this again, the @kdubb solution is correct one.

We would have to fix our interceptor to similarly to use the scope at the beginning of request, and add some tags on it. This would make all those tags available to any subsequent sentry use inside controllers etc.

This should be very small and backwards-compatible change.