Skip to content

Commit d2ca2a0

Browse files
committed
refactor(shared): remove the fxa-shared/monitoring wrapper
## Because - `fxa-shared/monitoring` is a 30-line wrapper that only calls `initTracing` then `initSentry`. - `fxa-admin-server` already dropped the wrapper and calls both directly, so the target shape is settled. - Every consumer already owns a small `monitoring` module, so the wrapper buys nothing. ## This pull request - Replaces `initMonitoring` with direct `initTracing` then `initSentry` calls in `fxa-admin-panel`, `fxa-auth-server`, `fxa-customs-server` and `fxa-profile-server`. The `tracing.sentry.enabled` and `sentry.skipOpenTelemetrySetup` cross-wiring the wrapper did is kept, in the same order. - Deletes `packages/fxa-shared/monitoring` and its `package.json` export. - Repoints `fxa-auth-server`, `fxa-admin-panel` and `fxa-event-broker` at `@fxa/shared/otel` for tracing. Boot order in `key_server.js` does not move; only the specifier changes. - Adds `@fxa/shared/log`, `@fxa/shared/otel` and `@fxa/shared/sentry-utils` to `fxa-event-broker/tsconfig.build.json`, whose `paths` block overrides the base one. - Rewrites `fxa-auth-server/lib/monitoring.spec.ts` to assert the two direct calls, their order and the flag cross-wiring. Sentry stays where it is on purpose. `fxa-shared/sentry/*` has about 30 importers and is a separate migration, so every `fxa-shared/sentry/...` import here is byte-identical to before. ## Issue that this pull request solves Closes: https://mozilla-hub.atlassian.net/browse/FXA-10645 Partly addresses https://mozilla-hub.atlassian.net/browse/FXA-10646. See "Other information" for why `fxa-shared/tracing` cannot go yet.
1 parent eb218a3 commit d2ca2a0

15 files changed

Lines changed: 118 additions & 148 deletions

File tree

packages/fxa-admin-panel/server/config/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import convict from 'convict';
66
import fs from 'fs';
77
import path from 'path';
88
import { GuardConfig } from '@fxa/shared/guards';
9-
import { tracingConfig } from 'fxa-shared/tracing/config';
9+
import { tracingConfig } from '@fxa/shared/otel';
1010

1111
convict.addFormats(require('convict-format-with-moment'));
1212
convict.addFormats(require('convict-format-with-validator'));

packages/fxa-admin-panel/server/lib/monitoring.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,24 @@
44

55
import Config from '../config';
66
import mozLog from 'mozlog';
7-
import { initMonitoring } from 'fxa-shared/monitoring';
7+
import { TracingOpts, initTracing } from '@fxa/shared/otel';
8+
import { InitSentryOpts, initSentry } from 'fxa-shared/sentry/node';
89
import { version } from '../../package.json';
910

10-
const config = Config.getProperties();
11-
const log = mozLog(config.logging)(config.logging.app);
12-
initMonitoring({
13-
log,
14-
config: {
15-
...config,
16-
release: version,
17-
},
18-
});
11+
const properties = Config.getProperties();
12+
const log = mozLog(properties.logging)(properties.logging.app);
13+
14+
const config: InitSentryOpts & { tracing: TracingOpts } = {
15+
...properties,
16+
release: version,
17+
};
18+
19+
// Sentry also uses OTEL under the hood. Tracing must start first, and each side
20+
// must know about the other, or traces and breadcrumbs bleed between requests.
21+
if (config.sentry?.dsn) {
22+
config.tracing.sentry = { enabled: true };
23+
}
24+
if (initTracing(config.tracing, log) && config.sentry) {
25+
config.sentry.skipOpenTelemetrySetup = true;
26+
}
27+
initSentry(config, log);

packages/fxa-auth-server/bin/key_server.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const {
3030
ProductConfigurationManager,
3131
StrapiClient,
3232
} = require('@fxa/shared/cms');
33-
const TracingProvider = require('fxa-shared/tracing/node-tracing');
33+
const TracingProvider = require('@fxa/shared/otel');
3434

3535
const { createNoopStatsd } = require('../lib/noop-statsd');
3636
const { AppError: error } = require('@fxa/accounts/errors');

packages/fxa-auth-server/config/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import convict from 'convict';
55
import fs from 'fs';
66
import { makeRedisConfig } from 'fxa-shared/db/config';
7-
import { tracingConfig } from 'fxa-shared/tracing/config';
7+
import { tracingConfig } from '@fxa/shared/otel';
88
import { CloudTasksConvictConfigFactory } from '@fxa/shared/cloud-tasks';
99
import path from 'path';
1010
import url from 'url';

