Skip to content

refactor(shared): remove the fxa-shared/monitoring wrapper - #21054

Open
vbudhram wants to merge 1 commit into
mainfrom
fxa-10645
Open

refactor(shared): remove the fxa-shared/monitoring wrapper#21054
vbudhram wants to merge 1 commit into
mainfrom
fxa-10645

Conversation

@vbudhram

@vbudhram vbudhram commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Because

  • main.ts imports ./monitoring, which already starts tracing. Then bootstrap() calls initTracing a second time.
  • The new @fxa/shared/otel implementation logs a Tracing already initialized! warning on a repeat call. The event-broker prints that warning on every startup.

This pull request

  • Removes the second initTracing call and its import from main.ts.
  • Removes the mozlog import, which the removed call was the only user of. The package still depends on mozlog through monitoring.ts.
  • Keeps import './monitoring'; first, so tracing still starts before the framework loads.

Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-10645

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on: packages/fxa-event-broker/src/main.ts.
  • Suggested review order: read main.ts, then monitoring.ts to confirm tracing still starts there.
  • Risky or complex parts: none. The import order matters, so ./monitoring must stay first.

Screenshots (Optional)

None. This change does not touch the user interface.

Other information (Optional)

  • nx run fxa-event-broker:test-unit: 61 passed, 0 failed.
  • nx lint fxa-event-broker: exit 0.
  • The sibling service fxa-admin-server already follows this pattern. Its main.ts imports ./monitoring and never calls initTracing itself.

@vbudhram
vbudhram requested a review from a team as a code owner August 18, 2026 20:24
@vbudhram vbudhram added the auto label Aug 18, 2026
Copilot AI balanced review requested due to automatic review settings August 18, 2026 20:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes the legacy monitoring wrapper while preserving tracing-before-Sentry initialization and migrating supported services to @fxa/shared/otel.

Changes:

  • Replaces wrapper usage with direct tracing and Sentry initialization.
  • Migrates tracing imports and build paths.
  • Updates auth-server monitoring tests.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/fxa-shared/package.json Removes the monitoring export.
packages/fxa-shared/monitoring/index.ts Deletes the legacy wrapper.
packages/fxa-profile-server/lib/monitoring.js Initializes tracing and Sentry directly.
packages/fxa-event-broker/tsconfig.build.json Adds shared-library path mappings.
packages/fxa-event-broker/src/monitoring.ts Uses the shared OTEL library.
packages/fxa-event-broker/src/main.ts Migrates the tracing import.
packages/fxa-event-broker/src/config.ts Uses shared tracing configuration.
packages/fxa-customs-server/lib/monitoring.js Initializes tracing and Sentry directly.
packages/fxa-auth-server/lib/routes/index.js Migrates tracing utilities.
packages/fxa-auth-server/lib/monitoring.spec.ts Tests direct initialization and ordering.
packages/fxa-auth-server/lib/monitoring.js Replaces wrapper initialization.
packages/fxa-auth-server/config/index.ts Uses shared tracing configuration.
packages/fxa-auth-server/bin/key_server.js Migrates the tracing provider import.
packages/fxa-admin-panel/server/lib/monitoring.ts Replaces wrapper initialization.
packages/fxa-admin-panel/server/config/index.ts Uses shared tracing configuration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/fxa-event-broker/src/main.ts Outdated
import mozLog from 'mozlog';

import { initTracing } from 'fxa-shared/tracing/node-tracing';
import { initTracing } from '@fxa/shared/otel';
@vbudhram vbudhram changed the title refactor(shared): remove the fxa-shared/monitoring wrapper test(admin-panel): map the OTLP exporter subpath for jest 27 Aug 18, 2026
## Because

- `main.ts` imports `./monitoring`, which already starts tracing. Then `bootstrap()` calls `initTracing` a second time.
- The new `@fxa/shared/otel` implementation logs a `Tracing already initialized!` warning on a repeat call. The event-broker prints that warning on every startup.

## This pull request

- Removes the second `initTracing` call and its import from `main.ts`.
- Removes the `mozlog` import, which the removed call was the only user of. The package still depends on `mozlog` through `monitoring.ts`.
- Keeps `import './monitoring';` first, so tracing still starts before the framework loads.

## Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-10645
@vbudhram vbudhram changed the title test(admin-panel): map the OTLP exporter subpath for jest 27 fix(event-broker): drop the redundant initTracing call in main Aug 18, 2026
@vbudhram vbudhram changed the title fix(event-broker): drop the redundant initTracing call in main refactor(shared): remove the fxa-shared/monitoring wrapper Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants