[ML] APM Latency Correlations: Fix formatting of duration values for log x axis and selection badge. #109214

walterra · 2021-08-19T09:54:41Z

Summary

Follow up to #108211.
Part of #106381.

Fix, clean up and unit tests for the log log charts x axis duration based ticks. This extends existing duration utilities to support a custom threshold to be able to fine tune the formatting for either single values with more detail or chart axis ticks where less detail is required. This is useful for log axis that cover a wider range of units. As can be seen in the screenshot, axis ticks will be formatted as full seconds from 1s onwards instead of 1,000 ms already up to 10 seconds. Because the threshold parameter is optional and defaults to 10, other uses of getDurationFormatter don't need to be changed.

Fixes the formatting of the selection badge.

Checklist

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

elasticmachine · 2021-08-19T10:18:09Z

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

elasticmachine · 2021-08-19T10:18:10Z

Pinging @elastic/ml-ui (:ml)

formgeist · 2021-08-19T10:58:35Z

Super small nit; I noticed that the chart x-axis label unit is without a space between the number and unit. I personally find it easier to read when there's a space between the value and unit.

Compared to e.g. our chart legends

formgeist

LGTM - I like that we're doing this for the logarithmic scale 👍

walterra · 2021-08-19T11:10:09Z

@formgeist I'll add the space between value and unit for consistency, will also make the code a bit simpler.

qn895 · 2021-08-19T19:20:50Z

x-pack/plugins/apm/common/utils/formatters/duration.ts

  unitKey: DurationTimeUnit,
-  value: number
+  value: number,
+  threshold: number = 10


super duper nit: I think it would be good to have a comment on what's the purpose of threshold, as it's not obvious at first glance.

Added in 03fadfe.

sorenlouv · 2021-08-19T23:01:04Z

x-pack/plugins/apm/common/utils/formatters/duration.ts

