Skip to content

Cache the results of #cloudfront_proxies and #trusted_proxies #31

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

willbryant
Copy link

These were being regenerated each call, which takes several ms.

Freeze the results of these methods and #ip_ranges to ensure they don't get accidentally modified by callers.

Stakeholder Overview (learn more)

Fixes a performance regression we suffered in production trying to filter out Cloudfront IPs from our request IP logging.

Risk Estimate (learn more)

  • ✅ Negligible risk!

Changes

Caches and freezes the results of the trusted_proxies and cloudfront_proxies methods.

Updated Dependencies
  • None

These were being regenerated each call, which takes several ms.

Freeze the results of these methods and #ip_ranges to ensure they don't get accidentally modified by callers. railtie was doing this, inadvertently, so use normal array addition there.
@willbryant willbryant force-pushed the cache_trusted_proxies branch from 4cfc3da to 3cf05b9 Compare February 24, 2025 01:32
Copy link

github-actions bot commented Apr 1, 2025

This PR is open and inactive for 30 days. Merging PRs open after a long time is error-prone. Please proceed to merging or make a comment to keep it open. You can also prevent PRs from being tagged stale or closed with 'keep-open' tag. If there is no activity in 90 days, this PR will be closed

@github-actions github-actions bot added the stale label Apr 1, 2025
@drinks drinks added keep-open and removed stale labels Apr 14, 2025
@drinks drinks self-requested a review April 14, 2025 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants