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

Loki: Make label browser accessible in query builder #58525

Merged
merged 28 commits into from Nov 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
da5e84d
add label browser button to query editor header
gwdawson Nov 9, 2022
9eb2e76
add dynamic button label text
gwdawson Nov 10, 2022
b5749c8
add LabelBrowserModal.tsx
gwdawson Nov 10, 2022
b5cedad
toggle label browser modal on click
gwdawson Nov 10, 2022
9a37336
pass required props to LabelBrowserModal
gwdawson Nov 10, 2022
970c65c
add placeholder to text input
gwdawson Nov 10, 2022
b6dfe96
render label browser inside of the modal
gwdawson Nov 10, 2022
94d910b
change button based on label status
gwdawson Nov 10, 2022
a7e04b5
remove label browser button from code mode
gwdawson Nov 10, 2022
a36d28c
fix element overlap in label browser
gwdawson Nov 10, 2022
e65401f
fix undefined app in feature tracking
gwdawson Nov 11, 2022
04fac42
remove all any types
gwdawson Nov 11, 2022
fa3ae6b
add tests for label browser button
gwdawson Nov 11, 2022
2ef5fa1
update modal component width
gwdawson Nov 21, 2022
44a399b
update label loading function
gwdawson Nov 21, 2022
5160d9c
add tests to LabelBrowserModal
gwdawson Nov 21, 2022
f81e32a
fix broken mock datasource
gwdawson Nov 21, 2022
2e5fcb1
update test names
gwdawson Nov 21, 2022
0055cfb
use stack component for button spacing
gwdawson Nov 22, 2022
adb9dc7
revert modal width
gwdawson Nov 22, 2022
527e0a2
update label search placeholder
gwdawson Nov 22, 2022
7be5f0a
remove unused import
gwdawson Nov 22, 2022
7da15c7
add test assertion for closed modal
gwdawson Nov 22, 2022
6bedc95
remove redundant if statement
gwdawson Nov 22, 2022
ee99f0a
remove unnecessary code
gwdawson Nov 23, 2022
4bb3b9d
update error message and fix position
gwdawson Nov 23, 2022
ae66de0
fix input placeholder text
gwdawson Nov 23, 2022
c6253eb
Merge branch 'main' into gareth/add-label-browser-to-builder-mode
gwdawson Nov 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -131,16 +131,11 @@ const getStyles = (theme: GrafanaTheme2) => ({
margin-bottom: ${theme.spacing(1)};
`,
status: css`
padding: ${theme.spacing(0.5)};
margin-bottom: ${theme.spacing(1)};
color: ${theme.colors.text.secondary};
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
/* using absolute positioning because flex interferes with ellipsis */
position: absolute;
width: 50%;
right: 0;
text-align: right;
transition: opacity 100ms linear;
opacity: 0;
`,
Expand Down Expand Up @@ -367,7 +362,7 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
async fetchSeries(selector: string, lastFacetted?: string) {
const { languageProvider } = this.props;
if (lastFacetted) {
this.updateLabelState(lastFacetted, { loading: true }, `Facetting labels for ${selector}`);
this.updateLabelState(lastFacetted, { loading: true }, `Loading labels for ${selector}`);
}
try {
const possibleLabels = await languageProvider.fetchSeriesLabels(selector, true);
Expand Down Expand Up @@ -467,7 +462,12 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
2. Find values for the selected labels
</Label>
<div>
<Input onChange={this.onChangeSearch} aria-label="Filter expression for values" value={searchTerm} />
<Input
onChange={this.onChangeSearch}
aria-label="Filter expression for values"
value={searchTerm}
placeholder={'Enter a label value'}
/>
</div>
<div className={styles.valueListArea}>
{selectedLabels.map((label) => (
gwdawson marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -520,6 +520,9 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
{selector}
</div>
{validationStatus && <div className={styles.validationStatus}>{validationStatus}</div>}
<div className={cx(styles.status, (status || error) && styles.statusShowing)}>
<span className={error ? styles.error : ''}>{error || status}</span>
</div>
<HorizontalGroup>
<Button aria-label="Use selector as logs button" disabled={empty} onClick={this.onClickRunLogsQuery}>
Show logs
Expand All @@ -543,9 +546,6 @@ export class UnthemedLokiLabelBrowser extends React.Component<BrowserProps, Brow
<Button aria-label="Selector clear button" variant="secondary" onClick={this.onClickClear}>
Clear
</Button>
<div className={cx(styles.status, (status || error) && styles.statusShowing)}>
<span className={error ? styles.error : ''}>{error || status}</span>
</div>
</HorizontalGroup>
</div>
</div>
Expand Down
Expand Up @@ -149,6 +149,11 @@ describe('LokiQueryEditorSelector', () => {
expect(screen.getByText('Rate')).toBeInTheDocument();
expect(screen.getByText('$__interval')).toBeInTheDocument();
});

it('renders the label browser button', async () => {
renderWithMode(QueryEditorMode.Code);
expect(await screen.findByTestId('label-browser-button')).toBeInTheDocument();
});
});

function renderWithMode(mode: QueryEditorMode) {
Expand Down
89 changes: 68 additions & 21 deletions public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx
Expand Up @@ -2,14 +2,15 @@ import React, { SyntheticEvent, useCallback, useEffect, useState } from 'react';

import { CoreApp, LoadingState } from '@grafana/data';
import { selectors } from '@grafana/e2e-selectors';
import { EditorHeader, EditorRows, FlexItem, Space } from '@grafana/experimental';
import { EditorHeader, EditorRows, FlexItem, Space, Stack } from '@grafana/experimental';
import { reportInteraction } from '@grafana/runtime';
import { Button, ConfirmModal } from '@grafana/ui';
import { QueryEditorModeToggle } from 'app/plugins/datasource/prometheus/querybuilder/shared/QueryEditorModeToggle';
import { QueryHeaderSwitch } from 'app/plugins/datasource/prometheus/querybuilder/shared/QueryHeaderSwitch';
import { QueryEditorMode } from 'app/plugins/datasource/prometheus/querybuilder/shared/types';

import { lokiQueryEditorExplainKey, useFlag } from '../../prometheus/querybuilder/shared/hooks/useFlag';
import { LabelBrowserModal } from '../querybuilder/components/LabelBrowserModal';
import { LokiQueryBuilderContainer } from '../querybuilder/components/LokiQueryBuilderContainer';
import { LokiQueryBuilderOptions } from '../querybuilder/components/LokiQueryBuilderOptions';
import { LokiQueryCodeEditor } from '../querybuilder/components/LokiQueryCodeEditor';
Expand All @@ -25,10 +26,12 @@ export const testIds = {
};

export const LokiQueryEditor = React.memo<LokiQueryEditorProps>((props) => {
const { onChange, onRunQuery, onAddQuery, data, app, queries } = props;
const { onChange, onRunQuery, onAddQuery, data, app, queries, datasource } = props;
const [parseModalOpen, setParseModalOpen] = useState(false);
const [queryPatternsModalOpen, setQueryPatternsModalOpen] = useState(false);
const [dataIsStale, setDataIsStale] = useState(false);
const [labelBrowserVisible, setLabelBrowserVisible] = useState(false);
const [labelsLoaded, setLabelsLoaded] = useState(false);
const { flag: explain, setFlag: setExplain } = useFlag(lokiQueryEditorExplainKey);

const query = getQueryWithDefaults(props.query);
Expand Down Expand Up @@ -70,6 +73,30 @@ export const LokiQueryEditor = React.memo<LokiQueryEditorProps>((props) => {
onChange(query);
};

const onClickChooserButton = () => {
gwdawson marked this conversation as resolved.
Show resolved Hide resolved
setLabelBrowserVisible((visible) => !visible);
};

const getChooserText = (logLabelsLoaded: boolean, hasLogLabels: boolean) => {
if (!logLabelsLoaded) {
return 'Loading labels...';
}
if (!hasLogLabels) {
return '(No labels found)';
}
return 'Label browser';
};

useEffect(() => {
datasource.languageProvider.start().then(() => {
setLabelsLoaded(true);
});
}, [datasource]);

const hasLogLabels = datasource.languageProvider.getLabelKeys().length > 0;
const labelBrowserText = getChooserText(labelsLoaded, hasLogLabels);
gwdawson marked this conversation as resolved.
Show resolved Hide resolved
const buttonDisabled = !(labelsLoaded && hasLogLabels);
gwdawson marked this conversation as resolved.
Show resolved Hide resolved

return (
<>
<ConfirmModal
Expand All @@ -93,25 +120,45 @@ export const LokiQueryEditor = React.memo<LokiQueryEditorProps>((props) => {
onAddQuery={onAddQuery}
/>
<EditorHeader>
<Button
aria-label={selectors.components.QueryBuilder.queryPatterns}
variant="secondary"
size="sm"
onClick={() => {
setQueryPatternsModalOpen((prevValue) => !prevValue);

const visualQuery = buildVisualQueryFromString(query.expr || '');
reportInteraction('grafana_loki_query_patterns_opened', {
version: 'v2',
app: app ?? '',
editorMode: query.editorMode,
preSelectedOperationsCount: visualQuery.query.operations.length,
preSelectedLabelsCount: visualQuery.query.labels.length,
});
}}
>
Kick start your query
</Button>
<LabelBrowserModal
gwdawson marked this conversation as resolved.
Show resolved Hide resolved
isOpen={labelBrowserVisible}
languageProvider={datasource.languageProvider}
query={query}
app={app}
onClose={() => setLabelBrowserVisible(false)}
onChange={onChangeInternal}
onRunQuery={onRunQuery}
/>
<Stack gap={1}>
<Button
aria-label={selectors.components.QueryBuilder.queryPatterns}
variant="secondary"
size="sm"
onClick={() => {
setQueryPatternsModalOpen((prevValue) => !prevValue);

const visualQuery = buildVisualQueryFromString(query.expr || '');
reportInteraction('grafana_loki_query_patterns_opened', {
version: 'v2',
app: app ?? '',
editorMode: query.editorMode,
preSelectedOperationsCount: visualQuery.query.operations.length,
preSelectedLabelsCount: visualQuery.query.labels.length,
});
}}
>
Kick start your query
</Button>
<Button
variant="secondary"
size="sm"
onClick={onClickChooserButton}
disabled={buttonDisabled}
data-testid="label-browser-button"
>
{labelBrowserText}
</Button>
</Stack>
<QueryHeaderSwitch label="Explain" value={explain} onChange={onExplainChange} />
<FlexItem grow={1} />
{app !== CoreApp.Explore && (
Expand Down
Expand Up @@ -13,7 +13,6 @@ import {
TypeaheadInput,
BracesPlugin,
DOMUtil,
Icon,
} from '@grafana/ui';
import { LocalStorageValueProvider } from 'app/core/components/LocalStorageValueProvider';

Expand All @@ -22,21 +21,10 @@ import { LokiDatasource } from '../datasource';
import { escapeLabelValueInSelector, shouldRefreshLabels } from '../languageUtils';
import { LokiQuery, LokiOptions } from '../types';

import { LokiLabelBrowser } from './LokiLabelBrowser';
import { MonacoQueryFieldWrapper } from './monaco-query-field/MonacoQueryFieldWrapper';

const LAST_USED_LABELS_KEY = 'grafana.datasources.loki.browser.labels';

function getChooserText(hasSyntax: boolean, hasLogLabels: boolean) {
if (!hasSyntax) {
return 'Loading labels...';
}
if (!hasLogLabels) {
return '(No labels found)';
}
return 'Label browser';
}

function willApplySuggestion(suggestion: string, { typeaheadContext, typeaheadText }: SuggestionsState): string {
// Modify suggestion based on context
switch (typeaheadContext) {
Expand Down Expand Up @@ -191,11 +179,6 @@ export class LokiQueryField extends React.PureComponent<LokiQueryFieldProps, Lok
onBlur,
} = this.props;

const { labelsLoaded, labelBrowserVisible } = this.state;
const hasLogLabels = datasource.languageProvider.getLabelKeys().length > 0;
const chooserText = getChooserText(labelsLoaded, hasLogLabels);
const buttonDisabled = !(labelsLoaded && hasLogLabels);

return (
<LocalStorageValueProvider<string[]> storageKey={LAST_USED_LABELS_KEY} defaultValue={[]}>
{(lastUsedLabels, onLastUsedLabelsSave, onLastUsedLabelsDelete) => {
Expand All @@ -205,14 +188,6 @@ export class LokiQueryField extends React.PureComponent<LokiQueryFieldProps, Lok
className="gf-form-inline gf-form-inline--xs-view-flex-column flex-grow-1"
data-testid={this.props['data-testid']}
>
<button
className="gf-form-label query-keyword pointer"
onClick={this.onClickChooserButton}
disabled={buttonDisabled}
>
{chooserText}
<Icon name={labelBrowserVisible ? 'angle-down' : 'angle-right'} />
</button>
<div className="gf-form gf-form--grow flex-shrink-1 min-width-15">
{config.featureToggles.lokiMonacoEditor ? (
<MonacoQueryFieldWrapper
Expand All @@ -239,19 +214,6 @@ export class LokiQueryField extends React.PureComponent<LokiQueryFieldProps, Lok
)}
</div>
</div>
{labelBrowserVisible && (
<div className="gf-form">
<LokiLabelBrowser
languageProvider={datasource.languageProvider}
onChange={this.onChangeLabelBrowser}
lastUsedLabels={lastUsedLabels || []}
storeLastUsedLabels={onLastUsedLabelsSave}
deleteLastUsedLabels={onLastUsedLabelsDelete}
app={app}
/>
</div>
)}

{ExtraFieldElement}
</>
);
Expand Down
@@ -0,0 +1,42 @@
import { render, screen } from '@testing-library/react';
import React from 'react';

import { LokiDatasource } from '../../datasource';
import { createLokiDatasource } from '../../mocks';
import { LokiQuery } from '../../types';

import { LabelBrowserModal, Props } from './LabelBrowserModal';

jest.mock('@grafana/runtime', () => ({
...jest.requireActual('@grafana/runtime'),
reportInteraction: jest.fn(),
}));

describe('LabelBrowserModal', () => {
let datasource: LokiDatasource, props: Props;

beforeEach(() => {
datasource = createLokiDatasource();

props = {
isOpen: true,
languageProvider: datasource.languageProvider,
query: {} as LokiQuery,
onClose: jest.fn(),
onChange: jest.fn(),
onRunQuery: jest.fn(),
};

jest.spyOn(datasource, 'metadataRequest').mockResolvedValue({});
});

it('renders the label browser modal when open', () => {
render(<LabelBrowserModal {...props} />);
expect(screen.getByRole('heading', { name: /label browser/i })).toBeInTheDocument();
});
gwdawson marked this conversation as resolved.
Show resolved Hide resolved

it("doesn't render the label browser modal when closed", () => {
render(<LabelBrowserModal {...props} isOpen={false} />);
expect(screen.queryByRole('heading', { name: /label browser/i })).toBeNull();
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would also love to have a test case for onChange when we change the query. Feel free to drop it in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a good candidate for a follow-up task.

});
@@ -0,0 +1,57 @@
import React from 'react';

import { CoreApp } from '@grafana/data';
import { Modal } from '@grafana/ui';
import { LocalStorageValueProvider } from 'app/core/components/LocalStorageValueProvider';

import LanguageProvider from '../../LanguageProvider';
import { LokiLabelBrowser } from '../../components/LokiLabelBrowser';
import { LokiQuery } from '../../types';

export interface Props {
isOpen: boolean;
languageProvider: LanguageProvider;
query: LokiQuery;
app?: CoreApp;
onClose: () => void;
onChange: (query: LokiQuery) => void;
onRunQuery: () => void;
}

export const LabelBrowserModal = (props: Props) => {
const { isOpen, onClose, languageProvider, app } = props;

const LAST_USED_LABELS_KEY = 'grafana.datasources.loki.browser.labels';

const changeQuery = (value: string) => {
const { query, onChange, onRunQuery } = props;

const nextQuery = { ...query, expr: value };
onChange(nextQuery);
onRunQuery();
};

const onChange = (selector: string) => {
changeQuery(selector);
onClose();
};

return (
<Modal isOpen={isOpen} title="Label browser" onDismiss={onClose}>
<LocalStorageValueProvider<string[]> storageKey={LAST_USED_LABELS_KEY} defaultValue={[]}>
{(lastUsedLabels, onLastUsedLabelsSave, onLastUsedLabelsDelete) => {
return (
<LokiLabelBrowser
languageProvider={languageProvider}
onChange={onChange}
lastUsedLabels={lastUsedLabels}
storeLastUsedLabels={onLastUsedLabelsSave}
deleteLastUsedLabels={onLastUsedLabelsDelete}
app={app}
/>
);
}}
</LocalStorageValueProvider>
</Modal>
);
};