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
 - updates a few proxyquire calls because they were cause module not
   found errors in CI
  • Loading branch information
chenba committed Sep 25, 2024
1 parent 3da291f commit d3be5f3
Show file tree
Hide file tree
Showing 33 changed files with 432 additions and 591 deletions.
13 changes: 6 additions & 7 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 @@ -166,12 +170,7 @@ function configure(config: SentryConfigOpts, log?: Logger) {
try {
Sentry.init({
...opts,
integrations: [
Sentry.browserTracingIntegration({
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
19 changes: 10 additions & 9 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 @@ -66,7 +66,7 @@ export class SentryPlugin implements ApolloServerPlugin<Context> {
return {
async willSendResponse({ contextValue }) {
try {
contextValue.transaction.finish();
contextValue.transaction.end();
} 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}`,
scope: contextValue.transaction,
});
} catch (err) {
log.error('sentry-plugin', err);
}

return () => {
span?.finish();
span?.end();
};
},
};
Expand Down
5 changes: 0 additions & 5 deletions libs/shared/sentry/src/lib/next/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ export function initSentryForNextjsClient(
try {
Sentry.init({
...opts,
integrations: [
Sentry.browserTracingIntegration({
enableInp: true,
}),
],
beforeSend: function (event: Sentry.ErrorEvent) {
return beforeSend(sentryEnabled, opts, event);
},
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 @@ -39,8 +39,7 @@ export function initSentry(config: InitSentryOpts, log: Logger) {
};

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

// Custom Integrations
...(config.integrations || []),
Expand All @@ -49,7 +48,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
9 changes: 4 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,10 @@
"@paypal/react-paypal-js": "^8.7.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/nextjs": "^8",
"@sentry/node": "^7.113.0",
"@sentry/opentelemetry-node": "^7.113.0",
"@sentry/browser": "^8.28.0",
"@sentry/nextjs": "^8.28.0",
"@sentry/node": "^8.28.0",
"@sentry/opentelemetry": "^8.28.0",
"@swc/helpers": "0.5.11",
"@type-cacheable/core": "^14.1.0",
"@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
3 changes: 2 additions & 1 deletion packages/fxa-auth-server/test/oauth/routes/jwks.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

const path = require('path');
const { assert } = require('chai');
const proxyquire = require('proxyquire');
const mocks = require('../../lib/mocks');
const keys = require('../../../lib/oauth/keys');

const routeModulePath = '../../../lib/routes/oauth/jwks';
const routeModulePath = path.join(__dirname, '../../../lib/routes/oauth/jwks');
var dependencies = mocks.require(
[{ path: '../../oauth/keys' }],
routeModulePath,
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
3 changes: 2 additions & 1 deletion packages/fxa-auth-server/test/oauth/routes/verify.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

const path = require('path');
const { assert } = require('chai');
const proxyquire = require('proxyquire');
const sinon = require('sinon');
Expand Down Expand Up @@ -50,7 +51,7 @@ describe('/verify POST', () => {
};

route = proxyquire(
'../../../lib/routes/oauth/verify',
path.join(__dirname, '../../../lib/routes/oauth/verify'),
dependencies
)({ log: mocks.log, glean: mocks.glean });
});
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
Loading

0 comments on commit d3be5f3

Please sign in to comment.