From 501157e7ed729df752c22a872c49e44a87e6d096 Mon Sep 17 00:00:00 2001 From: Barthelemy Antonin Date: Sun, 5 Nov 2023 13:34:27 +0100 Subject: [PATCH] Make ArchiveTrace button auto-configurable - Resolves #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 --- packages/jaeger-ui/index.html | 6 +++ .../src/components/TracePage/index.test.js | 16 ++++---- .../src/components/TracePage/index.tsx | 9 +++- .../src/constants/default-config.tsx | 5 ++- packages/jaeger-ui/src/types/config.tsx | 8 ++++ .../src/utils/config/get-config.test.js | 17 +++++--- .../jaeger-ui/src/utils/config/get-config.tsx | 41 ++++++++++++------- .../jaeger-ui/test/jest-per-test-setup.js | 1 + packages/jaeger-ui/typings/custom.d.ts | 3 +- 9 files changed, 75 insertions(+), 31 deletions(-) diff --git a/packages/jaeger-ui/index.html b/packages/jaeger-ui/index.html index f235500c46..d5baab4682 100644 --- a/packages/jaeger-ui/index.html +++ b/packages/jaeger-ui/index.html @@ -28,6 +28,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_COMPATIBILITIES = { "archiveStorage": false }; + const JAEGER_COMPATIBILITIES = DEFAULT_COMPATIBILITIES; + return JAEGER_COMPATIBILITIES; + } // Jaeger version data is embedded by the query-service via search/replace. function getJaegerVersion() { const DEFAULT_VERSION = {"gitCommit":"", "gitVersion":"", "buildDate":""}; diff --git a/packages/jaeger-ui/src/components/TracePage/index.test.js b/packages/jaeger-ui/src/components/TracePage/index.test.js index bed1d620b7..53a0ea332e 100644 --- a/packages/jaeger-ui/src/components/TracePage/index.test.js +++ b/packages/jaeger-ui/src/components/TracePage/index.test.js @@ -71,7 +71,7 @@ describe('makeShortcutCallbacks()', () => { }); it('returns callbacsks that adjust the range based on the `shortcutConfig` values', () => { - const fakeEvent = { preventDefault: () => {} }; + const fakeEvent = { preventDefault: () => { } }; const callbacks = makeShortcutCallbacks(adjRange); Object.keys(shortcutConfig).forEach((key, i) => { callbacks[key](fakeEvent); @@ -84,12 +84,12 @@ describe('makeShortcutCallbacks()', () => { describe('', () => { const trace = transformTraceData(traceGenerator.trace({})); const defaultProps = { - acknowledgeArchive: () => {}, - fetchTrace() {}, + acknowledgeArchive: () => { }, + fetchTrace() { }, focusUiFindMatches: jest.fn(), id: trace.traceID, history: { - replace: () => {}, + replace: () => { }, }, location: { search: null, @@ -400,8 +400,10 @@ describe('', () => { 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); + }); }); }); }); @@ -712,7 +714,7 @@ describe('', () => { describe('mapDispatchToProps()', () => { it('creates the actions correctly', () => { - expect(mapDispatchToProps(() => {})).toEqual({ + expect(mapDispatchToProps(() => { })).toEqual({ acknowledgeArchive: expect.any(Function), archiveTrace: expect.any(Function), fetchTrace: expect.any(Function), diff --git a/packages/jaeger-ui/src/components/TracePage/index.tsx b/packages/jaeger-ui/src/components/TracePage/index.tsx index 3a5e88a5a1..6db4dca432 100644 --- a/packages/jaeger-ui/src/components/TracePage/index.tsx +++ b/packages/jaeger-ui/src/components/TracePage/index.tsx @@ -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'; @@ -78,6 +78,7 @@ type TOwnProps = { type TReduxProps = { archiveEnabled: boolean; + storageCapabilities: StorageCapabilities | TNil; archiveTraceState: TraceArchive | TNil; criticalPathEnabled: boolean; embedded: null | EmbeddedState; @@ -326,6 +327,7 @@ export class TracePageImpl extends React.PureComponent { render() { const { archiveEnabled, + storageCapabilities, archiveTraceState, criticalPathEnabled, embedded, @@ -359,6 +361,7 @@ export class TracePageImpl extends React.PureComponent { } const isEmbedded = Boolean(embedded); + const hasArchiveStorage = Boolean(storageCapabilities?.archiveStorage); const headerProps = { focusUiFindMatches: this.focusUiFindMatches, slimView, @@ -380,7 +383,7 @@ export class TracePageImpl extends React.PureComponent { ref: this._searchBar, resultCount: findCount, disableJsonView, - showArchiveButton: !isEmbedded && archiveEnabled, + showArchiveButton: !isEmbedded && archiveEnabled && hasArchiveStorage, showShortcutsHelp: !isEmbedded, showStandaloneLink: isEmbedded, showViewOptions: !isEmbedded, @@ -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; @@ -453,6 +457,7 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP return { ...extractUiFindFromState(state), archiveEnabled, + storageCapabilities: storageCapabilities, archiveTraceState, criticalPathEnabled, embedded, diff --git a/packages/jaeger-ui/src/constants/default-config.tsx b/packages/jaeger-ui/src/constants/default-config.tsx index c5d4d53937..52d3418959 100644 --- a/packages/jaeger-ui/src/constants/default-config.tsx +++ b/packages/jaeger-ui/src/constants/default-config.tsx @@ -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, @@ -77,6 +77,9 @@ const defaultConfig: Config = { }, maxLimit: 1500, }, + storageCapabilities: { + archiveStorage: false, + }, tracking: { gaID: null, trackErrors: true, diff --git a/packages/jaeger-ui/src/types/config.tsx b/packages/jaeger-ui/src/types/config.tsx index 3f3fcc98a4..019fe36a32 100644 --- a/packages/jaeger-ui/src/types/config.tsx +++ b/packages/jaeger-ui/src/types/config.tsx @@ -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 = { // archiveEnabled enables the Archive Trace button in the trace view. @@ -129,6 +134,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 diff --git a/packages/jaeger-ui/src/utils/config/get-config.test.js b/packages/jaeger-ui/src/utils/config/get-config.test.js index d2456036df..4074a125cd 100644 --- a/packages/jaeger-ui/src/utils/config/get-config.test.js +++ b/packages/jaeger-ui/src/utils/config/get-config.test.js @@ -20,7 +20,7 @@ import getConfig, { getConfigValue } from './get-config'; import processDeprecation from './process-deprecation'; import defaultConfig, { deprecations, mergeFields } from '../../constants/default-config'; -describe('getConfig()', () => { +describe.only('getConfig()', () => { const warnFn = jest.fn(); let oldWarn; @@ -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', () => { @@ -54,15 +55,20 @@ describe('getConfig()', () => { }); }); - describe('`window.getJaegerUiConfig` is a function', () => { + describe('index functions are injected by backend', () => { let embedded; let getJaegerUiConfig; + let capabilities; + let getJaegerStorageCapabilities; beforeEach(() => { getConfig.apply({}, []); embedded = {}; getJaegerUiConfig = jest.fn(() => embedded); window.getJaegerUiConfig = getJaegerUiConfig; + capabilities = defaultConfig.capabilities; + getJaegerStorageCapabilities = jest.fn(() => capabilities); + window.getJaegerStorageCapabilities = getJaegerStorageCapabilities; }); it('returns the default config when the embedded config is `null`', () => { @@ -70,9 +76,10 @@ describe('getConfig()', () => { 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 = { 'archiveEnabled': true }; + expect(getConfig()).toEqual({ ...defaultConfig, ...embedded, ...capabilities }); }); describe('overwriting precedence and merging', () => { diff --git a/packages/jaeger-ui/src/utils/config/get-config.tsx b/packages/jaeger-ui/src/utils/config/get-config.tsx index 7d39ac7b93..230c586f1f 100644 --- a/packages/jaeger-ui/src/utils/config/get-config.tsx +++ b/packages/jaeger-ui/src/utils/config/get-config.tsx @@ -26,36 +26,47 @@ let haveWarnedDeprecations = false; * 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(); - if (!embedded) { - return { ...defaultConfig }; - } + const embedded = getUiConfig(); + const capabilities = getCapabilities(); // check for deprecated config values if (Array.isArray(deprecations)) { deprecations.forEach(deprecation => processDeprecation(embedded, deprecation, !haveWarnedDeprecations)); haveWarnedDeprecations = true; } - const rv = { ...defaultConfig, ...embedded }; + const rv = { ...defaultConfig, ...embedded, ...capabilities }; // 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) { + if (embedded && typeof embedded[key] === 'object' && embedded[key] !== null) { rv[key] = { ...defaultConfig[key], ...embedded[key] }; } } return rv; }); +function getUiConfig() { + 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 }; + } + + return getJaegerUiConfig(); +} + +function getCapabilities() { + const getJaegerCapabilities = window.getJaegerStorageCapabilities; + if (typeof getJaegerCapabilities !== 'function') { + return {}; + } + return getJaegerCapabilities(); +} + export default getConfig; export function getConfigValue(path: string) { diff --git a/packages/jaeger-ui/test/jest-per-test-setup.js b/packages/jaeger-ui/test/jest-per-test-setup.js index 2a411af60b..30793654eb 100644 --- a/packages/jaeger-ui/test/jest-per-test-setup.js +++ b/packages/jaeger-ui/test/jest-per-test-setup.js @@ -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: '', diff --git a/packages/jaeger-ui/typings/custom.d.ts b/packages/jaeger-ui/typings/custom.d.ts index cc78cbc49e..e3765503e9 100644 --- a/packages/jaeger-ui/typings/custom.d.ts +++ b/packages/jaeger-ui/typings/custom.d.ts @@ -22,12 +22,13 @@ declare interface Window { __webpack_public_path__: string; // eslint-disable-line camelcase // For getting ui config getJaegerUiConfig?: () => Record; + getJaegerStorageCapabilities?: () => Record; getJaegerVersion?: () => Record; } declare const __REACT_APP_GA_DEBUG__: string | undefined; declare const __REACT_APP_VSN_STATE__: string | undefined; -declare const __APP_ENVIRONMENT__:? string | undefined; +declare const __APP_ENVIRONMENT__: ?string | undefined; declare module 'combokeys' { export default class Combokeys {