[APM] Fix gap in timeline and fallback when traceroot is missing (#25346

) (#25422) * Fix gap in timeline * Show fallback when traceroot is unavailable * Fix test * Fix rendering issues with styled-components * Remove `parentTransactionSkew ` * Minor test fixes
elastic · Nov 8, 2018 · 57f5c67 · 57f5c67
1 parent c997e78
commit 57f5c67
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 91 deletions.
diff --git a/.../apm/public/components/app/TransactionDetails/Transaction/StickyTransactionProperties.tsx b/.../apm/public/components/app/TransactionDetails/Transaction/StickyTransactionProperties.tsx
@@ -55,7 +55,7 @@ export function StickyTransactionProperties({
     },
     {
       label: '% of trace',
-      val: asPercent(duration, totalDuration),
+      val: asPercent(duration, totalDuration, 'N/A'),
       width: '25%'
     },
     {

diff --git a/...ponents/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/WaterfallItem.tsx b/...ponents/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/WaterfallItem.tsx
@@ -31,20 +31,16 @@ const ItemBar = styled<ItemBarProps, any>('div')`
   box-sizing: border-box;
   position: relative;
   height: ${px(unit)};
-  left: ${props => props.left}%;
-  width: ${props => props.width}%;
   min-width: 2px;
   background-color: ${props => props.color};
 `;
 
 // Note: "direction: rtl;" is here to prevent text from running off of
 // the right edge and instead pushing it to the left. For an example of
 // how this works, see here: https://codepen.io/sqren/pen/JrXNjY
-const SpanLabel = styled<{ left: number }, any>('div')`
+const SpanLabel = styled.div`
   white-space: nowrap;
   position: relative;
-  left: ${props => `${props.left}%`};
-  width: ${props => `${Math.max(100 - props.left, 0)}%`};
   direction: rtl;
   text-align: left;
   margin: ${px(units.quarter)} 0 0;
@@ -133,8 +129,16 @@ export function WaterfallItem({
       isSelected={isSelected}
       onClick={onClick}
     >
-      <ItemBar left={left} width={width} color={color} type={item.docType} />
-      <Label left={left}>
+      <ItemBar
+        // using inline styles instead of props to avoid generating a css class for each item
+        style={{ left: `${left}%`, width: `${width}%` }}
+        color={color}
+        type={item.docType}
+      />
+      <Label
+        // using inline styles instead of props to avoid generating a css class for each item
+        style={{ left: `${left}%`, width: `${Math.max(100 - left, 0)}%` }}
+      >
         {item.name} <Prefix item={item} />
       </Label>
     </Container>

diff --git a/...ails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.test.ts b/...ails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.test.ts
@@ -4,13 +4,12 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { groupBy, indexBy } from 'lodash';
+import { groupBy } from 'lodash';
 import { Span } from 'x-pack/plugins/apm/typings/Span';
 import { Transaction } from 'x-pack/plugins/apm/typings/Transaction';
 import {
   getClockSkew,
   getWaterfallItems,
-  IWaterfallIndex,
   IWaterfallItem
 } from './waterfall_helpers';
 
@@ -95,86 +94,84 @@ describe('waterfall_helpers', () => {
         items,
         hit => (hit.parentId ? hit.parentId : 'root')
       );
-      const itemsById: IWaterfallIndex = indexBy(items, 'id');
       const entryTransactionItem = childrenByParentId.root[0];
       expect(
-        getWaterfallItems(childrenByParentId, itemsById, entryTransactionItem)
+        getWaterfallItems(childrenByParentId, entryTransactionItem)
       ).toMatchSnapshot();
     });
   });
 
   describe('getClockSkew', () => {
     it('should adjust when child starts before parent', () => {
-      const item = {
+      const child = {
         docType: 'transaction',
         timestamp: 0,
         duration: 50
       } as IWaterfallItem;
 
-      const parentItem = {
+      const parent = {
         timestamp: 100,
-        skew: 5,
-        duration: 100
+        duration: 100,
+        skew: 5
       } as IWaterfallItem;
-      const parentTransactionSkew = 1337;
 
-      expect(getClockSkew(item, parentItem, parentTransactionSkew)).toBe(130);
+      expect(getClockSkew(child, parent)).toBe(130);
     });
 
     it('should adjust when child starts after parent has ended', () => {
-      const item = {
+      const child = {
         docType: 'transaction',
         timestamp: 250,
         duration: 50
       } as IWaterfallItem;
 
-      const parentItem = {
+      const parent = {
         timestamp: 100,
-        skew: 5,
-        duration: 100
+        duration: 100,
+        skew: 5
       } as IWaterfallItem;
-      const parentTransactionSkew = 1337;
 
-      expect(getClockSkew(item, parentItem, parentTransactionSkew)).toBe(-120);
+      expect(getClockSkew(child, parent)).toBe(-120);
     });
 
     it('should not adjust when child starts within parent duration', () => {
-      const item = {
+      const child = {
         docType: 'transaction',
         timestamp: 150,
         duration: 50
       } as IWaterfallItem;
 
-      const parentItem = {
+      const parent = {
         timestamp: 100,
-        skew: 5,
-        duration: 100
+        duration: 100,
+        skew: 5
       } as IWaterfallItem;
-      const parentTransactionSkew = 1337;
 
-      expect(getClockSkew(item, parentItem, parentTransactionSkew)).toBe(0);
+      expect(getClockSkew(child, parent)).toBe(0);
     });
 
-    it('should return parentTransactionSkew for spans', () => {
-      const item = {
+    it('should return parent skew for spans', () => {
+      const child = {
         docType: 'span'
       } as IWaterfallItem;
 
-      const parentItem = {} as IWaterfallItem;
-      const parentTransactionSkew = 1337;
+      const parent = {
+        timestamp: 100,
+        duration: 100,
+        skew: 5
+      } as IWaterfallItem;
 
-      expect(getClockSkew(item, parentItem, parentTransactionSkew)).toBe(1337);
+      expect(getClockSkew(child, parent)).toBe(5);
     });
 
-    it('should handle missing parentItem', () => {
-      const item = {
+    it('should handle missing parent', () => {
+      const child = {
         docType: 'transaction'
       } as IWaterfallItem;
 
-      const parentItem = undefined;
-      const parentTransactionSkew = 1337;
+      const parent = undefined;
 
-      expect(getClockSkew(item, parentItem, parentTransactionSkew)).toBe(0);
+      expect(getClockSkew(child, parent)).toBe(0);
     });
   });
 });
diff --git a/...onDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.ts b/...onDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.ts
@@ -28,7 +28,7 @@ export interface IWaterfallGroup {
 
 export interface IWaterfall {
   traceRoot?: Transaction;
-  traceRootDuration: number;
+  traceRootDuration?: number;
 
   /**
    * Duration in us
@@ -145,18 +145,19 @@ function getSpanItem(span: Span): IWaterfallItemSpan {
 
 export function getClockSkew(
   item: IWaterfallItem,
-  parentItem: IWaterfallItem | undefined,
-  parentTransactionSkew: number
+  parentItem?: IWaterfallItem
 ) {
+  if (!parentItem) {
+    return 0;
+  }
+
   switch (item.docType) {
+    // don't calculate skew for spans. Just use parent's skew
     case 'span':
-      return parentTransactionSkew;
-    case 'transaction': {
-      // For some reason the parent span and related transactions might be missing.
-      if (!parentItem) {
-        return 0;
-      }
+      return parentItem.skew;
 
+    // transaction is the inital entry in a service. Calculate skew for this, and it will be propogated to all child spans
+    case 'transaction': {
       const parentStart = parentItem.timestamp + parentItem.skew;
       const parentEnd = parentStart + parentItem.duration;
 
@@ -182,28 +183,25 @@ export function getClockSkew(
 
 export function getWaterfallItems(
   childrenByParentId: IWaterfallGroup,
-  itemsById: IWaterfallIndex,
   entryTransactionItem: IWaterfallItem
 ) {
   function getSortedChildren(
     item: IWaterfallItem,
-    parentTransactionSkew: number
+    parentItem?: IWaterfallItem
   ): IWaterfallItem[] {
-    const parentItem = item.parentId ? itemsById[item.parentId] : undefined;
-    const skew = getClockSkew(item, parentItem, parentTransactionSkew);
     const children = sortBy(childrenByParentId[item.id] || [], 'timestamp');
 
     item.childIds = children.map(child => child.id);
     item.offset = item.timestamp - entryTransactionItem.timestamp;
-    item.skew = skew;
+    item.skew = getClockSkew(item, parentItem);
 
     const deepChildren = flatten(
-      children.map(child => getSortedChildren(child, skew))
+      children.map(child => getSortedChildren(child, item))
     );
     return [item, ...deepChildren];
   }
 
-  return getSortedChildren(entryTransactionItem, 0);
+  return getSortedChildren(entryTransactionItem);
 }
 
 function getTraceRoot(childrenByParentId: IWaterfallGroup) {
@@ -265,7 +263,6 @@ export function getWaterfall(
     return {
       services: [],
       duration: 0,
-      traceRootDuration: 0,
       items: [],
       itemsById: {},
       getTransactionById: () => undefined,
@@ -296,16 +293,10 @@ export function getWaterfall(
   );
   const entryTransactionItem = getTransactionItem(entryTransaction);
   const itemsById: IWaterfallIndex = indexBy(filteredHits, 'id');
-  const items = getWaterfallItems(
-    childrenByParentId,
-    itemsById,
-    entryTransactionItem
-  );
+  const items = getWaterfallItems(childrenByParentId, entryTransactionItem);
   const traceRoot = getTraceRoot(childrenByParentId);
   const duration = getDuration(items);
-  const traceRootDuration = traceRoot
-    ? traceRoot.transaction.duration.us
-    : duration;
+  const traceRootDuration = traceRoot && traceRoot.transaction.duration.us;
   const services = getServices(items);
   const getTransactionById = createGetTransactionById(itemsById);
   const serviceColors = getServiceColors(services);

diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/index.tsx
@@ -30,30 +30,44 @@ function MaybeViewTraceLink({
   transaction: ITransaction;
   waterfall: IWaterfall;
 }) {
+  // the traceroot cannot be found, so we cannot link to it
+  if (!waterfall.traceRoot) {
+    return (
+      <EuiFlexItem grow={false}>
+        <EuiToolTip content="The trace parent cannot be found">
+          <EuiButton iconType="apmApp" disabled={true}>
+            View full trace
+          </EuiButton>
+        </EuiToolTip>
+      </EuiFlexItem>
+    );
+  }
+
   const isRoot =
-    transaction.transaction.id ===
-    (waterfall.traceRoot && waterfall.traceRoot.transaction.id);
+    transaction.transaction.id === waterfall.traceRoot.transaction.id;
 
-  let button;
+  // the user is already viewing the full trace, so don't link to it
   if (isRoot) {
-    button = (
-      <EuiToolTip content="Currently viewing the full trace">
-        <EuiButton iconType="apmApp" disabled={true}>
-          View full trace
-        </EuiButton>
-      </EuiToolTip>
+    return (
+      <EuiFlexItem grow={false}>
+        <EuiToolTip content="Currently viewing the full trace">
+          <EuiButton iconType="apmApp" disabled={true}>
+            View full trace
+          </EuiButton>
+        </EuiToolTip>
+      </EuiFlexItem>
     );
+
+    // the user is viewing a zoomed in version of the trace. Link to the full trace
   } else {
-    button = <EuiButton iconType="apmApp">View full trace</EuiButton>;
+    return (
+      <EuiFlexItem grow={false}>
+        <TransactionLink transaction={waterfall.traceRoot}>
+          <EuiButton iconType="apmApp">View full trace</EuiButton>
+        </TransactionLink>
+      </EuiFlexItem>
+    );
   }
-
-  return (
-    <EuiFlexItem grow={false}>
-      <TransactionLink transaction={waterfall.traceRoot}>
-        {button}
-      </TransactionLink>
-    </EuiFlexItem>
-  );
 }
 
 interface Props {

diff --git a/x-pack/plugins/apm/public/components/shared/charts/Timeline/TimelineAxis.js b/x-pack/plugins/apm/public/components/shared/charts/Timeline/TimelineAxis.js
@@ -16,8 +16,8 @@ import { getTimeFormatter } from '../../../../utils/formatters';
 
 // Remove any tick that is too close to traceRootDuration
 const getXAxisTickValues = (tickValues, traceRootDuration) => {
-  if (!tickValues) {
-    return [];
+  if (traceRootDuration == null) {
+    return tickValues;
   }
 
   const padding = (tickValues[1] - tickValues[0]) / 2;
@@ -73,11 +73,13 @@ function TimelineAxis({ plotValues, agentMarks, traceRootDuration }) {
                 }}
               />
 
-              <LastTickValue
-                x={xScale(traceRootDuration)}
-                value={tickFormat(traceRootDuration)}
-                marginTop={28}
-              />
+              {traceRootDuration && (
+                <LastTickValue
+                  x={xScale(traceRootDuration)}
+                  value={tickFormat(traceRootDuration)}
+                  marginTop={28}
+                />
+              )}
 
               {agentMarks.map(agentMark => (
                 <AgentMarker

diff --git a/x-pack/plugins/apm/public/components/shared/charts/Timeline/VerticalLines.js b/x-pack/plugins/apm/public/components/shared/charts/Timeline/VerticalLines.js
@@ -43,9 +43,16 @@ class VerticalLines extends PureComponent {
           />
 
           <VerticalGridLines
-            tickValues={[...agentMarkTimes, traceRootDuration]}
+            tickValues={agentMarkTimes}
             style={{ stroke: colors.gray3 }}
           />
+
+          {traceRootDuration && (
+            <VerticalGridLines
+              tickValues={[traceRootDuration]}
+              style={{ stroke: colors.gray3 }}
+            />
+          )}
         </XYPlot>
       </div>
     );

diff --git a/...apm/public/components/shared/charts/Timeline/__test__/__snapshots__/Timeline.test.js.snap b/...apm/public/components/shared/charts/Timeline/__test__/__snapshots__/Timeline.test.js.snap
@@ -827,6 +827,11 @@ exports[`Timeline should render with data 1`] = `
                 y1={0}
                 y2={116}
               />
+            </g>
+            <g
+              className="rv-xy-plot__grid-lines"
+              transform="translate(50,100)"
+            >
               <line
                 className="rv-xy-plot__grid-lines__line"
                 style={