Glue: Validate target query in correlations page (#57245)
* feat: add draft version of validate button
* feat: add some styling and basics
* temp: intermediate result
* refactor: solve TODOs
* refactor: replace string in state
* refactor: replace error message style
* refactor: set validate state on change in ds
* refactor: add QueryRunner
* refactor: add QueryRunner
* temp: temporary status
* Emit PanelData to check if the query is valid
* refactor: clean up
* refactor: improve a11y of error message and adjust test
* Remove deprecated property call, change equality
* refactor: add changes from code review
* refactor: remove memory leak
* refactor: replace query runner
* refactor: adjust error handling
* refactor: move testing to related unit test
* refactor: clean up test for QueryEditorField
* refactor: clean up test for CorrelationsPage
* refactor: repair test
* refactor: clean up
* refactor: add refId in order avoid errors when running Loki queries
* refactor: replace buildQueryTransaction + set query to invalid if query is empty
* refactor: add empty query value to test cases
* refactor: end handleValidation after setIsValidQuery()
* refactor: refactor test
* refactor: fix last two tests
* refactor: modify validation
* refactor: add happy path
* refactor: clean up
* refactor: clean up tests (not final)
* refactor: further clean up
* refactor: add condition for failing
* refactor: finish clean up
* refactor: changes from code review
* refactor: add response state to condition
Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>
* refactor: fix prettier issue
* refactor: remove unused return
* refactor: replace change in queryAnalytics.ts
* refactor: remove correlations from query analytics
* refactor: remove unnecessary test preparation
* refactor: revert changes from commit 4997327
Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>
Co-authored-by: Kristina Durivage <kristina.durivage@grafana.com>
This commit is contained in:
co-authored by
Piotr Jamróz
Kristina Durivage
parent
321acca59f
commit
9400ccf478
@@ -16,6 +16,7 @@ export enum CoreApp {
|
|||||||
Unknown = 'unknown',
|
Unknown = 'unknown',
|
||||||
PanelEditor = 'panel-editor',
|
PanelEditor = 'panel-editor',
|
||||||
PanelViewer = 'panel-viewer',
|
PanelViewer = 'panel-viewer',
|
||||||
|
Correlations = 'correlations',
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface AppRootProps<T extends KeyValue = KeyValue> {
|
export interface AppRootProps<T extends KeyValue = KeyValue> {
|
||||||
|
|||||||
@@ -304,7 +304,7 @@ describe('CorrelationsPage', () => {
|
|||||||
expect(screen.getByRole('button', { name: /add$/i })).toBeInTheDocument();
|
expect(screen.getByRole('button', { name: /add$/i })).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('correctly adds correlations', async () => {
|
it('correctly adds first correlation', async () => {
|
||||||
const CTAButton = screen.getByRole('button', { name: /add correlation/i });
|
const CTAButton = screen.getByRole('button', { name: /add correlation/i });
|
||||||
expect(CTAButton).toBeInTheDocument();
|
expect(CTAButton).toBeInTheDocument();
|
||||||
|
|
||||||
@@ -440,7 +440,7 @@ describe('CorrelationsPage', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('correctly adds correlations', async () => {
|
it('correctly adds new correlation', async () => {
|
||||||
const addNewButton = screen.getByRole('button', { name: /add new/i });
|
const addNewButton = screen.getByRole('button', { name: /add new/i });
|
||||||
expect(addNewButton).toBeInTheDocument();
|
expect(addNewButton).toBeInTheDocument();
|
||||||
fireEvent.click(addNewButton);
|
fireEvent.click(addNewButton);
|
||||||
|
|||||||
@@ -1,15 +1,16 @@
|
|||||||
import { render, screen } from '@testing-library/react';
|
import { fireEvent, render, screen, waitFor, waitForElementToBeRemoved } from '@testing-library/react';
|
||||||
import React, { ReactNode } from 'react';
|
import React, { ReactNode } from 'react';
|
||||||
import { FormProvider, useForm } from 'react-hook-form';
|
import { FormProvider, useForm } from 'react-hook-form';
|
||||||
import { MockDataSourceApi } from 'test/mocks/datasource_srv';
|
import { MockDataSourceApi } from 'test/mocks/datasource_srv';
|
||||||
|
|
||||||
|
import { LoadingState } from '@grafana/data';
|
||||||
import { setDataSourceSrv } from '@grafana/runtime';
|
import { setDataSourceSrv } from '@grafana/runtime';
|
||||||
import { MockDataSourceSrv } from 'app/features/alerting/unified/mocks';
|
import { MockDataSourceSrv } from 'app/features/alerting/unified/mocks';
|
||||||
|
|
||||||
import { QueryEditorField } from './QueryEditorField';
|
import { QueryEditorField } from './QueryEditorField';
|
||||||
|
|
||||||
const Wrapper = ({ children }: { children: ReactNode }) => {
|
const Wrapper = ({ children }: { children: ReactNode }) => {
|
||||||
const methods = useForm();
|
const methods = useForm({ defaultValues: { query: {} } });
|
||||||
return <FormProvider {...methods}>{children}</FormProvider>;
|
return <FormProvider {...methods}>{children}</FormProvider>;
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -21,7 +22,7 @@ const defaultGetHandler = async (name: string) => {
|
|||||||
return dsApi;
|
return dsApi;
|
||||||
};
|
};
|
||||||
|
|
||||||
const renderWithContext = async (
|
const renderWithContext = (
|
||||||
children: ReactNode,
|
children: ReactNode,
|
||||||
getHandler: (name: string) => Promise<MockDataSourceApi> = defaultGetHandler
|
getHandler: (name: string) => Promise<MockDataSourceApi> = defaultGetHandler
|
||||||
) => {
|
) => {
|
||||||
@@ -33,7 +34,24 @@ const renderWithContext = async (
|
|||||||
render(<Wrapper>{children}</Wrapper>);
|
render(<Wrapper>{children}</Wrapper>);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const initiateDsApi = () => {
|
||||||
|
const dsApi = new MockDataSourceApi('dsApiMock');
|
||||||
|
dsApi.components = {
|
||||||
|
QueryEditor: () => <>query editor</>,
|
||||||
|
};
|
||||||
|
|
||||||
|
renderWithContext(<QueryEditorField name="query" dsUid="randomDsUid" />, async () => {
|
||||||
|
return dsApi;
|
||||||
|
});
|
||||||
|
|
||||||
|
return dsApi;
|
||||||
|
};
|
||||||
|
|
||||||
describe('QueryEditorField', () => {
|
describe('QueryEditorField', () => {
|
||||||
|
afterAll(() => {
|
||||||
|
jest.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
it('should render the query editor', async () => {
|
it('should render the query editor', async () => {
|
||||||
renderWithContext(<QueryEditorField name="query" dsUid="test" />);
|
renderWithContext(<QueryEditorField name="query" dsUid="test" />);
|
||||||
|
|
||||||
@@ -63,4 +81,90 @@ describe('QueryEditorField', () => {
|
|||||||
await screen.findByRole('alert', { name: 'Data source does not export a query editor.' })
|
await screen.findByRole('alert', { name: 'Data source does not export a query editor.' })
|
||||||
).toBeInTheDocument();
|
).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('Query validation', () => {
|
||||||
|
it('should result in succeeded validation if LoadingState.Done and data is available', async () => {
|
||||||
|
const dsApi = initiateDsApi();
|
||||||
|
|
||||||
|
await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i));
|
||||||
|
|
||||||
|
dsApi.result = {
|
||||||
|
data: [
|
||||||
|
{
|
||||||
|
name: 'test',
|
||||||
|
fields: [],
|
||||||
|
length: 1,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
state: LoadingState.Done,
|
||||||
|
};
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: /Validate query$/i }));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText('This query is valid.')).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should result in failed validation if LoadingState.Error and data is not available', async () => {
|
||||||
|
const dsApi = initiateDsApi();
|
||||||
|
|
||||||
|
await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i));
|
||||||
|
|
||||||
|
dsApi.result = {
|
||||||
|
data: [],
|
||||||
|
state: LoadingState.Error,
|
||||||
|
};
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: /Validate query$/i }));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
const alertEl = screen.getByRole('alert');
|
||||||
|
expect(alertEl).toBeInTheDocument();
|
||||||
|
expect(alertEl).toHaveTextContent(/this query is not valid/i);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should result in failed validation if LoadingState.Error and data is available', async () => {
|
||||||
|
const dsApi = initiateDsApi();
|
||||||
|
|
||||||
|
await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i));
|
||||||
|
|
||||||
|
dsApi.result = {
|
||||||
|
data: [
|
||||||
|
{
|
||||||
|
name: 'test',
|
||||||
|
fields: [],
|
||||||
|
length: 1,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
state: LoadingState.Error,
|
||||||
|
};
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: /Validate query$/i }));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
const alertEl = screen.getByRole('alert');
|
||||||
|
expect(alertEl).toBeInTheDocument();
|
||||||
|
expect(alertEl).toHaveTextContent(/this query is not valid/i);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should result in failed validation if result with LoadingState.Done and data is not available', async () => {
|
||||||
|
const dsApi = initiateDsApi();
|
||||||
|
|
||||||
|
await waitForElementToBeRemoved(() => screen.queryByText(/loading query editor/i));
|
||||||
|
|
||||||
|
dsApi.result = {
|
||||||
|
data: [],
|
||||||
|
state: LoadingState.Done,
|
||||||
|
};
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: /Validate query$/i }));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByText('This query is not valid.')).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,9 +1,24 @@
|
|||||||
import React from 'react';
|
import { css } from '@emotion/css';
|
||||||
|
import React, { useState } from 'react';
|
||||||
import { Controller } from 'react-hook-form';
|
import { Controller } from 'react-hook-form';
|
||||||
import { useAsync } from 'react-use';
|
import { useAsync } from 'react-use';
|
||||||
|
|
||||||
|
import { CoreApp, DataQuery, getDefaultTimeRange, GrafanaTheme2 } from '@grafana/data';
|
||||||
import { getDataSourceSrv } from '@grafana/runtime';
|
import { getDataSourceSrv } from '@grafana/runtime';
|
||||||
import { Field, LoadingPlaceholder, Alert } from '@grafana/ui';
|
import {
|
||||||
|
Field,
|
||||||
|
LoadingPlaceholder,
|
||||||
|
Alert,
|
||||||
|
Button,
|
||||||
|
HorizontalGroup,
|
||||||
|
Icon,
|
||||||
|
FieldValidationMessage,
|
||||||
|
useStyles2,
|
||||||
|
} from '@grafana/ui';
|
||||||
|
|
||||||
|
import { generateKey } from '../../../core/utils/explore';
|
||||||
|
import { QueryTransaction } from '../../../types';
|
||||||
|
import { runRequest } from '../../query/state/runRequest';
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
dsUid?: string;
|
dsUid?: string;
|
||||||
@@ -12,7 +27,19 @@ interface Props {
|
|||||||
error?: string;
|
error?: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function getStyle(theme: GrafanaTheme2) {
|
||||||
|
return {
|
||||||
|
valid: css`
|
||||||
|
color: ${theme.colors.success.text};
|
||||||
|
`,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
export const QueryEditorField = ({ dsUid, invalid, error, name }: Props) => {
|
export const QueryEditorField = ({ dsUid, invalid, error, name }: Props) => {
|
||||||
|
const [isValidQuery, setIsValidQuery] = useState<boolean | undefined>(undefined);
|
||||||
|
|
||||||
|
const style = useStyles2(getStyle);
|
||||||
|
|
||||||
const {
|
const {
|
||||||
value: datasource,
|
value: datasource,
|
||||||
loading: dsLoading,
|
loading: dsLoading,
|
||||||
@@ -23,8 +50,56 @@ export const QueryEditorField = ({ dsUid, invalid, error, name }: Props) => {
|
|||||||
}
|
}
|
||||||
return getDataSourceSrv().get(dsUid);
|
return getDataSourceSrv().get(dsUid);
|
||||||
}, [dsUid]);
|
}, [dsUid]);
|
||||||
|
|
||||||
const QueryEditor = datasource?.components?.QueryEditor;
|
const QueryEditor = datasource?.components?.QueryEditor;
|
||||||
|
|
||||||
|
const handleValidation = (value: DataQuery) => {
|
||||||
|
const interval = '1s';
|
||||||
|
const intervalMs = 1000;
|
||||||
|
const id = generateKey();
|
||||||
|
const queries = [{ ...value, refId: 'A' }];
|
||||||
|
|
||||||
|
const transaction: QueryTransaction = {
|
||||||
|
queries,
|
||||||
|
request: {
|
||||||
|
app: CoreApp.Correlations,
|
||||||
|
timezone: 'utc',
|
||||||
|
startTime: Date.now(),
|
||||||
|
interval,
|
||||||
|
intervalMs,
|
||||||
|
targets: queries,
|
||||||
|
range: getDefaultTimeRange(),
|
||||||
|
requestId: 'correlations_' + id,
|
||||||
|
scopedVars: {
|
||||||
|
__interval: { text: interval, value: interval },
|
||||||
|
__interval_ms: { text: intervalMs, value: intervalMs },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
id,
|
||||||
|
done: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
if (datasource) {
|
||||||
|
runRequest(datasource, transaction.request).subscribe((panelData) => {
|
||||||
|
if (
|
||||||
|
!panelData ||
|
||||||
|
panelData.state === 'Error' ||
|
||||||
|
(panelData.state === 'Done' && panelData.series.length === 0)
|
||||||
|
) {
|
||||||
|
setIsValidQuery(false);
|
||||||
|
} else if (
|
||||||
|
panelData.state === 'Done' &&
|
||||||
|
panelData.series.length > 0 &&
|
||||||
|
Boolean(panelData.series.find((element) => element.length > 0))
|
||||||
|
) {
|
||||||
|
setIsValidQuery(true);
|
||||||
|
} else {
|
||||||
|
setIsValidQuery(undefined);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Field label="Query" invalid={invalid} error={error}>
|
<Field label="Query" invalid={invalid} error={error}>
|
||||||
<Controller
|
<Controller
|
||||||
@@ -52,8 +127,31 @@ export const QueryEditorField = ({ dsUid, invalid, error, name }: Props) => {
|
|||||||
if (!QueryEditor) {
|
if (!QueryEditor) {
|
||||||
return <Alert title="Data source does not export a query editor."></Alert>;
|
return <Alert title="Data source does not export a query editor."></Alert>;
|
||||||
}
|
}
|
||||||
|
return (
|
||||||
return <QueryEditor onRunQuery={() => {}} onChange={onChange} datasource={datasource} query={value} />;
|
<>
|
||||||
|
<QueryEditor
|
||||||
|
onRunQuery={() => handleValidation(value)}
|
||||||
|
onChange={(value) => {
|
||||||
|
setIsValidQuery(undefined);
|
||||||
|
onChange(value);
|
||||||
|
}}
|
||||||
|
datasource={datasource}
|
||||||
|
query={value}
|
||||||
|
/>
|
||||||
|
<HorizontalGroup justify="flex-end">
|
||||||
|
{isValidQuery ? (
|
||||||
|
<div className={style.valid}>
|
||||||
|
<Icon name="check" /> This query is valid.
|
||||||
|
</div>
|
||||||
|
) : isValidQuery === false ? (
|
||||||
|
<FieldValidationMessage>This query is not valid.</FieldValidationMessage>
|
||||||
|
) : null}
|
||||||
|
<Button variant="secondary" icon={'check'} type="button" onClick={() => handleValidation(value)}>
|
||||||
|
Validate query
|
||||||
|
</Button>
|
||||||
|
</HorizontalGroup>
|
||||||
|
</>
|
||||||
|
);
|
||||||
}}
|
}}
|
||||||
/>
|
/>
|
||||||
</Field>
|
</Field>
|
||||||
|
|||||||
@@ -138,11 +138,10 @@ describe('emitDataRequestEvent - from a dashboard panel', () => {
|
|||||||
datasourceName: datasource.name,
|
datasourceName: datasource.name,
|
||||||
datasourceId: datasource.id,
|
datasourceId: datasource.id,
|
||||||
datasourceUid: datasource.uid,
|
datasourceUid: datasource.uid,
|
||||||
|
datasourceType: datasource.type,
|
||||||
|
source: 'dashboard',
|
||||||
panelId: 2,
|
panelId: 2,
|
||||||
dashboardId: 1,
|
dashboardId: 1,
|
||||||
dashboardName: 'Test Dashboard',
|
|
||||||
dashboardUid: 'test',
|
|
||||||
folderName: 'Test Folder',
|
|
||||||
dataSize: 0,
|
dataSize: 0,
|
||||||
duration: 1,
|
duration: 1,
|
||||||
totalQueries: 0,
|
totalQueries: 0,
|
||||||
@@ -162,11 +161,10 @@ describe('emitDataRequestEvent - from a dashboard panel', () => {
|
|||||||
datasourceName: datasource.name,
|
datasourceName: datasource.name,
|
||||||
datasourceId: datasource.id,
|
datasourceId: datasource.id,
|
||||||
datasourceUid: datasource.uid,
|
datasourceUid: datasource.uid,
|
||||||
|
datasourceType: datasource.type,
|
||||||
|
source: 'dashboard',
|
||||||
panelId: 2,
|
panelId: 2,
|
||||||
dashboardId: 1,
|
dashboardId: 1,
|
||||||
dashboardName: 'Test Dashboard',
|
|
||||||
dashboardUid: 'test',
|
|
||||||
folderName: 'Test Folder',
|
|
||||||
dataSize: 2,
|
dataSize: 2,
|
||||||
duration: 1,
|
duration: 1,
|
||||||
totalQueries: 2,
|
totalQueries: 2,
|
||||||
@@ -186,11 +184,10 @@ describe('emitDataRequestEvent - from a dashboard panel', () => {
|
|||||||
datasourceName: datasource.name,
|
datasourceName: datasource.name,
|
||||||
datasourceId: datasource.id,
|
datasourceId: datasource.id,
|
||||||
datasourceUid: datasource.uid,
|
datasourceUid: datasource.uid,
|
||||||
|
datasourceType: datasource.type,
|
||||||
|
source: 'dashboard',
|
||||||
panelId: 2,
|
panelId: 2,
|
||||||
dashboardId: 1,
|
dashboardId: 1,
|
||||||
dashboardName: 'Test Dashboard',
|
|
||||||
dashboardUid: 'test',
|
|
||||||
folderName: 'Test Folder',
|
|
||||||
dataSize: 2,
|
dataSize: 2,
|
||||||
duration: 1,
|
duration: 1,
|
||||||
totalQueries: 1,
|
totalQueries: 1,
|
||||||
|
|||||||
@@ -31,8 +31,8 @@ export function emitDataRequestEvent(datasource: DataSourceApi) {
|
|||||||
duration: data.request.endTime! - data.request.startTime,
|
duration: data.request.endTime! - data.request.startTime,
|
||||||
};
|
};
|
||||||
|
|
||||||
if (data.request.app === CoreApp.Explore) {
|
if (data.request.app === CoreApp.Explore || data.request.app === CoreApp.Correlations) {
|
||||||
enrichWithExploreInfo(eventData, data);
|
enrichWithInfo(eventData, data);
|
||||||
} else {
|
} else {
|
||||||
enrichWithDashboardInfo(eventData, data);
|
enrichWithDashboardInfo(eventData, data);
|
||||||
}
|
}
|
||||||
@@ -49,7 +49,7 @@ export function emitDataRequestEvent(datasource: DataSourceApi) {
|
|||||||
done = true;
|
done = true;
|
||||||
};
|
};
|
||||||
|
|
||||||
function enrichWithExploreInfo(eventData: DataRequestEventPayload, data: PanelData) {
|
function enrichWithInfo(eventData: DataRequestEventPayload, data: PanelData) {
|
||||||
const totalQueries = Object.keys(data.series).length;
|
const totalQueries = Object.keys(data.series).length;
|
||||||
eventData.totalQueries = totalQueries;
|
eventData.totalQueries = totalQueries;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ export class DatasourceSrvMock {
|
|||||||
export class MockDataSourceApi extends DataSourceApi {
|
export class MockDataSourceApi extends DataSourceApi {
|
||||||
result: DataQueryResponse = { data: [] };
|
result: DataQueryResponse = { data: [] };
|
||||||
|
|
||||||
constructor(name?: string, result?: DataQueryResponse, meta?: any, private error: string | null = null) {
|
constructor(name?: string, result?: DataQueryResponse, meta?: any, public error: string | null = null) {
|
||||||
super({ name: name ? name : 'MockDataSourceApi' } as DataSourceInstanceSettings);
|
super({ name: name ? name : 'MockDataSourceApi' } as DataSourceInstanceSettings);
|
||||||
if (result) {
|
if (result) {
|
||||||
this.result = result;
|
this.result = result;
|
||||||
|
|||||||
Reference in New Issue
Block a user