Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Revert "API-26059 Log durations of some sidekiq jobs (#12474)" #12840

Merged
merged 0 commits into from
May 31, 2023

Conversation

caseywilliams
Copy link
Contributor

Summary

Related issue(s)

  • This code is no longer needed now that DataDog access has been granted to the appeals team
  • It looks like there is some issue with require statements when running sidekiq jobs locally - see this support thread. We could fix the require statement, but it seems like just removing this code would be most efficient.

Testing done

What areas of the site does it impact?

Removes sidekiq middleware

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

@caseywilliams caseywilliams added Lighthouse lighthouse appeals Lighthouse API appeals banana-peels Lighthouse Banana Peels Team labels May 30, 2023
@caseywilliams caseywilliams requested review from a team as code owners May 30, 2023 23:20
@caseywilliams caseywilliams force-pushed the remove-sidekiq-logging branch from 4b3a7ae to 2639ef5 Compare May 30, 2023 23:23
@va-vsp-bot va-vsp-bot requested a deployment to remove-sidekiq-logging/main/main May 30, 2023 23:24 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to remove-sidekiq-logging/main/main May 30, 2023 23:29 Inactive
Copy link
Contributor

@kristen-brown kristen-brown left a comment

Choose a reason for hiding this comment

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

I compared it with the original PR that added the middleware, and it looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appeals Lighthouse API appeals banana-peels Lighthouse Banana Peels Team console-services-review Lighthouse lighthouse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants