From e182a5c655ebb03fb5e85af58f586407f59a3c73 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Fri, 13 Nov 2026 14:28:05 -0500 Subject: [PATCH] feat: let tiles and filters use a data source variable as their source --- .changeset/source-vars-4.md | 5 + packages/app/src/DBDashboardImportPage.tsx | 60 +++++++-- packages/app/src/DBDashboardPage.tsx | 8 +- .../__tests__/DBDashboardImportPage.test.tsx | 54 ++++++++ .../ChartEditor/PromqlChartEditor.tsx | 4 +- .../ChartEditor/RawSqlChartEditor.tsx | 6 +- .../ChartEditor/__tests__/utils.test.ts | 84 ++++++++++++ .../app/src/components/ChartEditor/utils.ts | 79 ++++++++--- .../ChartEditorControls.tsx | 1 + .../DBEditTimeChartForm/ChartPreviewPanel.tsx | 2 +- .../DBEditTimeChartForm/EditTimeChartForm.tsx | 127 ++++++++++++++++-- .../__tests__/ChartPreviewPanel.test.tsx | 4 +- .../components/DBEditTimeChartForm/utils.ts | 4 + .../AdhocFilterEditForm.tsx | 15 +++ .../DataSourceFilterEditForm.tsx | 23 +++- .../PromqlLabelFilterEditForm.tsx | 4 +- .../QueryExpressionFilterEditForm.tsx | 5 +- .../DataSourceFilterEditForm.test.tsx | 38 ++++++ .../app/src/components/SourceMultiSelect.tsx | 14 +- packages/app/src/components/SourceSelect.tsx | 25 ++-- .../__tests__/SourceSelect.test.tsx | 68 +++++++++- .../app/src/components/sourceSelectUtils.tsx | 109 ++++++++++++++- .../dashboard-source-variables.spec.ts | 94 ++++++++++++- .../dashboard-tile-editor-filters.spec.ts | 2 +- .../tests/e2e/page-objects/DashboardPage.ts | 6 + 25 files changed, 760 insertions(+), 81 deletions(-) create mode 100644 .changeset/source-vars-4.md create mode 100644 packages/app/src/components/DashboardFiltersModal/__tests__/DataSourceFilterEditForm.test.tsx diff --git a/.changeset/source-vars-4.md b/.changeset/source-vars-4.md new file mode 100644 index 0000000000..312539ba46 --- /dev/null +++ b/.changeset/source-vars-4.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/app': minor +--- + +feat: let tiles and filters use a data source variable as their source diff --git a/packages/app/src/DBDashboardImportPage.tsx b/packages/app/src/DBDashboardImportPage.tsx index 71fce453f0..eff35446bf 100644 --- a/packages/app/src/DBDashboardImportPage.tsx +++ b/packages/app/src/DBDashboardImportPage.tsx @@ -33,6 +33,7 @@ import { displayTypeRequiresSource, isRawSqlSavedChartConfig, } from '@hyperdx/common-utils/dist/guards'; +import { parseVariableReference } from '@hyperdx/common-utils/dist/referenceVariables'; import { type DashboardFilter, type DashboardTemplate, @@ -390,11 +391,25 @@ function resolveAppliesToSources( return { ids: slots.filter(Boolean), slots }; } +/** + * A data source variable reference (`$name`) is portable as written, so + * import neither maps it nor asks the user to. + */ +const isVariableRef = (name: string | undefined) => + parseVariableReference(name) != null; + +const sourceVariableRefs = (names: string[] | undefined) => + names?.filter(isVariableRef) ?? []; + +const mappableSourceNames = (names: string[] | undefined) => + names?.filter(name => !isVariableRef(name)); + /** Whether the filter names a single source that import must map. */ const isSourceBackedTemplateFilter = ( filter: DashboardFilter, ): filter is QueryExpressionDashboardFilter | PromqlLabelDashboardFilter => - isQueryExpressionFilter(filter) || isPrometheusLabelFilter(filter); + (isQueryExpressionFilter(filter) || isPrometheusLabelFilter(filter)) && + !isVariableRef(filter.source); /** Returns the filter if it names a source, or `undefined` if it doesn't. */ const sourceBackedTemplateFilter = (filter: DashboardFilter | undefined) => @@ -487,6 +502,7 @@ export function buildMappingFormSchema( // meaningful source even when the template names one. if ( config.source && + !isVariableRef(config.source) && displayTypeRequiresSource(config.displayType) && !data.tileSourceMappings[idx] ) { @@ -523,7 +539,10 @@ export function buildMappingFormSchema( const mappedSourceIds = data.filterAdhocSourceMappings?.[idx]?.filter( id => !!id, ); - if (!mappedSourceIds?.length) { + if ( + !mappedSourceIds?.length && + sourceVariableRefs(filter.sources).length === 0 + ) { ctx.addIssue({ code: z.ZodIssueCode.custom, path: ['filterAdhocSourceMappings', idx], @@ -624,7 +643,7 @@ export function Mapping({ input }: { input: DashboardTemplate }) { const tileSourceMappings = input.tiles.map(tile => { const config = tile.config as SavedChartConfig; - if (!config.source) return ''; + if (!config.source || isVariableRef(config.source)) return ''; const match = sources.find( source => source.name.toLowerCase() === config.source!.toLowerCase(), ); @@ -1042,15 +1061,24 @@ export function Mapping({ input }: { input: DashboardTemplate }) { // Zip the source mappings with the input filters const zippedFilters = input.filters?.map((filter, idx) => { - const appliesTo = data.filterAppliesToSourceMappings?.[idx]?.filter( - id => !!id?.length, - ); - const appliesToSourceIds = appliesTo?.length ? appliesTo : undefined; + // Data source variable references carry over unmapped + const appliesTo = [ + ...sourceVariableRefs( + appliesToTemplateFilter(filter)?.appliesToSourceIds, + ), + ...(data.filterAppliesToSourceMappings?.[idx]?.filter( + id => !!id?.length, + ) ?? []), + ]; + const appliesToSourceIds = appliesTo.length ? appliesTo : undefined; if (isAdhocFilter(filter)) { return { ...filter, - sources: - data.filterAdhocSourceMappings?.[idx]?.filter(id => !!id) ?? [], + sources: [ + ...sourceVariableRefs(filter.sources), + ...(data.filterAdhocSourceMappings?.[idx]?.filter(id => !!id) ?? + []), + ], appliesToSourceIds, }; } @@ -1145,7 +1173,7 @@ export function Mapping({ input }: { input: DashboardTemplate }) { return ( {/** Mapping for the tile's source, if one exists (they're optional for raw sql tiles) */} - {tile.config.source && ( + {tile.config.source && !isVariableRef(tile.config.source) && ( {tile.config.name} Data source @@ -1228,8 +1256,12 @@ export function Mapping({ input }: { input: DashboardTemplate }) { {/** Map filter sources */} {input.filters?.map((filter, i) => { - const appliesToSourceIds = - appliesToTemplateFilter(filter)?.appliesToSourceIds; + const appliesToSourceIds = mappableSourceNames( + appliesToTemplateFilter(filter)?.appliesToSourceIds, + ); + const adhocSourceNames = isAdhocFilter(filter) + ? mappableSourceNames(filter.sources) + : undefined; return ( {isSourceBackedTemplateFilter(filter) && ( @@ -1256,11 +1288,11 @@ export function Mapping({ input }: { input: DashboardTemplate }) { )} - {isAdhocFilter(filter) && ( + {isAdhocFilter(filter) && !!adhocSourceNames?.length && ( {filter.name} (Filter) Data sources - {filter.sources.join(', ')} + {adhocSourceNames.join(', ')} ` return shallow and prevents the tile-grid tree below // from being wrapped in an extra indentation level. const dashboardBody = ( - <> + {title} @@ -3580,7 +3584,7 @@ function DBDashboardPage({ variables={variables} /> )} - + ); return ( diff --git a/packages/app/src/__tests__/DBDashboardImportPage.test.tsx b/packages/app/src/__tests__/DBDashboardImportPage.test.tsx index fee5937fd7..3003cbc402 100644 --- a/packages/app/src/__tests__/DBDashboardImportPage.test.tsx +++ b/packages/app/src/__tests__/DBDashboardImportPage.test.tsx @@ -666,3 +666,57 @@ describe('Dashboard import - ad hoc filter sources', () => { expect(issuesFor(['src-logs'])).toBeUndefined(); }); }); + +describe('Dashboard import - data source variables', () => { + const sourceVariableTemplate = DashboardTemplateSchema.parse({ + version: '0.1.0', + name: 'Source Variables', + tiles: [builderTile('tile-line', 'line', '$logs', 0)], + filters: [ + { + id: 'ds', + type: 'DATA_SOURCE', + name: 'Logs source', + variableName: 'logs', + allowedSourceKinds: [SourceKind.Log], + minSelections: 1, + maxSelections: 1, + isBroadcastEnabled: false, + isVariableEnabled: true, + }, + { + id: 'svc', + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'ServiceName', + source: '$logs', + }, + { + id: 'adhoc', + type: 'ADHOC', + name: 'Conditions', + sourceType: 'sql', + sources: ['$logs', 'Traces'], + isVariableEnabled: true, + }, + ], + }); + + it('keeps references as written without asking for a mapping', async () => { + renderWithMantine(); + + const finish = await screen.findByRole('button', { + name: /finish import/i, + }); + expect(screen.queryByText('$logs')).not.toBeInTheDocument(); + await act(async () => { + fireEvent.click(finish); + }); + + await waitFor(() => expect(mockMutateAsync).toHaveBeenCalledTimes(1)); + const payload = mockMutateAsync.mock.calls[0][0]; + expect(payload.tiles[0].config.source).toBe('$logs'); + expect(payload.filters[1].source).toBe('$logs'); + expect(payload.filters[2].sources).toEqual(['$logs', 'src-traces']); + }); +}); diff --git a/packages/app/src/components/ChartEditor/PromqlChartEditor.tsx b/packages/app/src/components/ChartEditor/PromqlChartEditor.tsx index 838e3ec737..9a7942579e 100644 --- a/packages/app/src/components/ChartEditor/PromqlChartEditor.tsx +++ b/packages/app/src/components/ChartEditor/PromqlChartEditor.tsx @@ -21,6 +21,7 @@ import { IconCirclePlus } from '@tabler/icons-react'; import { HeatmapModeControl } from '@/components/DBEditTimeChartForm/HeatmapModeControl'; import { SourceSelectControlled } from '@/components/SourceSelect'; +import { useResolvedReference } from '@/components/sourceSelectUtils'; import { usePromqlMetricNames } from '@/hooks/usePromqlMetadata'; import { useSource } from '@/source'; @@ -80,7 +81,7 @@ export default function PromqlChartEditor({ ); }, []); - const sourceId = useWatch({ control, name: 'source' }); + const sourceId = useResolvedReference(useWatch({ control, name: 'source' })); const displayType = useWatch({ control, name: 'displayType' }); const { data: source } = useSource({ id: sourceId }); // The form can still hold a non-PromQL source right after switching a tile @@ -107,6 +108,7 @@ export default function PromqlChartEditor({ name="source" data-testid="source-selector" allowedSourceKinds={allowedSourceKinds} + allowSourceVariables /> {displayType === DisplayType.Heatmap && ( diff --git a/packages/app/src/components/ChartEditor/RawSqlChartEditor.tsx b/packages/app/src/components/ChartEditor/RawSqlChartEditor.tsx index ea2304d038..5019732ec9 100644 --- a/packages/app/src/components/ChartEditor/RawSqlChartEditor.tsx +++ b/packages/app/src/components/ChartEditor/RawSqlChartEditor.tsx @@ -41,6 +41,7 @@ import SourceSchemaPreview, { isSourceSchemaPreviewEnabled, } from '@/components/SourceSchemaPreview'; import { SourceSelectControlled } from '@/components/SourceSelect'; +import { useResolvedReference } from '@/components/sourceSelectUtils'; import { SQLEditorControlled } from '@/components/SQLEditor/SQLEditor'; import { type SQLCompletion } from '@/components/SQLEditor/utils'; import { IS_LOCAL_MODE } from '@/config'; @@ -148,7 +149,9 @@ export default function RawSqlChartEditor({ const displayType = useWatch({ control, name: 'displayType' }); const connection = useWatch({ control, name: 'connection' }); - const source = useWatch({ control, name: 'source' }); + // A data source variable reference syncs and expands macros as the source + // it currently selects. + const source = useResolvedReference(useWatch({ control, name: 'source' })); const sqlTemplate = useWatch({ control, name: 'sqlTemplate' }); const chartName = useWatch({ control, name: 'name' }); const sourceObject = sources?.find(s => s.id === source); @@ -293,6 +296,7 @@ export default function RawSqlChartEditor({ name="source" data-testid="source-selector" allowedSourceKinds={allowedSourceKinds} + allowSourceVariables connectionId={connection} size="xs" clearable diff --git a/packages/app/src/components/ChartEditor/__tests__/utils.test.ts b/packages/app/src/components/ChartEditor/__tests__/utils.test.ts index 63f4e2ded3..24f910d616 100644 --- a/packages/app/src/components/ChartEditor/__tests__/utils.test.ts +++ b/packages/app/src/components/ChartEditor/__tests__/utils.test.ts @@ -19,6 +19,7 @@ import { convertFormStateToSavedChartConfig, convertSavedChartConfigToFormState, getAllowedSourceKinds, + getInterchangeableSourceKind, promqlExpressionPlaceholder, validateChartForm, } from '@/components/ChartEditor/utils'; @@ -2705,3 +2706,86 @@ describe('promqlExpressionPlaceholder', () => { ); }); }); + +describe('data source variable references', () => { + it('saves the reference rather than the source it resolves to', () => { + const form: ChartEditorFormState = { + displayType: DisplayType.Line, + source: '$logs', + series: [seriesItem], + }; + expect(convertFormStateToSavedChartConfig(form, logSource)).toMatchObject({ + source: '$logs', + }); + }); + + it('queries the resolved source for a raw SQL chart', () => { + const form: ChartEditorFormState = { + configType: 'sql', + displayType: DisplayType.Table, + source: '$logs', + connection: 'conn-1', + sqlTemplate: 'SELECT 1', + series: [], + }; + expect(convertFormStateToSavedChartConfig(form, logSource)).toMatchObject({ + source: '$logs', + }); + expect( + convertFormStateToChartConfig(form, dateRange, logSource), + ).toMatchObject({ source: logSource.id }); + }); + + it('saves a reference with no selected source when its kind is known', () => { + const form: ChartEditorFormState = { + displayType: DisplayType.Line, + source: '$logs', + series: [ + { ...seriesItem, metricName: 'cpu', metricType: MetricsDataType.Gauge }, + ], + }; + const saved = convertFormStateToSavedChartConfig( + form, + undefined, + SourceKind.Log, + ); + expect(saved).toMatchObject({ source: '$logs' }); + expect(saved).toMatchObject({ + select: [expect.not.objectContaining({ metricName: 'cpu' })], + }); + }); + + it('validates a reference with no selected source by its kind', () => { + const form: ChartEditorFormState = { + displayType: DisplayType.Line, + source: '$logs', + series: [seriesItem], + }; + expect( + validateChartForm(form, undefined, jest.fn(), { + sourceKind: SourceKind.Log, + }), + ).toEqual([]); + expect( + validateChartForm(form, undefined, jest.fn(), { + missingSourceMessage: 'Select a source for $logs', + }), + ).toEqual([{ path: 'source', message: 'Select a source for $logs' }]); + }); +}); + +describe('getInterchangeableSourceKind', () => { + it.each<[SourceKind[], SourceKind | undefined]>([ + [[SourceKind.Log], SourceKind.Log], + [[SourceKind.Log, SourceKind.Trace], SourceKind.Log], + [[SourceKind.Log, SourceKind.Metric], undefined], + [[SourceKind.Log, SourceKind.Session], undefined], + [[SourceKind.Session, SourceKind.Promql], undefined], + [[], undefined], + ])( + 'returns a stand-in for %p only when charts treat them alike', + (kinds, expected) => { + expect(getInterchangeableSourceKind(kinds)).toBe(expected); + }, + ); +}); diff --git a/packages/app/src/components/ChartEditor/utils.ts b/packages/app/src/components/ChartEditor/utils.ts index 5607dcdaac..db0e51de74 100644 --- a/packages/app/src/components/ChartEditor/utils.ts +++ b/packages/app/src/components/ChartEditor/utils.ts @@ -31,6 +31,7 @@ import { isMetricSource, isRangeThresholdType, isTraceSource, + NON_PROMQL_SOURCE_KINDS, PromqlChartConfig, PromqlHeatmapMode, PromqlSavedChartConfig, @@ -83,6 +84,32 @@ export function buildRawSqlCompletions({ ]; } +/** + * Groups kinds that charts treat alike: validation and normalization only ask + * whether a source is a metric source and whether it supports formulas, and + * PromQL sources only serve PromQL charts. + */ +function chartSourceKindClass(kind: SourceKind): string { + if (kind === SourceKind.Promql) return 'promql'; + if (kind === SourceKind.Metric) return 'metric'; + return isFormulaSourceKind(kind) ? 'events' : 'session'; +} + +/** + * A kind that stands for all of `kinds` when charts treat them alike, so a + * chart built for one is valid on the others. Undefined when they differ. + */ +export function getInterchangeableSourceKind( + kinds: readonly SourceKind[], +): SourceKind | undefined { + const [first] = kinds; + if (first == null) return undefined; + const firstClass = chartSourceKindClass(first); + return kinds.every(kind => chartSourceKindClass(kind) === firstClass) + ? first + : undefined; +} + function normalizeChartConfig< C extends Pick< BuilderSavedChartConfig, @@ -98,15 +125,15 @@ function normalizeChartConfig< | 'heatmap' | 'where' >, ->(config: C, source: TSource): C { - const isMetricSource = source.kind === SourceKind.Metric; +>(config: C, sourceKind: SourceKind): C { + const isMetricSource = sourceKind === SourceKind.Metric; // Formulas (HDX-5080) render on metric and event (log/trace) sources, and // only on the display types the formula query paths support (time series / // table / number). Strip them elsewhere so a source or display-type switch // can't persist a config the renderer would reject. The form state keeps // them, so switching back restores the formula rows. const keepFormulas = - isFormulaSourceKind(source.kind) && + isFormulaSourceKind(sourceKind) && (config.formulas?.length ?? 0) > 0 && isFormulaDisplayType(config.displayType); return { @@ -208,10 +235,6 @@ export const isPromqlDisplayType = ( displayType === DisplayType.Number || displayType === DisplayType.Heatmap; -const NON_PROMQL_SOURCE_KINDS = Object.values(SourceKind).filter( - kind => kind !== SourceKind.Promql, -); - /** * Search and event patterns list raw rows out of the source's `from` table. A * metric source keeps its rows in `metricTables` and leaves `from.tableName` @@ -240,7 +263,7 @@ export function getAllowedSourceKinds({ if (isStringSelectDisplayType(displayType)) { return ROW_LISTING_SOURCE_KINDS; } - return NON_PROMQL_SOURCE_KINDS; + return [...NON_PROMQL_SOURCE_KINDS]; } const isCustomOrderByDisplayType = ( @@ -255,9 +278,14 @@ const isCustomOrderByDisplayType = ( // ChartEditorControls and the server-side surfaces cannot drift. export { isFormulaDisplayType, isFormulaSourceKind }; +/** + * `sourceKind` stands in for `source` when there is no source to read it from, + * as for a data source variable with no selection. + */ export function convertFormStateToSavedChartConfig( form: ChartEditorFormState, source: TSource | undefined, + sourceKind: SourceKind | undefined = source?.kind, ): SavedChartConfig | undefined { if (form.configType === 'promql' && isPromqlDisplayType(form.displayType)) { const promqlConfig: PromqlSavedChartConfig = { @@ -280,7 +308,7 @@ export function convertFormStateToSavedChartConfig( // 'alert', // TODO: Support alerts on PromQL (HDX-4636) ]), promqlExpression: formPromqlExpressions(form), - source: source?.id ?? form.source ?? '', + source: form.source || (source?.id ?? ''), legendTemplate: form.legendTemplate?.trim() || undefined, heatmap: formPromqlHeatmap(form), }; @@ -331,12 +359,12 @@ export function convertFormStateToSavedChartConfig( heatmap: formBuilderHeatmap(form), select: [], where: form.where ?? '', - source: source?.id ?? form.source ?? '', + source: form.source || (source?.id ?? ''), }; return config; } - if (source) { + if (sourceKind) { // Merge the series and select fields back together, and prevent the series field from being submitted const config: BuilderSavedChartConfig = { ...omit(form, [ @@ -354,10 +382,10 @@ export function convertFormStateToSavedChartConfig( : '' : form.series, where: form.where ?? '', - source: source.id, + source: form.source || (source?.id ?? ''), }; - return normalizeChartConfig(config, source); + return normalizeChartConfig(config, sourceKind); } } @@ -416,7 +444,7 @@ export function convertFormStateToChartConfig( ]), sqlTemplate: form.sqlTemplate ?? '', connection: form.connection ?? '', - source: form.source || undefined, + source: source?.id ?? (form.source || undefined), from: source?.from, implicitColumnExpression: source && (isLogSource(source) || isTraceSource(source)) @@ -489,7 +517,7 @@ export function convertFormStateToChartConfig( : mergedSelect, }; - return structuredClone(normalizeChartConfig(newConfig, source)); + return structuredClone(normalizeChartConfig(newConfig, source.kind)); } } @@ -612,6 +640,8 @@ export const validateChartForm = ( setError: UseFormSetError, { requireAlertDisplayName = false, + sourceKind = source?.kind, + missingSourceMessage = 'Source is required', }: { /** * Whether the alert must carry a name. Set for inline alerts, which have @@ -619,6 +649,13 @@ export const validateChartForm = ( * name the alert after whatever the chart happens to be called. */ requireAlertDisplayName?: boolean; + /** + * Stands in for `source` when there is no source to read the kind from, + * as for a data source variable with no selection. + */ + sourceKind?: SourceKind; + /** Replaces the generic error for a chart with no usable source. */ + missingSourceMessage?: string; } = {}, ) => { const errors: { path: Path; message: string }[] = []; @@ -642,9 +679,9 @@ export const validateChartForm = ( if ( !isRawSqlChart && form.displayType !== DisplayType.Markdown && - (!form.source || !source) + (!form.source || !sourceKind) ) { - errors.push({ path: `source`, message: 'Source is required' }); + errors.push({ path: `source`, message: missingSourceMessage }); } // Validate that valueExpressions are specified for each series. @@ -653,7 +690,7 @@ export const validateChartForm = ( if ( !isRawSqlChart && Array.isArray(form.series) && - source?.kind !== SourceKind.Metric && + sourceKind !== SourceKind.Metric && form.displayType !== DisplayType.Markdown && !isStringSelectDisplayType(form.displayType) ) { @@ -669,7 +706,7 @@ export const validateChartForm = ( // Validate metric names for metric sources if ( - source?.kind === SourceKind.Metric && + sourceKind === SourceKind.Metric && Array.isArray(form.series) && form.displayType !== DisplayType.Markdown && form.displayType !== DisplayType.Search && @@ -691,7 +728,7 @@ export const validateChartForm = ( // source kind + display type). if ( !isRawSqlChart && - isFormulaSourceKind(source?.kind) && + isFormulaSourceKind(sourceKind) && isFormulaDisplayType(form.displayType) && Array.isArray(form.formulas) ) { @@ -789,7 +826,7 @@ export const validateChartForm = ( !isRawSqlChart && Array.isArray(form.series) && form.displayType === DisplayType.Number && - !(form.formulas?.length && isFormulaSourceKind(source?.kind)) && + !(form.formulas?.length && isFormulaSourceKind(sourceKind)) && form.series.length > (form.seriesReturnType === 'ratio' ? 2 : 1) ) { errors.push({ diff --git a/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx b/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx index 4524f411ab..7c5d577cd3 100644 --- a/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx @@ -211,6 +211,7 @@ export function ChartEditorControls({ name="source" data-testid="source-selector" allowedSourceKinds={allowedSourceKinds} + allowSourceVariables onSchemaPreview={() => setIsSourceSchemaPreviewOpen(true)} isSchemaPreviewEnabled={isSourceSchemaPreviewEnabled(tableSource)} /> diff --git a/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx b/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx index 61e1b47af8..d08e213a89 100644 --- a/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx @@ -262,7 +262,7 @@ export function ChartPreviewPanel({ <> {isBlockedByRequiredFilters ? ( convertSavedChartConfigToFormState(chartConfig), [chartConfig], @@ -346,13 +356,54 @@ export default function EditTimeChartForm({ [configType, displayType, heatmapMode], ); + // The form keeps a data source variable reference as written; everything + // that queries uses the source the variable currently selects. + const sourceVariableName = parseVariableReference(sourceId); + const resolvedSourceId = useResolvedReference(sourceId); + const resolvedConnection = useResolvedReference(connection); + const sourceVariableDeclarations = useSourceVariableDeclarations(); + + const referenceVariableValues = useReferenceVariableValues(); + // Only raw SQL tiles store a connection of their own + const referenceVariableNames = useMemo( + () => + getConfigReferenceVariables({ + source: sourceId, + connection: isRawSqlInput ? connection : undefined, + }), + [sourceId, connection, isRawSqlInput], + ); + const usesReferenceVariable = referenceVariableNames.length > 0; + + // An alert must query one fixed source and connection, not the viewer's + // current selection. + const alertsEnabled = + (enableAlerts ?? dashboardId != null) && !usesReferenceVariable; + // A source selection that current mode can't query counts as no source, // so that nothing downstream builds a query from it. const { data: tableSource } = useSource({ - id: sourceId, + id: resolvedSourceId, kinds: allowedSourceKinds, }); + // Saving reads only the source's kind. With no source selected, a variable + // whose kinds charts all treat alike still determines it. + const sourceKind = useMemo(() => { + if (tableSource) return tableSource.kind; + if (sourceVariableName == null || resolvedSourceId != null) return; + const kinds = sourceVariableDeclarations + .find(variable => variable.name === sourceVariableName) + ?.allowedSourceKinds.filter(kind => allowedSourceKinds.includes(kind)); + return kinds && getInterchangeableSourceKind(kinds); + }, [ + tableSource, + sourceVariableName, + resolvedSourceId, + sourceVariableDeclarations, + allowedSourceKinds, + ]); + const databaseName = tableSource?.from.databaseName; const tableName = tableSource?.from.tableName; @@ -380,6 +431,10 @@ export default function EditTimeChartForm({ } }, [configType, displayType, previousDisplayType, setValue]); + useEffect(() => { + if (usesReferenceVariable) setValue('alert', undefined); + }, [usesReferenceVariable, setValue]); + const showGeneratedSql = TABS_WITH_GENERATED_SQL.has(activeTab) && !isPromqlInput; @@ -534,6 +589,7 @@ export default function EditTimeChartForm({ variables, unsatisfiedRequiredFilters, applySelections: resolvedApplyFilters, + referenceVariableNames, }); }, [ queriedConfig, @@ -541,6 +597,7 @@ export default function EditTimeChartForm({ variables, unsatisfiedRequiredFilters, resolvedApplyFilters, + referenceVariableNames, ]); // Attach the dashboard's filters and variables so that the preview queries @@ -555,6 +612,28 @@ export default function EditTimeChartForm({ }; }, [queriedConfig, previewDashboardFilters]); + // With no selection for a variable it references the tile has nothing to + // query, so there may be no queried config to compute the block from. + const missingRequiredFilterNames = useMemo(() => { + if (previewDashboardFilters) { + return previewDashboardFilters.missingRequiredFilterNames; + } + const unresolved = referenceVariableNames.filter( + name => !referenceVariableValues.get(name), + ); + return unresolved.length > 0 + ? getBlockingRequiredFilters(unsatisfiedRequiredFilters ?? [], { + referenceVariableNames: unresolved, + consumesBroadcastFilters: false, + }).map(filter => filter.name) + : undefined; + }, [ + previewDashboardFilters, + referenceVariableNames, + referenceVariableValues, + unsatisfiedRequiredFilters, + ]); + const dbTimeChartConfig = useMemo( () => computeDbTimeChartConfig(previewConfig, alert), [previewConfig, alert], @@ -584,10 +663,19 @@ export default function EditTimeChartForm({ const errors = validateChartForm(form, tableSource, setError, { // An inline alert has no tile to inherit a name from. requireAlertDisplayName: alertsEnabled && dashboardId == null, + sourceKind, + missingSourceMessage: + sourceVariableName != null + ? `Select a source for $${sourceVariableName} in the filter bar to save this tile` + : undefined, }); if (errors.length > 0) return { errors, config: null }; - const savedConfig = convertFormStateToSavedChartConfig(form, tableSource); + const savedConfig = convertFormStateToSavedChartConfig( + form, + tableSource, + sourceKind, + ); if (!savedConfig) { console.error( 'convertFormStateToSavedChartConfig returned undefined after validation passed. ' + @@ -621,6 +709,8 @@ export default function EditTimeChartForm({ }, [ tableSource, + sourceKind, + sourceVariableName, setError, alertsEnabled, dashboardId, @@ -646,11 +736,19 @@ export default function EditTimeChartForm({ return; } - const queriedConfig = convertFormStateToChartConfig( + // A raw SQL tile's connection may be a variable reference; the + // source is already resolved through `tableSource`. + const convertedQueriedConfig = convertFormStateToChartConfig( form, dateRange, tableSource, ); + const queriedConfig = + convertedQueriedConfig && + resolveConfigReferences( + convertedQueriedConfig, + referenceVariableValues, + ); if (config && queriedConfig) { const isRawSqlChart = @@ -670,6 +768,7 @@ export default function EditTimeChartForm({ setQueriedConfigAndSource, tableSource, dateRange, + referenceVariableValues, ], ); @@ -784,7 +883,7 @@ export default function EditTimeChartForm({ const allowedSourceItems = useFilteredSortedSourceItems({ sources, allowedSourceKinds, - connectionId: configType === 'sql' ? connection : undefined, + connectionId: configType === 'sql' ? resolvedConnection : undefined, groupBySection: true, }); @@ -817,6 +916,17 @@ export default function EditTimeChartForm({ // should be filled in. Raw SQL source is optional, so leave it. if (!sourceId && configType === 'sql') return; + // Kept on the same terms the picker offers it + if ( + isSourceVariableReferenceAccepted( + sourceId, + sourceVariableDeclarations, + allowedSourceKinds, + ) + ) { + return; + } + // If the currently selected source is still allowed, keep it. const selected = sources.find(s => s.id === sourceId); if ( @@ -846,6 +956,7 @@ export default function EditTimeChartForm({ setValue, sourceId, sources, + sourceVariableDeclarations, ]); useEffect(() => { @@ -1388,16 +1499,14 @@ export default function EditTimeChartForm({ dateRange={dateRange} activeTab={activeTab} alert={alert} - sourceId={sourceId} + sourceId={resolvedSourceId} onTimeRangeSelect={onTimeRangeSelect} chartConfigForExplanations={chartConfigForExplanations} showGeneratedSql={showGeneratedSql} showSampleEvents={showSampleEvents} showGeneratedPromql={isPromqlInput} dbTimeChartConfig={dbTimeChartConfig} - missingRequiredFilterNames={ - previewDashboardFilters?.missingRequiredFilterNames - } + missingRequiredFilterNames={missingRequiredFilterNames} setValue={(name, value) => setValue(name, value)} setDisplayType={type => setValue('displayType', type, { shouldDirty: true }) diff --git a/packages/app/src/components/DBEditTimeChartForm/__tests__/ChartPreviewPanel.test.tsx b/packages/app/src/components/DBEditTimeChartForm/__tests__/ChartPreviewPanel.test.tsx index 1b50d02185..92d73fca8c 100644 --- a/packages/app/src/components/DBEditTimeChartForm/__tests__/ChartPreviewPanel.test.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/__tests__/ChartPreviewPanel.test.tsx @@ -521,7 +521,7 @@ describe('ChartPreviewPanel', () => { expect( screen.getByText( - 'Missing required filters: Service, Environment. Select a value for each required filter, or turn off “Apply filters” to preview this tile without them.', + 'Missing required filters or variables: Service, Environment. Select a value for each required filter, or turn off “Apply filters” to preview this tile without them.', ), ).toBeInTheDocument(); expect(screen.queryByTestId('db-time-chart')).not.toBeInTheDocument(); @@ -544,7 +544,7 @@ describe('ChartPreviewPanel', () => { expect(screen.getByTestId('db-time-chart')).toBeInTheDocument(); expect( - screen.queryByText(/Missing required filters/), + screen.queryByText(/Missing required filters or variables/), ).not.toBeInTheDocument(); }); }); diff --git a/packages/app/src/components/DBEditTimeChartForm/utils.ts b/packages/app/src/components/DBEditTimeChartForm/utils.ts index d49cc65bc1..ed1e84dfe5 100644 --- a/packages/app/src/components/DBEditTimeChartForm/utils.ts +++ b/packages/app/src/components/DBEditTimeChartForm/utils.ts @@ -227,9 +227,12 @@ export function resolveTilePreviewFilters({ variables, unsatisfiedRequiredFilters, applySelections, + referenceVariableNames, }: { config: ChartConfigWithDateRange; sourceId: string | undefined; + /** The data source and connection variables the tile's stored config references. */ + referenceVariableNames?: readonly string[]; filters: BroadcastFilter[] | undefined; variables: ChartVariable[] | undefined; unsatisfiedRequiredFilters: DashboardFilter[] | undefined; @@ -265,6 +268,7 @@ export function resolveTilePreviewFilters({ sourceId, unsatisfiedRequiredFilters, referencedVariables: previewVariables, + referenceVariableNames, }), }; } diff --git a/packages/app/src/components/DashboardFiltersModal/AdhocFilterEditForm.tsx b/packages/app/src/components/DashboardFiltersModal/AdhocFilterEditForm.tsx index aa54074502..70071abcf5 100644 --- a/packages/app/src/components/DashboardFiltersModal/AdhocFilterEditForm.tsx +++ b/packages/app/src/components/DashboardFiltersModal/AdhocFilterEditForm.tsx @@ -3,6 +3,10 @@ import { Controller, UseFormSetValue, useWatch } from 'react-hook-form'; import { Separator } from '@clickhouse/click-ui'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { getAdhocFilterSourceKinds } from '@hyperdx/common-utils/dist/filters'; +import { + isSourceVariableReferenceAccepted, + parseVariableReference, +} from '@hyperdx/common-utils/dist/referenceVariables'; import { AdhocFilterSourceType, DashboardFilter, @@ -13,6 +17,7 @@ import { Box, SegmentedControl } from '@mantine/core'; import { CheckBoxControlled } from '@/components/InputControlled'; import SearchWhereInput from '@/components/SearchInput/SearchWhereInput'; import { SourceMultiSelectControlled } from '@/components/SourceMultiSelect'; +import { useSourceVariableDeclarations } from '@/components/sourceSelectUtils'; import { IS_PROMQL_ENABLED } from '@/config'; import { useSources } from '@/source'; @@ -40,6 +45,7 @@ export const AdhocFilterEditForm = ({ otherFilters, }: AdhocFilterEditFormProps) => { const { data: allSources } = useSources(); + const sourceVariables = useSourceVariableDeclarations(); const [sourceType, isBroadcastEnabled, sourceIds] = useWatch({ control, name: ['sourceType', 'isBroadcastEnabled', 'sources'], @@ -62,6 +68,13 @@ export const AdhocFilterEditForm = ({ // hold after switching from another filter type. const validateSourceKinds = (ids: string[] | undefined) => { const hasOtherKind = ids?.some(id => { + if (parseVariableReference(id) != null) { + return !isSourceVariableReferenceAccepted( + id, + sourceVariables, + allowedSourceKinds, + ); + } const source = allSources?.find(s => s.id === id); return source && !allowedSourceKinds.includes(source.kind); }); @@ -111,6 +124,7 @@ export const AdhocFilterEditForm = ({ comboboxProps={{ withinPortal: true }} placeholder="Select sources" allowedSourceKinds={allowedSourceKinds} + allowSourceVariables rules={{ validate: (value: string[] | undefined) => !value?.length @@ -164,6 +178,7 @@ export const AdhocFilterEditForm = ({ comboboxProps={{ withinPortal: true }} placeholder="Same as data sources" allowedSourceKinds={allowedSourceKinds} + allowSourceVariables rules={{ validate: validateSourceKinds }} /> diff --git a/packages/app/src/components/DashboardFiltersModal/DataSourceFilterEditForm.tsx b/packages/app/src/components/DashboardFiltersModal/DataSourceFilterEditForm.tsx index fdb6e8bc40..b7348e491f 100644 --- a/packages/app/src/components/DashboardFiltersModal/DataSourceFilterEditForm.tsx +++ b/packages/app/src/components/DashboardFiltersModal/DataSourceFilterEditForm.tsx @@ -1,7 +1,9 @@ -import { Controller, useFormState } from 'react-hook-form'; +import { Controller, useFormState, useWatch } from 'react-hook-form'; import { DashboardFilter, SourceKind } from '@hyperdx/common-utils/dist/types'; -import { Group, MultiSelect } from '@mantine/core'; +import { Alert, Group, MultiSelect } from '@mantine/core'; +import { IconAlertTriangle } from '@tabler/icons-react'; +import { getInterchangeableSourceKind } from '@/components/ChartEditor/utils'; import { SOURCE_KIND_LABELS } from '@/components/Sources/SourceForm/constants'; import { SOURCE_KIND_ICONS } from '@/components/sourceSelectUtils'; import { IS_PROMQL_ENABLED } from '@/config'; @@ -26,6 +28,10 @@ export const DataSourceFilterEditForm = ({ otherFilters, }: DataSourceFilterEditFormProps) => { const { errors } = useFormState({ control }); + const allowedSourceKinds = useWatch({ control, name: 'allowedSourceKinds' }); + const hasMixedKinds = + (allowedSourceKinds?.length ?? 0) > 1 && + getInterchangeableSourceKind(allowedSourceKinds) == null; return ( <> @@ -60,6 +66,19 @@ export const DataSourceFilterEditForm = ({ /> + {hasMixedKinds && ( + } + data-testid="filter-mixed-source-kinds-warning" + > + These kinds support different chart options, such as metrics and + formulas. A tile built while one kind is selected may not load when + the variable is switched to another, and a new tile can't be + saved until a source is selected. + + )} + ); diff --git a/packages/app/src/components/DashboardFiltersModal/PromqlLabelFilterEditForm.tsx b/packages/app/src/components/DashboardFiltersModal/PromqlLabelFilterEditForm.tsx index bb3d90a8f0..32092ff438 100644 --- a/packages/app/src/components/DashboardFiltersModal/PromqlLabelFilterEditForm.tsx +++ b/packages/app/src/components/DashboardFiltersModal/PromqlLabelFilterEditForm.tsx @@ -8,6 +8,7 @@ import { import { AutocompleteControlled } from '@/components/InputControlled'; import PromQLEditor from '@/components/PromQLEditor/PromQLEditor'; import { SourceSelectControlled } from '@/components/SourceSelect'; +import { useResolvedReference } from '@/components/sourceSelectUtils'; import { usePromqlLabelNames, usePromqlMetricNames, @@ -42,7 +43,7 @@ export const PromqlLabelFilterEditForm = ({ // Only a PromQL source can answer the label lookup; the form may briefly hold // another kind while the user switches types. - const sourceId = useWatch({ control, name: 'source' }); + const sourceId = useResolvedReference(useWatch({ control, name: 'source' })); const source = sources?.find(s => s.id === sourceId); const promqlSource = source && isPromqlSource(source) ? source : undefined; const { data: labelNames } = usePromqlLabelNames(promqlSource?.id); @@ -64,6 +65,7 @@ export const PromqlLabelFilterEditForm = ({ }} comboboxProps={{ withinPortal: true }} allowedSourceKinds={[SourceKind.Promql]} + allowSourceVariables /> diff --git a/packages/app/src/components/DashboardFiltersModal/QueryExpressionFilterEditForm.tsx b/packages/app/src/components/DashboardFiltersModal/QueryExpressionFilterEditForm.tsx index 8d88a40b0a..f45676a0ec 100644 --- a/packages/app/src/components/DashboardFiltersModal/QueryExpressionFilterEditForm.tsx +++ b/packages/app/src/components/DashboardFiltersModal/QueryExpressionFilterEditForm.tsx @@ -27,6 +27,7 @@ import SourceSchemaPreview, { isSourceSchemaPreviewEnabled, } from '@/components/SourceSchemaPreview'; import { SourceSelectControlled } from '@/components/SourceSelect'; +import { useResolvedReference } from '@/components/sourceSelectUtils'; import { SQLInlineEditorControlled } from '@/components/SQLEditor/SQLInlineEditor'; import { useSource } from '@/source'; import { getMetricTableName } from '@/utils'; @@ -60,7 +61,7 @@ export const QueryExpressionFilterEditForm = ({ showVariableOptions, }: QueryExpressionFilterEditFormProps) => { const sourceId = useWatch({ control, name: 'source' }); - const { data: source } = useSource({ id: sourceId }); + const { data: source } = useSource({ id: useResolvedReference(sourceId) }); const metricType = useWatch({ control, name: 'sourceMetricType' }); const tableName = source && getMetricTableName(source, metricType); @@ -137,6 +138,7 @@ export const QueryExpressionFilterEditForm = ({ isSchemaPreviewEnabled={isSourceSchemaPreviewEnabled(source)} disabled={!!presetSource} allowedSourceKinds={QUERY_EXPRESSION_FILTER_SOURCE_KINDS} + allowSourceVariables /> diff --git a/packages/app/src/components/DashboardFiltersModal/__tests__/DataSourceFilterEditForm.test.tsx b/packages/app/src/components/DashboardFiltersModal/__tests__/DataSourceFilterEditForm.test.tsx new file mode 100644 index 0000000000..63dd72a6f1 --- /dev/null +++ b/packages/app/src/components/DashboardFiltersModal/__tests__/DataSourceFilterEditForm.test.tsx @@ -0,0 +1,38 @@ +import { useForm } from 'react-hook-form'; +import { SourceKind } from '@hyperdx/common-utils/dist/types'; +import { screen } from '@testing-library/react'; + +import { DataSourceFilterEditForm } from '@/components/DashboardFiltersModal/DataSourceFilterEditForm'; +import { + FilterFormValues, + toFormValues, +} from '@/components/DashboardFiltersModal/filterFormState'; + +function Harness({ allowedSourceKinds }: { allowedSourceKinds: SourceKind[] }) { + const { control } = useForm({ + defaultValues: { + ...toFormValues(), + type: 'DATA_SOURCE', + allowedSourceKinds, + }, + }); + return ; +} + +const warning = () => screen.queryByTestId('filter-mixed-source-kinds-warning'); + +describe('DataSourceFilterEditForm', () => { + it('warns when the kinds support different chart options', () => { + renderWithMantine( + , + ); + expect(warning()).toBeInTheDocument(); + }); + + it('does not warn for kinds that charts treat alike', () => { + renderWithMantine( + , + ); + expect(warning()).not.toBeInTheDocument(); + }); +}); diff --git a/packages/app/src/components/SourceMultiSelect.tsx b/packages/app/src/components/SourceMultiSelect.tsx index 665761057e..2eccb1e30f 100644 --- a/packages/app/src/components/SourceMultiSelect.tsx +++ b/packages/app/src/components/SourceMultiSelect.tsx @@ -10,21 +10,25 @@ import { import { IconCheck } from '@tabler/icons-react'; import { - SOURCE_KIND_ICONS, + getSourceOptionIcon, useFilteredSortedSourceItems, useSourceKindMap, + useWithSourceVariableItems, } from '@/components/sourceSelectUtils'; import { useSources } from '@/source'; function SourceMultiSelectControlledComponent({ allowedSourceKinds, connectionId, + allowSourceVariables = false, size, placeholder, ...props }: { allowedSourceKinds?: SourceKind[]; connectionId?: string; + /** Whether to offer the dashboard's data source variables. */ + allowSourceVariables?: boolean; size?: string; placeholder?: string; } & UseControllerProps & @@ -37,18 +41,22 @@ function SourceMultiSelectControlledComponent({ const sourceKindMap = useSourceKindMap(data); - const items = useFilteredSortedSourceItems({ + const sourceItems = useFilteredSortedSourceItems({ sources: data, allowedSourceKinds, connectionId, }); + const items = useWithSourceVariableItems(sourceItems, { + enabled: allowSourceVariables, + allowedSourceKinds, + }); // Mantine passes `checked` to renderOption for MultiSelect items; show a // check on selected entries so the dropdown reflects current selection // without forcing the user to scan pills above. const renderOption = useCallback( ({ option, checked }: { option: ComboboxItem; checked?: boolean }) => { - const icon = SOURCE_KIND_ICONS[sourceKindMap.get(option.value) ?? '']; + const icon = getSourceOptionIcon(option.value, sourceKindMap); return ( {icon} diff --git a/packages/app/src/components/SourceSelect.tsx b/packages/app/src/components/SourceSelect.tsx index c17bfae308..2dd72a808f 100644 --- a/packages/app/src/components/SourceSelect.tsx +++ b/packages/app/src/components/SourceSelect.tsx @@ -1,4 +1,4 @@ -import { memo, useCallback, useMemo } from 'react'; +import { memo, useCallback } from 'react'; import { UseControllerProps, useWatch } from 'react-hook-form'; import { SourceKind, TSource } from '@hyperdx/common-utils/dist/types'; import { @@ -22,10 +22,11 @@ import { import SelectControlled from '@/components/SelectControlled'; import { - SOURCE_KIND_ICONS, + getSourceOptionIcon, sourceSelectFilter, useFilteredSortedSourceItems, useSourceKindMap, + useWithSourceVariableItems, } from '@/components/sourceSelectUtils'; import { useSources } from '@/source'; @@ -161,6 +162,7 @@ function SourceSelectControlledComponent({ allowedSourceKinds, connectionId, isSourceAllowed, + allowSourceVariables = false, comboboxProps, ...props }: { @@ -174,6 +176,8 @@ function SourceSelectControlledComponent({ connectionId?: string; /** Extra per-source gate for restrictions a kind can't express. */ isSourceAllowed?: (source: TSource) => boolean; + /** Whether to offer the dashboard's data source variables. */ + allowSourceVariables?: boolean; } & UseControllerProps & SelectProps) { const { data } = useSources(); @@ -182,23 +186,18 @@ function SourceSelectControlledComponent({ name: props.name, }); - const selectedSourceKind = useMemo( - () => data?.find(s => s.id === selectedSourceId)?.kind, - [data, selectedSourceId], - ); + const sourceKindMap = useSourceKindMap(data); - const leftIcon = SOURCE_KIND_ICONS[selectedSourceKind ?? ''] ?? ( + const leftIcon = getSourceOptionIcon(selectedSourceId, sourceKindMap) ?? ( ); - const sourceKindMap = useSourceKindMap(data); - // Mantine passes `checked` to renderOption for the currently selected // option; render a trailing check so the active source is obvious in the // dropdown (the closed input only shows the kind icon + label). const renderOption = useCallback( ({ option, checked }: { option: ComboboxItem; checked?: boolean }) => { - const icon = SOURCE_KIND_ICONS[sourceKindMap.get(option.value) ?? '']; + const icon = getSourceOptionIcon(option.value, sourceKindMap); return ( {icon} @@ -219,6 +218,10 @@ function SourceSelectControlledComponent({ isSourceAllowed, groupBySection: true, }); + const items = useWithSourceVariableItems(sourceItems, { + enabled: allowSourceVariables, + allowedSourceKinds, + }); const hasSelection = !!selectedSourceId; const hasMenu = @@ -233,7 +236,7 @@ function SourceSelectControlledComponent({ > ({ @@ -265,3 +270,64 @@ describe('SourceSelectControlled (grouped rendering + tag-style search)', () => expect(screen.queryByText('Other')).not.toBeInTheDocument(); }); }); + +describe('SourceSelectControlled (data source variables)', () => { + const SOURCES = [makeSource('logs', 'Logs', SourceKind.Log)]; + const sourceVariable = ( + variableName: string, + allowedSourceKinds: SourceKind[], + connection?: string, + ): DataSourceDashboardFilter => ({ + id: variableName, + type: 'DATA_SOURCE', + name: variableName, + variableName, + allowedSourceKinds, + connection, + minSelections: 1, + maxSelections: 1, + isBroadcastEnabled: false, + isVariableEnabled: true, + }); + const VARIABLES = [ + sourceVariable('logs_source', [SourceKind.Log]), + sourceVariable('any_source', [SourceKind.Log, SourceKind.Promql]), + ]; + + function VariableHarness({ + allowSourceVariables, + }: { + allowSourceVariables?: boolean; + }) { + const { control } = useForm(); + return ( + + + + ); + } + + it('offers only variables whose every kind the picker allows', async () => { + asMock(useSources).mockReturnValue({ data: SOURCES }); + renderWithMantine(); + await userEvent.click(screen.getByPlaceholderText('Data Source')); + + expect(await screen.findByText('Variables')).toBeInTheDocument(); + expect(screen.getByText('$logs_source')).toBeInTheDocument(); + expect(screen.queryByText('$any_source')).not.toBeInTheDocument(); + }); + + it('offers no variables unless the picker opts in', async () => { + asMock(useSources).mockReturnValue({ data: SOURCES }); + renderWithMantine(); + await userEvent.click(screen.getByPlaceholderText('Data Source')); + + expect(await screen.findByText('Logs')).toBeInTheDocument(); + expect(screen.queryByText('$logs_source')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/app/src/components/sourceSelectUtils.tsx b/packages/app/src/components/sourceSelectUtils.tsx index 1533d6e562..36d1606ab5 100644 --- a/packages/app/src/components/sourceSelectUtils.tsx +++ b/packages/app/src/components/sourceSelectUtils.tsx @@ -1,6 +1,18 @@ -import { useMemo } from 'react'; -import { parseVariableReference } from '@hyperdx/common-utils/dist/referenceVariables'; -import { SourceKind, TSource } from '@hyperdx/common-utils/dist/types'; +import { createContext, use, useMemo } from 'react'; +import { + getSourceVariableDeclarations, + isSourceVariableAccepted, + parseVariableReference, + ReferenceVariableValues, + resolveVariableReference, + SourceVariableDeclaration, + toVariableReference, +} from '@hyperdx/common-utils/dist/referenceVariables'; +import { + DashboardFilter, + SourceKind, + TSource, +} from '@hyperdx/common-utils/dist/types'; import { ComboboxItem, OptionsFilter } from '@mantine/core'; import { IconChartLine, @@ -28,6 +40,97 @@ export function getSourceOptionIcon( return SOURCE_KIND_ICONS[(value && sourceKindMap.get(value)) ?? '']; } +type ReferenceVariablesContextValue = { + sourceVariables: SourceVariableDeclaration[]; + values: ReferenceVariableValues; +}; + +const ReferenceVariablesContext = createContext( + { sourceVariables: [], values: new Map() }, +); + +/** + * Offers the data source variables `filters` declare to the pickers below it + * that opt in, and their selections to `useResolvedReference`. + */ +export function ReferenceVariablesProvider({ + filters, + values, + children, +}: { + filters: DashboardFilter[]; + values: ReferenceVariableValues; + children: React.ReactNode; +}) { + const value = useMemo( + () => ({ + sourceVariables: getSourceVariableDeclarations(filters), + values, + }), + [filters, values], + ); + return ( + + {children} + + ); +} + +/** + * The ID a source field currently queries: an ID as is, or the selection of + * the variable it references. + */ +export function useResolvedReference( + value: string | undefined, +): string | undefined { + const { values } = use(ReferenceVariablesContext); + return resolveVariableReference(value, values); +} + +export function useReferenceVariableValues(): ReferenceVariableValues { + return use(ReferenceVariablesContext).values; +} + +export function useSourceVariableDeclarations(): SourceVariableDeclaration[] { + return use(ReferenceVariablesContext).sourceVariables; +} + +const toVariableItem = (name: string): ComboboxItem => { + const reference = toVariableReference(name); + return { value: reference, label: reference }; +}; + +/** + * A source picker's items, preceded by a "Variables" group of the data source + * variables it can offer. A variable is offered only if every kind it can + * resolve to is allowed, so no selection can produce an unusable source. + */ +export function useWithSourceVariableItems( + sourceItems: ComboboxItem[] | SourceSelectGroup[], + { + enabled, + allowedSourceKinds, + }: { + enabled: boolean; + allowedSourceKinds?: SourceKind[]; + }, +): (ComboboxItem | SourceSelectGroup)[] { + const { sourceVariables } = use(ReferenceVariablesContext); + return useMemo(() => { + if (!enabled) return sourceItems; + const items = sourceVariables + .filter( + variable => + !allowedSourceKinds || + isSourceVariableAccepted(variable, allowedSourceKinds), + ) + .map(variable => toVariableItem(variable.name)); + return items.length > 0 + ? [{ group: 'Variables', items }, ...sourceItems] + : sourceItems; + }, [enabled, sourceVariables, allowedSourceKinds, sourceItems]); +} + /** Header for sources that have no section assigned. Internal to this module. */ const UNSECTIONED_SOURCE_GROUP = 'Other'; diff --git a/packages/app/tests/e2e/features/dashboard-source-variables.spec.ts b/packages/app/tests/e2e/features/dashboard-source-variables.spec.ts index 54b6e55380..5ee0cf0339 100644 --- a/packages/app/tests/e2e/features/dashboard-source-variables.spec.ts +++ b/packages/app/tests/e2e/features/dashboard-source-variables.spec.ts @@ -1,8 +1,10 @@ /** * A `DATA_SOURCE` dashboard filter is a single-select variable whose value is - * a source, picked from the filter bar. + * a source. Tiles and other filters reference it as `$name` in place of a + * source, block until it has a selection, and cannot carry alerts. */ import { DashboardPage } from '../page-objects/DashboardPage'; +import { SERVICES } from '../seed-clickhouse'; import { expect, test } from '../utils/base-test'; import { DEFAULT_LOGS_SOURCE_NAME, @@ -20,9 +22,17 @@ test.describe( await dashboardPage.goto(); }); - test('picks a source of the allowed kinds from the filter bar', async ({ + test('drives tiles and filters from the selected source', async ({ page, }) => { + const editor = dashboardPage.chartEditor; + const reloadWithoutFilterState = async () => { + const url = new URL(page.url()); + url.search = ''; + await page.goto(url.toString()); + await dashboardPage.waitForLoaded(); + }; + await test.step('Create a data source variable limited to logs', async () => { await dashboardPage.createNewDashboard(); await dashboardPage.openEditFiltersModal(); @@ -34,7 +44,21 @@ test.describe( await expect( dashboardPage.getFilterItemByName('Logs source'), ).toContainText('($logs)'); + }); + + await test.step('A queried filter can read from the variable', async () => { + await dashboardPage.addFilterToDashboard( + 'Service', + '$logs', + 'ServiceName', + ); + await expect( + dashboardPage.getFilterItemByName('Service'), + ).toContainText('$logs'); await dashboardPage.closeFiltersModal(); + await expect( + dashboardPage.getFilterPendingVariableWarning('Service'), + ).toBeVisible(); }); await test.step('The variable offers only allowed kinds', async () => { @@ -48,20 +72,76 @@ test.describe( await page.keyboard.press('Escape'); }); - await test.step('A saved selection is the initial value', async () => { + await test.step('A tile can be saved on the variable before it has a value', async () => { + await dashboardPage.addTile(); + await expect(editor.nameInput).toBeVisible(); + await editor.waitForDataToLoad(); + await expect(editor.alertButton).toBeVisible(); + await editor.setChartName('Variable tile'); + await editor.selectSource('$logs'); + await expect(editor.alertButton).toHaveCount(0); + await editor.save(); + await expect(dashboardPage.getTiles()).toHaveCount(1); + await expect( + dashboardPage.getTileMissingRequiredFilters(0), + ).toContainText('Logs source'); + }); + + await test.step('A selection loads the tile and the filter', async () => { await dashboardPage.selectReferenceFilterValue( 'Logs source', DEFAULT_LOGS_SOURCE_NAME, ); + await expect( + dashboardPage.getTileMissingRequiredFilters(0), + ).toHaveCount(0); + await expect(dashboardPage.getTileTimeChart(0)).toBeVisible(); + await expect( + dashboardPage.getFilterPendingVariableWarning('Service'), + ).toHaveCount(0); + await dashboardPage.openFilterDropdown('Service'); + await expect(dashboardPage.getFilterOption(SERVICES[0])).toBeVisible(); + await page.keyboard.press('Escape'); + }); + + await test.step('The editor keeps the reference', async () => { + await dashboardPage.editTile(0); + await expect(editor.source).toHaveValue('$logs'); + await expect(editor.alertButton).toHaveCount(0); + await editor.save(); + }); + + await test.step('A saved selection is the initial value', async () => { await dashboardPage.saveQueryAndFiltersAsDefault(); - const url = new URL(page.url()); - url.search = ''; - await page.goto(url.toString()); - await dashboardPage.waitForLoaded(); + await reloadWithoutFilterState(); await expect( dashboardPage.getFilterSelectByName('Logs source'), ).toHaveValue(DEFAULT_LOGS_SOURCE_NAME); + await expect( + dashboardPage.getTileMissingRequiredFilters(0), + ).toHaveCount(0); + }); + + await test.step('A selection the variable no longer offers stops applying', async () => { + await dashboardPage.openEditFiltersModal(); + await dashboardPage.openEditFilterForm('Logs source'); + const kindsInput = page.getByTestId( + 'filter-allowed-source-kinds-input', + ); + await kindsInput.click(); + await dashboardPage.getFilterOption('Traces').click(); + await dashboardPage.getFilterOption('Logs').click(); + await dashboardPage.getFilterNameInput().click(); + await page.getByTestId('save-filter-button').click(); + await dashboardPage.closeFiltersModal(); + + await expect( + dashboardPage.getFilterSelectByName('Logs source'), + ).toHaveValue(''); + await expect( + dashboardPage.getTileMissingRequiredFilters(0), + ).toContainText('Logs source'); }); }); }, diff --git a/packages/app/tests/e2e/features/dashboard-tile-editor-filters.spec.ts b/packages/app/tests/e2e/features/dashboard-tile-editor-filters.spec.ts index 314cf2ac1e..cd25f4f0c5 100644 --- a/packages/app/tests/e2e/features/dashboard-tile-editor-filters.spec.ts +++ b/packages/app/tests/e2e/features/dashboard-tile-editor-filters.spec.ts @@ -120,7 +120,7 @@ test.describe( dashboardPage.chartEditor.previewMissingRequiredFilters(); await expect(placeholder).toBeVisible({ timeout: 20000 }); await expect(placeholder).toContainText( - 'Missing required filters: Service', + 'Missing required filters or variables: Service', ); }); diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index 196dda893f..b032a50a63 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -632,6 +632,12 @@ export class DashboardPage { return new HeatmapComponent(this.page, this.getTile(tileIndex)); } + getTileTimeChart(tileIndex = 0): Locator { + return this.getTile(tileIndex) + .locator('.recharts-responsive-container') + .first(); + } + /** * Hover over a tile to reveal action buttons */