Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

bug(admin-server): Broken logs #18336

Merged
merged 1 commit into from
Feb 11, 2025
Merged

bug(admin-server): Broken logs #18336

merged 1 commit into from
Feb 11, 2025

Conversation

dschom
Copy link
Contributor

@dschom dschom commented Feb 7, 2025

Because

  • Some analytics were lost
  • A previous refactor broke logging

This pull request

  • Restores mzlog.service.ts to its previous glory
  • Fixes wonky injection
  • Fixes circular ref in admin-panel gaurds that would sometimes cause esbuild error
  • Fixes project.json in libs/shared/sentry to avoid esbuild error

Issue that this pull request solves

Closes: FXA-10920

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).

Screenshots (Optional)

Working Example from Train-294
28|admin-server | 2025-02-06T15:30:38: {"Timestamp":1738884638766000000,"Logger":"fxa-user-admin-server","Type":"default.admin-panel-events","Severity":6,"Pid":70121,"EnvVersion":"2.0","Fields":{"event":"account-search","search-type":"email","auto-completed":false}}

Broken Example from train-305. Note "Fields": { "0": "{...}"}. This won't ingest into our logging tables properly.
28|admin-server | 2025-02-06T15:06:20: {"Timestamp":1738883180094000000,"Logger":"fxa-user-admin-server","Type":"EventLoggingService.","Severity":6,"Pid":40424,"EnvVersion":"2.0","Fields":{"0":"{\"event\":\"account-search\",\"search-type\":\"email\",\"auto-completed\":false}","message":"admin-panel-events"}}

With fix in this PR:
28|admin-server | 2025-02-06T16:13:29: {"Timestamp":1738887209919000000,"Logger":"fxa-user-admin-server","Type":"EventLoggingService.admin-panel-events","Severity":6,"Pid":37694,"EnvVersion":"2.0","Fields":{"event":"account-search","search-type":"email","auto-completed":true}}

Other information (Optional)

The build error fixes cropped up of all a sudden. Maybe these should be a separate PR? Pulled my hair out trying to figure out why this was happening... I also added a few follows based on this PR to the logging cleanup epic. Basically our logger use is getting more and more inconsistent. We need a full pass to clean this up to remove the partial refactors and ensuing code duplication. This was started in the FXA-10970 branch, ended up bailing on the attempt, because the size of the commit was getting out of hand, hence the FXA-10970.2 branch name.

@dschom dschom requested a review from a team as a code owner February 7, 2025 15:40
Copy link
Contributor

@vbudhram vbudhram left a comment

Choose a reason for hiding this comment

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

@dschom Changes seem reasonable 👍🏽

Because:
- Some analytics were lost
- A refactor broke logging

This commit:
- Restores mzlog.service.ts to its previous glory
- Fixes wonky injection
- Fixes circular ref in admin-panel gaurds that would sometimes cause esbuild error
- Fixes project.json in libs/shared/sentry to avoid esbuild error
@dschom dschom merged commit c8ca8be into main Feb 11, 2025
23 checks passed
@dschom dschom deleted the FXA-10970.2 branch February 11, 2025 01:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants