Skip to content

Commit

Permalink
Merge pull request jaegertracing#143 from jaegertracing/issue-130-con…
Browse files Browse the repository at this point in the history
…fig-for-dag-max

Add a config value for the DAG cutoff
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
  • Loading branch information
tiffon committed Dec 19, 2017
2 parents a37fd9f + a8e7a68 commit 884c246
Show file tree
Hide file tree
Showing 12 changed files with 321 additions and 18 deletions.
3 changes: 2 additions & 1 deletion src/components/App/TopNav.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// limitations under the License.

import React from 'react';
import _get from 'lodash/get';
import { Link } from 'react-router-dom';
import { Dropdown, Menu } from 'semantic-ui-react';

Expand Down Expand Up @@ -64,7 +65,7 @@ const NAV_LINKS = [
},
];

if (getConfig().dependenciesMenuEnabled) {
if (_get(getConfig(), 'dependencies.menuEnabled')) {
NAV_LINKS.push({
key: 'dependencies',
to: prefixUrl('/dependencies'),
Expand Down
18 changes: 12 additions & 6 deletions src/components/DependencyGraph/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,32 @@
// limitations under the License.

import PropTypes from 'prop-types';
import _get from 'lodash/get';
import React, { Component } from 'react';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import { Menu } from 'semantic-ui-react';

import './DependencyGraph.css';
import * as jaegerApiActions from '../../actions/jaeger-api';
import { formatDependenciesAsNodesAndLinks } from '../../selectors/dependencies';
import DAG from './DAG';
import DependencyForceGraph from './DependencyForceGraph';
import NotFound from '../App/NotFound';
import * as jaegerApiActions from '../../actions/jaeger-api';
import { nodesPropTypes, linksPropTypes } from '../../propTypes/dependencies';
import { formatDependenciesAsNodesAndLinks } from '../../selectors/dependencies';
import getConfig from '../../utils/config/get-config';
import { FALLBACK_DAG_MAX_NUM_SERVICES } from '../../constants';

import DependencyForceGraph from './DependencyForceGraph';
import DAG from './DAG';
import './DependencyGraph.css';

// export for tests
export const GRAPH_TYPES = {
FORCE_DIRECTED: { type: 'FORCE_DIRECTED', name: 'Force Directed Graph' },
DAG: { type: 'DAG', name: 'DAG' },
};

const dagMaxNumServices =
_get(getConfig(), 'dependencies.dagMaxNumServices') || FALLBACK_DAG_MAX_NUM_SERVICES;

export default class DependencyGraphPage extends Component {
static propTypes = {
// eslint-disable-next-line react/forbid-prop-types
Expand Down Expand Up @@ -91,7 +97,7 @@ export default class DependencyGraphPage extends Component {

const GRAPH_TYPE_OPTIONS = [GRAPH_TYPES.FORCE_DIRECTED];

if (dependencies.length <= 100) {
if (dependencies.length <= dagMaxNumServices) {
GRAPH_TYPE_OPTIONS.push(GRAPH_TYPES.DAG);
}
return (
Expand Down
2 changes: 1 addition & 1 deletion src/components/SearchTracePage/TraceResultsScatterPlot.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import dimensions from 'react-dimensions';
import { XYPlot, XAxis, YAxis, MarkSeries, Hint } from 'react-vis';
import { compose, withState, withProps } from 'recompose';

import FALLBACK_TRACE_NAME from '../../constants/fallback-trace-name';
import { FALLBACK_TRACE_NAME } from '../../constants';
import { formatDuration } from '../../utils/date';

import './react-vis.css';
Expand Down
4 changes: 2 additions & 2 deletions src/components/SearchTracePage/TraceSearchResult.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import React from 'react';
import { sortBy } from 'lodash';
import moment from 'moment';

import { formatDuration } from '../../utils/date';
import TraceServiceTag from './TraceServiceTag';
import FALLBACK_TRACE_NAME from '../../constants/fallback-trace-name';
import { FALLBACK_TRACE_NAME } from '../../constants';
import { formatDuration } from '../../utils/date';

import './TraceSearchResult.css';

Expand Down
2 changes: 1 addition & 1 deletion src/components/TracePage/TracePageHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import * as React from 'react';
import { Dropdown, Menu } from 'semantic-ui-react';

import KeyboardShortcutsHelp from './KeyboardShortcutsHelp';
import FALLBACK_TRACE_NAME from '../../constants/fallback-trace-name';
import { FALLBACK_TRACE_NAME } from '../../constants';
import { formatDatetime, formatDuration } from '../../utils/date';

type TracePageHeaderProps = {
Expand Down
14 changes: 13 additions & 1 deletion src/constants/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@

import deepFreeze from 'deep-freeze';

import { FALLBACK_DAG_MAX_NUM_SERVICES } from './index';

export default deepFreeze({
dependencies: {
dagMaxNumServices: FALLBACK_DAG_MAX_NUM_SERVICES,
menuEnabled: true,
},
menu: [
{
label: 'About Jaeger',
Expand Down Expand Up @@ -46,5 +52,11 @@ export default deepFreeze({
],
},
],
dependenciesMenuEnabled: true,
});

export const deprecations = [
{
formerKey: 'dependenciesMenuEnabled',
currentKey: 'dependencies.menuEnabled',
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@
// See the License for the specific language governing permissions and
// limitations under the License.

export default '<cannot-find-trace-name>';
export const FALLBACK_DAG_MAX_NUM_SERVICES = 100;
export const FALLBACK_TRACE_NAME = '<cannot-find-trace-name>';
1 change: 1 addition & 0 deletions src/types/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export type ConfigMenuGroup = {
};

export type Config = {
dependencies?: { dagMaxServicesLen?: number, menuEnabled?: boolean },
gaTrackingID?: ?string,
menu: (ConfigMenuGroup | ConfigMenuItem)[],
};
17 changes: 12 additions & 5 deletions src/utils/config/get-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import defaultConfig from '../../constants/default-config';
import processDeprecation from './process-deprecation';
import defaultConfig, { deprecations } from '../../constants/default-config';

let haveWarned = false;
let haveWarnedFactoryFn = false;
let haveWarnedDeprecations = false;

/**
* Merge the embedded config from the query service (if present) with the
Expand All @@ -25,13 +27,18 @@ let haveWarned = false;
export default function getConfig() {
const getJaegerUiConfig = window.getJaegerUiConfig;
if (typeof getJaegerUiConfig !== 'function') {
if (!haveWarned) {
if (!haveWarnedFactoryFn) {
// eslint-disable-next-line no-console
console.warn('Embedded config not available');
haveWarned = true;
haveWarnedFactoryFn = true;
}
return { ...defaultConfig };
}
const embedded = getJaegerUiConfig() || {};
const embedded = getJaegerUiConfig();
// check for deprecated config values
if (embedded && Array.isArray(deprecations)) {
deprecations.forEach(deprecation => processDeprecation(embedded, deprecation, !haveWarnedDeprecations));
haveWarnedDeprecations = true;
}
return { ...defaultConfig, ...embedded };
}
87 changes: 87 additions & 0 deletions src/utils/config/get-config.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// 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.

/* eslint-disable no-console, import/first */

jest.mock('./process-deprecation');
jest.mock('../../constants/default-config', () => {
const actual = require.requireActual('../../constants/default-config');
// make sure there are deprecations
const deprecations = [{ currentKey: 'current.key', formerKey: 'former.key' }];
return { default: actual.default, deprecations };
});

import getConfig from './get-config';
import processDeprecation from './process-deprecation';
import defaultConfig from '../../constants/default-config';

describe('getConfig()', () => {
let oldWarn;
let warnFn;

beforeEach(() => {
oldWarn = console.warn;
warnFn = jest.fn();
console.warn = warnFn;
});

afterEach(() => {
console.warn = oldWarn;
});

describe('`window.getJaegerUiConfig` is not a function', () => {
it('warns once', () => {
getConfig();
expect(warnFn.mock.calls.length).toBe(1);
getConfig();
expect(warnFn.mock.calls.length).toBe(1);
});

it('returns the default config', () => {
expect(getConfig()).toEqual(defaultConfig);
});
});

describe('`window.getJaegerUiConfig` is a function', () => {
let embedded;
let getJaegerUiConfig;

beforeEach(() => {
embedded = {};
getJaegerUiConfig = jest.fn(() => embedded);
window.getJaegerUiConfig = getJaegerUiConfig;
});

it('merges the defaultConfig with the embedded config ', () => {
embedded = { novel: 'prop' };
expect(getConfig()).toEqual({ ...defaultConfig, ...embedded });
});

it('gives precedence to the embedded config', () => {
embedded = {};
Object.keys(defaultConfig).forEach(key => {
embedded[key] = key;
});
expect(getConfig()).toEqual(embedded);
});

it('processes deprecations every time `getConfig` is invoked', () => {
processDeprecation.mockClear();
getConfig();
expect(processDeprecation.mock.calls.length).toBe(1);
getConfig();
expect(processDeprecation.mock.calls.length).toBe(2);
});
});
});
68 changes: 68 additions & 0 deletions src/utils/config/process-deprecation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// @flow

// 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 _get from 'lodash/get';
import _has from 'lodash/has';
import _set from 'lodash/set';
import _unset from 'lodash/unset';

type Deprecation = {
formerKey: string,
currentKey: string,
};

/**
* Processes a deprecated config property with respect to a configuration.
* NOTE: This mutates `config`.
*
* If the deprecated config property is found to be set on `config`, it is
* moved to the new config property unless a conflicting setting exists. If
* `issueWarning` is `true`, warnings are issues when:
*
* - The deprecated config property is found to be set on `config`
* - The value at the deprecated config property is moved to the new property
* - The value at the deprecated config property is ignored in favor of the value at the new property
*/
export default function processDeprecation(config: {}, deprecation: Deprecation, issueWarning: boolean) {
const { formerKey, currentKey } = deprecation;
if (_has(config, formerKey)) {
let isTransfered = false;
let isIgnored = false;
if (!_has(config, currentKey)) {
// the new key is not set so transfer the value at the old key
const value = _get(config, formerKey);
_set(config, currentKey, value);
isTransfered = true;
} else {
isIgnored = true;
}
_unset(config, formerKey);

if (issueWarning) {
const warnings = [`"${formerKey}" is deprecated, instead use "${currentKey}"`];
if (isTransfered) {
warnings.push(`The value at "${formerKey}" has been moved to "${currentKey}"`);
}
if (isIgnored) {
warnings.push(
`The value at "${formerKey}" is being ignored in favor of the value at "${currentKey}"`
);
}
// eslint-disable-next-line no-console
console.warn(warnings.join('\n'));
}
}
}
Loading

0 comments on commit 884c246

Please sign in to comment.