Prometheus: Metrics explorer usability test improvements (#69528)

* remove infer type functionality because usability tests confirmed it was confusing/not helpful

* persist button option to open modal when typing in metric select

* update copy desc for setting that includes type and description in search

* when filtering by type, only return metrics with defined type

* give focused metric row more contrast, consistent with metric select focused option

* allow selection of metrics with unknown types and undefined types

* add highlighting to backend search

* augment counters created from summaries with (summary)

* remove type from search input setting and only search by name and description

* fix test to reflect that type has been removed from the metadata input search as duplicated by the filter

* add button to select metric, change wording, make table hover row consistent with grafana table

* add tooltip icon with docs link for metric types that are augmented, histogram and summary

* remove files slated for future refactoring

* style changes based on catherine's review

* remove border from settings btn, select btn increase to md, change col size in table, fix responsive inputs ui for sm screens
This commit is contained in:
Brendan O'Handley 2023-06-08 12:53:17 -04:00 committed by GitHub
parent cae3b4c6e6
commit ba97c492f9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 184 additions and 168 deletions

View File

@ -49,6 +49,14 @@ export function MetricSelect({
initialMetrics?: string[];
}>({});
const metricsModalOption: SelectableValue[] = [
{
value: 'BrowseMetrics',
label: 'Metrics explorer',
description: 'Browse and filter metrics and metadata with a fuzzy search',
},
];
const customFilterOption = useCallback((option: SelectableValue<any>, searchQuery: string) => {
const label = option.label ?? option.value;
if (!label) {
@ -61,7 +69,16 @@ export function MetricSelect({
}
const searchWords = searchQuery.split(splitSeparator);
return searchWords.reduce((acc, cur) => acc && label.toLowerCase().includes(cur.toLowerCase()), true);
return searchWords.reduce((acc, cur) => {
const matcheSearch = label.toLowerCase().includes(cur.toLowerCase());
let browseOption = false;
if (prometheusMetricEncyclopedia) {
browseOption = label === 'Metrics explorer';
}
return acc && (matcheSearch || browseOption);
}, true);
}, []);
const formatOptionLabel = useCallback(
@ -70,7 +87,8 @@ export function MetricSelect({
if (option['__isNew__']) {
return option.label;
}
// only matches on input, does not match on regex
// look into matching for regex input
return (
<Highlighter
searchWords={meta.inputValue.split(splitSeparator)}
@ -104,12 +122,19 @@ export function MetricSelect({
if (results.length > PROMETHEUS_QUERY_BUILDER_MAX_RESULTS) {
results.splice(0, results.length - PROMETHEUS_QUERY_BUILDER_MAX_RESULTS);
}
return results.map((result) => {
const resultsOptions = results.map((result) => {
return {
label: result.text,
value: result.text,
};
});
if (prometheusMetricEncyclopedia) {
return [...metricsModalOption, ...resultsOptions];
} else {
return resultsOptions;
}
});
};
@ -201,18 +226,11 @@ export function MetricSelect({
}
if (prometheusMetricEncyclopedia) {
// pass the initial metrics, possibly filtered by labels into the Metrics Modal
const metricsModalOption: SelectableValue[] = [
{
value: 'BrowseMetrics',
label: 'Metrics explorer',
description: 'Browse and filter metrics and metadata with a fuzzy search',
},
];
// pass the initial metrics into the Metrics Modal
setState({
// add the modal butoon option to the options
metrics: [...metricsModalOption, ...metrics],
isLoading: undefined,
// pass the initial metrics into the Metrics Modal
initialMetrics: initialMetrics,
});
} else {

View File

@ -14,18 +14,11 @@ type AdditionalSettingsProps = {
onChangeIncludeNullMetadata: () => void;
onChangeDisableTextWrap: () => void;
onChangeUseBackend: () => void;
onChangeInferType: () => void;
};
export function AdditionalSettings(props: AdditionalSettingsProps) {
const {
state,
onChangeFullMetaSearch,
onChangeIncludeNullMetadata,
onChangeDisableTextWrap,
onChangeUseBackend,
onChangeInferType,
} = props;
const { state, onChangeFullMetaSearch, onChangeIncludeNullMetadata, onChangeDisableTextWrap, onChangeUseBackend } =
props;
const theme = useTheme2();
const styles = getStyles(theme);
@ -63,18 +56,6 @@ export function AdditionalSettings(props: AdditionalSettingsProps) {
<Icon name="info-circle" size="xs" className={styles.settingsIcon} />
</Tooltip>
</div>
<div className={styles.selectItem}>
<Switch data-testid={testIds.inferType} value={state.inferType} onChange={() => onChangeInferType()} />
<div className={styles.selectItemLabel}>{placeholders.inferType}&nbsp;</div>
<Tooltip
content={
'For example, metrics ending in _sum, _count, will be given an inferred type of counter. Metrics ending in _bucket with be given a type of histogram.'
}
placement="bottom-end"
>
<Icon name="info-circle" size="xs" className={styles.settingsIcon} />
</Tooltip>
</div>
</>
);
}

View File

@ -159,7 +159,7 @@ describe('MetricsModal', () => {
});
});
it('searches by all metric metadata with a fuzzy search', async () => {
it('searches by name and description with a fuzzy search when setting is turned on', async () => {
// search for a_bucket by metadata type counter but only type countt
setup(defaultQuery, listOfMetrics);
let metricABucket: HTMLElement | null;
@ -179,7 +179,7 @@ describe('MetricsModal', () => {
const searchMetric = screen.getByTestId(testIds.searchMetric);
expect(searchMetric).toBeInTheDocument();
await userEvent.type(searchMetric, 'countt');
await userEvent.type(searchMetric, 'functions');
await waitFor(() => {
metricABucket = screen.getByText('a_bucket');

View File

@ -69,7 +69,6 @@ const {
setSelectedIdx,
setDisableTextWrap,
showAdditionalSettings,
setInferType,
} = stateSlice.actions;
export const MetricsModal = (props: MetricsModalProps) => {
@ -83,31 +82,26 @@ export const MetricsModal = (props: MetricsModalProps) => {
/**
* loads metrics and metadata on opening modal and switching off useBackend
*/
const updateMetricsMetadata = useCallback(
async (inferType: boolean) => {
// *** Loading Gif
dispatch(setIsLoading(true));
const updateMetricsMetadata = useCallback(async () => {
// *** Loading Gif
dispatch(setIsLoading(true));
const data: MetricsModalMetadata = await setMetrics(datasource, query, inferType, initialMetrics);
dispatch(
buildMetrics({
isLoading: false,
hasMetadata: data.hasMetadata,
metrics: data.metrics,
metaHaystackDictionary: data.metaHaystackDictionary,
nameHaystackDictionary: data.nameHaystackDictionary,
totalMetricCount: data.metrics.length,
filteredMetricCount: data.metrics.length,
})
);
},
[query, datasource, initialMetrics]
);
const data: MetricsModalMetadata = await setMetrics(datasource, query, initialMetrics);
dispatch(
buildMetrics({
isLoading: false,
hasMetadata: data.hasMetadata,
metrics: data.metrics,
metaHaystackDictionary: data.metaHaystackDictionary,
nameHaystackDictionary: data.nameHaystackDictionary,
totalMetricCount: data.metrics.length,
filteredMetricCount: data.metrics.length,
})
);
}, [query, datasource, initialMetrics]);
useEffect(() => {
updateMetricsMetadata(state.inferType);
// eslint-disable-next-line react-hooks/exhaustive-deps
updateMetricsMetadata();
}, [updateMetricsMetadata]);
const typeOptions: SelectableValue[] = promTypes.map((t: PromFilterOption) => {
@ -123,10 +117,10 @@ export const MetricsModal = (props: MetricsModalProps) => {
*/
const debouncedBackendSearch = useMemo(
() =>
debounce(async (metricText: string, inferType: boolean) => {
debounce(async (metricText: string) => {
dispatch(setIsLoading(true));
const metrics = await getBackendSearchMetrics(metricText, query.labels, datasource, inferType);
const metrics = await getBackendSearchMetrics(metricText, query.labels, datasource);
dispatch(
filterMetricsBackend({
@ -150,9 +144,9 @@ export const MetricsModal = (props: MetricsModalProps) => {
function searchCallback(query: string, fullMetaSearchVal: boolean) {
if (state.useBackend && query === '') {
// get all metrics data if a user erases everything in the input
updateMetricsMetadata(state.inferType);
updateMetricsMetadata();
} else if (state.useBackend) {
debouncedBackendSearch(query, state.inferType);
debouncedBackendSearch(query);
} else {
// search either the names or all metadata
// fuzzy search go!
@ -200,31 +194,17 @@ export const MetricsModal = (props: MetricsModalProps) => {
onChange({ ...query, disableTextWrap: !state.disableTextWrap });
tracking('grafana_prom_metric_encycopedia_disable_text_wrap_interaction', state, '');
}}
onChangeInferType={() => {
const inferType = !state.inferType;
dispatch(setInferType(inferType));
// update the type
if (state.useBackend) {
// if there is no query yet, it will infer the type on the api call
if (state.fuzzySearchQuery !== '') {
debouncedBackendSearch(state.fuzzySearchQuery, inferType);
}
} else {
// updates the metadata with the inferred type
updateMetricsMetadata(inferType);
}
}}
onChangeUseBackend={() => {
const newVal = !state.useBackend;
dispatch(setUseBackend(newVal));
onChange({ ...query, useBackend: newVal });
if (newVal === false) {
// rebuild the metrics metadata if we turn off useBackend
updateMetricsMetadata(state.inferType);
updateMetricsMetadata();
} else {
// check if there is text in the browse search and update
if (state.fuzzySearchQuery !== '') {
debouncedBackendSearch(state.fuzzySearchQuery, state.inferType);
debouncedBackendSearch(state.fuzzySearchQuery);
}
// otherwise wait for user typing
}
@ -259,9 +239,6 @@ export const MetricsModal = (props: MetricsModalProps) => {
}}
/>
</div>
<div>
<Spinner className={`${styles.loadingSpinner} ${state.isLoading ? styles.visible : ''}`} />
</div>
{state.hasMetadata && (
<div className={styles.inputItem}>
<MultiSelect
@ -274,6 +251,9 @@ export const MetricsModal = (props: MetricsModalProps) => {
/>
</div>
)}
<div>
<Spinner className={`${styles.loadingSpinner} ${state.isLoading ? styles.visible : ''}`} />
</div>
<div className={styles.inputItem}>
<Toggletip
aria-label="Additional settings"
@ -287,10 +267,15 @@ export const MetricsModal = (props: MetricsModalProps) => {
size="md"
onClick={() => dispatch(showAdditionalSettings())}
data-testid={testIds.showAdditionalSettings}
className={styles.noBorder}
>
Additional Settings
</Button>
<Button variant="secondary" icon={state.showAdditionalSettings ? 'angle-up' : 'angle-down'} />
<Button
className={styles.noBorder}
variant="secondary"
icon={state.showAdditionalSettings ? 'angle-up' : 'angle-down'}
/>
</ButtonGroup>
</Toggletip>
</div>
@ -368,5 +353,4 @@ export const testIds = {
resultsPerPage: 'results-per-page',
setUseBackend: 'set-use-backend',
showAdditionalSettings: 'show-additional-settings',
inferType: 'set-infer-type',
};

View File

@ -3,8 +3,9 @@ import React, { ReactElement, useEffect, useRef } from 'react';
import Highlighter from 'react-highlight-words';
import { GrafanaTheme2 } from '@grafana/data';
import { Icon, Tooltip, useTheme2 } from '@grafana/ui';
import { Button, Icon, Tooltip, useTheme2 } from '@grafana/ui';
import { docsTip } from '../../../configuration/ConfigEditor';
import { PromVisualQuery } from '../../types';
import { tracking } from './state/helpers';
@ -51,15 +52,7 @@ export function ResultsTable(props: ResultsTableProps) {
if (state.fullMetaSearch && metric) {
return (
<>
<td>
<Highlighter
textToHighlight={metric.type ?? ''}
searchWords={state.metaHaystackMatches}
autoEscape
highlightClassName={`${styles.matchHighLight} ${metric.inferred ? styles.italicized : ''}`}
/>{' '}
{inferredType(metric.inferred ?? false)}
</td>
<td>{displayType(metric.type ?? '')}</td>
<td>
<Highlighter
textToHighlight={metric.description ?? ''}
@ -73,25 +66,49 @@ export function ResultsTable(props: ResultsTableProps) {
} else {
return (
<>
<td className={metric.inferred ? styles.italicized : ''}>
{metric.type ?? ''} {inferredType(metric.inferred ?? false)}
</td>
<td>{displayType(metric.type ?? '')}</td>
<td>{metric.description ?? ''}</td>
</>
);
}
}
function inferredType(inferred: boolean): JSX.Element | undefined {
if (inferred) {
return (
<Tooltip content={'This metric type has been inferred'} placement="bottom-end">
<Icon name="info-circle" size="xs" />
</Tooltip>
);
} else {
return undefined;
function addHelpIcon(fullType: string, descriptiveType: string, link: string) {
return (
<>
{fullType}
<span className={styles.tooltipSpace}>
<Tooltip
content={
<>
When creating a {descriptiveType}, Prometheus exposes multiple series with the type counter.{' '}
{docsTip(link)}
</>
}
placement="bottom-start"
interactive={true}
>
<Icon name="info-circle" size="xs" />
</Tooltip>
</span>
</>
);
}
function displayType(type: string | null) {
if (!type) {
return '';
}
if (type.includes('(summary)')) {
return addHelpIcon(type, 'summary', 'https://prometheus.io/docs/concepts/metric_types/#summary');
}
if (type.includes('(histogram)')) {
return addHelpIcon(type, 'histogram', 'https://prometheus.io/docs/concepts/metric_types/#histogram');
}
return type;
}
function noMetricsMessages(): ReactElement {
@ -105,7 +122,7 @@ export function ResultsTable(props: ResultsTableProps) {
message = 'There are no metrics found. Try to expand your label filters.';
}
if (state.fuzzySearchQuery) {
if (state.fuzzySearchQuery || state.selectedTypes.length > 0) {
message = 'There are no metrics found. Try to expand your search and filters.';
}
@ -116,6 +133,21 @@ export function ResultsTable(props: ResultsTableProps) {
);
}
function textHighlight(state: MetricsModalState) {
if (state.useBackend) {
// highlight the input only for the backend search
// this highlight is equivalent to how the metric select highlights
// look into matching on regex input
return [state.fuzzySearchQuery];
} else if (state.fullMetaSearch) {
// highlight the matches in the ufuzzy metaHaystack
return state.metaHaystackMatches;
} else {
// highlight the ufuzzy name matches
return state.nameHaystackMatches;
}
}
return (
<table className={styles.table} ref={tableRef}>
<thead className={styles.stickyHeader}>
@ -124,9 +156,10 @@ export function ResultsTable(props: ResultsTableProps) {
{state.hasMetadata && (
<>
<th className={`${styles.typeWidth} ${styles.tableHeaderPadding}`}>Type</th>
<th className={styles.tableHeaderPadding}>Description</th>
<th className={`${styles.descriptionWidth} ${styles.tableHeaderPadding}`}>Description</th>
</>
)}
<th className={styles.selectButtonWidth}> </th>
</tr>
</thead>
<tbody>
@ -137,8 +170,6 @@ export function ResultsTable(props: ResultsTableProps) {
<tr
key={metric?.value ?? idx}
className={`${styles.row} ${isSelectedRow(idx) ? `${styles.selectedRow} selected-row` : ''}`}
onClick={() => selectMetric(metric)}
tabIndex={0}
onFocus={() => onFocusRow(idx)}
onKeyDown={(e) => {
if (e.code === 'Enter' && e.currentTarget.classList.contains('selected-row')) {
@ -149,12 +180,22 @@ export function ResultsTable(props: ResultsTableProps) {
<td className={styles.nameOverflow}>
<Highlighter
textToHighlight={metric?.value ?? ''}
searchWords={state.fullMetaSearch ? state.metaHaystackMatches : state.nameHaystackMatches}
searchWords={textHighlight(state)}
autoEscape
highlightClassName={styles.matchHighLight}
/>
</td>
{state.hasMetadata && metaRows(metric)}
<td>
<Button
size="md"
variant="secondary"
onClick={() => selectMetric(metric)}
className={styles.centerButton}
>
Select
</Button>
</td>
</tr>
);
})}
@ -186,7 +227,6 @@ const getStyles = (theme: GrafanaTheme2, disableTextWrap: boolean) => {
`,
row: css`
label: row;
cursor: pointer;
border-bottom: 1px solid ${theme.colors.border.weak}
&:last-child {
border-bottom: 0;
@ -207,13 +247,19 @@ const getStyles = (theme: GrafanaTheme2, disableTextWrap: boolean) => {
background-color: ${theme.components.textHighlight.background};
`,
nameWidth: css`
${disableTextWrap ? '' : 'width: 40%;'}
${disableTextWrap ? '' : 'width: 37.5%;'}
`,
nameOverflow: css`
${disableTextWrap ? '' : 'overflow-wrap: anywhere;'}
`,
typeWidth: css`
${disableTextWrap ? '' : 'width: 16%;'}
${disableTextWrap ? '' : 'width: 15%;'}
`,
descriptionWidth: css`
${disableTextWrap ? '' : 'width: 35%;'}
`,
selectButtonWidth: css`
${disableTextWrap ? '' : 'width: 12.5%;'}
`,
stickyHeader: css`
position: sticky;
@ -224,8 +270,13 @@ const getStyles = (theme: GrafanaTheme2, disableTextWrap: boolean) => {
text-align: center;
color: ${theme.colors.text.secondary};
`,
italicized: css`
font-style: italic;
tooltipSpace: css`
margin-left: 4px;
`,
centerButton: css`
display: block;
margin: auto;
border: none;
`,
};
};

View File

@ -16,7 +16,6 @@ const { setFilteredMetricCount } = stateSlice.actions;
export async function setMetrics(
datasource: PrometheusDatasource,
query: PromVisualQuery,
inferType: boolean,
initialMetrics?: string[]
): Promise<MetricsModalMetadata> {
// metadata is set in the metric select now
@ -34,9 +33,9 @@ export async function setMetrics(
let metricsData: MetricsData | undefined;
metricsData = initialMetrics?.map((m: string) => {
const metricData = buildMetricData(m, inferType, datasource);
const metricData = buildMetricData(m, datasource);
const metaDataString = `${m}¦${metricData.type}¦${metricData.description}`;
const metaDataString = `${m}¦${metricData.description}`;
nameHaystackDictionaryData[m] = metricData;
metaHaystackDictionaryData[metaDataString] = metricData;
@ -56,34 +55,27 @@ export async function setMetrics(
}
/**
* Builds the metric data object with type, description and inferred flag
* Builds the metric data object with type and description
*
* @param metric The metric name
* @param inferType state attribute that the infer type setting is on or off
* @param datasource The Prometheus datasource for mapping metradata to the metric name
* @returns A MetricData object.
*/
function buildMetricData(metric: string, inferType: boolean, datasource: PrometheusDatasource): MetricData {
function buildMetricData(metric: string, datasource: PrometheusDatasource): MetricData {
let type = getMetadataType(metric, datasource.languageProvider.metricsMetadata!);
let inferredType;
if (!type && inferType) {
type = metricTypeHints(metric);
if (type) {
inferredType = true;
}
}
const description = getMetadataHelp(metric, datasource.languageProvider.metricsMetadata!);
if (description?.toLowerCase().includes('histogram') && type !== 'histogram') {
type += ' (histogram)';
}
['histogram', 'summary'].forEach((t) => {
if (description?.toLowerCase().includes(t) && type !== t) {
type += ` (${t})`;
}
});
const metricData: MetricData = {
value: metric,
type: type,
description: description,
inferred: inferredType,
};
return metricData;
@ -123,22 +115,21 @@ export function filterMetrics(state: MetricsModalState): MetricsData {
if (m.type && t.value) {
return m.type.includes(t.value);
}
if (!m.type && t.value === 'no type') {
return true;
}
return false;
});
// missing type
const hasNoType = !m.type;
return matchesSelectedType || (hasNoType && state.includeNullMetadata);
// when a user filters for type, only return metrics with defined types
return matchesSelectedType;
});
}
if (!state.includeNullMetadata) {
filteredMetrics = filteredMetrics.filter((m: MetricData) => {
if (state.inferType && m.inferred) {
return true;
}
return m.type !== undefined && m.description !== undefined;
});
}
@ -188,8 +179,7 @@ export const calculateResultsPerPage = (results: number, defaultResults: number,
export async function getBackendSearchMetrics(
metricText: string,
labels: QueryBuilderLabelFilter[],
datasource: PrometheusDatasource,
inferType: boolean
datasource: PrometheusDatasource
): Promise<Array<{ value: string }>> {
const queryString = regexifyLabelValuesQueryString(metricText);
@ -202,24 +192,10 @@ export async function getBackendSearchMetrics(
const results = datasource.metricFindQuery(params);
return await results.then((results) => {
return results.map((result) => buildMetricData(result.text, inferType, datasource));
return results.map((result) => buildMetricData(result.text, datasource));
});
}
function metricTypeHints(metric: string): string {
const histogramMetric = metric.match(/^\w+_bucket$|^\w+_bucket{.*}$/);
if (histogramMetric) {
return 'counter (histogram)';
}
const counterMatch = metric.match(/\b(\w+_(total|sum|count))\b/);
if (counterMatch) {
return 'counter';
}
return '';
}
export function tracking(event: string, state?: MetricsModalState | null, metric?: string, query?: PromVisualQuery) {
switch (event) {
case 'grafana_prom_metric_encycopedia_tracking':
@ -230,7 +206,6 @@ export function tracking(event: string, state?: MetricsModalState | null, metric
fuzzySearchQuery: state?.fuzzySearchQuery,
fullMetaSearch: state?.fullMetaSearch,
selectedTypes: state?.selectedTypes,
inferType: state?.inferType,
});
case 'grafana_prom_metric_encycopedia_disable_text_wrap_interaction':
reportInteraction(event, {
@ -263,13 +238,20 @@ export const promTypes: PromFilterOption[] = [
description:
'A summary samples observations (usually things like request durations and response sizes) and can calculate configurable quantiles over a sliding time window.',
},
{
value: 'unknown',
description: 'These metrics have been given the type unknown in the metadata.',
},
{
value: 'no type',
description: 'These metrics have no defined type in the metadata.',
},
];
export const placeholders = {
browse: 'Search metrics by name',
metadataSearchSwitch: 'Include search with type and description',
metadataSearchSwitch: 'Include description in search',
type: 'Filter by type',
includeNullMetadata: 'Include results with no metadata',
setUseBackend: 'Enable regex search',
inferType: 'Infer metric type',
};

View File

@ -84,9 +84,6 @@ export const stateSlice = createSlice({
showAdditionalSettings: (state) => {
state.showAdditionalSettings = !state.showAdditionalSettings;
},
setInferType: (state, action: PayloadAction<boolean>) => {
state.inferType = action.payload;
},
},
});
@ -117,7 +114,6 @@ export function initialState(query?: PromVisualQuery): MetricsModalState {
disableTextWrap: query?.disableTextWrap ?? false,
selectedIdx: 0,
showAdditionalSettings: false,
inferType: true,
};
}
@ -171,8 +167,6 @@ export interface MetricsModalState {
selectedIdx: number;
/** Display toggle switches for settings */
showAdditionalSettings: boolean;
/** Check metric to match on substrings to infer prometheus type */
inferType: boolean;
}
/**

View File

@ -17,10 +17,14 @@ export const getStyles = (theme: GrafanaTheme2, disableTextWrap: boolean) => {
display: flex;
flex-direction: row;
flex-wrap: wrap;
gap: ${theme.spacing(2)};
`,
inputItemFirst: css`
flex-basis: 40%;
padding-right: 16px;
${theme.breakpoints.down('md')} {
padding-right: 0px;
padding-bottom: 16px;
}
`,
inputItem: css`
flex-grow: 1;
@ -80,6 +84,9 @@ export const getStyles = (theme: GrafanaTheme2, disableTextWrap: boolean) => {
settingsBtn: css`
float: right;
`,
noBorder: css`
border: none;
`,
resultsPerPageLabel: css`
color: ${theme.colors.text.secondary};
opacity: 75%;

View File

@ -4,7 +4,6 @@ export type MetricData = {
value: string;
type?: string | null;
description?: string;
inferred?: boolean;
};
export type PromFilterOption = {