feat: add sentry #59
feat: add sentry #59
Conversation
View stack outputs
|
a07686a to
992df44
Compare
stacks/config.js
Outdated
| /** | ||
| * @param {import('@serverless-stack/resources').Stack} stack | ||
| */ | ||
| export function setupSentry (stack) { |
There was a problem hiding this comment.
maybe we should check stage here instead? thoughts?
There was a problem hiding this comment.
yes, we're using the config file set config depending on the stage already, so let's do the if (!app.local) check once here rather than in every stack
There was a problem hiding this comment.
Use app.local to determine if we want Sentry. There isn't really a dev env. When i deploy locally the scope is olizilla.
app.local tells us if the deploy is in dev mode from running sst start, rather than a deploy. I think it'd be fine to put that check once in the config file as that is how we do other per stage config.
From the blog post it looks like you chose different options, can you explain a little about NODE_OPTIONS: "-r @sentry/serverless/dist/awslambda-auto", ? what does this do? Why not use it? Does it auto init with the sentry env vars?
I think that I removed it involuntarily, because I thought I had it. Adding it back, I read that it sets automatic instrumentation of Lambda functions in Sentry. But in practise we need to see what it sets up when we actually have events there. There are NO nice docs for it, just the blogpost I linked |
|
i'm trying to understand what it does. I can see
and
import * as Sentry from './index';
const lambdaTaskRoot = process.env.LAMBDA_TASK_ROOT;
if (lambdaTaskRoot) {
const handlerString = process.env._HANDLER;
if (!handlerString) {
throw Error(`LAMBDA_TASK_ROOT is non-empty(${lambdaTaskRoot}) but _HANDLER is not set`);
}
Sentry.AWSLambda.init({
_invokedByLambdaLayer: true,
});
Sentry.AWSLambda.tryPatchHandler(lambdaTaskRoot, handlerString);
} else {
throw Error('LAMBDA_TASK_ROOT environment variable is not set');
}...given we are manually wrapping and initialising sentry and have included |
|
It is weird that that they recommend both things in their (it is sentry's own) blogpost and then only one is needed. Did you setup this for pickup/linkdex? I saw
I can definitely try without it first and report back if it works. For this, I can add a simple route handler in our API that throws an Error |
|
The blogpost doesn't say to install
|
Yes please. This has been really helpful for testing Sentry in the web3.storage api |
f64c616 to
acf4009
Compare
acf4009 to
e2caa73
Compare
|
@olizilla great instinct! Looks like Lambda layer is not really required. We only need to do: within the stack, so that functions are able to get this. Otherwise, they do not send nothing to sentry. |
|
Also, will keep /error to test in staging and make sure everything is also good there |
|
|
||
| import parseSqsEvent from '../utils/parse-sqs-event.js' | ||
|
|
||
| import { writeSatnavIndex } from '../index.js' | ||
|
|
||
| Sentry.AWSLambda.init({ | ||
| dsn: process.env.SENTRY_DSN, | ||
| tracesSampleRate: 1.0, |
There was a problem hiding this comment.
Note that this is gonna send telemetry to sentry from every invocation... this opts us in to Sentry perf monitoring where it tracks the response time of every request rather than just errors. The sample rate of 1 means "send to sentry for every request, no filtering"
There was a problem hiding this comment.
yes, I guess we can tweak or disable when launching or after we understand better our performance profile
I have set a new one last week for new project |
Co-authored-by: Oli Evans <oli@protocol.ai>
This was missing from #59 and errors happening within ucanto service were not being propagated to sentry
Adds Sentry to project based on https://blog.sentry.io/2022/02/03/guest-post-how-to-monitor-lambda-functions-in-your-sst-application/
Needs:
Closes #20