Skip to content

Instantly share code, notes, and snippets.

@chertov
Created December 25, 2025 17:36
Show Gist options
  • Select an option

  • Save chertov/11e04f5f8f00746c90a8194f72853b12 to your computer and use it in GitHub Desktop.

Select an option

Save chertov/11e04f5f8f00746c90a8194f72853b12 to your computer and use it in GitHub Desktop.

Полное ревью PromQL Parser & Engine

Анализатор: Claude (Opus 4.5) Дата: 2025-12-25 Общий объём кода: 9015 строк Rust Статус: Код в процессе разработки, только чтение


Содержание

  1. Состояние проекта
  2. Анализ парсера AST
  3. Анализ движка eval
  4. Рефакторинг для читаемости
  5. Оптимизация производительности
  6. Технические риски и ошибки
  7. PromQL-специфика
  8. Rust-идиоматичность
  9. unwrap(), panic! и падения
  10. Обработка ошибок
  11. Строковые литералы

1. Состояние проекта

Структура файлов

promql/src/                           СТРОК   СТАТУС
├── lib.rs                              12    ✅ Отлично
├── parser.rs                           27    ✅ Отлично
├── duration.rs                        104    ✅ Отлично
├── ast/mod.rs                         142    ✅ Отлично
├── parser/ast_parser.rs             1243    ⚠️ Допустимо
└── engine/
    ├── mod.rs                         943    ✅ Хорошо
    ├── types.rs                       259    ✅ Отлично
    ├── storage.rs                      76    ✅ Отлично
    ├── regex.rs                        83    ✅ Отлично
    ├── time.rs                        104    ✅ Отлично
    ├── histogram.rs                   575    ⚠️ Сложно
    ├── aggregate.rs                   748    ⚠️ Требует рефакторинга
    ├── vector_ops.rs                  421    ✅ Хорошо
    └── functions/
        ├── mod.rs                     434    ✅ Отлично
        ├── math.rs                    226    ✅ Отлично
        ├── trig.rs                     65    ✅ Отлично
        ├── time.rs                    102    ✅ Отлично
        ├── label.rs                   156    ✅ Отлично
        ├── sort.rs                    192    ✅ Хорошо
        ├── range.rs                   726    ⚠️ Большой файл
        ├── rate.rs                   1323    ❌ Слишком большой
        ├── histogram_fn.rs            960    ⚠️ Большой файл
        └── info.rs                     94    ✅ Отлично

Общая оценка

Критерий Оценка Комментарий
Архитектура ★★★★☆ Хорошая модульность
Типобезопасность ★★★☆☆ Aggregate.op — String
Производительность ★★★★☆ Dispatch table, regex cache
Обработка ошибок ★★★☆☆ Много unwrap()
Читаемость ★★★☆☆ Большие файлы
PromQL-совместимость ★★★★☆ ~95% покрытие

2. Анализ качества парсера AST

Файл: ast/mod.rs (142 строки) — ★★★★★

Сильные стороны:

  1. Идиоматичное использование enum:
pub enum Expr {
    Number(f64),
    String(String),
    VectorSelector(VectorSelector),
    MatrixSelector(MatrixSelector),
    Subquery(SubqueryExpr),
    Call { name: String, args: Vec<Expr> },
    Aggregate { op: String, ... },  // ← Единственная проблема
    Unary { op: UnaryOp, expr: Box<Expr> },
    Binary { lhs: Box<Expr>, op: BinaryOp, ... },
}
  1. Правильное использование Box для рекурсивных структур

  2. BinaryOp как enum (правильно!):

pub enum BinaryOp {
    Add, Sub, Mul, Div, Mod, Atan2, Pow,
    Eq, NotEq, Gt, Ge, Lt, Le,
    And, Or, Unless,
}

Критическая проблема:

Aggregate {
    op: String,  // ❌ Должен быть enum AggregateOp
    ...
}

Файл: parser/ast_parser.rs (1243 строки) — ★★★★☆

Сильные стороны:

  1. Правильный приоритет операторов (recursive descent):
parse_or → parse_and_unless → parse_comparison →
parse_add_sub → parse_mul_div_mod_atan2 → parse_unary → parse_pow → parse_primary
  1. Полная поддержка escape-последовательностей:

    • \n, \r, \t, \x00, \u0000, \U00000000, октальные
  2. Чистые ошибки с позицией:

ParseError { message: String, offset: usize }

Проблемы:

  1. Строковые литералы в is_aggregate_op:
// ast_parser.rs:1189-1195
fn is_aggregate_op(name: &str) -> bool {
    match name.to_ascii_lowercase().as_str() {
        "sum" | "avg" | "min" | "max" | "count" | "stddev" | "stdvar"
        | "topk" | "bottomk" | "quantile" | "count_values" | "group"
        | "limitk" | "limit_ratio" => true,
        _ => false,
    }
}
  1. Потенциально небезопасные unwrap() (хотя после проверки длины):
