Changes according review

Signed-off-by: Patrick Coray <[email protected]>
jaegertracing · Dec 17, 2018 · 6da9b78 · 6da9b78
1 parent 3a619ce
commit 6da9b78
Show file tree

Hide file tree

Showing 8 changed files with 328 additions and 135 deletions.
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.css
@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 .OpNode {
+  width: 100%;
   border: 1px solid #111;
   cursor: pointer;
   white-space: nowrap;
@@ -23,7 +24,7 @@ limitations under the License.
 }
 
 .OpNode td,
-th {
+.OpNode th {
   border: none;
 }
 
@@ -36,6 +37,7 @@ th {
 }
 
 .OpNode--metricCell {
+  text-align: right;
   padding: 0.3rem 0.5rem;
   background: rgba(255, 255, 255, 0.3);
 }

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.js
@@ -15,7 +15,7 @@
 // limitations under the License.
 
 import * as React from 'react';
-import { Popover, Icon } from 'antd';
+import { Popover } from 'antd';
 import colorGenerator from '../../../utils/color-generator';
 
 import type { PVertex } from '../../../model/trace-dag/types';
@@ -24,112 +24,106 @@ import './OpNode.css';
 
 type Props = {
   count: number,
+  errors: number,
   time: number,
   percent: number,
   selfTime: number,
+  percentSelfTime: number,
   operation: string,
   service: string,
-  registerFn: any,
-};
-type State = {
-  nodeMode: boolean,
+  mode: string,
 };
 
-function formatPercent(percent) {
+export const MODE_SERVICE = 'service';
+export const MODE_TIME = 'time';
+export const MODE_SELFTIME = 'selftime';
+
+export function round2(percent: number) {
   return Math.round(percent * 100) / 100;
 }
 
-export default class OpNode extends React.PureComponent<Props, State> {
+export default class OpNode extends React.PureComponent<Props> {
   props: Props;
-  state: State;
 
-  static helpPopover() {
+  static helpTable() {
     const helpTable = (
-      <table className={`OpNode OpNode--mode-service`}>
+      <table className="OpNode OpNode--mode-service">
         <tbody>
           <tr>
-            <td className={`OpNode--metricCell`}>Count</td>
-            <td className={`OpNode--labelCell`}>
+            <td className="OpNode--metricCell">Count / Error</td>
+            <td className="OpNode--labelCell">
               <strong>Service</strong>
             </td>
-            <td className={`OpNode--metricCell`}>% of trace duration</td>
+            <td className="OpNode--metricCell">Avg</td>
           </tr>
           <tr>
-            <td className={`OpNode--metricCell`}>Duration</td>
-            <td className={`OpNode--labelCell`}>Operation</td>
-            <td className={`OpNode--metricCell`}>Self time</td>
+            <td className="OpNode--metricCell">Duration</td>
+            <td className="OpNode--labelCell">Operation</td>
+            <td className="OpNode--metricCell">Self time</td>
           </tr>
         </tbody>
       </table>
     );
-    return (
-      <Popover
-        overlayClassName={`OpNode--popover`}
-        placement="left"
-        mouseEnterDelay={0.25}
-        content={helpTable}
-      >
-        <Icon type="question-circle" />
-      </Popover>
-    );
-  }
-
-  constructor(props: Props) {
-    super(props);
-    this.state = {
-      nodeMode: false,
-    };
-    props.registerFn.call(this, this);
+    return helpTable;
   }
 
-  toggleNodeMode = () => {
-    const { nodeMode } = this.state;
-    this.setState({ nodeMode: !nodeMode });
-  };
-
   render() {
-    const { count, time, percent, selfTime, operation, service } = this.props;
-    const modeType = this.state.nodeMode ? 'time' : 'service';
+    const { count, errors, time, percent, selfTime, percentSelfTime, operation, service, mode } = this.props;
+
     // Spans over 20 % time are full red - we have probably to reconsider better approach
-    const percentBoosted = Math.min(percent / 20, 1);
-    const backgroundColor = this.state.nodeMode
-      ? [255, 0, 0, percentBoosted].join()
-      : colorGenerator
-          .getRgbColorByKey(service)
-          .concat(0.8)
-          .join();
+    let backgroundColor;
+    if (mode === MODE_TIME) {
+      const percentBoosted = Math.min(percent / 20, 1);
+      backgroundColor = [255, 0, 0, percentBoosted].join();
+    } else if (mode === MODE_SELFTIME) {
+      backgroundColor = [255, 0, 0, percentSelfTime / 100].join();
+    } else {
+      backgroundColor = colorGenerator
+        .getRgbColorByKey(service)
+        .concat(0.8)
+        .join();
+    }
+
     const table = (
-      <table className={`OpNode OpNode--mode-${modeType}`} cellSpacing="0">
+      <table className={`OpNode OpNode--mode-${mode}`} cellSpacing="0">
         <tbody
           style={{
             background: `rgba(${backgroundColor})`,
           }}
         >
           <tr>
-            <td className={`OpNode--metricCell OpNode--count`}>{count}</td>
-            <td className={`OpNode--labelCell OpNode--service`}>
+            <td className="OpNode--metricCell OpNode--count">
+              {count} / {errors}
+            </td>
+            <td className="OpNode--labelCell OpNode--service">
               <strong>{service}</strong>
             </td>
-            <td className={`OpNode--metricCell OpNode--percent`}>{formatPercent(percent)} %</td>
+            <td className="OpNode--metricCell OpNode--avg">{round2(time / 1000 / count)} ms</td>
           </tr>
           <tr>
-            <td className={`OpNode--metricCell OpNode--time`}>{time / 1000} ms</td>
-            <td className={`OpNode--labelCell OpNode--op`}>{operation}</td>
-            <td className={`OpNode--metricCell OpNode--selfTime`}>{selfTime / 1000} ms</td>
+            <td className="OpNode--metricCell OpNode--time">
+              {time / 1000} ms ({round2(percent)} %)
+            </td>
+            <td className="OpNode--labelCell OpNode--op">{operation}</td>
+            <td className="OpNode--metricCell OpNode--selfTime">
+              {selfTime / 1000} ms ({round2(percentSelfTime)} %)
+            </td>
           </tr>
         </tbody>
       </table>
     );
 
     return (
-      <Popover overlayClassName={`OpNode--popover`} mouseEnterDelay={0.25} content={table}>
+      <Popover overlayClassName="OpNode--popover" mouseEnterDelay={0.25} content={table}>
         {table}
       </Popover>
     );
   }
 }
 
-export function drawNode<T>(vertex: PVertex<T>) {
-  const { data, operation, service } = vertex.data;
-  return <OpNode {...data} operation={operation} service={service} />;
+export function getNodeDrawer(mode: string) {
+  return function drawNode<T>(vertex: PVertex<T>) {
+    const { data, operation, service } = vertex.data;
+    return <OpNode {...data} mode={mode} operation={operation} service={service} />;
+  };
 }
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js b/packages/jaeger-ui/src/components/TracePage/TraceGraph/OpNode.test.js
@@ -15,49 +15,53 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 
-import OpNode, { drawNode } from './OpNode';
+import OpNode, { getNodeDrawer, MODE_SERVICE, MODE_TIME, MODE_SELFTIME } from './OpNode';
 
 describe('<OpNode>', () => {
   let wrapper;
-  let cb;
+  let mode;
+  let props;
 
   beforeEach(() => {
-    cb = jest.fn();
-    const props = {
+    mode = MODE_SERVICE;
+    props = {
       count: 5,
+      errors: 0,
       time: 200000,
       percent: 7.89,
       selfTime: 180000,
+      percentSelfTime: 90,
       operation: 'op1',
       service: 'service1',
-      registerFn: cb,
     };
-    wrapper = shallow(<OpNode {...props} />);
+    wrapper = shallow(<OpNode {...props} mode={mode} />);
   });
 
   it('it does not explode', () => {
     expect(wrapper).toBeDefined();
-    expect(cb).toHaveBeenCalledTimes(1); // register
     expect(wrapper.find('.OpNode').length).toBe(1);
     expect(wrapper.find('.OpNode--mode-service').length).toBe(1);
   });
 
   it('it renders OpNode', () => {
-    expect(wrapper.find('.OpNode--count').text()).toBe('5');
-    expect(wrapper.find('.OpNode--time').text()).toBe('200 ms');
-    expect(wrapper.find('.OpNode--percent').text()).toBe('7.89 %');
-    expect(wrapper.find('.OpNode--selfTime').text()).toBe('180 ms');
+    expect(wrapper.find('.OpNode--count').text()).toBe('5 / 0');
+    expect(wrapper.find('.OpNode--time').text()).toBe('200 ms (7.89 %)');
+    expect(wrapper.find('.OpNode--avg').text()).toBe('40 ms');
+    expect(wrapper.find('.OpNode--selfTime').text()).toBe('180 ms (90 %)');
     expect(wrapper.find('.OpNode--op').text()).toBe('op1');
     expect(wrapper.find('.OpNode--service').text()).toBe('service1');
   });
 
   it('it switches mode', () => {
-    expect(wrapper.state().nodeMode).toBeFalsy();
-    wrapper.instance().toggleNodeMode();
-    expect(wrapper.state().nodeMode).toBeTruthy();
+    mode = MODE_TIME;
+    wrapper = shallow(<OpNode {...props} mode={mode} />);
+    expect(wrapper.find('.OpNode--mode-time').length).toBe(1);
+    mode = MODE_SELFTIME;
+    wrapper = shallow(<OpNode {...props} mode={mode} />);
+    expect(wrapper.find('.OpNode--mode-selftime').length).toBe(1);
   });
 
-  describe('drawNode()', () => {
+  describe('getNodeDrawer()', () => {
     it('it creates OpNode', () => {
       const vertex = {
         data: {
@@ -66,6 +70,7 @@ describe('<OpNode>', () => {
           data: {},
         },
       };
+      const drawNode = getNodeDrawer(MODE_SERVICE);
       const opNode = drawNode(vertex);
       expect(opNode.type === 'OpNode');
     });

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceGraph/TraceGraph.css b/packages/jaeger-ui/src/components/TracePage/TraceGraph/TraceGraph.css
@@ -22,13 +22,18 @@ limitations under the License.
   position: absolute;
   right: 0;
   top: 0;
+  display: flex;
+}
+
+.TraceGraph--sidebar-container {
+  display: flex;
+  z-index: 1;
 }
 
 .TraceGraph--menu {
   cursor: pointer;
-  position: absolute;
-  right: 1rem;
-  z-index: 1;
+  padding-right: 1rem;
+  padding-top: 1rem;
 }
 
 .TraceGraph--menu > li {
@@ -37,6 +42,15 @@ limitations under the License.
   padding-bottom: 0.3rem;
 }
 
+.TraceGraph--sidebar {
+  cursor: default;
+  box-shadow: -1px 0 #00000030;
+}
+
+.TraceGraph--help-content > div {
+  margin-top: 1rem;
+}
+
 .TraceGraph--dag {
   stroke-width: 1.2;
 }