TODO

Completed

скип лист коллекторов в конфиге
кэш для метрик
- fetch + build
- wrapper cache
клиент кинетика поддерживает сессию
Механизм остановки background updaters: Context-based cancellation с graceful shutdown
Background updater для всех метрик: Реализован generic updater для всех коллекторов
- Работает для system, hotspot, internet, interface, interface_stats, process
- Учитывает skip_collectors для каждого устройства
- Graceful shutdown через context cancellation и WaitGroup

High Priority

Critical Issues

(Пусто - все критические проблемы решены или перенесены в планируемые фичи)

Configuration & Architecture

Хардкод пути к конфигу: Нет CLI флагов для указания файла конфигурации
Device timeout не учитывается: HTTP клиент использует хардкод 5s таймаут
Добавить версионирование

Medium Priority

Features

Привести в порядок логгирование, добавить разные уровни (Error, Info, Debug), добавить в конфиг параметр отображения логов.
медиа коллектор
обработка ошибок для offline устройств (улучшенная)
Метрики ошибок экспортера: Экспортировать метрики о работе самого экспортера
- keenetic_scrape_success{device, collector} - успешность последнего scrape
- keenetic_scrape_duration_seconds{device, collector} - длительность сбора
- keenetic_scrape_errors_total{device, collector} - счетчик ошибок
- Позволит настраивать алерты на проблемы со сбором метрик
- Улучшит observability и диагностику проблем
Строго типизированный кеш с дженериками: Улучшение type safety
- ✅ Текущее состояние: Добавлены runtime проверки type assertions с обработкой ошибок
- 🎯 Цель: Переделать на TypedCache с CacheEntry[T any] для compile-time type safety
- Уберет необходимость runtime проверок и сделает код более безопасным
- См. вариант 1 из предыдущих обсуждений

Security & Production Readiness

Credentials в git: config.yaml содержит тестовые пароли
- Для production: использовать environment variables или secrets manager

Testing

Нет тестов: Нулевое покрытие тестами
- Unit tests для коллекторов
- Integration tests для Keenetic клиента
- Tests для кеширования

Low Priority

Keenetic Client Code Quality Issues

Найдено при анализе internal/keenetic/client.go (2025-10-16)

Critical Issues (безопасность/корректность)

Race condition при повторной аутентификации (client.go:249-261)
- Проблема: Если несколько горутин одновременно получат HTTP 401, все они параллельно вызовут authenticate(). Это создаст множественные конкурирующие запросы к /auth endpoint.
- Локация: Метод doGet(), блок обработки StatusUnauthorized
- Последствия: Непредсказуемое поведение, лишние запросы к API, возможные конфликты сессий
- Решение: Использовать sync.Mutex или sync.Once для синхронизации процесса re-authentication
- Пример:
```
var authMutex sync.Mutex
authMutex.Lock()
defer authMutex.Unlock()
if err := c.authenticate(ctx, c.login, c.password); err != nil { ... }
```
Race condition на полях Client (client.go:127-133)
- Проблема: Поля login, password, Hostname записываются в методах (authenticate(), fetchHostname()), которые могут вызываться из разных горутин без синхронизации
- Локация: Структура Client и методы, изменяющие её поля
- Последствия: Data races, неопределенное поведение, возможные паники
- Решение: Добавить sync.RWMutex для защиты полей или сделать их write-once (инициализация только в конструкторе)
- Детекция: go test -race покажет эти гонки
Отсутствие ограничения размера тела ответа при ошибках (client.go:264)
- Проблема: io.ReadAll(resp.Body) читает всё тело без ограничений при HTTP ошибках
- Локация: Блок обработки resp.StatusCode >= 400 в методе doGet()
- Последствия:
  - Потенциальная DoS атака через огромное тело ответа
  - OOM если сервер вернет гигабайты данных
  - Чувствительные данные могут попасть в логи
