diff --git a/src/components/App/TopNav.js b/src/components/App/TopNav.js index 53fca971f5..a51a886bef 100644 --- a/src/components/App/TopNav.js +++ b/src/components/App/TopNav.js @@ -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'; @@ -64,7 +65,7 @@ const NAV_LINKS = [ }, ]; -if (getConfig().dependenciesMenuEnabled) { +if (_get(getConfig(), 'dependencies.menuEnabled')) { NAV_LINKS.push({ key: 'dependencies', to: prefixUrl('/dependencies'), diff --git a/src/components/DependencyGraph/index.js b/src/components/DependencyGraph/index.js index 9afec6e836..df337decc2 100644 --- a/src/components/DependencyGraph/index.js +++ b/src/components/DependencyGraph/index.js @@ -13,19 +13,22 @@ // 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 = { @@ -33,6 +36,9 @@ export const GRAPH_TYPES = { 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 @@ -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 ( diff --git a/src/components/SearchTracePage/TraceResultsScatterPlot.js b/src/components/SearchTracePage/TraceResultsScatterPlot.js index 7396902af8..77583070bd 100644 --- a/src/components/SearchTracePage/TraceResultsScatterPlot.js +++ b/src/components/SearchTracePage/TraceResultsScatterPlot.js @@ -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'; diff --git a/src/components/SearchTracePage/TraceSearchResult.js b/src/components/SearchTracePage/TraceSearchResult.js index a86940377c..a78bf2ad02 100644 --- a/src/components/SearchTracePage/TraceSearchResult.js +++ b/src/components/SearchTracePage/TraceSearchResult.js @@ -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'; diff --git a/src/components/TracePage/TracePageHeader.js b/src/components/TracePage/TracePageHeader.js index 5f14fd62c7..089cf3ede6 100644 --- a/src/components/TracePage/TracePageHeader.js +++ b/src/components/TracePage/TracePageHeader.js @@ -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 = { diff --git a/src/constants/default-config.js b/src/constants/default-config.js index 24d974fd09..cff63f9f3c 100644 --- a/src/constants/default-config.js +++ b/src/constants/default-config.js @@ -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', @@ -46,5 +52,11 @@ export default deepFreeze({ ], }, ], - dependenciesMenuEnabled: true, }); + +export const deprecations = [ + { + formerKey: 'dependenciesMenuEnabled', + currentKey: 'dependencies.menuEnabled', + }, +]; diff --git a/src/constants/fallback-trace-name.js b/src/constants/index.js similarity index 84% rename from src/constants/fallback-trace-name.js rename to src/constants/index.js index cdae3030d8..f00ae68547 100644 --- a/src/constants/fallback-trace-name.js +++ b/src/constants/index.js @@ -12,4 +12,5 @@ // See the License for the specific language governing permissions and // limitations under the License. -export default ''; +export const FALLBACK_DAG_MAX_NUM_SERVICES = 100; +export const FALLBACK_TRACE_NAME = ''; diff --git a/src/types/config.js b/src/types/config.js index 1691f325d8..c3a8ffc126 100644 --- a/src/types/config.js +++ b/src/types/config.js @@ -25,6 +25,7 @@ export type ConfigMenuGroup = { }; export type Config = { + dependencies?: { dagMaxServicesLen?: number, menuEnabled?: boolean }, gaTrackingID?: ?string, menu: (ConfigMenuGroup | ConfigMenuItem)[], }; diff --git a/src/utils/config/get-config.js b/src/utils/config/get-config.js index 0288bd4fec..e3fa0a85e7 100644 --- a/src/utils/config/get-config.js +++ b/src/utils/config/get-config.js @@ -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 @@ -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 }; } diff --git a/src/utils/config/get-config.test.js b/src/utils/config/get-config.test.js new file mode 100644 index 0000000000..d3fec37aa1 --- /dev/null +++ b/src/utils/config/get-config.test.js @@ -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); + }); + }); +}); diff --git a/src/utils/config/process-deprecation.js b/src/utils/config/process-deprecation.js new file mode 100644 index 0000000000..938ae2af18 --- /dev/null +++ b/src/utils/config/process-deprecation.js @@ -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')); + } + } +} diff --git a/src/utils/config/process-deprecation.test.js b/src/utils/config/process-deprecation.test.js new file mode 100644 index 0000000000..2ce82b6f08 --- /dev/null +++ b/src/utils/config/process-deprecation.test.js @@ -0,0 +1,120 @@ +// 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 processDeprecation from './process-deprecation'; + +describe('processDeprecation()', () => { + const currentKey = 'current.key'; + const formerKey = 'former.key'; + const deprecation = { currentKey, formerKey }; + + let oldWarn; + let warnFn; + + beforeEach(() => { + oldWarn = console.warn; + warnFn = jest.fn(); + console.warn = warnFn; + }); + + afterEach(() => { + console.warn = oldWarn; + }); + + it('does nothing if `formerKey` is not set', () => { + const config = {}; + processDeprecation(config, deprecation, true); + expect(config).toEqual({}); + expect(warnFn.mock.calls.length).toBe(0); + }); + describe('`formerKey` is set', () => { + let value; + let config; + + describe('`currentKey` is not set', () => { + function generateConfig() { + value = Math.random(); + config = { + former: { key: value }, + }; + } + + beforeEach(() => { + generateConfig(); + processDeprecation(config, deprecation, false); + }); + + it('moves the value to `currentKey`', () => { + expect(config.current.key).toBe(value); + }); + + it('deletes `currentKey`', () => { + expect(config.former.key).not.toBeDefined(); + }); + + it('does not `console.warn()` when `issueWarning` is `false`', () => { + expect(warnFn.mock.calls.length).toBe(0); + }); + + it('`console.warn()`s when `issueWarning` is `true`', () => { + generateConfig(); + processDeprecation(config, deprecation, true); + expect(warnFn.mock.calls.length).toBe(1); + expect(warnFn.mock.calls[0].length).toBe(1); + const msg = warnFn.mock.calls[0][0]; + expect(msg).toMatch(/is deprecated/); + expect(msg).toMatch(/has been moved/); + }); + }); + + describe('`currentKey` is set', () => { + function generateConfig() { + value = Math.random(); + config = { + former: { key: value * 2 }, + current: { key: value }, + }; + } + + beforeEach(() => { + generateConfig(); + processDeprecation(config, deprecation, false); + }); + + it('leaves `currentKey` as-is', () => { + expect(config.current.key).toBe(value); + }); + + it('deletes `former`', () => { + expect(config.former.key).not.toBeDefined(); + }); + + it('does not `console.warn()` when `issueWarning` is `false`', () => { + expect(warnFn.mock.calls.length).toBe(0); + }); + + it('`console.warn()`s when `issueWarning` is `true`', () => { + generateConfig(); + processDeprecation(config, deprecation, true); + expect(warnFn.mock.calls.length).toBe(1); + expect(warnFn.mock.calls[0].length).toBe(1); + const msg = warnFn.mock.calls[0][0]; + expect(msg).toMatch(/is deprecated/); + expect(msg).toMatch(/is being ignored/); + }); + }); + }); +});