task(admin-server): Fix admin server startup times and some refactor #17938
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dschom
changed the title
dschom
force-pushed
the
FXA-10460-admin-server
branch
2 times, most recently
from
65b9e6e
to
763c096
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
dschom
force-pushed
the
FXA-10460-admin-server
branch
3 times, most recently
from
14b9341
to
47dcc5d
Compare
dschom
changed the title
Because: - We want to improve startup performance of admin-server - We determined using the @sentry/node package in the admin-server was source of slowdown - Using @sentry/nestjs made a big improvement in startup time. This Commit: - Switches to using @sentry/nestjs integration for admin-server - Refactors libs/sentry into three distinct contexts libs/sentry-nestjs, libs/sentry-node, libs/sentry-browser, and libs/sentry-utils - Extracts common functions to libs/sentry-utils so they can be reused.
dschom
force-pushed
the
FXA-10460-admin-server
branch
from
47dcc5d
to
14c64e1
Compare
4 tasks
chenba
reviewed
Oct 31, 2024
| @@ -17,7 +17,7 @@ import { DEFAULT_LOCALE, EN_GB_LOCALES } from './l10n.constants'; | |||
| * | |||
| */ | |||
| export function parseAcceptLanguage( | |||
| acceptLanguage: string, | |||
| acceptLanguage?: string, | |||
There was a problem hiding this comment.
There's a null check, but with == so I guess this is fine.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because
This pull request
Issue that this pull request solves
Closes: FXA-10402, FXA-10460, FXA-10648
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
We still need a follow ups to clean this up which involves removing
libs/sentryand removingfxa-shared/sentry, andfxa-shared/otel. Unfortunately this effort spiraled out of control due to a lack of typescript support (or quirks with existing tsconfigs) in our workspace packages. Special notes about this have been added to related tickets in the backlog under this epic. A semi broken attempt at fully porting this can be found here 😬.Also here's some notes on timing from local machine:
@sentry/node:7^ - With Sentry included - 3,566ms
@sentry/node:8^ - With Sentry included - 25,906ms
@sentry/node:8^ - With Sentry and all integrations filtered - 23,884ms
@sentry/node:8^ - With Sentry commented out entirely - 3,673ms
@sentry/nestjs:8^ - With Sentry - 8,558ms