diff --git a/apps/backend/src/app/app.module.ts b/apps/backend/src/app/app.module.ts index 918e76f9f..692337411 100644 --- a/apps/backend/src/app/app.module.ts +++ b/apps/backend/src/app/app.module.ts @@ -1,12 +1,10 @@ import { Module } from '@nestjs/common'; -import { APP_FILTER } from '@nestjs/core'; +import { APP_FILTER, APP_INTERCEPTOR } from '@nestjs/core'; import { LoggerModule } from 'nestjs-pino'; -import * as Sentry from '@sentry/node'; import { ConfigModule, ConfigService } from '@nestjs/config'; import { FastifyMulterModule } from '@nest-lab/fastify-multer'; import { ExceptionHandlerFilter } from './filters/exception-handler.filter'; import { ConfigFactory, Environment, validate } from './config'; -import { SentryModule } from './modules/sentry/sentry.module'; import { AuthModule } from './modules/auth/auth.module'; import { ActivitiesModule } from './modules/activities/activities.module'; import { AdminModule } from './modules/admin/admin.module'; @@ -22,6 +20,7 @@ import { MapReviewModule } from './modules/map-review/map-review.module'; import { DbModule } from './modules/database/db.module'; import { KillswitchModule } from './modules/killswitch/killswitch.module'; import { HealthcheckModule } from './modules/healthcheck/healthcheck.module'; +import { setupNestInterceptor } from '../instrumentation'; @Module({ imports: [ @@ -31,34 +30,6 @@ import { HealthcheckModule } from './modules/healthcheck/healthcheck.module'; isGlobal: true, validate }), - // We use Sentry in production for error logging and performance tracing. - // This is a small wrapper module around @sentry/node that only inits in - // production if a valid DSN is set. - SentryModule.forRootAsync({ - useFactory: async (config: ConfigService) => { - // Whether to enable SentryInterceptor. If enabled, we run a transaction - // for the lifetime of tracesSampleRate * all HTTP requests. This - // provides more detailed error - const enableTracing = config.getOrThrow('sentry.enableTracing'); - return { - environment: config.getOrThrow('env'), - enableTracing, - sentryOpts: { - // If this isn't set in prod we won't init Sentry. - dsn: config.getOrThrow('sentry.dsn'), - enableTracing, - environment: config.getOrThrow('sentry.env'), - tracesSampleRate: config.getOrThrow('sentry.tracesSampleRate'), - integrations: config.getOrThrow('sentry.tracePrisma') - ? [Sentry.prismaIntegration()] - : undefined, - debug: false - } - }; - }, - imports: [DbModule.forRoot()], - inject: [ConfigService] - }), // Pino is a highly performant logger that outputs logs as JSON, which we // then export to Grafana Loki. This module sets up `pino-http` which logs // all HTTP requests (so no need for a Nest interceptor). @@ -100,6 +71,10 @@ import { HealthcheckModule } from './modules/healthcheck/healthcheck.module'; { provide: APP_FILTER, useClass: ExceptionHandlerFilter + }, + { + provide: APP_INTERCEPTOR, + useFactory: setupNestInterceptor } ] }) diff --git a/apps/backend/src/app/config/config.interface.ts b/apps/backend/src/app/config/config.interface.ts index 89beba72c..7a8a8a707 100644 --- a/apps/backend/src/app/config/config.interface.ts +++ b/apps/backend/src/app/config/config.interface.ts @@ -39,6 +39,7 @@ export interface ConfigInterface { sentry: { dsn: string; enableTracing: boolean; + enableNodeProfiling: boolean; tracesSampleRate: number; tracePrisma: boolean; env: 'production' | 'staging'; diff --git a/apps/backend/src/app/config/config.ts b/apps/backend/src/app/config/config.ts index ef1f299a0..4e44f423d 100644 --- a/apps/backend/src/app/config/config.ts +++ b/apps/backend/src/app/config/config.ts @@ -45,7 +45,11 @@ export const ConfigFactory = (): ConfigInterface => { enableTracing: process.env['SENTRY_ENABLE_TRACING'] === 'true' || false, tracesSampleRate: +process.env['SENTRY_TRACE_SAMPLE_RATE'] || 0, tracePrisma: process.env['SENTRY_TRACE_PRISMA'] === 'true' || false, - env: (process.env['SENTRY_ENV'] || '') as 'production' | 'staging' + env: + ((process.env['SENTRY_ENV'] || '') as 'production' | 'staging') || + 'staging', + enableNodeProfiling: + process.env['SENTRY_ENABLE_NODE_PROFILING'] === 'true' || false }, sessionSecret: process.env['SESSION_SECRET'] || '', steam: { diff --git a/apps/backend/src/app/config/config.validation.ts b/apps/backend/src/app/config/config.validation.ts index e18e9f52e..3e8e2afa2 100644 --- a/apps/backend/src/app/config/config.validation.ts +++ b/apps/backend/src/app/config/config.validation.ts @@ -82,6 +82,10 @@ export class ConfigValidation { @IsBoolean() readonly SENTRY_TRACE_PRISMA?: boolean; + @IsOptional() + @IsBoolean() + readonly SENTRY_ENABLE_NODE_PROFILING?: boolean; + @IsOptional() @IsIn(['production', 'staging']) readonly SENTRY_ENV?: 'production' | 'staging'; diff --git a/apps/backend/src/app/filters/exception-handler.filter.ts b/apps/backend/src/app/filters/exception-handler.filter.ts index c5e736d99..e60ef290f 100644 --- a/apps/backend/src/app/filters/exception-handler.filter.ts +++ b/apps/backend/src/app/filters/exception-handler.filter.ts @@ -4,24 +4,21 @@ import { ExceptionFilter, HttpException, HttpStatus, - Inject, - Logger + Logger, + ServiceUnavailableException } from '@nestjs/common'; import { HttpAdapterHost } from '@nestjs/core'; import { PrismaClientKnownRequestError } from '@prisma/client/runtime/library'; import { ConfigService } from '@nestjs/config'; import '@sentry/tracing'; // Required according to https://github.com/getsentry/sentry-javascript/issues/4731#issuecomment-1075410543 import * as Sentry from '@sentry/node'; -import { SENTRY_INIT_STATE } from '../modules/sentry/sentry.const'; -import { SentryInitState } from '../modules/sentry/sentry.interface'; import { Environment } from '../config'; @Catch() export class ExceptionHandlerFilter implements ExceptionFilter { constructor( private readonly httpAdapterHost: HttpAdapterHost, - private readonly configService: ConfigService, - @Inject(SENTRY_INIT_STATE) private readonly sentryEnabled: SentryInitState + private readonly configService: ConfigService ) {} private readonly logger = new Logger('Exception Filter'); @@ -69,9 +66,9 @@ export class ExceptionHandlerFilter implements ExceptionFilter { message: exception.message }; - // In production, send to Sentry so long as it's enabled (if the DSN is - // invalid/empty it'll be disabled). - if (this.sentryEnabled) { + // In production, send to Sentry so long as it's enabled + // TODO: maybe still want to inject sentry init state token + if (Sentry.isInitialized()) { eventID = Sentry.captureException(exception); msg.eventID = eventID; } diff --git a/apps/backend/src/app/interceptors/sentry.interceptor.ts b/apps/backend/src/app/interceptors/sentry.interceptor.ts index 534c1c234..36ad74251 100644 --- a/apps/backend/src/app/interceptors/sentry.interceptor.ts +++ b/apps/backend/src/app/interceptors/sentry.interceptor.ts @@ -1,39 +1,35 @@ -import * as Sentry from '@sentry/node'; import { CallHandler, ExecutionContext, Injectable, + Logger, NestInterceptor } from '@nestjs/common'; -import { Observable, tap } from 'rxjs'; +import { Observable } from 'rxjs'; +import { getIsolationScope } from '@sentry/node'; +import { getDefaultIsolationScope } from '@sentry/core'; @Injectable() export class SentryInterceptor implements NestInterceptor { intercept(context: ExecutionContext, next: CallHandler): Observable { - const request = context.switchToHttp().getRequest(); - const { method, url } = request; + if (getIsolationScope() === getDefaultIsolationScope()) { + Logger.warn( + 'Isolation scope is still the default isolation scope, skipping setting transactionName.' + ); + return next.handle(); + } - // Based on https://github.com/ericjeker/nestjs-sentry-example/blob/main/src/sentry/sentry.interceptor.ts, - // but updated for Sentry 7, which majorly changed how transactions/spans are handled. - return Sentry.startSpan( - { - op: 'http.server', - name: `${method} ${url}` - }, - (rootSpan: Sentry.Span) => - Sentry.startSpan( - { - op: 'http.handler', - name: `${context.getClass().name}.${context.getHandler().name}` - }, - (span: Sentry.Span) => - next.handle().pipe( - tap(() => { - span.end(); - rootSpan.end(); - }) - ) - ) - ); + if (context.getType() !== 'http') { + return next.handle(); + } + + const req = context.switchToHttp().getRequest(); + if (req.route) { + getIsolationScope().setTransactionName( + `${req.method?.toUpperCase() ?? 'UNKNOWN'} ${req.route.path}` + ); + } + + return next.handle(); } } diff --git a/apps/backend/src/app/modules/sentry/sentry.const.ts b/apps/backend/src/app/modules/sentry/sentry.const.ts deleted file mode 100644 index 719efb49b..000000000 --- a/apps/backend/src/app/modules/sentry/sentry.const.ts +++ /dev/null @@ -1,2 +0,0 @@ -export const SENTRY_MODULE_OPTIONS = Symbol('SentryModuleOptions'); -export const SENTRY_INIT_STATE = Symbol('SentryInitState'); diff --git a/apps/backend/src/app/modules/sentry/sentry.interface.ts b/apps/backend/src/app/modules/sentry/sentry.interface.ts deleted file mode 100644 index f328bd007..000000000 --- a/apps/backend/src/app/modules/sentry/sentry.interface.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { ModuleMetadata } from '@nestjs/common'; -import { NodeOptions } from '@sentry/node'; -import { Environment } from '../../config'; - -export interface SentryModuleOptions { - environment: Environment; - enableTracing: boolean; - sentryOpts: NodeOptions; -} - -export interface SentryModuleAsyncOptions - extends Pick { - inject?: any[]; - useFactory?: (...args: any[]) => Promise; -} - -export type SentryInitState = boolean; diff --git a/apps/backend/src/app/modules/sentry/sentry.module.ts b/apps/backend/src/app/modules/sentry/sentry.module.ts deleted file mode 100644 index 01200e2db..000000000 --- a/apps/backend/src/app/modules/sentry/sentry.module.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { DynamicModule, Logger, Module, Provider } from '@nestjs/common'; -import * as Sentry from '@sentry/node'; -import { Environment } from '../../config'; -import { - SentryInitState, - SentryModuleAsyncOptions, - SentryModuleOptions -} from './sentry.interface'; -import { SENTRY_INIT_STATE, SENTRY_MODULE_OPTIONS } from './sentry.const'; -import { SentryInterceptor } from '../../interceptors/sentry.interceptor'; -import { APP_INTERCEPTOR } from '@nestjs/core'; - -@Module({}) -export class SentryModule { - static forRootAsync(options: SentryModuleAsyncOptions): DynamicModule { - // Provides the options passed in from app.module to other providers - const optionsProvider: Provider = { - provide: SENTRY_MODULE_OPTIONS, - useFactory: options.useFactory, - inject: options.inject ?? [] - }; - - return { - module: SentryModule, - imports: options.imports, - providers: [ - optionsProvider, - { - inject: [SENTRY_MODULE_OPTIONS], - // Tracks whether we initialised Sentry and provides it to the - // below provider and other modules - provide: SENTRY_INIT_STATE, - // Instantiates Sentry, if in production and DSN is set - useFactory: ({ - environment, - sentryOpts - }: SentryModuleOptions): SentryInitState => { - const logger = new Logger('Sentry Module Setup'); - - if (environment !== Environment.PRODUCTION) { - logger.log(' Not in prod, not initing sentry'); - return false; - } - - if (!sentryOpts?.dsn || !sentryOpts?.environment) { - logger.warn( - 'Sentry DSN or environment not set, not initializing!' - ); - return false; - } - - Sentry.init(sentryOpts); - logger.log({ sentryOpts }, 'Initialised Sentry'); - return true; - } - }, - { - inject: [SENTRY_MODULE_OPTIONS, SENTRY_INIT_STATE], - provide: APP_INTERCEPTOR, - // Only actually instantiate the service if we initialised Sentry and - // have tracing enabled - useFactory: ( - { enableTracing }: SentryModuleOptions, - initState: SentryInitState - ) => - enableTracing && initState ? new SentryInterceptor() : undefined - } - ], - exports: [SENTRY_INIT_STATE] - }; - } -} diff --git a/apps/backend/src/instrumentation.ts b/apps/backend/src/instrumentation.ts new file mode 100644 index 000000000..01df45811 --- /dev/null +++ b/apps/backend/src/instrumentation.ts @@ -0,0 +1,78 @@ +/* eslint @typescript-eslint/naming-convention: 0 */ +import * as Sentry from '@sentry/node'; +import { + NodeOptions, + nestIntegration, + prismaIntegration, + spanToJSON, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN +} from '@sentry/node'; +import { Integration } from '@sentry/types'; +import { nodeProfilingIntegration } from '@sentry/profiling-node'; +import { pino } from 'pino'; +import { SentryInterceptor } from './app/interceptors/sentry.interceptor'; +import { Environment } from './app/config'; + +// Sentry now has a dedicated Nest in integration but doesn't let us integrate +// nicely with our exception filter, also it's in an alpha state. +// Mostly following their approach from +// https://github.com/getsentry/sentry-javascript/blob/develop/packages/node/src/integrations/tracing/nest.ts + +const dsn = process.env['SENTRY_DSN']; +const enableTracing = process.env['SENTRY_ENABLE_TRACING'] === 'true'; +const sampleRate = +process.env['SENTRY_TRACE_SAMPLE_RATE']; + +const integrations: Integration[] = [nestIntegration()]; + +const logger = pino(); + +if (process.env['SENTRY_TRACE_PRISMA'] === 'true') { + integrations.push(prismaIntegration()); +} + +//https://docs.sentry.io/platforms/javascript/guides/node/profiling/#runtime-flags +if (process.env['SENTRY_ENABLE_NODE_PROFILING'] === 'true') { + integrations.push(nodeProfilingIntegration()); +} + +const opts: NodeOptions = { + dsn, + environment: process.env['SENTRY_ENV'], + enableTracing, + tracesSampleRate: sampleRate, + profilesSampleRate: sampleRate, + debug: false, + integrations +}; + +if (process.env['NODE_ENV'] === Environment.PRODUCTION && dsn) { + logger.info('Initializing Sentry'); + Sentry.init(opts); +} + +export function setupNestInterceptor() { + if (!Sentry.isInitialized() || !process.env['SENTRY_DSN']) return; + + const client = Sentry.getClient(); + if (!client) return; + + client.on('spanStart', (span) => { + const attributes = spanToJSON(span).data || {}; + + // this is one of: app_creation, request_context, handler + const type = attributes['nestjs.type']; + + // If this is already set, or we have no nest.js span, no need to process again... + if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) { + return; + } + + span.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.nestjs', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.nestjs` + }); + }); + + return new SentryInterceptor(); +} diff --git a/apps/backend/src/main.ts b/apps/backend/src/main.ts index d117a9ffb..0a4f38da0 100644 --- a/apps/backend/src/main.ts +++ b/apps/backend/src/main.ts @@ -1,3 +1,4 @@ +import './instrumentation'; // This must be first import!! import { NestFactory, Reflector } from '@nestjs/core'; import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger'; import {