Merge ticket 055: make ottyl_* reserved params usable in dashboard PromQL queries
Resolved conflicts with ticket 051's unknown-metric guard in evaluate_promql (reordered so ottyl_fmt's categorical rejection and the reserved-name skip both run before 051's declaration/bucket-data check, since ottyl_seq/ottyl_ts are synthesized from the log index and never have a MetricDeclaration or bucket entries) and merged both tickets' additions to the reserved-params Docs.tsx paragraph. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o
This commit is contained in:
+52
-1
@@ -8,6 +8,7 @@ use crate::dto::{
|
||||
ParamDeclarationDto, LogsParams, ProfileSummaryDto, PromqlParams, SaveDashboardRequest,
|
||||
SettingsDto, TestAlertNotificationRequest, TimelineParams, UpdateAlertRuleRequest, UpdateSettingsRequest,
|
||||
};
|
||||
use crate::data_value::MetricPrimitiveKind;
|
||||
use crate::engine::{
|
||||
DeclareMetricRequest, DeclareParamRequest, EngineHandle, EngineMessage, EngineQuery,
|
||||
EngineResponse,
|
||||
@@ -312,12 +313,61 @@ async fn get_stats(State(state): State<ApiState>) -> impl IntoResponse {
|
||||
|
||||
async fn list_params(State(state): State<ApiState>) -> impl IntoResponse {
|
||||
match ask(&state.engine, EngineQuery::ParamDeclarations).await {
|
||||
Ok(EngineResponse::ParamDeclarations(declarations)) => Json(declarations).into_response(),
|
||||
Ok(EngineResponse::ParamDeclarations(mut declarations)) => {
|
||||
// `ottyl_seq`/`ottyl_ts`/`ottyl_fmt` (ticket 055) are computed line-locally rather than
|
||||
// stored as index `ParamDeclaration` entries (see `crate::engine::declare_param`'s
|
||||
// rejection of these names), so they're merged in here purely for discoverability
|
||||
// rather than via `param_declarations_range`. This is deliberately a merge only in this
|
||||
// one HTTP-facing listing, not in `EngineQuery::ParamDeclarations` itself — other
|
||||
// internal callers of that query (promoting a metric's referenced params, replaying
|
||||
// declarations on profile switch) must keep seeing only real, persistable entries.
|
||||
declarations.extend(reserved_param_declaration_dtos());
|
||||
Json(declarations).into_response()
|
||||
}
|
||||
Ok(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(),
|
||||
Err(status) => status.into_response(),
|
||||
}
|
||||
}
|
||||
|
||||
/// `GET /api/params`-only synthesized entries for the three reserved names
|
||||
/// (`crate::engine::OTTYL_RESERVED_PARAM_NAMES`) — see `list_params`. `message_match_regex`/
|
||||
/// `capture_group` are empty since these are never text-extracted; `reserved: true` marks them as
|
||||
/// non-deletable/non-editable, distinct from `auto_derived`.
|
||||
fn reserved_param_declaration_dtos() -> [ParamDeclarationDto; 3] {
|
||||
[
|
||||
ParamDeclarationDto {
|
||||
name: "ottyl_seq".to_string(),
|
||||
message_match_regex: String::new(),
|
||||
capture_group: String::new(),
|
||||
json_field_path: None,
|
||||
kind: MetricPrimitiveKind::Number,
|
||||
elapsed_since_last: false,
|
||||
auto_derived: false,
|
||||
reserved: true,
|
||||
},
|
||||
ParamDeclarationDto {
|
||||
name: "ottyl_ts".to_string(),
|
||||
message_match_regex: String::new(),
|
||||
capture_group: String::new(),
|
||||
json_field_path: None,
|
||||
kind: MetricPrimitiveKind::Timestamp,
|
||||
elapsed_since_last: false,
|
||||
auto_derived: false,
|
||||
reserved: true,
|
||||
},
|
||||
ParamDeclarationDto {
|
||||
name: "ottyl_fmt".to_string(),
|
||||
message_match_regex: String::new(),
|
||||
capture_group: String::new(),
|
||||
json_field_path: None,
|
||||
kind: MetricPrimitiveKind::String,
|
||||
elapsed_since_last: false,
|
||||
auto_derived: false,
|
||||
reserved: true,
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
async fn declare_param(
|
||||
State(state): State<ApiState>,
|
||||
Json(body): Json<DeclareParamRequest>,
|
||||
@@ -345,6 +395,7 @@ async fn declare_param(
|
||||
// A request that reached here was accepted by `declare_param`, which always stores
|
||||
// it as non-auto (see `engine::declare_param`).
|
||||
auto_derived: false,
|
||||
reserved: false,
|
||||
};
|
||||
(StatusCode::CREATED, Json(dto)).into_response()
|
||||
}
|
||||
|
||||
@@ -28,6 +28,14 @@ pub struct ParamDeclarationDto {
|
||||
/// See `crate::data_value::ParamDeclaration::auto_derived`. The frontend uses this to disable
|
||||
/// editing — `POST /api/params` also rejects a request naming an auto-derived param.
|
||||
pub auto_derived: bool,
|
||||
/// Set only for the built-in `ottyl_seq`/`ottyl_ts`/`ottyl_fmt` names
|
||||
/// (`crate::engine::OTTYL_RESERVED_PARAM_NAMES`), synthesized into the `GET /api/params`
|
||||
/// response by `crate::api::list_params` rather than backed by any index entry — distinct from
|
||||
/// `auto_derived` (which *is* a real, if machine-discovered, `ParamDeclaration`). Always
|
||||
/// `false` for every other entry. There is no removal path for these at all (not just a
|
||||
/// disabled one), so the frontend should treat `reserved` as non-deletable/non-editable.
|
||||
#[serde(default)]
|
||||
pub reserved: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize)]
|
||||
|
||||
+180
-15
@@ -249,6 +249,13 @@ fn declare_param(
|
||||
index: &mut BTreeMap<IndexKey, DataValue>,
|
||||
request: DeclareParamRequest,
|
||||
) -> Result<(), String> {
|
||||
if OTTYL_RESERVED_PARAM_NAMES.contains(&request.name.as_str()) {
|
||||
return Err(format!(
|
||||
"'{}' is a reserved name, present on every log line without declaring anything; \
|
||||
it can't be redeclared as a param",
|
||||
request.name
|
||||
));
|
||||
}
|
||||
if let Some(DataValue::ParamDeclaration(existing)) =
|
||||
index.get(&IndexKey::ParamDeclaration { name: request.name.clone() })
|
||||
{
|
||||
@@ -1498,6 +1505,7 @@ fn handle_query(
|
||||
kind,
|
||||
elapsed_since_last,
|
||||
auto_derived,
|
||||
reserved: false,
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
@@ -1703,6 +1711,24 @@ fn declared_value_kind(
|
||||
}
|
||||
}
|
||||
|
||||
/// The three reserved param names usable without declaring anything — see `compile_ottyl_seq_comparison`/
|
||||
/// `compile_ottyl_ts_comparison`/`compile_ottyl_fmt_comparison` for the filter-bar equivalent, and
|
||||
/// `webapp/src/components/Docs.tsx` for the user-facing docs. Shared by `evaluate_promql` (dashboard
|
||||
/// panels, this file) and `crate::api::list_params` (`GET /api/params` discoverability).
|
||||
pub const OTTYL_RESERVED_PARAM_NAMES: [&str; 3] = ["ottyl_seq", "ottyl_ts", "ottyl_fmt"];
|
||||
|
||||
/// For `ottyl_seq`/`ottyl_ts` only (not `ottyl_fmt`, which `evaluate_promql` rejects outright before
|
||||
/// this is consulted): returns a `(sequence, timestamp_ms) -> value` mapping used to synthesize a
|
||||
/// virtual PromQL series straight from the ingested-log index, since neither is backed by a
|
||||
/// `MetricDeclaration`/bucket the way every other metric name is.
|
||||
fn ottyl_reserved_numeric_value_fn(metric_name: &str) -> Option<fn(u64, i64) -> f64> {
|
||||
match metric_name {
|
||||
"ottyl_seq" => Some(|sequence, _timestamp_ms| sequence as f64),
|
||||
"ottyl_ts" => Some(|_sequence, timestamp_ms| timestamp_ms as f64),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Evaluates a parsed PromQL query against `index`, producing chart-ready series: baseline is one series per label combination; `sum()`/`avg()` merge series into one series per `by_labels` group (the whole result collapses to a single label-less series when `by_labels` is empty); `rate()` — bare or wrapped in `sum()`/`avg()` — turns each series into a windowed occurrences-per-second reading via `windowed_rate_series`, so it plots as a real line rather than one trailing point; `histogram()` groups by `by_labels` the same way sum()/avg() do, then buckets each group's values independently, so `histogram(m) by (label)` yields one histogram (a family of `le`-labeled series carrying the group's own labels) per label group instead of collapsing all groups together.
|
||||
/// `sum()`/`avg()` over a `String`-kind metric are rejected up front — `coerce_to_f64` has no
|
||||
/// numeric reading for a string, so silently treating every occurrence as `1.0` would let a sum
|
||||
@@ -1715,13 +1741,30 @@ fn evaluate_promql(
|
||||
start_ms: Option<i64>,
|
||||
end_ms: Option<i64>,
|
||||
) -> Result<Vec<PromqlSeriesDto>, String> {
|
||||
// `ottyl_fmt` is categorical ("json"/"logfmt"/"none", `crate::logfmt::detect_format`) with no
|
||||
// sensible numeric reading, unlike `ottyl_seq`/`ottyl_ts` below — rather than silently
|
||||
// returning an empty series (the failure mode ticket 055 calls out), a dashboard query over it
|
||||
// is rejected up front with a message pointing back at the filter/query bar, where it already
|
||||
// works (see `compile_ottyl_fmt_comparison`).
|
||||
if parsed.metric_name == "ottyl_fmt" {
|
||||
return Err(
|
||||
"'ottyl_fmt' is a categorical value (json/logfmt/none), not a number, so it can't be \
|
||||
used in a dashboard PromQL query; filter on it in the search bar instead, e.g. \
|
||||
ottyl_fmt==json"
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
|
||||
// Ticket 051: a metric with neither a declaration nor any recorded bucket data is genuinely
|
||||
// unknown (as opposed to a declared-but-empty-so-far metric, or an undeclared scraped
|
||||
// Prometheus metric ingested via `--metrics-url`'s `IngestMetricSample`, which never gets a
|
||||
// `MetricDeclaration` but still has bucket data) — most commonly because a dashboard panel
|
||||
// still references a metric that has since been deleted. Failing here with a clear error beats
|
||||
// a panic or a silently-empty result.
|
||||
if !index.contains_key(&IndexKey::MetricDeclaration { name: parsed.metric_name.clone() })
|
||||
// a panic or a silently-empty result. `ottyl_seq`/`ottyl_ts` (ticket 055) are exempt: they're
|
||||
// synthesized straight from the ingested-log index below and never have a `MetricDeclaration`
|
||||
// or bucket entries of their own.
|
||||
if ottyl_reserved_numeric_value_fn(&parsed.metric_name).is_none()
|
||||
&& !index.contains_key(&IndexKey::MetricDeclaration { name: parsed.metric_name.clone() })
|
||||
&& bucket_values(index, &parsed.metric_name, None).next().is_none()
|
||||
{
|
||||
return Err(format!(
|
||||
@@ -1740,19 +1783,41 @@ fn evaluate_promql(
|
||||
}
|
||||
|
||||
let mut series_points: BTreeMap<BTreeMap<String, String>, Vec<(i64, f64)>> = BTreeMap::new();
|
||||
for (labels, (_, timestamp, value)) in bucket_values(index, &parsed.metric_name, None)
|
||||
.filter(|(labels, _)| matches_label_matchers(labels, &parsed.matchers))
|
||||
.filter(|(_, (_, timestamp, _))| {
|
||||
start_ms.is_none_or(|start| timestamp.as_millis() >= start)
|
||||
&& end_ms.is_none_or(|end| timestamp.as_millis() <= end)
|
||||
})
|
||||
{
|
||||
let display_labels: BTreeMap<String, String> =
|
||||
labels.iter().map(|(name, value)| (name.clone(), value.to_string())).collect();
|
||||
series_points
|
||||
.entry(display_labels)
|
||||
.or_default()
|
||||
.push((timestamp.as_millis(), coerce_to_f64(value)));
|
||||
if let Some(reserved_value) = ottyl_reserved_numeric_value_fn(&parsed.metric_name) {
|
||||
// `ottyl_seq`/`ottyl_ts` are computed line-locally (see `compile_ottyl_seq_comparison`/
|
||||
// `compile_ottyl_ts_comparison`) rather than backed by a `MetricDeclaration` and bucket
|
||||
// entries, so they're synthesized here straight from the ingested-log index instead of
|
||||
// going through `bucket_values`. Neither carries any labels, so a non-empty matcher list
|
||||
// simply matches nothing, same as an unmatched label filter would against a real metric.
|
||||
let no_labels: BTreeMap<String, MetricPrimitive> = BTreeMap::new();
|
||||
if matches_label_matchers(&no_labels, &parsed.matchers) {
|
||||
for (sequence, timestamp, _text) in logs_range(index) {
|
||||
let timestamp_ms = timestamp.as_millis();
|
||||
if start_ms.is_none_or(|start| timestamp_ms >= start)
|
||||
&& end_ms.is_none_or(|end| timestamp_ms <= end)
|
||||
{
|
||||
series_points
|
||||
.entry(BTreeMap::new())
|
||||
.or_default()
|
||||
.push((timestamp_ms, reserved_value(sequence, timestamp_ms)));
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
for (labels, (_, timestamp, value)) in bucket_values(index, &parsed.metric_name, None)
|
||||
.filter(|(labels, _)| matches_label_matchers(labels, &parsed.matchers))
|
||||
.filter(|(_, (_, timestamp, _))| {
|
||||
start_ms.is_none_or(|start| timestamp.as_millis() >= start)
|
||||
&& end_ms.is_none_or(|end| timestamp.as_millis() <= end)
|
||||
})
|
||||
{
|
||||
let display_labels: BTreeMap<String, String> =
|
||||
labels.iter().map(|(name, value)| (name.clone(), value.to_string())).collect();
|
||||
series_points
|
||||
.entry(display_labels)
|
||||
.or_default()
|
||||
.push((timestamp.as_millis(), coerce_to_f64(value)));
|
||||
}
|
||||
}
|
||||
for points in series_points.values_mut() {
|
||||
points.sort_by_key(|(timestamp_ms, _)| *timestamp_ms);
|
||||
@@ -3382,6 +3447,106 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn promql_count_over_ottyl_seq_gives_one_point_per_ingested_log_with_no_metric_declaration() {
|
||||
let mut index = BTreeMap::new();
|
||||
ingest_log(&mut index, 0, "line one");
|
||||
ingest_log(&mut index, 1, "line two");
|
||||
ingest_log(&mut index, 2, "line three");
|
||||
|
||||
let parsed = promql::parse_query("count(ottyl_seq)").unwrap();
|
||||
let series = evaluate_promql(&index, &parsed, None, None).unwrap();
|
||||
assert_eq!(series.len(), 1);
|
||||
assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0), (2, 1.0)]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn promql_bare_ottyl_seq_returns_the_actual_sequence_numbers() {
|
||||
let mut index = BTreeMap::new();
|
||||
ingest_log(&mut index, 0, "line zero");
|
||||
ingest_log(&mut index, 5, "line five");
|
||||
|
||||
let parsed = promql::parse_query("ottyl_seq").unwrap();
|
||||
let mut series = evaluate_promql(&index, &parsed, None, None).unwrap();
|
||||
assert_eq!(series.len(), 1);
|
||||
series[0].points.sort_by_key(|(ts, _)| *ts);
|
||||
assert_eq!(series[0].points, vec![(0, 0.0), (5, 5.0)]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn promql_bare_ottyl_ts_returns_ingest_timestamps() {
|
||||
let mut index = BTreeMap::new();
|
||||
ingest_log(&mut index, 0, "a");
|
||||
ingest_log(&mut index, 1, "b");
|
||||
|
||||
let parsed = promql::parse_query("ottyl_ts").unwrap();
|
||||
let series = evaluate_promql(&index, &parsed, None, None).unwrap();
|
||||
assert_eq!(series.len(), 1);
|
||||
assert_eq!(series[0].points, vec![(0, 0.0), (1, 1.0)]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn promql_rate_over_ottyl_ts_produces_a_windowed_series_not_an_empty_one() {
|
||||
let mut index = BTreeMap::new();
|
||||
for sequence in 0..5u64 {
|
||||
ingest_log(&mut index, sequence, "line");
|
||||
}
|
||||
|
||||
let parsed = promql::parse_query("rate(ottyl_ts[5s])").unwrap();
|
||||
let series = evaluate_promql(&index, &parsed, None, None).unwrap();
|
||||
assert_eq!(series.len(), 1);
|
||||
assert!(!series[0].points.is_empty(), "rate(ottyl_ts[...]) must not silently return no data");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn promql_ottyl_seq_with_label_matcher_matches_nothing_since_it_has_no_labels() {
|
||||
let mut index = BTreeMap::new();
|
||||
ingest_log(&mut index, 0, "line");
|
||||
|
||||
let parsed = promql::parse_query(r#"ottyl_seq{path="/a"}"#).unwrap();
|
||||
let series = evaluate_promql(&index, &parsed, None, None).unwrap();
|
||||
assert!(series.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn promql_over_ottyl_fmt_is_explicitly_rejected_not_silently_empty() {
|
||||
let mut index = BTreeMap::new();
|
||||
ingest_log(&mut index, 0, r#"{"level":"error"}"#);
|
||||
|
||||
let parsed = promql::parse_query("count(ottyl_fmt)").unwrap();
|
||||
let err = evaluate_promql(&index, &parsed, None, None).unwrap_err();
|
||||
assert!(err.contains("ottyl_fmt"), "error should name the rejected metric: {err}");
|
||||
assert!(err.contains("categorical") || err.contains("filter"), "error should explain why: {err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn handle_query_promql_returns_series_for_ottyl_seq_end_to_end() {
|
||||
let mut index = BTreeMap::new();
|
||||
ingest_log(&mut index, 0, "a");
|
||||
ingest_log(&mut index, 1, "b");
|
||||
|
||||
let response = handle_query(
|
||||
&index,
|
||||
EngineQuery::Promql { query: "count(ottyl_seq)".to_string(), start_ms: None, end_ms: None },
|
||||
0,
|
||||
);
|
||||
match response {
|
||||
EngineResponse::Promql(Ok(series)) => {
|
||||
assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0)]);
|
||||
}
|
||||
_ => panic!("expected EngineResponse::Promql(Ok(_))"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn declare_param_rejects_reserved_ottyl_names() {
|
||||
let mut index = BTreeMap::new();
|
||||
for name in OTTYL_RESERVED_PARAM_NAMES {
|
||||
let err = declare_param(&mut index, request(name, "x", "x")).unwrap_err();
|
||||
assert!(err.contains("reserved"), "expected a 'reserved' error for '{name}', got: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
// -- FilterBar query integration --
|
||||
|
||||
fn logs_query(filter: Option<&str>) -> EngineQuery {
|
||||
|
||||
Reference in New Issue
Block a user