Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trace quality view & Ddg Decorations #564

Merged
merged 33 commits into from
Apr 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
6ac4dc1
WIP: Action and types for decorations
everett980 Mar 18, 2020
76e8149
Add PAD reducer, fix types, fix year
everett980 Mar 18, 2020
d2747c0
Fix and test reducer, fix types, fix another year
everett980 Mar 18, 2020
21a40d4
Add another pad reducer test
everett980 Mar 18, 2020
da3b0d2
WIP: Begin testing action
everett980 Mar 20, 2020
d11f866
WIP: Finish action tests TODO: Move stringSupplant
everett980 Mar 23, 2020
c619247
Move and test stringSupplant
everett980 Mar 24, 2020
74bc7e0
Cleanup
everett980 Mar 24, 2020
a423550
WIP: Decorate nodes, selector/detail side panel
everett980 Mar 27, 2020
4b2374e
WIP: Style side panel
everett980 Mar 30, 2020
3ed4589
WIP: Continue styling side panel, fetch details in
everett980 Mar 31, 2020
177066b
WIP: Improve TS handling of union of arrays
everett980 Mar 31, 2020
dd64994
WIP: Limit % circle size, update cursor for
everett980 Apr 1, 2020
c98e434
WIP: Handle list, begin overflow management
everett980 Apr 2, 2020
0160f79
WIP: Manage overflow, begin handling styled values
everett980 Apr 2, 2020
10c2e41
WIP: Handle styled values, render loading&err,
everett980 Apr 2, 2020
ecc8dd0
Add info modal, begin clean up TODO clean up&test
everett980 Apr 2, 2020
9d631db
Merge branch 'jaegertracingmaster' into ddg-pad-side-panel
everett980 Apr 2, 2020
276f638
Fix: rowKeys, setViewModifier argument name,
everett980 Apr 6, 2020
1856a14
Handle linked cells, fix cell sort order
everett980 Apr 7, 2020
0d9dc94
Test existing files, track decorations viewed,
everett980 Apr 7, 2020
d6ddfb3
Test SidePanel/ index&track WIP test DetailsPanel
everett980 Apr 8, 2020
412c050
WIP test DetailsPanel
everett980 Apr 8, 2020
25b5b06
Finish DetailsPanel tests
everett980 Apr 9, 2020
e41e112
Clean up
everett980 Apr 9, 2020
ca8c7fb
Add skeleton components and fetch quality metrics
everett980 Apr 9, 2020
140014c
WIP: Render all data and dropdowns except banner
everett980 Apr 13, 2020
e789c4c
WIP: Style components, implement lookback
everett980 Apr 14, 2020
0928275
Debounce InputNumber, limit search url length, add
everett980 Apr 15, 2020
52f48d7
Cleanup
everett980 Apr 15, 2020
af66ff9
Add support for decoration links
everett980 Apr 24, 2020
dc28540
Clean up and add quality-metrics top nav link
everett980 Apr 24, 2020
60eaca9
Merge branch 'master' into trace-quality-view
everett980 Apr 24, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/jaeger-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
"query-string": "^6.3.0",
"raven-js": "^3.22.1",
"react": "^16.3.2",
"react-circular-progressbar": "^2.0.3",
"react-dimensions": "^1.3.0",
"react-dom": "^16.3.2",
"react-ga": "^2.4.1",
Expand Down
82 changes: 42 additions & 40 deletions packages/jaeger-ui/src/actions/path-agnostic-decorations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,23 @@

import _set from 'lodash/set';

import { processed, getDecoration as getDecorationImpl } from './path-agnostic-decorations';
import { _processed, getDecoration as getDecorationImpl } from './path-agnostic-decorations';
import * as getConfig from '../utils/config/get-config';
import stringSupplant from '../utils/stringSupplant';
import JaegerAPI from '../api/jaeger';

jest.mock('lru-memoize', () => () => x => x);

