Timeline transaction flyout #24027

jasonrhodes · 2018-10-15T14:51:26Z

Summary

This replaces work in #24010 after discussions with the team and me better understanding the work here. Closes #23567

UPDATE: this work also includes persisting the flyout and flyout active tab state in the query string for both the transaction flyouts and the span flyouts.

e.g.

example transaction link (with service tab active)
http://localhost:5601/app/apm#/opbeans-java/transactions/request/APIRestController~23products?_g=()&kuery=&flyoutDetailTab=service&activeTimelineId=6da58c75083c86e9

example span link
http://localhost:5601/app/apm#/opbeans-java/transactions/request/APIRestController~23products?_g=()&kuery=&flyoutDetailTab=null&activeTimelineId=1b394e907aaf708d

sorenlouv · 2018-10-15T14:56:55Z

Thanks for the very thorough description and checkboxes.
Did you mean to merge it into master or apm/dt?

jasonrhodes · 2018-10-15T15:23:47Z

@sqren defiitely meant apm/dt, keep forgetting to switch that base when I create.

Looks like the checkboxes are new, modeled after EUI's PR template or similar!

sorenlouv · 2018-10-15T15:26:01Z

Looks like the checkboxes are new, modeled after EUI's PR template or similar!

Oh, that makes sense. I was very impressed that you were going to test it in IE11 at this stage :D

elasticmachine · 2018-10-15T15:34:39Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

sorenlouv · 2018-10-15T19:19:42Z

.../public/components/app/TransactionDetails/Transaction/TransactionPropertiesTableForFlyout.js

+  location: PropTypes.object.isRequired,
+  transaction: PropTypes.object.isRequired,
+  urlParams: PropTypes.object.isRequired
+};


Perhaps convert to typescript, since it's a new file anyway.

sorenlouv · 2018-10-15T19:21:21Z

.../public/components/app/TransactionDetails/Transaction/TransactionPropertiesTableForFlyout.js

+                  ...location,
+                  search: fromQuery({
+                    ...toQuery(location.search),
+                    flyoutDetailTab: key


Does this currently work? I think you'll have to update urlparams.js to make it work (this is something I want to change)

Yeah! I figured that out eventually (before I saw this comment unfortunately :D)

elasticmachine · 2018-10-15T21:27:42Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

sorenlouv · 2018-10-16T08:04:33Z

.../app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/TransactionFlyout/index.tsx

@@ -11,10 +11,13 @@ import {
  EuiTitle
 } from '@elastic/eui';
 import React from 'react';
+import { IUrlParams } from '..';


fyi: my PR moves the urlParams interface: https://github.com/elastic/kibana/pull/23636/files#diff-07bc963256e81bcf166ef7e957b5aa11R150

.../public/components/app/TransactionDetails/Transaction/TransactionPropertiesTableForFlyout.js

.../public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/index.tsx

sorenlouv

LGTM. Only a few minor things.

elasticmachine · 2018-10-16T19:08:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

sorenlouv · 2018-10-16T19:29:25Z

.../public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/index.tsx

@@ -38,6 +39,8 @@ export interface IUrlParams {
  transactionType: string;
  transactionName: string;
  errorGroupId: string;
+  flyoutDetailTab: string | null;
+  activeTimelineId: string | null;


Would this work?

Suggested change

activeTimelineId: string | null;

activeTimelineId?: string;

(btw. had to try out the new suggestion feature ;) )

Also, you might want to rebase with apm/dt. Lots of typescript changes :)

The ? doesn't work here either because that is an alias for string | undefined but not for null. I was using null to clear out query param values since you can't remove them easily, since they are merged in with old ones. The spanId that exists in 6.4 sets them to null so I went with that. :)

*Open to suggestions on how to make this work better. :)

elasticmachine · 2018-10-17T02:26:02Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

jasonrhodes · 2018-10-17T02:29:13Z

retest

jasonrhodes · 2018-10-17T03:15:37Z

This is all rebased and feedback addressed. :D

