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

[BE] refactor: TooManyRequest 로깅에 사용되는 IP 추출 로직 변경 #944

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

nayonsoso
Copy link
Contributor


🚀 어떤 기능을 구현했나요 ?

  • 현재는 로드벨런서의 IP 를 remoteAddress 로 로깅하고 있습니다. 이는 의미가 없습니다😅
  • 따라서 '실제 요청 IP'를 추출할 수 있도록 로직을 수정했습니다.

🔥 어떻게 해결했나요 ?

  • IP 를 추출하는 우리의 코드가 프록서 서버에 종속적이면 안된다고 생각했습니다.
  • 그래서 aws LB 를 사용하는 경우의 헤더(X-FORWARDED-FOR)와 Nginx 를 사용할 때의 헤더(X-REAL-IP)를 대상으로 추출하고,
  • 이들이 없다면 그제야 HttpRequest 의 remoteAddr를 추출하도록 작성했습니다.

📝 어떤 부분에 집중해서 리뷰해야 할까요?

  • 프록시 종류에 종속적이지 않게 짰는데 과하진 않겠죠?

📚 참고 자료, 할 말

Copy link

github-actions bot commented Oct 25, 2024

Test Results

154 tests  ±0   151 ✅ ±0   4s ⏱️ ±0s
 59 suites ±0     3 💤 ±0 
 59 files   ±0     0 ❌ ±0 

Results for commit bb859c8. ± Comparison against base commit 11a2b05.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@donghoony donghoony left a comment

Choose a reason for hiding this comment

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

한가지만 확인해주세요~!

@nayonsoso nayonsoso force-pushed the be/refactor/943-x-forwarded-for-header branch from 1b4ee76 to bb859c8 Compare November 11, 2024 05:23
Copy link
Contributor

@Kimprodp Kimprodp left a comment

Choose a reason for hiding this comment

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

이거 조금만 더 볼게용..

Copy link
Contributor

@Kimprodp Kimprodp left a comment

Choose a reason for hiding this comment

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

우리가 이제 nginx를 사용할텐데 웹 서버의 기능을 단순히 https 를 위해서 사용하기 보다 진짜 웹 서버의 기능을 활용했으면 좋겠어서 드는 의견을 적어봅니다요.

본론부터 말하면 "nginx 에서 제공하는 요청 제한을 사용하면 애플리케이션에서 트래픽 제어를 담당하지 않아도 된다." 입니다.

https://blog.nginx.org/blog/rate-limiting-nginx
https://findstar.pe.kr/2018/06/24/nginx-rate-limiting/
https://letsmakemyselfprogrammer.tistory.com/99

프록시로써 트래픽 제어 기능을 제공해 주는데 사용하지 않고 모든 요청을 애플리케이션에서 받는다는 것이 비효율적이라는 생각이 듭니다.

만약 요청 제한 로직이 복잡하고, 여러 세부 조건(로그인 등)들을 따져야 한다면 애플리케이션에서 하는 게 맞다고 생각해요. 하지만 지금 제한 기준은 단순히 IP, user-agent로만 되어있어서 복잡하지 않기 때문에 그냥 웹 서버를 통해서 충분히 처리 가능하다 생각합니다.

이렇게 유효하지 않은 요청을 프록시 선에서 처리해 주면 애플리케이션의 부담도 줄일 수 있어요. nginx으ㅣ 기능으로 완벽히 대체가 가능하다면 트래픽 제어 역할을 nginx에서 하는 의견에 대해 어떻게 생각하시나요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
4 participants