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

Fetch ddg action and reducer #398

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
4e990da
WIP-implement all DDG actions&reducers
everett980 Jun 6, 2019
a8e0fe9
Finish tests and clean up files
everett980 Jun 7, 2019
ec450e2
Rename fn, Make tests DRY-er
everett980 Jun 7, 2019
4113737
Add guardReducer that propagates meta
everett980 Jun 12, 2019
d027b61
Create error and loading views and temp header
everett980 Jun 12, 2019
1789da6
Merge branch 'jag-500-create-graph-edges-and-vertices-from-parsed-pay…
everett980 Jun 13, 2019
93f2d2a
Merge branch 'fetch-ddg-action-and-reducer' into ddg-error-and-loading
everett980 Jun 13, 2019
a4be7e7
Move and add tests
everett980 Jun 13, 2019
50cad45
Merge branch 'jag-500-create-graph-edges-and-vertices-from-parsed-pay…
everett980 Jun 17, 2019
0cbe260
Merge branch 'fetch-ddg-action-and-reducer' into ddg-error-and-loading
everett980 Jun 17, 2019
f419baf
Improve tests, improve absent op handling
everett980 Jun 17, 2019
c46e04c
WIP - Implement Graph, TODO update tests, style
everett980 Jun 18, 2019
7cbd2d7
Add tests and comments, get visKey from query
everett980 Jun 18, 2019
d2e4aa1
Remove temporary text
everett980 Jun 18, 2019
2f9462d
Merge branch 'ddg-error-and-loading' into ddg-graph-component
everett980 Jun 19, 2019
3d1ac6b
Merge branch 'jag-500-create-graph-edges-and-vertices-from-parsed-pay…
everett980 Jun 21, 2019
d532c5d
Require meta in payload, rename view enum
everett980 Jun 21, 2019
be29f79
Merge branch 'fetch-ddg-action-and-reducer' into ddg-error-and-loading
everett980 Jun 24, 2019
18d3992
Merge and fix tests
everett980 Jun 24, 2019
5765a0b
Merge branch 'jag-500-create-graph-edges-and-vertices-from-parsed-pay…
everett980 Jun 24, 2019
f1a49ce
Flatten state
everett980 Jun 24, 2019
ccde31e
Merge branch 'fetch-ddg-action-and-reducer' into ddg-error-and-loading
everett980 Jun 24, 2019
b3cc5f1
Adopt flatter redux slice
everett980 Jun 24, 2019
b5aaae1
Merge branch 'ddg-error-and-loading' into ddg-graph-component
everett980 Jun 24, 2019
f8f3250
Improve test coverage
everett980 Jun 25, 2019
4cfdac1
Merge branch 'fetch-ddg-action-and-reducer' into ddg-error-and-loading
everett980 Jun 25, 2019
a61c55f
Disable codecov for temporary implementation
everett980 Jun 25, 2019
877138a
Move extractQuery into url, add params to url
everett980 Jun 25, 2019
0c329f8
Merge and update tests, TODO Graph component tsets
everett980 Jun 26, 2019
7916ff4
Finish last tests, rename key to encoding
everett980 Jun 27, 2019
8e43092
Eradicate styleState nomenclature, use kwarg
everett980 Jun 27, 2019
2d379f9
Split overloaded action into multiple use cases
everett980 Jun 28, 2019
125cabb
Merge branch 'fetch-ddg-action-and-reducer' into ddg-error-and-loading
everett980 Jun 28, 2019
740b0d5
Use stateKey() kwarg, fix typo
everett980 Jun 28, 2019
af09c81
Merge branch 'ddg-error-and-loading' into ddg-graph-component
everett980 Jun 28, 2019
c876011
Update shouldComponentUpdate paths
everett980 Jul 1, 2019
fc98987
Merge pull request #5 from everett980/ddg-graph-component
everett980 Jul 2, 2019
a6cab98
Merge pull request #4 from everett980/ddg-error-and-loading
everett980 Jul 2, 2019
ef15710
Add clarifying comment and new lines
everett980 Jul 2, 2019
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
32 changes: 32 additions & 0 deletions packages/jaeger-ui/src/actions/deep-dependency-graph.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) 2017 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import _identity from 'lodash/identity';
import { createActions } from 'redux-actions';

import { TDdgAddStylePayload, TDdgClearStylePayload } from '../model/ddg/types';
import generateActionTypes from '../utils/generate-action-types';

export const actionTypes = generateActionTypes('@jaeger-ui/DEEP-DEPENDENCY-GRAPH', [
'ADD_STYLE_STATE',
everett980 marked this conversation as resolved.
Show resolved Hide resolved
'CLEAR_STYLE_STATE',
]);

const addStyleState: (kwarg: TDdgAddStylePayload) => TDdgAddStylePayload = _identity;
everett980 marked this conversation as resolved.
Show resolved Hide resolved
const clearStyleState: (kwarg: TDdgClearStylePayload) => TDdgClearStylePayload = _identity;

