Микросервис на Go для автоматического назначения ревьюеров на Pull Request’ы внутри команды.
Взаимодействие — только через HTTP API (спецификация: openapi.yml).
Хранение данных — PostgreSQL.
Добавлен простой эндпоинт статистики (/stats).
Реализован скрипт нагрузочного тестирования для k6 на JavaScript (load-test.js).
Добавлен метод массовой деактивации пользователей команды и безопасная переназначаемость открытых PR (эндпоинт /team/bulkDeactivate).
Реализовано E2E-тестирование (main_test.go).
Описана конфигурация линтера golangci-lint (.golangci.yml).
- Docker
- docker-compose
- (опционально) Go (для запуска
go test, запуска линтераgolangci-lint)
Клонирование репозитория:
git clone https://github.com/RSMT98/prreviewer.git
cd prreviewerЗапуск:
docker-compose up --build
# если возникнет ошибка вида 'Permission denied' — выполните команду с sudo:
# sudo docker-compose up --buildЧто произойдёт:
- поднимется контейнер
dbс PostgreSQL:- БД:
pr_service - пользователь/пароль:
pr_service/pr_service - на старте выполнится
db/init.sql(создаст таблицы и индексы)
- БД:
- поднимется контейнер
app:- собирается бинарник
pr-serviceиз исходников (Dockerfile) - приложение подключается к БД по
DB_DSN=postgres://pr_service:pr_service@db:5432/pr_service?sslmode=disable - сервис слушает
:8080внутри контейнера и проброшен на хостlocalhost:8080
- собирается бинарник
Проверка, что сервис поднялся:
curl http://localhost:8080/health
# {"status":"ok"}E2E-тесты (main_test.go) работают как чёрный ящик: они ходят по HTTP на http://localhost:8080.
- Убедитесь, что
docker-compose upуже запущен и сервис отвечает на/health. - В другом терминале, из корня проекта:
go mod tidy
go test ./...Что проверяется:
- полный жизненный цикл PR:
- создание команды, PR, назначение ревьюров
/users/getReview/pullRequest/reassign- идемпотентный
/pullRequest/merge - запрет переназначения после MERGED
- массовая деактивация
/team/bulkDeactivateи отсутствие OPEN PR у деактивированных пользователей - конфликты
/team/add(TEAM_EXISTS) - корректная работа
/users/setIsActive - ошибки создания/merge PR (NOT_FOUND, PR_EXISTS)
- согласованность
/statsс фактическими назначениями
Скрипт — load-test.js. Он запускает два сценария:
general— типичная работа сервиса:- создание PR
- перенос ревьюера (
/pullRequest/reassign) - merge (двойной вызов для проверки идемпотентности)
- чтение
/users/getReview,/stats,/health
bulkDeactivate— постоянные массовые деактивации пользователей команды (/team/bulkDeactivate)- проверки, что у деактивированных пользователей не осталось OPEN PR.
Запуск (сервис уже должен работать на localhost:8080):
docker run --rm -i --network=host grafana/k6 run - < load-test.js
# если возникнет ошибка вида 'Permission denied' — выполните команду с sudo:
# sudo docker run --rm -i --network=host grafana/k6 run - < load-test.jsВ options скрипта:
TEAM_COUNT = 10USERS_PER_TEAM = 20(итого 200 пользователей)INACTIVE_PER_TEAM = 2(часть пользователей сразу неактивна)- пороговые значения:
biz_error_rate: rate<0.001(см. ниже в разделе про архитектуру)p(99)по длительности запросов:- общий сценарий —
< 300ms bulkDeactivate—< 100ms
- общий сценарий —
Пример фактических результатов:
█ THRESHOLDS
biz_error_rate
✓ 'rate<0.001' rate=0.00%
http_req_duration{scenario:"bulkDeactivate"}
✓ 'p(99)<100' p(99)=7.93ms
http_req_duration{scenario:"general"}
✓ 'p(99)<300' p(99)=6.16ms
█ TOTAL RESULTS
checks_total.......: 1472 18.382332/s
checks_succeeded...: 100.00% 1472 out of 1472
checks_failed......: 0.00% 0 out of 1472
✓ team created or exists
✓ bulk deactivate 200
✓ deactivated u-9-9 has only MERGED PRs or none
✓ deactivated u-9-7 has only MERGED PRs or none
✓ deactivated u-9-10 has only MERGED PRs or none
✓ deactivated u-1-9 has only MERGED PRs or none
✓ deactivated u-1-13 has only MERGED PRs or none
✓ deactivated u-1-15 has only MERGED PRs or none
✓ create status 201
✓ reassign ok or domain conflict
✓ deactivated u-3-13 has only MERGED PRs or none
✓ deactivated u-3-10 has only MERGED PRs or none
✓ deactivated u-3-12 has only MERGED PRs or none
✓ merge1 status 200
✓ merge2 still 200 (idempotent)
✓ deactivated u-10-12 has only MERGED PRs or none
✓ deactivated u-10-1 has only MERGED PRs or none
✓ deactivated u-10-7 has only MERGED PRs or none
✓ deactivated u-5-20 has only MERGED PRs or none
...
✓ deactivated u-8-7 has only MERGED PRs or none
✓ deactivated u-7-10 has only MERGED PRs or none
✓ deactivated u-7-20 has only MERGED PRs or none
CUSTOM
biz_error_rate....................: 0.00% 0 out of 2507
HTTP
http_req_duration.................: avg=2.67ms min=138.15µs med=1.76ms max=11.88ms p(90)=4.96ms p(95)=5.41ms
{ expected_response:true }......: avg=2.68ms min=138.15µs med=1.81ms max=11.88ms p(90)=4.96ms p(95)=5.42ms
{ scenario:"bulkDeactivate" }...: avg=1.96ms min=350.02µs med=680.98µs max=11.88ms p(90)=5.94ms p(95)=6.78ms
{ scenario:"general" }..........: avg=2.76ms min=138.15µs med=3.86ms max=10.49ms p(90)=4.88ms p(95)=5.14ms
http_req_failed...................: 0.63% 16 out of 2507
http_reqs.........................: 2507 31.307409/s
EXECUTION
iteration_duration................: avg=1.01s min=1s med=1.02s max=1.02s p(90)=1.02s p(95)=1.02s
iterations........................: 419 5.232471/s
vus...............................: 2 min=1 max=6
vus_max...........................: 6 min=6 max=6
NETWORK
data_received.....................: 5.1 MB 64 kB/s
data_sent.........................: 364 kB 4.5 kB/s
running (1m20.1s), 0/6 VUs, 419 complete and 6 interrupted iterations
bulkDeactivate ✓ [ 100% ] 1 VUs 1m20s
general ✓ [ 100% ] 0/5 VUs 1m20s
Видно, что требования CLI выполняются даже при RPS≈30.
Используется golangci-lint с конфигурацией .golangci.yml.
Установка:
go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
# возможно ещё потребуется сделать:
# echo 'export PATH="$PATH:$(go env GOPATH)/bin"' >> ~/.bashrc
# source ~/.bashrcЗапуск:
go mod tidy
golangci-lint runПроблема: многие операции логически работают внутри одной команды, и при одновременных вызовах есть риск гонок:
/pullRequest/create— выбирает ревьюров из команды автора;/pullRequest/reassign— выбирает замену из команды старого ревьюра;/users/setIsActive— может деактивировать пользователя;/team/bulkDeactivate— массово деактивирует пользователей команды и переназначает ревьюров в открытых PR.
Если дать всем этим операциям свободно крутиться параллельно, можно получить:
- двойные назначения,
- потерю обновлений,
- странные комбинации ревьюров.
Решение: map мьютексов по командам (sync.Map + *sync.Mutex)
var teamLocks sync.Map
func lockTeam(teamName string) func() {
v, _ := teamLocks.LoadOrStore(teamName, &sync.Mutex{})
mu := v.(*sync.Mutex)
mu.Lock()
return func() { mu.Unlock() }
}- Для каждой команды создаётся свой
*sync.Mutex. - Операции, которые меняют состояние команды или её PR, берут
lockTeam(teamName):/pullRequest/create— поauthorTeam./pullRequest/reassign— по команде ревьюера./users/setIsActive— поteam_nameпользователя./team/bulkDeactivate— поteam_nameиз запроса.
В сочетании с:
- транзакциями (
BEGIN ... COMMIT), SELECT ... FOR UPDATEдля конкретных PR,
это даёт:
- отсутствие гонок внутри команды;
- при этом команды между собой работают параллельно, что более чем достаточно для RPS=5.
В OpenAPI перечислены фиксированные коды ошибок:
TEAM_EXISTSPR_EXISTSPR_MERGEDNOT_ASSIGNEDNO_CANDIDATENOT_FOUND
Решение:
-
для доменных ошибок используются именно эти коды, с подходящими HTTP-статусами:
- 400
TEAM_EXISTS— повторная попытка создать ту же команду; - 409
PR_EXISTS— создание PR с уже существующимpull_request_id; - 409
PR_MERGED— попытка переназначения ревьюра после MERGED; - 409
NOT_ASSIGNED— пользователь не был ревьюром этого PR; - 409
NO_CANDIDATE— нет подходящего активного кандидата для переназначения; - 404
NOT_FOUND— не найден PR/пользователь/команда.
- 400
-
для внутренних ошибок сервера (500) используется тот же формат, но с кодом из enum (
NOT_FOUND), чтобы не выходить за рамки схемы. Семантически это не идеально, но позволяет сохранить единообразный формат ответа и укладываться в ограниченный список кодов.
Исходно k6-метрика http_req_failed считала любой неуспешный статус (4xx/5xx) как ошибку.
В нагрузочном сценарии есть /pullRequest/reassign, который по бизнес-логике может возвращать 409 — и это не баг, а ожидаемый доменный конфликт.
Два типичных случая:
-
NOT_ASSIGNEDпри reassignВ скрипте
mainScenarioпоследовательность такая:- создаём PR (
/pullRequest/create), получаем список ревьюров; - сразу после этого вызываем
/pullRequest/reassignдля одного из них.
Параллельно сценарий
bulkDeactivateделает:/team/bulkDeactivate→ может перераспределить или удалить ревьюров в открытых PR.
Если между
createиreassignдругой поток уже успел снять или переназначить этого ревьюра (черезbulkDeactivate), то при вызове/pullRequest/reassign:- PR уже не содержит старого ревьюра;
- сервис корректно отвечает 409
NOT_ASSIGNED.
Это не инфраструктурная ошибка, а честный ответ "не могу выполнить операцию, потому что состояние уже поменялось".
- создаём PR (
-
NO_CANDIDATEпри reassignАналогично, если к моменту
reassignв команде не осталось ни одного подходящего активного кандидата (все деактивированы или уже назначены):candidatesстановится пустым;- сервис возвращает 409
NO_CANDIDATE.
Это тоже нормальный доменный конфликт: операция логически не может быть выполнена.
Оба этих кейса начали портить http_req_failed, хотя бизнес-логика была корректной.
Чтобы разделить:
- инфраструктурные/системные ошибки (5xx),
- и ожидаемые доменные конфликты (409 с
NOT_ASSIGNED/NO_CANDIDATE),
в load-test.js была добавлена своя метрика:
export const biz_error_rate = new Rate('biz_error_rate');
function trackError(res) {
biz_error_rate.add(res.status >= 500);
}То есть:
biz_error_rateсчитает только ответы со статусом>= 500;- 4xx (включая 409) не учитываются как "бизнес-ошибка".
Таким образом:
biz_error_rate = 0означает, что не было ни одной 5xx-ошибки — сервис стабилен;http_req_failedможет быть >0 из-за 409, но это нормальные бизнес-конфликты.
В спецификации openapi.yml для эндпоинта /pullRequest/reassign есть несогласованность:
- в схеме запроса есть поле под названием
old_user_id; - в примере запроса используется поле
old_reviewer_id.
Чтобы обеспечить совместимость с обеими трактовками спецификации, было принято решение поддерживать оба варианта:
type reassignPRRequest struct {
PullRequestID string `json:"pull_request_id"`
OldUserID string `json:"old_user_id"`
OldReviewerID string `json:"old_reviewer_id"`
}
func handlePRReassign(w http.ResponseWriter, r *http.Request) {
var req reassignPRRequest
// ...
oldID := req.OldUserID
if oldID == "" {
oldID = req.OldReviewerID
}
// ...
}Логика такая:
-
Если клиент отправляет корректное поле по схеме (
old_user_id) — оно используется. -
Если клиент следует примеру из OpenAPI и отправляет
old_reviewer_id— запрос тоже будет принят. -
Если оба поля пустые — запрос считается некорректным и возвращается ошибка валидации.
Таким образом сервис и следует формальной схеме, и остаётся совместим с примером из спецификации.