Add ddg menu item, fetch server ops, expand GA cov (#501)

* Temp: Default link visible with banner Signed-off-by: Everett Ross <reverett@uber.com> * WIP: Use server ops in ddg, track conv&collapse Signed-off-by: Everett Ross <reverett@uber.com> * Add tests Signed-off-by: Everett Ross <reverett@uber.com> * Remove DDG announcement banner Signed-off-by: Everett Ross <reverett@uber.com> * Remove default ddg.menuEnabled Signed-off-by: Everett Ross <reverett@uber.com> * Add ddg.menuEnabled to types/config Signed-off-by: Everett Ross <reverett@uber.com>
jaegertracing · Dec 19, 2019 · 135ccfe · 135ccfe
1 parent d02881b
commit 135ccfe
Show file tree

Hide file tree

Showing 23 changed files with 450 additions and 159 deletions.
diff --git a/packages/jaeger-ui/src/actions/jaeger-api.js b/packages/jaeger-ui/src/actions/jaeger-api.js
@@ -47,6 +47,12 @@ export const fetchServiceOperations = createAction(
   serviceName => ({ serviceName })
 );
 
+export const fetchServiceServerOps = createAction(
+  '@JAEGER_API/FETCH_SERVICE_SERVER_OP',
+  serviceName => JaegerAPI.fetchServiceServerOps(serviceName),
+  serviceName => ({ serviceName })
+);
+
 export const fetchDeepDependencyGraph = createAction(
   '@JAEGER_API/FETCH_DEEP_DEPENDENCY_GRAPH',
   query => JaegerAPI.fetchDeepDependencyGraph(query),

diff --git a/packages/jaeger-ui/src/actions/jaeger-api.test.js b/packages/jaeger-ui/src/actions/jaeger-api.test.js
@@ -119,8 +119,16 @@ describe('actions/jaeger-api', () => {
     expect(called.verify()).toBeTruthy();
   });
 
-  // Temporary mock used until backend is available, TODO revert & re-enable test
-  xit('@JAEGER_API/FETCH_DEEP_DEPENDENCY_GRAPH should fetch the graph by params', () => {
+  it('@JAEGER_API/FETCH_SERVICE_SERVER_OP should call the JaegerAPI', () => {
+    const called = mock
+      .expects('fetchServiceServerOps')
+      .once()
+      .withExactArgs('service');
+    jaegerApiActions.fetchServiceServerOps('service');
+    expect(called.verify()).toBeTruthy();
+  });
+
+  it('@JAEGER_API/FETCH_DEEP_DEPENDENCY_GRAPH should fetch the graph by params', () => {
     mock.expects('fetchDeepDependencyGraph').withExactArgs(query);
     jaegerApiActions.fetchDeepDependencyGraph(query);
     expect(() => mock.verify()).not.toThrow();

diff --git a/packages/jaeger-ui/src/api/jaeger.js b/packages/jaeger-ui/src/api/jaeger.js
@@ -89,6 +89,14 @@ const JaegerAPI = {
   fetchServiceOperations(serviceName) {
     return getJSON(`${this.apiRoot}services/${encodeURIComponent(serviceName)}/operations`);
   },
+  fetchServiceServerOps(service) {
+    return getJSON(`${this.apiRoot}operations`, {
+      query: {
+        service,
+        spanKind: 'server',
+      },
+    });
+  },
   fetchServices() {
     return getJSON(`${this.apiRoot}services`);
   },

diff --git a/packages/jaeger-ui/src/api/jaeger.test.js b/packages/jaeger-ui/src/api/jaeger.test.js
@@ -81,6 +81,18 @@ describe('fetchDependencies', () => {
   });
 });
 
+describe('fetchServiceServerOps', () => {
+  it('GETs the specified query', () => {
+    const service = 'serviceName';
+    const query = { service, spanKind: 'server' };
+    JaegerAPI.fetchServiceServerOps(service);
+    expect(fetchMock).toHaveBeenLastCalledWith(
+      `${DEFAULT_API_ROOT}operations?${queryString.stringify(query)}`,
+      defaultOptions
+    );
+  });
+});
+
 describe('fetchTrace', () => {
   const generatedTraces = traceGenerator.traces({ numberOfTraces: 5 });
 

diff --git a/packages/jaeger-ui/src/components/App/TopNav.test.js b/packages/jaeger-ui/src/components/App/TopNav.test.js
@@ -16,7 +16,7 @@ import React from 'react';
 import { shallow } from 'enzyme';
 import { Link } from 'react-router-dom';
 
-import { TopNavImpl as TopNav } from './TopNav';
+import { mapStateToProps, TopNavImpl as TopNav } from './TopNav';
 
 describe('<TopNav>', () => {
   const labelGitHub = 'GitHub';
@@ -78,8 +78,8 @@ describe('<TopNav>', () => {
       expect(items.length).toBe(1);
     });
 
-    it('renders the "Dependencies" button', () => {
-      const items = wrapper.find(Link).findWhere(link => /Dependencies/.test(link.text()));
+    it('renders the "System Architecture" button', () => {
+      const items = wrapper.find(Link).findWhere(link => /System Architecture/.test(link.text()));
       expect(items.length).toBe(1);
     });
   });
@@ -126,3 +126,10 @@ describe('<TopNav>', () => {
     });
   });
 });
+
+describe('mapStateToProps', () => {
+  it('returns entire state', () => {
+    const testState = {};
+    expect(mapStateToProps(testState)).toBe(testState);
+  });
+});
diff --git a/packages/jaeger-ui/src/components/App/TopNav.tsx b/packages/jaeger-ui/src/components/App/TopNav.tsx
@@ -19,7 +19,8 @@ import { connect } from 'react-redux';
 import { RouteComponentProps, Link, withRouter } from 'react-router-dom';
 
 import TraceIDSearchInput from './TraceIDSearchInput';
-import * as dependencies from '../DependencyGraph/url';
+import * as dependencyGraph from '../DependencyGraph/url';
+import * as deepDependencies from '../DeepDependencies/url';
 import * as searchUrl from '../SearchTracePage/url';
 import * as diffUrl from '../TraceDiff/url';
 import { ReduxState } from '../../types';
@@ -44,9 +45,17 @@ const NAV_LINKS = [
 
 if (getConfigValue('dependencies.menuEnabled')) {
   NAV_LINKS.push({
-    to: dependencies.getUrl(),
-    matches: dependencies.matches,
-    text: 'Dependencies',
+    to: dependencyGraph.getUrl(),
+    matches: dependencyGraph.matches,
+    text: 'System Architecture',
+  });
+}
+
+if (getConfigValue('deepDependencies.menuEnabled')) {
+  NAV_LINKS.push({
+    to: deepDependencies.getUrl(),
+    matches: deepDependencies.matches,
+    text: 'Service Dependencies',
   });
 }
 
@@ -117,7 +126,8 @@ export function TopNavImpl(props: Props) {
 
 TopNavImpl.CustomNavDropdown = CustomNavDropdown;
 
-function mapStateToProps(state: ReduxState) {
+// export for tests
+export function mapStateToProps(state: ReduxState) {
   return state;
 }
 

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/LayoutSettings/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/LayoutSettings/index.tsx
@@ -43,7 +43,7 @@ export const densityOptions = [
     title: 'One node per resource',
     note: 'Most conscise',
     description:
-      "This setting represents each resource one time in the graph, regardless of whether or not it's upstream or downstream of the focal node. This results in the most desnse graph layout, possible.",
+      "This setting represents each resource one time in the graph, regardless of whether or not it's upstream or downstream of the focal node. This results in the most dense graph layout, possible.",
   },
   {
     option: EDdgDensity.UpstreamVsDownstream,
@@ -104,7 +104,6 @@ export default class LayoutSettings extends React.PureComponent<TProps> {
                   <h4>Show operations</h4>
                   <p>
                     Controls whether or not the operations are considered when creating nodes for the graph.
-                    Note: The operation of the focal node is always shown.
                   </p>
                 </div>
               </Checkbox>

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx
@@ -120,7 +120,6 @@ export default class Header extends React.PureComponent<TProps> {
       services,
       setDensity,
       setDistance,
-      setOperation,
       setService,
       showOperations,
       showParameters,

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js
@@ -37,7 +37,7 @@ describe('DeepDependencyGraphPage', () => {
       addViewModifier: jest.fn(),
       fetchDeepDependencyGraph: () => {},
       fetchServices: jest.fn(),
-      fetchServiceOperations: jest.fn(),
+      fetchServiceServerOps: jest.fn(),
       graphState: {
         model: {
           distanceToPathElems: new Map(),
@@ -48,7 +48,7 @@ describe('DeepDependencyGraphPage', () => {
       history: {
         push: jest.fn(),
       },
-      operationsForService: {},
+      serverOpsForService: {},
       removeViewModifierFromIndices: jest.fn(),
       urlState: {
         start: 'testStart',
@@ -81,7 +81,7 @@ describe('DeepDependencyGraphPage', () => {
     describe('constructor', () => {
       beforeEach(() => {
         props.fetchServices.mockReset();
-        props.fetchServiceOperations.mockReset();
+        props.fetchServiceServerOps.mockReset();
       });
 
       it('fetches services if services are not provided', () => {
@@ -94,12 +94,12 @@ describe('DeepDependencyGraphPage', () => {
       it('fetches operations if service is provided without operations', () => {
         const { service, ...urlState } = props.urlState;
         new DeepDependencyGraphPageImpl({ ...props, urlState }); // eslint-disable-line no-new
-        expect(props.fetchServiceOperations).not.toHaveBeenCalled();
-        new DeepDependencyGraphPageImpl({ ...props, operationsForService: { [service]: [] } }); // eslint-disable-line no-new
-        expect(props.fetchServiceOperations).not.toHaveBeenCalled();
+        expect(props.fetchServiceServerOps).not.toHaveBeenCalled();
+        new DeepDependencyGraphPageImpl({ ...props, serverOpsForService: { [service]: [] } }); // eslint-disable-line no-new
+        expect(props.fetchServiceServerOps).not.toHaveBeenCalled();
         new DeepDependencyGraphPageImpl(props); // eslint-disable-line no-new
-        expect(props.fetchServiceOperations).toHaveBeenLastCalledWith(service);
-        expect(props.fetchServiceOperations).toHaveBeenCalledTimes(1);
+        expect(props.fetchServiceServerOps).toHaveBeenLastCalledWith(service);
+        expect(props.fetchServiceServerOps).toHaveBeenCalledTimes(1);
       });
     });
 
@@ -343,7 +343,7 @@ describe('DeepDependencyGraphPage', () => {
         });
 
         beforeEach(() => {
-          props.fetchServiceOperations.mockReset();
+          props.fetchServiceServerOps.mockReset();
           trackSetServiceSpy.mockClear();
         });
 
@@ -359,17 +359,17 @@ describe('DeepDependencyGraphPage', () => {
 
         it('fetches operations for service when not yet provided', () => {
           ddgPageImpl.setService(service);
-          expect(props.fetchServiceOperations).toHaveBeenLastCalledWith(service);
-          expect(props.fetchServiceOperations).toHaveBeenCalledTimes(1);
+          expect(props.fetchServiceServerOps).toHaveBeenLastCalledWith(service);
+          expect(props.fetchServiceServerOps).toHaveBeenCalledTimes(1);
           expect(trackSetServiceSpy).toHaveBeenCalledTimes(1);
 
           const pageWithOpForService = new DeepDependencyGraphPageImpl({
             ...props,
-            operationsForService: { [service]: [props.urlState.operation] },
+            serverOpsForService: { [service]: [props.urlState.operation] },
           });
-          const { length: callCount } = props.fetchServiceOperations.mock.calls;
+          const { length: callCount } = props.fetchServiceServerOps.mock.calls;
           pageWithOpForService.setService(service);
-          expect(props.fetchServiceOperations).toHaveBeenCalledTimes(callCount);
+          expect(props.fetchServiceServerOps).toHaveBeenCalledTimes(callCount);
           expect(trackSetServiceSpy).toHaveBeenCalledTimes(2);
         });
       });
@@ -680,15 +680,15 @@ describe('DeepDependencyGraphPage', () => {
 
       it('passes correct operations to Header', () => {
         const wrapper = shallow(
-          <DeepDependencyGraphPageImpl {...props} graph={graph} operationsForService={undefined} />
+          <DeepDependencyGraphPageImpl {...props} graph={graph} serverOpsForService={undefined} />
         );
         expect(wrapper.find(Header).prop('operations')).toBe(undefined);
 
-        const operationsForService = {
+        const serverOpsForService = {
           [props.urlState.service]: ['testOperation0', 'testOperation1'],
         };
-        wrapper.setProps({ operationsForService });
-        expect(wrapper.find(Header).prop('operations')).toBe(operationsForService[props.urlState.service]);
+        wrapper.setProps({ serverOpsForService });
+        expect(wrapper.find(Header).prop('operations')).toBe(serverOpsForService[props.urlState.service]);
 
         const { service: _, ...urlStateWithoutService } = props.urlState;
         wrapper.setProps({ urlState: urlStateWithoutService });
@@ -703,7 +703,7 @@ describe('DeepDependencyGraphPage', () => {
         addViewModifier: expect.any(Function),
         fetchDeepDependencyGraph: expect.any(Function),
         fetchServices: expect.any(Function),
-        fetchServiceOperations: expect.any(Function),
+        fetchServiceServerOps: expect.any(Function),
         removeViewModifierFromIndices: expect.any(Function),
       });
     });
@@ -724,7 +724,7 @@ describe('DeepDependencyGraphPage', () => {
       },
     };
     const services = [service];
-    const operationsForService = {
+    const serverOpsForService = {
       [service]: ['some operation'],
     };
     const state = {
@@ -735,7 +735,7 @@ describe('DeepDependencyGraphPage', () => {
         },
       },
       services: {
-        operationsForService,
+        serverOpsForService,
         otherState: 'otherState',
         services,
       },
@@ -821,9 +821,9 @@ describe('DeepDependencyGraphPage', () => {
       expect(doneResult.graph).toBe(mockGraph);
     });
 
-    it('includes services and operationsForService', () => {
+    it('includes services and serverOpsForService', () => {
       expect(mapStateToProps(state, ownProps)).toEqual(
-        expect.objectContaining({ operationsForService, services })
+        expect.objectContaining({ serverOpsForService, services })
       );
     });