Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Histogram UI fixes #363

Merged
merged 12 commits into from
May 3, 2023
16 changes: 11 additions & 5 deletions src/components/queryBuilder/Filters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,12 @@ const FilterValueNumberItem = (props: { value: number; onChange: (value: number)
};

const FilterValueSingleStringItem = (props: { value: string; onChange: (value: string) => void }) => {
const [value, setValue] = useState(props.value || '');
return (
<div data-testid="query-builder-filters-single-string-value-container">
<Input
type="text"
value={value}
onChange={(e) => setValue(e.currentTarget.value)}
onBlur={() => props.onChange(value)}
defaultValue={props.value}
onBlur={(e) => props.onChange(e.currentTarget.value)}
/>
</div>
);
Expand Down Expand Up @@ -142,7 +140,15 @@ export const FilterValueEditor = (props: {
</div>
);
}
return <FilterValueSingleStringItem value={filter.value} onChange={onStringFilterValueChange} />;

return (
<FilterValueSingleStringItem
value={filter.value}
onChange={onStringFilterValueChange}
// enforce input re-render when filter changes to avoid stale input value
key={filter.value}
/>
);
} else if (utils.isMultiFilter(filter)) {
const onMultiFilterValueChange = (value: string[]) => {
onFilterChange({ ...filter, value });
Expand Down
11 changes: 4 additions & 7 deletions src/components/queryBuilder/LogLevelField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,14 @@ import { FullField } from 'types';
interface LogLevelEditorProps {
fieldsList: FullField[];
onLogLevelFieldChange: (logLevelField: string) => void;
logLevelField: string | null;
slvrtrn marked this conversation as resolved.
Show resolved Hide resolved
}

export const LogLevelFieldEditor = (props: LogLevelEditorProps) => {
const { label, tooltip } = selectors.components.QueryEditor.QueryBuilder.LOG_LEVEL_FIELD;
const columns: SelectableValue[] = (props.fieldsList || [])
.filter((f) => f.name !== '*')
.map((f) => ({ label: f.label, value: f.name }));
if (columns.length) {
columns.push({
label: '-',
value: undefined, // allow to de-select the field
});
}
return (
<div className="gf-form">
<InlineFormLabel width={8} className="query-keyword" tooltip={tooltip}>
Expand All @@ -28,8 +23,10 @@ export const LogLevelFieldEditor = (props: LogLevelEditorProps) => {
<Select
options={columns}
width={20}
onChange={(e) => props.onLogLevelFieldChange(e.value)}
onChange={(e) => props.onLogLevelFieldChange(e?.value ?? e)}
menuPlacement={'bottom'}
value={props.logLevelField}
isClearable={true}
/>
</div>
);
Expand Down
1 change: 1 addition & 0 deletions src/components/queryBuilder/QueryBuilder.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('QueryBuilder', () => {
database: 'db',
table: 'foo',
fields: [],
filters: [],
}}
onBuilderOptionsChange={() => {}}
datasource={mockDs}
Expand Down
41 changes: 28 additions & 13 deletions src/components/queryBuilder/QueryBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ interface QueryBuilderProps {

export const QueryBuilder = (props: QueryBuilderProps) => {
const [baseFieldsList, setBaseFieldsList] = useState<FullField[]>([]);
const [timeField, setTimeField] = useState<string | null>(null);
const [logLevelField, setLogLevelField] = useState<string | null>(null);
const builder = defaultsDeep(props.builderOptions, defaultCHBuilderQuery.builderOptions);
useEffect(() => {
const fetchBaseFields = async (database: string, table: string) => {
Expand All @@ -47,17 +49,20 @@ export const QueryBuilder = (props: QueryBuilderProps) => {
fields.push({ name: '*', label: 'ALL', type: 'string', picklistValues: [] });
setBaseFieldsList(fields);

const dateTimeFields = fields.filter((f) => isDateTimeType(f.type));
if (dateTimeFields.length > 0) {
const filter: Filter & PredefinedFilter = {
operator: FilterOperator.WithInGrafanaTimeRange,
filterType: 'custom',
key: dateTimeFields[0].name,
type: 'datetime',
condition: 'AND',
restrictToFields: dateTimeFields,
};
onFiltersChange([filter]);
// if no filters are set, we add a default one for the time range
if (builder.filters?.length === 0) {
const dateTimeFields = fields.filter((f) => isDateTimeType(f.type));
if (dateTimeFields.length > 0) {
const filter: Filter & PredefinedFilter = {
operator: FilterOperator.WithInGrafanaTimeRange,
filterType: 'custom',
key: dateTimeFields[0].name,
type: 'datetime',
condition: 'AND',
restrictToFields: dateTimeFields,
};
onFiltersChange([filter]);
}
}

// When changing from SQL Editor to Query Builder, we need to find out if the
Expand Down Expand Up @@ -94,6 +99,8 @@ export const QueryBuilder = (props: QueryBuilderProps) => {

const onDatabaseChange = (database = '') => {
setBaseFieldsList([]);
setTimeField(null);
setLogLevelField(null);
const queryOptions: SqlBuilderOptions = {
...builder,
database,
Expand All @@ -108,6 +115,8 @@ export const QueryBuilder = (props: QueryBuilderProps) => {
};

const onTableChange = (table = '') => {
setTimeField(null);
setLogLevelField(null);
const queryOptions: SqlBuilderOptions = {
...builder,
table,
Expand Down Expand Up @@ -165,11 +174,13 @@ export const QueryBuilder = (props: QueryBuilderProps) => {
};

const onTimeFieldChange = (timeField = '', timeFieldType = '') => {
setTimeField(timeField);
const queryOptions: SqlBuilderOptions = { ...builder, timeField, timeFieldType };
props.onBuilderOptionsChange(queryOptions);
};

const onLogLevelFieldChange = (logLevelField = '') => {
setLogLevelField(logLevelField);
const queryOptions: SqlBuilderOptions = { ...builder, logLevelField };
props.onBuilderOptionsChange(queryOptions);
};
Expand Down Expand Up @@ -221,14 +232,18 @@ export const QueryBuilder = (props: QueryBuilderProps) => {
builder.mode === BuilderMode.List && props.format === Format.LOGS && props.app === CoreApp.Explore && (
<>
<TimeFieldEditor
timeField={builder.timeField}
timeField={timeField}
timeFieldType={builder.timeFieldType}
onTimeFieldChange={onTimeFieldChange}
fieldsList={fieldsList}
timeFieldTypeCheckFn={isDateTimeType}
labelAndTooltip={selectors.components.QueryEditor.QueryBuilder.LOGS_VOLUME_TIME_FIELD}
/>
<LogLevelFieldEditor fieldsList={fieldsList} onLogLevelFieldChange={onLogLevelFieldChange} />
<LogLevelFieldEditor
logLevelField={logLevelField}
fieldsList={fieldsList}
onLogLevelFieldChange={onLogLevelFieldChange}
/>
</>
)
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/queryBuilder/TimeField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { FullField } from 'types';

interface TimeFieldEditorProps {
fieldsList: FullField[];
timeField: string;
timeField: string | null;
timeFieldType: string;
onTimeFieldChange: (timeField: string, timeFieldType: string) => void;
timeFieldTypeCheckFn: (type: string) => boolean;
Expand Down
66 changes: 66 additions & 0 deletions src/data/CHDatasource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
getTimeZone,
getTimeZoneInfo,
MetricFindValue,
QueryFixAction,
ScopedVars,
SupplementaryQueryType,
TypedVariableModel,
Expand All @@ -21,6 +22,8 @@ import {
BuilderMode,
CHConfig,
CHQuery,
Filter,
FilterOperator,
Format,
FullField,
OrderByDirection,
Expand Down Expand Up @@ -246,6 +249,69 @@ export class Datasource
return rawQuery;
}

modifyQuery(query: CHQuery, action: QueryFixAction): CHQuery {
// support filtering by field value in Explore
if (
query.queryType === QueryType.Builder &&
action.options !== undefined &&
'key' in action.options &&
'value' in action.options
) {
let filters: Filter[] = query.builderOptions.filters || [];
if (action.type === 'ADD_FILTER') {
// we need to remove *any other EQ or NE* for the same field,
// because we don't want to end up with two filters like `level=info` AND `level=error`
filters = (query.builderOptions.filters ?? []).filter(
(f) =>
!(
f.type === 'string' &&
f.key === action.options?.key &&
(f.operator === FilterOperator.Equals || f.operator === FilterOperator.NotEquals)
)
);
filters.push({
condition: 'AND',
key: action.options.key,
type: 'string',
filterType: 'custom',
operator: FilterOperator.Equals,
value: action.options.value,
});
} else if (action.type === 'ADD_FILTER_OUT') {
// with this we might want to add multiple values as NE filters
// for example, `level != info` AND `level != debug`
// thus, here we remove only exactly matching NE filters or an existing EQ filter for this field
filters = (query.builderOptions.filters ?? []).filter(
(f) =>
!(
(f.type === 'string' &&
f.key === action.options?.key &&
'value' in f &&
f.value === action.options?.value &&
f.operator === FilterOperator.NotEquals) ||
(f.type === 'string' && f.key === action.options?.key && f.operator === FilterOperator.Equals)
)
);
filters.push({
condition: 'AND',
key: action.options.key,
type: 'string',
filterType: 'custom',
operator: FilterOperator.NotEquals,
value: action.options.value,
});
}
const updatedBuilder = { ...query.builderOptions, filters };
return {
...query,
// the query is updated to trigger the URL update and propagation to the panels
rawSql: getSQLFromQueryOptions(updatedBuilder),
builderOptions: updatedBuilder,
};
}
return query;
}

private getMacroArgs(query: string, argsIndex: number): string[] {
const args = [] as string[];
const re = /\(|\)|,/g;
Expand Down
1 change: 0 additions & 1 deletion src/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export const plugin = new DataSourcePlugin<Datasource, CHQuery, CHConfig>(Dataso
getAppEvents().subscribe<DashboardLoadedEvent<CHQuery>>(
DashboardLoadedEvent,
({ payload: { dashboardId, orgId, grafanaVersion, queries } }) => {

const clickhouseQueries = queries[pluginJson.id]?.filter((q) => !q.hide);
if (!clickhouseQueries?.length) {
return;
Expand Down
38 changes: 29 additions & 9 deletions src/tracking.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,50 @@ describe('analyzeQueries', () => {
{
description: 'should count 1 sql query',
queries: [{ queryType: QueryType.SQL }],
expectedCounters: { builder_queries: 0, sql_queries: 1, builder_table_queries: 0, builder_aggregate_queries: 0, builder_time_series_queries: 0 },
expectedCounters: {
builder_queries: 0,
sql_queries: 1,
builder_table_queries: 0,
builder_aggregate_queries: 0,
builder_time_series_queries: 0,
},
},
{
description: 'should count 1 builder query (with a default mode of Table)',
queries: [{ queryType: QueryType.Builder, builderOptions: { mode: BuilderMode.List } }],
expectedCounters: { builder_queries: 1, sql_queries: 0, builder_table_queries: 1, builder_aggregate_queries: 0, builder_time_series_queries: 0 },
expectedCounters: {
builder_queries: 1,
sql_queries: 0,
builder_table_queries: 1,
builder_aggregate_queries: 0,
builder_time_series_queries: 0,
},
},
{
description: 'should count 1 builder query with a mode of Aggregate',
queries: [{ queryType: QueryType.Builder, builderOptions: { mode: BuilderMode.Aggregate } }],
expectedCounters: { builder_queries: 1, sql_queries: 0, builder_table_queries: 0, builder_aggregate_queries: 1, builder_time_series_queries: 0 },
expectedCounters: {
builder_queries: 1,
sql_queries: 0,
builder_table_queries: 0,
builder_aggregate_queries: 1,
builder_time_series_queries: 0,
},
},
{
description: 'should count 1 builder query with a mode of Time Series',
queries: [{ queryType: QueryType.Builder, builderOptions: { mode: BuilderMode.Trend } }],
expectedCounters: { builder_queries: 1, sql_queries: 0, builder_table_queries: 0, builder_aggregate_queries: 0, builder_time_series_queries: 1 },
expectedCounters: {
builder_queries: 1,
sql_queries: 0,
builder_table_queries: 0,
builder_aggregate_queries: 0,
builder_time_series_queries: 1,
},
},
].forEach((t) => {
it(t.description, () => {
expect(
analyzeQueries(
t.queries as CHQuery[]
)
).toMatchObject(t.expectedCounters);
expect(analyzeQueries(t.queries as CHQuery[])).toMatchObject(t.expectedCounters);
});
});
});
21 changes: 10 additions & 11 deletions src/tracking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ export const trackClickhouseDashboardLoaded = (props: ClickhouseDashboardLoadedP
};

export type ClickhouseCounters = {
sql_queries: number,
builder_queries: number,
builder_table_queries: number,
builder_aggregate_queries: number,
builder_time_series_queries: number
sql_queries: number;
builder_queries: number;
builder_table_queries: number;
builder_aggregate_queries: number;
builder_time_series_queries: number;
};

export interface ClickhouseDashboardLoadedProps extends ClickhouseCounters {
Expand All @@ -19,23 +19,23 @@ export interface ClickhouseDashboardLoadedProps extends ClickhouseCounters {
dashboard_id: string;
org_id?: number;
[key: string]: any;
};
}

export const analyzeQueries = (queries: CHQuery[]): ClickhouseCounters => {
const counters = {
sql_queries: 0,
builder_queries: 0,
builder_table_queries: 0,
builder_aggregate_queries: 0,
builder_time_series_queries: 0
builder_time_series_queries: 0,
};

queries.forEach((query) => {
switch (query.queryType) {
case (QueryType.SQL):
case QueryType.SQL:
counters.sql_queries++;
break;
case (QueryType.Builder):
case QueryType.Builder:
counters.builder_queries++;
if (query.builderOptions.mode === BuilderMode.Aggregate) {
counters.builder_aggregate_queries++;
Expand All @@ -50,4 +50,3 @@ export const analyzeQueries = (queries: CHQuery[]): ClickhouseCounters => {

return counters;
};

Loading