[ML] Fix UI inconsistencies in APM Failed transaction correlations #109187

qn895 · 2021-08-18T23:43:04Z

Summary

This PR addresses the issues from #109061 / #109093 / #106381

Show the same empty state in the correlations table

Fix license prompt text

- [x] Functional tests for the new tab - [x] Make the p value & error rate columns visible only when esInspect mode is enabled

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

formgeist

Overall everything looks great! 👏 Just a few copy additions/changes suggested.

formgeist · 2021-08-19T06:40:39Z

...ins/apm/public/components/app/correlations/failed_transactions_correlations_help_popover.tsx

+      title={i18n.translate(
+        'xpack.apm.correlations.failedTransactions.helpPopover.title',
+        {
+          defaultMessage: 'Failed transactions correlations',


Suggested change

defaultMessage: 'Failed transactions correlations',

defaultMessage: 'Failed transaction correlations',

Updated here 6f9ffa1

formgeist · 2021-08-19T06:46:09Z

...ins/apm/public/components/app/correlations/failed_transactions_correlations_help_popover.tsx

    >
      <p>
        <FormattedMessage
-          id="xpack.apm.correlations.failurePopoverBasicExplanation"
+          id="xpack.apm.correlations.failedTransactions.helpPopover.basicExplanation"
          defaultMessage="Correlations help you discover which attributes are contributing to failed transactions."


Suggested change

defaultMessage="Correlations help you discover which attributes are contributing to failed transactions."

defaultMessage="Correlations help you discover which attributes are contributing to failed transactions. Transactions are considered a failure when it returns a status code >= 5xx."

Just added the description of failed transactions because I figured it might be helpful in this context.

Updated here 6f9ffa1

walterra · 2021-08-19T11:20:05Z

I noticed when trying to sort the Impact-column low/medium/high gets sorted alphabetically. Is there a way we can sort it by the underlying impact threshold?

For example, here I'd expect it to sort by descending impact (high/medium/low) instead of descending alphabetically:

walterra · 2021-08-19T11:26:57Z

x-pack/plugins/apm/public/components/app/transaction_details/distribution/index.tsx

@@ -132,7 +132,7 @@ export function TransactionDistribution({

  return (
    <div data-test-subj="apmTransactionDistributionTabContent">
-      <EuiFlexGroup>
+      <EuiFlexGroup style={{ minHeight: 56 }}>


Can you explain why this is necessary? Could we use an EUI px var instead?

Added a comment for it here. The small size selection text is causing the width of that title box to be smaller than the other two (which have the help button), so when you navigate between the tabs there's a bit of a "flickering" or shifting of the title/header spacer.

walterra · 2021-08-19T11:28:47Z

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

@@ -0,0 +1,38 @@
+/*


Nit: Suggest to name this file correlations_log.tsx for consistency.

Renamed here 6f9ffa1

…tion

qn895 · 2021-08-19T16:49:58Z

I noticed when trying to sort the Impact-column low/medium/high gets sorted alphabetically. Is there a way we can sort it by the underlying impact threshold?

Thanks for caching that. Updated here 84a0514

Latest changes also include:

Remove the event.outcome as a field candidate (because event.outcome: failure would always be significant in this case)
Shrink the column width for impact (previously it was at 116px)
Added badge for High, medium, low [APM] Correlations: Show impact levels (high, medium, low) as colored badges indicating their severity #109093

- Fix issue with filtering not handling space or other special characters correctly in both Latency & failed transaction correlations

…ctions-ux

…use orderBy

…ctions-ux

sorenlouv · 2021-08-22T22:44:55Z

x-pack/test/apm_api_integration/tests/correlations/failed_transactions.ts

+    };
+  };
+
+  registry.when('quynh on trial license without data', { config: 'trial', archives: [] }, () => {


Just testing stuff out with your own name I assume :D

Suggested change

registry.when('quynh on trial license without data', { config: 'trial', archives: [] }, () => {

registry.when('on trial license without data', { config: 'trial', archives: [] }, () => {

Fixed in 909e203 😅

walterra

Latest changes LGTM

sorenlouv · 2021-08-23T09:25:27Z

x-pack/test/apm_api_integration/tests/correlations/failed_transactions.ts

+          const response = await supertest
+            .post(`/internal/bsearch`)
+            .set('kbn-xsrf', 'foo')
+            .send(reqBody);
+
+          followUpResponse = parseBfetchResponse(response)[0];


It looks like you always want to parse the responses made to the /bsearch endpoint. In that case, what about creating a bfetchClient which takes the request body and returns the (parsed) response?

Suggested change

const response = await supertest

.post(`/internal/bsearch`)

.set('kbn-xsrf', 'foo')

.send(reqBody);

followUpResponse = parseBfetchResponse(response)[0];

const {status, response} = await bfetchClient(supertest, reqBody);

You could also make bFetchClient a service similar to supertestAsApmWriteUser and then you can call it without specifying supertest:

Suggested change

const response = await supertest

.post(`/internal/bsearch`)

.set('kbn-xsrf', 'foo')

.send(reqBody);

followUpResponse = parseBfetchResponse(response)[0];

const bfetchClient = context.getService('bfetchClient');

const {status, response} = await bfetchClient(reqBody);

Moved this to an item on the 7.16 meta issue: #109220

kibanamachine · 2021-08-23T11:28:15Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1611	1615	+4

Async chunks

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

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

History

💚 Build #147587 succeeded e4c0d3a
💔 Build #147535 failed e683e57
💔 Build #147520 failed 6403616
💔 Build #147181 failed 85caf98
💚 Build #147133 succeeded 1497d3d

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

cc @qn895

…lastic#109187) - Show the same empty state in the correlations table - Add "Correlations" title above the table - Add EuiSpacer between the sections before and after progress - Update the copy within the beta badge title=Failed transaction correlations description=Failed transaction correlations is not GA... - Remove s size from the beta badge - Move the help popover to the top of the panel (similar to the Latency correlations tab) - Move the Cancel/Refresh option to the right of the progress bar (similar to the Latency correlations tab) - When the correlation job is running the correlations tab should show a loading state similar to the latency correlations table - Indicate in the table headers Score is sorted by default - Add sortability to both Latency and failed transactions correlations table - Refactor to prevent duplicate code/components like Log, progress bar - Fix alignments of the tab content header (previously navigating from one Trace samples tab to Latency correlation tabs will cause a minor jump in the header, or the titles within the same tab were not the same size ) - Remove the event.outcome as a field candidate (because event.outcome: failure would always be significant in this case) - Shrink the column width for impact (previously it was at 116px) - Added badge for High, medium, low [APM] Correlations: Show impact levels (high, medium, low) as colored badges indicating their severity - Fix license prompt text - Functional tests for the new tab - Make the p value & error rate columns visible only when esInspect mode is enabled

kibanamachine · 2021-08-23T11:31:08Z

💚 Backport successful

Status	Branch	Result
✅	7.15
✅	7.x

The backport PRs will be merged automatically after passing CI.

…109187) (#109625) - Show the same empty state in the correlations table - Add "Correlations" title above the table - Add EuiSpacer between the sections before and after progress - Update the copy within the beta badge title=Failed transaction correlations description=Failed transaction correlations is not GA... - Remove s size from the beta badge - Move the help popover to the top of the panel (similar to the Latency correlations tab) - Move the Cancel/Refresh option to the right of the progress bar (similar to the Latency correlations tab) - When the correlation job is running the correlations tab should show a loading state similar to the latency correlations table - Indicate in the table headers Score is sorted by default - Add sortability to both Latency and failed transactions correlations table - Refactor to prevent duplicate code/components like Log, progress bar - Fix alignments of the tab content header (previously navigating from one Trace samples tab to Latency correlation tabs will cause a minor jump in the header, or the titles within the same tab were not the same size ) - Remove the event.outcome as a field candidate (because event.outcome: failure would always be significant in this case) - Shrink the column width for impact (previously it was at 116px) - Added badge for High, medium, low [APM] Correlations: Show impact levels (high, medium, low) as colored badges indicating their severity - Fix license prompt text - Functional tests for the new tab - Make the p value & error rate columns visible only when esInspect mode is enabled Co-authored-by: Quynh Nguyen <43350163+qn895@users.noreply.github.com>

…109187) (#109626) - Show the same empty state in the correlations table - Add "Correlations" title above the table - Add EuiSpacer between the sections before and after progress - Update the copy within the beta badge title=Failed transaction correlations description=Failed transaction correlations is not GA... - Remove s size from the beta badge - Move the help popover to the top of the panel (similar to the Latency correlations tab) - Move the Cancel/Refresh option to the right of the progress bar (similar to the Latency correlations tab) - When the correlation job is running the correlations tab should show a loading state similar to the latency correlations table - Indicate in the table headers Score is sorted by default - Add sortability to both Latency and failed transactions correlations table - Refactor to prevent duplicate code/components like Log, progress bar - Fix alignments of the tab content header (previously navigating from one Trace samples tab to Latency correlation tabs will cause a minor jump in the header, or the titles within the same tab were not the same size ) - Remove the event.outcome as a field candidate (because event.outcome: failure would always be significant in this case) - Shrink the column width for impact (previously it was at 116px) - Added badge for High, medium, low [APM] Correlations: Show impact levels (high, medium, low) as colored badges indicating their severity - Fix license prompt text - Functional tests for the new tab - Make the p value & error rate columns visible only when esInspect mode is enabled Co-authored-by: Quynh Nguyen <43350163+qn895@users.noreply.github.com>

qn895 added 2 commits August 18, 2021 18:40

[ML] Fix UI inconsistencies, add sorting

8dade28

[ML] Fix columns for latency sortable

eb5ba19

qn895 requested review from walterra and formgeist August 19, 2021 02:47

qn895 self-assigned this Aug 19, 2021

qn895 added :ml apm:correlations release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v8.0.0 labels Aug 19, 2021

qn895 changed the title ~~[ML] Fix UI in APM Failed transaction correlations~~ [ML] Fix UI inconsistencies in APM Failed transaction correlations Aug 19, 2021

qn895 added the v7.16.0 label Aug 19, 2021

[ML] Fix columns for latency sortable

420e183

qn895 force-pushed the ml-fix-apm-transactions-ux branch from a145372 to 420e183 Compare August 19, 2021 03:15

formgeist approved these changes Aug 19, 2021

View reviewed changes

walterra reviewed Aug 19, 2021

View reviewed changes

qn895 added 6 commits August 19, 2021 10:20

[ML] Rename log -> correlations_log.tsx, update help texts

6f9ffa1

[ML] Remove event.outcome as candidate for failed transaction correla…

bdd7143

…tion

[ML] Shrink the score column

4a5be20

[ML] Add colored badge for high, medium, low thresholds

cf9284c

[ML] Comment on hard coded height

be26575

[ML] Sort by normalized

84a0514

qn895 added 3 commits August 19, 2021 12:03

[ML] Fix latency encoding

3e34c5a

[ML] Add functional tests

1d0a6f5

[ML] Update jest tests

8300462

qn895 force-pushed the ml-fix-apm-transactions-ux branch from 860880d to 8300462 Compare August 19, 2021 19:39

[ML] Fix license text, test subj

1497d3d

qn895 added 7 commits August 20, 2021 09:51

Merge remote-tracking branch 'upstream/master' into ml-fix-apm-transa…

8e52377

…ctions-ux

[ML] Update correlations_log, add more tests for asAbsoluteDateTime, …

82d5457

…use orderBy

[ML] Only show the p-value hover on inspect mode

4ab3871

[ML] Add new error rate columns

0b66643

Merge remote-tracking branch 'upstream/master' into ml-fix-apm-transa…

5999243

…ctions-ux

[ML] Add new error rate columns

6403616

[ML] Add api integration test for failed transaction ss

e683e57

sorenlouv approved these changes Aug 20, 2021

View reviewed changes

qn895 added 2 commits August 22, 2021 16:23

[ML] Fix api tests

7d52366

Merge remote-tracking branch 'upstream/master' into ml-fix-apm-transa…

e4c0d3a

…ctions-ux

sorenlouv reviewed Aug 22, 2021

View reviewed changes

fix api test names

909e203

walterra approved these changes Aug 23, 2021

View reviewed changes

walterra added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 23, 2021

walterra enabled auto-merge (squash) August 23, 2021 09:23

sorenlouv reviewed Aug 23, 2021

View reviewed changes

walterra merged commit 63ef9d1 into elastic:master Aug 23, 2021

kibanamachine mentioned this pull request Aug 23, 2021

[7.15] [ML] Fix UI inconsistencies in APM Failed transaction correlations (#109187) #109625

Merged

kibanamachine mentioned this pull request Aug 23, 2021

[7.x] [ML] Fix UI inconsistencies in APM Failed transaction correlations (#109187) #109626

Merged

walterra mentioned this pull request Aug 23, 2021

[APM] Failed transaction correlations UX polish #109061

Closed

9 tasks

walterra mentioned this pull request Aug 24, 2021

[ML] Increase Test Coverage 7.15 #106113

Closed

14 tasks

formgeist linked an issue Aug 25, 2021 that may be closed by this pull request

[APM] Correlations: Show impact levels (high, medium, low) as colored badges indicating their severity #109093

Closed

formgeist mentioned this pull request Aug 25, 2021

[APM] Correlations: Show impact levels (high, medium, low) as colored badges indicating their severity #109093

Closed

qn895 deleted the ml-fix-apm-transactions-ux branch September 21, 2021 14:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Fix UI inconsistencies in APM Failed transaction correlations #109187

[ML] Fix UI inconsistencies in APM Failed transaction correlations #109187

qn895 commented Aug 18, 2021 •

edited

Loading

formgeist left a comment

formgeist Aug 19, 2021

qn895 Aug 19, 2021

formgeist Aug 19, 2021

qn895 Aug 19, 2021

walterra commented Aug 19, 2021

walterra Aug 19, 2021

qn895 Aug 19, 2021

walterra Aug 19, 2021

qn895 Aug 19, 2021

qn895 commented Aug 19, 2021 •

edited

Loading

sorenlouv Aug 22, 2021

walterra Aug 23, 2021

walterra left a comment

sorenlouv Aug 23, 2021 •

edited

Loading

walterra Aug 23, 2021

kibanamachine commented Aug 23, 2021

kibanamachine commented Aug 23, 2021

	defaultMessage: 'Failed transactions correlations',
	defaultMessage: 'Failed transaction correlations',

	defaultMessage="Correlations help you discover which attributes are contributing to failed transactions."
	defaultMessage="Correlations help you discover which attributes are contributing to failed transactions. Transactions are considered a failure when it returns a status code >= 5xx."

	registry.when('quynh on trial license without data', { config: 'trial', archives: [] }, () => {
	registry.when('on trial license without data', { config: 'trial', archives: [] }, () => {

[ML] Fix UI inconsistencies in APM Failed transaction correlations #109187

[ML] Fix UI inconsistencies in APM Failed transaction correlations #109187

Conversation

qn895 commented Aug 18, 2021 • edited Loading

Summary

Checklist

formgeist left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra commented Aug 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qn895 commented Aug 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

sorenlouv Aug 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Aug 23, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

kibanamachine commented Aug 23, 2021

💚 Backport successful

qn895 commented Aug 18, 2021 •

edited

Loading

qn895 commented Aug 19, 2021 •

edited

Loading

sorenlouv Aug 23, 2021 •

edited

Loading