Skip to content

Commit 43e067a

Browse files
committed
feat: Broadcast adhoc dashboard filter conditions
1 parent bc503c1 commit 43e067a

26 files changed

Lines changed: 1198 additions & 102 deletions
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@hyperdx/common-utils': patch
3+
'@hyperdx/app': patch
4+
---
5+
6+
feat: Broadcast adhoc dashboard filter conditions

‎packages/app/src/DBDashboardPage.tsx‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ import {
3535
isTimeSeriesDisplayType,
3636
} from '@hyperdx/common-utils/dist/core/utils';
3737
import { getBlockingRequiredFilterNames } from '@hyperdx/common-utils/dist/dashboardFilterValues';
38+
import {
39+
getPromqlFilters,
40+
getSqlFilters,
41+
} from '@hyperdx/common-utils/dist/filters';
3842
import {
3943
displayTypeRequiresSource,
4044
isBuilderChartConfig,
@@ -51,12 +55,12 @@ import {
5155
import { isMissingFiltersMacro } from '@hyperdx/common-utils/dist/macros';
5256
import {
5357
AlertState,
58+
BroadcastFilter,
5459
ChartConfigWithDateRange,
5560
ChartVariable,
5661
DashboardContainer as DashboardContainerSchema,
5762
DashboardFilter,
5863
DisplayType,
59-
Filter,
6064
getSampleWeightExpression,
6165
isLogSource,
6266
isTraceSource,
@@ -471,7 +475,7 @@ const Tile = ({
471475
onSettled?: () => void;
472476
granularity: SQLInterval | undefined;
473477
onTimeRangeSelect: (start: Date, end: Date) => void;
474-
filters?: Filter[];
478+
filters?: BroadcastFilter[];
475479
variables?: ChartVariable[];
476480
/** The dashboard's required filters that have nothing selected. */
477481
unsatisfiedRequiredFilters?: DashboardFilter[];
@@ -654,6 +658,15 @@ const Tile = ({
654658
missingRequiredFilterNames.length > 0 &&
655659
displayTypeRequiresSource(chart.config.displayType);
656660

661+
const sqlFilters = useMemo(
662+
() => (filters ? getSqlFilters(filters) : undefined),
663+
[filters],
664+
);
665+
const promqlFilters = useMemo(
666+
() => (filters ? getPromqlFilters(filters) : undefined),
667+
[filters],
668+
);
669+
657670
useEffect(() => {
658671
if (isPromqlSavedChartConfig(chart.config)) {
659672
if (source != null) {
@@ -664,6 +677,7 @@ const Tile = ({
664677
dateRange,
665678
granularity,
666679
variables: tileVariables,
680+
filters: promqlFilters,
667681
});
668682
}
669683
return;
@@ -676,7 +690,7 @@ const Tile = ({
676690
...chart.config,
677691
dateRange,
678692
granularity,
679-
filters,
693+
filters: sqlFilters,
680694
variables: tileVariables,
681695
});
682696
} else if (source != null) {
@@ -695,7 +709,7 @@ const Tile = ({
695709
sampleWeightExpression: getSampleWeightExpression(source),
696710
dateRange,
697711
granularity,
698-
filters,
712+
filters: sqlFilters,
699713
variables: tileVariables,
700714
});
701715
}
@@ -736,13 +750,21 @@ const Tile = ({
736750
? source.bodyExpression
737751
: undefined,
738752
sampleWeightExpression: getSampleWeightExpression(source),
739-
filters,
753+
filters: sqlFilters,
740754
variables: tileVariables,
741755
metricTables: isMetricSource ? source.metricTables : undefined,
742756
});
743757
}
744758
}
745-
}, [source, chart, dateRange, granularity, filters, tileVariables]);
759+
}, [
760+
source,
761+
chart,
762+
dateRange,
763+
granularity,
764+
sqlFilters,
765+
promqlFilters,
766+
tileVariables,
767+
]);
746768

747769
const [hovered, setHovered] = useState(false);
748770

@@ -813,7 +835,7 @@ const Tile = ({
813835
where: builderConfig?.where,
814836
whereLanguage: builderConfig?.whereLanguage,
815837
select: builderConfig?.select,
816-
filters,
838+
filters: sqlFilters,
817839
},
818840
);
819841

@@ -823,10 +845,10 @@ const Tile = ({
823845
);
824846

825847
const filterWarning = useMemo(() => {
826-
const doFiltersExist = !!filters?.filter(
848+
const doFiltersExist = !!sqlFilters?.filter(
827849
f => (f.type === 'lucene' || f.type === 'sql') && f.condition.trim(),
828850
)?.length;
829-
const doLuceneFiltersExist = !!filters?.filter(
851+
const doLuceneFiltersExist = !!sqlFilters?.filter(
830852
f => f.type === 'lucene' && f.condition.trim(),
831853
)?.length;
832854

@@ -862,7 +884,7 @@ const Tile = ({
862884
<IconZoomExclamation size={16} color="var(--color-text-danger)" />
863885
</Tooltip>
864886
);
865-
}, [filters, queriedConfig, source]);
887+
}, [sqlFilters, queriedConfig, source]);
866888

867889
const replaySearchUrl = useMemo(() => {
868890
return buildDashboardReplaySearchUrl({
@@ -1656,7 +1678,7 @@ const EditTileModal = ({
16561678
isSaving?: boolean;
16571679
onSave: (chart: Tile) => void;
16581680
variables?: ChartVariable[];
1659-
getDashboardFilters: (sourceId: string | undefined) => Filter[];
1681+
getDashboardFilters: (sourceId: string | undefined) => BroadcastFilter[];
16601682
unsatisfiedRequiredFilters?: DashboardFilter[];
16611683
}) => {
16621684
const contextZIndex = useZIndex();
@@ -2435,7 +2457,7 @@ function DBDashboardPage({
24352457
// `sourceId`. Shared with the tile editor so its preview queries what the
24362458
// tile does.
24372459
const getTileFilters = useCallback(
2438-
(sourceId: string | undefined): Filter[] => [
2460+
(sourceId: string | undefined): BroadcastFilter[] => [
24392461
{
24402462
type: whereLanguage === 'sql' ? 'sql' : 'lucene',
24412463
condition: where,

‎packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useState } from 'react';
2+
import { isAdhocConditionSupported } from '@hyperdx/common-utils/dist/filters';
23
import {
34
ADHOC_FILTER_MAX_CONDITIONS,
45
AdhocDashboardFilter as AdhocDashboardFilterType,
@@ -103,6 +104,10 @@ export function AdhocDashboardFilter({
103104
operator={operator.label}
104105
value={condition.value}
105106
isExcluded={operator.isExcluded}
107+
isInvalid={
108+
!isAdhocConditionSupported(filter.sourceType, condition)
109+
}
110+
invalidReason={`Filter not applied: PromQL has no ${condition.operator} operator. Remove this condition and add it again with a supported operator.`}
106111
onRemove={() => remove(index)}
107112
renderPopover={close => (
108113
<AdhocConditionEditor

‎packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,14 @@ const editorId = 'adhoc-condition-editor-Conditions';
4040

4141
const addButton = () => screen.getByTestId('adhoc-filter-add-Conditions');
4242

43-
const renderFilter = (conditions: AdhocFilterCondition[]) => {
43+
const renderFilter = (
44+
conditions: AdhocFilterCondition[],
45+
definition: AdhocDashboardFilterType = filter,
46+
) => {
4447
const onChange = jest.fn();
4548
renderWithMantine(
4649
<AdhocDashboardFilter
47-
filter={filter}
50+
filter={definition}
4851
conditions={conditions}
4952
onChange={onChange}
5053
dateRange={dateRange}
@@ -74,6 +77,20 @@ describe('AdhocDashboardFilter', () => {
7477
expect(pills[1]).toHaveTextContent('Level regex ^err');
7578
});
7679

80+
it('marks conditions a PromQL filter cannot apply as invalid', () => {
81+
renderFilter(
82+
[
83+
{ key: 'job', operator: 'LIKE', value: '%api%' },
84+
{ key: 'env', operator: '=', value: 'prod' },
85+
],
86+
{ ...filter, sourceType: 'promql' },
87+
);
88+
89+
const pills = screen.getAllByTestId('adhoc-condition-pill');
90+
expect(pills[0]).toHaveAttribute('data-invalid', 'true');
91+
expect(pills[1]).not.toHaveAttribute('data-invalid');
92+
});
93+
7794
it('removes a condition', async () => {
7895
const onChange = renderFilter([
7996
existing,

‎packages/app/src/components/AdhocDashboardFilter/__tests__/useAdhocFilterOptions.test.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ describe('getSqlSourceColumnKeys', () => {
8686
]),
8787
).toEqual(['ServiceName']);
8888
});
89+
90+
it('quotes column names that are not bare identifiers', () => {
91+
expect(getSqlSourceColumnKeys([column('service-name', 'String')])).toEqual([
92+
'`service-name`',
93+
]);
94+
});
8995
});
9096

9197
describe('getValueSuggestions', () => {

‎packages/app/src/components/AdhocDashboardFilter/useAdhocFilterOptions.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,26 +45,37 @@ const unionSorted = (lists: (string[] | undefined)[]) =>
4545
Array.from(new Set(lists.flatMap(list => list ?? []))).sort();
4646

4747
/**
48-
* A SQL source's filterable keys, in the search sidebar's key format: scalar
49-
* columns, and the keys inside its map and JSON columns.
48+
* Quote a column name that isn't a bare identifier (e.g. `log-attributes`), so
49+
* the stored key is SQL that broadcasting can use verbatim.
50+
*/
51+
const toQuotedKeys = (keys: string[], columns: ColumnMeta[]) => {
52+
const columnNames = new Set(columns.map(column => column.name));
53+
return keys.map(key => toQuotedClickHouseKeyExpression(key, columnNames));
54+
};
55+
56+
/**
57+
* A SQL source's filterable keys, as SQL expressions in the search sidebar's
58+
* key format: scalar columns, and the keys inside its map and JSON columns.
5059
*/
5160
export function getSqlSourceKeys(fields: Field[], columns: ColumnMeta[]) {
5261
const columnNames = (types: JSDataType[]) =>
5362
filterColumnMetaByType(columns, types)?.map(column => column.name) ?? [];
5463
const jsonColumns = columnNames([JSDataType.JSON]);
5564
const mapColumns = columnNames([JSDataType.Map]);
56-
return fields
65+
const keys = fields
5766
.filter(field => field.path.length > 1 || isJSDataTypeScalar(field.jsType))
5867
.map(field => mergePath(field.path, jsonColumns, mapColumns));
68+
return toQuotedKeys(keys, columns);
5969
}
6070

6171
/** A SQL source's scalar columns, the only keys known without field metadata. */
6272
export function getSqlSourceColumnKeys(columns: ColumnMeta[]) {
63-
return columns
73+
const keys = columns
6474
.filter(column =>
6575
isJSDataTypeScalar(convertCHDataTypeToJSType(column.type)),
6676
)
6777
.map(column => column.name);
78+
return toQuotedKeys(keys, columns);
6879
}
6980

7081
/**

‎packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ import {
2222
isRawSqlSavedChartConfig,
2323
} from '@hyperdx/common-utils/dist/guards';
2424
import {
25+
BroadcastFilter,
2526
ChartConfigWithDateRange,
2627
ChartVariable,
2728
DashboardFilter,
2829
DisplayType,
29-
Filter,
3030
HeatmapMode,
3131
SavedChartConfig,
3232
SourceKind,
@@ -106,6 +106,7 @@ import {
106106
displayTypeToActiveTab,
107107
resolveTilePreviewFilters,
108108
TABS_WITH_GENERATED_SQL,
109+
withBroadcastFilters,
109110
zSavedChartConfig,
110111
} from './utils';
111112

@@ -115,7 +116,7 @@ type EditTimeChartFormProps = {
115116
/** Variables and their selected values, from the parent dashboard (if one exists). */
116117
variables?: ChartVariable[];
117118
/** Function returning the dashboard's broadcast filters for the given source, if any. */
118-
getDashboardFilters?: (sourceId: string | undefined) => Filter[];
119+
getDashboardFilters?: (sourceId: string | undefined) => BroadcastFilter[];
119120
/** The dashboard's required filters that have nothing selected, if any */
120121
unsatisfiedRequiredFilters?: DashboardFilter[];
121122
displayedTimeInputValue?: string;
@@ -524,8 +525,7 @@ export default function EditTimeChartForm({
524525
return queriedConfig;
525526
}
526527
return {
527-
...queriedConfig,
528-
filters: previewDashboardFilters?.filters,
528+
...withBroadcastFilters(queriedConfig, previewDashboardFilters?.filters),
529529
variables: previewDashboardFilters?.variables,
530530
};
531531
}, [queriedConfig, previewDashboardFilters]);

‎packages/app/src/components/DBEditTimeChartForm/__tests__/utils.test.ts‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import {
33
ChartVariable,
44
DashboardFilter,
55
DisplayType,
6+
Filter,
7+
PromqlConditionFilter,
68
PromqlExpressionList,
79
SavedChartConfig,
810
SourceKind,
@@ -23,6 +25,7 @@ import {
2325
seriesToFilters,
2426
tabQueriesData,
2527
TABS_WITH_GENERATED_SQL,
28+
withBroadcastFilters,
2629
} from '@/components/DBEditTimeChartForm/utils';
2730

2831
// ---------------------------------------------------------------------------
@@ -639,6 +642,10 @@ describe('resolveTilePreviewFilters', () => {
639642
{ type: 'sql' as const, condition: "ServiceName IN ('api')" },
640643
];
641644
const variables = [{ name: 'service', values: ['api'] }];
645+
const promqlFilter: PromqlConditionFilter = {
646+
type: 'promql',
647+
condition: 'job="api"',
648+
};
642649

643650
const promqlConfig: ChartConfigWithDateRange = {
644651
configType: 'promql',
@@ -696,8 +703,30 @@ describe('resolveTilePreviewFilters', () => {
696703
});
697704
});
698705

699-
it('never hands filters to a PromQL tile', () => {
700-
expect(resolve({ config: promqlConfig }).filters).toBeUndefined();
706+
it('never hands SQL filters to a PromQL tile', () => {
707+
expect(resolve({ config: promqlConfig }).filters).toEqual([]);
708+
});
709+
710+
it('hands each tile the filters in its own query language', () => {
711+
const filters = [...dashboardFilters, promqlFilter];
712+
713+
expect(resolve({ config: promqlConfig, filters }).filters).toEqual([
714+
promqlFilter,
715+
]);
716+
expect(resolve({ filters }).filters).toEqual(dashboardFilters);
717+
expect(
718+
resolve({ config: rawSqlWithFiltersMacro, filters }).filters,
719+
).toEqual(dashboardFilters);
720+
});
721+
722+
it('drops PromQL filters when turned off', () => {
723+
expect(
724+
resolve({
725+
config: promqlConfig,
726+
filters: [promqlFilter],
727+
applySelections: false,
728+
}).filters,
729+
).toBeUndefined();
701730
});
702731

703732
it('drops the filters a raw-SQL template would not apply', () => {
@@ -761,6 +790,32 @@ describe('resolveTilePreviewFilters', () => {
761790
});
762791
});
763792

793+
describe('withBroadcastFilters', () => {
794+
const sqlFilter: Filter = { type: 'sql', condition: "ServiceName = 'api'" };
795+
const promqlFilter: PromqlConditionFilter = {
796+
type: 'promql',
797+
condition: 'job="api"',
798+
};
799+
const promqlConfig: ChartConfigWithDateRange = {
800+
configType: 'promql',
801+
promqlExpression: 'up',
802+
connection: 'local',
803+
dateRange,
804+
};
805+
806+
it('attaches only PromQL filters to a PromQL config', () => {
807+
expect(
808+
withBroadcastFilters(promqlConfig, [sqlFilter, promqlFilter]),
809+
).toMatchObject({ filters: [promqlFilter] });
810+
});
811+
812+
it('attaches only SQL and Lucene filters to other configs', () => {
813+
expect(
814+
withBroadcastFilters(builderConfig, [sqlFilter, promqlFilter]),
815+
).toMatchObject({ filters: [sqlFilter] });
816+
});
817+
});
818+
764819
// ---------------------------------------------------------------------------
765820
// buildSampleEventsConfig
766821
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)