Skip to content

Commit 6d40435

Browse files
authored
Merge pull request #61 from VibeEngineeringAI/upstream/promql-filters-1830
feat: save PromQL queries in filters (port upstream parseablehq#1830)
2 parents 6aaee1d + b1be0a8 commit 6d40435

8 files changed

Lines changed: 124 additions & 21 deletions

File tree

‎frontend/src/features/logs/SavedViews.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ describe('server saved views', () => {
8787
]);
8888
expect(available.map(({ decoded }) => decoded?.dataset)).toEqual(['app', 'app', undefined]);
8989
expect(savedLogViewsForDataset([canonical], 'sql')).toEqual([]);
90+
const promql = {
91+
...classic,
92+
filter_name: 'Request rate',
93+
query: { filter_type: 'promql' as const, filter_query: 'rate(requests_total[5m])' },
94+
};
95+
expect(savedLogViewsForDataset([promql], 'app')).toEqual([]);
9096
});
9197
it('rejects classic formats and mismatched dataset metadata', () => {
9298
const { explorer_state: _state, ...classic } = savedLogViewBody(state, 'Classic');

‎frontend/src/features/logs/SavedViews.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,11 @@ export function savedLogViewsForDataset(views: SavedLogView[], dataset: string)
2525
return decoded.dataset === dataset ? [{ view, decoded }] : [];
2626
} catch {
2727
// Classic dataset-scoped views remain visible, but canonical views need a
28-
// valid explorer state to establish which dataset they belong to.
29-
return view.stream_name !== 'sql' && view.stream_name === dataset
28+
// valid explorer state to establish which dataset they belong to. Saved
29+
// PromQL queries share the endpoint but are not log views.
30+
return view.query.filter_type !== 'promql' &&
31+
view.stream_name !== 'sql' &&
32+
view.stream_name === dataset
3033
? [{ view, decoded: undefined }]
3134
: [];
3235
}

‎frontend/src/features/sql/serverSavedQueries.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,17 @@ const classicSearch: SavedLogView = {
9292
};
9393

9494
describe('server saved SQL queries', () => {
95+
it('ignores saved PromQL filters', () => {
96+
const promql: SavedLogView = {
97+
...classicSearch,
98+
filter_id: '01HPROMQL',
99+
stream_name: 'otel_metrics',
100+
query: { filter_type: 'promql', filter_query: 'sum(rate(http_requests_total[5m]))' },
101+
};
102+
expect(savedLogView(promql)).toBe(true);
103+
expect(serverSavedQuery(promql)).toBeUndefined();
104+
expect(serverSavedQueries([promql])).toEqual([]);
105+
});
95106
it('accepts the real classic fixtures through the client contract', () => {
96107
for (const view of [classicSql, classicLogsSql, nativeLogsSql, classicBuilder, classicSearch])
97108
expect(savedLogView(view)).toBe(true);

‎frontend/src/lib/logsContract.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,17 @@ describe('saved view HTTP contract', () => {
106106
: client.convertLogViewToSql('id'),
107107
).rejects.toBeInstanceOf(ApiError);
108108
});
109+
it('accepts saved PromQL filters in the shared list without failing log views', async () => {
110+
const promql: SavedLogView = {
111+
...body,
112+
filter_id: 'promql-id',
113+
stream_name: 'otel_metrics',
114+
query: { filter_type: 'promql', filter_query: 'sum(rate(http_requests_total[5m]))' },
115+
};
116+
const saved = { ...body, filter_id: 'server-id' };
117+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue(json([saved, promql])));
118+
expect(await createClient({ mode: 'live' }).listLogViews()).toEqual([saved, promql]);
119+
});
109120
it('validates optional explorer state while retaining classic metadata', () => {
110121
for (const explorer_state of [undefined, null, 'encoded'])
111122
expect(savedLogView({ ...body, explorer_state, classic_metadata: { anything: true } })).toBe(

‎frontend/src/lib/logsContract.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export function savedLogView(value: unknown): value is SavedLogView {
4040
optionalString(value.explorer_state) &&
4141
object(value.query) &&
4242
typeof value.query.filter_type === 'string' &&
43-
['filter', 'sql', 'search'].includes(value.query.filter_type) &&
43+
['filter', 'sql', 'search', 'promql'].includes(value.query.filter_type) &&
4444
optionalString(value.query.filter_query) &&
4545
(value.query.filter_builder == null || builder(value.query.filter_builder)) &&
4646
(value.time_filter == null ||

‎frontend/src/lib/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ export type SavedLogView = {
130130
filter_name: string;
131131
stream_name: string;
132132
query: {
133-
filter_type: 'filter' | 'sql' | 'search';
133+
filter_type: 'filter' | 'sql' | 'search' | 'promql';
134134
filter_query?: string | null;
135135
filter_builder?: SavedLogViewBuilder | null;
136136
};

‎src/handlers/http/users/filters.rs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ mod tests {
306306
assert_eq!(converted, once);
307307
converted.stream_name = legacy.stream_name.clone();
308308
assert_eq!(converted, legacy);
309-
for kind in [FilterType::Filter, FilterType::Search] {
309+
for kind in [FilterType::Filter, FilterType::Search, FilterType::Promql] {
310310
converted.query.filter_type = kind;
311311
let unchanged = converted.clone();
312312
assert!(super::convert_filter_to_sql(&mut converted).is_err());
@@ -333,6 +333,16 @@ mod tests {
333333
assert!(validate_stream_name(SQL_FILTER_STREAM_NAME, &FilterType::Search).is_err());
334334
}
335335

336+
#[test]
337+
fn promql_filters_require_a_concrete_dataset_name() {
338+
// PromQL evaluation is scoped to one dataset, and listing authorizes
339+
// the saved filter by that dataset, so the SQL sentinel is not valid.
340+
assert!(validate_stream_name("otel_metrics", &FilterType::Promql).is_ok());
341+
for stream_name in [SQL_FILTER_STREAM_NAME, "", "team1/serviceA", "../metrics"] {
342+
assert!(validate_stream_name(stream_name, &FilterType::Promql).is_err());
343+
}
344+
}
345+
336346
#[test]
337347
fn rejects_path_like_stream_names() {
338348
let stream_name = "team1/serviceA".to_owned();

‎src/users/filters.rs‎

Lines changed: 78 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ pub enum FilterType {
8181
Filter,
8282
SQL,
8383
Search,
84+
Promql,
8485
}
8586

8687
impl FilterType {
@@ -89,6 +90,7 @@ impl FilterType {
8990
FilterType::Filter => "filter",
9091
FilterType::SQL => "sql",
9192
FilterType::Search => "search",
93+
FilterType::Promql => "promql",
9294
}
9395
}
9496
}
@@ -190,23 +192,26 @@ impl Filters {
190192
let query: &str = f.query.filter_query.as_deref().unwrap_or("");
191193
let filter_type = &f.query.filter_type;
192194

193-
// if filter type is SQL, check if the user has access to the dataset based on the query string
194-
// if filter type is search or filter, check if the user has access to the dataset based on the dataset name
195-
if *filter_type == FilterType::SQL {
196-
if (user_auth_for_query(key, query).await).is_ok() {
197-
_filters.push(f.clone())
195+
match filter_type {
196+
// SQL queries can reference multiple datasets, so authorize the query itself.
197+
FilterType::SQL => {
198+
if (user_auth_for_query(key, query).await).is_ok() {
199+
_filters.push(f.clone())
200+
}
198201
}
199-
} else if *filter_type == FilterType::Search || *filter_type == FilterType::Filter {
200-
let dataset_name = &f.stream_name;
201-
if user_auth_for_datasets(
202-
&permissions,
203-
std::slice::from_ref(dataset_name),
204-
&tenant_id,
205-
)
206-
.await
207-
.is_ok()
208-
{
209-
_filters.push(f.clone())
202+
// Other filter types are scoped by the saved filter's dataset.
203+
FilterType::Search | FilterType::Filter | FilterType::Promql => {
204+
let dataset_name = &f.stream_name;
205+
if user_auth_for_datasets(
206+
&permissions,
207+
std::slice::from_ref(dataset_name),
208+
&tenant_id,
209+
)
210+
.await
211+
.is_ok()
212+
{
213+
_filters.push(f.clone())
214+
}
210215
}
211216
}
212217
}
@@ -247,6 +252,63 @@ mod tests {
247252
})).unwrap()
248253
}
249254

255+
fn promql_filter(stream_name: &str) -> Filter {
256+
serde_json::from_value(serde_json::json!({
257+
"version":"v2", "user_id":"owner", "filter_id":format!("promql-{stream_name}"),
258+
"stream_name":stream_name, "filter_name":"Request rate", "tenant_id":null,
259+
"query":{"filter_type":"promql","filter_query":"sum(rate(http_requests_total[5m]))","filter_builder":null},
260+
"time_filter":null
261+
})).unwrap()
262+
}
263+
264+
// `rbac::map::init` needs `PARSEABLE`, so tests create the maps directly.
265+
// A user without groups never reads `ROLES`, which other tests seed.
266+
fn session_with(permissions: Vec<crate::rbac::role::Permission>) -> SessionKey {
267+
use crate::rbac::map::{SESSIONS, Sessions, USER_GROUPS, USERS, UserGroups};
268+
USERS.get_or_init(|| parking_lot::RwLock::new(crate::rbac::map::Users::default()));
269+
SESSIONS.get_or_init(|| parking_lot::RwLock::new(Sessions::default()));
270+
USER_GROUPS.get_or_init(|| parking_lot::RwLock::new(UserGroups::default()));
271+
let key = SessionKey::SessionId(ulid::Ulid::new());
272+
crate::rbac::map::mut_sessions().track_new(
273+
format!("user-{key:?}"),
274+
key.clone(),
275+
chrono::DateTime::<chrono::Utc>::MAX_UTC,
276+
permissions,
277+
&None,
278+
);
279+
key
280+
}
281+
282+
#[test]
283+
fn promql_filter_type_round_trips_as_lowercase() {
284+
let filter = promql_filter("otel_metrics");
285+
assert_eq!(filter.query.filter_type, FilterType::Promql);
286+
assert_eq!(filter.query.filter_type.to_str(), "promql");
287+
let value = serde_json::to_value(&filter).unwrap();
288+
assert_eq!(value["query"]["filter_type"], "promql");
289+
assert_eq!(serde_json::from_value::<Filter>(value).unwrap(), filter);
290+
}
291+
292+
#[tokio::test]
293+
async fn promql_filters_are_authorized_by_dataset_not_parsed_as_sql() {
294+
use crate::rbac::role::{Action, Permission};
295+
let cache = Filters::default();
296+
let filter = promql_filter("otel_metrics");
297+
cache.update(&filter, &None).await;
298+
299+
// PromQL is not SQL: routing it through SQL table extraction would
300+
// fail and silently hide the filter from authorized users.
301+
let reader = session_with(vec![Permission::Resource(Action::Query, None)]);
302+
assert_eq!(cache.list_filters(&reader).await, vec![filter]);
303+
304+
let unauthorized = session_with(vec![]);
305+
assert!(cache.list_filters(&unauthorized).await.is_empty());
306+
307+
for key in [reader, unauthorized] {
308+
crate::rbac::map::mut_sessions().remove_session(&key);
309+
}
310+
}
311+
250312
#[tokio::test]
251313
async fn saved_filter_cache_prefers_administrators_own_copy_of_shared_id() {
252314
let cache = Filters::default();

0 commit comments

Comments
 (0)