[ML] APM Correlations: Chart for failed transactions correlations tab. #110172

walterra · 2021-08-26T06:22:33Z

Summary

Latency Distribution chart for Failed Transactions Correlations Tab

Updates the distribution chart component to be able to retrieve data for an array of area instead of the previously hard coded 2 area.
Updates server side query functions to accept an array of term filters instead of just a single field/value pair.
The colors for the areas are definitely up for discussion (we can adapt them in a follow up after a feedback round).

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios
This was checked for cross-browser compatibility
This was checked for breaking API changes and was labeled appropriately

This reverts commit ca86f76.

qn895 · 2021-09-09T14:03:04Z

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

      params: {
        environment: 'ENVIRONMENT_ALL',
        start: '2020',
        end: '2021',
        kuery: '',
+        percentileThreshold: 95,


nit: can use DEFAULT_PERCENTILE_THRESHOLD for this line

qn895 · 2021-09-09T14:21:38Z

Just noticed the column sizes look off when es inspect mode is off. Would be good to have the Field name column have more space here.

The colors are also bit hard to distinguish for me, especially when some of areas overlap.

Screen.Recording.2021-09-09.at.09.32.48.mov

peteharverson · 2021-09-09T15:01:38Z

The score column is getting too much width for me, with or without the es inspect on. As @qn895 suggested, would be good to give the field name / value columns more width.

peteharverson · 2021-09-09T15:09:02Z

Is it worth adding the (subsequently removed for 7.15) lines about the chart from 2d14d2a to the help popover?

(added this to the 7.16 meta issue #109220)

peteharverson

Tested and overall looks good. Would be good to reduce the width of the score column, and identify better colours to use in the chart, but that polish can be added in a follow-up.

sorenlouv · 2021-09-09T17:11:30Z

x-pack/plugins/apm/public/components/shared/charts/transaction_distribution_chart/index.tsx

-        histogram[i + 1].doc_count = CHART_PLACEHOLDER_VALUE;
-      }
+
+  for (let i = 0; i < histogram.length - 1; i++) {


Can we rewrite this from a for. Preferably replace it with map and/or filter

Refactored this to use reduce in 0a56509.

sorenlouv · 2021-09-09T17:16:48Z

x-pack/plugins/apm/server/lib/search_strategies/queries/query_failure_correlation.ts

+  for (let i = 0; i < result.length; i++) {
+    const current = result[i];
+
+    const histogram = await fetchTransactionDurationRanges(
+      esClient,
+      params,
+      histogramRangeSteps,
+      [
+        { fieldName: EVENT_OUTCOME, fieldValue: EventOutcome.failure },
+        { fieldName: current.fieldName, fieldValue: current.fieldValue },
+      ]
+    );
+
+    current.histogram = histogram;
+  }


Are you using a for loop to ensure the promises are resolved in sequence? And what does current.histogram do?

This was a bit of a leftover from experimentation. I refactored it to get rid of the current and use for of in
95f4a49. It's sequential by intention because we don't know how many calls there could be to avoid hammering ES.

sorenlouv · 2021-09-09T17:22:23Z

x-pack/plugins/apm/server/lib/search_strategies/queries/query_histograms_generator.ts

@@ -69,8 +71,7 @@ export async function* fetchTransactionDurationHistograms(
          esClient,
          params,
          histogramRangeSteps,
-          item.fieldName,
-          item.fieldValue
+          [item]


If you destructure the arguments this function become much easier to read. Right now it's pretty opaque what [item] is.

sorenlouv · 2021-09-09T17:25:03Z

x-pack/plugins/apm/server/lib/search_strategies/queries/query_percentiles.ts

@@ -23,10 +23,9 @@ import { SIGNIFICANT_VALUE_DIGITS } from '../constants';
 export const getTransactionDurationPercentilesRequest = (
  params: SearchStrategyParams,
  percents?: number[],
-  fieldName?: FieldValuePair['fieldName'],
-  fieldValue?: FieldValuePair['fieldValue']
+  termFilters?: FieldValuePair[]


Same here. Using destructuring will make it easier to read

To do this consistently for all queries would require to refactor more files than the ones already touched in this PR, unrelated to the feature. I added an item to the meta issue to pick it up in a follow up #109220

Doing in a follow-up sgtm. Thanks

walterra · 2021-09-09T22:29:53Z

@peteharverson @qn895 Fixed the width of the Score column in 869cb88

peteharverson

Reduced width for score column and latest changes LGTM.

I added an item for updating the failed transaction tab help popover with info on the chart to #109220

sorenlouv · 2021-09-13T13:50:02Z

x-pack/plugins/apm/public/components/shared/charts/transaction_distribution_chart/index.tsx

@@ -137,15 +131,15 @@ export function TransactionDistributionChart({

  // This will create y axis ticks for 1, 10, 100, 1000 ...
  const yMax =
-    Math.max(...(overallHistogram ?? []).map((d) => d.doc_count)) ?? 0;
+    Math.max(
+      ...flatten(data.map((d) => d.histogram)).map((d) => d.doc_count)


nit: Can you use flatMap here?

Suggested change

...flatten(data.map((d) => d.histogram)).map((d) => d.doc_count)

...data.flatMap((d) => d.histogram).map((d) => d.doc_count)

peteharverson · 2021-09-13T13:52:23Z

@elasticmachine merge upstream

kibanamachine · 2021-09-13T16:06:54Z

💚 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`	4.4MB	4.4MB	+1.6KB

History

💚 Build #152407 succeeded 869cb88
💚 Build #152227 succeeded b053e55
💔 Build #152193 failed 9ef0fed
💔 Build #152185 failed 31d6825
💚 Build #152084 succeeded f0a300e

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

cc @walterra

elastic#110172) * [ML] Fix tooltip text. * Revert "[ML] Fix tooltip text." This reverts commit ca86f76. * [ML] Chart prototype. * [ML] Hover support for failed transactions chart. * [ML] Add p-value column. * [ML] Code consolidation. * [ML] Fix naming inconsistencies. * [ML] Fix naming inconsistencies. * [ML] Fix naming inconsistencies. * [ML] Consolidate hooks. * [ML] Consolidate hooks. * [ML] Consolidate hooks. * [ML] Use function overloads. * [ML] Fix naming inconsistencies. * [ML] Fix jest test. * [ML] Fix chart loading behavior. * [ML] Rename values to latencyCorrelations. * [ML] Clean up types. * [ML] Add function overloads. * [Ml] Update API integration tests. * [ML] Rename values to failedTransactionsCorrelations. * [ML] Fix naming inconsistencies. * [ML] Fix naming inconsistencies. * [ML] Fix naming inconsistencies. * [ML] Fix jest test. * [ML] Fix API integration test * [ML] Clean up chart data. * [ML] Fix chart props. * [ML] Tweak types for failed correlations. * [ML] Improve FieldValuePair type usage. * [ML] Remove 'async' from variable names. * [ML] Fix typo. * [ML] Simplify mock. * [ML] Refactor code that used type guards. * [ML] Comment about feature availability. * [ML] Simplify check. * [ML] Simplify selectedHistogram. * [ML] Improve column type safety. * [ML] Simplify selectedTerm. * [ML] Simplify sorting. * [ML] Fix regresssion when there's no data for failed transactions. * [ML] Rename fieldFilter to termFilters. * [ML] Update api integration test assertions. * [ML] Fix failed transactions params. * [ML] Tweak chart title. * [ML] Tweak chart colors. * [ML] Add translation. * [ML] Tweak selectedTerm if statement. * [ML] Fix types. * [ML] Fix assertion text. * [ML] Refactor replaceHistogramDotsWithBars. * [ML] Refactor fetchFailedTransactionsCorrelationPValues. * [ML] Fix score column width. Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-09-13T16:11:43Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

#110172) (#111983) * [ML] Fix tooltip text. * Revert "[ML] Fix tooltip text." This reverts commit ca86f76. * [ML] Chart prototype. * [ML] Hover support for failed transactions chart. * [ML] Add p-value column. * [ML] Code consolidation. * [ML] Fix naming inconsistencies. * [ML] Fix naming inconsistencies. * [ML] Fix naming inconsistencies. * [ML] Consolidate hooks. * [ML] Consolidate hooks. * [ML] Consolidate hooks. * [ML] Use function overloads. * [ML] Fix naming inconsistencies. * [ML] Fix jest test. * [ML] Fix chart loading behavior. * [ML] Rename values to latencyCorrelations. * [ML] Clean up types. * [ML] Add function overloads. * [Ml] Update API integration tests. * [ML] Rename values to failedTransactionsCorrelations. * [ML] Fix naming inconsistencies. * [ML] Fix naming inconsistencies. * [ML] Fix naming inconsistencies. * [ML] Fix jest test. * [ML] Fix API integration test * [ML] Clean up chart data. * [ML] Fix chart props. * [ML] Tweak types for failed correlations. * [ML] Improve FieldValuePair type usage. * [ML] Remove 'async' from variable names. * [ML] Fix typo. * [ML] Simplify mock. * [ML] Refactor code that used type guards. * [ML] Comment about feature availability. * [ML] Simplify check. * [ML] Simplify selectedHistogram. * [ML] Improve column type safety. * [ML] Simplify selectedTerm. * [ML] Simplify sorting. * [ML] Fix regresssion when there's no data for failed transactions. * [ML] Rename fieldFilter to termFilters. * [ML] Update api integration test assertions. * [ML] Fix failed transactions params. * [ML] Tweak chart title. * [ML] Tweak chart colors. * [ML] Add translation. * [ML] Tweak selectedTerm if statement. * [ML] Fix types. * [ML] Fix assertion text. * [ML] Refactor replaceHistogramDotsWithBars. * [ML] Refactor fetchFailedTransactionsCorrelationPValues. * [ML] Fix score column width. Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Walter Rafelsberger <[email protected]>

walterra added 3 commits August 24, 2021 17:21

[ML] Fix tooltip text.

ca86f76

Revert "[ML] Fix tooltip text."

b17a0b9

This reverts commit ca86f76.

[ML] Chart prototype.

a88dba1

walterra self-assigned this Aug 26, 2021

walterra added 26 commits August 26, 2021 13:43

Merge branch 'master' of https://github.com/elastic/kibana

c6afb7b

Merge branch 'master' into ml-apm-failed-transactions-chart

a7d7bf6

[ML] Hover support for failed transactions chart.

1a1c692

[ML] Add p-value column.

6136ed0

Merge branch 'master' of https://github.com/elastic/kibana

cc04280

[ML] Code consolidation.

7f39a6b

[ML] Fix naming inconsistencies.

547ae65

[ML] Fix naming inconsistencies.

64cc796

[ML] Fix naming inconsistencies.

ade9431

[ML] Consolidate hooks.

be265ee

[ML] Consolidate hooks.

013369b

[ML] Consolidate hooks.

cb6c9de

[ML] Use function overloads.

5244347

[ML] Fix naming inconsistencies.

9104784

[ML] Fix jest test.

9a4b99d

[ML] Fix chart loading behavior.

afbcd8b

[ML] Rename values to latencyCorrelations.

f131283

[ML] Clean up types.

c9b3040

[ML] Add function overloads.

ecd1d75

Merge branch 'master' into ml-apm-correlations-consolidation-1

89dfda2

[Ml] Update API integration tests.

130f498

Merge branch 'master' into ml-apm-correlations-consolidation-1

9ddde60

[ML] Rename values to failedTransactionsCorrelations.

1236eb6

[ML] Fix naming inconsistencies.

bfa89a1

[ML] Fix naming inconsistencies.

8724c98

[ML] Fix naming inconsistencies.

aa4f909

qn895 reviewed Sep 9, 2021

View reviewed changes

peteharverson reviewed Sep 9, 2021

View reviewed changes

sorenlouv reviewed Sep 9, 2021

View reviewed changes

walterra added 3 commits September 9, 2021 23:42

[ML] Refactor replaceHistogramDotsWithBars.

0a56509

[ML] Refactor fetchFailedTransactionsCorrelationPValues.

95f4a49

[ML] Fix score column width.

869cb88

walterra mentioned this pull request Sep 9, 2021

[APM] Latency correlations 7.16 meta issue #109220

Closed

11 tasks

peteharverson approved these changes Sep 10, 2021

View reviewed changes

sorenlouv reviewed Sep 13, 2021

View reviewed changes

sorenlouv approved these changes Sep 13, 2021

View reviewed changes

Merge branch 'master' into ml-apm-failed-transactions-chart

3ffe075

peteharverson merged commit 065701a into elastic:master Sep 13, 2021

kibanamachine mentioned this pull request Sep 13, 2021

[7.x] [ML] APM Correlations: Chart for failed transactions correlations tab. (#110172) #111983

Merged

walterra deleted the ml-apm-failed-transactions-chart branch October 18, 2021 12:17

peteharverson added release_note:feature Makes this part of the condensed release notes and removed release_note:enhancement labels Nov 2, 2021

walterra mentioned this pull request Nov 9, 2021

[APM] Latency correlations meta issue #118009

Open

16 tasks

lcawl mentioned this pull request Nov 11, 2021

[DOCS] Refresh APM correlation screenshots #116723

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] APM Correlations: Chart for failed transactions correlations tab. #110172

[ML] APM Correlations: Chart for failed transactions correlations tab. #110172

walterra commented Aug 26, 2021 •

edited

Loading

qn895 Sep 9, 2021

qn895 commented Sep 9, 2021 •

edited

Loading

peteharverson commented Sep 9, 2021

peteharverson commented Sep 9, 2021 •

edited

Loading

peteharverson left a comment

sorenlouv Sep 9, 2021 •

edited

Loading

walterra Sep 9, 2021

sorenlouv Sep 9, 2021

walterra Sep 9, 2021

sorenlouv Sep 9, 2021

sorenlouv Sep 9, 2021

walterra Sep 9, 2021

sorenlouv Sep 13, 2021

walterra commented Sep 9, 2021 •

edited

Loading

peteharverson left a comment

sorenlouv Sep 13, 2021 •

edited

Loading

peteharverson commented Sep 13, 2021

kibanamachine commented Sep 13, 2021

kibanamachine commented Sep 13, 2021

	...flatten(data.map((d) => d.histogram)).map((d) => d.doc_count)
	...data.flatMap((d) => d.histogram).map((d) => d.doc_count)

[ML] APM Correlations: Chart for failed transactions correlations tab. #110172

[ML] APM Correlations: Chart for failed transactions correlations tab. #110172

Conversation

walterra commented Aug 26, 2021 • edited Loading

Summary

Checklist

Choose a reason for hiding this comment

qn895 commented Sep 9, 2021 • edited Loading

peteharverson commented Sep 9, 2021

peteharverson commented Sep 9, 2021 • edited Loading

peteharverson left a comment

Choose a reason for hiding this comment

sorenlouv Sep 9, 2021 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra commented Sep 9, 2021 • edited Loading

peteharverson left a comment

Choose a reason for hiding this comment

sorenlouv Sep 13, 2021 • edited Loading

Choose a reason for hiding this comment

peteharverson commented Sep 13, 2021

kibanamachine commented Sep 13, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

kibanamachine commented Sep 13, 2021

💚 Backport successful

walterra commented Aug 26, 2021 •

edited

Loading

qn895 commented Sep 9, 2021 •

edited

Loading

peteharverson commented Sep 9, 2021 •

edited

Loading

sorenlouv Sep 9, 2021 •

edited

Loading

walterra commented Sep 9, 2021 •

edited

Loading

sorenlouv Sep 13, 2021 •

edited

Loading