Skip to content

[REFAC#503] rate_limit 로그에 label 필드 추가 — throttled 호스트/IP 사후 추적 가능 - #514

Merged
juhy0987 merged 1 commit into
mainfrom
refactor/#503/rate-limit-log-host
May 19, 2026
Merged

juhy0987 merged 1 commit into
mainfrom
refactor/#503/rate-limit-log-host

Conversation

@juhy0987

@juhy0987 juhy0987 commented May 19, 2026

Copy link
Copy Markdown
Member

연관 이슈


구현 내용

라이브 회차에서 관측된 rate limit reached, waiting for token 로그 137회 발화에 host/ip 정보 부재 — 어느 호스트가 throttled 인지 디버깅 출발점 없던 observability 갭 보강.

변경 사항

1) pkg//internal/ 시그니처 확장

internal/processor/fetcher/rate_limiter/token_bucket.go:

  • TokenBucketRateLimiterlabel string 필드 추가
  • NewRateLimiter(rph, burst int, label string) — 신규 인자 추가 (issue 본문 옵션 A)
  • label 은 throttle 로그의 추적 식별자 — 빈 문자열 허용

2) 3개 Debug 로그 모두에 label 필드 emit

메시지 의미
rate limit reached, waiting for token wait 시작 (waitCount==1 시)
rate limit wait completed wait 종료
rate limit wait context done ctx cancel/timeout

3) Production 호출처 갱신

internal/processor/fetcher/rate_limiter/ip_registry.go:130:

limiter := NewRateLimiter(rph, r.burst, ip)  // IP 그대로 label

같은 IP 의 모든 host 가 limiter 를 공유하므로 host 보다 IP 가 정확한 식별자.

4) 테스트 호출처 12곳 일괄 갱신

token_bucket_test.go 11곳 + ip_registry_test.go 1곳 모두 "test" label 로 갱신.

단위 테스트 4건 신규

  • TestRateLimiter_Wait_LogsLabelOnReached — wait 시작 로그에 IP label emit
  • TestRateLimiter_Wait_LogsLabelOnCompleted — wait 완료 로그에 host label emit
  • TestRateLimiter_Wait_LogsLabelOnContextCancelled — ctx cancel 로그에 label emit
  • TestRateLimiter_EmptyLabel_StillEmits — 빈 label 도 JSON 필드 부착 ("label":"")

운영 회복

라이브에서 rate=0.028 (≈ 1req/36s) 의 극단 throttle 이 발화하더라도 다음과 같이 즉시 식별 가능:

{
  "level": "debug",
  "label": "203.0.113.42",
  "rate": 0.028,
  "burst": 10,
  "wait_ms": 34935,
  "message": "rate limit reached, waiting for token"
}

CI / 머지 게이트 점검

변경 영향 범위

  • 영향 패키지/모듈:
    • internal/processor/fetcher/rate_limiter (시그니처 변경)
    • test 호출처 12곳
  • 위험도: Low — 시그니처 변경이지만 production 호출처는 1곳 (ip_registry), 그 외는 모두 test. 로그 추가만 (분기/동작 무변경).

Required Status Checks

  • 통과 확인 대상:
    • Commit Lint
    • PR Title Lint
    • Linked Issue Check
    • Format Check
    • Build
    • Test
    • Lint

롤백 계획

  • 본 PR revert 만으로 원복 — 로그 필드 추가만이라 데이터/상태 영향 없음.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Refactor

    • Enhanced rate limiting system with improved logging that includes identifiers for throttled requests, enabling better monitoring and debugging of rate limit behavior.
  • Tests

    • Added comprehensive test coverage for rate limit logging functionality.

Review Change Stack

)

- NewRateLimiter 시그니처 확장: (rph, burst, label string) — label 은 throttle 로그의 추적 식별자
- TokenBucketRateLimiter 가 label 필드 보존
- 3개 Debug 로그 모두에 label 필드 emit:
  - "rate limit reached, waiting for token"
  - "rate limit wait completed"
  - "rate limit wait context done"
- IP registry 의 단일 production 호출처 (ip_registry.go:130) 갱신 — IP 그대로 label 로 전달
- 테스트 호출처 12곳 일괄 갱신 (간단 "test" label)
- 단위 테스트 4건 신규: label emit 검증 (Reached / Completed / ContextCancelled) + EmptyLabel 도 필드 부착 검증

운영 회복: 라이브에서 'rate=0.028' 등 극단 throttle 로그가 어느 호스트/IP 인지 즉시 식별 가능. PR #509 머지 후 라이브 51분 동안 137회 throttle 발화에서 host 정보 부재로 디버깅 출발점 부재했던 문제 해소.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 19, 2026 00:36
@juhy0987 juhy0987 added the refactor Code refactoring label May 19, 2026
@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 098eece4-28aa-4a29-a40c-68b951364f2e

📥 Commits

Reviewing files that changed from the base of the PR and between e1e138c and b2c4b89.

📒 Files selected for processing (4)
  • internal/processor/fetcher/rate_limiter/ip_registry.go
  • internal/processor/fetcher/rate_limiter/token_bucket.go
  • test/internal/processor/fetcher/rate_limiter/ip_registry_test.go
  • test/internal/processor/fetcher/rate_limiter/token_bucket_test.go