- Решение: Использовать io.LimitReader(resp.Body, maxBodySize) (например, 1MB)
- Пример:
```
limitedBody := io.LimitReader(resp.Body, 1024*1024) // 1MB max
data, err := io.ReadAll(limitedBody)
if err != nil {
    return fmt.Errorf("failed to read error body: %w", err)
}
```
Недостаточная валидация входных данных
- Проблема в NewClient() (client.go:135-145): Не проверяется что URL имеет scheme (http/https) и host
- Проблема в Init() и authenticate(): Нет проверки что login и password не пустые
- Локация: Конструктор и методы инициализации
- Последствия: Невалидные клиенты могут быть созданы, приводя к ошибкам позже
- Решение: Добавить валидацию в NewClient():
```
if parsed.Scheme != "http" && parsed.Scheme != "https" {
    return nil, fmt.Errorf("URL must have http or https scheme")
}
if parsed.Host == "" {
    return nil, fmt.Errorf("URL must have a host")
}
```
- Решение для credentials: Проверять в Init() перед вызовом authenticate()

Serious Issues (надежность)

Игнорируется timeout из конфигурации (client.go:139)
- Проблема: HTTP клиент всегда использует хардкод Timeout: 5 * time.Second, несмотря на то что в config.yaml есть поле timeout для каждого устройства
- Локация: Инициализация httpClient в NewClient()
- Последствия: Невозможно настроить timeout для медленных устройств или быстрых сетей
- Решение: Принимать timeout как параметр в NewClient() или добавить метод SetTimeout()
- Связано: High Priority issue "Device timeout не учитывается"
Игнорирование ошибки создания cookiejar (client.go:136)
- Проблема: jar, _ := cookiejar.New(nil) игнорирует потенциальную ошибку
- Локация: Инициализация в NewClient()
- Последствия: Нарушение best practice, теоретически может скрыть проблему
- Решение: Обрабатывать ошибку:
```
jar, err := cookiejar.New(nil)
if err != nil {
    return nil, fmt.Errorf("failed to create cookie jar: %w", err)
}
```
GetConnectedInterfaceStats() молча игнорирует ошибки (client.go:355-358)
- Проблема: При ошибке получения статистики интерфейса просто continue, без логирования
- Локация: Цикл по интерфейсам в GetConnectedInterfaceStats()
- Последствия: Невозможно отличить "интерфейс disconnected" от "ошибка API"
- Решение: Логировать ошибки или возвращать частичные результаты с map ошибок:
```
type InterfaceStatsResult struct {
    Stats map[string]InterfaceStats
    Errors map[string]error
}
```
GetProcessInfo() молча пропускает невалидные записи (client.go:298-300)
- Проблема: При ошибке json.Unmarshal просто continue, нет информации о количестве пропущенных процессов
- Локация: Цикл парсинга процессов в GetProcessInfo()
- Последствия: Тихая потеря данных, сложно диагностировать проблемы с API
- Решение: Добавить счетчик ошибок или логирование
Контекст не проверяется внутри длительных операций (client.go:343-363)
- Проблема: В GetConnectedInterfaceStats() делается N запросов в цикле, но ctx.Done() не проверяется между итерациями
- Локация: Цикл по интерфейсам
- Последствия: Невозможно отменить долгую операцию, если context был cancelled
- Решение: Добавить проверку:
```
for _, iface := range interfaces {
    select {
    case <-ctx.Done():
        return nil, ctx.Err()
    default:
    }
    // ... continue processing
}
```

Design Issues (проектирование)

Неэффективная повторная аутентификация
- Проблема: При каждом 401 делается полный цикл (GET /auth для получения challenge + POST /auth), даже если challenge еще валиден
- Локация: Логика re-authentication в doGet()
- Последствия: Лишние запросы к API, увеличенная latency
- Решение: Кешировать challenge с разумным TTL или использовать только POST при re-auth
Отсутствие retry логики
- Проблема: При временных сетевых ошибках (connection reset, timeout) сразу возвращается ошибка
- Локация: Все API методы
- Последствия: Низкая устойчивость к временным проблемам сети
- Решение: Добавить configurable retry с exponential backoff для idempotent операций
- Библиотеки: github.com/cenkalti/backoff/v4 или встроенный механизм
Нет метрик и observability
- Проблема: Невозможно отследить важные события:
  - Количество re-authentication попыток
  - Время выполнения запросов (latency по endpoint)
  - Частота ошибок по типам и endpoint
  - Размер ответов API
