Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SLOs] In Embeddables, show all related instances option #175503

Merged
merged 20 commits into from
Feb 1, 2024
Merged

[SLOs] In Embeddables, show all related instances option #175503

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6f08f41
update tests
shahzad31 Jan 25, 2024
6848f2e
Merge branch 'main' of https://github.com/elastic/kibana into slo-car…
shahzad31 Jan 25, 2024
180de9e
update to show all related instances
shahzad31 Jan 25, 2024
5d526d8
Update x-pack/plugins/triggers_actions_ui/public/application/sections…
shahzad31 Jan 25, 2024
325d59a
Merge branch 'main' into slo-card-config
shahzad31 Jan 29, 2024
2e30069
Merge branch 'main' of https://github.com/elastic/kibana into slo-car…
shahzad31 Jan 30, 2024
c0d0d07
use ALL_VALUE
shahzad31 Jan 30, 2024
dded4ff
Merge branch 'slo-card-config' of https://github.com/shahzad31/kibana…
shahzad31 Jan 30, 2024
c974f12
fix alerts query
shahzad31 Jan 30, 2024
5e76b34
Merge branch 'main' of https://github.com/elastic/kibana into slo-car…
shahzad31 Jan 30, 2024
246ecfd
reload on save
shahzad31 Jan 30, 2024
53113ba
Merge branch 'main' of https://github.com/elastic/kibana into slo-car…
shahzad31 Jan 30, 2024
07c02b4
Update x-pack/plugins/observability/public/embeddable/slo/overview/sl…
shahzad31 Jan 31, 2024
d63412d
Update x-pack/plugins/observability/public/embeddable/slo/overview/sl…
shahzad31 Jan 31, 2024
fc88574
Merge branch 'main' of https://github.com/elastic/kibana into slo-car…
shahzad31 Jan 31, 2024
f990859
instances
shahzad31 Jan 31, 2024
1fe1d49
Merge branch 'main' of https://github.com/elastic/kibana into slo-car…
shahzad31 Feb 1, 2024
7f375c8
Merge branch 'slo-card-config' of https://github.com/shahzad31/kibana…
shahzad31 Feb 1, 2024
00e347a
update
shahzad31 Feb 1, 2024
0972b0a
fix 1i18n
shahzad31 Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions x-pack/plugins/observability/public/embeddable/slo/alerts/components/slo_alerts_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@ interface Props {
timeRange: TimeRange;
onLoaded?: () => void;
lastReloadRequestTime: number | undefined;
showAllGroupByInstances?: boolean;
}

