Skip to content

Commit

Permalink
deps(sentry): upgrade Sentry to version 8
Browse files Browse the repository at this point in the history
Because:
 - Sentry 8 is the latest version

This commit:
 - deletes some integration code since Sentry does it automatically
 - updates code to use the Sentry 8 API
  • Loading branch information
chenba committed Sep 10, 2024
1 parent b9e82fd commit 5d440c9
Show file tree
Hide file tree
Showing 30 changed files with 587 additions and 267 deletions.
8 changes: 6 additions & 2 deletions libs/shared/sentry/src/lib/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ export const _Sentry = {
* Modified error object data
* @private
*/
function beforeSend(opts: SentryConfigOpts, event: Sentry.Event, hint?: any) {
function beforeSend(
opts: SentryConfigOpts,
event: Sentry.ErrorEvent,
hint?: Sentry.EventHint
) {
if (sentryEnabled === false) {
return null;
}
Expand Down Expand Up @@ -171,7 +175,7 @@ function configure(config: SentryConfigOpts, log?: Logger) {
enableInp: true,
}),
],
beforeSend: function (event: Sentry.Event, hint?: any) {
beforeSend: function (event: Sentry.ErrorEvent, hint: Sentry.EventHint) {
return beforeSend(opts, event, hint);
},
});
Expand Down
9 changes: 5 additions & 4 deletions libs/shared/sentry/src/lib/nest/sentry.interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
NestInterceptor,
} from '@nestjs/common';
import * as Sentry from '@sentry/node';
import { Transaction } from '@sentry/types';

import { ignoreError, processException } from '../reporting';

Expand All @@ -22,11 +21,13 @@ export class SentryInterceptor implements NestInterceptor {
// If there is http context request start a transaction for it. Note that this will not
// pick up graphql queries
const req = context.switchToHttp().getRequest();
let transaction: Transaction;
let transaction: Sentry.Span;

if (req) {
transaction = Sentry.startTransaction({
transaction = Sentry.startInactiveSpan({
op: 'nestjs.http',
name: `${req.method} ${req.path}`,
forceTransaction: true,
});
}

Expand All @@ -42,7 +43,7 @@ export class SentryInterceptor implements NestInterceptor {
},
}),
finalize(() => {
transaction?.finish();
transaction?.end();
})
);
}
Expand Down
17 changes: 9 additions & 8 deletions libs/shared/sentry/src/lib/nest/sentry.plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
} from '@apollo/server';
import { Plugin } from '@nestjs/apollo';
import * as Sentry from '@sentry/node';
import { Transaction } from '@sentry/types';

