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.

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

Add SLO create callout to service overview, transactions page and transactions details #159958

Merged
merged 7 commits into from
Jun 20, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
EuiFlexItem,
EuiLink,
EuiPanel,
EuiSpacer,
} from '@elastic/eui';
import {
isRumAgentName,
Expand All @@ -36,6 +37,8 @@ import { ServiceOverviewDependenciesTable } from './service_overview_dependencie
import { ServiceOverviewErrorsTable } from './service_overview_errors_table';
import { ServiceOverviewInstancesChartAndTable } from './service_overview_instances_chart_and_table';
import { ServiceOverviewThroughputChart } from './service_overview_throughput_chart';
import { SloCallout } from '../../shared/slo_callout';
import { useLocalStorage } from '../../../hooks/use_local_storage';
/**
* The height a chart should be if it's next to a table with 5 rows and a title.
* Add the height of the pagination row.
Expand All @@ -49,7 +52,7 @@ export function ServiceOverview() {

const {
query,
query: { kuery, environment, rangeFrom, rangeTo },
query: { kuery, environment, rangeFrom, rangeTo, transactionType },
} = useApmParams('/services/{serviceName}/overview');

const { start, end } = useTimeRange({ rangeFrom, rangeTo });
Expand All @@ -76,13 +79,29 @@ export function ServiceOverview() {
? 'column'
: 'row';

const [sloCalloutDismissed, setSloCalloutDismissed] = useLocalStorage(
Copy link
Contributor

@yngrdyn yngrdyn Jun 20, 2023

Choose a reason for hiding this comment

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

Are we cleaning this value? if not, this user will not see the callout again unless he is using another computer, incognito mode, another window, etc. Is that the intention? if that's not the intention maybe using sessionStorage could help us here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, as far as I know if the user clicks to hide the callout it's not going to show again as we do in other places

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we discussed that the callout will be hidden if the user chooses to hide it or clicks the button to crate an SLO.

'apm.sloCalloutDismissed',
false
);

return (
<AnnotationsContextProvider
serviceName={serviceName}
environment={environment}
start={start}
end={end}
>
{!sloCalloutDismissed && (
Copy link
Contributor

@yngrdyn yngrdyn Jun 20, 2023

Choose a reason for hiding this comment

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

Shouldn't we also show this callout if an SLO is not current configured for this variables? or are we planning to show it even when the user has one configured?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I think after creating one we shouldn’t show the callout again, @gbamparop do we know if we can check if there are SLOs already created for the user?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point, we could reach to the AO team for this, but I would leave it simple for 8.9.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please create a follow up story for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are dismissing the callout after navigating to create SLO, change added here 2a6aa66

Copy link
Contributor

@yngrdyn yngrdyn Jun 20, 2023

Choose a reason for hiding this comment

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

What about detecting if an SLO rule is already created? Maybe a follow up for that one?

Copy link
Contributor Author

@MiriamAparicio MiriamAparicio Jun 20, 2023

Choose a reason for hiding this comment

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

@yngrdyn I created this issue #160017

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't offer yet an easy way to search for an SLO with specific params. You could search for all SLOs created with the sli.apm.transactionErrorRate or sli.apm.transactionDuration indicator, but that would require to handle pagination, etc.. I would not recommend doing this.

On a side note, are we planning of supporting creating SLO with the sli.apm.transactionDuration indicator with the threshold value set to a default value like the 95th (99th? avg?) percentile latency of the (service, env, transactionType, transactionName) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @kdelemme, I think for the porpoise of this, checking if there's at least one created will work for us.

<SloCallout
dismissCallout={() => {
setSloCalloutDismissed(true);
}}
serviceName={serviceName}
environment={environment}
transactionType={transactionType}
/>
)}
<EuiSpacer />
<ChartPointerEventContextProvider>
<EuiFlexGroup direction="column" gutterSize="s">
{fallbackToTransactions && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import { TransactionCharts } from '../../shared/charts/transaction_charts';
import { replace } from '../../shared/links/url_helpers';
import { TransactionDetailsTabs } from './transaction_details_tabs';
import { isServerlessAgent } from '../../../../common/agent_name';
import { useLocalStorage } from '../../../hooks/use_local_storage';
import { SloCallout } from '../../shared/slo_callout';

export function TransactionDetails() {
const { path, query } = useAnyOfApmParams(
Expand All @@ -32,6 +34,7 @@ export function TransactionDetails() {
transactionType: transactionTypeFromUrl,
comparisonEnabled,
offset,
environment,
} = query;
const { start, end } = useTimeRange({ rangeFrom, rangeTo });
const apmRouter = useApmRouter();
Expand Down Expand Up @@ -61,9 +64,24 @@ export function TransactionDetails() {
);

const isServerless = isServerlessAgent(serverlessType);
const [sloCalloutDismissed, setSloCalloutDismissed] = useLocalStorage(
'apm.sloCalloutDismissed',
false
);

return (
<>
{!sloCalloutDismissed && (
<SloCallout
dismissCallout={() => {
setSloCalloutDismissed(true);
}}
serviceName={serviceName}
environment={environment}
transactionType={transactionType}
transactionName={transactionName}
/>
)}
{fallbackToTransactions && <AggregatedTransactionsBadge />}
<EuiSpacer size="s" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ import { useHistory } from 'react-router-dom';
import { isServerlessAgent } from '../../../../common/agent_name';
import { useApmServiceContext } from '../../../context/apm_service/use_apm_service_context';
import { useApmParams } from '../../../hooks/use_apm_params';
import { useLocalStorage } from '../../../hooks/use_local_storage';
import { useTimeRange } from '../../../hooks/use_time_range';
import { AggregatedTransactionsBadge } from '../../shared/aggregated_transactions_badge';
import { TransactionCharts } from '../../shared/charts/transaction_charts';
import { replace } from '../../shared/links/url_helpers';
import { SloCallout } from '../../shared/slo_callout';
import { TransactionsTable } from '../../shared/transactions_table';

export function TransactionOverview() {
Expand Down Expand Up @@ -48,8 +50,23 @@ export function TransactionOverview() {

const isServerless = isServerlessAgent(serverlessType);

const [sloCalloutDismissed, setSloCalloutDismissed] = useLocalStorage(
'apm.sloCalloutDismissed',
false
);

return (
<>
{!sloCalloutDismissed && (
<SloCallout
dismissCallout={() => {
setSloCalloutDismissed(true);
}}
serviceName={serviceName}
environment={environment}
transactionType={transactionType}
/>
)}
{fallbackToTransactions && (
<>
<EuiFlexGroup>
Expand Down
114 changes: 114 additions & 0 deletions x-pack/plugins/apm/public/components/shared/slo_callout/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* 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 {
EuiButton,
EuiCallOut,
EuiFlexGroup,
EuiFlexItem,
EuiLink,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { CreateSLOInput } from '@kbn/slo-schema';
import { encode } from '@kbn/rison';
import React from 'react';
import { FormattedMessage } from '@kbn/i18n-react';
import { useApmPluginContext } from '../../../context/apm_plugin/use_apm_plugin_context';

interface Props {
dismissCallout: () => void;
serviceName: string;
environment: string;
transactionType?: string;
transactionName?: string;
}
type RecursivePartial<T> = {
[P in keyof T]?: T[P] extends Array<infer U>
? Array<RecursivePartial<U>>
: T[P] extends object | undefined
? RecursivePartial<T[P]>
: T[P];
};

export function SloCallout({
dismissCallout,
serviceName,
environment,
transactionType,
transactionName,
}: Props) {
const { core } = useApmPluginContext();
const { basePath } = core.http;

const sloInput: RecursivePartial<CreateSLOInput> = {
indicator: {
type: 'sli.apm.transactionErrorRate',
params: {
service: serviceName,
environment,
transactionName: transactionName ?? '',
transactionType: transactionType ?? '',
},
},
};
const sloParams = `?_a=${encode(sloInput)}`;

return (
<EuiCallOut
title={i18n.translate('xpack.apm.slo.callout.title', {
defaultMessage: 'Respond quicker with SLOs',
})}
iconType="lock"
>
<EuiFlexGroup direction="row" gutterSize="s" alignItems="center">
<EuiFlexItem>
<p>
<FormattedMessage
id="xpack.apm.slo.callout.description"
defaultMessage="How quickly will you respond if the service goes down? Keep the performance, speed and user experience high with a SLO"
/>
</p>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup>
<EuiFlexItem>
<EuiLink
data-test-subj="apmCreateSloLink"
href={basePath.prepend(
`/app/observability/slos/create${sloParams}`
)}
Comment on lines +79 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

😍 Looking forward to see this in APM 👏🏻

>
<EuiButton
data-test-subj="apmSloCalloutCreateSloButton"
onClick={() => {
dismissCallout();
}}
>
{i18n.translate('xpack.apm.slo.callout.createButton', {
defaultMessage: 'Create SLO',
})}
</EuiButton>
</EuiLink>
</EuiFlexItem>
<EuiFlexItem>
<EuiButton
color="text"
data-test-subj="apmSloDismissButton"
onClick={() => {
dismissCallout();
}}
>
{i18n.translate('xpack.apm.slo.callout.dimissButton', {
defaultMessage: 'Hide this',
})}
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
</EuiCallOut>
);
}
1 change: 1 addition & 0 deletions x-pack/plugins/apm/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
"@kbn/dashboard-plugin",
"@kbn/controls-plugin",
"@kbn/core-http-server",
"@kbn/slo-schema",
],
"exclude": [
"target/**/*",
Expand Down