Skip to content

Commit

Permalink
[SLO]: require instance id in slo details schema (elastic#209020)
Browse files Browse the repository at this point in the history
## Summary

Resolves elastic#180590

Since SLO instanceId is provided in all APIs, it will be made required
for sloWithDataResponseSchema and the SLOWithSummaryResponse type.
Checks for the existence of instanceId have either been removed or
changed to check for ALL_VALUE (`*`)

---------

Co-authored-by: Kevin Delemme <kevin.delemme@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
4 people authored Feb 4, 2025
1 parent f332644 commit df573d7
Show file tree
Hide file tree
Showing 20 changed files with 59 additions and 87 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ pageLoadAssetSize:
serverlessSearch: 72995
sessionView: 77750
share: 88160
slo: 37039
slo: 45000
snapshotRestore: 79032
spaces: 57868
stackAlerts: 58316
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@ import {

const sloWithDataResponseSchema = t.intersection([
sloDefinitionSchema,
t.type({ summary: summarySchema, groupings: groupingsSchema }),
t.type({ summary: summarySchema, groupings: groupingsSchema, instanceId: allOrAnyString }),
t.partial({
instanceId: allOrAnyString,
meta: metaSchema,
remote: remoteSchema,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* 2.0.
*/

import { ALL_VALUE } from '@kbn/slo-schema/src/schema/common';

export const SLOS_BASE_PATH = '/app/slos';
export const SLOS_PATH = '/' as const;
export const SLOS_WELCOME_PATH = '/welcome' as const;
Expand All @@ -25,9 +27,9 @@ export const paths = {
sloEdit: (sloId: string) => `${SLOS_BASE_PATH}/edit/${encodeURIComponent(sloId)}`,
sloEditWithEncodedForm: (sloId: string, encodedParams: string) =>
`${SLOS_BASE_PATH}/edit/${encodeURIComponent(sloId)}?_a=${encodedParams}`,
sloDetails: (sloId: string, instanceId?: string, remoteName?: string, tabId?: string) => {
sloDetails: (sloId: string, instanceId: string, remoteName?: string, tabId?: string) => {
const qs = new URLSearchParams();
if (!!instanceId && instanceId !== '*') qs.append('instanceId', instanceId);
if (instanceId !== ALL_VALUE) qs.append('instanceId', instanceId);
if (!!remoteName) qs.append('remoteName', remoteName);
if (tabId) {
return `${SLOS_BASE_PATH}/${encodeURIComponent(sloId)}/${tabId}?${qs.toString()}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { ALL_VALUE } from '@kbn/slo-schema';
import React, { useState } from 'react';
import { SloSelector } from '../alerts/slo_selector';
import type { EmbeddableProps } from './types';
Expand Down Expand Up @@ -76,7 +75,7 @@ export function Configuration({ onCreate, onCancel }: Props) {
onSelected={(slo) => {
setHasError(slo === undefined);
if (slo && 'id' in slo) {
setSelectedSlo({ sloId: slo.id, sloInstanceId: slo.instanceId ?? ALL_VALUE });
setSelectedSlo({ sloId: slo.id, sloInstanceId: slo.instanceId });
}
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@
* 2.0.
*/

import React, { useEffect, useState } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiLoadingChart } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { euiStyled } from '@kbn/kibana-react-plugin/common';
import { ALL_VALUE, SLOWithSummaryResponse } from '@kbn/slo-schema';
import { SLOWithSummaryResponse } from '@kbn/slo-schema';
import React, { useEffect, useState } from 'react';
import { Subject } from 'rxjs';
import { SloOverviewDetails } from '../common/slo_overview_details';
import { formatHistoricalData } from '../../../utils/slo/chart_data_formatter';
import { useFetchHistoricalSummary } from '../../../hooks/use_fetch_historical_summary';
import { useFetchActiveAlerts } from '../../../hooks/use_fetch_active_alerts';
import { useFetchHistoricalSummary } from '../../../hooks/use_fetch_historical_summary';
import { useFetchRulesForSlo } from '../../../hooks/use_fetch_rules_for_slo';
import { SloCardItemBadges } from '../../../pages/slos/components/card_view/slo_card_item_badges';
import { SloCardChart } from '../../../pages/slos/components/card_view/slo_card_item';
import { useFetchSloDetails } from '../../../hooks/use_fetch_slo_details';
import { SloCardChart } from '../../../pages/slos/components/card_view/slo_card_item';
import { SloCardItemBadges } from '../../../pages/slos/components/card_view/slo_card_item_badges';
import { formatHistoricalData } from '../../../utils/slo/chart_data_formatter';
import { SloOverviewDetails } from '../common/slo_overview_details';

import { SingleSloCustomInput } from './types';

Expand Down Expand Up @@ -54,7 +54,7 @@ export function SloOverview({ sloId, sloInstanceId, remoteName, reloadSubject }:
});

const { data: activeAlertsBySlo } = useFetchActiveAlerts({
sloIdsAndInstanceIds: slo ? [[slo.id, slo.instanceId ?? ALL_VALUE] as [string, string]] : [],
sloIdsAndInstanceIds: slo ? [[slo.id, slo.instanceId] as [string, string]] : [],
});

const { data: historicalSummaries = [] } = useFetchHistoricalSummary({
Expand Down Expand Up @@ -96,8 +96,7 @@ export function SloOverview({ sloId, sloInstanceId, remoteName, reloadSubject }:
const activeAlerts = activeAlertsBySlo.get(slo);

const historicalSummary = historicalSummaries.find(
(histSummary) =>
histSummary.sloId === slo.id && histSummary.instanceId === (slo.instanceId ?? ALL_VALUE)
(histSummary) => histSummary.sloId === slo.id && histSummary.instanceId === slo.instanceId
)?.data;

const historicalSliData = formatHistoricalData(historicalSummary, 'sli_value');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export function SloCardChartList({ sloId }: { sloId: string }) {

const historicalSummary =
historicalSummaries.find(
(hist) => hist.sloId === slo.id && hist.instanceId === (slo.instanceId ?? ALL_VALUE)
(hist) => hist.sloId === slo.id && hist.instanceId === slo.instanceId
)?.data ?? [];

const lastArray = chartsData[chartsData.length - 1];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import {
AlertConsumers,
SLO_RULE_TYPE_IDS,
} from '@kbn/rule-registry-plugin/common/technical_rule_data_field_names';
import { ALL_VALUE } from '@kbn/slo-schema/src/schema/common';
import { useKibana } from './use_kibana';
import { sloKeys } from './query_key_factory';
import { ActiveAlerts } from './active_alerts';

import { SLO_LONG_REFETCH_INTERVAL } from '../constants';

type SloIdAndInstanceId = [string, string];
Expand Down Expand Up @@ -83,7 +83,9 @@ export function useFetchActiveAlerts({
bool: {
filter: [
{ term: { 'slo.id': sloId } },
...(instanceId === '*' ? [] : [{ term: { 'slo.instanceId': instanceId } }]),
...(instanceId === ALL_VALUE
? []
: [{ term: { 'slo.instanceId': instanceId } }]),
],
},
})),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function useFetchSloDetails({
params: {
path: { id: sloId! },
query: {
...(!!instanceId && instanceId !== ALL_VALUE && { instanceId }),
...(instanceId !== ALL_VALUE && { instanceId }),
...(remoteName && { remoteName }),
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import type { SerializableRecord } from '@kbn/utility-types';
import type { LocatorDefinition } from '@kbn/share-plugin/public';
import { sloDetailsLocatorID } from '@kbn/observability-plugin/common';
import { ALL_VALUE } from '@kbn/slo-schema/src/schema/common';

export interface SloDetailsLocatorParams extends SerializableRecord {
sloId?: string;
Expand All @@ -19,7 +20,9 @@ export class SloDetailsLocatorDefinition implements LocatorDefinition<SloDetails

public readonly getLocation = async ({ sloId, instanceId }: SloDetailsLocatorParams) => {
const queryParams =
!!instanceId && instanceId !== '*' ? `?instanceId=${encodeURIComponent(instanceId)}` : '';
!!instanceId && instanceId !== ALL_VALUE
? `?instanceId=${encodeURIComponent(instanceId)}`
: '';
const path = !!sloId ? `/${encodeURIComponent(sloId)}${queryParams}` : '/';

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
} from '@elastic/eui';
import { css } from '@emotion/react';
import { i18n } from '@kbn/i18n';
import { ALL_VALUE, SLOWithSummaryResponse } from '@kbn/slo-schema';
import { SLOWithSummaryResponse } from '@kbn/slo-schema';
import React, { useEffect, useState } from 'react';
import { useHistory, useLocation } from 'react-router-dom';
import useDebounce from 'react-use/lib/useDebounce';
Expand Down Expand Up @@ -48,7 +48,7 @@ export function SLOGroupingValueSelector({ slo, groupingKey, value }: Props) {
const { isLoading, isError, data } = useFetchSloGroupings({
sloId: slo.id,
groupingKey,
instanceId: slo.instanceId ?? ALL_VALUE,
instanceId: slo.instanceId,
search: debouncedSearch,
remoteName,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { ALL_VALUE, SLOWithSummaryResponse } from '@kbn/slo-schema';
import { EuiFlexItem } from '@elastic/eui';
import { SLOWithSummaryResponse } from '@kbn/slo-schema';
import React from 'react';
import { TimeBounds } from '../types';
import { useFetchHistoricalSummary } from '../../../hooks/use_fetch_historical_summary';
import { formatHistoricalData } from '../../../utils/slo/chart_data_formatter';
import { SloTabId } from './slo_details';
import { SliChartPanel } from './sli_chart_panel';
import { TimeBounds } from '../types';
import { ErrorBudgetChartPanel } from './error_budget_chart_panel';
import { SliChartPanel } from './sli_chart_panel';
import { SloTabId } from './slo_details';

export interface Props {
slo: SLOWithSummaryResponse;
Expand All @@ -38,8 +38,7 @@ export function HistoricalDataCharts({

const sloHistoricalSummary = historicalSummaries.find(
(historicalSummary) =>
historicalSummary.sloId === slo.id &&
historicalSummary.instanceId === (slo.instanceId ?? ALL_VALUE)
historicalSummary.sloId === slo.id && historicalSummary.instanceId === slo.instanceId
);

const errorBudgetBurnDownData = formatHistoricalData(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
* 2.0.
*/
import { EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';
import React, { Fragment } from 'react';
import { AlertConsumers, SLO_RULE_TYPE_IDS } from '@kbn/rule-data-utils';
import React, { Fragment } from 'react';

import { ALL_VALUE, SLOWithSummaryResponse } from '@kbn/slo-schema';
import { SLO_ALERTS_TABLE_ID } from '@kbn/observability-shared-plugin/common';
import { SLOWithSummaryResponse } from '@kbn/slo-schema';
import { useKibana } from '../../../hooks/use_kibana';

export interface Props {
Expand Down Expand Up @@ -38,7 +38,7 @@ export function SloDetailsAlerts({ slo }: Props) {
bool: {
filter: [
{ term: { 'slo.id': slo.id } },
{ term: { 'slo.instanceId': slo.instanceId ?? ALL_VALUE } },
{ term: { 'slo.instanceId': slo.instanceId } },
],
},
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { rulesLocatorID, RulesParams } from '@kbn/observability-plugin/public';
import { ALL_VALUE, SLOWithSummaryResponse } from '@kbn/slo-schema';
import { SLOWithSummaryResponse } from '@kbn/slo-schema';
import { Rule } from '@kbn/triggers-actions-ui-plugin/public';
import path from 'path';
import { paths } from '../../../../common/locators/paths';
Expand Down Expand Up @@ -77,11 +77,7 @@ export const useSloActions = ({
}
};

const detailsUrl = paths.sloDetails(
slo.id,
![slo.groupBy].flat().includes(ALL_VALUE) && slo.instanceId ? slo.instanceId : undefined,
slo.remote?.remoteName
);
const detailsUrl = paths.sloDetails(slo.id, slo.instanceId, slo.remote?.remoteName);

const remoteDeleteUrl = createRemoteSloDeleteUrl(slo, spaceId);
const remoteResetUrl = createRemoteSloResetUrl(slo, spaceId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { EuiNotificationBadge, EuiToolTip } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { ALL_VALUE, SLOWithSummaryResponse } from '@kbn/slo-schema';
import { SLOWithSummaryResponse } from '@kbn/slo-schema';
import React from 'react';
import { paths } from '../../../../common/locators/paths';
import { useFetchActiveAlerts } from '../../../hooks/use_fetch_active_alerts';
Expand All @@ -31,7 +31,7 @@ export const useSloDetailsTabs = ({
setSelectedTabId?: (val: SloTabId) => void;
}) => {
const { data: activeAlerts } = useFetchActiveAlerts({
sloIdsAndInstanceIds: slo ? [[slo.id, slo.instanceId ?? ALL_VALUE]] : [],
sloIdsAndInstanceIds: slo ? [[slo.id, slo.instanceId]] : [],
shouldRefetch: isAutoRefreshing,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function SloEditPage() {
...(!!slo
? [
{
href: basePath.prepend(paths.sloDetails(slo!.id)),
href: basePath.prepend(paths.sloDetails(slo.id, slo.instanceId)),
text: slo!.name,
},
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
useIsWithinBreakpoints,
} from '@elastic/eui';
import { EuiFlexGridProps } from '@elastic/eui/src/components/flex/flex_grid';
import { ALL_VALUE, SLOWithSummaryResponse } from '@kbn/slo-schema';
import { SLOWithSummaryResponse } from '@kbn/slo-schema';
import React from 'react';
import { useFetchActiveAlerts } from '../../../../hooks/use_fetch_active_alerts';
import { useFetchHistoricalSummary } from '../../../../hooks/use_fetch_historical_summary';
Expand Down Expand Up @@ -44,9 +44,7 @@ const useColumns = () => {
};

export function SloListCardView({ sloList, loading, error }: Props) {
const sloIdsAndInstanceIds = sloList.map(
(slo) => [slo.id, slo.instanceId ?? ALL_VALUE] as [string, string]
);
const sloIdsAndInstanceIds = sloList.map((slo) => [slo.id, slo.instanceId] as [string, string]);
const { data: activeAlertsBySlo } = useFetchActiveAlerts({ sloIdsAndInstanceIds });
const { data: rulesBySlo, refetchRules } = useFetchRulesForSlo({
sloIds: sloIdsAndInstanceIds.map((item) => item[0]),
Expand All @@ -67,7 +65,7 @@ export function SloListCardView({ sloList, loading, error }: Props) {
{sloList
.filter((slo) => slo.summary)
.map((slo) => (
<EuiFlexItem key={`${slo.id}-${slo.instanceId ?? 'ALL_VALUE'}`}>
<EuiFlexItem key={`${slo.id}-${slo.instanceId}`}>
<SloCardItem
slo={slo}
loading={loading}
Expand All @@ -78,7 +76,7 @@ export function SloListCardView({ sloList, loading, error }: Props) {
historicalSummaries.find(
(historicalSummary) =>
historicalSummary.sloId === slo.id &&
historicalSummary.instanceId === (slo.instanceId ?? ALL_VALUE)
historicalSummary.instanceId === slo.instanceId
)?.data
}
historicalSummaryLoading={historicalSummaryLoading}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ export function SloListCompactView({ sloList, loading, error }: Props) {
const spaceId = useSpace();

const percentFormat = uiSettings.get('format:percent:defaultPattern');
const sloIdsAndInstanceIds = sloList.map(
(slo) => [slo.id, slo.instanceId ?? ALL_VALUE] as [string, string]
);
const sloIdsAndInstanceIds = sloList.map((slo) => [slo.id, slo.instanceId] as [string, string]);

const { data: permissions } = usePermissions();
const filteredRuleTypes = useGetFilteredRuleTypes();
Expand Down Expand Up @@ -182,13 +180,7 @@ export function SloListCompactView({ sloList, loading, error }: Props) {
}),
onClick: (slo: SLOWithSummaryResponse) => {
const sloDetailsUrl = basePath.prepend(
paths.sloDetails(
slo.id,
![slo.groupBy].flat().includes(ALL_VALUE) && slo.instanceId
? slo.instanceId
: undefined,
slo.remote?.remoteName
)
paths.sloDetails(slo.id, slo.instanceId, slo.remote?.remoteName)
);
navigateToUrl(sloDetailsUrl);
},
Expand Down Expand Up @@ -395,13 +387,7 @@ export function SloListCompactView({ sloList, loading, error }: Props) {
'data-test-subj': 'sloItem',
render: (_, slo: SLOWithSummaryResponse) => {
const sloDetailsUrl = basePath.prepend(
paths.sloDetails(
slo.id,
![slo.groupBy].flat().includes(ALL_VALUE) && slo.instanceId
? slo.instanceId
: undefined,
slo.remote?.remoteName
)
paths.sloDetails(slo.id, slo.instanceId, slo.remote?.remoteName)
);
return (
<EuiToolTip position="top" content={slo.name} display="block">
Expand Down Expand Up @@ -453,8 +439,7 @@ export function SloListCompactView({ sloList, loading, error }: Props) {
const historicalSliData = formatHistoricalData(
historicalSummaries.find(
(historicalSummary) =>
historicalSummary.sloId === slo.id &&
historicalSummary.instanceId === (slo.instanceId ?? ALL_VALUE)
historicalSummary.sloId === slo.id && historicalSummary.instanceId === slo.instanceId
)?.data,
'sli_value'
);
Expand Down Expand Up @@ -487,8 +472,7 @@ export function SloListCompactView({ sloList, loading, error }: Props) {
const errorBudgetBurnDownData = formatHistoricalData(
historicalSummaries.find(
(historicalSummary) =>
historicalSummary.sloId === slo.id &&
historicalSummary.instanceId === (slo.instanceId ?? ALL_VALUE)
historicalSummary.sloId === slo.id && historicalSummary.instanceId === slo.instanceId
)?.data,
'error_budget_remaining'
);
Expand Down
Loading

0 comments on commit df573d7

Please sign in to comment.