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

[ML] APM Correlations: Fix usage in load balancing/HA setups. #115145

Merged
merged 52 commits into from
Nov 9, 2021
Merged

[ML] APM Correlations: Fix usage in load balancing/HA setups. #115145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
e1fa9c4
[ML] Move data fetching for overall latency histogram to custom hook.
walterra Oct 14, 2021
46f59d6
[ML] Migrates field candidates to regular endpoint.
walterra Oct 15, 2021
0e37684
[ML] Fetch latency correlations via regular endpoints.
walterra Oct 15, 2021
7d75d5f
[ML] Fetch failed transaction correlations via regular endpoints.
walterra Oct 16, 2021
37aa29b
[ML] Fix types.
walterra Oct 17, 2021
414a992
[ML] Rename common/search_strategies to common/correlations.
walterra Oct 17, 2021
83f5ae0
[ML] Rename server/lib/search_strategies to server/lib/correlations.
walterra Oct 17, 2021
8d92d94
[ML] Fix API integration tests.
walterra Oct 17, 2021
9de6ff6
[ML] Remove the no longer needed 'took' attribute.
walterra Oct 17, 2021
30d3883
[ML] Adds chunking to field value pair loading.
walterra Oct 17, 2021
91e5f13
[ML] Fix ccsWarning.
walterra Oct 17, 2021
c1cf1e5
[ML] Fix jest test.
walterra Oct 17, 2021
72eb07d
[ML] Fix item check.
walterra Oct 17, 2021
ba86ce1
[ML] Refactor away from using generators.
walterra Oct 18, 2021
3368aba
[ML] Remove references to log.
walterra Oct 19, 2021
430aa12
[ML] Get rid of SearchStrategy references in type/var names.
walterra Oct 19, 2021
aa80643
[ML] Deduplicate some code.
walterra Oct 19, 2021
4f080bf
[ML] Adds debouncing to correlation analysis.
walterra Oct 19, 2021
8868bb9
[ML] Fix types.
walterra Oct 19, 2021
5a3b927
[ML] Cleanup. Fix progress.
walterra Oct 20, 2021
9ccd53b
[ML] Add comments.
walterra Oct 20, 2021
d15d18e
[ML] Fix chart labels.
walterra Oct 20, 2021
8bb304b
[ML] Add platinum license guard to correlations API endpoints.
walterra Oct 20, 2021
10952cc
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
walterra Oct 29, 2021
da37fd3
[ML] Move progress parts to constants.
walterra Oct 29, 2021
cbeb5ef
[ML] Adds comments to explain the purpose of responseUpdate.
walterra Oct 29, 2021
a0992ef
[ML] Remove unnecessary cast to string.
walterra Oct 29, 2021
a6e75d7
[ML] Remove usage of deprecated useUrlParams.
walterra Oct 29, 2021
deddd79
[ML] Remove casting as Response.
walterra Oct 29, 2021
45da6c5
[ML] Add abort signal.
walterra Oct 29, 2021
71270c1
[ML] Switch endpoint to POST.
walterra Oct 29, 2021
312e9b2
[ML] Improved error handling.
walterra Oct 29, 2021
0cdb94f
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
kibanamachine Nov 1, 2021
f0f0d44
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
kibanamachine Nov 2, 2021
4497e74
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
walterra Nov 2, 2021
81139a8
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
walterra Nov 2, 2021
87c4222
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
walterra Nov 3, 2021
f47e654
[ML] Fix unmounting. Fix percentileThresholdValue. Add unit tests to …
walterra Nov 3, 2021
b65bab6
[ML] Focus assertion on error only.
walterra Nov 3, 2021
c0b6b3e
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
walterra Nov 3, 2021
2b755d3
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
kibanamachine Nov 3, 2021
5310686
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
kibanamachine Nov 3, 2021
0e5de24
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
walterra Nov 5, 2021
55472ef
[ML] Tweak test assertions and adds tests for cancellation.
walterra Nov 5, 2021
72bcc11
[ML] Adds comment to clarify the use of abortControler and isCancelle…
walterra Nov 5, 2021
3e411cd
Merge branch 'ml-apm-correlations-fix-load-balancing' of github.com:w…
walterra Nov 5, 2021
97890cf
[ML] Use abort signal instead of isCancelledRef.
walterra Nov 5, 2021
24d1d2b
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
walterra Nov 5, 2021
86b78c1
[ML] Fix test imports.
walterra Nov 5, 2021
de63d24
Merge branch 'main' into ml-apm-correlations-fix-load-balancing
walterra Nov 8, 2021
22f785c
fix field value pair error handling
walterra Nov 8, 2021
9872afc
[ML] Fix jest test.
walterra Nov 8, 2021
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
8 changes: 2 additions & 6 deletions ...server/lib/search_strategies/constants.ts → ...gins/apm/common/correlations/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,5 @@ export const KS_TEST_THRESHOLD = 0.1;

