Skip to content

Conversation

@YuHima03
Copy link
Contributor

  • リクエストレート調査用にミドルウェアを追加しました.
    • レート制限は行わず, ログだけ残すようになっています.
    • エンドポイント別ではなく, 全体でのレートを計測するようになっています.
  • リクエストレートが 20req/s を超えるとログを記録するようになっています.
    • 厳密には 40req/s まで許容する場合がありますが, 平均すると閾値は 20req/s になっています.
  • レート計算用に, キュー的なものを用意しています.
    • 最大で ユーザー数 * 12 * sizeof(int) バイトの領域を確保するようになっています. (24KiBくらい?)

@YuHima03 YuHima03 requested a review from Copilot April 25, 2025 13:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces a new middleware for logging request rates without enforcing rate limiting, and it leverages a custom in-memory queue to calculate the request rate across the application.

  • Added RateLimiterWithLogging middleware to log high request rates.
  • Updated router/v3/router.go to integrate the new middleware on the API group.
  • Implemented an in-memory access log for rate tracking in router/middlewares/rate_limit.go.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
router/v3/router.go Integrated new RateLimiterWithLogging middleware into API routes.
router/middlewares/rate_limit.go Added middleware implementation for logging request rates.

@YuHima03 YuHima03 marked this pull request as ready for review April 29, 2025 09:18
}

// RateLimiterWithLogging リクエストレートを記録するミドルウェア (レートが閾値を上回った場合にログを残す)
func RateLimiterWithLogging(rate rate.Limit, burst int, logger *zap.Logger) echo.MiddlewareFunc {
Copy link
Member

Choose a reason for hiding this comment

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

これ、レートリミットをしてそうに見えちゃうので(実際はしてないので混乱の元になる)命名をちょっと練ってほしいです!

@Takeno-hito
Copy link
Member

  • レートリミットの閾値をどうするかは先にある程度目安を立ててから試してほしいです!
  • (独自実装が多めになってて、ここはライブラリを頼れないのか検討の余地はありそう)

@Takeno-hito Takeno-hito requested a review from ramdos0207 May 7, 2025 14:26
mutex.Lock()
l := accessLogs.Get(ip)
l.Add(ok)
if !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

逐一ログ出力するとうるさそうなので、ログ出したという事実も手元に持っておいてアクセスログがクリアされるまでは再度ログを出さない、みたいなことをするのもアリかも?(必須ではない)

@YuHima03
Copy link
Contributor Author

レートはアクセスログから統計取ってくるのが良い気がするので一旦draftにします 🙇

@YuHima03 YuHima03 marked this pull request as draft May 16, 2025 08:18
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.

4 participants