fastify / help

Need help with Fastify? File an Issue here.
https://www.fastify.io/
64 stars 8 forks source link

Help finding best approach to integrate Sentry #601

Closed danechitoaie closed 2 years ago

danechitoaie commented 2 years ago

Hi,

I need some help/pointers where to start so I can integrate Sentry into a fastify application. Unfortunately they (Sentry) only provide integrations for Express and Koa so I will need to do it myself. I'm looking at the source code of the express js integration https://github.com/getsentry/sentry-javascript/blob/master/packages/node/src/handlers.ts#L412-L417

And from what I see they use domains and wrap the handler in a domain.run callback:

    const local = domain.create();
    local.add(req);
    local.add(res);
    local.on('error', next);

    local.run(() => {
    ...

Any idea how I can do the same in Fastify? As from what I see the faastify architecture is different, we have separate hooks for onRequest, onResponse. Any idea how I would be able to wrap a handler in this domain.run() ?

kibertoad commented 2 years ago

https://www.npmjs.com/package/fastify-sentry doesn't work?

danechitoaie commented 2 years ago

It's not maintained from what it seems (last update 3 years ago) and it only sets a error handler and catches the errors in there. https://github.com/alex-ppg/fastify-sentry/blob/master/index.js#L11

Since then Sentry also added support for tracing and performance monitoring and being able to performance track specific actions/parts of a handler, etc.. So there much more functionality available that that plugin does not support.

xr0master commented 2 years ago

@danechitoaie I am also checking it, and I am planning to do this integration like the express sentry. I think hooks onRequest and onError will help here. I also think doing the tracing and the performance monitoring via hooks also won't be complicated.

danechitoaie commented 2 years ago

Let me know how it goes.

I was able to successfully use the onRequest/onResponse hooks to start/finish a transaction for performance monitoring. Transactions are isolated per request so they work ok. The problem is that Sentry seems to have some functionality that only works in a global way (which doesn't make sense and causes concurrency issues on nodejs server side as here you have many requests handling different users).

For example one of the places where I got stuck was this: https://docs.sentry.io/platforms/node/enriching-events/identify-user/

They basically say you should use

Sentry.setUser({ email: "john.doe@example.com" });

to set a user. Problem is that this sets the data globally not isolated per each request. So you end up kind of like having a global variable that you keep overwriting in each request with different data and you can see from this that it's not ok, and leads to having some requests associated with incorrect users from other requests.

Same problem reported by other users as well: https://forum.sentry.io/t/correct-way-to-setuser-in-node-sdk/14355

If you look at the source code of their handler for Express.js here https://github.com/getsentry/sentry-javascript/blob/master/packages/node/src/handlers.ts#L412-L447

They use domains

    const local = domain.create();
    local.add(req);
    local.add(res);
    local.on('error', next);

    local.run(() => {
    ...

And run the express.js handler inside that callback passed to domain.run(). I'm not familiar with Node.js domains functionality but somehow even if they keep setting that scope.addEventProcessor each time the request is processed because it runs inside the domin.run callback it's added only once. If I try to do the same in the onRequest hook I end up having the event processor being added multiple times, once for each request, and not just once and that's it.

And in Fastify i was not able to figure out how to use domains since we would basically have to wrap the handler in a domain.run(). Also I don't think we should even use domains as it seems they are deprecated in Node.js 16.x and above.

Anyway... as I was saying my blocker at the moment is that I can't associate a user with a transaction. I have some workaround ideas in my head by they seem like hacks. So please do let us know if you find any better way.

jsumners commented 2 years ago

You may want to reach out to your account representative and request they add support.

mcollina commented 2 years ago

From what you are telling, there are so many things that use out-of-date in that Sentry code that you are probably better off with using a competitor. domain has been deprecated for ages for example.

xr0master commented 2 years ago

@danechitoaie They use the domain in the express.js to catch errors, the fastify hook onError also catches them.

As far as I can see, they create a scope and add the user to the scope. So, it should be for each session and cannot be global... Strange. But even if there is some problem with this, it is possible to add the user at the very end when the error has been caught.

danechitoaie commented 2 years ago

@xr0master that's just for catching errors. That would work as Sentry.captureErrror/captureMessage have the ability to pass the context as an extra param so we can push the user there.

But my the problem is not with capturing errors. The problem is with tracing. With tracing you want to send events even when there's no error (so Sentry.captureErrror/captureMessage is never called, onError hook is never called). Just normal successful requests that I want to trace for performance and get info about how much time this op took, how much time that other op took, etc. And problem is that you can't associate a user with these requests unless you use the global Sentry.setUser which has the problems described above.

xr0master commented 2 years ago

A basic Sentry integration would be something like this:

import { captureException, getCurrentHub } from '@sentry/core';

import type { Event } from '@sentry/node';
import type { FastifyInstance, FastifyRequest } from 'fastify';

function parseRequest(event: Event, req: FastifyRequest) {
  event.contexts = {
    ...event.contexts,
    runtime: {
      name: 'node',
      version: process.version,
    },
  };

  event.request = {
    ...event.request,
    url: `${req.protocol}://${req.hostname}${req.url}`,
    method: req.method,
    headers: req.headers as Record<string, string>,
    query_string: req.query as Record<string, string>,
  };

  return event;
}

export const sentryHandler = (fastify: FastifyInstance) => {
  fastify.addHook('onRequest', (req, res, done) => {
    const currentHub = getCurrentHub();

    currentHub.configureScope((scope) => {
      scope.addEventProcessor((event) => parseRequest(event, req));
    });

    done();
  });

  fastify.addHook('onError', (req, res, error) => {
    if (!error.statusCode || error.statusCode >= 500) {
      captureException(error);
      return res;
    }
  });
};

@danechitoaie Sorry friend, as you said you want to measure performance. Why do you need to know the users for this? However, it could be something like this:

const currentHub = getCurrentHub();

currentHub.configureScope((scope) => {
  scope.setUser('my best user')
});

You can also use tags. IMO

danechitoaie commented 2 years ago
  fastify.addHook('onRequest', (req, res, done) => {
    const currentHub = getCurrentHub();

    currentHub.configureScope((scope) => {
      scope.addEventProcessor((event) => parseRequest(event, req));
    });

    done();
  });

This will keep adding the event processor every time a request is made. Try to add a console.log or something inside parse request to see. It will end up being exponentially called with each request being done until one point where the app will crash I guess.

xr0master commented 2 years ago

@danechitoaie you are right! The scope is not created correctly... ok, we can do it manually.

fastify.addHook('onRequest', (req, res, done) => {
  const currentHub = getCurrentHub();
  currentHub.pushScope(); // create a new request scope

  currentHub.configureScope((scope) => {
    scope.addEventProcessor((event) => parseRequest(event, req));
  });

  done();
});

fastify.addHook('onResponse', (req, res, done) => {
  const currentHub = getCurrentHub();
  currentHub.popScope(); // destroy the current request scope

  done();
});

I tested it and it looks good. The error is clear without duplicates.

danechitoaie commented 2 years ago

Any chance that the onResponse hook will not execute? In case there's som error, or something and we end up with a scope that was pushed but not popped?

kibertoad commented 2 years ago

@danechitoaie Can't you use onError hook as a safety net?

danechitoaie commented 2 years ago

Yes. It was just not clear to me from the request lifecycle is the onResponse always executed, even for errors? As technically even an error triggers a response set to the user. Or if there's some kind of unhandled error only onError is executed and onResponse is skipped. Or third option both are executed?

Eomm commented 2 years ago

I found this on twitter https://github.com/immobiliare/fastify-sentry

danechitoaie commented 2 years ago

@Eomm this one is the same as the others. Only has support for catching errors. It doesn't have support for tracing/performance monitoring.

kurtwiersma-imaware commented 2 years ago

@danechitoaie we are trying to get Sentry performance monitoring and tracing working with Fastify. Were you able to get it working? If so could you post the complete final set of code you used?

We successfully are using Fastify-Sentry to track errors but are hoping to also have APM info come in as well and haven't found any examples online of how to do that yet other then this thread.

danechitoaie commented 2 years ago

I've setup the following plugin:

import { FastifyInstance, FastifyPluginOptions, FastifyRequest, FastifyReply } from "fastify";
import fp from "fastify-plugin";
import * as Sentry from "@sentry/node";
import { Transaction } from "@sentry/types";

declare module "fastify" {
    interface FastifyRequest {
        sentryTx: Transaction;
    }
}

interface SentryPluginOptions extends FastifyPluginOptions {
    environment: string;
    dsn: string;
}

export default fp(async (app: FastifyInstance, options: SentryPluginOptions) => {
    Sentry.init({
        enabled: options.environment === "production",
        dsn: options.dsn,
        environment: options.environment,
        defaultIntegrations: false,
    });

    Sentry.configureScope((scope) => {
        scope.addEventProcessor((event) => {
            const traceData = event.contexts?.trace?.data as {
                user?: {
                    uid: string;
                    upn: string;
                    ip: string;
                };

                request?: {
                    method: string;
                };
            };

            if (!traceData) {
                return event;
            }

            const { user } = traceData;
            if (user) {
                event.user = {
                    id: user.uid,
                    username: user.upn,
                    email: user.upn,
                    // eslint-disable-next-line @typescript-eslint/naming-convention
                    ip_address: user.ip,
                    ...event.user,
                };
            }

            const { request } = traceData;
            if (request) {
                event.request = {
                    method: request.method,
                    ...event.request,
                };
            }

            return event;
        });
    });

    app.decorateRequest("sentryTx", null);

    app.addHook("onRequest", (request: FastifyRequest, _reply: FastifyReply, done) => {
        request.sentryTx = Sentry.startTransaction({
            name: `${request.method} ${request.url}`,
            op: "http.server",
            description: "HTTP request",
        });
        request.sentryTx.setData("request", { method: request.method });
        done();
    });

    app.addHook("onResponse", (request: FastifyRequest, reply: FastifyReply, done) => {
        request.sentryTx.setHttpStatus(reply.statusCode);
        request.sentryTx.finish();
        done();
    });
});

And then wherever you want to track the performance of some block of code:

        const sentryTxSpan = request.sentryTx.startChild({ op: "op.name", description: "OP Description" });
        try {
            // ...
            // do stuff
            // ...
        } catch (err) {
            // ...
        } finally {
            sentryTxSpan.finish();
        }

And to attach user info to the requests:

request.sentryTx.setData("user", { uid: oid, upn, ip: ipaddr });

Notice in the first block of code how I'm reading data from the context object and moving it to event.user.

kurtwiersma-imaware commented 2 years ago

@danechitoaie this is very helpful thank you! Did you also have to provide tracesSampleRate: 1.0 or something similar to the Sentry.init() call?

kurtwiersma-imaware commented 2 years ago

I had to add import '@sentry/tracing'; to the top of @danechitoaie 's code along with tracesSampleRate: 1.0 and then it worked. Thank you Daniel!

danechitoaie commented 2 years ago

Maybe you also need to remove defaultIntegrations: false,. In my case I didn't need all those default tracing integrations. I just wanted to use my custom ones.

mohd-akram commented 8 months ago

This is how I'm using Sentry in Fastify, for reference:

import middie from "@fastify/middie";
import Sentry from "@sentry/node";

Sentry.init({ dsn: "your-dsn-here" });

export default async function (fastify, opts) {
  await fastify.register(middie);
  // The request handler must be the first middleware on the app
  fastify.use(Sentry.Handlers.requestHandler());

  const sentryErrorHandler = Sentry.Handlers.errorHandler();
  fastify.setErrorHandler(function (error, request, reply) {
    sentryErrorHandler(error, request.raw, reply.raw, () => {});
    return reply.status(error.statusCode ?? 500);
  });
}
wise-introvert commented 7 months ago

I've setup the following plugin:

import { FastifyInstance, FastifyPluginOptions, FastifyRequest, FastifyReply } from "fastify";
import fp from "fastify-plugin";
import * as Sentry from "@sentry/node";
import { Transaction } from "@sentry/types";

declare module "fastify" {
    interface FastifyRequest {
        sentryTx: Transaction;
    }
}

interface SentryPluginOptions extends FastifyPluginOptions {
    environment: string;
    dsn: string;
}

export default fp(async (app: FastifyInstance, options: SentryPluginOptions) => {
    Sentry.init({
        enabled: options.environment === "production",
        dsn: options.dsn,
        environment: options.environment,
        defaultIntegrations: false,
    });

    Sentry.configureScope((scope) => {
        scope.addEventProcessor((event) => {
            const traceData = event.contexts?.trace?.data as {
                user?: {
                    uid: string;
                    upn: string;
                    ip: string;
                };

                request?: {
                    method: string;
                };
            };

            if (!traceData) {
                return event;
            }

            const { user } = traceData;
            if (user) {
                event.user = {
                    id: user.uid,
                    username: user.upn,
                    email: user.upn,
                    // eslint-disable-next-line @typescript-eslint/naming-convention
                    ip_address: user.ip,
                    ...event.user,
                };
            }

            const { request } = traceData;
            if (request) {
                event.request = {
                    method: request.method,
                    ...event.request,
                };
            }

            return event;
        });
    });

    app.decorateRequest("sentryTx", null);

    app.addHook("onRequest", (request: FastifyRequest, _reply: FastifyReply, done) => {
        request.sentryTx = Sentry.startTransaction({
            name: `${request.method} ${request.url}`,
            op: "http.server",
            description: "HTTP request",
        });
        request.sentryTx.setData("request", { method: request.method });
        done();
    });

    app.addHook("onResponse", (request: FastifyRequest, reply: FastifyReply, done) => {
        request.sentryTx.setHttpStatus(reply.statusCode);
        request.sentryTx.finish();
        done();
    });
});

And then wherever you want to track the performance of some block of code:

        const sentryTxSpan = request.sentryTx.startChild({ op: "op.name", description: "OP Description" });
        try {
            // ...
            // do stuff
            // ...
        } catch (err) {
            // ...
        } finally {
            sentryTxSpan.finish();
        }

And to attach user info to the requests:

request.sentryTx.setData("user", { uid: oid, upn, ip: ipaddr });

Notice in the first block of code how I'm reading data from the context object and moving it to event.user.

@danechitoaie, how do I use this with fastify.register? New to Fasitfy and trying to setup Sentry :sweat_smile:

xr0master commented 7 months ago

@wise-introvert, you may check the topic about Fastify https://github.com/getsentry/sentry-javascript/issues/4784#issuecomment-1695298330

kibertoad commented 7 months ago

@anonrig Any thoughts and recommendations on this?

mydea commented 7 months ago

Hey, we are currently working on v8 of the SDK which will have an updated tracing engine for Node under the hood. We'll add proper Fastify support in the process of doing so! You can follow progress in this issue: https://github.com/getsentry/sentry-javascript/issues/4784

lilouartz commented 1 month ago

Related issue https://github.com/getsentry/sentry-javascript/issues/4784#issuecomment-2295017926