...Details/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.test.ts

...ctionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.ts

jasonrhodes · 2018-10-17T03:59:10Z

Just remembered, for tomorrow: this flyout needs a link to go to the transaction group

elasticmachine · 2018-10-17T04:50:18Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

jasonrhodes · 2018-10-17T17:06:38Z

retest

sorenlouv · 2018-10-16T19:57:44Z

.../public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/index.tsx

-    const { waterfall } = this.props;
+    const { waterfall, location, urlParams } = this.props;
+    const currentItem =
+      urlParams.activeTimelineId && waterfall.index[urlParams.activeTimelineId];


Why not just:

Suggested change

urlParams.activeTimelineId && waterfall.index[urlParams.activeTimelineId];

waterfall.index[urlParams.activeTimelineId];

.../public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/index.tsx

jasonrhodes · 2018-10-19T02:38:40Z

OK if builds pass (lol) then I think this is in good shape to move on for now. We should rethink the sticky properties stuff later bc I'm not thrilled with how it's working right now, but I was able to use EUI flex components and get it laid out like the mock up so I think that's a win for this week. :)

elasticmachine · 2018-10-19T04:17:20Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-10-19T13:22:45Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

… values

elasticmachine · 2018-10-19T14:42:44Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

jasonrhodes · 2018-10-19T15:22:59Z

x-pack/plugins/apm/public/utils/documentation.ts