📝 Walkthrough

Walkthrough

The PR adds a label field to the rate limiter to identify which IP is throttled in log output. The label is threaded through the constructor, included in all throttle-related log statements, wired into the IP registry, and verified by new test coverage.

Changes

Rate limiter label tracking for IP throttling identification

Layer / File(s) Summary
Core label infrastructure in rate limiter
internal/processor/fetcher/rate_limiter/token_bucket.go
TokenBucketRateLimiter gains a label string field, and NewRateLimiter signature is extended to accept label string and initialize the field in the constructor.
Label emission in throttle logs
internal/processor/fetcher/rate_limiter/token_bucket.go
All three throttle-related log paths—"rate limit reached, waiting for token", "rate limit wait completed", and "rate limit wait context done"—now include the label field in their emitted log output.
IP registry integration and test
internal/processor/fetcher/rate_limiter/ip_registry.go, test/internal/processor/fetcher/rate_limiter/ip_registry_test.go
The IP registry's getOrCreate method passes the IP as the label parameter when constructing new rate limiters, and the corresponding test is updated to pass the label argument.
Existing test parameter updates
test/internal/processor/fetcher/rate_limiter/token_bucket_test.go
All existing test cases are updated to pass the new label parameter when constructing rate limiters.
New tests for label logging verification
test/internal/processor/fetcher/rate_limiter/token_bucket_test.go
Four new test functions verify that the label field is present and correctly populated in throttle-related logs for reached, completed, context-cancelled, and empty-label scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • EinSofINTEREST/IssueTracker#325: Both PRs update the IP rate limiter registry's limiter-creation path in internal/processor/fetcher/rate_limiter/ip_registry.go—this PR adds an IP-based label to NewRateLimiter(...), while the retrieved PR changes how rph is chosen via a resolver when calling NewRateLimiter(...).

Suggested labels

enhancement

Poem

🐰 A label for each limiter,
To track which host grows bitter,
No more mystery logs so blind,
The IP's presence now we'll find! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.79% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title in Korean describes adding a label field to rate limit logs for host/IP traceability, which directly aligns with the main change of adding label support to TokenBucketRateLimiter and logging.
Linked Issues check ✅ Passed The PR implements all requirements from issue #503: adds label parameter to NewRateLimiter signature, stores label in TokenBucketRateLimiter, includes label in all three debug logs, passes IP as label in ip_registry.go, and adds comprehensive test coverage for label field emission.
Out of Scope Changes check ✅ Passed All changes are directly related to adding label field support to rate limit logging as specified in issue #503; no unrelated modifications to other components or functionality are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/#503/rate-limit-log-host

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a label field to the TokenBucketRateLimiter to improve log traceability, allowing for the identification of specific IPs or hosts being throttled. The NewRateLimiter constructor signature was updated to include this label, and the Wait method now includes it in debug logs. Related tests and the IP registry were updated to support this change, and new test cases were added to verify label emission. I have no feedback to provide as there were no review comments.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

이 PR은 TokenBucketRateLimiter의 throttle 관련 Debug 로그에 추적 식별자(label: host/IP 등)를 포함시켜, 운영 환경에서 어떤 대상이 rate limit에 걸렸는지 사후 추적 가능하도록 observability를 보강합니다.

Changes:

  • TokenBucketRateLimiterlabel 필드를 추가하고, NewRateLimiter(requestsPerHour, burst, label string)로 생성자 시그니처를 확장
  • throttle 관련 3개 Debug 로그에 label 필드 emit
  • 프로덕션 호출부(IPRateLimiterRegistry) 및 테스트 호출부에서 신규 시그니처로 갱신, label 로그 검증 테스트 추가

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
internal/processor/fetcher/rate_limiter/token_bucket.go limiter에 label 저장 및 throttle 로그에 label 필드 추가(생성자 시그니처 변경 포함)
internal/processor/fetcher/rate_limiter/ip_registry.go IP 기반 registry에서 limiter 생성 시 IP를 label로 전달
test/internal/processor/fetcher/rate_limiter/token_bucket_test.go 생성자 호출 갱신 + throttle 로그에 label이 포함되는지 단위 테스트 4건 추가
test/internal/processor/fetcher/rate_limiter/ip_registry_test.go 생성자 호출 시그니처 변경 반영
Comments suppressed due to low confidence (1)

internal/processor/fetcher/rate_limiter/token_bucket.go:103

  • ctx_err 필드에 error 값을 그대로 넣으면 zerolog JSON marshal 결과가 {}로 찍혀 실제 에러 문자열이 남지 않을 가능성이 큽니다(예: context.Canceled). ctx_err에는 ctxErr.Error() 같은 문자열을 넣거나, log.WithError(ctxErr)를 사용해 표준 error 필드로 기록하도록 변경하는 게 로그 추적에 더 안전합니다.
		case <-ctx.Done():
			ctxErr := ctx.Err()
			log.WithFields(map[string]interface{}{
				"label":      r.label,
				"wait_count": waitCount,
				"rate":       r.rate,
				"burst":      r.burst,
				"ctx_err":    ctxErr,
			}).Debug("rate limit wait context done")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[REFACTOR] rate_limit 로그에 host/IP 필드 추가 — throttled 호스트 사후 추적 가능하도록

2 participants