Skip to content

Commit

Permalink
fix(slo): Handle partial indicator url state (elastic#167247)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kdelemme
kdelemme authored Sep 29, 2023
1 parent 8c17d8a commit 9d3213e
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 43 deletions.
20 changes: 14 additions & 6 deletions ...ublic/pages/slo_edit/components/apm_availability/apm_availability_indicator_type_form.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@

import { EuiFlexGroup, EuiFlexItem, EuiIconTip } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import React, { useEffect } from 'react';
import { useFormContext } from 'react-hook-form';
import { useFetchApmIndex } from '../../../../hooks/slo/use_fetch_apm_indices';
import { useFetchIndexPatternFields } from '../../../../hooks/slo/use_fetch_index_pattern_fields';
import { CreateSLOForm } from '../../types';
import { FieldSelector } from '../apm_common/field_selector';
Expand All @@ -17,10 +18,17 @@ import { IndexFieldSelector } from '../common/index_field_selector';
import { QueryBuilder } from '../common/query_builder';

export function ApmAvailabilityIndicatorTypeForm() {
const { watch } = useFormContext<CreateSLOForm>();
const index = watch('indicator.params.index');
const { watch, setValue } = useFormContext<CreateSLOForm>();
const { data: apmIndex } = useFetchApmIndex();

useEffect(() => {
if (apmIndex !== '') {
setValue('indicator.params.index', apmIndex);
}
}, [setValue, apmIndex]);

const { isLoading: isIndexFieldsLoading, data: indexFields = [] } =
useFetchIndexPatternFields(index);
useFetchIndexPatternFields(apmIndex);
const partitionByFields = indexFields.filter((field) => field.aggregatable);

return (
Expand Down Expand Up @@ -144,8 +152,8 @@ export function ApmAvailabilityIndicatorTypeForm() {
placeholder={i18n.translate('xpack.observability.slo.sloEdit.groupBy.placeholder', {
defaultMessage: 'Select an optional field to partition by',
})}
isLoading={!!index && isIndexFieldsLoading}
isDisabled={!index}
isLoading={!!apmIndex && isIndexFieldsLoading}
isDisabled={!apmIndex}
/>

<DataPreviewChart />
Expand Down
20 changes: 14 additions & 6 deletions ...vability/public/pages/slo_edit/components/apm_latency/apm_latency_indicator_type_form.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@

import { EuiFieldNumber, EuiFlexGroup, EuiFlexItem, EuiFormRow, EuiIconTip } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import React, { useEffect } from 'react';
import { Controller, useFormContext } from 'react-hook-form';
import { useFetchApmIndex } from '../../../../hooks/slo/use_fetch_apm_indices';
import { useFetchIndexPatternFields } from '../../../../hooks/slo/use_fetch_index_pattern_fields';
import { CreateSLOForm } from '../../types';
import { FieldSelector } from '../apm_common/field_selector';
Expand All @@ -17,10 +18,17 @@ import { IndexFieldSelector } from '../common/index_field_selector';
import { QueryBuilder } from '../common/query_builder';

export function ApmLatencyIndicatorTypeForm() {
const { control, watch, getFieldState } = useFormContext<CreateSLOForm>();
const index = watch('indicator.params.index');
const { control, watch, getFieldState, setValue } = useFormContext<CreateSLOForm>();
const { data: apmIndex } = useFetchApmIndex();

useEffect(() => {
if (apmIndex !== '') {
setValue('indicator.params.index', apmIndex);
}
}, [setValue, apmIndex]);

const { isLoading: isIndexFieldsLoading, data: indexFields = [] } =
useFetchIndexPatternFields(index);
useFetchIndexPatternFields(apmIndex);
const partitionByFields = indexFields.filter((field) => field.aggregatable);

return (
Expand Down Expand Up @@ -187,8 +195,8 @@ export function ApmLatencyIndicatorTypeForm() {
placeholder={i18n.translate('xpack.observability.slo.sloEdit.groupBy.placeholder', {
defaultMessage: 'Select an optional field to partition by',
})}
isLoading={!!index && isIndexFieldsLoading}
isDisabled={!index}
isLoading={!!apmIndex && isIndexFieldsLoading}
isDisabled={!apmIndex}
/>

<DataPreviewChart />
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function SloEditForm({ slo }: Props) {
sloIds: slo?.id ? [slo.id] : undefined,
});

const sloFormValuesUrlState = useParseUrlState();
const sloFormValuesFromUrlState = useParseUrlState();
const isAddRuleFlyoutOpen = useAddRuleFlyoutState(isEditMode);
const [isCreateRuleCheckboxChecked, setIsCreateRuleCheckboxChecked] = useState(true);

Expand All @@ -73,7 +73,7 @@ export function SloEditForm({ slo }: Props) {
}, [isEditMode, rules, slo]);

const methods = useForm<CreateSLOForm>({
defaultValues: Object.assign({}, SLO_EDIT_FORM_DEFAULT_VALUES, sloFormValuesUrlState),
defaultValues: Object.assign({}, SLO_EDIT_FORM_DEFAULT_VALUES, sloFormValuesFromUrlState),
values: transformSloResponseToCreateSloForm(slo),
mode: 'all',
});
Expand Down
152 changes: 152 additions & 0 deletions x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.test.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { transformPartialUrlStateToFormState as transform } from './process_slo_form_values';

describe('Transform Partial URL State into partial State Form', () => {
describe('indicators', () => {
it("returns an empty '{}' when no indicator type is specified", () => {
expect(transform({ indicator: { params: { index: 'my-index' } } })).toEqual({});
});

it('handles partial APM Availability state', () => {
expect(
transform({
indicator: {
type: 'sli.apm.transactionErrorRate',
params: {
service: 'override-service',
},
},
})
).toEqual({
indicator: {
type: 'sli.apm.transactionErrorRate',
params: {
service: 'override-service',
environment: '',
filter: '',
index: '',
transactionName: '',
transactionType: '',
},
},
});
});

it('handles partial APM Latency state', () => {
expect(
transform({
indicator: {
type: 'sli.apm.transactionDuration',
params: {
service: 'override-service',
},
},
})
).toEqual({
indicator: {
type: 'sli.apm.transactionDuration',
params: {
service: 'override-service',
environment: '',
filter: '',
index: '',
transactionName: '',
transactionType: '',
threshold: 250,
},
},
});
});

it('handles partial Custom KQL state', () => {
expect(
transform({
indicator: {
type: 'sli.kql.custom',
params: {
good: "some.override.filter:'foo'",
index: 'override-index',
},
},
})
).toEqual({
indicator: {
type: 'sli.kql.custom',
params: {
index: 'override-index',
timestampField: '',
filter: '',
good: "some.override.filter:'foo'",
total: '',
},
},
});
});

it('handles partial Custom Metric state', () => {
expect(
transform({
indicator: {
type: 'sli.metric.custom',
params: {
index: 'override-index',
},
},
})
).toEqual({
indicator: {
type: 'sli.metric.custom',
params: {
index: 'override-index',
filter: '',
timestampField: '',
good: {
equation: 'A',
metrics: [{ aggregation: 'sum', field: '', name: 'A' }],
},
total: {
equation: 'A',
metrics: [{ aggregation: 'sum', field: '', name: 'A' }],
},
},
},
});
});

it('handles partial Custom Histogram state', () => {
expect(
transform({
indicator: {
type: 'sli.histogram.custom',
params: {
index: 'override-index',
},
},
})
).toEqual({
indicator: {
type: 'sli.histogram.custom',
params: {
index: 'override-index',
filter: '',
timestampField: '',
good: {
aggregation: 'value_count',
field: '',
},
total: {
aggregation: 'value_count',
field: '',
},
},
},
});
});
});
});
74 changes: 56 additions & 18 deletions x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,17 @@
* 2.0.
*/

import { CreateSLOInput, SLOWithSummaryResponse, UpdateSLOInput } from '@kbn/slo-schema';
import { CreateSLOInput, Indicator, SLOWithSummaryResponse, UpdateSLOInput } from '@kbn/slo-schema';
import { assertNever } from '@kbn/std';
import { RecursivePartial } from '@kbn/utility-types';
import { toDuration } from '../../../utils/slo/duration';
import {
APM_AVAILABILITY_DEFAULT_VALUES,
APM_LATENCY_DEFAULT_VALUES,
CUSTOM_KQL_DEFAULT_VALUES,
CUSTOM_METRIC_DEFAULT_VALUES,
HISTOGRAM_DEFAULT_VALUES,
} from '../constants';
import { CreateSLOForm } from '../types';

export function transformSloResponseToCreateSloForm(
Expand Down Expand Up @@ -91,21 +100,50 @@ export function transformValuesToUpdateSLOInput(values: CreateSLOForm): UpdateSL
};
}

export function transformPartialCreateSLOInputToPartialCreateSLOForm(
values: Partial<CreateSLOInput>
): Partial<CreateSLOForm> {
return {
...values,
...(values.objective && {
objective: {
target: values.objective.target * 100,
...(values.objective.timesliceTarget && {
timesliceTarget: values.objective.timesliceTarget * 100,
}),
...(values.objective.timesliceWindow && {
timesliceWindow: String(toDuration(values.objective.timesliceWindow).value),
}),
},
}),
};
function transformPartialIndicatorState(
indicator?: RecursivePartial<Indicator>
): Indicator | undefined {
if (indicator === undefined || indicator.type === undefined) return undefined;

const indicatorType = indicator.type;
switch (indicatorType) {
case 'sli.apm.transactionDuration':
return {
type: 'sli.apm.transactionDuration' as const,
params: Object.assign({}, APM_LATENCY_DEFAULT_VALUES.params, indicator.params ?? {}),
};
case 'sli.apm.transactionErrorRate':
return {
type: 'sli.apm.transactionErrorRate' as const,
params: Object.assign({}, APM_AVAILABILITY_DEFAULT_VALUES.params, indicator.params ?? {}),
};
case 'sli.histogram.custom':
return {
type: 'sli.histogram.custom' as const,
params: Object.assign({}, HISTOGRAM_DEFAULT_VALUES.params, indicator.params ?? {}),
};
case 'sli.kql.custom':
return {
type: 'sli.kql.custom' as const,
params: Object.assign({}, CUSTOM_KQL_DEFAULT_VALUES.params, indicator.params ?? {}),
};
case 'sli.metric.custom':
return {
type: 'sli.metric.custom' as const,
params: Object.assign({}, CUSTOM_METRIC_DEFAULT_VALUES.params, indicator.params ?? {}),
};
default:
assertNever(indicatorType);
}
}

export function transformPartialUrlStateToFormState(
values: RecursivePartial<Pick<CreateSLOInput, 'indicator'>>
): Partial<CreateSLOForm> | {} {
const state: Partial<CreateSLOForm> = {};

const parsedIndicator = transformPartialIndicatorState(values.indicator);
if (parsedIndicator !== undefined) state.indicator = parsedIndicator;

return state;
}
7 changes: 4 additions & 3 deletions x-pack/plugins/observability/public/pages/slo_edit/hooks/use_parse_url_state.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@

import { createKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public';
import { CreateSLOInput } from '@kbn/slo-schema';
import { RecursivePartial } from '@kbn/utility-types';
import { useHistory } from 'react-router-dom';
import { transformPartialCreateSLOInputToPartialCreateSLOForm } from '../helpers/process_slo_form_values';
import { transformPartialUrlStateToFormState } from '../helpers/process_slo_form_values';
import { CreateSLOForm } from '../types';

export function useParseUrlState(): Partial<CreateSLOForm> | null {
Expand All @@ -19,7 +20,7 @@ export function useParseUrlState(): Partial<CreateSLOForm> | null {
useHashQuery: false,
});

const urlParams = urlStateStorage.get<Partial<CreateSLOInput>>('_a');
const urlParams = urlStateStorage.get<RecursivePartial<CreateSLOInput>>('_a');

return !!urlParams ? transformPartialCreateSLOInputToPartialCreateSLOForm(urlParams) : null;
return !!urlParams ? transformPartialUrlStateToFormState(urlParams) : null;
}
2 changes: 1 addition & 1 deletion x-pack/plugins/observability/public/pages/slo_edit/hooks/use_unregister_fields.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ export function useUnregisterFields({ isEditMode }: { isEditMode: boolean }) {
const [indicatorTypeState, setIndicatorTypeState] = useState<IndicatorType>(
watch('indicator.type')
);
const indicatorType = watch('indicator.type');

const indicatorType = watch('indicator.type');
useEffect(() => {
if (indicatorType !== indicatorTypeState && !isEditMode) {
setIndicatorTypeState(indicatorType);
Expand Down
Loading

0 comments on commit 9d3213e

Please sign in to comment.