Correlations: Add transformation editor (#66217)

* There was an attempt

* Change disabled state based on transformation type

* Add validation to transformation type

* Revert "Add validation to transformation type"

This reverts commit 2188a3d9a93aec5eeafcdd40510391ba1a53671a.

* Add validation to transformation type

* Move transformations editor to a separate file

* Make name more descriptive

* Ensure type dropdown has always the same width

* Add tooltips around transformation options

* Slight style changes

* Remove autofocus on append, integrate read only to transformationeditor, save values that disappear so they come back

* Remove yaml changes

* Have variable background color work with alternating colors on different themes

* Make expression required for regular expressions

* Remove unused empty form object

* Fix bug about transformation’s values saved in memory

* Better validation formatting for expression

* Add labels and (for now) non working test, attempt to fix saved transformation delete/add bug

* Fix datalink comment

* Remove fancy CSS due to background change

* Fix deleting saved transformation bug, finish tests

* Consolidate transformation types

* Double check aria labels

* Change aria labels, fix tests

* Add a transformation with the create correlation test

---------

Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>
This commit is contained in:
Kristina 2023-04-18 07:17:30 -05:00 committed by GitHub
parent 15c9ced944
commit 9d69d3173f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 447 additions and 37 deletions

View File

@ -52,15 +52,20 @@ export interface DataLink<T extends DataQuery = any> {
origin?: DataLinkConfigOrigin;
}
/** @internal */
export enum SupportedTransformationTypes {
/**
* We provide tooltips with information about these to guide the user, please
* check for validity when adding more transformation types.
*
* @internal
*/
export enum SupportedTransformationType {
Regex = 'regex',
Logfmt = 'logfmt',
}
/** @internal */
export interface DataLinkTransformationConfig {
type: SupportedTransformationTypes;
type: SupportedTransformationType;
field?: string;
expression?: string;
mapValue?: string;

View File

@ -1,4 +1,4 @@
import { UseFormReturn, FieldValues, FieldErrors } from 'react-hook-form';
import { UseFormReturn, FieldValues, FieldErrors, FieldArrayMethodProps } from 'react-hook-form';
export type { SubmitHandler as FormsOnSubmit, FieldErrors as FormFieldErrors } from 'react-hook-form';
export type FormAPI<T extends FieldValues> = Omit<UseFormReturn<T>, 'trigger' | 'handleSubmit'> & {
@ -9,7 +9,7 @@ type FieldArrayValue = Partial<FieldValues> | Array<Partial<FieldValues>>;
export interface FieldArrayApi {
fields: Array<Record<string, any>>;
append: (value: FieldArrayValue) => void;
append: (value: FieldArrayValue, options?: FieldArrayMethodProps) => void;
prepend: (value: FieldArrayValue) => void;
remove: (index?: number | number[]) => void;
swap: (indexA: number, indexB: number) => void;

View File

@ -2,12 +2,13 @@ import { render, waitFor, screen, fireEvent, within, Matcher, getByRole } from '
import userEvent from '@testing-library/user-event';
import { merge, uniqueId } from 'lodash';
import React from 'react';
import { openMenu } from 'react-select-event';
import { Observable } from 'rxjs';
import { TestProvider } from 'test/helpers/TestProvider';
import { MockDataSourceApi } from 'test/mocks/datasource_srv';
import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock';
import { DataSourcePluginMeta } from '@grafana/data';
import { DataSourcePluginMeta, SupportedTransformationType } from '@grafana/data';
import { BackendSrv, setDataSourceSrv, BackendSrvRequest, reportInteraction } from '@grafana/runtime';
import { contextSrv } from 'app/core/services/context_srv';
import { configureStore } from 'app/store/configureStore';
@ -274,6 +275,14 @@ describe('CorrelationsPage', () => {
await userEvent.clear(screen.getByRole('textbox', { name: /results field/i }));
await userEvent.type(screen.getByRole('textbox', { name: /results field/i }), 'Line');
// add transformation
await userEvent.click(screen.getByRole('button', { name: /add transformation/i }));
const typeFilterSelect = screen.getAllByLabelText('Type');
openMenu(typeFilterSelect[0]);
await userEvent.click(screen.getByText('Regular expression'));
await userEvent.type(screen.getByLabelText(/expression/i), 'test expression');
await userEvent.click(await screen.findByRole('button', { name: /add$/i }));
expect(mocks.reportInteraction).toHaveBeenLastCalledWith('grafana_correlations_added');
@ -342,7 +351,14 @@ describe('CorrelationsPage', () => {
targetUID: 'loki',
uid: '1',
label: 'Some label',
config: { field: 'line', target: {}, type: 'query' },
config: {
field: 'line',
target: {},
type: 'query',
transformations: [
{ type: SupportedTransformationType.Regex, expression: 'url=http[s]?://(S*)', mapValue: 'path' },
],
},
},
{
sourceUID: 'prometheus',
@ -472,12 +488,64 @@ describe('CorrelationsPage', () => {
await userEvent.click(screen.getByRole('button', { name: /next$/i }));
await userEvent.click(screen.getByRole('button', { name: /next$/i }));
await userEvent.click(screen.getByRole('button', { name: /save$/i }));
expect(await screen.findByRole('cell', { name: /edited label$/i })).toBeInTheDocument();
expect(mocks.reportInteraction).toHaveBeenLastCalledWith('grafana_correlations_edited');
});
it('correctly edits transformations', async () => {
// wait for table to appear
await screen.findByRole('table');
const tableRows = queryRowsByCellValue('Source', 'loki');
const rowExpanderButton = within(tableRows[0]).getByRole('button', { name: /toggle row expanded/i });
await userEvent.click(rowExpanderButton);
await userEvent.click(screen.getByRole('button', { name: /next$/i }));
await userEvent.click(screen.getByRole('button', { name: /next$/i }));
// select Logfmt, be sure expression field is disabled
let typeFilterSelect = screen.getAllByLabelText('Type');
openMenu(typeFilterSelect[0]);
await userEvent.click(screen.getByText('Logfmt'));
let expressionInput = screen.queryByLabelText(/expression/i);
expect(expressionInput).toBeInTheDocument();
expect(expressionInput).toBeDisabled();
// select Regex, be sure expression field is not disabled and contains the former expression
openMenu(typeFilterSelect[0]);
await userEvent.click(screen.getByText('Regular expression', { selector: 'span' }));
expressionInput = screen.queryByLabelText(/expression/i);
expect(expressionInput).toBeInTheDocument();
expect(expressionInput).toBeEnabled();
expect(expressionInput).toHaveAttribute('value', 'url=http[s]?://(S*)');
// select Logfmt, delete, then add a new one to be sure the value is blank
openMenu(typeFilterSelect[0]);
await userEvent.click(screen.getByText('Logfmt'));
await userEvent.click(screen.getByRole('button', { name: /remove transformation/i }));
expressionInput = screen.queryByLabelText(/expression/i);
expect(expressionInput).not.toBeInTheDocument();
await userEvent.click(screen.getByRole('button', { name: /add transformation/i }));
typeFilterSelect = screen.getAllByLabelText('Type');
openMenu(typeFilterSelect[0]);
await userEvent.click(screen.getByText('Regular expression'));
expressionInput = screen.queryByLabelText(/expression/i);
expect(expressionInput).toBeInTheDocument();
expect(expressionInput).toBeEnabled();
expect(expressionInput).not.toHaveValue('url=http[s]?://(S*)');
await userEvent.click(screen.getByRole('button', { name: /save$/i }));
expect(screen.getByText('Please define an expression')).toBeInTheDocument();
await userEvent.type(screen.getByLabelText(/expression/i), 'test expression');
await userEvent.click(screen.getByRole('button', { name: /save$/i }));
expect(mocks.reportInteraction).toHaveBeenLastCalledWith('grafana_correlations_edited');
});
});
describe('Read only correlations', () => {
@ -487,7 +555,12 @@ describe('CorrelationsPage', () => {
targetUID: 'loki',
uid: '1',
label: 'Some label',
config: { field: 'line', target: {}, type: 'query' },
config: {
field: 'line',
target: {},
type: 'query',
transformations: [{ type: SupportedTransformationType.Regex, expression: '(?:msg)=' }],
},
},
];
@ -532,6 +605,16 @@ describe('CorrelationsPage', () => {
expect(descriptionInput).toBeInTheDocument();
expect(descriptionInput).toHaveAttribute('readonly');
await userEvent.click(screen.getByRole('button', { name: /next$/i }));
await userEvent.click(screen.getByRole('button', { name: /next$/i }));
// expect the transformation to exist but be read only
const expressionInput = screen.queryByLabelText(/expression/i);
expect(expressionInput).toBeInTheDocument();
expect(expressionInput).toHaveAttribute('readonly');
expect(screen.queryByRole('button', { name: 'add transformation' })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'remove transformation' })).not.toBeInTheDocument();
// we don't expect the save button to be rendered
expect(screen.queryByRole('button', { name: 'save' })).not.toBeInTheDocument();
});

View File

@ -9,6 +9,7 @@ import { getDatasourceSrv } from 'app/features/plugins/datasource_srv';
import { getVariableUsageInfo } from '../../explore/utils/links';
import { TransformationsEditor } from './TransformationsEditor';
import { useCorrelationsFormContext } from './correlationsFormContext';
import { getInputId } from './utils';
@ -33,12 +34,11 @@ export const ConfigureCorrelationSourceForm = () => {
const variables = getVariableUsageInfo(currentTargetQuery, {}).variables.map(
(variable) => variable.variableName + (variable.fieldPath ? `.${variable.fieldPath}` : '')
);
return (
<>
<FieldSet label="Configure source data source (3/3)">
<p>
Links are displayed with results of the selected origin source data. They shown along with the value of the
Links are displayed with results of the selected origin source data. They show along with the value of the
provided <em>results field</em>.
</p>
<Controller
@ -84,27 +84,26 @@ export const ConfigureCorrelationSourceForm = () => {
readOnly={readOnly}
/>
</Field>
{variables.length > 0 && (
<Card>
<Card.Heading>Variables used in the target query</Card.Heading>
<Card.Description>
<div>
You have used following variables in the target query:{' '}
{variables.map((name, i) => (
<span className={styles.variable} key={i}>
{name}
{i < variables.length - 1 ? ', ' : ''}
</span>
))}
</div>
<div>
A data point needs to provide values to all variables as fields or as transformations output to make the
correlation button appear in the visualization.
</div>
You have used following variables in the target query:{' '}
{variables.map((name, i) => (
<span className={styles.variable} key={i}>
{name}
{i < variables.length - 1 ? ', ' : ''}
</span>
))}
<br />A data point needs to provide values to all variables as fields or as transformations output to make
the correlation button appear in the visualization.
<br />
Note: Not every variable needs to be explicitly defined below. A transformation such as{' '}
<span className={styles.variable}>logfmt</span> will create variables for every key/value pair.
</Card.Description>
</Card>
)}
<TransformationsEditor readOnly={readOnly} />
</FieldSet>
</>
);

View File

@ -0,0 +1,315 @@
import { css } from '@emotion/css';
import { compact, fill } from 'lodash';
import React, { useState } from 'react';
import { useFormContext } from 'react-hook-form';
import { GrafanaTheme2, SupportedTransformationType } from '@grafana/data';
import { Stack } from '@grafana/experimental';
import {
Button,
Field,
FieldArray,
Icon,
IconButton,
Input,
InputControl,
Label,
Select,
Tooltip,
useStyles2,
} from '@grafana/ui';
type Props = { readOnly: boolean };
const getStyles = (theme: GrafanaTheme2) => ({
heading: css`
font-size: ${theme.typography.h5.fontSize};
font-weight: ${theme.typography.fontWeightRegular};
`,
// set fixed position from the top instead of centring as the container
// may get bigger when the for is invalid
removeButton: css`
margin-top: 25px;
`,
});
export const TransformationsEditor = (props: Props) => {
const { control, formState, register, setValue, watch, getValues } = useFormContext();
const { readOnly } = props;
const [keptVals, setKeptVals] = useState<Array<{ expression?: string; mapValue?: string }>>([]);
const styles = useStyles2(getStyles);
const transformOptions = getTransformOptions();
return (
<>
<input type="hidden" {...register('id')} />
<FieldArray name="config.transformations" control={control}>
{({ fields, append, remove }) => (
<>
<Stack direction="column" alignItems="flex-start">
<div className={styles.heading}>Transformations</div>
{fields.length === 0 && <div> No transformations defined.</div>}
{fields.length > 0 && (
<div>
{fields.map((fieldVal, index) => {
return (
<Stack direction="row" key={fieldVal.id} alignItems="top">
<Field
label={
<Stack gap={0.5}>
<Label htmlFor={`config.transformations.${fieldVal.id}-${index}.type`}>Type</Label>
<Tooltip
content={
<div>
<p>The type of transformation that will be applied to the source data.</p>
</div>
}
>
<Icon name="info-circle" size="sm" />
</Tooltip>
</Stack>
}
invalid={!!formState.errors?.config?.transformations?.[index]?.type}
error={formState.errors?.config?.transformations?.[index]?.type?.message}
validationMessageHorizontalOverflow={true}
>
<InputControl
render={({ field: { onChange, ref, ...field } }) => {
// input control field is not manipulated with remove, use value from control
return (
<Select
{...field}
value={fieldVal.type}
onChange={(value) => {
if (!readOnly) {
const currentValues = getValues().config.transformations[index];
let keptValsCopy = fill(Array(index + 1), {});
keptVals.forEach((keptVal, i) => (keptValsCopy[i] = keptVal));
keptValsCopy[index] = {
expression: currentValues.expression,
mapValue: currentValues.mapValue,
};
setKeptVals(keptValsCopy);
const newValueDetails = getSupportedTransTypeDetails(value.value);
if (newValueDetails.showExpression) {
setValue(
`config.transformations.${index}.expression`,
keptVals[index]?.expression || ''
);
} else {
setValue(`config.transformations.${index}.expression`, '');
}
if (newValueDetails.showMapValue) {
setValue(
`config.transformations.${index}.mapValue`,
keptVals[index]?.mapValue || ''
);
} else {
setValue(`config.transformations.${index}.mapValue`, '');
}
onChange(value.value);
}
}}
options={transformOptions}
width={25}
inputId={`config.transformations.${fieldVal.id}-${index}.type`}
/>
);
}}
control={control}
name={`config.transformations.${index}.type`}
rules={{ required: { value: true, message: 'Please select a transformation type' } }}
/>
</Field>
<Field
label={
<Stack gap={0.5}>
<Label htmlFor={`config.transformations.${fieldVal.id}.field`}>Field</Label>
<Tooltip
content={
<div>
<p>
Optional. The field to transform. If not specified, the transformation will be
applied to the results field.
</p>
</div>
}
>
<Icon name="info-circle" size="sm" />
</Tooltip>
</Stack>
}
>
<Input
{...register(`config.transformations.${index}.field`)}
readOnly={readOnly}
defaultValue={fieldVal.field}
label="field"
id={`config.transformations.${fieldVal.id}.field`}
/>
</Field>
<Field
label={
<Stack gap={0.5}>
<Label htmlFor={`config.transformations.${fieldVal.id}.expression`}>
Expression
{getSupportedTransTypeDetails(watch(`config.transformations.${index}.type`))
.requireExpression
? ' *'
: ''}
</Label>
<Tooltip
content={
<div>
<p>
Required for regular expression. The expression the transformation will use.
Logfmt does not use further specifications.
</p>
</div>
}
>
<Icon name="info-circle" size="sm" />
</Tooltip>
</Stack>
}
invalid={!!formState.errors?.config?.transformations?.[index]?.expression}
error={formState.errors?.config?.transformations?.[index]?.expression?.message}
>
<Input
{...register(`config.transformations.${index}.expression`, {
required: getSupportedTransTypeDetails(watch(`config.transformations.${index}.type`))
.requireExpression
? 'Please define an expression'
: undefined,
})}
defaultValue={fieldVal.expression}
readOnly={readOnly}
disabled={
!getSupportedTransTypeDetails(watch(`config.transformations.${index}.type`))
.showExpression
}
id={`config.transformations.${fieldVal.id}.expression`}
/>
</Field>
<Field
label={
<Stack gap={0.5}>
<Label htmlFor={`config.transformations.${fieldVal.id}.mapValue`}>Map value</Label>
<Tooltip
content={
<div>
<p>
Optional. Defines the name of the variable. This is currently only valid for
regular expressions with a single, unnamed capture group.
</p>
</div>
}
>
<Icon name="info-circle" size="sm" />
</Tooltip>
</Stack>
}
>
<Input
{...register(`config.transformations.${index}.mapValue`)}
defaultValue={fieldVal.mapValue}
readOnly={readOnly}
disabled={
!getSupportedTransTypeDetails(watch(`config.transformations.${index}.type`)).showMapValue
}
id={`config.transformations.${fieldVal.id}.mapValue`}
/>
</Field>
{!readOnly && (
<div className={styles.removeButton}>
<IconButton
type="button"
tooltip="Remove transformation"
name={'trash-alt'}
onClick={() => {
remove(index);
const keptValsCopy: Array<{ expression?: string; mapValue?: string } | undefined> = [
...keptVals,
];
keptValsCopy[index] = undefined;
setKeptVals(compact(keptValsCopy));
}}
ariaLabel="Remove transformation"
>
Remove
</IconButton>
</div>
)}
</Stack>
);
})}
</div>
)}
{!readOnly && (
<Button
icon="plus"
onClick={() => append({ type: undefined }, { shouldFocus: false })}
variant="secondary"
type="button"
>
Add transformation
</Button>
)}
</Stack>
</>
)}
</FieldArray>
</>
);
};
interface SupportedTransformationTypeDetails {
label: string;
value: string;
description?: string;
showExpression: boolean;
showMapValue: boolean;
requireExpression?: boolean;
}
function getSupportedTransTypeDetails(transType: SupportedTransformationType): SupportedTransformationTypeDetails {
switch (transType) {
case SupportedTransformationType.Logfmt:
return {
label: 'Logfmt',
value: SupportedTransformationType.Logfmt,
description: 'Parse provided field with logfmt to get variables',
showExpression: false,
showMapValue: false,
};
case SupportedTransformationType.Regex:
return {
label: 'Regular expression',
value: SupportedTransformationType.Regex,
description:
'Field will be parsed with regex. Use named capture groups to return multiple variables, or a single unnamed capture group to add variable to named map value.',
showExpression: true,
showMapValue: true,
requireExpression: true,
};
default:
return { label: transType, value: transType, showExpression: false, showMapValue: false };
}
}
const getTransformOptions = () => {
return Object.values(SupportedTransformationType).map((transformationType) => {
const transType = getSupportedTransTypeDetails(transformationType);
return {
label: transType.label,
value: transType.value,
description: transType.description,
};
});
};

View File

@ -1,3 +1,5 @@
import { SupportedTransformationType } from '@grafana/data';
import { CorrelationConfig } from '../types';
export interface FormDTO {
@ -9,3 +11,9 @@ export interface FormDTO {
}
export type EditFormDTO = Omit<FormDTO, 'targetUID' | 'sourceUID'>;
export type TransformationDTO = {
type: SupportedTransformationType;
expression?: string;
mapValue?: string;
};

View File

@ -1,6 +1,6 @@
import logfmt from 'logfmt';
import { ScopedVars, DataLinkTransformationConfig, SupportedTransformationTypes } from '@grafana/data';
import { ScopedVars, DataLinkTransformationConfig, SupportedTransformationType } from '@grafana/data';
import { safeStringifyValue } from 'app/core/utils/explore';
export const getTransformationVars = (
@ -10,7 +10,7 @@ export const getTransformationVars = (
): ScopedVars => {
let transformationScopedVars: ScopedVars = {};
let transformVal: { [key: string]: string | boolean | null | undefined } = {};
if (transformation.type === SupportedTransformationTypes.Regex && transformation.expression) {
if (transformation.type === SupportedTransformationType.Regex && transformation.expression) {
const regexp = new RegExp(transformation.expression, 'gi');
const matches = fieldValue.matchAll(regexp);
for (const match of matches) {
@ -20,7 +20,7 @@ export const getTransformationVars = (
transformVal[transformation.mapValue || fieldName] = match[1] || match[0];
}
}
} else if (transformation.type === SupportedTransformationTypes.Logfmt) {
} else if (transformation.type === SupportedTransformationType.Logfmt) {
transformVal = logfmt.parse(fieldValue);
}

View File

@ -8,7 +8,7 @@ import {
Field,
FieldType,
InterpolateFunction,
SupportedTransformationTypes,
SupportedTransformationType,
TimeRange,
toDataFrame,
} from '@grafana/data';
@ -285,8 +285,8 @@ describe('explore links utils', () => {
datasourceUid: 'uid_1',
datasourceName: 'test_ds',
transformations: [
{ type: SupportedTransformationTypes.Logfmt },
{ type: SupportedTransformationTypes.Regex, expression: 'host=(dev|prod)', mapValue: 'environment' },
{ type: SupportedTransformationType.Logfmt },
{ type: SupportedTransformationType.Regex, expression: 'host=(dev|prod)', mapValue: 'environment' },
],
},
};
@ -334,8 +334,8 @@ describe('explore links utils', () => {
datasourceUid: 'uid_1',
datasourceName: 'test_ds',
transformations: [
{ type: SupportedTransformationTypes.Regex, expression: 'fieldA=(asparagus|broccoli)' },
{ type: SupportedTransformationTypes.Regex, expression: 'fieldB=(apple|banana)' },
{ type: SupportedTransformationType.Regex, expression: 'fieldA=(asparagus|broccoli)' },
{ type: SupportedTransformationType.Regex, expression: 'fieldB=(apple|banana)' },
],
},
};
@ -375,7 +375,7 @@ describe('explore links utils', () => {
query: { query: 'http_requests{app=${application} isOnline=${online}}' },
datasourceUid: 'uid_1',
datasourceName: 'test_ds',
transformations: [{ type: SupportedTransformationTypes.Logfmt }],
transformations: [{ type: SupportedTransformationType.Logfmt }],
},
};
@ -414,7 +414,7 @@ describe('explore links utils', () => {
query: { query: 'http_requests{app=${application}}' },
datasourceUid: 'uid_1',
datasourceName: 'test_ds',
transformations: [{ type: SupportedTransformationTypes.Logfmt, field: 'fieldNamedInTransformation' }],
transformations: [{ type: SupportedTransformationType.Logfmt, field: 'fieldNamedInTransformation' }],
},
};
@ -468,7 +468,7 @@ describe('explore links utils', () => {
datasourceName: 'test_ds',
transformations: [
{
type: SupportedTransformationTypes.Regex,
type: SupportedTransformationType.Regex,
expression: '(?=.*(?<application>(grafana|loki)))(?=.*(?<environment>(dev|prod)))',
},
],
@ -546,7 +546,7 @@ describe('explore links utils', () => {
query: { query: 'http_requests{app=${application} env=${diffVar}}' },
datasourceUid: 'uid_1',
datasourceName: 'test_ds',
transformations: [{ type: SupportedTransformationTypes.Logfmt }],
transformations: [{ type: SupportedTransformationType.Logfmt }],
},
};
@ -571,7 +571,7 @@ describe('explore links utils', () => {
query: { query: 'http_requests{app=test}' },
datasourceUid: 'uid_1',
datasourceName: 'test_ds',
transformations: [{ type: SupportedTransformationTypes.Logfmt }],
transformations: [{ type: SupportedTransformationType.Logfmt }],
},
};