import {
ExtraContext,
Expand All @@ -33,14 +32,15 @@ import { Inject } from '@nestjs/common';
import { MozLoggerService } from '@fxa/shared/mozlog';

interface Context extends BaseContext {
transaction: Transaction;
transaction: Sentry.Span;
request: Request;
}

export async function createContext(ctx: any): Promise<Context> {
const transaction = Sentry.startTransaction({
const transaction = Sentry.startInactiveSpan({
op: 'gql',
name: 'GraphQLTransaction',
forceTransaction: true,
});
return { request: ctx.req, transaction };
}
Expand All @@ -57,7 +57,7 @@ export class SentryPlugin implements ApolloServerPlugin<Context> {

if (request.operationName != null) {
try {
contextValue.transaction.setName(request.operationName);
contextValue.transaction.updateName(request.operationName);
} catch (err) {
log.error('sentry-plugin', err);
}
Expand All @@ -75,18 +75,19 @@ export class SentryPlugin implements ApolloServerPlugin<Context> {
async executionDidStart() {
return {
willResolveField({ contextValue, info }) {
let span: any;
let span: Sentry.Span;
try {
span = contextValue.transaction.startChild({
span = Sentry.startInactiveSpan({
op: 'resolver',
description: `${info.parentType.name}.${info.fieldName}`,
name: `${info.parentType.name}.${info.fieldName}`,
parentSpan: contextValue.transaction,
});
} catch (err) {
log.error('sentry-plugin', err);
}

return () => {
span?.finish();
span?.end();
};
},
};
Expand Down
6 changes: 2 additions & 4 deletions libs/shared/sentry/src/lib/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import * as Sentry from '@sentry/node';
import { ErrorEvent } from '@sentry/types';
import { ExtraErrorData } from '@sentry/integrations';
import { extraErrorDataIntegration } from '@sentry/node';
import { SentryConfigOpts } from './models/SentryConfigOpts';
import { buildSentryConfig } from './config-builder';
import { tagFxaName } from './reporting';
Expand Down Expand Up @@ -36,8 +36,7 @@ export function initSentry(config: InitSentryOpts, log: Logger) {
};

const integrations = [
// Default
new ExtraErrorData({ depth: 5 }),
extraErrorDataIntegration({ depth: 5 }),

// Custom Integrations
...(config.integrations || []),
Expand All @@ -46,7 +45,6 @@ export function initSentry(config: InitSentryOpts, log: Logger) {
try {
Sentry.init({
// Defaults Options
instrumenter: 'otel',
normalizeDepth: 6,
maxValueLength: 500,

Expand Down
8 changes: 4 additions & 4 deletions libs/shared/sentry/src/lib/reporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,11 @@ export function reportRequestException(
}

Sentry.withScope((scope: Sentry.Scope) => {
scope.addEventProcessor((event: Sentry.Event) => {
scope.addEventProcessor((event) => {
if (request) {
const sentryEvent = Sentry.Handlers.parseRequest(event, request);
sentryEvent.level = 'error';
return sentryEvent;
event.request = Sentry.extractRequestData(request);
event.level = 'error';
return event;
}
return null;
});
Expand Down
7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,9 @@
"@paypal/react-paypal-js": "^8.6.0",
"@radix-ui/react-form": "^0.0.3",
"@radix-ui/react-tooltip": "^1.1.2",
"@sentry/browser": "^7.113.0",
"@sentry/integrations": "^7.113.0",
"@sentry/node": "^7.113.0",
"@sentry/opentelemetry-node": "^7.113.0",
"@sentry/browser": "^8.28.0",
"@sentry/node": "^8.28.0",
"@sentry/opentelemetry": "^8.28.0",
"@swc/helpers": "0.5.11",
"@type-cacheable/core": "^14.0.1",
"@type-cacheable/ioredis-adapter": "^10.0.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/browserid-verifier/lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ if (sentryConfig.dsn) {
return event;
},
});
app.use(Sentry.Handlers.requestHandler());
Sentry.setupExpressErrorHandler(app);
}

var verifier = new CCVerifier({
Expand Down
5 changes: 1 addition & 4 deletions packages/fxa-admin-panel/server/lib/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ logger.info('version', { version: version });

// Initialize Sentry
const sentryConfig = config.get('sentry');
if (sentryConfig.dsn) {
app.use(Sentry.Handlers.requestHandler());
}

app.use(
helmet.frameguard({
Expand Down Expand Up @@ -145,7 +142,7 @@ if (proxyUrl) {

// Send errors to sentry.
if (sentryConfig.dsn) {
app.use(Sentry.Handlers.errorHandler());
Sentry.setupExpressErrorHandler(app);
}

export default app;
Expand Down
2 changes: 1 addition & 1 deletion packages/fxa-auth-server/lib/monitoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ initMonitoring({
...config.getProperties(),
release: version,
eventFilters: [filterSentryEvent],
integrations: [new Sentry.Integrations.LinkedErrors({ key: 'jse_cause' })],
integrations: [Sentry.linkedErrorsIntegration({ key: 'jse_cause' })],
},
});

Expand Down
30 changes: 10 additions & 20 deletions packages/fxa-auth-server/lib/sentry.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,8 @@ function reportSentryError(err, request) {

Sentry.withScope((scope) => {
if (request) {
scope.addEventProcessor((_sentryEvent) => {
const sentryEvent = Sentry.Handlers.parseRequest(
_sentryEvent,
request.raw.req
);
scope.addEventProcessor((sentryEvent) => {
sentryEvent.request = Sentry.extractRequestData(request.raw.req);
sentryEvent.level = 'error';
return sentryEvent;
});
Expand Down Expand Up @@ -106,9 +103,7 @@ function reportSentryError(err, request) {

async function configureSentry(server, config, processName = 'key_server') {
if (config.sentry.dsn) {
Sentry.configureScope((scope) => {
scope.setTag('process', processName);
});
Sentry.getCurrentScope().setTag('process', processName);

if (!server) {
return;
Expand All @@ -120,6 +115,7 @@ async function configureSentry(server, config, processName = 'key_server') {
method(request, h) {
request.sentryScope = new Sentry.Scope();

/**
// Make a transaction per request so we can get performance monitoring. There are
// some limitations to this approach, and distributed tracing will be off due to
// hapi's architecture.
Expand All @@ -128,23 +124,17 @@ async function configureSentry(server, config, processName = 'key_server') {
// looks like there might be some other solutions that are more complex, but would work
// with hapi and distributed tracing.
//
const transaction = Sentry.startTransaction(
{
op: 'auth-server',
name: `${request.method.toUpperCase()} ${request.path}`,
},
{
request: Sentry.Handlers.extractRequestData(request.raw.req),
}
);

Sentry.configureScope((scope) => {
scope.setSpan(transaction);
const transaction = Sentry.startInactiveSpan({
op: 'auth-server',
name: `${request.method.toUpperCase()} ${request.path}`,
forceTransaction: true,
request: Sentry.extractRequestData(request.raw.req),
});
request.app.sentry = {
transaction,
};
//*/

return h.continue;
},
Expand Down
4 changes: 3 additions & 1 deletion packages/fxa-auth-server/test/oauth/routes/token.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
const { assert } = require('chai');
const buf = require('buf').hex;
const hex = require('buf').to.hex;
const path = require('path');
const proxyquire = require('proxyquire');
const sinon = require('sinon');

Expand All @@ -23,7 +24,8 @@ const CODE_WITHOUT_KEYS = 'f0f0f0';
const mockDb = { touchSessionToken: sinon.stub() };
const mockStatsD = { increment: sinon.stub() };
const mockGlean = { oauth: { tokenCreated: sinon.stub() } };
const tokenRoutes = proxyquire('../../../lib/routes/oauth/token', {
const tokenRoutePath = path.join(__dirname, '../../../lib/routes/oauth/token');
const tokenRoutes = proxyquire(tokenRoutePath, {
'../../oauth/assertion': async () => true,
'../../oauth/client': {
authenticateClient: (_, params) => ({
Expand Down
6 changes: 0 additions & 6 deletions packages/fxa-content-server/app/scripts/lib/sentry.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,6 @@ function SentryMetrics(config) {
const opts = buildSentryConfig(config, this._logger);
Sentry.init({
...opts,
instrumenter: 'otel',
integrations: [
Sentry.browserTracingIntegration({
enableInp: true,
}),
],
beforeSend(event) {
event = tagFxaName(event, opts.clientName);
event = beforeSend(event);
Expand Down
23 changes: 9 additions & 14 deletions packages/fxa-content-server/server/bin/fxa-content-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,6 @@ function makeApp() {
app.set('view engine', 'html');
app.set('views', PAGE_TEMPLATE_DIRECTORY);

// The request handler must be the first item
app.use(sentry.sentryModule.Handlers.requestHandler());

// i18n adds metadata to a request to help
// with translating templates on the server.
app.use(i18n);
Expand Down Expand Up @@ -237,17 +234,15 @@ function makeApp() {
});

// The sentry error handler must be before any other error middleware
app.use(
sentry.sentryModule.Handlers.errorHandler({
shouldHandleError(error) {
const success = tryCaptureValidationError(error);

// If the validation was explicitly captured, we return false. Otherwise the
// error is reported twice.
return !success;
},
})
);
sentry.sentryModule.setupExpressErrorHandler(app, {
shouldHandleError(error) {
const success = tryCaptureValidationError(error);

// If the validation was explicitly captured, we return false. Otherwise the
// error is reported twice.
return !success;
},
});

// log and capture any errors
app.use((err, req, res, next) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/fxa-content-server/server/lib/sentry.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ if (config.get('sentry.dsn')) {
event = tagFxaName(event, opts.serverName);
return event;
},
integrations: [new Sentry.Integrations.LinkedErrors({ key: 'jse_cause' })],
integrations: [Sentry.linkedErrorsIntegration({ key: 'jse_cause' })],
});
}

Expand Down
2 changes: 1 addition & 1 deletion packages/fxa-customs-server/lib/monitoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ initMonitoring({
config: {
...config,
release: version,
integrations: [new Sentry.Integrations.LinkedErrors({ key: 'jse_cause' })],
integrations: [Sentry.linkedErrorsIntegration({ key: 'jse_cause' })],
},
});
Loading

0 comments on commit 5d440c9

Please sign in to comment.