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

FFS-2175: Removing Skylight from the project #402

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

GeorgeCodes19
Copy link
Contributor

@GeorgeCodes19 GeorgeCodes19 commented Dec 19, 2024

Ticket

Relates to FFS-2175.

Changes

This PR is for clean up efforts. We are keeping MiniProfiler. I didn't see any specific instructions about placing mini profiler at the bottom of the Gemfile. It seems that requiring it below the database related Gems i.e. gem "pg", "~> 1.1" is the only requirement for accurate profiling. I verified this by leaving the Gemfile declaration where it currently is and at the end of the Gemfile and noticed no difference in the flamegraphs.

  • Removal of SKYLIGHT_AUTHENTICATION environment variable from ECS/Fargate via Terraform
  • Removal of Skylight from Gemfile

Context for reviewers

Originally, we installed Skylight in order to determine if we could gain additional insight into why our network requests were taking so long to resolve. In our discovery it wasn't uncommon for users to wait > 5s from request -> response. Skylight is one of the tools that we deployed to gain further insight. While it's a great alternative to New Relic and rbspy, it's a bit redundant.

Acceptance testing

  • No acceptance testing needed
    • This change will not affect the user experience (bugfix, dependency updates, etc.)
  • Acceptance testing prior to merge
    • This change can be verified visually via screenshots attached below or by sending a link to a local development environment to the acceptance tester
    • Acceptance testing should be done by design for visual changes, product for behavior/logic changes, or both for changes that impact both.
  • Acceptance testing after merge
    • This change is hard to test locally, so we'll test it in the demo environment (deployed automatically after merge.)
    • Make sure to notify the team once this PR is merged so we don't inadvertently deploy the unaccepted change to production. (e.g. :alert: Deploy block! @ffs-eng I just merged PR [#123] and will be doing acceptance testing in demo - please don't deploy until I'm finished!)

@GeorgeCodes19 GeorgeCodes19 merged commit 51ba09d into main Dec 20, 2024
20 checks passed
@GeorgeCodes19 GeorgeCodes19 deleted the george/ffs-2175-remove-skylight branch December 20, 2024 00:02
YvetteWhiteUSDS added a commit that referenced this pull request Jan 3, 2025
* Added controller to add javascript on send button.

* FFS-2175: Adding DB query to /health endpoint (#401)

Co-authored-by: Tom Dooner <[email protected]>

* FFS-2175: Removing Skylight from the project (#402)

* Wire up Caseworker Invitation JS controller

Co-Authored-By: Yvette White <[email protected]>

* Create `uswds_form_with` helper to use Turbo on invitation page

Co-Authored-By: Yvette White <[email protected]>

---------

Co-authored-by: George Byers <[email protected]>
Co-authored-by: Tom Dooner <[email protected]>
Co-authored-by: Yvette White <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants