From 5c57cad5064b6721c86bce580ab5a35bdd19c890 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Mon, 25 Nov 2019 11:24:36 -0500 Subject: [PATCH 1/5] Temp: Default link visible with banner Signed-off-by: Everett Ross --- .../jaeger-ui/src/components/App/TopNav.tsx | 17 ++++++++++--- .../jaeger-ui/src/components/App/index.css | 25 +++++++++++++++++++ .../DeepDependencies/Header/index.tsx | 1 - .../src/components/DeepDependencies/url.tsx | 13 +++++----- .../SearchResults/AltViewOptions.test.js | 6 ++++- .../SearchResults/AltViewOptions.tsx | 21 +++++++++++++++- .../SearchResults/index.track.tsx | 5 ++++ .../src/constants/default-config.tsx | 4 +++ 8 files changed, 79 insertions(+), 13 deletions(-) diff --git a/packages/jaeger-ui/src/components/App/TopNav.tsx b/packages/jaeger-ui/src/components/App/TopNav.tsx index 168273e923..578c5fd704 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', }); } diff --git a/packages/jaeger-ui/src/components/App/index.css b/packages/jaeger-ui/src/components/App/index.css index 430743cf06..23b57889de 100644 --- a/packages/jaeger-ui/src/components/App/index.css +++ b/packages/jaeger-ui/src/components/App/index.css @@ -27,3 +27,28 @@ a { a:hover { color: #00474e; } + +/* TODO: Delete to end of file */ +a[href='/deep-dependencies'] { + overflow: hidden; + padding-right: 40px; + position: relative; +} + +.ant-menu-horizontal > li:last-child { + padding-right: 0px; +} + +a[href='/deep-dependencies']:after { + content: 'New!'; + background-color: red; + height: 60px; + overflow: hidden; + padding-left: 30px; + padding-top: 25px; + position: absolute; + right: -50px; + top: -25px; + transform: rotateZ(45deg); + width: 100px; +} 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/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..7f5e4e2cbd 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js @@ -31,7 +31,11 @@ describe('AltViewOptions', () => { }); it('renders correct label', () => { - const getLabel = () => wrapper.find(Button).prop('children'); + const getLabel = () => + wrapper + .find(Button) + .first() + .prop('children'); expect(getLabel()).toBe('Deep Dependency Graph'); wrapper.setProps({ traceResultsView: false }); 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/index.track.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.track.tsx index ec9d2fae01..c8c9a090a8 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.track.tsx +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.track.tsx @@ -16,6 +16,7 @@ import { trackEvent } from '../../../utils/tracking'; // export for tests export const CATEGORY_ALT_VIEW = 'jaeger/ux/search-results/alt-view'; +export const CATEGORY_CONVERSIONS = 'jaeger/ux/search-results/conversions'; export enum EAltViewActions { Ddg = 'ddg', @@ -25,3 +26,7 @@ export enum EAltViewActions { export function trackAltView(view: EAltViewActions) { trackEvent(CATEGORY_ALT_VIEW, view); } + +export function trackConversions(view: EAltViewActions) { + trackEvent(CATEGORY_CONVERSIONS, view); +} diff --git a/packages/jaeger-ui/src/constants/default-config.tsx b/packages/jaeger-ui/src/constants/default-config.tsx index a7b332cb89..e67bc26bed 100644 --- a/packages/jaeger-ui/src/constants/default-config.tsx +++ b/packages/jaeger-ui/src/constants/default-config.tsx @@ -20,6 +20,10 @@ export default deepFreeze( Object.defineProperty( { archiveEnabled: false, + // TODO: remove default + deepDependencies: { + menuEnabled: true, + }, dependencies: { dagMaxNumServices: FALLBACK_DAG_MAX_NUM_SERVICES, menuEnabled: true, From 5e06f6d165a3e47e2a9529d48195d9dcdefd7e61 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Tue, 3 Dec 2019 13:37:03 -0500 Subject: [PATCH 2/5] WIP: Use server ops in ddg, track conv&collapse Signed-off-by: Everett Ross --- packages/jaeger-ui/src/actions/jaeger-api.js | 6 ++++ packages/jaeger-ui/src/api/jaeger.js | 8 +++++ .../Header/LayoutSettings/index.tsx | 2 +- .../components/DeepDependencies/index.test.js | 8 ++--- .../src/components/DeepDependencies/index.tsx | 11 +++--- .../SearchResults/ResultItem.tsx | 4 ++- .../SearchResults/ResultItemTitle.tsx | 20 ++++++----- .../ResultItemTitle.test.js.snap | 1 + .../TraceTimelineViewer/duck.track.test.js | 4 +++ .../TraceTimelineViewer/duck.track.tsx | 36 ++++++++++++------- packages/jaeger-ui/src/reducers/services.js | 35 +++++++++++++++--- .../jaeger-ui/src/reducers/services.test.js | 36 ++++++++++--------- packages/jaeger-ui/src/types/index.tsx | 1 + 13 files changed, 119 insertions(+), 53 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/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/components/DeepDependencies/Header/LayoutSettings/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/LayoutSettings/index.tsx index d2c4ef6dbf..a9a7df72e7 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, diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js index 44dd3dcd48..1d427621bf 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js @@ -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({ operationsForService: serverOpsForService, services }) ); }); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx index 59d739bbca..b941e65719 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx @@ -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: operationsForService } = stServices; const urlState = getUrlState(ownProps.location.search); const { density, operation, service, showOp: urlStateShowOp } = urlState; const showOp = urlStateShowOp !== undefined ? urlStateShowOp : operation !== undefined; @@ -354,10 +354,11 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP // export for tests export function mapDispatchToProps(dispatch: Dispatch): TDispatchProps { - const { fetchDeepDependencyGraph, fetchServiceOperations, fetchServices } = bindActionCreators( - jaegerApiActions, - dispatch - ); + const { + fetchDeepDependencyGraph, + fetchServiceServerOps: fetchServiceOperations, + fetchServices, + } = bindActionCreators(jaegerApiActions, dispatch); const { addViewModifier, removeViewModifierFromIndices } = bindActionCreators(ddgActions, dispatch); return { diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx index 2d1ff6f903..3993381d17 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 */} { 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..6b5a9f329b 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, + fetchServiceServerOps as fetchServerOps, + fetchServiceOperations as fetchOps, +} 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,25 @@ function fetchServicesErred(state, { payload: error }) { return { ...state, error, loading: false, services: [] }; } +function fetchServerOpsStarted(state, { meta: { serviceName } }) { + const serverOpForService = { + ...state.operationsForService, + [serviceName]: [], + }; + return { ...state, serverOpForService }; +} + +function fetchServerOpsDone(state, { meta: { serviceName }, payload }) { + const { data: serverOpStructs } = payload; + 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 +91,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..4092084500 100644 --- a/packages/jaeger-ui/src/reducers/services.test.js +++ b/packages/jaeger-ui/src/reducers/services.test.js @@ -19,10 +19,11 @@ const initialState = serviceReducer(undefined, {}); function verifyInitialState() { expect(initialState).toEqual({ - services: null, - loading: false, error: null, + loading: false, operationsForService: {}, + serverOpsForService: {}, + services: null, }); } @@ -35,19 +36,21 @@ it('#92 - ensures services is at least an empty array', () => { type: `${fetchServices}_FULFILLED`, payload: { data: services }, }); - expect(state).toEqual({ + const expected = { + ...initialState, services: [], - operationsForService: {}, - loading: false, - error: null, - }); + }; + expect(state).toEqual(expected); }); it('should handle a fetch services with loading state', () => { const state = serviceReducer(initialState, { type: `${fetchServices}_PENDING`, }); - const expected = { ...initialState, loading: true }; + const expected = { + ...initialState, + loading: true, + }; expect(state).toEqual(expected); }); @@ -57,12 +60,11 @@ it('should handle successful services fetch', () => { type: `${fetchServices}_FULFILLED`, payload: { data: services.slice() }, }); - expect(state).toEqual({ + const expected = { + ...initialState, services, - operationsForService: {}, - loading: false, - error: null, - }); + }; + expect(state).toEqual(expected); }); it('should handle a failed services fetch', () => { @@ -71,12 +73,12 @@ it('should handle a failed services fetch', () => { type: `${fetchServices}_REJECTED`, payload: error, }); - expect(state).toEqual({ + const expected = { + ...initialState, error, services: [], - operationsForService: {}, - loading: false, - }); + }; + expect(state).toEqual(expected); }); it('should handle a successful fetching operations for a service ', () => { 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; From 1e71bdc4832efec36d32f4f2e5bcb7929b4141f6 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Mon, 16 Dec 2019 15:48:43 -0500 Subject: [PATCH 3/5] Add tests Signed-off-by: Everett Ross --- .../jaeger-ui/src/actions/jaeger-api.test.js | 12 +- packages/jaeger-ui/src/api/jaeger.test.js | 12 + .../src/components/App/TopNav.test.js | 9 +- .../jaeger-ui/src/components/App/TopNav.tsx | 3 +- .../Header/LayoutSettings/index.tsx | 1 - .../components/DeepDependencies/index.test.js | 40 ++-- .../src/components/DeepDependencies/index.tsx | 39 ++-- .../SearchResults/AltViewOptions.test.js | 64 ++++- .../SearchResults/ResultItem.tsx | 2 +- .../TraceTimelineViewer/duck.track.test.js | 43 +++- packages/jaeger-ui/src/reducers/services.js | 7 +- .../jaeger-ui/src/reducers/services.test.js | 221 ++++++++++++------ 12 files changed, 320 insertions(+), 133 deletions(-) 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.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..6743ace855 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'; @@ -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 578c5fd704..411a933e07 100644 --- a/packages/jaeger-ui/src/components/App/TopNav.tsx +++ b/packages/jaeger-ui/src/components/App/TopNav.tsx @@ -126,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 a9a7df72e7..23e261f186 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/LayoutSettings/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/LayoutSettings/index.tsx @@ -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/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js index 1d427621bf..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), }); }); @@ -823,7 +823,7 @@ describe('DeepDependencyGraphPage', () => { it('includes services and serverOpsForService', () => { expect(mapStateToProps(state, ownProps)).toEqual( - expect.objectContaining({ operationsForService: serverOpsForService, 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 b941e65719..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, serverOpsForService: 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,17 +354,16 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP // export for tests export function mapDispatchToProps(dispatch: Dispatch): TDispatchProps { - const { - fetchDeepDependencyGraph, - fetchServiceServerOps: fetchServiceOperations, - fetchServices, - } = bindActionCreators(jaegerApiActions, dispatch); + const { fetchDeepDependencyGraph, fetchServiceServerOps, fetchServices } = bindActionCreators( + jaegerApiActions, + dispatch + ); const { addViewModifier, removeViewModifierFromIndices } = bindActionCreators(ddgActions, dispatch); return { addViewModifier, fetchDeepDependencyGraph, - fetchServiceOperations, + fetchServiceServerOps, fetchServices, removeViewModifierFromIndices, }; 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 7f5e4e2cbd..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,28 +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) - .first() - .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/ResultItem.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx index 3993381d17..3f8f300a9e 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx @@ -58,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 ( -
+
{ 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,15 +117,16 @@ 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( diff --git a/packages/jaeger-ui/src/reducers/services.js b/packages/jaeger-ui/src/reducers/services.js index 6b5a9f329b..733d85eb0d 100644 --- a/packages/jaeger-ui/src/reducers/services.js +++ b/packages/jaeger-ui/src/reducers/services.js @@ -45,15 +45,14 @@ function fetchServicesErred(state, { payload: error }) { } function fetchServerOpsStarted(state, { meta: { serviceName } }) { - const serverOpForService = { + const serverOpsForService = { ...state.operationsForService, [serviceName]: [], }; - return { ...state, serverOpForService }; + return { ...state, serverOpsForService }; } -function fetchServerOpsDone(state, { meta: { serviceName }, payload }) { - const { data: serverOpStructs } = payload; +function fetchServerOpsDone(state, { meta: { serviceName }, payload: { data: serverOpStructs } }) { if (!Array.isArray(serverOpStructs)) return state; const serverOpsForService = { diff --git a/packages/jaeger-ui/src/reducers/services.test.js b/packages/jaeger-ui/src/reducers/services.test.js index 4092084500..754895bebe 100644 --- a/packages/jaeger-ui/src/reducers/services.test.js +++ b/packages/jaeger-ui/src/reducers/services.test.js @@ -12,87 +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({ - error: null, - loading: false, - operationsForService: {}, - serverOpsForService: {}, - services: null, +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); }); - const expected = { - ...initialState, - 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 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); }); - 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 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, - services, - }; - 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 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); }); - const expected = { - ...initialState, - error, - services: [], - }; - expect(state).toEqual(expected); -}); -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 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 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); + }); + + 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 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); }); From 00a51f62bbd88e175edca6fa5285223f3de6112c Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Wed, 18 Dec 2019 17:30:01 -0500 Subject: [PATCH 4/5] Add disclaimers for (near) empty graph&adblocker Signed-off-by: Everett Ross --- .../src/components/DeepDependencies/index.css | 6 + .../components/DeepDependencies/index.test.js | 150 +++++++++++++++--- .../src/components/DeepDependencies/index.tsx | 92 ++++++++--- 3 files changed, 201 insertions(+), 47 deletions(-) diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.css b/packages/jaeger-ui/src/components/DeepDependencies/index.css index 34ab2c82e4..2f99030ed9 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.css +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.css @@ -23,6 +23,12 @@ limitations under the License. right: 0; } +.Ddg--center { + margin: auto; + text-align: center; + max-width: 60%; +} + .Ddg--graphWrapper { flex: 1; overflow: hidden; diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js index 7baa1d78fa..67a6ca24c2 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js @@ -23,10 +23,12 @@ import Graph from './Graph'; import Header from './Header'; import ErrorMessage from '../common/ErrorMessage'; import LoadingIndicator from '../common/LoadingIndicator'; +import * as getSearchUrl from '../SearchTracePage/url'; import { fetchedState } from '../../constants'; import getStateEntryKey from '../../model/ddg/getStateEntryKey'; import * as GraphModel from '../../model/ddg/GraphModel'; import * as codec from '../../model/ddg/visibility-codec'; +import * as getConfig from '../../utils/config/get-config'; import { ECheckedStatus, EDirection, EDdgDensity, EViewModifier } from '../../model/ddg/types'; @@ -74,6 +76,7 @@ describe('DeepDependencyGraphPage', () => { getVisWithUpdatedGeneration: jest.fn(), }, }; + const { operation: _o, ...urlStateWithoutOp } = props.urlState; const ddgPageImpl = new DeepDependencyGraphPageImpl(props); const ddgWithoutGraph = new DeepDependencyGraphPageImpl(propsWithoutGraph); const setIdx = visibilityIdx => ({ visibilityIdx }); @@ -187,7 +190,6 @@ describe('DeepDependencyGraphPage', () => { it('removes op from urlState', () => { ddgPageImpl.clearOperation(); - const { operation: _o, ...urlStateWithoutOp } = props.urlState; expect(getUrlSpy).toHaveBeenLastCalledWith(urlStateWithoutOp, undefined); expect(trackClearOperationSpy).toHaveBeenCalledTimes(1); }); @@ -596,25 +598,27 @@ describe('DeepDependencyGraphPage', () => { describe('render', () => { const vertices = [{ key: 'key0' }, { key: 'key1' }, { key: 'key2' }]; + const visibleFindCount = vertices.length - 1; const graph = { - getVisible: () => ({ - edges: [ - { - from: vertices[0].key, - to: vertices[1].key, - }, - { - from: vertices[1].key, - to: vertices[2].key, - }, - ], - vertices, - }), + getVisible: jest.fn(), getDerivedViewModifiers: () => ({ edges: new Map(), vertices: new Map() }), - getHiddenUiFindMatches: () => new Set(vertices.slice(1)), - getVisibleUiFindMatches: () => new Set(vertices.slice(0, 1)), + getHiddenUiFindMatches: () => new Set(vertices.slice(visibleFindCount)), + getVisibleUiFindMatches: () => new Set(vertices.slice(0, visibleFindCount)), getVisibleIndices: () => new Set(), }; + graph.getVisible.mockReturnValue({ + edges: [ + { + from: vertices[0].key, + to: vertices[1].key, + }, + { + from: vertices[1].key, + to: vertices[2].key, + }, + ], + vertices, + }); it('renders message to query a ddg when no graphState is provided', () => { const message = shallow() @@ -639,9 +643,111 @@ describe('DeepDependencyGraphPage', () => { expect(errorComponent.prop('error')).toBe(error); }); - it('renders graph when done', () => { - const wrapper = shallow(); - expect(wrapper.find(Graph)).toHaveLength(1); + describe('graphState.state === fetchedState.DONE', () => { + function makeGraphState(specifiedDistance, vertexCount = 1) { + graph.getVisible.mockReturnValueOnce({ + edges: [], + vertices: vertices.slice(vertices.length - vertexCount), + }); + return { + graphState: { + ...props.graphState, + model: { + ...props.graphState.model, + distanceToPathElems: specifiedDistance + ? new Map([[specifiedDistance, 'test elem']]) + : new Map(), + }, + }, + }; + } + let getConfigValueSpy; + let getSearchUrlSpy; + let wrapper; + + beforeAll(() => { + getConfigValueSpy = jest.spyOn(getConfig, 'getConfigValue'); + getSearchUrlSpy = jest.spyOn(getSearchUrl, 'getUrl'); + }); + + beforeEach(() => { + getConfigValueSpy.mockClear(); + getSearchUrlSpy.mockClear(); + wrapper = shallow(); + }); + + it('renders graph if there are multiple vertices visible', () => { + const graphComponent = wrapper.find(Graph); + + expect(graphComponent).toHaveLength(1); + expect(graphComponent.prop('vertices')).toBe(vertices); + }); + + it('renders disclaimer to show more hops if one or fewer vertices are visible and more hops were in paylaod', () => { + const expectedHeader = 'There is nothing visible to show'; + const expectedInstruction = 'Select at least one hop to view'; + expect(wrapper.find(Graph)).toHaveLength(1); + + wrapper.setProps(makeGraphState(1)); + expect(wrapper.find(Graph)).toHaveLength(0); + expect(wrapper.find('h1.Ddg--center').text()).toBe(expectedHeader); + expect(wrapper.find('p.Ddg--center').text()).toBe(expectedInstruction); + + wrapper.setProps(makeGraphState(-1, 0)); + expect(wrapper.find(Graph)).toHaveLength(0); + expect(wrapper.find('h1.Ddg--center').text()).toBe(expectedHeader); + expect(wrapper.find('p.Ddg--center').text()).toBe(expectedInstruction); + }); + + it('renders disclaimer that service has no known dependencies with correct link to verify', () => { + const expectedHeader = 'There are no dependencies'; + const { operation, service } = props.urlState; + const expectedInstruction = (withOp = true) => + `No traces were found that contain ${service}${ + withOp ? `:${operation}` : '' + } and any other service where span.kind is ‘server’.`; + const lookback = 'test look back'; + getConfigValueSpy.mockReturnValue(lookback); + const mockUrl = 'test search url'; + getSearchUrlSpy.mockReturnValue(mockUrl); + + expect(wrapper.find(Graph)).toHaveLength(1); + + wrapper.setProps(makeGraphState()); + expect(wrapper.find(Graph)).toHaveLength(0); + expect(wrapper.find('h1.Ddg--center').text()).toBe(expectedHeader); + expect( + wrapper + .find('p.Ddg--center') + .first() + .text() + ).toBe(expectedInstruction()); + expect(wrapper.find('a').prop('href')).toBe(mockUrl); + expect(getSearchUrlSpy).toHaveBeenLastCalledWith({ + lookback, + minDuration: '0ms', + operation, + service, + tags: '{"span.kind":"server"}', + }); + + wrapper.setProps({ urlState: urlStateWithoutOp, ...makeGraphState() }); + expect(wrapper.find(Graph)).toHaveLength(0); + expect(wrapper.find('h1.Ddg--center').text()).toBe(expectedHeader); + expect( + wrapper + .find('p.Ddg--center') + .first() + .text() + ).toBe(expectedInstruction(false)); + expect(wrapper.find('a').prop('href')).toBe(mockUrl); + expect(getSearchUrlSpy).toHaveBeenLastCalledWith({ + lookback, + minDuration: '0ms', + service, + tags: '{"span.kind":"server"}', + }); + }); }); it('renders indication of unknown graphState', () => { @@ -674,8 +780,10 @@ describe('DeepDependencyGraphPage', () => { expect(wrapper.find(Header).prop('hiddenUiFindMatches')).toBe(undefined); wrapper.setProps({ graph }); - expect(wrapper.find(Header).prop('uiFindCount')).toBe(1); - expect(wrapper.find(Header).prop('hiddenUiFindMatches').size).toBe(vertices.length - 1); + expect(wrapper.find(Header).prop('uiFindCount')).toBe(visibleFindCount); + expect(wrapper.find(Header).prop('hiddenUiFindMatches').size).toBe( + vertices.length - visibleFindCount + ); }); it('passes correct operations to Header', () => { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx index bd33a37b3b..19bf5f30be 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx @@ -25,6 +25,7 @@ import { getUrl, getUrlState, sanitizeUrlState, ROUTE_PATH } from './url'; import ErrorMessage from '../common/ErrorMessage'; import LoadingIndicator from '../common/LoadingIndicator'; import { extractUiFindFromState, TExtractUiFindFromStateReturn } from '../common/UiFindInput'; +import { getUrl as getSearchUrl } from '../SearchTracePage/url'; import ddgActions from '../../actions/ddg'; import * as jaegerApiActions from '../../actions/jaeger-api'; import { fetchedState } from '../../constants'; @@ -39,6 +40,7 @@ import { TDdgSparseUrlState, } from '../../model/ddg/types'; import { encode, encodeDistance } from '../../model/ddg/visibility-codec'; +import { getConfigValue } from '../../utils/config/get-config'; import { ReduxState } from '../../types'; import { TDdgStateEntry } from '../../types/TDdgState'; @@ -262,37 +264,75 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { visEncoding, viewModifiers ); - // TODO: using `key` here is a hack, debug digraph to fix the underlying issue - content = ( - - ); + if (vertices.length > 1) { + // TODO: using `key` here is a hack, debug digraph to fix the underlying issue + content = ( + + ); + } else if ( + graphState.model.distanceToPathElems.has(-1) || + graphState.model.distanceToPathElems.has(1) + ) { + content = ( + <> +

There is nothing visible to show

+

Select at least one hop to view

+ + ); + } else { + const lookback = getConfigValue('search.maxLookback.value'); + const checkLink = getSearchUrl({ + lookback, + minDuration: '0ms', + operation, + service, + tags: '{"span.kind":"server"}', + }); + content = ( + <> +

There are no dependencies

+

+ No traces were found that contain {service} + {operation && `:${operation}`} and any other service where span.kind is ‘server’. +

+

+ Confirm by searching +

+ + ); + } } else if (graphState.state === fetchedState.LOADING) { content = ; } else if (graphState.state === fetchedState.ERROR) { - content = ; + content = ( + <> + +

If you are using an adblocker, whitelist Jaeger and retry.

+ + ); } else { content = ( -
-

Unknown graphState:

-

${JSON.stringify(graphState)}

-
+ <> +

Unknown graphState:

+

{JSON.stringify(graphState, null, 2)}

+ ); } From 0be2f7acff16c01a739a363c09b422b473b2245e Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Thu, 19 Dec 2019 16:51:28 -0500 Subject: [PATCH 5/5] Remove default enabled Signed-off-by: Everett Ross --- .../jaeger-ui/src/components/App/index.css | 25 ------------------- .../src/constants/default-config.tsx | 4 --- 2 files changed, 29 deletions(-) diff --git a/packages/jaeger-ui/src/components/App/index.css b/packages/jaeger-ui/src/components/App/index.css index 23b57889de..430743cf06 100644 --- a/packages/jaeger-ui/src/components/App/index.css +++ b/packages/jaeger-ui/src/components/App/index.css @@ -27,28 +27,3 @@ a { a:hover { color: #00474e; } - -/* TODO: Delete to end of file */ -a[href='/deep-dependencies'] { - overflow: hidden; - padding-right: 40px; - position: relative; -} - -.ant-menu-horizontal > li:last-child { - padding-right: 0px; -} - -a[href='/deep-dependencies']:after { - content: 'New!'; - background-color: red; - height: 60px; - overflow: hidden; - padding-left: 30px; - padding-top: 25px; - position: absolute; - right: -50px; - top: -25px; - transform: rotateZ(45deg); - width: 100px; -} diff --git a/packages/jaeger-ui/src/constants/default-config.tsx b/packages/jaeger-ui/src/constants/default-config.tsx index e67bc26bed..a7b332cb89 100644 --- a/packages/jaeger-ui/src/constants/default-config.tsx +++ b/packages/jaeger-ui/src/constants/default-config.tsx @@ -20,10 +20,6 @@ export default deepFreeze( Object.defineProperty( { archiveEnabled: false, - // TODO: remove default - deepDependencies: { - menuEnabled: true, - }, dependencies: { dagMaxNumServices: FALLBACK_DAG_MAX_NUM_SERVICES, menuEnabled: true,