Skip to content

Commit

Permalink
Fix changes to correlations
Browse files Browse the repository at this point in the history
  • Loading branch information
gelicia committed Dec 20, 2023
1 parent f24bf2f commit 1f5adbe
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 39 deletions.
21 changes: 11 additions & 10 deletions public/app/features/correlations/CorrelationsPage.test.tsx
Expand Up @@ -287,7 +287,7 @@ describe('CorrelationsPage', () => {
await userEvent.click(screen.getByRole('button', { name: /add transformation/i }));
const typeFilterSelect = screen.getAllByLabelText('Type');
openMenu(typeFilterSelect[0]);
await userEvent.click(screen.getAllByText('Regular expression')[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 }));
Expand Down Expand Up @@ -363,7 +363,7 @@ describe('CorrelationsPage', () => {
provisioned: false,
config: {
field: 'line',
target: { title: '', url: '' },
target: {},
type: 'query',
transformations: [
{ type: SupportedTransformationType.Regex, expression: 'url=http[s]?://(S*)', mapValue: 'path' },
Expand All @@ -375,7 +375,7 @@ describe('CorrelationsPage', () => {
targetUID: 'loki',
uid: '2',
label: 'Prometheus to Loki',
config: { field: 'label', target: { title: '', url: '' }, type: 'query' },
config: { field: 'label', target: {}, type: 'query' },
provisioned: false,
},
]
Expand Down Expand Up @@ -538,7 +538,7 @@ describe('CorrelationsPage', () => {

// 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' }));
await userEvent.click(screen.getByText('Regular expression'));
expressionInput = screen.queryByLabelText(/expression/i);
expect(expressionInput).toBeInTheDocument();
expect(expressionInput).toBeEnabled();
Expand All @@ -554,7 +554,8 @@ describe('CorrelationsPage', () => {
await userEvent.click(screen.getByRole('button', { name: /add transformation/i }));
typeFilterSelect = screen.getAllByLabelText('Type');
openMenu(typeFilterSelect[0]);
await userEvent.click(screen.getAllByText('Regular expression')[1]);
const menu = await screen.findByLabelText('Select options menu');
await userEvent.click(within(menu).getByText('Regular expression'));
expressionInput = screen.queryByLabelText(/expression/i);
expect(expressionInput).toBeInTheDocument();
expect(expressionInput).toBeEnabled();
Expand Down Expand Up @@ -597,7 +598,7 @@ describe('CorrelationsPage', () => {
provisioned: false,
config: {
field: 'line',
target: { title: '', url: '' },
target: {},
type: 'query',
transformations: [
{ type: SupportedTransformationType.Regex, expression: 'url=http[s]?://(S*)', mapValue: 'path' },
Expand All @@ -612,7 +613,7 @@ describe('CorrelationsPage', () => {
provisioned: false,
config: {
field: 'line',
target: { title: '', url: '' },
target: {},
type: 'query',
transformations: [
{ type: SupportedTransformationType.Regex, expression: 'url=http[s]?://(S*)', mapValue: 'path' },
Expand All @@ -624,15 +625,15 @@ describe('CorrelationsPage', () => {
targetUID: 'loki',
uid: '3',
label: 'Prometheus to Loki',
config: { field: 'label', target: { title: '', url: '' }, type: 'query' },
config: { field: 'label', target: {}, type: 'query' },
provisioned: false,
},
{
sourceUID: 'prometheus',
targetUID: 'prometheus',
uid: '4',
label: 'Prometheus to Prometheus',
config: { field: 'label', target: { title: '', url: '' }, type: 'query' },
config: { field: 'label', target: {}, type: 'query' },
provisioned: false,
},
]
Expand All @@ -659,7 +660,7 @@ describe('CorrelationsPage', () => {
provisioned: true,
config: {
field: 'line',
target: { title: '', url: '' },
target: {},
type: 'query',
transformations: [{ type: SupportedTransformationType.Regex, expression: '(?:msg)=' }],
},
Expand Down
Expand Up @@ -84,7 +84,7 @@ export const ConfigureCorrelationSourceForm = () => {
)}
htmlFor="source"
invalid={!!formState.errors.sourceUID}
error={String(formState.errors.sourceUID?.message || '')}
error={formState.errors.sourceUID?.message}
>
<DataSourcePicker
onChange={withDsUID(onChange)}
Expand Down
Expand Up @@ -63,8 +63,7 @@ const TransformationEditorRow = (props: Props) => {
</Stack>
}
invalid={!!formState.errors?.config?.transformations?.[index]?.type}
// @ts-expect-error TODO Figure out how to get proper types here
error={formState.errors?.config?.transformations?.[index]?.type?.message}
error={formState.errors?.config?.transformations?.[index]?.message}
validationMessageHorizontalOverflow={true}
>
<Select
Expand Down
4 changes: 2 additions & 2 deletions public/app/features/correlations/types.ts
@@ -1,4 +1,4 @@
import { DataLink, DataLinkTransformationConfig } from '@grafana/data';
import { DataLinkTransformationConfig } from '@grafana/data';

export interface AddCorrelationResponse {
correlation: Correlation;
Expand Down Expand Up @@ -30,7 +30,7 @@ type CorrelationConfigType = 'query';

export interface CorrelationConfig {
field: string;
target: DataLink;
target: object; // this contains anything that would go in the query editor, so any extension off DataQuery a datasource would have, and needs to be generic
type: CorrelationConfigType;
transformations?: DataLinkTransformationConfig[];
}
Expand Down
12 changes: 6 additions & 6 deletions public/app/features/correlations/utils.test.ts
Expand Up @@ -20,7 +20,7 @@ describe('correlations utils', () => {
datasourceName: prometheus.name,
query: {
datasource: { uid: prometheus.uid },
title: 'target Prometheus query',
expr: 'target Prometheus query',
},
},
},
Expand All @@ -31,7 +31,7 @@ describe('correlations utils', () => {
datasourceName: elastic.name,
query: {
datasource: { uid: elastic.uid },
title: 'target Elastic query',
expr: 'target Elastic query',
},
},
},
Expand All @@ -50,7 +50,7 @@ describe('correlations utils', () => {
datasourceUid: elastic.uid,
datasourceName: elastic.name,
query: {
title: 'target Elastic query',
expr: 'target Elastic query',
},
},
});
Expand Down Expand Up @@ -111,7 +111,7 @@ function setup() {
label: 'logs to metrics',
source: loki,
target: prometheus,
config: { type: 'query', field: 'traceId', target: { title: 'target Prometheus query', url: '' } },
config: { type: 'query', field: 'traceId', target: { expr: 'target Prometheus query' } },
provisioned: false,
},
// Test multiple correlations attached to the same field
Expand All @@ -120,15 +120,15 @@ function setup() {
label: 'logs to logs',
source: loki,
target: elastic,
config: { type: 'query', field: 'traceId', target: { title: 'target Elastic query', url: '' } },
config: { type: 'query', field: 'traceId', target: { expr: 'target Elastic query' } },
provisioned: false,
},
{
uid: 'prometheus-to-elastic',
label: 'metrics to logs',
source: prometheus,
target: elastic,
config: { type: 'query', field: 'value', target: { title: 'target Elastic query', url: '' } },
config: { type: 'query', field: 'value', target: { expr: 'target Elastic query' } },
provisioned: false,
},
];
Expand Down
6 changes: 3 additions & 3 deletions public/app/features/explore/CorrelationHelper.tsx
Expand Up @@ -84,10 +84,10 @@ export const CorrelationHelper = ({ exploreId, correlations }: Props) => {
useEffect(() => {
const subscription = watch((value) => {
let dirty = correlationDetails?.correlationDirty || false;

if (!dirty && (value.label !== defaultLabel || value.description !== '')) {
let description = value.description || '';
if (!dirty && (value.label !== defaultLabel || description !== '')) {
dirty = true;
} else if (dirty && value.label === defaultLabel && (value.description || '').trim() === '') {
} else if (dirty && value.label === defaultLabel && description.trim() === '') {
dirty = false;
}
dispatch(
Expand Down
27 changes: 15 additions & 12 deletions public/app/features/explore/CorrelationTransformationAddModal.tsx
Expand Up @@ -3,7 +3,7 @@ import React, { useId, useState, useMemo, useEffect } from 'react';
import Highlighter from 'react-highlight-words';
import { useForm } from 'react-hook-form';

import { DataLinkTransformationConfig, ScopedVars, SupportedTransformationType } from '@grafana/data';
import { DataLinkTransformationConfig, ScopedVars } from '@grafana/data';
import { Button, Field, Icon, Input, InputControl, Label, Modal, Select, Tooltip, Stack } from '@grafana/ui';

import {
Expand Down Expand Up @@ -101,18 +101,21 @@ export const CorrelationTransformationAddModal = ({
isExpressionValid = !formFieldsVis.expressionDetails.show;
}
setIsExpValid(isExpressionValid);
const transformationVars = getTransformationVars(
{
type: formValues.type || SupportedTransformationType.Regex, // TODO does this default value make sense?
expression: isExpressionValid ? expression : '',
mapValue: formValues.mapValue,
},
fieldList[formValues.field!] || '',
formValues.field!
);
let transKeys = [];
if (formValues.type) {
const transformationVars = getTransformationVars(
{
type: formValues.type,
expression: isExpressionValid ? expression : '',
mapValue: formValues.mapValue,
},
fieldList[formValues.field!] || '',
formValues.field!
);

const transKeys = Object.keys(transformationVars);
setTransformationVars(transKeys.length > 0 ? { ...transformationVars } : {});
transKeys = Object.keys(transformationVars);
setTransformationVars(transKeys.length > 0 ? { ...transformationVars } : {});
}

if (transKeys.length === 0 || !isExpressionValid) {
setValidToSave(false);
Expand Down
1 change: 0 additions & 1 deletion public/app/features/explore/state/correlations.ts
Expand Up @@ -83,7 +83,6 @@ export function saveCurrentCorrelation(
description,
config: {
field: targetPane.correlationEditorHelperData.resultField,
// @ts-expect-error TODO check the type
target: targetPane.queries[0],
type: 'query',
transformations: transformations,
Expand Down
4 changes: 2 additions & 2 deletions public/app/features/explore/utils/links.ts
Expand Up @@ -299,8 +299,8 @@ const builtInVariables = [
* @param query
* @param scopedVars
*/
export function getVariableUsageInfo<T extends DataLink>(
query: T,
export function getVariableUsageInfo(
query: object,
scopedVars: ScopedVars
): { variables: VariableInterpolation[]; allVariablesDefined: boolean } {
let variables: VariableInterpolation[] = [];
Expand Down

0 comments on commit 1f5adbe

Please sign in to comment.