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

Add a config value for the DAG cutoff #143

Merged
merged 5 commits into from
Dec 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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