packages/fxa-auth-server/lib/monitoring.js

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,27 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
const { initMonitoring } = require('fxa-shared/monitoring');
5+
const { initTracing } = require('@fxa/shared/otel');
6+
const { initSentry } = require('fxa-shared/sentry/node');
67
const Sentry = require('@sentry/node');
7-
const { config } = require('../config');
8-
const logger = require('./log')(
9-
config.getProperties().log.level,
10-
'configure-sentry'
11-
);
8+
const { config: convictConfig } = require('../config');
9+
const config = convictConfig.getProperties();
1210
const { version } = require('../package.json');
1311
const { ignoreErrors } = require('@fxa/accounts/errors');
1412

15-
/**
16-
* Initialize sentry & otel
17-
*/
18-
initMonitoring({
19-
logger,
20-
config: {
21-
...config.getProperties(),
13+
const logger = require('./log')(config.log.level, 'configure-sentry');
14+
15+
// Sentry also uses OTEL under the hood. Tracing must start first, and each side
16+
// must know about the other, or traces and breadcrumbs bleed between requests.
17+
if (config.sentry?.dsn) {
18+
config.tracing.sentry = { enabled: true };
19+
}
20+
if (initTracing(config.tracing, logger)) {
21+
config.sentry.skipOpenTelemetrySetup = true;
22+
}
23+
initSentry(
24+
{
25+
...config,
2226
release: version,
2327
eventFilters: [filterSentryEvent],
2428
integrations: [
@@ -27,7 +31,8 @@ initMonitoring({
2731
Sentry.linkedErrorsIntegration({ key: 'jse_cause' }),
2832
],
2933
},
30-
});
34+
logger
35+
);
3136

3237
/**
3338
* Filter a sentry event for PII in addition to the default filters.

packages/fxa-auth-server/lib/monitoring.spec.ts

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
const mockInitMonitoring = jest.fn();
5+
const mockInitTracing = jest.fn().mockReturnValue({});
6+
const mockInitSentry = jest.fn();
67
const mockIgnoreErrors = jest.fn();
78
const mockLogger = {
89
info: jest.fn(),
@@ -17,14 +18,18 @@ const mockLinkedErrorsIntegration = jest
1718
.fn()
1819
.mockReturnValue({ name: 'LinkedErrors' });
1920

20-
jest.mock('fxa-shared/monitoring', () => ({
21-
initMonitoring: mockInitMonitoring,
21+
jest.mock('@fxa/shared/otel', () => ({
22+
initTracing: mockInitTracing,
23+
}));
24+
jest.mock('fxa-shared/sentry/node', () => ({
25+
initSentry: mockInitSentry,
2226
}));
2327
jest.mock('../config', () => ({
2428
config: {
2529
getProperties: jest.fn().mockReturnValue({
2630
log: { level: 'debug' },
2731
sentry: { dsn: 'https://test@sentry.io/123' },
32+
tracing: { serviceName: 'fxa-auth-server' },
2833
}),
2934
},
3035
}));
@@ -40,20 +45,23 @@ jest.mock('@sentry/node', () => ({
4045
linkedErrorsIntegration: mockLinkedErrorsIntegration,
4146
}));
4247

43-
// Importing the module triggers the top-level initMonitoring() call once.
48+
// Importing the module triggers the top-level init calls once.
4449
import './monitoring';
4550

4651
// Snapshot one-shot call args before `clearMocks: true` wipes them
4752
// between tests; the module-load side-effect can't be replayed.
48-
const initMonitoringCallCount = mockInitMonitoring.mock.calls.length;
49-
const initMonitoringArg = mockInitMonitoring.mock.calls[0]?.[0];
53+
const initTracingCalls = mockInitTracing.mock.calls.map((c) => [...c]);
54+
const initSentryCalls = mockInitSentry.mock.calls.map((c) => [...c]);
55+
const initSentryConfig = initSentryCalls[0]?.[0];
56+
const tracingCallOrder = mockInitTracing.mock.invocationCallOrder[0];
57+
const sentryCallOrder = mockInitSentry.mock.invocationCallOrder[0];
5058
const hapiIntegrationCallCount = mockHapiIntegration.mock.calls.length;
5159
const linkedErrorsIntegrationCalls = mockLinkedErrorsIntegration.mock.calls.map(
5260
(c) => c[0]
5361
);
5462
const logCalls = mockLog.mock.calls.map((c) => [...c]);
5563
const filterSentryEvent: (event: any, hint?: any) => any =
56-
initMonitoringArg.config.eventFilters[0];
64+
initSentryConfig.eventFilters[0];
5765

5866
describe('monitoring', () => {
5967
beforeEach(() => {
@@ -64,24 +72,37 @@ describe('monitoring', () => {
6472
jest.restoreAllMocks();
6573
});
6674

67-
it('calls initMonitoring on module load', () => {
68-
expect(initMonitoringCallCount).toBe(1);
75+
it('initializes tracing before sentry on module load', () => {
76+
expect(initTracingCalls).toHaveLength(1);
77+
expect(initSentryCalls).toHaveLength(1);
78+
expect(tracingCallOrder).toBeLessThan(sentryCallOrder);
6979

70-
// Logger is the return value of the mocked log(level, name)
71-
expect(initMonitoringArg.logger).toBe(mockLogger);
80+
// Both get the return value of the mocked log(level, name)
81+
expect(initTracingCalls[0][1]).toBe(mockLogger);
82+
expect(initSentryCalls[0][1]).toBe(mockLogger);
7283
expect(logCalls).toContainEqual(['debug', 'configure-sentry']);
84+
});
85+
86+
it('cross-wires the sentry and otel flags', () => {
87+
// A sentry dsn tells otel that sentry is present...
88+
expect(initTracingCalls[0][0]).toEqual({
89+
serviceName: 'fxa-auth-server',
90+
sentry: { enabled: true },
91+
});
92+
// ...and a live tracer tells sentry to skip its own otel setup.
93+
expect(initSentryConfig.sentry.skipOpenTelemetrySetup).toBe(true);
94+
});
7395

74-
// Config includes spread properties, release, eventFilters, integrations
75-
expect(initMonitoringArg.config).toEqual(
96+
it('passes the spread config, release, eventFilters and integrations', () => {
97+
expect(initSentryConfig).toEqual(
7698
expect.objectContaining({
7799
log: { level: 'debug' },
78-
sentry: { dsn: 'https://test@sentry.io/123' },
79100
release: '1.234.0',
80101
})
81102
);
82-
expect(initMonitoringArg.config.eventFilters).toHaveLength(1);
83-
expect(typeof initMonitoringArg.config.eventFilters[0]).toBe('function');
84-
expect(initMonitoringArg.config.integrations).toHaveLength(2);
103+
expect(initSentryConfig.eventFilters).toHaveLength(1);
104+
expect(typeof initSentryConfig.eventFilters[0]).toBe('function');
105+
expect(initSentryConfig.integrations).toHaveLength(2);
85106
});
86107

87108
it('passes Sentry integrations with correct configuration', () => {

packages/fxa-auth-server/lib/routes/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
'use strict';
66

77
const url = require('url');
8-
const tracing = require('fxa-shared/tracing/node-tracing');
8+
const tracing = require('@fxa/shared/otel');
99

1010
module.exports = function (
1111
log,

packages/fxa-customs-server/lib/monitoring.js

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,26 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
const Sentry = require('@sentry/node');
6-
const { initMonitoring } = require('fxa-shared/monitoring');
6+
const { initTracing } = require('fxa-shared/tracing/node-tracing');
7+
const { initSentry } = require('fxa-shared/sentry/node');
8+
const { version } = require('../package.json');
9+
710
const config = require('./config').getProperties();
811
const log = require('./log')(config.log.level, 'configure-sentry');
9-
const { version } = require('../package.json');
1012

11-
initMonitoring({
12-
log,
13-
config: {
13+
// Sentry also uses OTEL under the hood. Tracing must start first, and each side
14+
// must know about the other, or traces and breadcrumbs bleed between requests.
15+
if (config.sentry?.dsn) {
16+
config.tracing.sentry = { enabled: true };
17+
}
18+
if (initTracing(config.tracing, log)) {
19+
config.sentry.skipOpenTelemetrySetup = true;
20+
}
21+
initSentry(
22+
{
1423
...config,
1524
release: version,
1625
integrations: [Sentry.linkedErrorsIntegration({ key: 'jse_cause' })],
1726
},
18-
});
27+
log
28+
);

packages/fxa-event-broker/src/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import convict from 'convict';
66
import fs from 'fs';
77
import path from 'path';
8-
import { tracingConfig } from 'fxa-shared/tracing/config';
8+
import { tracingConfig } from '@fxa/shared/otel';
99

1010
const FIVE_MINUTES = 60 * 5;
1111

packages/fxa-event-broker/src/main.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { ConfigService } from '@nestjs/config';
1313
import { NestFactory } from '@nestjs/core';
1414
import mozLog from 'mozlog';
1515

16-
import { initTracing } from 'fxa-shared/tracing/node-tracing';
16+
import { initTracing } from '@fxa/shared/otel';
1717

1818
import { AppModule } from './app.module';
1919
import Config, { AppConfig } from './config';

0 commit comments

Comments
 (0)