[APM] Handle other values popup when correlated value is not in top 10 #118069

qn895 · 2021-11-09T18:19:07Z

Summary

This PR shows the percentage/stats for the the selected item in addition to the top 10 values. If the value is not in the top 10 values, an additional query is made to calculate the percentage for the field name/field value.

It also removes the percentiles agg in the numeric fields because we don't need it.

Screen.Recording.2021-12-06.at.09.46.04.mov

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

elasticmachine · 2021-11-10T00:50:25Z

Pinging @elastic/ml-ui (:ml)

elasticmachine · 2021-11-10T16:38:14Z

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

smith · 2021-11-10T21:36:14Z

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

+      )}
+
+      {topValueStats.topValuesSampleSize !== undefined && (
+        <Fragment>


You can just use <>. You only need to import Fragment if you'll need to give it a key.

Updated here 58d8bcc

smith · 2021-11-10T21:37:02Z

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

+          <EuiHorizontalRule margin="s" />
+          <EuiSpacer size="s" />
+          <EuiText size="xs">
+            <FormattedMessage


Most places in APM we just use i18n.translate directly instead of using the FormattedMessage component.

Updated here 58d8bcc

walterra · 2021-11-11T08:57:23Z

Noticed the following:

I'm not sure if the results here are correct. The field/value shows up in the table, that's lets me assume there must be some data, there's also a corresponding histogram in the chart, see the tooltip. Nonetheless, the query for the tooltip doesn't find any results. To reproduce, this happens with the antifraud data set, servicename frontend, transaction type createOrder.

walterra

Just added a few small suggestions for the code. It would be really great if we could already add an API integration test for the additional API endpoint as part of this PR.

walterra · 2021-11-11T09:05:59Z

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

@@ -184,6 +184,7 @@ export default function ApiTest({ getService }: FtrProviderContext) {
          body: {
            ...getOptions(),
            fieldsToSample: [...fieldsToSample],
+            samplerShardSize: 5000,


This uses a different size than the runtime code. Should we change this to 10000 and also use SAMPLER_SHARD_SIZE from x-pack/plugins/apm/common/correlations/constants.ts?

walterra · 2021-11-11T09:06:05Z

x-pack/test/apm_api_integration/tests/correlations/latency.spec.ts

@@ -195,6 +195,7 @@ export default function ApiTest({ getService }: FtrProviderContext) {
            body: {
              ...getOptions(),
              fieldsToSample: [...fieldsToSample],
+              samplerShardSize: 5000,


This uses a different size than the runtime code. Should we change this to 10000 and also use SAMPLER_SHARD_SIZE from x-pack/plugins/apm/common/correlations/constants.ts?

walterra · 2021-11-11T09:06:35Z

x-pack/plugins/apm/server/lib/correlations/queries/field_stats/get_field_stats.test.ts

@@ -20,7 +20,7 @@ const params = {
  includeFrozen: false,
  environment: ENVIRONMENT_ALL.value,
  kuery: '',
-  samplerShardSize: 5000,
+  samplerShardSize: 10000,


Can we use SAMPLER_SHARD_SIZE from x-pack/plugins/apm/common/correlations/constants.ts?

walterra · 2021-11-11T09:06:45Z

x-pack/plugins/apm/server/lib/correlations/queries/field_stats/get_field_stats.test.ts

@@ -76,7 +69,7 @@ describe('field_stats', () => {
              },
            },
          },
-          sampler: { shard_size: 5000 },
+          sampler: { shard_size: 10000 },


Can we use SAMPLER_SHARD_SIZE from x-pack/plugins/apm/common/correlations/constants.ts?

walterra · 2021-11-11T09:06:53Z

x-pack/plugins/apm/server/lib/correlations/queries/field_stats/get_field_stats.test.ts

@@ -88,7 +81,7 @@ describe('field_stats', () => {

      const expectedAggs = {
        sample: {
-          sampler: { shard_size: 5000 },
+          sampler: { shard_size: 10000 },


Can we use SAMPLER_SHARD_SIZE from x-pack/plugins/apm/common/correlations/constants.ts?

walterra · 2021-11-11T09:06:58Z

x-pack/plugins/apm/server/lib/correlations/queries/field_stats/get_field_stats.test.ts

@@ -105,7 +98,7 @@ describe('field_stats', () => {

      const expectedAggs = {
        sample: {
-          sampler: { shard_size: 5000 },
+          sampler: { shard_size: 10000 },


Can we use SAMPLER_SHARD_SIZE from x-pack/plugins/apm/common/correlations/constants.ts?

formgeist · 2021-11-11T11:33:41Z

@qn895 This might be a later enhancement, but I wonder if the popover should show a loading spinner if the results are still waiting to be completed. Right now, the top 10 list is loaded, but the selected term might come in after because it's outside the top 10. Here's an example of that behavior;

formgeist

LGTM 👍 Added a small comment about possibly adding a loading state to the popover to indicate when it's still fetching results once the top 10 is loaded. And a smaller suggestion on changing the text color of the "outside top 10" message.

formgeist · 2021-11-11T11:43:53Z

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

+      {Array.isArray(fieldValueStats?.topValues) && (
+        <>
+          <EuiHorizontalRule margin="s" />
+          <EuiText size="xs">


Suggested change

<EuiText size="xs">

<EuiText size="xs" color="subdued">

Lowering the contrast of this message since it's just a help text label for the result.

Or looking at the EUI 41.0.0 upgrade PR, should color="text" be used rather than subdued?

text is darker contrast than subdued - what's the EUI upgrade change you're referring to?

Sorry - you're right @formgeist, the EUI 41.0.0 upgrade only changes it to text for EuiButtonIcon.

peteharverson · 2021-11-11T14:28:26Z

As commented by @walterra in #118069 (comment), if the calculations / queries are correct for this sort of situation, we should adjust the text in the popover.

Maybe here the top label should say Top value, and the bottom one could say something like Selected term not found in sample of 20000 documents:

peteharverson · 2021-11-11T14:29:22Z

Is the rounding causing this? Is it really 99.7% for 200?

formgeist · 2021-11-11T14:47:45Z

As commented by @walterra in #118069 (comment), if the calculations / queries are correct for this sort of situation, we should adjust the text in the popover.

Maybe here the top label should say Top value, and the bottom one could say something like Selected term not found in sample of 20000 documents:

I thought that was described by the "Calculated from sample of x documents", so we don't have to also describe it, but perhaps we really need to repeat it just to make sure it's understood.

walterra

Latest changes LGTM

qn895 · 2021-12-07T15:00:41Z

@elasticmachine merge upstream

qn895 · 2021-12-07T19:21:55Z

@elasticmachine merge upstream

kibana-ci · 2021-12-07T20:35:06Z

💚 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.8MB	2.8MB	+1.3KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`apm`	46	47	+1

History

💔 Build #11796 failed 153914f
💚 Build #11535 succeeded 0da5cf6
💚 Build #11180 succeeded e5e0650
💔 Build #11166 failed 41ccdf024b1c68107edb0c43c71dce7837486d35
💔 Build #5997 failed 58d8bcc

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

cc @qn895

elastic#118069) * [ML] Add stats for field value not in top 10 * [ML] Fix tests * [ML] Add message * [ML] Reverse label, remove Fragment, switch to i18n * Fix sample shard size import, subdued text * Fix sample shard size import, subdued text * Move routes after refactor * Add loading spinner * Fix sampler shard size Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-12-07T20:38:17Z

💚 Backport successful

Status	Branch	Result
✅	8.0

This backport PR will be merged automatically after passing CI.

#118069) (#120680) * [ML] Add stats for field value not in top 10 * [ML] Fix tests * [ML] Add message * [ML] Reverse label, remove Fragment, switch to i18n * Fix sample shard size import, subdued text * Fix sample shard size import, subdued text * Move routes after refactor * Add loading spinner * Fix sampler shard size Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Quynh Nguyen <[email protected]>

elastic#118069) * [ML] Add stats for field value not in top 10 * [ML] Fix tests * [ML] Add message * [ML] Reverse label, remove Fragment, switch to i18n * Fix sample shard size import, subdued text * Fix sample shard size import, subdued text * Move routes after refactor * Add loading spinner * Fix sampler shard size Co-authored-by: Kibana Machine <[email protected]>

[ML] Add stats for field value not in top 10

3c5c620

qn895 added :ml apm:correlations labels Nov 9, 2021

qn895 requested review from walterra and peteharverson November 9, 2021 18:19

qn895 self-assigned this Nov 9, 2021

[ML] Fix tests

cd5fcfc

qn895 marked this pull request as ready for review November 10, 2021 00:50

qn895 requested a review from a team as a code owner November 10, 2021 00:50

qn895 added release_note:enhancement v8.0.0 v8.1.0 labels Nov 10, 2021

qn895 added 2 commits November 10, 2021 10:36

[ML] Add message

5fdadb4

Merge remote-tracking branch 'upstream/main' into apm-popover-not-top-10

900bdb5

botelastic bot added the Team:APM All issues that need APM UI Team support label Nov 10, 2021

smith approved these changes Nov 10, 2021

View reviewed changes

[ML] Reverse label, remove Fragment, switch to i18n

58d8bcc

qn895 requested a review from formgeist November 10, 2021 23:26

walterra reviewed Nov 11, 2021

View reviewed changes

formgeist approved these changes Nov 11, 2021

View reviewed changes

qn895 added 3 commits November 11, 2021 12:13

Fix sample shard size import, subdued text

4508e51

Fix sample shard size import, subdued text

5cf1072

Merge upstream/main into branch

1482d8b

qn895 added 3 commits November 29, 2021 11:31

Move routes after refactor

75877a5

Merge upstream/main into branch

13ac698

Add loading spinner

45282ef

elastic deleted a comment from kibanamachine Dec 5, 2021

Merge remote-tracking branch 'upstream/main' into apm-popover-not-top-10

e5e0650

qn895 force-pushed the apm-popover-not-top-10 branch from 41ccdf0 to e5e0650 Compare December 6, 2021 05:13

Fix sampler shard size

0da5cf6

walterra approved these changes Dec 7, 2021

View reviewed changes

Merge branch 'main' into apm-popover-not-top-10

153914f

Merge branch 'main' into apm-popover-not-top-10

017651d

qn895 enabled auto-merge (squash) December 7, 2021 19:24

qn895 added the auto-backport Deprecated - use backport:version if exact versions are needed label Dec 7, 2021

qn895 merged commit f5141e4 into elastic:main Dec 7, 2021

kibanamachine mentioned this pull request Dec 7, 2021

[8.0] [APM] Handle other values popup when correlated value is not in top 10 (#118069) #120680

Merged

qn895 deleted the apm-popover-not-top-10 branch December 8, 2021 21:58

peteharverson mentioned this pull request Apr 11, 2023

[ML] Explain Log Rate Spikes: adds popover to analysis table for viewing other field values #154689

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Handle other values popup when correlated value is not in top 10 #118069

[APM] Handle other values popup when correlated value is not in top 10 #118069

qn895 commented Nov 9, 2021 •

edited

Loading

elasticmachine commented Nov 10, 2021

elasticmachine commented Nov 10, 2021

smith Nov 10, 2021

qn895 Nov 10, 2021

smith Nov 10, 2021

qn895 Nov 10, 2021

walterra commented Nov 11, 2021

walterra left a comment

walterra Nov 11, 2021

walterra Nov 11, 2021

walterra Nov 11, 2021

walterra Nov 11, 2021

walterra Nov 11, 2021

walterra Nov 11, 2021

formgeist commented Nov 11, 2021

formgeist left a comment •

edited

Loading

formgeist Nov 11, 2021

peteharverson Nov 11, 2021

formgeist Nov 11, 2021

peteharverson Nov 11, 2021

peteharverson commented Nov 11, 2021

peteharverson commented Nov 11, 2021

formgeist commented Nov 11, 2021

walterra left a comment

qn895 commented Dec 7, 2021

qn895 commented Dec 7, 2021

kibana-ci commented Dec 7, 2021

kibanamachine commented Dec 7, 2021

[APM] Handle other values popup when correlated value is not in top 10 #118069

[APM] Handle other values popup when correlated value is not in top 10 #118069

Conversation

qn895 commented Nov 9, 2021 • edited Loading

Summary

Checklist

elasticmachine commented Nov 10, 2021

elasticmachine commented Nov 10, 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

walterra commented Nov 11, 2021

walterra 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

formgeist commented Nov 11, 2021

formgeist left a comment • edited Loading

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

peteharverson commented Nov 11, 2021

peteharverson commented Nov 11, 2021

formgeist commented Nov 11, 2021

walterra left a comment

Choose a reason for hiding this comment

qn895 commented Dec 7, 2021

qn895 commented Dec 7, 2021

kibana-ci commented Dec 7, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Public APIs missing exports

History

kibanamachine commented Dec 7, 2021

💚 Backport successful

qn895 commented Nov 9, 2021 •

edited

Loading

formgeist left a comment •

edited

Loading