# 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 (безопасность/корректность) 1. **Race condition при повторной аутентификации** (`client.go:249-261`) - **Проблема**: Если несколько горутин одновременно получат HTTP 401, все они параллельно вызовут `authenticate()`. Это создаст множественные конкурирующие запросы к `/auth` endpoint. - **Локация**: Метод `doGet()`, блок обработки `StatusUnauthorized` - **Последствия**: Непредсказуемое поведение, лишние запросы к API, возможные конфликты сессий - **Решение**: Использовать `sync.Mutex` или `sync.Once` для синхронизации процесса re-authentication - **Пример**: ```go var authMutex sync.Mutex authMutex.Lock() defer authMutex.Unlock() if err := c.authenticate(ctx, c.login, c.password); err != nil { ... } ``` 2. **Race condition на полях Client** (`client.go:127-133`) - **Проблема**: Поля `login`, `password`, `Hostname` записываются в методах (`authenticate()`, `fetchHostname()`), которые могут вызываться из разных горутин без синхронизации - **Локация**: Структура `Client` и методы, изменяющие её поля - **Последствия**: Data races, неопределенное поведение, возможные паники - **Решение**: Добавить `sync.RWMutex` для защиты полей или сделать их write-once (инициализация только в конструкторе) - **Детекция**: `go test -race` покажет эти гонки 3. **Отсутствие ограничения размера тела ответа при ошибках** (`client.go:264`) - **Проблема**: `io.ReadAll(resp.Body)` читает всё тело без ограничений при HTTP ошибках - **Локация**: Блок обработки `resp.StatusCode >= 400` в методе `doGet()` - **Последствия**: - Потенциальная DoS атака через огромное тело ответа - OOM если сервер вернет гигабайты данных - Чувствительные данные могут попасть в логи - **Решение**: Использовать `io.LimitReader(resp.Body, maxBodySize)` (например, 1MB) - **Пример**: ```go 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) } ``` 4. **Недостаточная валидация входных данных** - **Проблема в `NewClient()`** (`client.go:135-145`): Не проверяется что URL имеет scheme (http/https) и host - **Проблема в `Init()` и `authenticate()`**: Нет проверки что `login` и `password` не пустые - **Локация**: Конструктор и методы инициализации - **Последствия**: Невалидные клиенты могут быть созданы, приводя к ошибкам позже - **Решение**: Добавить валидацию в `NewClient()`: ```go 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 (надежность) 5. **Игнорируется timeout из конфигурации** (`client.go:139`) - **Проблема**: HTTP клиент всегда использует хардкод `Timeout: 5 * time.Second`, несмотря на то что в config.yaml есть поле `timeout` для каждого устройства - **Локация**: Инициализация `httpClient` в `NewClient()` - **Последствия**: Невозможно настроить timeout для медленных устройств или быстрых сетей - **Решение**: Принимать timeout как параметр в `NewClient()` или добавить метод `SetTimeout()` - **Связано**: High Priority issue "Device timeout не учитывается" 6. **Игнорирование ошибки создания cookiejar** (`client.go:136`) - **Проблема**: `jar, _ := cookiejar.New(nil)` игнорирует потенциальную ошибку - **Локация**: Инициализация в `NewClient()` - **Последствия**: Нарушение best practice, теоретически может скрыть проблему - **Решение**: Обрабатывать ошибку: ```go jar, err := cookiejar.New(nil) if err != nil { return nil, fmt.Errorf("failed to create cookie jar: %w", err) } ``` 7. **GetConnectedInterfaceStats() молча игнорирует ошибки** (`client.go:355-358`) - **Проблема**: При ошибке получения статистики интерфейса просто `continue`, без логирования - **Локация**: Цикл по интерфейсам в `GetConnectedInterfaceStats()` - **Последствия**: Невозможно отличить "интерфейс disconnected" от "ошибка API" - **Решение**: Логировать ошибки или возвращать частичные результаты с map ошибок: ```go type InterfaceStatsResult struct { Stats map[string]InterfaceStats Errors map[string]error } ``` 8. **GetProcessInfo() молча пропускает невалидные записи** (`client.go:298-300`) - **Проблема**: При ошибке `json.Unmarshal` просто `continue`, нет информации о количестве пропущенных процессов - **Локация**: Цикл парсинга процессов в `GetProcessInfo()` - **Последствия**: Тихая потеря данных, сложно диагностировать проблемы с API - **Решение**: Добавить счетчик ошибок или логирование 9. **Контекст не проверяется внутри длительных операций** (`client.go:343-363`) - **Проблема**: В `GetConnectedInterfaceStats()` делается N запросов в цикле, но `ctx.Done()` не проверяется между итерациями - **Локация**: Цикл по интерфейсам - **Последствия**: Невозможно отменить долгую операцию, если context был cancelled - **Решение**: Добавить проверку: ```go for _, iface := range interfaces { select { case <-ctx.Done(): return nil, ctx.Err() default: } // ... continue processing } ``` #### Design Issues (проектирование) 10. **Неэффективная повторная аутентификация** - **Проблема**: При каждом 401 делается полный цикл (GET /auth для получения challenge + POST /auth), даже если challenge еще валиден - **Локация**: Логика re-authentication в `doGet()` - **Последствия**: Лишние запросы к API, увеличенная latency - **Решение**: Кешировать challenge с разумным TTL или использовать только POST при re-auth 11. **Отсутствие retry логики** - **Проблема**: При временных сетевых ошибках (connection reset, timeout) сразу возвращается ошибка - **Локация**: Все API методы - **Последствия**: Низкая устойчивость к временным проблемам сети - **Решение**: Добавить configurable retry с exponential backoff для idempotent операций - **Библиотеки**: `github.com/cenkalti/backoff/v4` или встроенный механизм 12. **Нет метрик и observability** - **Проблема**: Невозможно отследить важные события: - Количество re-authentication попыток - Время выполнения запросов (latency по endpoint) - Частота ошибок по типам и endpoint - Размер ответов API - **Локация**: Весь клиент - **Последствия**: Сложно диагностировать проблемы в production, нет visibility - **Решение**: Интегрировать с Prometheus metrics или добавить structured logging - **Связано**: Medium Priority issue "Метрики ошибок экспортера" 13. **Отсутствие метода Cleanup/Close** - **Проблема**: `Client` владеет `httpClient` с connections pool и cookiejar, но нет способа явно освободить ресурсы - **Локация**: Структура `Client` - **Последствия**: Goroutine leaks в тестах, невозможность graceful cleanup - **Решение**: Добавить метод: ```go func (c *Client) Close() error { c.httpClient.CloseIdleConnections() return nil } ``` #### Code Quality Issues (качество кода) 14. **Утечка ресурсов при двойном defer** (`client.go:247, 260`) - **Проблема**: ```go 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`) или рефакторить логику 15. **Смешанный язык в комментариях** (`client.go:250`) - **Проблема**: Комментарий `// закрываем перед повтором` на русском языке - **Локация**: Различные места в файле - **Нарушение**: CLAUDE.md требует все комментарии на английском - **Решение**: Перевести все комментарии на английский 16. **Потенциальная утечка credentials в логах** (`client.go:264`) - **Проблема**: В error message включается `fullURL.String()`, который может содержать query параметры с токенами - **Локация**: Обработка ошибок в `doGet()` - **Последствия**: Чувствительные данные могут попасть в логи - **Решение**: Sanitize URL перед логированием или использовать `fullURL.Redacted()` 17. **Недостаточная типизация для статусов** - **Проблема**: Поля типа string для статусов легко опечатать: ```go Connected string `json:"connected"` // "yes"/"no" State string `json:"state"` Link string `json:"link"` ``` - **Локация**: Структуры моделей данных - **Последствия**: Легко сделать ошибку при сравнении ("Yes" vs "yes") - **Решение**: Использовать custom types с константами: ```go type ConnectionStatus string const ( ConnectionStatusYes ConnectionStatus = "yes" ConnectionStatusNo ConnectionStatus = "no" ) ``` 18. **Неиспользуемые поля в структурах** - **Проблема**: `InterfaceStats.LastOverflow` (`client.go:118`) не используется нигде - **Локация**: Определения структур - **Последствия**: Мертвый код, увеличивает cognitive load - **Решение**: Удалить или задокументировать зачем поле сохранено 19. **Магические числа** - **Проблема**: `5 * time.Second` (`client.go:139`), `400` (`client.go:263`) - **Локация**: Различные места - **Последствия**: Плохая читаемость и maintainability - **Решение**: Использовать именованные константы: ```go 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**: В целом правильно, но не проверяется в циклах