Skip to content

imp(): mv rate limiting to grouper#163

Open
e11sy wants to merge 8 commits into
masterfrom
imp/mv-rate-limits-to-grouper
Open

imp(): mv rate limiting to grouper#163
e11sy wants to merge 8 commits into
masterfrom
imp/mv-rate-limits-to-grouper

Conversation

@e11sy

@e11sy e11sy commented Jul 7, 2026

Copy link
Copy Markdown
Member

This PR is part of work related to rate limiting issue, se workers part

Problem

  • Rate limiting logic lived in collector and was affected by sentry logs, metrics etc, that were filtered in sentry worker

it've been leading to rate limiting inconsistency
counter of events filled too fast, grouper managed to save 5-10 repetitions, however rate limits were configured to 1000+ per hour

Solution

Move collector rate-limiting logic to the grouper, where we can count actual amount of events that are being saved to db

here we just removed rate limiting logic

also updated lint CI, it used latest go 1.26 and failed (fixed to 1.21, as we have in tests)

@e11sy
e11sy marked this pull request as ready for review July 13, 2026 18:45
@e11sy
e11sy requested review from khaydarov and n0str as code owners July 13, 2026 18:45
Comment thread README.md
Per-project rate limiting is enforced in the **grouper worker** (not in collector). Collector still rejects requests from workspace-blocked projects via `DisabledProjectsSet`.

```json
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Меня смущает, что удалена вся логика rate-limits в коллекторе. Предлагаю чтение и блокирование все-таки оставить, чтобы исключить ситуацию, когда у нас какой-то проект шлет пачками ошибки за пределами лимитов и мы нагружаем группера, чтобы его блокировать

Comment thread pkg/redis/client_test.go
"github.com/stretchr/testify/assert"
)

func setupTestRedis(t *testing.T) (*RedisClient, *miniredis.Miniredis) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Тесты редиса некоторые нужно было оставить. Следует удалить только TestUpdateRateLimit, сохранив Redis setup и large-set test.

}
log.Debugf("Found project with ID %s for integration token %s", projectId, integrationSecret)

projectLimits, ok := handler.AccountsMongoDBClient.GetProjectLimits(projectId)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Статистику тоже хотим удалять? events-rate-limited перестанет удаляться, если это убрать. А в группер заблокированные ивенты не попадут

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Moves rate-limiting responsibilities out of hawk.collector so the collector only blocks explicitly disabled projects, while rate limiting is handled downstream (grouper). Also updates the lint workflow to use a stable Go toolchain/action versions.

Changes:

  • Removed per-project rate limiting and related metrics emission from error/release handlers, along with the MongoDB “project limits” cache.
  • Removed Redis UpdateRateLimit logic and associated tests.
  • Updated GitHub Actions lint workflow to use Go 1.21 and pinned golangci-lint/action versions; updated env samples/docs accordingly.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
README.md Reworded rate limiting documentation to reflect that it’s no longer enforced in collector.
pkg/server/releasehandler/handler.go Removed project limits cache lookup; kept disabled-project blocking check.
pkg/server/errorshandler/handler.go Removed rate-limiting checks/metrics; kept disabled-project blocking check.
pkg/server/errorshandler/handler_sentry.go Same as above for Sentry ingestion path.
pkg/redis/client.go Aliased go-redis import and removed UpdateRateLimit implementation.
pkg/redis/client_test.go Deleted Redis rate-limit tests (and other Redis tests in the same file).
pkg/accounts/mongodb.go Removed projectLimits cache storage and accessor.
pkg/accounts/cache.go Removed projects/workspaces/plans rate-limit cache building logic and related types.
cmd/config.go Removed PROJECTS_LIMITS_UPDATE_PERIOD from collector config.
cmd/collector/main.go Removed periodic project-limits cache refresh.
.github/workflows/lint.yml Upgraded actions and pinned Go + golangci-lint versions.
.env.sample Removed PROJECTS_LIMITS_UPDATE_PERIOD.
.env.docker Removed PROJECTS_LIMITS_UPDATE_PERIOD.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md Outdated
Comment on lines +170 to +172
Per-project rate limiting is enforced in the **grouper worker** (not in collector). Collector still rejects requests from workspace-blocked projects via `DisabledProjectsSet`.

```json
{
"code": 402,
"error": true,
"message": "Rate limit exceeded"
}
```
See `workers/workers/grouper/README.md` and `workers/workers/grouper/.env.sample` for configuration (`PROJECTS_LIMITS_UPDATE_PERIOD`, `REDIS_RATE_LIMITS_KEY`).
Comment thread pkg/redis/client.go
Comment on lines 227 to 229
// TSCreateIfNotExists creates a RedisTimeSeries key if it doesn't exist.
// It sets optional retention policy and attaches labels.
func (r *RedisClient) TSCreateIfNotExists(
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.

3 participants