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

qn895 · 2022-03-02T15:15:43Z

Summary

This PR updates it so that it will show at least one fall back correlation value when no other significant correlation is found. These will show up as 'Very low'

It also adds an impact column to the Latency tab, as well as converting the progress bar in Failed transactions tab to showing numerical value. This makes it consistent across both tabs.

### Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

peteharverson · 2022-03-03T14:21:54Z

x-pack/plugins/apm/public/components/app/correlations/latency_correlations.tsx

+              {i18n.translate(
+                'xpack.apm.correlations.failedTransactions.correlationsTable.impactLabel',
+                {
+                  defaultMessage: 'Impact',


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.

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"?:

kibana/x-pack/plugins/apm/public/components/app/correlations/latency_correlations.tsx

Line 129 in 247d3a5

'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..."

Updated here abb1e2a

peteharverson

Gave this a test against the apm-correlations-eden and apm-correlations-antifraud-hist data sets and overall looks good. I couldn't find any transactions which show fallback results in the failed transaction tab, but did for the Latency tab, and the consistent columns work well I think.

elasticmachine · 2022-03-03T16:04:23Z

Pinging @elastic/ml-ui (:ml)

elasticmachine · 2022-03-03T19:50:15Z

Pinging @elastic/apm-ui (Team:apm)

qn895 · 2022-03-07T15:34:51Z

@elasticmachine merge upstream

sorenlouv · 2022-03-09T16:38:46Z

x-pack/plugins/apm/server/routes/correlations/queries/query_significant_correlations.ts

+      [{ fieldName, fieldValue }]
+    );
+
+    if (typeof fallbackResult === 'object' && fallbackResult !== null) {


afaict from the type annotation fallbackResult is LatencyCorrelation | undefined. In other words: it's either an object, or it's undefined. In that case, isn't this enough?

Suggested change

if (typeof fallbackResult === 'object' && fallbackResult !== null) {

if (fallbackResult) {

Updated here 9f57ca3

sorenlouv · 2022-03-09T16:39:38Z

x-pack/plugins/apm/server/routes/correlations/queries/query_significant_correlations.ts

+
+    if (typeof fallbackResult === 'object' && fallbackResult !== null) {
+      fallbackResult = {
+        ...(fallbackResult as LatencyCorrelation),


This shouldn't be necessary. The if clause should act as a typeguard and narrow the type from optional to LatencyCorrelation

Suggested change

...(fallbackResult as LatencyCorrelation),

...fallbackResult,

Updated here 9f57ca3

sorenlouv · 2022-03-09T16:43:07Z

x-pack/plugins/apm/server/routes/correlations/queries/query_significant_correlations.ts

+  fulfilled.forEach((d: LatencyCorrelation | undefined) => {
+    if (d === undefined) return;
+    if (Array.isArray(d.histogram)) {
+      latencyCorrelations.push(d);
+    } else {
+      if (!fallbackResult) {
+        fallbackResult = d;
+      } else {
+        if (
+          d.ksTest > fallbackResult.ksTest &&
+          d.correlation > fallbackResult.correlation
+        ) {
+          fallbackResult = d;
+        }
+      }
+    }
+  });


Any possibility of rewriting this to using filter and map? Immutable code is often easier to reason about and less error prone.

Updated here 9f57ca3

sorenlouv · 2022-03-09T16:45:48Z

x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts

+      setResponse({
+        ...responseUpdate,
+        fallbackResult,
+        loaded: LOADED_DONE,
+        isRunning: false,
+      });


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?

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.

sorenlouv

Just a few comments. Overall lgtm

…umns-with-fallback-results

…://github.com/qn895/kibana into apm-consolidate-columns-with-fallback-results

…umns-with-fallback-results

kibana-ci · 2022-03-15T21:55:56Z

💔 Build Failed

Failed CI Steps

Checks

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	2.7MB	2.7MB	+1.6KB

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`apm`	15	14	-1
`uptime`	7	6	-1
total			-2

ESLint disabled line counts

id	before	after	diff
`apm`	88	85	-3
`uptime`	48	42	-6
total			-9

Total ESLint disabled count

id	before	after	diff
`apm`	103	99	-4
`uptime`	55	48	-7
total			-11

History

💛 Build #28060 was flaky 1a8eb64
💚 Build #27249 succeeded dd2b011
💔 Build #27117 failed afd96a9

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @qn895

peteharverson

Tested latest changes and LGTM

…umns-with-fallback-results

qn895 · 2022-03-18T12:12:25Z

@elasticmachine merge upstream

qn895 · 2022-03-21T15:09:35Z

@elasticmachine merge upstream

kibana-ci · 2022-03-21T16:43:03Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	2.9MB	2.9MB	+1.6KB

History

💛 Build #31766 was flaky 7618d4b
💚 Build #31441 succeeded e632407
💔 Build #31405 failed d7e62bb
💔 Build #30745 failed abb1e2a
💛 Build #28060 was flaky 1a8eb64
💚 Build #27249 succeeded dd2b011

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @qn895

kibanamachine · 2022-03-23T16:46:19Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126683 or prevent reminders by adding the backport:skip label.

qn895 added 6 commits March 2, 2022 09:11

Remove score column for failed transactions

575f816

Show at least one correlated value

0743eb4

Add very low badge for fallback transactions

dacd1be

Match two columns

c7266d0

Update constants & i18n

acef4cf

Update failed transactions

afd96a9

qn895 self-assigned this Mar 2, 2022

qn895 added :ml apm:ml Integration between APM and ML release_note:enhancement v8.2.0 labels Mar 2, 2022

Fix types

dd2b011

peteharverson reviewed Mar 3, 2022

View reviewed changes

qn895 requested a review from walterra March 3, 2022 16:04

qn895 marked this pull request as ready for review March 3, 2022 16:04

qn895 requested a review from a team as a code owner March 3, 2022 16:04

qn895 requested a review from lcawl March 3, 2022 16:42

botelastic bot added the Team:APM All issues that need APM UI Team support label Mar 3, 2022

qn895 mentioned this pull request Mar 7, 2022

[ML] Show at least one correlated value for APM correlations #123933

Closed

9 tasks

Merge branch 'main' into apm-consolidate-columns-with-fallback-results

1a8eb64

sorenlouv reviewed Mar 9, 2022

View reviewed changes

sorenlouv approved these changes Mar 9, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/main' into apm-consolidate-col…

0ff6a84

…umns-with-fallback-results

qn895 added 4 commits March 15, 2022 10:58

Merge branch 'apm-consolidate-columns-with-fallback-results' of https…

7a81024

…://github.com/qn895/kibana into apm-consolidate-columns-with-fallback-results

Change to immutable

9f57ca3

Merge remote-tracking branch 'upstream/main' into apm-consolidate-col…

bfe6cfb

…umns-with-fallback-results

Add tooltip

abb1e2a

peteharverson approved these changes Mar 16, 2022

View reviewed changes

qn895 added 3 commits March 17, 2022 08:37

[ML] Fix types

eb78525

Merge remote-tracking branch 'upstream/main' into apm-consolidate-col…

d7e62bb

…umns-with-fallback-results

[ML] Fix translation

e632407

Merge branch 'main' into apm-consolidate-columns-with-fallback-results

7618d4b

Merge branch 'main' into apm-consolidate-columns-with-fallback-results

a32e958

qn895 enabled auto-merge (squash) March 21, 2022 15:09

qn895 merged commit 3a4c6de into elastic:main Mar 21, 2022

peteharverson mentioned this pull request Mar 23, 2022

[APM] Latency correlations: Always display at least one correlated term #119613

Closed

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 23, 2022

peteharverson added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Mar 23, 2022

peteharverson mentioned this pull request Apr 12, 2022

[APM] Correlations: Consolidate correlation, score and impact columns #119617

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

qn895 commented Mar 2, 2022 •

edited

Loading

peteharverson Mar 3, 2022

lcawl Mar 3, 2022 •

edited

Loading

qn895 Mar 15, 2022

peteharverson left a comment

elasticmachine commented Mar 3, 2022

elasticmachine commented Mar 3, 2022

qn895 commented Mar 7, 2022

sorenlouv Mar 9, 2022

qn895 Mar 15, 2022

sorenlouv Mar 9, 2022 •

edited

Loading

qn895 Mar 15, 2022

sorenlouv Mar 9, 2022 •

edited

Loading

qn895 Mar 15, 2022

sorenlouv Mar 9, 2022 •

edited

Loading

qn895 Mar 15, 2022

sorenlouv left a comment

kibana-ci commented Mar 15, 2022 •

edited

Loading

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

peteharverson left a comment

qn895 commented Mar 18, 2022

qn895 commented Mar 21, 2022

kibana-ci commented Mar 21, 2022

kibanamachine commented Mar 23, 2022

	if (typeof fallbackResult === 'object' && fallbackResult !== null) {
	if (fallbackResult) {

	...(fallbackResult as LatencyCorrelation),
	...fallbackResult,

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

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

Conversation

qn895 commented Mar 2, 2022 • edited Loading

Summary

Risk Matrix

For maintainers

peteharverson Mar 3, 2022

Choose a reason for hiding this comment

lcawl Mar 3, 2022 • edited Loading

Choose a reason for hiding this comment

qn895 Mar 15, 2022

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

elasticmachine commented Mar 3, 2022

elasticmachine commented Mar 3, 2022

qn895 commented Mar 7, 2022

sorenlouv Mar 9, 2022

Choose a reason for hiding this comment

qn895 Mar 15, 2022

Choose a reason for hiding this comment

sorenlouv Mar 9, 2022 • edited Loading

Choose a reason for hiding this comment

qn895 Mar 15, 2022

Choose a reason for hiding this comment

sorenlouv Mar 9, 2022 • edited Loading

Choose a reason for hiding this comment

qn895 Mar 15, 2022

Choose a reason for hiding this comment

sorenlouv Mar 9, 2022 • edited Loading

Choose a reason for hiding this comment

qn895 Mar 15, 2022

Choose a reason for hiding this comment

sorenlouv left a comment

Choose a reason for hiding this comment

kibana-ci commented Mar 15, 2022 • edited Loading

💔 Build Failed

Failed CI Steps

Metrics [docs]

Async chunks

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

peteharverson left a comment

Choose a reason for hiding this comment

qn895 commented Mar 18, 2022

qn895 commented Mar 21, 2022

kibana-ci commented Mar 21, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

History

kibanamachine commented Mar 23, 2022

qn895 commented Mar 2, 2022 •

edited

Loading

lcawl Mar 3, 2022 •

edited

Loading

sorenlouv Mar 9, 2022 •

edited

Loading

sorenlouv Mar 9, 2022 •

edited

Loading

sorenlouv Mar 9, 2022 •

edited

Loading

kibana-ci commented Mar 15, 2022 •

edited

Loading