From 135ccfe95f0fc5f65573f49808dd6e34d9739a2b Mon Sep 17 00:00:00 2001 From: Everett Date: Thu, 19 Dec 2019 14:53:54 -0500 Subject: [PATCH] Add ddg menu item, fetch server ops, expand GA cov (#501) * Temp: Default link visible with banner Signed-off-by: Everett Ross * WIP: Use server ops in ddg, track conv&collapse Signed-off-by: Everett Ross * Add tests Signed-off-by: Everett Ross * Remove DDG announcement banner Signed-off-by: Everett Ross * Remove default ddg.menuEnabled Signed-off-by: Everett Ross * Add ddg.menuEnabled to types/config Signed-off-by: Everett Ross --- packages/jaeger-ui/src/actions/jaeger-api.js | 6 + .../jaeger-ui/src/actions/jaeger-api.test.js | 12 +- packages/jaeger-ui/src/api/jaeger.js | 8 + packages/jaeger-ui/src/api/jaeger.test.js | 12 + .../src/components/App/TopNav.test.js | 13 +- .../jaeger-ui/src/components/App/TopNav.tsx | 20 +- .../Header/LayoutSettings/index.tsx | 3 +- .../DeepDependencies/Header/index.tsx | 1 - .../components/DeepDependencies/index.test.js | 46 ++-- .../src/components/DeepDependencies/index.tsx | 32 +-- .../src/components/DeepDependencies/url.tsx | 13 +- .../SearchResults/AltViewOptions.test.js | 60 ++++- .../SearchResults/AltViewOptions.tsx | 21 +- .../SearchResults/ResultItem.tsx | 4 +- .../SearchResults/ResultItemTitle.tsx | 20 +- .../ResultItemTitle.test.js.snap | 1 + .../SearchResults/index.track.tsx | 5 + .../TraceTimelineViewer/duck.track.test.js | 47 +++- .../TraceTimelineViewer/duck.track.tsx | 36 +-- packages/jaeger-ui/src/reducers/services.js | 34 ++- .../jaeger-ui/src/reducers/services.test.js | 213 +++++++++++++----- packages/jaeger-ui/src/types/config.tsx | 1 + packages/jaeger-ui/src/types/index.tsx | 1 + 23 files changed, 450 insertions(+), 159 deletions(-) diff --git a/packages/jaeger-ui/src/actions/jaeger-api.js b/packages/jaeger-ui/src/actions/jaeger-api.js index fd5ce198fe..8bde3b8baa 100644 --- 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 index 71bab795b0..602ab2edd2 100644 --- 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 index 9105d27d5b..a1a115dd96 100644 --- 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 index dd8263b226..fc7a57b6f4 100644 --- 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 index efb569f4bd..ec89e63d96 100644 --- 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('', () => { const labelGitHub = 'GitHub'; @@ -78,8 +78,8 @@ describe('', () => { 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('', () => { }); }); }); + +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 index 168273e923..411a933e07 100644 --- 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 index d2c4ef6dbf..23e261f186 100644 --- 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 {

Show operations

Controls whether or not the operations are considered when creating nodes for the graph. - Note: The operation of the focal node is always shown.

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx index e306a39eda..46947bebbe 100644 --- 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 { 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 index 44dd3dcd48..7baa1d78fa 100644 --- 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( - + ); 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 }) ); }); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx index 59d739bbca..bd33a37b3b 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx @@ -48,7 +48,7 @@ export type TDispatchProps = { addViewModifier?: (kwarg: TDdgModelParams & { viewModifier: number; visibilityIndices: number[] }) => void; fetchDeepDependencyGraph?: (query: TDdgModelParams) => void; fetchServices?: () => void; - fetchServiceOperations?: (service: string) => void; + fetchServiceServerOps?: (service: string) => void; removeViewModifierFromIndices?: ( kwarg: TDdgModelParams & { viewModifier: number; visibilityIndices: number[] } ) => void; @@ -57,7 +57,7 @@ export type TDispatchProps = { export type TReduxProps = TExtractUiFindFromStateReturn & { graph: GraphModel | undefined; graphState?: TDdgStateEntry; - operationsForService?: Record; + serverOpsForService?: Record; services?: string[] | null; showOp: boolean; urlState: TDdgSparseUrlState; @@ -92,7 +92,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { super(props); DeepDependencyGraphPageImpl.fetchModelIfStale(props); - const { fetchServices, fetchServiceOperations, operationsForService, services, urlState } = props; + const { fetchServices, fetchServiceServerOps, serverOpsForService, services, urlState } = props; const { service } = urlState; if (!services && fetchServices) { @@ -100,11 +100,11 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { } if ( service && - operationsForService && - !Reflect.has(operationsForService, service) && - fetchServiceOperations + serverOpsForService && + !Reflect.has(serverOpsForService, service) && + fetchServiceServerOps ) { - fetchServiceOperations(service); + fetchServiceServerOps(service); } } @@ -180,9 +180,9 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { }; setService = (service: string) => { - const { fetchServiceOperations, operationsForService } = this.props; - if (operationsForService && !Reflect.has(operationsForService, service) && fetchServiceOperations) { - fetchServiceOperations(service); + const { fetchServiceServerOps, serverOpsForService } = this.props; + if (serverOpsForService && !Reflect.has(serverOpsForService, service) && fetchServiceServerOps) { + fetchServiceServerOps(service); } this.updateUrlState({ operation: undefined, service, visEncoding: undefined }); trackSetService(); @@ -239,7 +239,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { extraUrlArgs, graph, graphState, - operationsForService, + serverOpsForService, services, showOp, uiFind, @@ -305,7 +305,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { distanceToPathElems={distanceToPathElems} hiddenUiFindMatches={hiddenUiFindMatches} operation={operation} - operations={operationsForService && operationsForService[service || '']} + operations={serverOpsForService && serverOpsForService[service || '']} service={service} services={services} setDensity={this.setDensity} @@ -329,7 +329,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { // export for tests export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxProps { const { services: stServices } = state; - const { services, operationsForService } = stServices; + const { services, serverOpsForService } = stServices; const urlState = getUrlState(ownProps.location.search); const { density, operation, service, showOp: urlStateShowOp } = urlState; const showOp = urlStateShowOp !== undefined ? urlStateShowOp : operation !== undefined; @@ -344,7 +344,7 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP return { graph, graphState, - operationsForService, + serverOpsForService, services, showOp, urlState: sanitizeUrlState(urlState, _get(graphState, 'model.hash')), @@ -354,7 +354,7 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP // export for tests export function mapDispatchToProps(dispatch: Dispatch): TDispatchProps { - const { fetchDeepDependencyGraph, fetchServiceOperations, fetchServices } = bindActionCreators( + const { fetchDeepDependencyGraph, fetchServiceServerOps, fetchServices } = bindActionCreators( jaegerApiActions, dispatch ); @@ -363,7 +363,7 @@ export function mapDispatchToProps(dispatch: Dispatch): TDispatchPro return { addViewModifier, fetchDeepDependencyGraph, - fetchServiceOperations, + fetchServiceServerOps, fetchServices, removeViewModifierFromIndices, }; diff --git a/packages/jaeger-ui/src/components/DeepDependencies/url.tsx b/packages/jaeger-ui/src/components/DeepDependencies/url.tsx index 40c615965b..69c25284df 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/url.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/url.tsx @@ -30,12 +30,13 @@ export function matches(path: string) { export function getUrl(args?: { [key: string]: unknown; showOp?: boolean }, baseUrl: string = ROUTE_PATH) { if (args && !_isEmpty(args)) { - const stringifyArgs = Reflect.has(args, 'showOp') - ? { - ...args, - showOp: args.showOp ? 1 : 0, - } - : args; + const stringifyArgs = + Reflect.has(args, 'showOp') && args.showOp !== undefined + ? { + ...args, + showOp: args.showOp ? 1 : 0, + } + : args; return `${baseUrl}?${queryString.stringify(stringifyArgs)}`; } return baseUrl; diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js index 74da688e73..ad8b484a0f 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js @@ -17,24 +17,78 @@ import { shallow } from 'enzyme'; import { Button } from 'antd'; import AltViewOptions from './AltViewOptions'; +import * as url from '../../DeepDependencies/url'; +import * as getConfig from '../../../utils/config/get-config'; describe('AltViewOptions', () => { + let getConfigValueSpy; + let getUrlSpy; + let getUrlStateSpy; + let openSpy; + + let wrapper; + const getBtn = (btnIndex = 0) => wrapper.find(Button).at(btnIndex); + const getLabel = (btnIndex = 0) => getBtn(btnIndex).prop('children'); const props = { traceResultsView: true, onTraceGraphViewClicked: jest.fn(), }; - let wrapper; + + beforeAll(() => { + getUrlSpy = jest.spyOn(url, 'getUrl'); + getUrlStateSpy = jest.spyOn(url, 'getUrlState'); + getConfigValueSpy = jest.spyOn(getConfig, 'getConfigValue'); + openSpy = jest.spyOn(window, 'open').mockImplementation(); + }); beforeEach(() => { - props.onTraceGraphViewClicked.mockClear(); + jest.clearAllMocks(); wrapper = shallow(); }); + afterAll(() => { + openSpy.mockRestore(); + }); + it('renders correct label', () => { - const getLabel = () => wrapper.find(Button).prop('children'); expect(getLabel()).toBe('Deep Dependency Graph'); wrapper.setProps({ traceResultsView: false }); expect(getLabel()).toBe('Trace Results'); }); + + it('renders button to view full ddg iff ddg is enabled and search results are viewed as ddg', () => { + expect(getLabel()).toBe('Deep Dependency Graph'); + expect(wrapper.find(Button).length).toBe(1); + + getConfigValueSpy.mockReturnValue(true); + wrapper.setProps({ traceResultsView: false }); + expect(wrapper.find(Button).length).toBe(2); + expect(getLabel()).toBe('Trace Results'); + expect(getLabel(1)).toBe('View All Dependencies'); + + wrapper.setProps({ traceResultsView: true }); + expect(getLabel()).toBe('Deep Dependency Graph'); + expect(wrapper.find(Button).length).toBe(1); + }); + + it('opens correct ddg url with correct target when view full ddg button is clicked', () => { + const mockUrl = 'test url'; + const mockUrlState = { service: 'serviceName', showOp: true }; + getConfigValueSpy.mockReturnValue(true); + getUrlSpy.mockReturnValue(mockUrl); + getUrlStateSpy.mockReturnValue(mockUrlState); + + wrapper.setProps({ traceResultsView: false }); + const viewDdgBtn = getBtn(1); + viewDdgBtn.simulate('click', {}); + expect(getUrlSpy).toHaveBeenLastCalledWith(mockUrlState); + expect(openSpy).toHaveBeenLastCalledWith(mockUrl, '_self'); + + viewDdgBtn.simulate('click', { ctrlKey: true }); + expect(openSpy).toHaveBeenLastCalledWith(mockUrl, '_blank'); + + viewDdgBtn.simulate('click', { metaKey: true }); + expect(openSpy).toHaveBeenLastCalledWith(mockUrl, '_blank'); + }); }); diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx index d99d9b1248..4d3254952f 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx @@ -15,16 +15,35 @@ import * as React from 'react'; import { Button } from 'antd'; +import { trackConversions, EAltViewActions } from './index.track'; +import { getUrl, getUrlState } from '../../DeepDependencies/url'; +import { getConfigValue } from '../../../utils/config/get-config'; + type Props = { onTraceGraphViewClicked: () => void; traceResultsView: boolean; }; +function viewAllDep({ ctrlKey, metaKey }: React.MouseEvent) { + trackConversions(EAltViewActions.Ddg); + const { density, operation, service, showOp } = getUrlState(window.location.search); + window.open(getUrl({ density, operation, service, showOp }), ctrlKey || metaKey ? '_blank' : '_self'); +} + export default function AltViewOptions(props: Props) { const { onTraceGraphViewClicked, traceResultsView } = props; - return ( + const toggleBtn = ( ); + if (traceResultsView || !getConfigValue('deepDependencies.menuEnabled')) return toggleBtn; + return ( + <> + {toggleBtn} + + + ); } diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx index 2d1ff6f903..3f8f300a9e 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx @@ -19,6 +19,7 @@ import { Link } from 'react-router-dom'; import { sortBy } from 'lodash'; import moment from 'moment'; +import { trackConversions, EAltViewActions } from './index.track'; import * as markers from './ResultItem.markers'; import ResultItemTitle from './ResultItemTitle'; import colorGenerator from '../../../utils/color-generator'; @@ -38,6 +39,7 @@ type Props = { }; const isErrorTag = ({ key, value }: KeyValuePair) => key === 'error' && (value === true || value === 'true'); +const trackTraceConversions = () => trackConversions(EAltViewActions.Traces); export default class ResultItem extends React.PureComponent { render() { @@ -56,7 +58,7 @@ export default class ResultItem extends React.PureComponent { const numSpans = spans.length; const numErredSpans = spans.filter(sp => sp.tags.some(isErrorTag)).length; return ( -
+
evt.stopPropagation(); + export default class ResultItemTitle extends React.PureComponent { static defaultProps: Partial = { disableComparision: false, @@ -80,16 +82,18 @@ export default class ResultItemTitle extends React.PureComponent { } } const isErred = state === fetchedState.ERROR; + // Separate propagation management and toggle manegement due to ant-design#16400 + const checkboxProps = { + className: 'ResultItemTitle--item ub-flex-none', + checked: !isErred && isInDiffCohort, + disabled: isErred, + onChange: this.toggleComparison, + onClick: stopCheckboxPropagation, + }; + return (
- {!disableComparision && ( - - )} + {!disableComparision && } {/* TODO: Shouldn't need cast */} { category: track.CATEGORY_COLUMN, extraTrackArgs: [columnWidth.tracked], }, + { + action: track.ACTION_COLLAPSE_ALL, + category: track.CATEGORY_EXPAND_COLLAPSE, + msg: 'tracks a GA event for collapsing all', + type: types.COLLAPSE_ALL, + }, + { + action: track.ACTION_COLLAPSE_ONE, + category: track.CATEGORY_EXPAND_COLLAPSE, + msg: 'tracks a GA event for collapsing a level', + type: types.COLLAPSE_ONE, + }, { msg: 'tracks a GA event for collapsing a parent', type: types.CHILDREN_TOGGLE, category: track.CATEGORY_PARENT, extraTrackArgs: [123], }, + { + action: track.ACTION_EXPAND_ALL, + category: track.CATEGORY_EXPAND_COLLAPSE, + msg: 'tracks a GA event for expanding all', + type: types.COLLAPSE_ALL, + }, + { + action: track.ACTION_EXPAND_ONE, + category: track.CATEGORY_EXPAND_COLLAPSE, + msg: 'tracks a GA event for expanding a level', + type: types.COLLAPSE_ONE, + }, { msg: 'tracks a GA event for toggling a detail row', type: types.DETAIL_TOGGLE, @@ -93,25 +117,30 @@ describe('middlewareHooks', () => { }, ]; - cases.forEach(_case => { - const { msg, type, category, extraTrackArgs = [], payloadCustom = null } = _case; - it(msg, () => { - const action = { type, payload: payloadCustom || payload }; - track.middlewareHooks[type](store, action); - expect(trackEvent.mock.calls.length).toBe(1); - expect(trackEvent.mock.calls[0]).toEqual([category, expect.any(String), ...extraTrackArgs]); - }); - }); + cases.forEach( + ({ action = expect.any(String), msg, type, category, extraTrackArgs = [], payloadCustom = null }) => { + it(msg, () => { + const reduxAction = { type, payload: payloadCustom || payload }; + track.middlewareHooks[type](store, reduxAction); + expect(trackEvent.mock.calls.length).toBe(1); + expect(trackEvent.mock.calls[0]).toEqual([category, action, ...extraTrackArgs]); + }); + } + ); it('has the correct keys and they refer to functions', () => { expect(Object.keys(track.middlewareHooks).sort()).toEqual( [ types.CHILDREN_TOGGLE, + types.COLLAPSE_ALL, + types.COLLAPSE_ONE, types.DETAIL_TOGGLE, types.DETAIL_TAGS_TOGGLE, types.DETAIL_PROCESS_TOGGLE, types.DETAIL_LOGS_TOGGLE, types.DETAIL_LOG_ITEM_TOGGLE, + types.EXPAND_ALL, + types.EXPAND_ONE, types.SET_SPAN_NAME_COLUMN_WIDTH, ].sort() ); diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.track.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.track.tsx index 2604ee9244..f36821c2bf 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.track.tsx +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/duck.track.tsx @@ -26,16 +26,21 @@ type TSpanIdHooks = { }; const ACTION_RESIZE = 'resize'; +const ACTION_COLLAPSE_ALL = 'collapse-all'; +const ACTION_COLLAPSE_ONE = 'collapse-one'; +const ACTION_EXPAND_ALL = 'expand-all'; +const ACTION_EXPAND_ONE = 'expand-one'; const CATEGORY_BASE = 'jaeger/ux/trace/timeline'; // export for tests -export const CATEGORY_TAGS = `${CATEGORY_BASE}/tags`; -export const CATEGORY_PROCESS = `${CATEGORY_BASE}/process`; +export const CATEGORY_COLUMN = `${CATEGORY_BASE}/column`; +export const CATEGORY_EXPAND_COLLAPSE = `${CATEGORY_BASE}/expand-collapse`; export const CATEGORY_LOGS = `${CATEGORY_BASE}/logs`; export const CATEGORY_LOGS_ITEM = `${CATEGORY_BASE}/logs/item`; -export const CATEGORY_COLUMN = `${CATEGORY_BASE}/column`; export const CATEGORY_PARENT = `${CATEGORY_BASE}/parent`; +export const CATEGORY_PROCESS = `${CATEGORY_BASE}/process`; export const CATEGORY_ROW = `${CATEGORY_BASE}/row`; +export const CATEGORY_TAGS = `${CATEGORY_BASE}/tags`; function getDetail(store: Store, { payload }: Action) { return payload ? store.getState().traceTimeline.detailStates.get(payload.spanID) : undefined; @@ -84,23 +89,28 @@ function trackLogsItem(store: Store, action: Action trackEvent(CATEGORY_LOGS_ITEM, getToggleValue(isOpen)); } -const logs = (detail: DetailState) => trackEvent(CATEGORY_LOGS, getToggleValue(detail.logs.isOpen)); -const process = (detail: DetailState) => trackEvent(CATEGORY_PROCESS, getToggleValue(detail.isProcessOpen)); -const tags = (detail: DetailState) => trackEvent(CATEGORY_TAGS, getToggleValue(detail.isTagsOpen)); -const detailRow = (isOpen: boolean) => trackEvent(CATEGORY_ROW, getToggleValue(isOpen)); -const columnWidth = (_: any, { payload }: Action) => +const trackColumnWidth = (_: any, { payload }: Action) => payload && trackEvent(CATEGORY_COLUMN, ACTION_RESIZE, Math.round(payload.width * 1000)); +const trackDetailRow = (isOpen: boolean) => trackEvent(CATEGORY_ROW, getToggleValue(isOpen)); +const trackLogs = (detail: DetailState) => trackEvent(CATEGORY_LOGS, getToggleValue(detail.logs.isOpen)); +const trackProcess = (detail: DetailState) => + trackEvent(CATEGORY_PROCESS, getToggleValue(detail.isProcessOpen)); +const trackTags = (detail: DetailState) => trackEvent(CATEGORY_TAGS, getToggleValue(detail.isTagsOpen)); const hooks: TSpanIdHooks = { [types.CHILDREN_TOGGLE]: trackParent, - [types.DETAIL_TOGGLE]: (store, action) => detailRow(Boolean(getDetail(store, action))), - [types.DETAIL_TAGS_TOGGLE]: (store, action) => trackDetailState(store, action, tags), - [types.DETAIL_PROCESS_TOGGLE]: (store, action) => trackDetailState(store, action, process), - [types.DETAIL_LOGS_TOGGLE]: (store, action) => trackDetailState(store, action, logs), + [types.DETAIL_TOGGLE]: (store, action) => trackDetailRow(Boolean(getDetail(store, action))), + [types.DETAIL_TAGS_TOGGLE]: (store, action) => trackDetailState(store, action, trackTags), + [types.DETAIL_PROCESS_TOGGLE]: (store, action) => trackDetailState(store, action, trackProcess), + [types.DETAIL_LOGS_TOGGLE]: (store, action) => trackDetailState(store, action, trackLogs), }; export const middlewareHooks = { ...hooks, + [types.COLLAPSE_ALL]: () => trackEvent(CATEGORY_EXPAND_COLLAPSE, ACTION_COLLAPSE_ALL), + [types.COLLAPSE_ONE]: () => trackEvent(CATEGORY_EXPAND_COLLAPSE, ACTION_COLLAPSE_ONE), [types.DETAIL_LOG_ITEM_TOGGLE]: trackLogsItem, - [types.SET_SPAN_NAME_COLUMN_WIDTH]: columnWidth, + [types.EXPAND_ALL]: () => trackEvent(CATEGORY_EXPAND_COLLAPSE, ACTION_EXPAND_ALL), + [types.EXPAND_ONE]: () => trackEvent(CATEGORY_EXPAND_COLLAPSE, ACTION_EXPAND_ONE), + [types.SET_SPAN_NAME_COLUMN_WIDTH]: trackColumnWidth, }; diff --git a/packages/jaeger-ui/src/reducers/services.js b/packages/jaeger-ui/src/reducers/services.js index 9ea8ad370f..608d47cab1 100644 --- a/packages/jaeger-ui/src/reducers/services.js +++ b/packages/jaeger-ui/src/reducers/services.js @@ -14,15 +14,20 @@ import { handleActions } from 'redux-actions'; -import { fetchServices, fetchServiceOperations as fetchOps } from '../actions/jaeger-api'; +import { + fetchServices, + fetchServiceOperations as fetchOps, + fetchServiceServerOps as fetchServerOps, +} from '../actions/jaeger-api'; import { localeStringComparator } from '../utils/sort'; const initialState = { + error: null, + loading: false, + operationsForService: {}, + serverOpsForService: {}, // `services` initial value of `null` indicates they haven't yet been loaded services: null, - operationsForService: {}, - loading: false, - error: null, }; function fetchStarted(state) { @@ -39,6 +44,24 @@ function fetchServicesErred(state, { payload: error }) { return { ...state, error, loading: false, services: [] }; } +function fetchServerOpsStarted(state, { meta: { serviceName } }) { + const serverOpsForService = { + ...state.operationsForService, + [serviceName]: [], + }; + return { ...state, serverOpsForService }; +} + +function fetchServerOpsDone(state, { meta: { serviceName }, payload: { data: serverOpStructs } }) { + if (!Array.isArray(serverOpStructs)) return state; + + const serverOpsForService = { + ...state.operationsForService, + [serviceName]: serverOpStructs.map(({ name }) => name).sort(localeStringComparator), + }; + return { ...state, serverOpsForService }; +} + function fetchOpsStarted(state, { meta: { serviceName } }) { const operationsForService = { ...state.operationsForService, @@ -67,6 +90,9 @@ export default handleActions( [`${fetchServices}_FULFILLED`]: fetchServicesDone, [`${fetchServices}_REJECTED`]: fetchServicesErred, + [`${fetchServerOps}_PENDING`]: fetchServerOpsStarted, + [`${fetchServerOps}_FULFILLED`]: fetchServerOpsDone, + [`${fetchOps}_PENDING`]: fetchOpsStarted, [`${fetchOps}_FULFILLED`]: fetchOpsDone, }, diff --git a/packages/jaeger-ui/src/reducers/services.test.js b/packages/jaeger-ui/src/reducers/services.test.js index 40814e814e..754895bebe 100644 --- a/packages/jaeger-ui/src/reducers/services.test.js +++ b/packages/jaeger-ui/src/reducers/services.test.js @@ -12,85 +12,174 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { fetchServices, fetchServiceOperations } from '../actions/jaeger-api'; +import { fetchServices, fetchServiceOperations, fetchServiceServerOps } from '../actions/jaeger-api'; import serviceReducer from './services'; const initialState = serviceReducer(undefined, {}); -function verifyInitialState() { - expect(initialState).toEqual({ - services: null, - loading: false, - error: null, - operationsForService: {}, +describe('reducers/services', () => { + function verifyInitialState() { + expect(initialState).toEqual({ + error: null, + loading: false, + operationsForService: {}, + serverOpsForService: {}, + services: null, + }); + } + + beforeEach(verifyInitialState); + afterEach(verifyInitialState); + + const ops = ['a', 'b']; + const serviceName = 'test service'; + + it('#92 - ensures services is at least an empty array', () => { + const services = null; + const state = serviceReducer(initialState, { + type: `${fetchServices}_FULFILLED`, + payload: { data: services }, + }); + const expected = { + ...initialState, + services: [], + }; + expect(state).toEqual(expected); }); -} -beforeEach(verifyInitialState); -afterEach(verifyInitialState); + it('should handle a fetch services with loading state', () => { + const state = serviceReducer(initialState, { + type: `${fetchServices}_PENDING`, + }); + const expected = { + ...initialState, + loading: true, + }; + expect(state).toEqual(expected); + }); -it('#92 - ensures services is at least an empty array', () => { - const services = null; - const state = serviceReducer(initialState, { - type: `${fetchServices}_FULFILLED`, - payload: { data: services }, + it('should handle successful services fetch', () => { + const services = ['a', 'b', 'c']; + const state = serviceReducer(initialState, { + type: `${fetchServices}_FULFILLED`, + payload: { data: services.slice() }, + }); + const expected = { + ...initialState, + services, + }; + expect(state).toEqual(expected); }); - expect(state).toEqual({ - services: [], - operationsForService: {}, - loading: false, - error: null, + + it('should handle a failed services fetch', () => { + const error = new Error('some-message'); + const state = serviceReducer(initialState, { + type: `${fetchServices}_REJECTED`, + payload: error, + }); + const expected = { + ...initialState, + error, + services: [], + }; + expect(state).toEqual(expected); }); -}); -it('should handle a fetch services with loading state', () => { - const state = serviceReducer(initialState, { - type: `${fetchServices}_PENDING`, + it('should handle a successful operations for a service fetch', () => { + const state = serviceReducer(initialState, { + type: `${fetchServiceOperations}_FULFILLED`, + meta: { serviceName }, + payload: { data: ops.slice() }, + }); + const expected = { + ...initialState, + operationsForService: { + [serviceName]: ops, + }, + }; + expect(state).toEqual(expected); }); - const expected = { ...initialState, loading: true }; - expect(state).toEqual(expected); -}); -it('should handle successful services fetch', () => { - const services = ['a', 'b', 'c']; - const state = serviceReducer(initialState, { - type: `${fetchServices}_FULFILLED`, - payload: { data: services.slice() }, + it('should handle if successful operations for a service fetch returns non-array', () => { + const state = serviceReducer(initialState, { + type: `${fetchServiceOperations}_FULFILLED`, + meta: { serviceName }, + payload: { data: null }, + }); + const expected = { + ...initialState, + operationsForService: { + [serviceName]: [], + }, + }; + expect(state).toEqual(expected); }); - expect(state).toEqual({ - services, - operationsForService: {}, - loading: false, - error: null, + + it('should handle a pending operations for a service fetch', () => { + const state = serviceReducer( + { + ...initialState, + operationsForService: { + [serviceName]: ops, + }, + }, + { + type: `${fetchServiceOperations}_PENDING`, + meta: { serviceName }, + } + ); + const expected = { + ...initialState, + operationsForService: { + [serviceName]: [], + }, + }; + expect(state).toEqual(expected); }); -}); -it('should handle a failed services fetch', () => { - const error = new Error('some-message'); - const state = serviceReducer(initialState, { - type: `${fetchServices}_REJECTED`, - payload: error, + it('should handle a successful server operations for a service fetch', () => { + const state = serviceReducer(initialState, { + type: `${fetchServiceServerOps}_FULFILLED`, + meta: { serviceName }, + payload: { data: ops.map(name => ({ name })) }, + }); + const expected = { + ...initialState, + serverOpsForService: { + [serviceName]: ops, + }, + }; + expect(state).toEqual(expected); }); - expect(state).toEqual({ - error, - services: [], - operationsForService: {}, - loading: false, + + it('should no-op if successful server operations for a service fetch returns non-array', () => { + const state = serviceReducer(initialState, { + type: `${fetchServiceServerOps}_FULFILLED`, + meta: { serviceName }, + payload: { data: 'invalid data' }, + }); + expect(state).toBe(initialState); }); -}); -it('should handle a successful fetching operations for a service ', () => { - const ops = ['a', 'b']; - const state = serviceReducer(initialState, { - type: `${fetchServiceOperations}_FULFILLED`, - meta: { serviceName: 'serviceA' }, - payload: { data: ops.slice() }, + it('should handle a pending server operations for a service fetch', () => { + const state = serviceReducer( + { + ...initialState, + serverOpsForService: { + [serviceName]: ops, + }, + }, + { + type: `${fetchServiceServerOps}_PENDING`, + meta: { serviceName }, + } + ); + const expected = { + ...initialState, + serverOpsForService: { + [serviceName]: [], + }, + }; + expect(state).toEqual(expected); }); - const expected = { - ...initialState, - operationsForService: { - serviceA: ops, - }, - }; - expect(state).toEqual(expected); }); diff --git a/packages/jaeger-ui/src/types/config.tsx b/packages/jaeger-ui/src/types/config.tsx index 50de4fde77..97c0ef3032 100644 --- a/packages/jaeger-ui/src/types/config.tsx +++ b/packages/jaeger-ui/src/types/config.tsx @@ -39,6 +39,7 @@ export type LinkPatternsConfig = { export type Config = { archiveEnabled?: boolean; + deepDependencies?: { menuEnabled?: boolean }; dependencies?: { dagMaxServicesLen?: number; menuEnabled?: boolean }; menu: (ConfigMenuGroup | ConfigMenuItem)[]; search?: { maxLookback: { label: string; value: string } }; diff --git a/packages/jaeger-ui/src/types/index.tsx b/packages/jaeger-ui/src/types/index.tsx index 8f93ef7dc7..c862732068 100644 --- a/packages/jaeger-ui/src/types/index.tsx +++ b/packages/jaeger-ui/src/types/index.tsx @@ -52,6 +52,7 @@ export type ReduxState = { }; services: { services: (string[]) | TNil; + serverOpsForService: Record; operationsForService: Record; loading: boolean; error: ApiError | TNil;