export const getSloInstanceFilter = (sloId: string, sloInstanceId: string) => {
export const getSloInstanceFilter = (
sloId: string,
sloInstanceId: string,
showAllGroupByInstances = false
) => {
return {
bool: {
must: [
Expand All @@ -33,7 +38,7 @@ export const getSloInstanceFilter = (sloId: string, sloInstanceId: string) => {
'slo.id': sloId,
},
},
...(sloInstanceId !== ALL_VALUE
...(sloInstanceId !== ALL_VALUE && !showAllGroupByInstances
mgiota marked this conversation as resolved.
Show resolved Hide resolved
? [
{
term: {
Expand All @@ -47,7 +52,11 @@ export const getSloInstanceFilter = (sloId: string, sloInstanceId: string) => {
};
};

export const useSloAlertsQuery = (slos: SloItem[], timeRange: TimeRange) => {
export const useSloAlertsQuery = (
slos: SloItem[],
timeRange: TimeRange,
showAllGroupByInstances?: boolean
) => {
return useMemo(() => {
const query: AlertsTableStateProps['query'] = {
bool: {
Expand All @@ -66,25 +75,34 @@ export const useSloAlertsQuery = (slos: SloItem[], timeRange: TimeRange) => {
},
{
bool: {
should: slos.map((slo) => getSloInstanceFilter(slo.id, slo.instanceId)),
should: slos.map((slo) =>
getSloInstanceFilter(slo.id, slo.instanceId, showAllGroupByInstances)
mgiota marked this conversation as resolved.
Show resolved Hide resolved
),
},
},
],
},
};

return query;
}, [slos, timeRange]);
}, [showAllGroupByInstances, slos, timeRange.from]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to current changes, but I was wondering why we don't have the timeRange.to in the range query. If user selects to see alerts from a specific date to another date, then we don't take the to into consideration. Do we need to create a ticket for this?

range: {
              '@timestamp': {
                gte: timeRange.from,
              },
            },

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shahzad31 If user selects to time range in the timepicker, it won't be applied right? Shouldn't the range contain the timeRange.to as well? Do you think we can do this in this PR? Otherwise a follow up PR is fine as well

};

export function SloAlertsTable({ slos, deps, timeRange, onLoaded, lastReloadRequestTime }: Props) {
export function SloAlertsTable({
slos,
deps,
timeRange,
onLoaded,
lastReloadRequestTime,
showAllGroupByInstances,
}: Props) {
const {
triggersActionsUi: { alertsTableConfigurationRegistry, getAlertsStateTable: AlertsStateTable },
} = deps;

return (
<AlertsStateTable
query={useSloAlertsQuery(slos, timeRange)}
query={useSloAlertsQuery(slos, timeRange, showAllGroupByInstances)}
alertsTableConfigurationRegistry={alertsTableConfigurationRegistry}
configurationId={SLO_ALERTS_TABLE_CONFIG_ID}
featureIds={[AlertConsumers.SLO, AlertConsumers.OBSERVABILITY]}
Expand Down
7 changes: 6 additions & 1 deletion x-pack/plugins/observability/public/embeddable/slo/alerts/slo_alerts_embeddable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,11 @@ export class SLOAlertsEmbeddable extends AbstractEmbeddable<
const queryClient = new QueryClient();

const I18nContext = this.deps.i18n.Context;
const { slos, timeRange = { from: 'now-15m/m', to: 'now' } } = this.getInput();
const {
slos,
timeRange = { from: 'now-15m/m', to: 'now' },
showAllGroupByInstances,
} = this.getInput();

const deps = this.deps;
const kibanaVersion = this.kibanaVersion;
Expand All @@ -129,6 +133,7 @@ export class SLOAlertsEmbeddable extends AbstractEmbeddable<
slos={slos}
timeRange={timeRange}
reloadSubject={this.reloadSubject}
showAllGroupByInstances={showAllGroupByInstances}
/>
</QueryClientProvider>
</Router>
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/observability/public/embeddable/slo/alerts/slo_alerts_wrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ interface Props {
embeddable: IEmbeddable<SloAlertsEmbeddableInput, EmbeddableOutput>;
onRenderComplete?: () => void;
reloadSubject: Subject<SloAlertsEmbeddableInput | undefined>;
showAllGroupByInstances?: boolean;
}

export function SloAlertsWrapper({
Expand All @@ -38,6 +39,7 @@ export function SloAlertsWrapper({
timeRange: initialTimeRange,
onRenderComplete,
reloadSubject,
showAllGroupByInstances,
}: Props) {
const {
application: { navigateToUrl },
Expand Down Expand Up @@ -152,6 +154,7 @@ export function SloAlertsWrapper({
timeRange={timeRange}
onLoaded={() => setIsTableLoaded(true)}
lastReloadRequestTime={lastRefreshTime}
showAllGroupByInstances={showAllGroupByInstances}
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
26 changes: 25 additions & 1 deletion x-pack/plugins/observability/public/embeddable/slo/alerts/slo_configuration.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
EuiButtonEmpty,
EuiFlexGroup,
EuiFlexItem,
EuiSpacer,
EuiSwitch,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import { i18n } from '@kbn/i18n';
Expand All @@ -30,9 +32,16 @@ interface SloConfigurationProps {
}

export function SloConfiguration({ initialInput, onCreate, onCancel }: SloConfigurationProps) {
const [showAllGroupByInstances, setShowAllGroupByInstances] = useState(
initialInput?.showAllGroupByInstances ?? false
);
const [selectedSlos, setSelectedSlos] = useState(initialInput?.slos ?? []);

const [hasError, setHasError] = useState(false);
const onConfirmClick = () => onCreate({ slos: selectedSlos });

const onConfirmClick = () => onCreate({ slos: selectedSlos, showAllGroupByInstances });

const hasGroupBy = selectedSlos?.some((slo) => slo.instanceId !== '*');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use the ALL_VALUE here


return (
<EuiModal
Expand Down Expand Up @@ -71,6 +80,21 @@ export function SloConfiguration({ initialInput, onCreate, onCancel }: SloConfig
/>
</EuiFlexItem>
</EuiFlexGroup>
{hasGroupBy && (
<>
<EuiSpacer />
<EuiSwitch
label={i18n.translate(
'xpack.observability.sloConfiguration.euiSwitch.showAllGroupByLabel',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shahzad31 How about we prefix the copy with xpack.observability.sloAlertsEmbeddable to be consistent with rest i18n copies? It would become xpack.observability.sloAlertsEmbeddable.sloConfiguration.euiSwitch.showAllGroupByLabel that long though..

{ defaultMessage: 'Show all related group-by instances' }
)}
checked={showAllGroupByInstances}
onChange={(e) => {
setShowAllGroupByInstances(e.target.checked);
}}
/>
</>
)}
</EuiModalBody>
<EuiModalFooter>
<EuiButtonEmpty onClick={onCancel} data-test-subj="sloCancelButton">
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/observability/public/embeddable/slo/alerts/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface EmbeddableSloProps {
slos: SloItem[];
timeRange?: TimeRange;
lastReloadRequestTime?: number | undefined;
showAllGroupByInstances?: boolean;
}

export type SloAlertsEmbeddableInput = EmbeddableInput & EmbeddableSloProps;
25 changes: 24 additions & 1 deletion x-pack/plugins/observability/public/embeddable/slo/overview/slo_configuration.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
EuiButtonEmpty,
EuiFlexGroup,
EuiFlexItem,
EuiSwitch,
EuiSpacer,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
shahzad31 marked this conversation as resolved.
Show resolved Hide resolved
import { i18n } from '@kbn/i18n';
Expand All @@ -29,8 +31,14 @@ interface SloConfigurationProps {

export function SloConfiguration({ onCreate, onCancel }: SloConfigurationProps) {
const [selectedSlo, setSelectedSlo] = useState<EmbeddableSloProps>();
const [showAllGroupByInstances, setShowAllGroupByInstances] = useState(false);

const onConfirmClick = () =>
onCreate({ sloId: selectedSlo?.sloId, sloInstanceId: selectedSlo?.sloInstanceId });
onCreate({
showAllGroupByInstances,
sloId: selectedSlo?.sloId,
sloInstanceId: selectedSlo?.sloInstanceId,
});
const [hasError, setHasError] = useState(false);

return (
Expand All @@ -57,6 +65,21 @@ export function SloConfiguration({ onCreate, onCancel }: SloConfigurationProps)
/>
</EuiFlexItem>
</EuiFlexGroup>
{selectedSlo?.sloInstanceId !== '*' && (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use ALL_VALUE here

shahzad31 marked this conversation as resolved.
Show resolved Hide resolved
<>
<EuiSpacer />
<EuiSwitch
label={i18n.translate(
'xpack.observability.sloConfiguration.euiSwitch.showAllGroupByLabel',
{ defaultMessage: 'Show all related group-by instances' }
)}
checked={showAllGroupByInstances}
onChange={(e) => {
setShowAllGroupByInstances(e.target.checked);
}}
/>
</>
)}
</EuiModalBody>
<EuiModalFooter>
<EuiButtonEmpty onClick={onCancel} data-test-subj="sloCancelButton">
Expand Down
28 changes: 20 additions & 8 deletions x-pack/plugins/observability/public/embeddable/slo/overview/slo_embeddable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@ import {
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';

import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { type CoreStart, IUiSettingsClient, ApplicationStart } from '@kbn/core/public';
import {
type CoreStart,
IUiSettingsClient,
ApplicationStart,
NotificationsStart,
} from '@kbn/core/public';
import { Subject } from 'rxjs';
import { SloCardChartList } from './slo_overview_grid';
import { SloOverview } from './slo_overview';
import type { SloEmbeddableInput } from './types';

Expand All @@ -28,6 +34,7 @@ interface SloEmbeddableDeps {
http: CoreStart['http'];
i18n: CoreStart['i18n'];
application: ApplicationStart;
notifications: NotificationsStart;
}

export class SLOEmbeddable extends AbstractEmbeddable<SloEmbeddableInput, EmbeddableOutput> {
Expand Down Expand Up @@ -65,20 +72,25 @@ export class SLOEmbeddable extends AbstractEmbeddable<SloEmbeddableInput, Embedd
// required for the export feature to work
this.node.setAttribute('data-shared-item', '');

const { sloId, sloInstanceId } = this.getInput();
const { sloId, sloInstanceId, showAllGroupByInstances } = this.getInput();
const queryClient = new QueryClient();

const I18nContext = this.deps.i18n.Context;
ReactDOM.render(
<I18nContext>
<KibanaContextProvider services={this.deps}>
<QueryClientProvider client={queryClient}>
<SloOverview
onRenderComplete={() => this.onRenderComplete()}
sloId={sloId}
sloInstanceId={sloInstanceId}
reloadSubject={this.reloadSubject}
/>
{showAllGroupByInstances ? (
<SloCardChartList sloId={sloId!} />
) : (
<SloOverview
onRenderComplete={() => this.onRenderComplete()}
sloId={sloId}
sloInstanceId={sloInstanceId}
reloadSubject={this.reloadSubject}
showAllGroupByInstances={showAllGroupByInstances}
/>
)}
</QueryClientProvider>
</KibanaContextProvider>
</I18nContext>,
Expand Down
8 changes: 2 additions & 6 deletions x-pack/plugins/observability/public/embeddable/slo/overview/slo_embeddable_factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,8 @@ export class SloOverviewEmbeddableFactoryDefinition

public async create(initialInput: SloEmbeddableInput, parent?: IContainer) {
try {
const [{ uiSettings, application, http, i18n: i18nService }] = await this.getStartServices();
return new SLOEmbeddable(
{ uiSettings, application, http, i18n: i18nService },
initialInput,
parent
);
const [coreStart, pluginStart] = await this.getStartServices();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

return new SLOEmbeddable({ ...coreStart, ...pluginStart }, initialInput, parent);
} catch (e) {
return new ErrorEmbeddable(e, initialInput, parent);
}
Expand Down
Loading