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

[ML] Show at least one correlation value and consolidate correlations columns #126683

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,17 @@

import { i18n } from '@kbn/i18n';

export const FAILED_TRANSACTIONS_IMPACT_THRESHOLD = {
HIGH: i18n.translate(
'xpack.apm.correlations.failedTransactions.highImpactText',
{
defaultMessage: 'High',
}
),
MEDIUM: i18n.translate(
'xpack.apm.correlations.failedTransactions.mediumImpactText',
{
defaultMessage: 'Medium',
}
),
LOW: i18n.translate(
'xpack.apm.correlations.failedTransactions.lowImpactText',
{
defaultMessage: 'Low',
}
),
export const CORRELATIONS_IMPACT_THRESHOLD = {
HIGH: i18n.translate('xpack.apm.correlations.highImpactText', {
defaultMessage: 'High',
}),
MEDIUM: i18n.translate('xpack.apm.correlations.mediumImpactText', {
defaultMessage: 'Medium',
}),
LOW: i18n.translate('xpack.apm.correlations.lowImpactText', {
defaultMessage: 'Low',
}),
VERY_LOW: i18n.translate('xpack.apm.correlations.veryLowImpactText', {
defaultMessage: 'Very low',
}),
} as const;
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { FieldValuePair, HistogramItem } from '../types';

import { FAILED_TRANSACTIONS_IMPACT_THRESHOLD } from './constants';
import { CORRELATIONS_IMPACT_THRESHOLD } from './constants';
import { FieldStats } from '../field_stats_types';

export interface FailedTransactionsCorrelation extends FieldValuePair {
Expand All @@ -22,7 +22,7 @@ export interface FailedTransactionsCorrelation extends FieldValuePair {
}

export type FailedTransactionsCorrelationsImpactThreshold =
typeof FAILED_TRANSACTIONS_IMPACT_THRESHOLD[keyof typeof FAILED_TRANSACTIONS_IMPACT_THRESHOLD];
typeof CORRELATIONS_IMPACT_THRESHOLD[keyof typeof CORRELATIONS_IMPACT_THRESHOLD];

export interface FailedTransactionsCorrelationsResponse {
ccsWarning: boolean;
Expand All @@ -31,4 +31,5 @@ export interface FailedTransactionsCorrelationsResponse {
overallHistogram?: HistogramItem[];
errorHistogram?: HistogramItem[];
fieldStats?: FieldStats[];
fallbackResult?: FailedTransactionsCorrelation;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import { FieldStats } from '../field_stats_types';

export interface LatencyCorrelation extends FieldValuePair {
correlation: number;
histogram: HistogramItem[];
histogram?: HistogramItem[];
ksTest: number;
isFallbackResult?: boolean;
}

export interface LatencyCorrelationsResponse {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/apm/common/correlations/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface FieldValuePair {
// but for example `http.response.status_code` which is part of
// of the list of predefined field candidates is of type long/number.
fieldValue: string | number;
isFallbackResult?: boolean;
}

export interface HistogramItem {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@ import { i18n } from '@kbn/i18n';

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

import { asPercent } from '../../../../common/utils/formatters';
import {
asPercent,
asPreciseDecimal,
} from '../../../../common/utils/formatters';
import { FailedTransactionsCorrelation } from '../../../../common/correlations/failed_transactions_correlations/types';
import { FieldStats } from '../../../../common/correlations/field_stats_types';

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

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

import { CorrelationsTable } from './correlations_table';
Expand Down Expand Up @@ -239,11 +240,7 @@ export function FailedTransactionsCorrelations({
</>
),
render: (_, { normalizedScore }) => {
return (
<>
<ImpactBar size="m" value={normalizedScore * 100} />
</>
);
return <div>{asPreciseDecimal(normalizedScore, 2)}</div>;
},
sortable: true,
},
Expand All @@ -260,8 +257,11 @@ export function FailedTransactionsCorrelations({
)}
</>
),
render: (_, { pValue }) => {
const label = getFailedTransactionsCorrelationImpactLabel(pValue);
render: (_, { pValue, isFallbackResult }) => {
const label = getFailedTransactionsCorrelationImpactLabel(
pValue,
isFallbackResult
);
return label ? (
<EuiBadge color={label.color}>{label.impact}</EuiBadge>
) : null;
Expand Down Expand Up @@ -377,18 +377,30 @@ export function FailedTransactionsCorrelations({
sort: { field: sortField, direction: sortDirection },
};

const correlationTerms = useMemo(
() =>
orderBy(
response.failedTransactionsCorrelations,
// The smaller the p value the higher the impact
// So we want to sort by the normalized score here
// which goes from 0 -> 1
sortField === 'pValue' ? 'normalizedScore' : sortField,
sortDirection
),
[response.failedTransactionsCorrelations, sortField, sortDirection]
);
const correlationTerms = useMemo(() => {
if (
progress.loaded === 1 &&
response?.failedTransactionsCorrelations?.length === 0 &&
response.fallbackResult !== undefined
) {
return [{ ...response.fallbackResult, isFallbackResult: true }];
}

return orderBy(
response.failedTransactionsCorrelations,
// The smaller the p value the higher the impact
// So we want to sort by the normalized score here
// which goes from 0 -> 1
sortField === 'pValue' ? 'normalizedScore' : sortField,
sortDirection
);
}, [
response.failedTransactionsCorrelations,
response.fallbackResult,
progress.loaded,
sortField,
sortDirection,
]);

const [pinnedSignificantTerm, setPinnedSignificantTerm] =
useState<FailedTransactionsCorrelation | null>(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@

import { i18n } from '@kbn/i18n';
import { EuiTheme } from '../../../../../../../src/plugins/kibana_react/common';
import type {
FieldValuePair,
HistogramItem,
} from '../../../../common/correlations/types';
import type { HistogramItem } from '../../../../common/correlations/types';
import { TransactionDistributionChartData } from '../../shared/charts/transaction_distribution_chart';
import { LatencyCorrelation } from '../../../../common/correlations/latency_correlations/types';
import { FailedTransactionsCorrelation } from '../../../../common/correlations/failed_transactions_correlations/types';

export function getTransactionDistributionChartData({
euiTheme,
Expand All @@ -22,7 +21,7 @@ export function getTransactionDistributionChartData({
euiTheme: EuiTheme;
allTransactionsHistogram?: HistogramItem[];
failedTransactionsHistogram?: HistogramItem[];
selectedTerm?: FieldValuePair & { histogram: HistogramItem[] };
selectedTerm?: LatencyCorrelation | FailedTransactionsCorrelation | undefined;
}) {
const transactionDistributionChartData: TransactionDistributionChartData[] =
[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ import {
EuiSpacer,
EuiTitle,
EuiToolTip,
EuiBadge,
} from '@elastic/eui';
import { Direction } from '@elastic/eui/src/services/sort/sort_direction';
import { EuiTableSortingType } from '@elastic/eui/src/components/basic_table/table_types';

import { i18n } from '@kbn/i18n';

import { FormattedMessage } from '@kbn/i18n-react';
import { useUiTracker } from '../../../../../observability/public';

import { asPreciseDecimal } from '../../../../common/utils/formatters';
Expand All @@ -48,6 +50,18 @@ import { getTransactionDistributionChartData } from './get_transaction_distribut
import { useTheme } from '../../../hooks/use_theme';
import { ChartTitleToolTip } from './chart_title_tool_tip';
import { MIN_TAB_TITLE_HEIGHT } from '../transaction_details/distribution';
import { getLatencyCorrelationImpactLabel } from './utils/get_failed_transactions_correlation_impact_label';

export function FallbackCorrelationBadge() {
return (
<EuiBadge>
<FormattedMessage
id="xpack.apm.correlations.latencyCorrelations.fallbackCorrelationBadgeMessage"
defaultMessage="Very low"
/>
</EuiBadge>
);
}

export function LatencyCorrelations({ onFilter }: { onFilter: () => void }) {
const {
Expand Down Expand Up @@ -151,6 +165,31 @@ export function LatencyCorrelations({ onFilter }: { onFilter: () => void }) {
},
sortable: true,
},
{
width: '116px',
field: 'pValue',
name: (
<>
{i18n.translate(
'xpack.apm.correlations.failedTransactions.correlationsTable.impactLabel',
{
defaultMessage: 'Impact',
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to add an info tooltip for the Impact column, and one for the Score column on the failed transactions tab, if only to say what the range of values is.

Copy link
Contributor

@lcawl lcawl Mar 3, 2022

Choose a reason for hiding this comment

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

The "impact" seems pretty straight-forward to me and is already mentioned in the popover help for these pages, so IMO a tooltip there isn't necessary.

However, I think it makes sense to have a tooltip for "score". Can we base it on what we have
for "correlation"?:

'The correlation score [0-1] of an attribute; the greater the score, the more an attribute increases latency.',

For example: "The score [0-1] of an attribute; the greater the score, the more an attribute contributes to failed transactions." or "more likely it is that an attribute..."

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here abb1e2a

}
)}
</>
),
render: (_, { correlation, isFallbackResult }) => {
const label = getLatencyCorrelationImpactLabel(
correlation,
isFallbackResult
);
return label ? (
<EuiBadge color={label.color}>{label.impact}</EuiBadge>
) : null;
},
sortable: true,
},

{
field: 'fieldName',
name: i18n.translate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export function useFailedTransactionsCorrelations() {
// and histogram data for statistically significant results.
const responseUpdate: FailedTransactionsCorrelationsResponse = {
ccsWarning: false,
fallbackResult: undefined,
};

const [overallHistogramResponse, errorHistogramRespone] =
Expand Down Expand Up @@ -149,6 +150,7 @@ export function useFailedTransactionsCorrelations() {

const failedTransactionsCorrelations: FailedTransactionsCorrelation[] =
[];
let fallbackResult: FailedTransactionsCorrelation | undefined;
const fieldsToSample = new Set<string>();
const chunkSize = 10;
let chunkLoadCounter = 0;
Expand Down Expand Up @@ -177,6 +179,21 @@ export function useFailedTransactionsCorrelations() {
getFailedTransactionsCorrelationsSortedByScore([
...failedTransactionsCorrelations,
]);
} else {
// If there's no significant correlations found and there's a fallback result
// Update the highest ranked/scored fall back result
if (pValues.fallbackResult) {
if (!fallbackResult) {
fallbackResult = pValues.fallbackResult;
} else {
if (
pValues.fallbackResult.normalizedScore >
fallbackResult.normalizedScore
) {
fallbackResult = pValues.fallbackResult;
}
}
}
}

chunkLoadCounter++;
Expand Down Expand Up @@ -209,7 +226,12 @@ export function useFailedTransactionsCorrelations() {
);

responseUpdate.fieldStats = stats;
setResponse({ ...responseUpdate, loaded: LOADED_DONE, isRunning: false });
setResponse({
...responseUpdate,
fallbackResult,
loaded: LOADED_DONE,
isRunning: false,
});
Comment on lines +229 to +234
Copy link
Member

@sorenlouv sorenlouv Mar 9, 2022

Choose a reason for hiding this comment

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

Now that we are no longer using kibana search strategies, would it be possible to use useFetcher for loading data or is that still blocked by something?

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed in Slack, since this might be out of scope of the PR, let's table this for now and revisit in the future for a better implementation.

setResponse.flush();
} catch (e) {
if (!abortCtrl.current.signal.aborted) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ export function useLatencyCorrelations() {
chunkSize
);

const fallbackResults: LatencyCorrelation[] = [];
for (const fieldValuePairChunk of fieldValuePairChunks) {
const significantCorrelations = await callApmApi(
'POST /internal/apm/correlations/significant_correlations',
Expand All @@ -197,6 +198,12 @@ export function useLatencyCorrelations() {
);
responseUpdate.latencyCorrelations =
getLatencyCorrelationsSortedByCorrelation([...latencyCorrelations]);
} else {
// If there's no correlation results that matches the criteria
// Consider the fallback results
if (significantCorrelations.fallbackResult) {
fallbackResults.push(significantCorrelations.fallbackResult);
}
}

chunkLoadCounter++;
Expand All @@ -213,6 +220,23 @@ export function useLatencyCorrelations() {
}
}

if (latencyCorrelations.length === 0 && fallbackResults.length > 0) {
// Rank the fallback results and show at least one value
const sortedFallbackResults = fallbackResults
.filter((r) => r.correlation > 0)
.sort((a, b) => b.correlation - a.correlation);

responseUpdate.latencyCorrelations = sortedFallbackResults
.slice(0, 1)
.map((r) => ({ ...r, isFallbackResult: true }));
setResponse({
...responseUpdate,
loaded:
LOADED_FIELD_VALUE_PAIRS +
(chunkLoadCounter / fieldValuePairChunks.length) *
PROGRESS_STEP_CORRELATIONS,
});
}
setResponse.flush();

const { stats } = await callApmApi(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@
*/

import { getFailedTransactionsCorrelationImpactLabel } from './get_failed_transactions_correlation_impact_label';
import { FAILED_TRANSACTIONS_IMPACT_THRESHOLD } from '../../../../../common/correlations/failed_transactions_correlations/constants';
import { CORRELATIONS_IMPACT_THRESHOLD } from '../../../../../common/correlations/failed_transactions_correlations/constants';

const EXPECTED_RESULT = {
HIGH: {
impact: FAILED_TRANSACTIONS_IMPACT_THRESHOLD.HIGH,
impact: CORRELATIONS_IMPACT_THRESHOLD.HIGH,
color: 'danger',
},
MEDIUM: {
impact: FAILED_TRANSACTIONS_IMPACT_THRESHOLD.MEDIUM,
impact: CORRELATIONS_IMPACT_THRESHOLD.MEDIUM,
color: 'warning',
},
LOW: {
impact: FAILED_TRANSACTIONS_IMPACT_THRESHOLD.LOW,
impact: CORRELATIONS_IMPACT_THRESHOLD.LOW,
color: 'default',
},
};
Expand Down
Loading