Skip to content

Commit

Permalink
Make ArchiveTrace button auto-configurable (#1944)
Browse files Browse the repository at this point in the history
- Resolves jaegertracing/jaeger#4874

The button to archive a trace is now configured based on the state of
the QueryService in addition to the UI configuration.

All corresponding tests have been updated.

- [X] I have read

https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [X] I have signed all commits
- [X] I have added unit tests for the new functionality
- [X] I have run lint and test steps successfully

---------

Signed-off-by: Barthelemy Antonin <antobarth@gmail.com>

Signed-off-by: Barthelemy Antonin <antobarth@gmail.com>
  • Loading branch information
thecoons committed Nov 11, 2023
1 parent 59916aa commit 8a88b39
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 37 deletions.
6 changes: 6 additions & 0 deletions packages/jaeger-ui/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@
const JAEGER_CONFIG = DEFAULT_CONFIG;
return JAEGER_CONFIG;
}
// Jaeger storage compabilities data is embedded by the query-service via search-replace.
function getJaegerStorageCapabilities() {
const DEFAULT_STORAGE_CAPABILITIES = { "archiveStorage": false };
const JAEGER_STORAGE_CAPABILITIES = DEFAULT_STORAGE_CAPABILITIES;
return JAEGER_STORAGE_CAPABILITIES;
}
// Jaeger version data is embedded by the query-service via search/replace.
function getJaegerVersion() {
const DEFAULT_VERSION = {'gitCommit':'', 'gitVersion':'', 'buildDate':''};
Expand Down
8 changes: 6 additions & 2 deletions packages/jaeger-ui/src/components/TracePage/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,12 @@ describe('<TracePage>', () => {
it('is true when not embedded and archive is enabled', () => {
[{ timeline: {} }, undefined].forEach(embedded => {
[true, false].forEach(archiveEnabled => {
wrapper.setProps({ embedded, archiveEnabled });
expect(wrapper.find(TracePageHeader).prop('showArchiveButton')).toBe(!embedded && archiveEnabled);
[{ archiveStorage: false }, { archiveStorage: true }].forEach(storageCapabilities => {
wrapper.setProps({ embedded, archiveEnabled, storageCapabilities });
expect(wrapper.find(TracePageHeader).prop('showArchiveButton')).toBe(
!embedded && archiveEnabled && storageCapabilities.archiveStorage
);
});
});
});
});
Expand Down
9 changes: 7 additions & 2 deletions packages/jaeger-ui/src/components/TracePage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ import updateUiFind from '../../utils/update-ui-find';
import TraceStatistics from './TraceStatistics/index';
import TraceSpanView from './TraceSpanView/index';
import TraceFlamegraph from './TraceFlamegraph/index';
import { TraceGraphConfig } from '../../types/config';
import { StorageCapabilities, TraceGraphConfig } from '../../types/config';

import './index.css';
import memoizedTraceCriticalPath from './CriticalPath/index';
Expand All @@ -78,6 +78,7 @@ type TOwnProps = {

type TReduxProps = {
archiveEnabled: boolean;
storageCapabilities: StorageCapabilities | TNil;
archiveTraceState: TraceArchive | TNil;
criticalPathEnabled: boolean;
embedded: null | EmbeddedState;
Expand Down Expand Up @@ -326,6 +327,7 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
render() {
const {
archiveEnabled,
storageCapabilities,
archiveTraceState,
criticalPathEnabled,
embedded,
Expand Down Expand Up @@ -359,6 +361,7 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
}

const isEmbedded = Boolean(embedded);
const hasArchiveStorage = Boolean(storageCapabilities?.archiveStorage);
const headerProps = {
focusUiFindMatches: this.focusUiFindMatches,
slimView,
Expand All @@ -380,7 +383,7 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
ref: this._searchBar,
resultCount: findCount,
disableJsonView,
showArchiveButton: !isEmbedded && archiveEnabled,
showArchiveButton: !isEmbedded && archiveEnabled && hasArchiveStorage,
showShortcutsHelp: !isEmbedded,
showStandaloneLink: isEmbedded,
showViewOptions: !isEmbedded,
Expand Down Expand Up @@ -445,6 +448,7 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP
const trace = id ? traces[id] : null;
const archiveTraceState = id ? archive[id] : null;
const archiveEnabled = Boolean(config.archiveEnabled);
const storageCapabilities = config.storageCapabilities;
const { disableJsonView, criticalPathEnabled } = config;
const { state: locationState } = router.location;
const searchUrl = (locationState && locationState.fromSearch) || null;
Expand All @@ -453,6 +457,7 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP
return {
...extractUiFindFromState(state),
archiveEnabled,
storageCapabilities,
archiveTraceState,
criticalPathEnabled,
embedded,
Expand Down
5 changes: 4 additions & 1 deletion packages/jaeger-ui/src/constants/default-config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { version } from '../../package.json';
import { Config } from '../types/config';

const defaultConfig: Config = {
archiveEnabled: false,
archiveEnabled: true,
criticalPathEnabled: true,
dependencies: {
dagMaxNumServices: FALLBACK_DAG_MAX_NUM_SERVICES,
Expand Down Expand Up @@ -77,6 +77,9 @@ const defaultConfig: Config = {
},
maxLimit: 1500,
},
storageCapabilities: {
archiveStorage: false,
},
tracking: {
gaID: null,
trackErrors: true,
Expand Down
8 changes: 8 additions & 0 deletions packages/jaeger-ui/src/types/config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ export type TraceGraphConfig = {
layoutManagerMemory?: number;
};

export type StorageCapabilities = {
// archiveStorage indicates whether the query service supports archive storage.
archiveStorage?: boolean;
};

// Default values are provided in packages/jaeger-ui/src/constants/default-config.tsx
export type Config = {
//
Expand Down Expand Up @@ -130,6 +135,9 @@ export type Config = {
// TODO when is it useful?
scripts?: readonly TScript[];

// storage capabilities given by the query service.
storageCapabilities?: StorageCapabilities;

// topTagPrefixes defines a set of prefixes for span tag names that are considered
// "important" and cause the matching tags to appear higher in the list of tags.
// For example, topTagPrefixes=['http.'] would cause all span tags that begin with
Expand Down
23 changes: 13 additions & 10 deletions packages/jaeger-ui/src/utils/config/get-config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ describe('getConfig()', () => {
console.warn = oldWarn;
});

describe('`window.getJaegerUiConfig` is not a function', () => {
describe('index functions are not yet injected by backend', () => {
beforeAll(() => {
window.getJaegerUiConfig = undefined;
window.getJaegerStorageCapabilities = undefined;
});

it('warns once', () => {
Expand All @@ -54,25 +55,27 @@ describe('getConfig()', () => {
});
});

describe('`window.getJaegerUiConfig` is a function', () => {
describe('index functions are injected by backend', () => {
let embedded;
let getJaegerUiConfig;
let capabilities;

beforeEach(() => {
getConfig.apply({}, []);
embedded = {};
getJaegerUiConfig = jest.fn(() => embedded);
window.getJaegerUiConfig = getJaegerUiConfig;
window.getJaegerUiConfig = jest.fn(() => embedded);
capabilities = defaultConfig.storageCapabilities;
window.getJaegerStorageCapabilities = jest.fn(() => capabilities);
});

it('returns the default config when the embedded config is `null`', () => {
embedded = null;
expect(getConfig()).toEqual(defaultConfig);
});

it('merges the defaultConfig with the embedded config ', () => {
it('merges the defaultConfig with the embedded config and storage capabilities', () => {
embedded = { novel: 'prop' };
expect(getConfig()).toEqual({ ...defaultConfig, ...embedded });
capabilities = { archiveStorage: true };
expect(getConfig()).toEqual({ ...defaultConfig, ...embedded, storageCapabilities: capabilities });
});

describe('overwriting precedence and merging', () => {
Expand All @@ -84,7 +87,7 @@ describe('getConfig()', () => {
keys.forEach(key => {
embedded[key] = key;
});
expect(getConfig()).toEqual({ ...defaultConfig, ...embedded });
expect(getConfig()).toEqual({ ...defaultConfig, ...embedded, storageCapabilities: capabilities });
});
});

Expand All @@ -94,7 +97,7 @@ describe('getConfig()', () => {
mergeFields.forEach((k, i) => {
embedded[k] = i ? true : null;
});
expect(getConfig()).toEqual({ ...defaultConfig, ...embedded });
expect(getConfig()).toEqual({ ...defaultConfig, ...embedded, storageCapabilities: capabilities });
});

it('merges object values', () => {
Expand All @@ -106,7 +109,7 @@ describe('getConfig()', () => {
embedded[key] = { a: true, b: false };
const expected = { ...defaultConfig, ...embedded };
expected[key] = { ...defaultConfig[key], ...embedded[key] };
expect(getConfig()).toEqual(expected);
expect(getConfig()).toEqual({ ...expected, storageCapabilities: capabilities });
});
});
});
Expand Down
47 changes: 25 additions & 22 deletions packages/jaeger-ui/src/utils/config/get-config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,44 +18,47 @@ import memoizeOne from 'memoize-one';
import processDeprecation from './process-deprecation';
import defaultConfig, { deprecations, mergeFields } from '../../constants/default-config';

let haveWarnedFactoryFn = false;
let haveWarnedDeprecations = false;

/**
* Merge the embedded config from the query service (if present) with the
* default config from `../../constants/default-config`.
*/
const getConfig = memoizeOne(function getConfig() {
const getJaegerUiConfig = window.getJaegerUiConfig;
if (typeof getJaegerUiConfig !== 'function') {
if (!haveWarnedFactoryFn) {
// eslint-disable-next-line no-console
console.warn('Embedded config not available');
haveWarnedFactoryFn = true;
}
return { ...defaultConfig };
}
const embedded = getJaegerUiConfig();
const capabilities = getCapabilities();

const embedded = getUiConfig();
if (!embedded) {
return { ...defaultConfig };
return { ...defaultConfig, storageCapabilities: capabilities };
}
// check for deprecated config values
if (Array.isArray(deprecations)) {
deprecations.forEach(deprecation => processDeprecation(embedded, deprecation, !haveWarnedDeprecations));
haveWarnedDeprecations = true;
deprecations.forEach(deprecation => processDeprecation(embedded, deprecation, true));
}
const rv = { ...defaultConfig, ...embedded };
// mergeFields config values should be merged instead of fully replaced
const keys = mergeFields;
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
if (typeof embedded[key] === 'object' && embedded[key] !== null) {
mergeFields.forEach(key => {
if (embedded && typeof embedded[key] === 'object' && embedded[key] !== null) {
rv[key] = { ...defaultConfig[key], ...embedded[key] };
}
}
return rv;
});
return { ...rv, storageCapabilities: capabilities };
});

function getUiConfig() {
const getter = window.getJaegerUiConfig;
if (typeof getter !== 'function') {
// eslint-disable-next-line no-console
console.warn('Embedded config not available');
return { ...defaultConfig };
}
return getter();
}

function getCapabilities() {
const getter = window.getJaegerStorageCapabilities;
const capabilities = typeof getter === 'function' ? getter() : null;
return capabilities ?? defaultConfig.storageCapabilities;
}

export default getConfig;

export function getConfigValue(path: string) {
Expand Down
1 change: 1 addition & 0 deletions packages/jaeger-ui/test/jest-per-test-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ expect.addSnapshotSerializer(createSerializer({ mode: 'deep' }));
// Calls to get-config.tsx and get-version.tsx warn if these globals are not functions.
// This file is executed before each test file, so they may be overridden safely.
window.getJaegerUiConfig = () => ({});
window.getJaegerStorageCapabilities = () => ({});
window.getJaegerVersion = () => ({
gitCommit: '',
gitVersion: '',
Expand Down
1 change: 1 addition & 0 deletions packages/jaeger-ui/typings/custom.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ declare interface Window {
__webpack_public_path__: string; // eslint-disable-line camelcase
// For getting ui config
getJaegerUiConfig?: () => Record<string, any>;
getJaegerStorageCapabilities?: () => Record<string, any>;
getJaegerVersion?: () => Record<string, any>;
}

Expand Down

0 comments on commit 8a88b39

Please sign in to comment.