// ast_parser.rs:428-433
1 => Ok((None, Box::new(args.into_iter().next().unwrap()))),
2 => {
    let param = it.next().unwrap();
    let expr = it.next().unwrap();
    ...
}

3. Анализ качества движка eval

Файл: engine/mod.rs (943 строки) — ★★★★☆

Сильные стороны:

  1. Чистая структура контекста:
pub struct EvalContext<'a> {
    pub ts_ms: TimestampMs,
    pub eval_step_ms: Option<TimestampMs>,
    pub query_start_ms: Option<TimestampMs>,
    pub query_end_ms: Option<TimestampMs>,
    pub lookback_delta_ms: TimestampMs,
    pub storage: Option<&'a dyn Storage>,
}
  1. Разделение eval/finalize:
pub fn eval_instant(expr: &Expr, ctx: EvalContext<'_>) -> Result<Value, EvalError>
fn eval_instant_raw(expr: &Expr, ctx: EvalContext<'_>) -> Result<Value, EvalError>
fn finalize_value(value: Value) -> Result<Value, EvalError>
  1. Использование BinaryOp enum (правильно!):
fn apply_binop(op: BinaryOp, a: f64, b: f64) -> f64 {
    match op {
        BinaryOp::Add => a + b,
        BinaryOp::Sub => a - b,
        // ...
    }
}

Файл: engine/functions/mod.rs (434 строки) — ★★★★★

Отличная реализация dispatch table:

type FnImpl = fn(&[Expr], EvalContext<'_>) -> Result<Value, EvalError>;
static FUNCTIONS: OnceLock<HashMap<&'static str, FnImpl>> = OnceLock::new();

pub(super) fn eval_call(name: &str, args: &[Expr], ctx: EvalContext<'_>) -> Result<Value, EvalError> {
    let name_lc = name.to_ascii_lowercase();
    if let Some(f) = FUNCTIONS.get_or_init(init_functions).get(name_lc.as_str()).copied() {
        return f(args, ctx);
    }
    Err(EvalError::UnsupportedOwned(format!("function call {name}")))
}

Преимущества:

  • O(1) lookup вместо O(n) if-chains
  • Thread-safe с OnceLock
  • Легко расширяемо

Файл: engine/aggregate.rs (748 строк) — ★★★☆☆

Проблемы:

  1. Массовое использование строковых литералов:
// aggregate.rs:20-277
match op {
    "topk" | "bottomk" => { ... }
    "limitk" => { ... }
    "limit_ratio" => { ... }
    "count_values" => { ... }
    "quantile" => { ... }
    "group" => { ... }
    "stddev" | "stdvar" => { ... }
    _ => {}
}

if op == "count" { ... }
if op == "sum" || op == "avg" { ... }
  1. Сложная структура AggState:
struct AggState {
    sum: f64, sum_c: f64,
    sum_has_nan: bool, sum_has_pos_inf: bool, sum_has_neg_inf: bool,
    count: usize, min: Option<f64>, max: Option<f64>,
    drop_meta: bool,
    avg_max_abs: f64, avg_scaled_sum: f64, avg_scaled_c: f64,
    avg_has_nan: bool, avg_has_pos_inf: bool, avg_has_neg_inf: bool,
}

Слишком много полей — нужно разбить на специализированные структуры.

Сильные стороны:

  • Neumaier summation для численной стабильности
  • Welford variance для stddev/stdvar

4. Рекомендации по рефакторингу для читаемости

4.1. Разбить большие файлы

Файл Строк Рекомендация
rate.rs 1323 Разбить на: float_rate.rs, histogram_rate.rs, deriv.rs
histogram_fn.rs 960 Разбить на: quantile.rs, fraction.rs, stats.rs
aggregate.rs 748 Вынести AggState в отдельный модуль

4.2. Упростить длинные функции

eval_rate в rate.rs (~120 строк):

// Текущая структура:
pub fn eval_rate(args: &[Expr], ctx: EvalContext<'_>) -> Result<Value, EvalError> {
    // 1. Проверка аргументов
    // 2. Извлечение range параметров из MatrixSelector или Subquery
    // 3. Обработка float серий
    // 4. Обработка histogram серий
    // 5. Формирование результата
}

// Рекомендация — разбить на:
fn extract_range_params(...) -> Result<RangeParams, EvalError>;
fn rate_float_series(...) -> Option<f64>;
fn rate_histogram_series(...) -> Result<Option<Histogram>, EvalError>;

4.3. Убрать дублирование кода

Повторяющийся паттерн проверки типов:

// Встречается 15+ раз:
let series = match eval_instant_raw(&args[0], ctx)? {
    Value::RangeVector(series) => series,
    other => {
        return Err(EvalError::TypeError {
            expected: "range-vector",
            got: other.kind(),
        })
    }
};

// Рекомендация — вспомогательная функция:
fn expect_range_vector(expr: &Expr, ctx: EvalContext<'_>) -> Result<Vec<RangeSeries>, EvalError> {
    match eval_instant_raw(expr, ctx)? {
        Value::RangeVector(v) => Ok(v),
        other => Err(EvalError::TypeError { expected: "range-vector", got: other.kind() }),
    }
}

5. Рекомендации по оптимизации производительности

5.1. Избыточное клонирование

Найдено 69 вызовов .clone() в проекте.

Примеры неоптимального клонирования:

// histogram.rs:206-207 — можно избежать clone через take
let min = *mapped.keys().next().unwrap();
let max = *mapped.keys().next_back().unwrap();

// vector_ops.rs:14 — clone в цикле
out.push((name.clone(), v.to_owned()));

// rate.rs:67 — clone гистограммы в цикле
PointValue::Histogram(h) => hists.push((p.ts_ms, h.clone())),

Рекомендации:

  • Использовать Cow<'_, str> для временных строк
  • Применять std::mem::take вместо clone где возможно
  • Рассмотреть Arc<Histogram> для shared ownership

5.2. Неэффективный LRU-кэш regex

// regex.rs:49-56
while self.order.len() > self.cap {
    let Some(oldest) = self.order.first().cloned() else { break };
    self.order.remove(0);  // O(n) операция!
    self.map.remove(&oldest);
}

Проблема: Vec::remove(0) — O(n) операция.

Рекомендация: Использовать VecDeque или готовую реализацию LRU:

use lru::LruCache;
// или std::collections::VecDeque для O(1) pop_front

5.3. Аллокации в горячих путях

// functions/mod.rs:271-272 — аллокация на каждый вызов функции
let name_lc = name.to_ascii_lowercase();  // новая String!

Рекомендация: Case-insensitive HashMap или предварительная нормализация.

5.4. Многократная сортировка

// Встречается ~20 раз:
out.sort_by(|a, b| a.labels.cmp(&b.labels));

Рекомендация: Рассмотреть IndexSet или сортировку только при необходимости.


6. Технические риски и ошибки

6.1. Потенциальное переполнение при конверсии типов

// Найдено 78 приведений типов (as i64, as u64, as f64, as usize)

// time.rs:97 — потенциальная потеря точности
let millis: u64 = secs.checked_mul(1_000)...

// rate.rs:95
let sampled_interval = (last_t.saturating_sub(first_t) as f64) / 1000.0;

Риски:

  • as f64 для больших i64 теряет точность при |x| > 2^53
  • as usize может паниковать на 32-bit платформах

Рекомендация: Использовать TryFrom с обработкой ошибок.

6.2. Потенциальный бесконечный цикл

// mod.rs:436-441
while t <= end_ms {
    // ...
    let next = t.saturating_add(step_ms);
    if next <= t { break; }  // Защита есть ✓
    t = next;
}

Защита от бесконечного цикла есть — хорошо.

6.3. Непроверенный regex от пользователя

// regex.rs:59-74
fn compile_promql_matcher_regex(pattern: &str) -> Result<Regex, EvalError> {
    let anchored = format!("^(?:{pattern})$");  // pattern от пользователя!
    let re = Regex::new(&anchored)...
}

Риски:

  • ReDoS атаки через сложные regex
  • Нет лимита на сложность regex

Рекомендация: Добавить таймаут или ограничение сложности.

6.4. Floating point edge cases

// histogram.rs:141-142
fn is_zero(&self) -> bool {
    self.sum == 0.0 && self.count == 0.0 ...  // Не учитывает -0.0
}

-0.0 == 0.0 в Rust возвращает true, но для полноты:

Рекомендация: Использовать self.sum.abs() == 0.0 если нужна строгость.


7. PromQL-специфика

7.1. Staleness markers

// types.rs:117-121
pub enum SampleValue {
    Float(f64),
    Histogram(super::Histogram),
    Stale,  // ✓ Правильно реализовано
}

7.2. Lookback delta

// mod.rs:23
pub const DEFAULT_LOOKBACK_DELTA_MS: TimestampMs = 300_000; // 5m ✓

7.3. Counter reset detection

// rate.rs:86-93 — правильная обработка сбросов счётчика
let mut result = last_v - first_v;
let mut prev = first_v;
for (_, curr) in floats.iter().skip(1) {
    if *curr < prev {
        result += prev;  // ✓ Корректно
    }
    prev = *curr;
}

7.4. Отсутствующая функциональность

Не реализовано (или не обнаружено):

  • histogram_stddev_over_time / histogram_stdvar_over_time
  • histogram_quantile с Native Histogram (проверить покрытие)
  • Некоторые edge cases в offset модификаторе с отрицательными значениями

7.5. Extrapolation в rate/increase

// rate.rs:100-130 — реализована экстраполяция ✓
let mut duration_to_start = (first_t.saturating_sub(range_start_ms) as f64) / 1000.0;
// ... extrapolation logic

8. Rust-идиоматичность

8.1. Хорошие практики (соблюдены)

✅ Использование Result<T, E> вместо exceptions ✅ #[derive(Debug, Clone, PartialEq)] для типов данных ✅ impl std::error::Error для типов ошибок ✅ Lifetime параметры (EvalContext<'a>) ✅ Option<T> вместо null ✅ Pattern matching ✅ Итераторы вместо циклов с индексами

8.2. Нарушения идиоматичности

1. Строки вместо enum:

// ❌ Плохо
Aggregate { op: String, ... }
match op { "sum" | "avg" => ... }

// ✓ Хорошо (как BinaryOp)
Aggregate { op: AggregateOp, ... }
match op { AggregateOp::Sum | AggregateOp::Avg => ... }

2. Избыточное клонирование:

// ❌ Плохо
for (k, v) in labels.iter() {
    out.push((k.to_owned(), v.to_owned()));
}

// ✓ Лучше (если labels можно consume)
out.extend(labels.into_iter());

3. Magic numbers:

// ❌ Плохо
RegexCache::new(128)  // Почему 128?

// ✓ Лучше
const REGEX_CACHE_SIZE: usize = 128;
RegexCache::new(REGEX_CACHE_SIZE)

4. Отсутствие документации:

// ❌ Публичные функции без документации
pub fn eval_instant(expr: &Expr, ctx: EvalContext<'_>) -> Result<Value, EvalError>

// ✓ Рекомендуется
/// Evaluates a PromQL expression at a single instant in time.
///
/// # Arguments
/// * `expr` - The parsed PromQL expression
/// * `ctx` - Evaluation context containing timestamp and storage
///
/// # Returns
/// The evaluated value or an error
pub fn eval_instant(...)

9. unwrap(), panic! и потенциальные падения

Найдено: 37 вызовов unwrap(), 0 panic!, 0 expect()

Безопасные unwrap (после проверки):

// ast_parser.rs:428 — после проверки args.len() == 1
1 => Ok((None, Box::new(args.into_iter().next().unwrap()))),

// rate.rs:81-84 — после проверки floats.len() >= 2
if floats.len() < 2 { continue; }
let first_t = floats.first().unwrap().0;

Потенциально опасные unwrap:

// histogram.rs:206-207 — mapped может быть пустым?
let min = *mapped.keys().next().unwrap();
let max = *mapped.keys().next_back().unwrap();

Проверка контекста:

// Есть проверка выше:
if self.buckets.is_empty() && self.n_buckets.is_empty() {
    return self.clone();
}

Безопасно — проверка есть.

Рекомендации:

1. Заменить unwrap() на expect() с сообщением:

// ❌ Плохо
floats.first().unwrap()

// ✓ Лучше (для отладки)
floats.first().expect("floats guaranteed non-empty after len check")

2. Использовать if-let где возможно:

// ❌ Плохо
let first = samples.first().unwrap();

// ✓ Лучше
let Some(first) = samples.first() else { continue };

3. Единственный unreachable!:

// types.rs:251
_ => unreachable!(),

Этот unreachable находится внутри exhaustive match на MatchOp::RegexMatch | MatchOp::RegexNoMatch, поэтому действительно недостижим. Однако для безопасности:

// ✓ Рекомендация — использовать match guard вместо nested match
match m.op {
    MatchOp::Equal => { ... }
    MatchOp::NotEqual => { ... }
    MatchOp::RegexMatch if !is_match => return Ok(false),
    MatchOp::RegexNoMatch if is_match => return Ok(false),
    MatchOp::RegexMatch | MatchOp::RegexNoMatch => {}
}

10. Качественная обработка ошибок

Текущее состояние

// types.rs:87-94
pub enum EvalError {
    Unsupported(&'static str),
    UnsupportedOwned(String),
    TypeError { expected: &'static str, got: &'static str },
}

Проблемы

1. Недостаточная детализация ошибок:

Err(EvalError::Unsupported("rate arity"))  // Какая arity ожидалась?

2. Нет контекста позиции:

// При ошибке eval неясно, какое выражение вызвало проблему

3. Нет цепочки ошибок:

// Потеря контекста при конвертации
re.map_err(|_| EvalError::Unsupported("invalid regex matcher"))
//          ↑ Оригинальная ошибка regex теряется

Рекомендации

1. Расширить EvalError:

pub enum EvalError {
    Unsupported { feature: &'static str, context: Option<String> },
    TypeError { expected: &'static str, got: &'static str, expr: Option<String> },
    ArityError { function: String, expected: usize, got: usize },
    RegexError { pattern: String, source: regex::Error },
    StorageError(Box<dyn std::error::Error + Send + Sync>),
}

2. Использовать thiserror crate:

use thiserror::Error;

#[derive(Error, Debug)]
pub enum EvalError {
    #[error("unsupported: {feature}")]
    Unsupported { feature: &'static str },

    #[error("type error: expected {expected}, got {got}")]
    TypeError { expected: &'static str, got: &'static str },

    #[error("invalid regex `{pattern}`: {source}")]
    RegexError { pattern: String, #[source] source: regex::Error },
}

3. Добавить span information:

pub struct EvalErrorWithSpan {
    pub error: EvalError,
    pub span: Option<Span>,  // Позиция в исходном запросе
}

11. Использование &str литералов

Найдено ~50 случаев использования строковых литералов в match/if

Категории:

1. Агрегации (критично — должен быть enum):

// aggregate.rs
match op {
    "topk" | "bottomk" => ...
    "sum" | "avg" => ...
    "min" | "max" => ...
}

2. Специальные имена меток (допустимо):

// Константы для метаданных
k == "__name__"
k == "__type__"
k == "__unit__"

Рекомендация: Вынести в константы:

pub const LABEL_NAME: &str = "__name__";
pub const LABEL_TYPE: &str = "__type__";
pub const LABEL_UNIT: &str = "__unit__";

3. Парсинг histogram (допустимо для DSL):

// histogram.rs
match key {
    "schema" => ...
    "sum" => ...
    "count" => ...
}

4. Парсинг специальных значений (допустимо):

match s {
    "NaN" | "nan" => Some(f64::NAN),
    "+Inf" | "Inf" => Some(f64::INFINITY),
}

Главная рекомендация: Создать enum AggregateOp

// Добавить в ast/mod.rs:
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum AggregateOp {
    Sum, Avg, Min, Max, Count,
    Stddev, Stdvar,
    Topk, Bottomk,
    Quantile, CountValues,
    Group, Limitk, LimitRatio,
}

impl AggregateOp {
    pub fn from_str(s: &str) -> Option<Self> {
        Some(match s.to_ascii_lowercase().as_str() {
            "sum" => Self::Sum,
            "avg" => Self::Avg,
            // ...
            _ => return None,
        })
    }
}

// Изменить Expr:
Aggregate {
    op: AggregateOp,  // вместо String
    ...
}

// Изменить aggregate.rs:
match op {
    AggregateOp::Topk | AggregateOp::Bottomk => ...
    AggregateOp::Sum | AggregateOp::Avg => ...
}

Преимущества:

  • Compile-time проверка
  • Exhaustive matching (компилятор покажет пропущенные cases)
  • Нет опечаток
  • IDE autocomplete
  • Лучшая производительность (нет string comparison)

Сводка рекомендаций по приоритету

# Задача Приоритет Сложность Влияние
1 Создать AggregateOp enum 🔴 Высокий Средняя Высокое
2 Заменить опасные unwrap() на expect() 🔴 Высокий Низкая Безопасность
3 Разбить rate.rs на модули 🟡 Средний Средняя Читаемость
4 Улучшить LRU кэш regex (VecDeque) 🟡 Средний Низкая Производительность
5 Вынести константы меток 🟢 Низкий Низкая Maintainability
6 Добавить документацию к pub функциям 🟢 Низкий Средняя DX
7 Расширить EvalError 🟡 Средний Средняя Отладка
8 Разбить histogram_fn.rs 🟢 Низкий Средняя Читаемость

Заключение

Проект имеет хорошую архитектуру после проведённого рефакторинга:

  • Dispatch table вместо if-chains для функций
  • Модульная структура
  • Regex кэширование
  • Правильная обработка Native Histograms

Главные проблемы:

  1. Aggregate.op: String — должен быть enum
  2. Много unwrap() без expect()
  3. Большие файлы (rate.rs: 1323 строки)
  4. Недостаточно детализированные ошибки

Код production-ready для MVP, но требует доработки для долгосрочной поддержки.

План устранения замечаний по ревью PromQL (metrics-promql)

Дата: 2025-12-25
Источники: review_claude.md, review_codex.md, review_gemini.md

Контекст и цель

promql/ — Rust-реализация PromQL (AST + парсер + in-memory eval), проверяемая вендоренным корпусом Prometheus promqltest.

Текущее состояние: promqltest проходит полностью (full eval).

Базовый инвариант проекта: promqltest должен оставаться зелёным (parse-only + full eval).

Примечание: проект внутренний, breaking changes допустимы (API/структуры можно менять, если это повышает качество и снижает техдолг).

Важно: выполнение этого плана начинается только после согласования. Правило на выполнение: каждый пункт → тесты → commit.

Обязательные проверки на каждом шаге

  • CARGO_TARGET_DIR=target_tmp cargo test --workspace
  • CARGO_TARGET_DIR=target_tmp cargo run -q -p metrics-promql-tests --bin promqltest_status (должно быть 1777/1777 PASS)

Сводка замечаний (консолидировано)

Parser/AST

  1. Структура файла парсера: promql/src/parser/ast_parser.rs слишком большой (сканер/лексика/грамматика смешаны) → вынести scanner слой и оставить грамматику в отдельном модуле.
  2. Защита от глубоких выражений: риск stack overflow на “патологически” вложенных запросах → лимит глубины и тест на глубину.
  3. Комментарии #: поддержать # ... до конца строки (вне строковых литералов).
  4. Типизация доменных сущностей: агрегаторы (и опционально builtins) как enum, а не строки.
  5. Тестовое покрытие edge-cases: расширить unit-тесты парсера (escape-последовательности, special floats, комментарии) и зафиксировать совместимость с Prometheus.
  6. (P2) Spans/line/col: улучшение диагностики ошибок парсинга.

Engine/eval

  1. Stringly-typed агрегаторы: Aggregate.op как строка тянет match "sum" | "avg" | ... и риск опечаток → AggregateOp enum.
  2. Возможные “тихие” потери данных: там, где Prometheus возвращает warnings/info, сейчас часто делается silent drop или Unsupported → нужен механизм диагностик (хотя бы в debug/расширенном API).
  3. Повторяющиеся шаблоны: распаковка Value/проверка arity размазана по функциям → вынести expect_* helpers и стандартизировать ошибки.
  4. Системные label-ключи как “магические строки”: __name__, job, instance, __type__, __unit__ встречаются повсеместно → вынести в константы.
  5. Дубли сортировок/нормализаций: много sort_by(labels) внутри функций, а затем сортировка/финализация ещё раз → один источник истины для сортировки.
  6. Паники/unwrap: убрать “голые” unwrap() из runtime-логики (заменить на expect("invariant: ...") или Result), чтобы будущие изменения не превращались в падения.
  7. (P2) NaN/Inf и диапазоны времени: централизовать float ordering (NaN-семантика для sort/min/max) и аккуратно проверить конвертации времени/диапазоны.

Performance/DevX

  1. Regex cache: текущая реализация bounded cache корректная, но неидеальна по асимптотике (O(n) операции) → улучшить структуру данных (без потери boundedness).
  2. label_replace regex: сейчас компилируется каждый вызов → кэш или переиспользование.
  3. Vector matching: сигнатуры лейблов вычисляются многократно → кэшировать/предвычислять сигнатуры на входных векторах.
  4. InMemoryStorage: полный перебор O(N) в select_series → (опционально) индекс по __name__ и/или по matcher-ключам.
  5. Аллокации/clone: в горячих местах много clone()/to_owned() → точечные оптимизации без потери детерминизма тестов.
  6. (P3) Документация API/контрактов: добавить rustdoc к ключевым типам/функциям, чтобы снизить стоимость поддержки.

Definition of Done (DoD)

  • Строковые литералы корректно сохраняют UTF‑8 (есть unit-тесты).
  • Разделены правила идентификаторов: метрика допускает :, label/function — нет (есть unit-тесты).
  • Функции вызываются через единый dispatcher (registry/table), без длинного if-chain.
  • Regex matcher компилируется с bounded cache.
  • engine/mod.rs разрезан на модули (types, storage, regex, time, vector_ops, aggregate, ...).
  • Парсер декомпозирован (scanner/lexer отдельно), добавлен лимит глубины, добавлены (минимум) комментарии #.
  • Расширены unit-тесты парсера на edge-cases (escape/special floats/комментарии), зафиксирована совместимость.
  • Агрегаторы типизированы: AggregateOp enum проходит через parser → AST → engine.
  • Введены константы системных label-ключей, устранены “магические строки” в engine.
  • Устранены двойные сортировки/нормализации (ровно один источник истины).
  • Большие файлы (functions/rate.rs, functions/histogram_fn.rs, parser/parser.rs) декомпозированы без изменения логики.
  • В runtime-коде нет “голых” unwrap() (только expect("invariant: ...") или обработка через Result).
  • Добавлен расширенный режим eval с диагностикой (warnings/info); при необходимости внесены breaking changes и обновлены тесты/обвязка.

План работ (инкрементально, с тестами и коммитами)

После согласования: каждый пункт → тесты → commit.

Фаза 0 — документация и “охранные” тесты (P0) (сделано)

  1. Добавить этот план в репозиторий (данный файл).
  2. Исправить ссылку на парсер в promql/README.md.

Фаза 1 — Parser: корректность и совместимость идентификаторов (P0–P1) (сделано)

  1. Починить UTF‑8 в parse_string_literal (promql/src/parser/scanner.rs):

    • копить raw bytes для обычных символов,
    • escape-символы добавлять как Unicode → UTF‑8 bytes,
    • в конце собирать String из bytes,
    • добавить unit-тест: "привет" в label matcher парсится без порчи.
  2. Разделить идентификаторы по контексту (promql/src/parser/scanner.rs):

    • parse_metric_identifier() допускает :,
    • parse_label_identifier() запрещает : (Prometheus-правило),
    • parse_function_identifier() запрещает : и используется там, где парсим имена функций в duration-expr,
    • применить к label matchers и спискам labels (by/on/ignoring/group_left/...),
    • добавить unit-тесты:
      • metric{a:b="x"} → ошибка,
      • sum by(a:b) (x) → ошибка,
      • :metric{job="x"} → ОК.

Фаза 2 — Engine: убрать “магические” элементы и дубли (P0) (сделано)

  1. Ввести CUSTOM_BUCKET_SCHEMA: i32 = -53 в promql/src/engine/histogram.rs + использовать везде вместо -53.
  2. Убрать дублирование scale_histogram/compact_histogram:
    • удалить локальные определения внутри аггрегатора (оставить одну реализацию),
    • убедиться, что поведение не меняется (promqltest зелёный).

Фаза 3 — Engine: диспетчер функций и модульность (P0–P1) (сделано)

  1. Вынести eval_call из promql/src/engine/mod.rs в отдельный модуль без изменения логики.

  2. Заменить if-chain в eval_call на dispatch table (P0):

    • FnImpl = fn(&[Expr], EvalContext) -> Result<Value, EvalError>,
    • static FUNCTIONS: OnceLock<HashMap<&'static str, FnImpl>>,
    • быстрый lookup по lowercase имени; fallback if-chain удалён.
  3. Разбить engine/functions.rs на модули (P1):

    • engine/functions/mod.rs + подмодули math.rs, trig.rs, time.rs, label.rs, range.rs, rate.rs, histogram_fn.rs, sort.rs, info.rs,
    • перемещение кода “без изменения логики”, затем локальные упрощения.
  4. постепенная нарезка engine/mod.rs (P1):

  • на первом проходе — только перемещения кода,
  • текущий прогресс:
    • engine/types.rs
    • engine/storage.rs
    • engine/regex.rs
    • engine/time.rs
    • engine/vector_ops.rs
    • engine/aggregate.rs
    • engine/selector.rs
    • engine/subquery.rs
    • engine/mod.rs уменьшен примерно до ~613 LOC (было ~2768 LOC до нарезки).

Фаза 4 — Parser: декомпозиция + устойчивость (P0–P1)

  1. Добавить поддержку комментариев # ... (вне строк):
  • изменить skip_ws/сканер так, чтобы # до конца строки считалось whitespace,
  • добавить unit-тесты на: 1 + 2 # comment и # full-line comment\nup.
  1. Ввести лимит глубины парсинга (защита от stack overflow):
  • MAX_PARSE_DEPTH (например 256/512),
  • инкремент на входе в рекурсивные точки (скобки/primary/expr),
  • при превышении — ParseError с понятным сообщением,
  • тест: искусственно сгенерированная строка с глубокой вложенностью → ошибка (без panic).
  1. Декомпозировать парсер на модули:
  • parser/scanner.rs: байтовый ввод, peek/next/skip_ws, комментарии, позиция,
  • parser/parser.rs: грамматика (recursive descent),
  • сохранить семантику и совместимость с тестовым корпусом (breaking changes API допустимы),
  • после разрезки — только механические перемещения, без логических изменений.
  1. Расширить unit-тесты парсера на edge-cases (без изменения логики парсера):
  • escape-последовательности (включая \\xNN, octal, \\u, \\U),
  • NaN/Inf/+Inf/-Inf (если поддерживаем в грамматике чисел),
  • # комментарии вне строк и внутри строковых литералов,
  • “мусорный ввод” → ошибка без panic (минимум smoke).

Фаза 5 — Engine: типизация доменных сущностей (P0–P1)

  1. Ввести AggregateOp enum в AST и протащить до eval:
  • добавить AggregateOp (варианты: sum/avg/min/max/count/stddev/stdvar/topk/bottomk/quantile/count_values/group/limitk/limit_ratio),
  • обновить Expr::Aggregate,
  • в парсере заменить is_aggregate_op(name: &str) -> bool на AggregateOp::from_str(...),
  • в engine заменить строковые match op { "sum" => ... } на match AggregateOp::Sum => ...,
  • добавить unit-тесты на парсинг агрегаторов (case-insensitive + отрицательные кейсы),
  • прогнать promqltest и зафиксировать совместимость.
  1. Типизировать builtins/имена функций:
  • BuiltinFn enum (и парсинг на enum), чтобы убрать аллокацию to_ascii_lowercase() в eval,
  • цель: уменьшить stringly-typed логику и аллокации на горячем пути.

Фаза 6 — Engine: константы, helpers, safety (P1)

  1. Централизовать системные label-ключи:
  • LABEL_NAME, LABEL_JOB, LABEL_INSTANCE, LABEL_TYPE, LABEL_UNIT, LABEL_LE,
  • заменить строковые литералы в engine/functions на константы,
  • добавить микро-тесты (если нужно) на is_meta_label/фильтрацию.
  1. Вынести повторяющиеся expect_* распаковки Value:
  • expect_scalar, expect_string, expect_instant_vector, expect_range_vector,
  • унифицировать сообщения об ошибках (TypeError, arity) и снизить копипаст.
  1. Устранить двойные сортировки/нормализации:
  • договориться: сортировка делается либо только в finalize_value, либо только на уровне функций,
  • по одному модулю убирать out.sort_by(...) там, где это безопасно,
  • метрика успеха: тесты зелёные, поведение не меняется.
  1. Аудит unwrap()/expect()/unreachable!() в runtime-логике:
  • заменить “голые” unwrap() на expect("invariant: ...") там, где инвариант реально гарантирован,
  • там, где инвариант не очевиден → вернуть EvalError/ParseError и добавить тест,
  • цель: исключить неявные падения при будущих рефакторингах.
  1. Уточнить и расширить EvalError/ParseError (используем thiserror и добавляем детальный контекст):
  • разделить “unsupported feature” vs “invalid input”,
  • добавить контекст (функция/агрегатор/индекс аргумента),
  • добавить более структурные причины (InvalidRegex, ZeroDivision, StorageError, ...),
  • line/col + span в ParseError/EvalError.
  1. Pretty-printer + round-trip тесты (P3).

Фаза 7 — Декомпозиция больших файлов (P1–P2)

  1. Разбить promql/src/engine/functions/rate/mod.rs:
  • rate/mod.rs как фасад,
  • подмодули: rate/float.rs, rate/histogram.rs, rate/extrapolation.rs + тематические (rate_eval.rs, increase.rs, delta.rs, instant.rs, resets.rs, predict.rs),
  • цель: вынести общие helpers (extrapolation, reset detection) и убрать дубли.
  1. Разбить promql/src/engine/functions/histogram_fn/mod.rs:
  • histogram_fn/mod.rs как фасад,
  • подмодули: histogram_fn/stats.rs, histogram_fn/quantile.rs, histogram_fn/fraction.rs (+ общие helpers в histogram_fn/buckets.rs),
  • цель: сделать файл ревью-пригодным и изолировать сложные куски.
  1. Разбить promql/src/engine/functions/range/mod.rs на тематические подмодули (over_time.rs, stats.rs, ts_of.rs, smoothing.rs).

  2. Разбить promql/src/engine/aggregate/mod.rs на подмодули:

  • выделить общие helpers (grouping key, grouping labels, sort/topk primitives),
  • вынести по файлам: “обычные” агрегаторы vs topk/bottomk vs histogram-specific ветки,
  • без изменения логики (только перемещения + минимальная чистка).
  1. Продолжить нарезку promql/src/engine/mod.rs:
  • вынести selectors (VectorSelector/MatrixSelector) в engine/selector.rs,
  • вынести subquery в engine/subquery.rs,
  • оставить engine/mod.rs фасадом (re-export + orchestration).

Фаза 8 — Performance/DevX (P2)

  1. Улучшить bounded regex cache в engine/regex.rs:
  • убрать очевидно O(n) операции (если где-то ещё остались),
  • рассмотреть VecDeque/IndexMap/lru crate (решение выбрать с учётом зависимостей и простоты),
  • добавить const REGEX_CACHE_SIZE вместо “магического” 128.
  1. Добавить кэширование regex в label_replace (отдельный bounded cache, т.к. паттерны другие).

  2. Кэшировать сигнатуры для vector matching (vector_ops.rs):

  • предвычислять (signature, sample) на входных векторах,
  • не вызывать vector_match_signature(...) внутри горячих циклов,
  • прогнать promqltest и убедиться, что детерминизм/порядок не сломались.
  1. (Опционально) Оптимизации аллокаций LabelSet:
  • LabelSet::from_sorted_unique(...) или builder,
  • точечные места с массовым to_owned()/clone().
  1. (Опционально) Централизовать float ordering (NaN/Inf) для sort/min/max:
  • один comparator (Prometheus-совместимый) и использование его в sort, topk/bottomk, min/max,
  • добавить targeted unit-тесты на NaN/Inf поведение.

Фаза 9 — Диагностика

  1. Добавить расширенный режим eval с диагностикой (warnings/info):
  • новый тип результата: EvalOutput { value: Value, warnings: Vec<EvalWarning> },
  • можно добавить новый API (eval_instant_with_warnings) или заменить существующий — приоритет promqltest + читаемость.
  1. (P3) Документировать API/контракты (rustdoc):
  • parse_promql, ключевые AST типы, eval_instant/контекст,
  • добавить “источники истины” (ссылки на Prometheus/promqltest) рядом с важными константами/семантикой.

Фаза 10 — (Опционально) Storage (P3)

  1. (Опционально) Индексация InMemoryStorage:
  • минимум: HashMap<metric_name, Vec<idx>> для ускорения select_series по metric,
  • аккуратно сохранить корректность при matchers без metric.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment