[APM] Transactions breakdown graph: Add breakdown KPI component #38658

dgieselaar · 2019-06-11T14:50:14Z

Closes #36445. (See linked ticket for user story.)

~~(Based off of #37360, that needs to be merged first for the diff to be accurate).~~

To test this, start the apm-integration-testing server with the following command:
./scripts/compose.py start master --with-opbeans-java --opbeans-java-agent-branch=pr/588/head --apm-server-build https://github.com/elastic/apm-server.git@master and delete the template for the metric index.

Definition of Done for APM UI

Flag and create issues of things that are left out
Before/after images or gif of UI changes
Release labels
Designer sign-off for UI related changes
PM approval for bigger, user-facing features

elasticmachine · 2019-06-11T15:52:16Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-12T10:48:11Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-13T14:31:11Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-18T14:38:06Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-19T08:40:33Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

formgeist

@dgieselaar Just made some minor design tweak suggestions to the KPI display.

formgeist · 2019-06-19T10:40:28Z

...blic/components/app/TransactionOverview/TransactionBreakdown/TransactionBreakdownKpiList.tsx

+      wrap={false}
+    >
+      <EuiFlexItem grow={false}>
+        <Dot color={color} />


You could replace this styled component with <EuiIcon type="dot" color={color} />

formgeist · 2019-06-19T10:41:37Z

...blic/components/app/TransactionOverview/TransactionBreakdown/TransactionBreakdownKpiList.tsx

+      <EuiFlexItem grow={false}>
+        <Dot color={color} />
+      </EuiFlexItem>
+      <EuiFlexItem>


I reckon we need to add a min-width to this like: <EuiFlexItem style={{ minWidth: 110 }}>

formgeist · 2019-06-19T10:50:13Z

...blic/components/app/TransactionOverview/TransactionBreakdown/TransactionBreakdownKpiList.tsx

+}> = ({ percentage, count }) => {
+  return (
+    <EuiFlexGroup alignItems="center" gutterSize="s">
+      <EuiFlexItem>


I think it looks a little nicer if you add grow={false} to this item; `

formgeist · 2019-06-19T10:53:57Z

@dgieselaar Remember, we should add this component, as well as the graph, to the Transaction group detail page as well.

elasticmachine · 2019-06-20T11:01:21Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

dgieselaar · 2019-06-20T11:06:57Z

edit: see opening post for instructions.

dgieselaar · 2019-06-20T11:12:24Z

@formgeist any suggestions for empty/loading/error states?

formgeist · 2019-06-20T11:13:51Z

@dgieselaar Let me sketch some up 👍

formgeist · 2019-06-20T11:48:15Z

@dgieselaar I've added an example of the empty state to the original design issue, see the description.

Wrt the loading state, I believe we generally have a horizontal loader and show the panels' empty states until the panels and data is available. This is obviously less than ideal if the loading is longer than a few hundred ms but I think we need to solve that in another larger context. I'd prefer showing the panel with its title and label cleanly until data is available keeping a minimum height. If possible wait with displaying the error message until no data is returned for the request. Thoughts?

elasticmachine · 2019-06-20T12:10:33Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-21T10:06:36Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-24T20:15:57Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-25T10:16:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

dgieselaar · 2019-06-25T12:04:51Z

@formgeist any advice on what kind of error message should be shown (and where)? In the message bar (not sure what it's called) or inline in the component? @ogupte are there any other examples of the UI handling network errors?

formgeist · 2019-06-25T12:08:46Z

@dgieselaar Did you see the empty state example that I made last week? Is this what you're looking for?

elasticmachine · 2019-06-27T11:49:59Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…rendered

dgieselaar · 2019-06-28T06:47:54Z

retest

ogupte · 2019-06-28T06:36:50Z

...acy/plugins/apm/public/components/shared/TransactionBreakdown/TransactionBreakdownHeader.tsx

+          </h3>
+        </EuiTitle>
+      </EuiFlexItem>
+      {!hideShowChartButton && (


We may want to use the ternary pattern in JSX composition just to be consistent:

{!hideShowChartButton ? (...) : null}

ogupte · 2019-06-28T06:58:01Z

x-pack/legacy/plugins/apm/server/lib/transactions/breakdown/index.ts

+
+  const sumAllSelfTimes = resp.aggregations.sum_all_self_times.value || 0;
+
+  const breakdowns = flatten(


i think it's ok to use flatten. If we find that it really inflates the size of the kibana builds, then we can always optimize it as needed.

elasticmachine · 2019-06-28T08:23:55Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

dgieselaar · 2019-06-28T08:31:54Z

retest

elasticmachine · 2019-06-28T09:28:23Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

dgieselaar · 2019-06-28T14:07:50Z

@formgeist I'm going to merge this if you don't mind, it will allow me to prepare the charts PR. Want to keep things moving a bit because of the feature freeze on Tuesday. If there's anything you'd like to improve we can pick it up in #39531.

dgieselaar · 2019-06-28T14:08:24Z

retest

elasticmachine · 2019-06-28T15:10:03Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

Will address any further feedback in #39531.

formgeist · 2019-07-01T07:19:28Z

@dgieselaar Yeah, good call on merging this in 👍

…tic#38658) * [APM] Transactions breakdown graph: Add breakdown KPI component Closes elastic#36445. * Remove percentage, line wrapping, correct query * Add transaction breakdown chart to transaction details page * Move files into new plugin dir * Add empty state; update labels * Use new mapping * Move urlParams to useTransactionBreakdown hook * Fix types * Only show empty state in TransactionBreakdown if previously data was rendered * Restrict no of KPIs to 20 * Use ternary in render of TransactionBreakdownHeader for consistency

sorenlouv · 2019-07-02T22:53:33Z

x-pack/legacy/plugins/apm/public/hooks/useTransactionBreakdown.ts

+  const { data, error, status } = useFetcher(
+    async () => {
+      if (serviceName && start && end) {
+        return callApi<TransactionBreakdownAPIResponse>({


We have a convention of keeping rest calls separately in services/rest. Any reason to not do that here?

I think we previously discussed this shortly, whether this abstraction (of placing rest calls in a separate file) was worth it. I opted to give it a shot without one. Will we use the call without a hook? If we do use a separate file, do we need a separate file with the hook as well?

sorenlouv · 2019-07-02T22:56:21Z

x-pack/legacy/plugins/apm/public/components/shared/TransactionBreakdown/index.tsx

+
+  const hasHits = data && data.length > 0;
+
+  return receivedDataDuringLifetime ? (


Bailing early would make this a bit easier to digest:

if (!receivedDataDuringLifetime) { return null; }

(that would also be a small optimization if you moved it to the top although my primary motivation is readability)

dgieselaar requested a review from a team as a code owner June 11, 2019 14:50

dgieselaar requested review from ogupte and formgeist June 12, 2019 07:08

dgieselaar force-pushed the transaction-breakdown branch from aa3673b to 96fcdc1 Compare June 12, 2019 09:50

dgieselaar force-pushed the transaction-breakdown branch from 96fcdc1 to 936ddeb Compare June 13, 2019 13:30

dgieselaar added the release_note:roadmap label Jun 13, 2019

dgieselaar force-pushed the transaction-breakdown branch from 936ddeb to cfa8f96 Compare June 18, 2019 13:38

dgieselaar force-pushed the transaction-breakdown branch from cfa8f96 to 562243d Compare June 19, 2019 07:35

formgeist previously requested changes Jun 19, 2019

View reviewed changes

dgieselaar force-pushed the transaction-breakdown branch from 562243d to 1fdd265 Compare June 20, 2019 09:48

dgieselaar force-pushed the transaction-breakdown branch from 1416ba0 to 4d32543 Compare June 21, 2019 09:02

dgieselaar force-pushed the transaction-breakdown branch from 4d32543 to 2483bfe Compare June 24, 2019 19:17

dgieselaar force-pushed the transaction-breakdown branch from 2483bfe to 551d640 Compare June 25, 2019 09:11

dgieselaar added 3 commits June 27, 2019 12:06

Add empty state; update labels

c98ce8d

Use new mapping

98d35a4

Move urlParams to useTransactionBreakdown hook

028a3ad

dgieselaar force-pushed the transaction-breakdown branch from 3cb9d0e to 028a3ad Compare June 27, 2019 10:12

Fix types

a03250b

dgieselaar added 2 commits June 27, 2019 20:57

Only show empty state in TransactionBreakdown if previously data was …

fe50570

…rendered

Restrict no of KPIs to 20

08e690c

dgieselaar requested review from ogupte and formgeist June 27, 2019 20:02

ogupte approved these changes Jun 28, 2019

View reviewed changes

Use ternary in render of TransactionBreakdownHeader for consistency

263478a

dgieselaar merged commit 3512463 into elastic:master Jun 28, 2019

dgieselaar deleted the transaction-breakdown branch June 28, 2019 15:27

dgieselaar mentioned this pull request Jun 28, 2019

[7.x] [APM] Transactions breakdown graph: Add breakdown KPI component (#38658) #39910

Merged

dgieselaar mentioned this pull request Jun 28, 2019

[APM] Transaction breakdown charts #39531

Merged

5 tasks

sorenlouv reviewed Jul 2, 2019

View reviewed changes

bmorelli25 mentioned this pull request Jul 15, 2019

[docs][APM] Breakdown graph and screenshots #41175

Merged


		const sumAllSelfTimes = resp.aggregations.sum_all_self_times.value \|\| 0;

		const breakdowns = flatten(


		const hasHits = data && data.length > 0;

		return receivedDataDuringLifetime ? (

[APM] Transactions breakdown graph: Add breakdown KPI component #38658

[APM] Transactions breakdown graph: Add breakdown KPI component #38658

Conversation

dgieselaar commented Jun 11, 2019 • edited Loading

Definition of Done for APM UI

elasticmachine commented Jun 11, 2019

💔 Build Failed

elasticmachine commented Jun 12, 2019

💔 Build Failed

elasticmachine commented Jun 13, 2019

💚 Build Succeeded

elasticmachine commented Jun 18, 2019

💔 Build Failed

elasticmachine commented Jun 19, 2019

💔 Build Failed

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

formgeist commented Jun 19, 2019

elasticmachine commented Jun 20, 2019

💚 Build Succeeded

dgieselaar commented Jun 20, 2019 • edited Loading

dgieselaar commented Jun 20, 2019

formgeist commented Jun 20, 2019

formgeist commented Jun 20, 2019

elasticmachine commented Jun 20, 2019

💚 Build Succeeded

elasticmachine commented Jun 21, 2019

💔 Build Failed

elasticmachine commented Jun 24, 2019

💔 Build Failed

elasticmachine commented Jun 25, 2019

💚 Build Succeeded

dgieselaar commented Jun 25, 2019

formgeist commented Jun 25, 2019

elasticmachine commented Jun 27, 2019

💔 Build Failed

dgieselaar commented Jun 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Jun 28, 2019

💔 Build Failed

dgieselaar commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

💔 Build Failed

dgieselaar commented Jun 28, 2019

dgieselaar commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

💚 Build Succeeded

formgeist commented Jul 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorenlouv Jul 2, 2019 • edited Loading

Choose a reason for hiding this comment

dgieselaar commented Jun 11, 2019 •

edited

Loading

dgieselaar commented Jun 20, 2019 •

edited

Loading

sorenlouv Jul 2, 2019 •

edited

Loading