- Локация: Весь клиент
- Последствия: Сложно диагностировать проблемы в production, нет visibility
- Решение: Интегрировать с Prometheus metrics или добавить structured logging
- Связано: Medium Priority issue "Метрики ошибок экспортера"
Отсутствие метода Cleanup/Close
- Проблема: Client владеет httpClient с connections pool и cookiejar, но нет способа явно освободить ресурсы
- Локация: Структура Client
- Последствия: Goroutine leaks в тестах, невозможность graceful cleanup
- Решение: Добавить метод:
```
func (c *Client) Close() error {
    c.httpClient.CloseIdleConnections()
    return nil
}
```

Code Quality Issues (качество кода)

Утечка ресурсов при двойном defer (client.go:247, 260)
- Проблема:
```
resp, err := doRequest()
defer resp.Body.Close()  // defer на первый resp
if resp.StatusCode == http.StatusUnauthorized {
    _ = resp.Body.Close()  // явное закрытие
    resp, err = doRequest()  // переприсваивание переменной
    defer resp.Body.Close()  // defer на второй resp
}
```
- Локация: Метод doGet()
- Последствия: Путаница, потенциальное двойное закрытие, плохая читаемость
- Решение: Использовать именованные переменные (resp1, resp2) или рефакторить логику
Смешанный язык в комментариях (client.go:250)
- Проблема: Комментарий // закрываем перед повтором на русском языке
- Локация: Различные места в файле
- Нарушение: CLAUDE.md требует все комментарии на английском
- Решение: Перевести все комментарии на английский
Потенциальная утечка credentials в логах (client.go:264)
- Проблема: В error message включается fullURL.String(), который может содержать query параметры с токенами
- Локация: Обработка ошибок в doGet()
- Последствия: Чувствительные данные могут попасть в логи
- Решение: Sanitize URL перед логированием или использовать fullURL.Redacted()
Недостаточная типизация для статусов
- Проблема: Поля типа string для статусов легко опечатать:
```
Connected string `json:"connected"` // "yes"/"no"
State     string `json:"state"`
Link      string `json:"link"`
```
- Локация: Структуры моделей данных
- Последствия: Легко сделать ошибку при сравнении ("Yes" vs "yes")
- Решение: Использовать custom types с константами:
```
type ConnectionStatus string
const (
    ConnectionStatusYes ConnectionStatus = "yes"
    ConnectionStatusNo  ConnectionStatus = "no"
)
```
Неиспользуемые поля в структурах
- Проблема: InterfaceStats.LastOverflow (client.go:118) не используется нигде
- Локация: Определения структур
- Последствия: Мертвый код, увеличивает cognitive load
- Решение: Удалить или задокументировать зачем поле сохранено
Магические числа
- Проблема: 5 * time.Second (client.go:139), 400 (client.go:263)
- Локация: Различные места
- Последствия: Плохая читаемость и maintainability
- Решение: Использовать именованные константы:
```
const (
    defaultHTTPTimeout = 5 * time.Second
    httpErrorStatusCode = 400
)
```

Общие рекомендации

Добавить линтеры: golangci-lint с конфигом покажет большинство этих проблем
Race detector: go test -race для всех тестов
Structured logging: Заменить fmt.Errorf на proper logging с levels
Error wrapping: Везде используется, это хорошо (Go 1.13+ style)
Context usage: В целом правильно, но не проверяется в циклах

18 KiB Raw Permalink Blame History Unescape Escape

TODO

Completed

High Priority

Critical Issues

Configuration & Architecture

Medium Priority

Features

Security & Production Readiness

Testing

Low Priority

Keenetic Client Code Quality Issues

Critical Issues (безопасность/корректность)

Serious Issues (надежность)

Design Issues (проектирование)

Code Quality Issues (качество кода)

Общие рекомендации

18 KiB

Raw Permalink Blame History