describe('getDecoration', () => {
let getConfigValueSpy;
let fetchDecorationSpy;
let resolves;
let rejects;

const opUrl = 'opUrl?service=#service&operation=#operation';
const url = 'opUrl?service=#service';
const valuePath = 'withoutOpPath.#service';
const opValuePath = 'opPath.#service.#operation';
const opSummaryUrl = 'opSummaryUrl?service=#service&operation=#operation';
const summaryUrl = 'summaryUrl?service=#service';
const summaryPath = 'withoutOpPath.#service';
const opSummaryPath = 'opPath.#service.#operation';
const withOpID = 'decoration id with op url and op path';
const partialID = 'decoration id with op url without op path';
const withoutOpID = 'decoration id with only url';
Expand All @@ -48,21 +50,21 @@ describe('getDecoration', () => {
getConfigValueSpy = jest.spyOn(getConfig, 'getConfigValue').mockReturnValue([
{
id: withOpID,
url,
opUrl,
valuePath,
opValuePath,
summaryUrl,
opSummaryUrl,
summaryPath,
opSummaryPath,
},
{
id: partialID,
url,
opUrl,
valuePath,
summaryUrl,
opSummaryUrl,
summaryPath,
},
{
id: withoutOpID,
url,
valuePath,
summaryUrl,
summaryPath,
},
]);
fetchDecorationSpy = jest.spyOn(JaegerAPI, 'fetchDecoration').mockImplementation(
Expand All @@ -76,7 +78,7 @@ describe('getDecoration', () => {

beforeEach(() => {
fetchDecorationSpy.mockClear();
processed.clear();
_processed.clear();
resolves = [];
rejects = [];
});
Expand All @@ -102,42 +104,42 @@ describe('getDecoration', () => {

it('resolves to include single response for op decoration given op', async () => {
const promise = getDecoration(withOpID, service, operation);
resolves[0](_set({}, stringSupplant(opValuePath, { service, operation }), testVal));
resolves[0](_set({}, stringSupplant(opSummaryPath, { service, operation }), testVal));
const res = await promise;
expect(res).toEqual(_set({}, `${withOpID}.withOp.${service}.${operation}`, testVal));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(opUrl, { service, operation }));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(opSummaryUrl, { service, operation }));
});

it('resolves to include single response for op decoration not given op', async () => {
const promise = getDecoration(withOpID, service);
resolves[0](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[0](_set({}, stringSupplant(summaryPath, { service }), testVal));
const res = await promise;
expect(res).toEqual(_set({}, `${withOpID}.withoutOp.${service}`, testVal));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(url, { service }));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(summaryUrl, { service }));
});

it('resolves to include single response for malformed op decoration given op', async () => {
const promise = getDecoration(partialID, service, operation);
resolves[0](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[0](_set({}, stringSupplant(summaryPath, { service }), testVal));
const res = await promise;
expect(res).toEqual(_set({}, `${partialID}.withoutOp.${service}`, testVal));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(url, { service }));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(summaryUrl, { service }));
});

it('resolves to include single response for svc decoration given op', async () => {
const promise = getDecoration(withoutOpID, service, operation);
resolves[0](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[0](_set({}, stringSupplant(summaryPath, { service }), testVal));
const res = await promise;
expect(res).toEqual(_set({}, `${withoutOpID}.withoutOp.${service}`, testVal));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(url, { service }));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(summaryUrl, { service }));
});

it('resolves to include single response for svc decoration not given op', async () => {
const promise = getDecoration(withoutOpID, service);
resolves[0](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[0](_set({}, stringSupplant(summaryPath, { service }), testVal));
const res = await promise;
expect(res).toEqual(_set({}, `${withoutOpID}.withoutOp.${service}`, testVal));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(url, { service }));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(summaryUrl, { service }));
});

it('handles error responses', async () => {
Expand All @@ -148,7 +150,7 @@ describe('getDecoration', () => {
expect(res0).toEqual(
_set({}, `${withoutOpID}.withoutOp.${service}`, `Unable to fetch decoration: ${message}`)
);
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(url, { service }));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(summaryUrl, { service }));

const err = 'foo error without message';
const promise1 = getDecoration(withOpID, service, operation);
Expand All @@ -157,21 +159,21 @@ describe('getDecoration', () => {
expect(res1).toEqual(
_set({}, `${withOpID}.withOp.${service}.${operation}`, `Unable to fetch decoration: ${err}`)
);
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(opUrl, { service, operation }));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(opSummaryUrl, { service, operation }));
});

it('defaults value if valuePath not found in response', async () => {
it('defaults value if summaryPath not found in response', async () => {
const promise = getDecoration(withoutOpID, service);
resolves[0]();
const res = await promise;
expect(res).toEqual(
_set(
{},
`${withoutOpID}.withoutOp.${service}`,
`${stringSupplant(valuePath, { service })} not found in response`
`\`${stringSupplant(summaryPath, { service })}\` not found in response`
)
);
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(url, { service }));
expect(fetchDecorationSpy).toHaveBeenLastCalledWith(stringSupplant(summaryUrl, { service }));
});

it('returns undefined if invoked before previous invocation is resolved', () => {
Expand All @@ -182,13 +184,13 @@ describe('getDecoration', () => {
it('resolves to include responses for all concurrent requests', async () => {
const otherOp = 'other op';
const promise = getDecoration(withOpID, service, operation);
resolves[0](_set({}, stringSupplant(opValuePath, { service, operation }), testVal));
resolves[0](_set({}, stringSupplant(opSummaryPath, { service, operation }), testVal));
getDecoration(partialID, service, operation);
resolves[1](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[1](_set({}, stringSupplant(summaryPath, { service }), testVal));
getDecoration(withOpID, service);
resolves[2](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[2](_set({}, stringSupplant(summaryPath, { service }), testVal));
getDecoration(withoutOpID, service);
resolves[3](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[3](_set({}, stringSupplant(summaryPath, { service }), testVal));
const message = 'foo error message';
getDecoration(withOpID, service, otherOp);
rejects[4]({ message });
Expand Down Expand Up @@ -222,15 +224,15 @@ describe('getDecoration', () => {
it('scopes promises to not include previous promise results', async () => {
const otherOp = 'other op';
const promise0 = getDecoration(withOpID, service, operation);
resolves[0](_set({}, stringSupplant(opValuePath, { service, operation }), testVal));
resolves[0](_set({}, stringSupplant(opSummaryPath, { service, operation }), testVal));
getDecoration(partialID, service, operation);
resolves[1](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[1](_set({}, stringSupplant(summaryPath, { service }), testVal));
const res0 = await promise0;

const promise1 = getDecoration(withOpID, service);
resolves[2](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[2](_set({}, stringSupplant(summaryPath, { service }), testVal));
getDecoration(withoutOpID, service);
resolves[3](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[3](_set({}, stringSupplant(summaryPath, { service }), testVal));
const message = 'foo error message';
getDecoration(withOpID, service, otherOp);
rejects[4]({ message });
Expand Down Expand Up @@ -272,15 +274,15 @@ describe('getDecoration', () => {

it('no-ops for already processed id, service, and operation', async () => {
const promise0 = getDecoration(withOpID, service, operation);
resolves[0](_set({}, stringSupplant(opValuePath, { service, operation }), testVal));
resolves[0](_set({}, stringSupplant(opSummaryPath, { service, operation }), testVal));
const res0 = await promise0;
expect(res0).toEqual(_set({}, `${withOpID}.withOp.${service}.${operation}`, testVal));

const promise1 = getDecoration(withOpID, service, operation);
expect(promise1).toBeUndefined();

const promise2 = getDecoration(withoutOpID, service);
resolves[1](_set({}, stringSupplant(valuePath, { service }), testVal));
resolves[1](_set({}, stringSupplant(summaryPath, { service }), testVal));
const res1 = await promise2;
expect(res1).toEqual(_set({}, `${withoutOpID}.withoutOp.${service}`, testVal));
expect(fetchDecorationSpy).toHaveBeenCalledTimes(2);
Expand Down
28 changes: 16 additions & 12 deletions packages/jaeger-ui/src/actions/path-agnostic-decorations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import _get from 'lodash/get';
import _memoize from 'lodash/memoize';
import _set from 'lodash/set';
import memoize from 'lru-memoize';
import { createActions, ActionFunctionAny, Action } from 'redux-actions';

import JaegerAPI from '../api/jaeger';
Expand All @@ -23,9 +24,12 @@ import { getConfigValue } from '../utils/config/get-config';
import generateActionTypes from '../utils/generate-action-types';
import stringSupplant from '../utils/stringSupplant';

// wrapping JaegerAPI.fetchDecoration is necessary for tests to properly mock inside memoization
const fetchDecoration = memoize(10)((url: string) => JaegerAPI.fetchDecoration(url));

export const actionTypes = generateActionTypes('@jaeger-ui/PATH_AGNOSTIC_DECORATIONS', ['GET_DECORATION']);

const getDecorationSchema = _memoize((id: string): TPathAgnosticDecorationSchema | undefined => {
export const getDecorationSchema = _memoize((id: string): TPathAgnosticDecorationSchema | undefined => {
const schemas = getConfigValue('pathAgnosticDecorations') as TPathAgnosticDecorationSchema[] | undefined;
if (!schemas) return undefined;
return schemas.find(s => s.id === id);
Expand All @@ -39,16 +43,17 @@ let resolve: undefined | ((arg: TNewData) => void);

// Bespoke memoization-adjacent solution necessary as this should return `undefined`, not an old promise, on
// duplicate calls
export const processed = new Map<string, Map<string, Set<string | undefined>>>();
// exported for tests
export const _processed = new Map<string, Map<string, Set<string | undefined>>>();

export function getDecoration(
id: string,
service: string,
operation?: string
): Promise<TNewData> | undefined {
const processedID = processed.get(id);
const processedID = _processed.get(id);
if (!processedID) {
processed.set(id, new Map<string, Set<string | undefined>>([[service, new Set([operation])]]));
_processed.set(id, new Map<string, Set<string | undefined>>([[service, new Set([operation])]]));
} else {
const processedService = processedID.get(service);
if (!processedService) processedID.set(service, new Set([operation]));
Expand All @@ -67,24 +72,23 @@ export function getDecoration(
}

pendingCount = pendingCount ? pendingCount + 1 : 1;
const { url, opUrl, valuePath, opValuePath } = schema;
const { summaryUrl, opSummaryUrl, summaryPath, opSummaryPath } = schema;
let promise: Promise<Record<string, any>>;
let getPath: string;
let setPath: string;
if (opValuePath && opUrl && operation) {
promise = JaegerAPI.fetchDecoration(stringSupplant(opUrl, { service, operation }));
getPath = stringSupplant(opValuePath, { service, operation });
if (opSummaryPath && opSummaryUrl && operation) {
promise = fetchDecoration(stringSupplant(opSummaryUrl, { service, operation }));
getPath = stringSupplant(opSummaryPath, { service, operation });
setPath = `${id}.withOp.${service}.${operation}`;
} else {
promise = JaegerAPI.fetchDecoration(stringSupplant(url, { service }));
getPath = stringSupplant(valuePath, { service });
getPath = valuePath;
promise = fetchDecoration(stringSupplant(summaryUrl, { service }));
getPath = stringSupplant(summaryPath, { service });
setPath = `${id}.withoutOp.${service}`;
}

promise
.then(res => {
return _get(res, getPath, `${getPath} not found in response`);
return _get(res, getPath, `\`${getPath}\` not found in response`);
})
.catch(err => {
return `Unable to fetch decoration: ${err.message || err}`;
Expand Down
3 changes: 3 additions & 0 deletions packages/jaeger-ui/src/api/jaeger.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ const JaegerAPI = {
archiveTrace(id) {
return getJSON(`${this.apiRoot}archive/${id}`, { method: 'POST' });
},
fetchQualityMetrics(service, lookback) {
return getJSON(`/qualitymetrics-v2`, { query: { service, lookback } });
},
fetchDecoration(url) {
return getJSON(url);
},
Expand Down
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/App/Page.css
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ limitations under the License.
display: flex;
flex-direction: column;
left: 0;
min-height: calc(100% - 46px);
min-height: calc(100% - var(--nav-height));
position: absolute;
right: 0;
top: 46px;
top: var(--nav-height);
}
9 changes: 9 additions & 0 deletions packages/jaeger-ui/src/components/App/TopNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { RouteComponentProps, Link, withRouter } from 'react-router-dom';
import TraceIDSearchInput from './TraceIDSearchInput';
import * as dependencyGraph from '../DependencyGraph/url';
import * as deepDependencies from '../DeepDependencies/url';
import * as qualityMetrics from '../QualityMetrics/url';
import * as searchUrl from '../SearchTracePage/url';
import * as diffUrl from '../TraceDiff/url';
import { ReduxState } from '../../types';
Expand Down Expand Up @@ -59,6 +60,14 @@ if (getConfigValue('deepDependencies.menuEnabled')) {
});
}

if (getConfigValue('qualityMetrics.menuEnabled')) {
NAV_LINKS.push({
to: qualityMetrics.getUrl(),
matches: qualityMetrics.matches,
text: 'Quality Metrics',
});
}

function getItemLink(item: ConfigMenuItem) {
const { label, anchorTarget, url } = item;
return (
Expand Down
3 changes: 3 additions & 0 deletions packages/jaeger-ui/src/components/App/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import DependencyGraph from '../DependencyGraph';
import { ROUTE_PATH as dependenciesPath } from '../DependencyGraph/url';
import DeepDependencies from '../DeepDependencies';
import { ROUTE_PATH as deepDependenciesPath } from '../DeepDependencies/url';
import QualityMetrics from '../QualityMetrics';
import { ROUTE_PATH as qualityMetricsPath } from '../QualityMetrics/url';
import SearchTracePage from '../SearchTracePage';
import { ROUTE_PATH as searchPath } from '../SearchTracePage/url';
import TraceDiff from '../TraceDiff';
Expand Down Expand Up @@ -60,6 +62,7 @@ export default class JaegerUIApp extends Component {
<Route path={tracePath} component={TracePage} />
<Route path={dependenciesPath} component={DependencyGraph} />
<Route path={deepDependenciesPath} component={DeepDependencies} />
<Route path={qualityMetricsPath} component={QualityMetrics} />

<Redirect exact path="/" to={searchPath} />
<Redirect exact path={prefixUrl()} to={searchPath} />
Expand Down
Loading