export const ERROR_CORRELATION_THRESHOLD = 0.02;

/**
* Field stats/top values sampling constants
*/

export const SAMPLER_TOP_TERMS_THRESHOLD = 100000;
export const SAMPLER_TOP_TERMS_SHARD_SIZE = 5000;
export const DEFAULT_PERCENTILE_THRESHOLD = 95;
export const DEBOUNCE_INTERVAL = 100;
0 ...ed_transactions_correlations/constants.ts → ...ed_transactions_correlations/constants.ts
View file
Open in desktop
File renamed without changes.
8 changes: 2 additions & 6 deletions ...failed_transactions_correlations/types.ts → ...failed_transactions_correlations/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,8 @@ export interface FailedTransactionsCorrelation extends FieldValuePair {
export type FailedTransactionsCorrelationsImpactThreshold =
typeof FAILED_TRANSACTIONS_IMPACT_THRESHOLD[keyof typeof FAILED_TRANSACTIONS_IMPACT_THRESHOLD];

export interface FailedTransactionsCorrelationsParams {
percentileThreshold: number;
}

export interface FailedTransactionsCorrelationsRawResponse {
log: string[];
export interface FailedTransactionsCorrelationsResponse {
ccsWarning: boolean;
failedTransactionsCorrelations?: FailedTransactionsCorrelation[];
percentileThresholdValue?: number;
overallHistogram?: HistogramItem[];
Expand Down
4 changes: 2 additions & 2 deletions ...on/search_strategies/field_stats_types.ts → .../common/correlations/field_stats_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
*/

import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { SearchStrategyParams } from './types';
import { CorrelationsParams } from './types';

export interface FieldStatsCommonRequestParams extends SearchStrategyParams {
export interface FieldStatsCommonRequestParams extends CorrelationsParams {
samplerShardSize: number;
}

Expand Down
18 changes: 2 additions & 16 deletions ..._strategies/latency_correlations/types.ts → ...orrelations/latency_correlations/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,8 @@ export interface LatencyCorrelation extends FieldValuePair {
ksTest: number;
}

export interface LatencyCorrelationSearchServiceProgress {
started: number;
loadedHistogramStepsize: number;
loadedOverallHistogram: number;
loadedFieldCandidates: number;
loadedFieldValuePairs: number;
loadedHistograms: number;
}

export interface LatencyCorrelationsParams {
percentileThreshold: number;
analyzeCorrelations: boolean;
}

export interface LatencyCorrelationsRawResponse {
log: string[];
export interface LatencyCorrelationsResponse {
ccsWarning: boolean;
overallHistogram?: HistogramItem[];
percentileThresholdValue?: number;
latencyCorrelations?: LatencyCorrelation[];
Expand Down
23 changes: 4 additions & 19 deletions ...ins/apm/common/search_strategies/types.ts → .../plugins/apm/common/correlations/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,35 +26,20 @@ export interface ResponseHit {
_source: ResponseHitSource;
}

export interface RawResponseBase {
ccsWarning: boolean;
took: number;
}

export interface SearchStrategyClientParamsBase {
export interface CorrelationsClientParams {
environment: string;
kuery: string;
serviceName?: string;
transactionName?: string;
transactionType?: string;
}

export interface RawSearchStrategyClientParams
extends SearchStrategyClientParamsBase {
start?: string;
end?: string;
}

export interface SearchStrategyClientParams
extends SearchStrategyClientParamsBase {
start: number;
end: number;
}

export interface SearchStrategyServerParams {
export interface CorrelationsServerParams {
index: string;
includeFrozen?: boolean;
}

export type SearchStrategyParams = SearchStrategyClientParams &
SearchStrategyServerParams;
export type CorrelationsParams = CorrelationsClientParams &
CorrelationsServerParams;
0 ...get_prioritized_field_value_pairs.test.ts → ...get_prioritized_field_value_pairs.test.ts
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions ...ries/get_prioritized_field_value_pairs.ts → ...tils/get_prioritized_field_value_pairs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
*/

import { FIELDS_TO_ADD_AS_CANDIDATE } from '../constants';
import { hasPrefixToInclude } from '../utils';
import { hasPrefixToInclude } from './has_prefix_to_include';

import type { FieldValuePair } from '../../../../common/search_strategies/types';
import type { FieldValuePair } from '../types';

export const getPrioritizedFieldValuePairs = (
fieldValuePairs: FieldValuePair[]
Expand Down
0 ...egies/utils/has_prefix_to_include.test.ts → ...tions/utils/has_prefix_to_include.test.ts
View file
Open in desktop
File renamed without changes.
0 ...strategies/utils/has_prefix_to_include.ts → ...rrelations/utils/has_prefix_to_include.ts
View file
Open in desktop
File renamed without changes.
3 changes: 2 additions & 1 deletion ...failed_transactions_correlations/index.ts → ...ns/apm/common/correlations/utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
* 2.0.
*/

export { failedTransactionsCorrelationsSearchServiceProvider } from './failed_transactions_correlations_search_service';
export { getPrioritizedFieldValuePairs } from './get_prioritized_field_value_pairs';
export { hasPrefixToInclude } from './has_prefix_to_include';
15 changes: 0 additions & 15 deletions x-pack/plugins/apm/common/search_strategies/constants.ts
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion x-pack/plugins/apm/public/components/app/correlations/context_popover/context_popover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
import React, { Fragment, useState } from 'react';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { FieldStats } from '../../../../../common/search_strategies/field_stats_types';
import { FieldStats } from '../../../../../common/correlations/field_stats_types';
import { OnAddFilter, TopValues } from './top_values';
import { useTheme } from '../../../../hooks/use_theme';

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/public/components/app/correlations/context_popover/top_values.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
EuiToolTip,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FieldStats } from '../../../../../common/search_strategies/field_stats_types';
import { FieldStats } from '../../../../../common/correlations/field_stats_types';
import { asPercent } from '../../../../../common/utils/formatters';
import { useTheme } from '../../../../hooks/use_theme';

Expand Down
38 changes: 0 additions & 38 deletions x-pack/plugins/apm/public/components/app/correlations/correlations_log.tsx
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion x-pack/plugins/apm/public/components/app/correlations/correlations_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type { Criteria } from '@elastic/eui/src/components/basic_table/basic_tab
import { FETCH_STATUS } from '../../../hooks/use_fetcher';
import { useUiTracker } from '../../../../../observability/public';
import { useTheme } from '../../../hooks/use_theme';
import type { FieldValuePair } from '../../../../common/search_strategies/types';
import type { FieldValuePair } from '../../../../common/correlations/types';

const PAGINATION_SIZE_OPTIONS = [5, 10, 20, 50];

Expand Down
53 changes: 19 additions & 34 deletions x-pack/plugins/apm/public/components/app/correlations/failed_transactions_correlations.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,45 +29,38 @@ import type { Direction } from '@elastic/eui/src/services/sort/sort_direction';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';

import {
enableInspectEsQueries,
useUiTracker,
} from '../../../../../observability/public';
import { useUiTracker } from '../../../../../observability/public';

import { asPercent } from '../../../../common/utils/formatters';
import { FailedTransactionsCorrelation } from '../../../../common/search_strategies/failed_transactions_correlations/types';
import {
APM_SEARCH_STRATEGIES,
DEFAULT_PERCENTILE_THRESHOLD,
} from '../../../../common/search_strategies/constants';
import { FieldStats } from '../../../../common/search_strategies/field_stats_types';
import { FailedTransactionsCorrelation } from '../../../../common/correlations/failed_transactions_correlations/types';
import { DEFAULT_PERCENTILE_THRESHOLD } from '../../../../common/correlations/constants';
import { FieldStats } from '../../../../common/correlations/field_stats_types';

import { useApmPluginContext } from '../../../context/apm_plugin/use_apm_plugin_context';
import { useLocalStorage } from '../../../hooks/useLocalStorage';
import { FETCH_STATUS } from '../../../hooks/use_fetcher';
import { useSearchStrategy } from '../../../hooks/use_search_strategy';
import { useTheme } from '../../../hooks/use_theme';

import { ImpactBar } from '../../shared/ImpactBar';
import { push } from '../../shared/Links/url_helpers';

import { CorrelationsTable } from './correlations_table';
import { FailedTransactionsCorrelationsHelpPopover } from './failed_transactions_correlations_help_popover';
import { isErrorMessage } from './utils/is_error_message';
import { getFailedTransactionsCorrelationImpactLabel } from './utils/get_failed_transactions_correlation_impact_label';
import { getOverallHistogram } from './utils/get_overall_histogram';
import {
TransactionDistributionChart,
TransactionDistributionChartData,
} from '../../shared/charts/transaction_distribution_chart';
import { CorrelationsLog } from './correlations_log';
import { CorrelationsEmptyStatePrompt } from './empty_state_prompt';
import { CrossClusterSearchCompatibilityWarning } from './cross_cluster_search_warning';
import { CorrelationsProgressControls } from './progress_controls';
import { useTransactionColors } from './use_transaction_colors';
import { CorrelationsContextPopover } from './context_popover';
import { OnAddFilter } from './context_popover/top_values';

import { useFailedTransactionsCorrelations } from './use_failed_transactions_correlations';

export function FailedTransactionsCorrelations({
onFilter,
}: {
Expand All @@ -77,18 +70,12 @@ export function FailedTransactionsCorrelations({
const transactionColors = useTransactionColors();

const {
core: { notifications, uiSettings },
core: { notifications },
} = useApmPluginContext();
const trackApmEvent = useUiTracker({ app: 'apm' });

const inspectEnabled = uiSettings.get<boolean>(enableInspectEsQueries);

const { progress, response, startFetch, cancelFetch } = useSearchStrategy(
APM_SEARCH_STRATEGIES.APM_FAILED_TRANSACTIONS_CORRELATIONS,
{
percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
}
);
const { progress, response, startFetch, cancelFetch } =
useFailedTransactionsCorrelations();

const fieldStats: Record<string, FieldStats> | undefined = useMemo(() => {
return response.fieldStats?.reduce((obj, field) => {
Expand All @@ -97,7 +84,6 @@ export function FailedTransactionsCorrelations({
}, {} as Record<string, FieldStats>);
}, [response?.fieldStats]);

const progressNormalized = progress.loaded / progress.total;
const { overallHistogram, hasData, status } = getOverallHistogram(
response,
progress.isRunning
Expand Down Expand Up @@ -368,7 +354,7 @@ export function FailedTransactionsCorrelations({
}, [fieldStats, onAddFilter, showStats]);

useEffect(() => {
if (isErrorMessage(progress.error)) {
if (progress.error) {
notifications.toasts.addDanger({
title: i18n.translate(
'xpack.apm.correlations.failedTransactions.errorTitle',
Expand All @@ -377,7 +363,7 @@ export function FailedTransactionsCorrelations({
'An error occurred performing correlations on failed transactions',
}
),
text: progress.error.toString(),
text: progress.error,
});
}
}, [progress.error, notifications.toasts]);
Expand Down Expand Up @@ -439,7 +425,7 @@ export function FailedTransactionsCorrelations({

const showCorrelationsEmptyStatePrompt =
correlationTerms.length < 1 &&
(progressNormalized === 1 || !progress.isRunning);
(progress.loaded === 1 || !progress.isRunning);

const transactionDistributionChartData: TransactionDistributionChartData[] =
[];
Expand All @@ -457,8 +443,8 @@ export function FailedTransactionsCorrelations({
if (Array.isArray(response.errorHistogram)) {
transactionDistributionChartData.push({
id: i18n.translate(
'xpack.apm.transactionDistribution.chart.allFailedTransactionsLabel',
{ defaultMessage: 'All failed transactions' }
'xpack.apm.transactionDistribution.chart.failedTransactionsLabel',
{ defaultMessage: 'Failed transactions' }
),
histogram: response.errorHistogram,
});
Expand Down Expand Up @@ -525,7 +511,7 @@ export function FailedTransactionsCorrelations({
<EuiText color="subdued" size="xs">
<FormattedMessage
id="xpack.apm.transactionDetails.tabs.failedTransactionsCorrelationsChartDescription"
defaultMessage="Log-log plot for latency (x) by transactions (y) with overlapping bands for {br}{allTransactions}, {allFailedTransactions} and {focusTransaction}."
defaultMessage="Log-log plot for latency (x) by transactions (y) with overlapping bands for {br}{allTransactions}, {failedTransactions} and {focusTransaction}."
Copy link
Member

Choose a reason for hiding this comment

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

I think this sentence it overly technical. Do we need to say that it's a log-log plot?

Copy link
Contributor

Choose a reason for hiding this comment

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

I quite like the use of log-log in the chart sub-title, as it helps to make it clear that log scales are used for both axes. The use of x and y is probably unnecessary - how about something like Log-log plot for count of transactions by latency with overlapping bands for .. ?

Copy link
Member

Choose a reason for hiding this comment

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

Can we take this by one of the copy writers? If you add a screenshot here to give some context, I think @bmorelli25 can give some feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sentence was introduced in #113039 (there's also screenshots in that PR's description), the wording didn't change in this PR besides the tweak sneaked in to remove "all" from "all failed transactions" decided in last weeks weekly. If we want to change the whole sentence can we move that discussion to a separate issue to not widen the scope of this PR?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I saw this in a previous PR as well. Just want to make sure that we don't release it as-is unless it's been ok'd by an Observability copywriter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an issue to follow up on this here: #117659

values={{
br: <br />,
allTransactions: (
Expand All @@ -536,13 +522,13 @@ export function FailedTransactionsCorrelations({
/>
</span>
),
allFailedTransactions: (
failedTransactions: (
<span
style={{ color: transactionColors.ALL_FAILED_TRANSACTIONS }}
>
<FormattedMessage
id="xpack.apm.transactionDetails.tabs.failedTransactionsCorrelationsChartAllFailedTransactions"
defaultMessage="all failed transactions"
id="xpack.apm.transactionDetails.tabs.failedTransactionsCorrelationsChartFailedTransactions"
defaultMessage="failed transactions"
/>
</span>
),
Expand Down Expand Up @@ -621,7 +607,7 @@ export function FailedTransactionsCorrelations({
<EuiSpacer size="s" />

<CorrelationsProgressControls
progress={progressNormalized}
progress={progress.loaded}
isRunning={progress.isRunning}
onRefresh={startFetch}
onCancel={cancelFetch}
Expand Down Expand Up @@ -654,7 +640,6 @@ export function FailedTransactionsCorrelations({
)}
{showCorrelationsEmptyStatePrompt && <CorrelationsEmptyStatePrompt />}
</div>
{inspectEnabled && <CorrelationsLog logMessages={response.log ?? []} />}
</div>
);
}
Loading