@@ -46,7 +47,8 @@ export function getUnitLabelAndConvertedValue(
          defaultMessage: 'h',
        }),
        convertedValue: asDecimalOrInteger(
-          moment.duration(value / 1000).asHours()
+          moment.duration(value / 1000).asHours(),


I know you didn't touch this but:

nit: Can you extract this calculation and re-use it for all units, to make it obvious that we are converting from milliseconds

const ms = value / 1000

Suggested change

moment.duration(value / 1000).asHours(),

moment.duration(ms).asHours(),

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

walterra · 2021-08-20T08:35:01Z

@elasticmachine merge upstream

kibanamachine · 2021-08-20T10:57:15Z

💚 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	-543.0B

History

💔 Build #147205 failed 3bbfa59
💚 Build #146898 succeeded fce5d55
💚 Build #146859 succeeded 9ba2d50
💚 Build #146853 succeeded 63b4753

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

cc @walterra

…log x axis and selection badge. (elastic#109214) - Fix, clean up and unit tests for the log log charts x axis duration based ticks. This extends existing duration utilities to support a custom threshold to be able to fine tune the formatting for either single values with more detail or chart axis ticks where less detail is required. This is useful for log axis that cover a wider range of units. As can be seen in the screenshot, axis ticks will be formatted as full seconds from 1s onwards instead of 1,000 ms already up to 10 seconds. Because the threshold parameter is optional and defaults to 10, other uses of getDurationFormatter don't need to be changed. - Fixes the formatting of the selection badge.

kibanamachine · 2021-08-20T11:04:08Z

💚 Backport successful

Status	Branch	Result
✅	7.15
✅	7.x

The backport PRs will be merged automatically after passing CI.

…log x axis and selection badge. (#109214) (#109416) - Fix, clean up and unit tests for the log log charts x axis duration based ticks. This extends existing duration utilities to support a custom threshold to be able to fine tune the formatting for either single values with more detail or chart axis ticks where less detail is required. This is useful for log axis that cover a wider range of units. As can be seen in the screenshot, axis ticks will be formatted as full seconds from 1s onwards instead of 1,000 ms already up to 10 seconds. Because the threshold parameter is optional and defaults to 10, other uses of getDurationFormatter don't need to be changed. - Fixes the formatting of the selection badge. Co-authored-by: Walter Rafelsberger <walter@elastic.co>

…log x axis and selection badge. (#109214) (#109415) - Fix, clean up and unit tests for the log log charts x axis duration based ticks. This extends existing duration utilities to support a custom threshold to be able to fine tune the formatting for either single values with more detail or chart axis ticks where less detail is required. This is useful for log axis that cover a wider range of units. As can be seen in the screenshot, axis ticks will be formatted as full seconds from 1s onwards instead of 1,000 ms already up to 10 seconds. Because the threshold parameter is optional and defaults to 10, other uses of getDurationFormatter don't need to be changed. - Fixes the formatting of the selection badge. Co-authored-by: Walter Rafelsberger <walter@elastic.co>

[ML] Improve formatting of x axis ticks.

63b4753

walterra added bug Fixes for quality problems that affect the customer experience Team:APM All issues that need APM UI Team support :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes apm:correlations v7.15.0 labels Aug 19, 2021

walterra self-assigned this Aug 19, 2021

[ML] Fix selection formatting.

9ba2d50

walterra marked this pull request as ready for review August 19, 2021 10:18

walterra requested a review from a team as a code owner August 19, 2021 10:18

walterra requested review from formgeist, sorenlouv and qn895 August 19, 2021 10:18

walterra changed the title ~~[ML] APM Latency Correlations: Fix formatting of axis ticks.~~ [ML] APM Latency Correlations: Fix formatting of duration values for log x axis and selection badge. Aug 19, 2021

walterra mentioned this pull request Aug 19, 2021

[APM] Latency correlations 7.15 meta issue #106381

Closed

24 tasks

formgeist approved these changes Aug 19, 2021

View reviewed changes

walterra added 2 commits August 19, 2021 14:08

[ML] Use default formatter with space between value and unit.

090b8c4

[ML] Adds unit tests for getFormattedSelection.

fce5d55

qn895 reviewed Aug 19, 2021

View reviewed changes

sorenlouv reviewed Aug 19, 2021

View reviewed changes

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

sorenlouv approved these changes Aug 19, 2021

View reviewed changes

walterra added 2 commits August 20, 2021 09:50

[ML] Feedback tweaks.

03fadfe

[ML] Feedback tweaks.

3bbfa59

walterra added auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 labels Aug 20, 2021

walterra enabled auto-merge (squash) August 20, 2021 07:53

Merge branch 'master' into ml-apm-distribution-chart-axis-fix

41890b5

walterra merged commit ca6bb4b into elastic:master Aug 20, 2021

kibanamachine mentioned this pull request Aug 20, 2021

[7.15] [ML] APM Latency Correlations: Fix formatting of duration values for log x axis and selection badge. (#109214) #109415

Merged

kibanamachine mentioned this pull request Aug 20, 2021

[7.x] [ML] APM Latency Correlations: Fix formatting of duration values for log x axis and selection badge. (#109214) #109416

Merged

walterra deleted the ml-apm-distribution-chart-axis-fix branch August 20, 2021 11:06

walterra mentioned this pull request Aug 24, 2021

[ML] Increase Test Coverage 7.15 #106113

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] APM Latency Correlations: Fix formatting of duration values for log x axis and selection badge. #109214

[ML] APM Latency Correlations: Fix formatting of duration values for log x axis and selection badge. #109214

walterra commented Aug 19, 2021 •

edited

Loading

elasticmachine commented Aug 19, 2021

elasticmachine commented Aug 19, 2021

formgeist commented Aug 19, 2021

formgeist left a comment

walterra commented Aug 19, 2021

qn895 Aug 19, 2021

walterra Aug 20, 2021

sorenlouv Aug 19, 2021 •

edited

Loading

walterra commented Aug 20, 2021

kibanamachine commented Aug 20, 2021

kibanamachine commented Aug 20, 2021

	moment.duration(value / 1000).asHours(),
	moment.duration(ms).asHours(),

[ML] APM Latency Correlations: Fix formatting of duration values for log x axis and selection badge. #109214

[ML] APM Latency Correlations: Fix formatting of duration values for log x axis and selection badge. #109214

Conversation

walterra commented Aug 19, 2021 • edited Loading

Summary

Checklist

elasticmachine commented Aug 19, 2021

elasticmachine commented Aug 19, 2021

formgeist commented Aug 19, 2021

formgeist left a comment

Choose a reason for hiding this comment

walterra commented Aug 19, 2021

qn895 Aug 19, 2021

Choose a reason for hiding this comment

walterra Aug 20, 2021

Choose a reason for hiding this comment

sorenlouv Aug 19, 2021 • edited Loading

Choose a reason for hiding this comment

walterra commented Aug 20, 2021

kibanamachine commented Aug 20, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

kibanamachine commented Aug 20, 2021

💚 Backport successful

walterra commented Aug 19, 2021 •

edited

Loading

sorenlouv Aug 19, 2021 •

edited

Loading