+): AgentDoc | void {
+  if (!agentName) {
+    return;
+  }


have to keep this return type for now because otherwise callers of this function don't know what they're getting returned, because TS can't infer from the lodash get call.

But splitting out AgentDoc as an interface and using it here and in the list above uncovered a bug with the javascript object having the URL assigned to the text key instead of url, so yay typescript!

jasonrhodes · 2018-10-19T15:23:06Z

x-pack/plugins/apm/public/utils/documentation.ts

@@ -112,7 +123,7 @@ export const APM_AGENT_DOCS = {
      url: `${DOCS_ROOT}/agent/ruby/1.x/advanced.html#_adding_tags`
    },
    javascript: {
-      text: `${DOCS_ROOT}/agent/js-base/0.x/api.html#apm-set-tags`
+      url: `${DOCS_ROOT}/agent/js-base/0.x/api.html#apm-set-tags`


TS was all worth it

elasticmachine · 2018-10-19T17:05:20Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-10-19T17:37:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Adds traces overview with mock data (#22628) * Adds traces overview with mock data * Updates service overview snapshots * Updates based on review feedback * Adds tests for ManagedTable and ImpactBar * Refactored transaction overview to use new managed table component * Fixes tab size * Cleans up some tests and types * Cleans up types and tests * kbn bootstrap changes to x-pack yarn.lock * Removed jsconfig file in apm * Updates snapshot tests * Reversed bogus yarn lock change in x-pack * review feedback wip * Resolves typescript issues * [APM] Distributed tracing - Trace details (waterfall) (#22763) * [APM] Add typescript to waterfall (#23635) * [APM] Migrate get_trace and constants to Typescript (#23634) * Bump lock files * [APM] Add types for setup_request (#23762) * [APM] Adds trace overview queries and some refactoring (#23605) Refactoring includes: * ImpactBar component to align EuiProgress usage for impact bars * Sharing some logic between transaction and trace queries * Typescript support * Quick fix ‘banana’ * [APM] Ensure backwards compatibility for v1 and v2 (#23636) Fix tests Make interfaces versioned Fix ts issues Rename eventType to docType WIP Working prototype Fix tests Limit get_transaction by trace.id * Fixes trace links on traces overview (#24089) * [APM] use react-redux-request (#24117) * [APM] use react-redux-request * Stricter type checking * Bump yarn lock files * Fix package.json * Updated yarn lockfile for new yarn version * Updated dependency issues for react-router-dom types * [APM] Display transaction info on span flyout (#24189) * [APM] Display transaction info on span flyout * Add links * Fix tests * Eui fixes * Remove color from span type * Fix failing tests * Adds versions to transaction and span itmes when they are returned from the API (#24186) * Timeline transaction flyout (#24027) * Duplicates transaction properties table for use in flyout, with dummy values * Brings in real location and url param data for transaction flyout * Converts flyout to TS * Adds query param state for flyouts with ts support * Updates styles and uses EuiTabs for transaction flyout * Updates type references after rebase * Updated index var name per review feedback * WIP transaction flyout * WIP transaction flyout * WIP * Fixes waterfall helpers after rebasing span flyout changes * Finalizes styling for transaction flyover from timeline * Fixes layout for span flyout header * Removed accidentally committed file (should be in other branch) * Small tweaks to the sticky property layout styling * Updates snapshot tests after sticky properties converting to EUI * Final review tweaks for TS * [APM] Minor docs cleanup (#24325) * [APM] Minor docs cleanup * Fix tests * Make `text` required * Simplify documentation module * Break docs into separate files * Remove `translateAgentName` method * Formatting * [APM] Fix issues with v1 spans (#24332) * [APM] Add agent marks (#24361) * Typescript migration for the transaction endpoints (#24397) * DT transaction sample header (#24294) Transaction sample header completed * Fixes link target for traces overview to include trans/trace ids as query params * Converts Transaction index file to TS * Adds trace link to sample section * Refactors the trace link and applies it to two usages * Implements transaction sample action context menu * Calculates and implements duration percentage * Re-typed how transaction groups work * Fixes transaction flyout links and context menu * Removes unnecessary ms multiplication * Removes unused commented code * Finalizes infra links * Fixes some type shenanigans

This was referenced Oct 15, 2018

WIP: Timeline transaction flyout #24010

Closed

[APM] DT Transaction flyout #23567

Closed

jasonrhodes changed the base branch from master to apm/dt October 15, 2018 15:23

sorenlouv reviewed Oct 15, 2018

View reviewed changes

sorenlouv reviewed Oct 16, 2018

View reviewed changes

.../public/components/app/TransactionDetails/Transaction/TransactionPropertiesTableForFlyout.js Outdated Show resolved Hide resolved

sorenlouv reviewed Oct 16, 2018

View reviewed changes

.../public/components/app/TransactionDetails/Transaction/TransactionPropertiesTableForFlyout.js Outdated Show resolved Hide resolved

sorenlouv reviewed Oct 16, 2018

View reviewed changes

.../public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/index.tsx Outdated Show resolved Hide resolved

sorenlouv reviewed Oct 16, 2018

View reviewed changes

jasonrhodes force-pushed the timeline-transaction-flyout-b branch 2 times, most recently from 5e0a476 to 8e4a72f Compare October 17, 2018 03:11

jasonrhodes commented Oct 17, 2018

View reviewed changes

...Details/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.test.ts Show resolved Hide resolved

jasonrhodes commented Oct 17, 2018

View reviewed changes

...ctionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.ts Outdated Show resolved Hide resolved

jasonrhodes changed the title ~~WIP: Timeline transaction flyout (B)~~ Timeline transaction flyout (B) Oct 17, 2018

jasonrhodes changed the title ~~Timeline transaction flyout (B)~~ Timeline transaction flyout Oct 17, 2018

sorenlouv reviewed Oct 17, 2018

View reviewed changes

jasonrhodes force-pushed the timeline-transaction-flyout-b branch from a68d7ad to 8020e0e Compare October 19, 2018 12:24

jasonrhodes added 16 commits October 19, 2018 10:34

Duplicates transaction properties table for use in flyout, with dummy…

3aa080b

… values

Brings in real location and url param data for transaction flyout

5821c35

Converts flyout to TS

2c65450

Adds query param state for flyouts with ts support

6c6f22b

Updates styles and uses EuiTabs for transaction flyout

50ebc5c

Updates type references after rebase

a606f68

Updated index var name per review feedback

c0dda8e

WIP transaction flyout

7d2a04a

WIP transaction flyout

c77250e

WIP

42f62eb

Fixes waterfall helpers after rebasing span flyout changes

7d0667b

Finalizes styling for transaction flyover from timeline

2b7a827

Fixes layout for span flyout header

921dcd4

Removed accidentally committed file (should be in other branch)

ca7d342

Small tweaks to the sticky property layout styling

202d982

Updates snapshot tests after sticky properties converting to EUI

bd965ff

jasonrhodes force-pushed the timeline-transaction-flyout-b branch from 51ef796 to bd965ff Compare October 19, 2018 14:43

Final review tweaks for TS

d091d8a

jasonrhodes commented Oct 19, 2018

View reviewed changes

jasonrhodes merged commit 5ffd3a6 into elastic:apm/dt Oct 19, 2018

jasonrhodes deleted the timeline-transaction-flyout-b branch October 19, 2018 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Timeline transaction flyout #24027

Timeline transaction flyout #24027

jasonrhodes commented Oct 15, 2018 •

edited

Loading

sorenlouv commented Oct 15, 2018

jasonrhodes commented Oct 15, 2018

sorenlouv commented Oct 15, 2018

elasticmachine commented Oct 15, 2018

sorenlouv Oct 15, 2018

sorenlouv Oct 15, 2018

jasonrhodes Oct 15, 2018

elasticmachine commented Oct 15, 2018

sorenlouv Oct 16, 2018 •

edited

Loading

sorenlouv left a comment

elasticmachine commented Oct 16, 2018

sorenlouv Oct 16, 2018

sorenlouv Oct 16, 2018

jasonrhodes Oct 17, 2018 •

edited

Loading

elasticmachine commented Oct 17, 2018

jasonrhodes commented Oct 17, 2018

jasonrhodes commented Oct 17, 2018

jasonrhodes commented Oct 17, 2018

elasticmachine commented Oct 17, 2018

jasonrhodes commented Oct 17, 2018

sorenlouv Oct 16, 2018

jasonrhodes commented Oct 19, 2018

elasticmachine commented Oct 19, 2018

elasticmachine commented Oct 19, 2018

elasticmachine commented Oct 19, 2018

jasonrhodes Oct 19, 2018

jasonrhodes Oct 19, 2018

elasticmachine commented Oct 19, 2018

elasticmachine commented Oct 19, 2018

	urlParams.activeTimelineId && waterfall.index[urlParams.activeTimelineId];
	waterfall.index[urlParams.activeTimelineId];

Timeline transaction flyout #24027

Timeline transaction flyout #24027

Conversation

jasonrhodes commented Oct 15, 2018 • edited Loading

Summary

sorenlouv commented Oct 15, 2018

jasonrhodes commented Oct 15, 2018

sorenlouv commented Oct 15, 2018

elasticmachine commented Oct 15, 2018

💔 Build Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 15, 2018

💔 Build Failed

sorenlouv Oct 16, 2018 • edited Loading

Choose a reason for hiding this comment

sorenlouv left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 16, 2018

💔 Build Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonrhodes Oct 17, 2018 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Oct 17, 2018

💔 Build Failed

jasonrhodes commented Oct 17, 2018

jasonrhodes commented Oct 17, 2018

jasonrhodes commented Oct 17, 2018

elasticmachine commented Oct 17, 2018

💔 Build Failed

jasonrhodes commented Oct 17, 2018

Choose a reason for hiding this comment

jasonrhodes commented Oct 19, 2018

elasticmachine commented Oct 19, 2018

💔 Build Failed

elasticmachine commented Oct 19, 2018

💔 Build Failed

elasticmachine commented Oct 19, 2018

💔 Build Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 19, 2018

💚 Build Succeeded

elasticmachine commented Oct 19, 2018

💚 Build Succeeded

jasonrhodes commented Oct 15, 2018 •

edited

Loading

sorenlouv Oct 16, 2018 •

edited

Loading

jasonrhodes Oct 17, 2018 •

edited

Loading