Skip to content

Commit 30fca01

Browse files
committed
chore(admin): Delete unused code from fxa-shared
Because: - Cleaning up code duplication, in favor of code ported to libs This Commit: - Deletes unreferenced code
1 parent 0672fee commit 30fca01

38 files changed

Lines changed: 82 additions & 3076 deletions

libs/shared/monitoring/src/index.ts

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,68 @@ export type MonitoringConfig = {
1515

1616
let initialized = false;
1717

18-
// IMPORTANT! This initialization function must be called first thing when a server starts. If it's called after server
19-
// frameworks initialized instrumentation might not work properly.
18+
/**
19+
* IMPORTANT! This initialization function must be called first thing when a server starts. If it's called after server
20+
* frameworks initialized instrumentation might not work properly.
21+
*/
22+
2023
/**
2124
* Initializes modules related to error monitoring, performance monitoring, and tracing.
2225
* @param opts - Initialization options. See underlying implementations for more details.
2326
*/
2427
export function initMonitoring(opts: MonitoringConfig) {
25-
const { log, config } = opts;
28+
const { log: logger, config } = opts;
29+
const log = logger || console;
30+
2631
if (initialized) {
27-
opts.log?.warn('monitoring', 'Monitoring can only be initialized once');
32+
log.warn('monitoring', 'Monitoring can only be initialized once');
2833
return;
2934
}
3035
initialized = true;
3136

37+
/**
38+
* IMPORTANT!
39+
*
40+
* Sentry also uses OTEL under the hood. Which means:
41+
* - Mind the order of initialization. Otel should be first, if configured!
42+
* - Mind the config.tracing.sentry.enabled flag
43+
* - Mind the config.sentry.skipOpenTelemetrySetup flag
44+
*
45+
* If the order or flags aren't correct the following could happen:
46+
* - Traces disappear
47+
* - Sentry errors don't get recorded
48+
* - Sentry context bleeds between requests (ie breadcrumbs, stack traces, etc. seem off)
49+
*/
50+
51+
let nodeTracingInitialized = false;
3252
if (config.tracing) {
33-
initTracing(config.tracing, log || console);
53+
// Important! Sentry also uses OTEL under the hood. Flip this flag so the two can co-exist!
54+
// If you start seeing funny stack traces, or cross pollinating bread crumbs, something went
55+
// sideways here.
56+
if (config.sentry?.dsn) {
57+
config.tracing.sentry = { enabled: true };
58+
}
59+
60+
log.info('monitoring', {
61+
msg: `Initialize Tracing with: ${JSON.stringify(config.tracing)}`,
62+
});
63+
nodeTracingInitialized = !!initTracing(config.tracing, log);
3464
}
65+
66+
// Important! Order matters here. If otel is configured, this shoudl be done after OTEL initialization!
3567
if (config && config.sentry) {
36-
initSentry(config, log || console);
68+
if (nodeTracingInitialized) {
69+
config.sentry.skipOpenTelemetrySetup = true;
70+
}
71+
72+
log.info('monitoring', {
73+
msg: `Initializing Sentry: ${JSON.stringify({
74+
env: config.sentry?.env,
75+
clientName: config.sentry.clientName,
76+
serverName: config.sentry.serverName,
77+
hasDsn: !!config.sentry?.dsn,
78+
})}`,
79+
});
80+
initSentry(config, log);
3781
}
3882
}

libs/shared/sentry-utils/src/lib/models/sentry-config-opts.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ export type SentryConfigOpts = {
3030
/** The tracing sample rate. Setting this above 0 will aso result in performance metrics being captured. */
3131
tracesSampleRate?: number;
3232

33+
/** Flag indicating that otel should not be setup. */
34+
skipOpenTelemetrySetup?: boolean;
35+
3336
/** A function that determines the sample rate for a given event. Setting this will override tracesSampleRate. */
3437
tracesSampler?: (context: { name?: string }) => number;
3538

packages/fxa-admin-panel/server/jest.config.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,15 @@ module.exports = {
2020
moduleNameMapper: pathsToModuleNameMapper(compilerOptions.paths, {
2121
prefix: '<rootDir>/../../../',
2222
}),
23+
moduleNameMapper: {
24+
...pathsToModuleNameMapper(compilerOptions.paths, {
25+
prefix: '<rootDir>/../../../',
26+
}),
27+
'^@opentelemetry/otlp-exporter-base/node-http$':
28+
'@opentelemetry/otlp-exporter-base/build/src/index-node-http.js',
29+
'^@opentelemetry/otlp-exporter-base/browser-http$':
30+
'@opentelemetry/otlp-exporter-base/build/src/index-browser-http.js',
31+
},
2332
coveragePathIgnorePatterns: ['<rootDir>'],
2433
coverageThreshold: {
2534
global: {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const {
2121
ProductConfigurationManager,
2222
StrapiClient,
2323
} = require('@fxa/shared/cms');
24-
const { TracingProvider } = require('@fxa/shared/otel');
24+
const { getCurrent: getCurrentTracingProvider } = require('@fxa/shared/otel');
2525

2626
const { AppError: error } = require('@fxa/accounts/errors');
2727
const { JWTool } = require('@fxa/vendored/jwtool');
@@ -87,7 +87,7 @@ async function run(config) {
8787
const log = require('../lib/log')({
8888
...config.log,
8989
statsd,
90-
nodeTracer: TracingProvider.getCurrent(),
90+
nodeTracer: getCurrentTracingProvider(),
9191
});
9292
Container.set(AuthLogger, log);
9393

packages/fxa-auth-server/jest.config.js

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
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 { pathsToModuleNameMapper } = require('ts-jest');
6+
const { compilerOptions } = require('../../tsconfig.base.json');
7+
58
module.exports = {
69
preset: 'ts-jest',
710
testEnvironment: 'node',
@@ -13,11 +16,13 @@ module.exports = {
1316
},
1417
transformIgnorePatterns: ['/node_modules/(?!(@fxa|fxa-shared)/)'],
1518
moduleNameMapper: {
16-
'^@fxa/shared/(.*)$': '<rootDir>/../../libs/shared/$1/src',
17-
'^@fxa/accounts/(.*)$': '<rootDir>/../../libs/accounts/$1/src',
18-
'^@fxa/payments/(.*)$': '<rootDir>/../../libs/payments/$1/src',
19-
'^@fxa/profile/(.*)$': '<rootDir>/../../libs/profile/$1/src',
20-
'^fxa-shared/(.*)$': '<rootDir>/../fxa-shared/$1',
19+
...pathsToModuleNameMapper(compilerOptions.paths, {
20+
prefix: '<rootDir>/../../../',
21+
}),
22+
'^@opentelemetry/otlp-exporter-base/node-http$':
23+
'@opentelemetry/otlp-exporter-base/build/src/index-node-http.js',
24+
'^@opentelemetry/otlp-exporter-base/browser-http$':
25+
'@opentelemetry/otlp-exporter-base/build/src/index-browser-http.js',
2126
},
2227
testTimeout: 10000,
2328
clearMocks: true,

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,11 @@ Lug.prototype.getTraceId = function (data) {
6868

6969
if (this.nodeTracer) {
7070
try {
71-
result = { traceId: this.nodeTracer.getTraceId() };
72-
} catch {
71+
result = { traceId: this.nodeTracer.getTraceParentId() };
72+
} catch (err) {
7373
// don't let a tracing issue break logging
74-
this.debug('log', { msg: 'could not get trace id' });
74+
console.warn('Could not get trace id');
75+
console.error(err);
7576
}
7677
}
7778
return result;

packages/fxa-auth-server/test/local/server.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const customs = mocks.mockCustoms();
1818
const sandbox = sinon.createSandbox();
1919
const mockReportValidationError = sandbox.stub();
2020
const server = proxyquire(`../../lib/server`, {
21-
'fxa-shared/sentry/report-validation-error': {
21+
'@fxa/shared/sentry-node': {
2222
reportValidationError: mockReportValidationError,
2323
},
2424
});

packages/fxa-shared/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,15 @@ Or by building up the new set in place:
9393

9494
This utility allows for easy configuration of tracing. To use this in a service:
9595

96-
- Add the config chunk in fxa-shared/tracing/config to your packages's config.
96+
- Add the config chunk in @fxa/shared/otel to your packages's config.
9797

9898
- Then initialize as follows. This invocation should happen as early as possible in the service's or app's lifecycle. Ideally it's
9999
the first thing done, even before importing other modules.
100100

101101
```
102102
// For services
103103
const config = require('../config');
104-
require('fxa-shared/tracing/node-tracing').initTracing(config.get('tracing'));
104+
require('@fxa/shared/otel').initTracing(config.get('tracing'));
105105
```
106106

107107
To see traces on your local system, set the following environment variables:
@@ -122,7 +122,7 @@ TRACING_OTEL_COLLECTOR_JAEGER_ENABLED=true
122122
123123
```
124124

125-
The default config for tracing found at fxa-shared/tracing/config.ts will pick up these variables and result in traces showing up in Jaeger which runs locally at `localhost:16686`.
125+
The default config for tracing found at @fxa/shared/otel will pick up these variables and result in traces showing up in Jaeger which runs locally at `localhost:16686`.
126126

127127
It's important to note that sentry also supports tracing integration. So we typically let a call to 'initSentry', a function located in the sentry/node.ts module do the work of initializing tracing.
128128

packages/fxa-shared/monitoring/index.ts

Lines changed: 0 additions & 81 deletions
This file was deleted.

packages/fxa-shared/package.json

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,6 @@
121121
"import": "./dist/esm/packages/fxa-shared/log/*.js",
122122
"require": "./dist/cjs/packages/fxa-shared/log/*.js"
123123
},
124-
"./monitoring": {
125-
"import": "./dist/esm/packages/fxa-shared/monitoring/index.js",
126-
"require": "./dist/cjs/packages/fxa-shared/monitoring/index.js"
127-
},
128124
"./metrics/*": {
129125
"import": "./dist/esm/packages/fxa-shared/metrics/*.js",
130126
"require": "./dist/cjs/packages/fxa-shared/metrics/*.js"
@@ -169,14 +165,6 @@
169165
"import": "./dist/esm/packages/fxa-shared/scripts/*.js",
170166
"require": "./dist/cjs/packages/fxa-shared/scripts/*.js"
171167
},
172-
"./sentry": {
173-
"import": "./dist/esm/packages/fxa-shared/sentry/index.js",
174-
"require": "./dist/cjs/packages/fxa-shared/sentry/index.js"
175-
},
176-
"./sentry/*": {
177-
"import": "./dist/esm/packages/fxa-shared/sentry/*.js",
178-
"require": "./dist/cjs/packages/fxa-shared/sentry/*.js"
179-
},
180168
"./speed-trap/*": {
181169
"import": "./dist/esm/packages/fxa-shared/speed-trap/*.js",
182170
"require": "./dist/cjs/packages/fxa-shared/speed-trap/*.js"
@@ -185,10 +173,6 @@
185173
"import": "./dist/esm/packages/fxa-shared/subscriptions/*.js",
186174
"require": "./dist/cjs/packages/fxa-shared/subscriptions/*.js"
187175
},
188-
"./tracing/*": {
189-
"import": "./dist/esm/packages/fxa-shared/tracing/*.js",
190-
"require": "./dist/cjs/packages/fxa-shared/tracing/*.js"
191-
},
192176
"./test/db/models/auth/*": {
193177
"import": "./dist/esm/packages/fxa-shared/test/db/models/auth/*.js",
194178
"require": "./dist/cjs/packages/fxa-shared/test/db/models/auth/*.js"

0 commit comments

Comments
 (0)