export const actions = createActions<TDdgAddStylePayload | TDdgClearStylePayload>({
[actionTypes.ADD_STYLE_STATE]: addStyleState,
[actionTypes.CLEAR_STYLE_STATE]: clearStyleState,
});
6 changes: 6 additions & 0 deletions packages/jaeger-ui/src/actions/jaeger-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ export const fetchServiceOperations = createAction(
serviceName => ({ serviceName })
);

export const fetchDeepDependencyGraph = createAction(
'@JAEGER_API/FETCH_DEEP_DEPENDENCY_GRAPH',
query => JaegerAPI.fetchDeepDependencyGraph(query),
query => ({ query })
);

export const fetchDependencies = createAction('@JAEGER_API/FETCH_DEPENDENCIES', () =>
JaegerAPI.fetchDependencies()
);
197 changes: 112 additions & 85 deletions packages/jaeger-ui/src/actions/jaeger-api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,90 +27,117 @@ import isPromise from 'is-promise';
import * as jaegerApiActions from './jaeger-api';
import JaegerAPI from '../api/jaeger';

it('@JAEGER_API/FETCH_TRACE should fetch the trace by id', () => {
const api = JaegerAPI;
describe('actions/jaeger-api', () => {
const query = { param: 'value' };
const id = 'my-trace-id';
const mock = sinon.mock(api);

mock.expects('fetchTrace').withExactArgs(id);
jaegerApiActions.fetchTrace(id);
expect(() => mock.verify()).not.toThrow();

mock.restore();
});

it('@JAEGER_API/FETCH_TRACE should return the promise', () => {
const api = JaegerAPI;
const id = 'my-trace-id';
const mock = sinon.mock(api);

const { payload } = jaegerApiActions.fetchTrace(id);
expect(isPromise(payload)).toBeTruthy();

mock.restore();
});

it('@JAEGER_API/FETCH_TRACE should attach the id as meta', () => {
const api = JaegerAPI;
const id = 'my-trace-id';
const mock = sinon.mock(api);

const { meta } = jaegerApiActions.fetchTrace(id);
expect(meta.id).toBe(id);

mock.restore();
});

it('@JAEGER_API/SEARCH_TRACES should fetch the trace by id', () => {
const api = JaegerAPI;
const query = { service: 's', limit: 1 };
const mock = sinon.mock(api);

mock.expects('searchTraces').withExactArgs(query);
jaegerApiActions.searchTraces(query);
expect(() => mock.verify()).not.toThrow();

mock.restore();
});

it('@JAEGER_API/SEARCH_TRACES should return the promise', () => {
const api = JaegerAPI;
const query = { myQuery: 'whatever' };
const mock = sinon.mock(api);

const { payload } = jaegerApiActions.searchTraces(query);
expect(isPromise(payload)).toBeTruthy();

mock.restore();
});

it('@JAEGER_API/SEARCH_TRACES should attach the query as meta', () => {
const api = JaegerAPI;
const query = { myQuery: 'whatever' };
const mock = sinon.mock(api);

const { meta } = jaegerApiActions.searchTraces(query);
expect(meta.query).toEqual(query);

mock.restore();
});

it('@JAEGER_API/FETCH_SERVICES should return a promise', () => {
const api = JaegerAPI;
const mock = sinon.mock(api);
const { payload } = jaegerApiActions.fetchServices();
expect(isPromise(payload)).toBeTruthy();
mock.restore();
});

it('@JAEGER_API/FETCH_SERVICE_OPERATIONS should call the JaegerAPI', () => {
const api = JaegerAPI;
const mock = sinon.mock(api);
const called = mock
.expects('fetchServiceOperations')
.once()
.withExactArgs('service');
jaegerApiActions.fetchServiceOperations('service');
expect(called.verify()).toBeTruthy();
mock.restore();
const ids = [id, id];
let mock;

beforeEach(() => {
mock = sinon.mock(JaegerAPI);
});

afterEach(() => {
mock.restore();
});

it('@JAEGER_API/FETCH_TRACE should fetch the trace by id', () => {
mock.expects('fetchTrace').withExactArgs(id);
jaegerApiActions.fetchTrace(id);
expect(() => mock.verify()).not.toThrow();
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these can be driven by an array of params?

const tests = [
  {
    msg: '@JAEGER_API/SEARCH_TRACES should fetch the trace by id',
    fn: 'searchTraces',
    arg: query,  // or null
  },
  // etc ... 
];
tests.forEach(test => {
  const { msg, fn, args } = test;
  it(msg, () => {
    mock.expects(fn).withExactArgs(arg);
    jaegerApiActions[fn](arg);
    expect(() => mock.verify()).not.toThrow();
  });
});

Seems like the same with the promise tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's true for most, but not @JAEGER_API/FETCH_MULTIPLE_TRACES which calls an action with one name and an API fn with another. I know that's not hard to handle (optional extra key on tests), but perhaps best to leave jaeger-api.test and api/jaeger.test clean up for its own targeted PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I mentioned it is because you re-wrote the tests in this PR.


it('@JAEGER_API/FETCH_TRACE should return the promise', () => {
const { payload } = jaegerApiActions.fetchTrace(id);
expect(isPromise(payload)).toBeTruthy();
});

it('@JAEGER_API/FETCH_TRACE should attach the id as meta', () => {
const { meta } = jaegerApiActions.fetchTrace(id);
expect(meta.id).toBe(id);
});

it('@JAEGER_API/FETCH_MULTIPLE_TRACES should fetch traces by ids', () => {
mock.expects('searchTraces').withExactArgs(sinon.match.has('traceID', ids));
jaegerApiActions.fetchMultipleTraces(ids);
expect(() => mock.verify()).not.toThrow();
});

it('@JAEGER_API/FETCH_MULTIPLE_TRACES should return the promise', () => {
const { payload } = jaegerApiActions.fetchMultipleTraces(ids);
expect(isPromise(payload)).toBeTruthy();
});

it('@JAEGER_API/FETCH_MULTIPLE_TRACES should attach the ids as meta', () => {
const { meta } = jaegerApiActions.fetchMultipleTraces(ids);
expect(meta.ids).toBe(ids);
});

it('@JAEGER_API/ARCHIVE_TRACE should archive the trace by id', () => {
mock.expects('archiveTrace').withExactArgs(id);
jaegerApiActions.archiveTrace(id);
expect(() => mock.verify()).not.toThrow();
});

it('@JAEGER_API/ARCHIVE_TRACE should return the promise', () => {
const { payload } = jaegerApiActions.archiveTrace(id);
expect(isPromise(payload)).toBeTruthy();
});

it('@JAEGER_API/ARCHIVE_TRACE should attach the id as meta', () => {
const { meta } = jaegerApiActions.archiveTrace(id);
expect(meta.id).toBe(id);
});

it('@JAEGER_API/SEARCH_TRACES should fetch the trace by id', () => {
mock.expects('searchTraces').withExactArgs(query);
jaegerApiActions.searchTraces(query);
expect(() => mock.verify()).not.toThrow();
});

it('@JAEGER_API/SEARCH_TRACES should return the promise', () => {
const { payload } = jaegerApiActions.searchTraces(query);
expect(isPromise(payload)).toBeTruthy();
});

it('@JAEGER_API/SEARCH_TRACES should attach the query as meta', () => {
const { meta } = jaegerApiActions.searchTraces(query);
expect(meta.query).toEqual(query);
});

it('@JAEGER_API/FETCH_SERVICES should return a promise', () => {
const { payload } = jaegerApiActions.fetchServices();
expect(isPromise(payload)).toBeTruthy();
});

it('@JAEGER_API/FETCH_SERVICE_OPERATIONS should call the JaegerAPI', () => {
const called = mock
.expects('fetchServiceOperations')
.once()
.withExactArgs('service');
jaegerApiActions.fetchServiceOperations('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();
});

it('@JAEGER_API/FETCH_DEEP_DEPENDENCY_GRAPH should return the promise', () => {
const { payload } = jaegerApiActions.fetchDeepDependencyGraph(query);
expect(isPromise(payload)).toBeTruthy();
});

it('@JAEGER_API/FETCH_DEEP_DEPENDENCY_GRAPH should attach the query as meta', () => {
const { meta } = jaegerApiActions.fetchDeepDependencyGraph(query);
expect(meta.query).toEqual(query);
});

it('@JAEGER_API/FETCH_DEPENDENCIES should call the JaegerAPI', () => {
const called = mock.expects('fetchDependencies').once();
jaegerApiActions.fetchDependencies();
expect(called.verify()).toBeTruthy();
});
});
21 changes: 12 additions & 9 deletions packages/jaeger-ui/src/api/jaeger.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,23 +76,26 @@ export const DEFAULT_DEPENDENCY_LOOKBACK = moment.duration(1, 'weeks').asMillise

const JaegerAPI = {
apiRoot: DEFAULT_API_ROOT,
fetchTrace(id) {
return getJSON(`${this.apiRoot}traces/${id}`);
},
archiveTrace(id) {
return getJSON(`${this.apiRoot}archive/${id}`, { method: 'POST' });
},
searchTraces(query) {
return getJSON(`${this.apiRoot}traces`, { query });
fetchDeepDependencyGraph(query) {
return getJSON(`${this.apiRoot}deep-dependency-graph`, { query });
},
fetchServices() {
return getJSON(`${this.apiRoot}services`);
fetchDependencies(endTs = new Date().getTime(), lookback = DEFAULT_DEPENDENCY_LOOKBACK) {
return getJSON(`${this.apiRoot}dependencies`, { query: { endTs, lookback } });
},
fetchServiceOperations(serviceName) {
return getJSON(`${this.apiRoot}services/${encodeURIComponent(serviceName)}/operations`);
},
fetchDependencies(endTs = new Date().getTime(), lookback = DEFAULT_DEPENDENCY_LOOKBACK) {
return getJSON(`${this.apiRoot}dependencies`, { query: { endTs, lookback } });
fetchServices() {
return getJSON(`${this.apiRoot}services`);
},
fetchTrace(id) {
return getJSON(`${this.apiRoot}traces/${id}`);
},
searchTraces(query) {
return getJSON(`${this.apiRoot}traces`, { query });
},
};

Expand Down
Loading