Skip to content
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
46 changes: 24 additions & 22 deletions packages/jaeger-ui/src/actions/path-agnostic-decorations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import JaegerAPI from '../api/jaeger';
jest.mock('lru-memoize', () => () => x => x);

describe('getDecoration', () => {
let getConfigValueSpy;
let getConfigSpy;
let fetchDecorationSpy;
let resolves;
let rejects;
Expand All @@ -36,26 +36,28 @@ describe('getDecoration', () => {
};

beforeAll(() => {
getConfigValueSpy = jest.spyOn(getConfig, 'getConfigValue').mockReturnValue([
{
id: withOpID,
summaryUrl,
opSummaryUrl,
summaryPath,
opSummaryPath,
},
{
id: partialID,
summaryUrl,
opSummaryUrl,
summaryPath,
},
{
id: withoutOpID,
summaryUrl,
summaryPath,
},
]);
getConfigSpy = jest.spyOn(getConfig, 'default').mockReturnValue({
pathAgnosticDecorations: [
{
id: withOpID,
summaryUrl,
opSummaryUrl,
summaryPath,
opSummaryPath,
},
{
id: partialID,
summaryUrl,
opSummaryUrl,
summaryPath,
},
{
id: withoutOpID,
summaryUrl,
summaryPath,
},
],
});
fetchDecorationSpy = jest.spyOn(JaegerAPI, 'fetchDecoration').mockImplementation(
() =>
new Promise((res, rej) => {
Expand All @@ -79,7 +81,7 @@ describe('getDecoration', () => {
});

it('returns undefined if no schemas exist in config', () => {
getConfigValueSpy.mockReturnValueOnce();
getConfigSpy.mockReturnValueOnce({});
expect(getDecoration('foo', service, operation)).toBeUndefined();
});

Expand Down
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/actions/path-agnostic-decorations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { createActions, ActionFunctionAny, Action } from 'redux-actions';

import JaegerAPI from '../api/jaeger';
import { TNewData, TPathAgnosticDecorationSchema } from '../model/path-agnostic-decorations/types';
import { getConfigValue } from '../utils/config/get-config';
import getConfig from '../utils/config/get-config';
import generateActionTypes from '../utils/generate-action-types';
import stringSupplant from '../utils/stringSupplant';

Expand All @@ -19,7 +19,7 @@ const fetchDecoration = memoize(10)((url: string) => JaegerAPI.fetchDecoration(u
export const actionTypes = generateActionTypes('@jaeger-ui/PATH_AGNOSTIC_DECORATIONS', ['GET_DECORATION']);

export const getDecorationSchema = _memoize((id: string): TPathAgnosticDecorationSchema | undefined => {
const schemas = getConfigValue('pathAgnosticDecorations') as TPathAgnosticDecorationSchema[] | undefined;
const schemas = getConfig().pathAgnosticDecorations;
if (!schemas) return undefined;
return schemas.find(s => s.id === id);
});
Expand Down
14 changes: 5 additions & 9 deletions packages/jaeger-ui/src/components/App/ThemeProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ import { act, fireEvent, render, renderHook, screen, waitFor } from '@testing-li

import AppThemeProvider, { useThemeMode } from './ThemeProvider';
import { THEME_STORAGE_KEY } from './ThemeStorage';
import { getConfigValue } from '../../utils/config/get-config';
import getConfig from '../../utils/config/get-config';

jest.mock('../../utils/config/get-config', () => ({
getConfigValue: jest.fn(),
__esModule: true,
default: jest.fn(),
}));

function setupMatchMedia(matches = false) {
Expand Down Expand Up @@ -49,12 +50,7 @@ describe('AppThemeProvider', () => {
window.localStorage.clear();
delete document.body.dataset.theme;
setupMatchMedia(false);
(getConfigValue as jest.Mock).mockImplementation(key => {
if (key === 'themes.enabled') {
return true;
}
return undefined;
});
(getConfig as unknown as jest.Mock).mockReturnValue({ themes: { enabled: true } });
});

it('initializes using the stored preference when present', () => {
Expand Down Expand Up @@ -129,7 +125,7 @@ describe('AppThemeProvider', () => {
});

it('ignores stored preference when themes are disabled', () => {
(getConfigValue as jest.Mock).mockReturnValue(false);
(getConfig as unknown as jest.Mock).mockReturnValue({ themes: { enabled: false } });
window.localStorage.setItem(THEME_STORAGE_KEY, 'dark');

render(
Expand Down
14 changes: 5 additions & 9 deletions packages/jaeger-ui/src/components/App/ThemeStorage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
// SPDX-License-Identifier: Apache-2.0

import { THEME_STORAGE_KEY, readStoredTheme, writeStoredTheme, getInitialTheme } from './ThemeStorage';
import { getConfigValue } from '../../utils/config/get-config';
import getConfig from '../../utils/config/get-config';

jest.mock('../../utils/config/get-config', () => ({
getConfigValue: jest.fn(),
__esModule: true,
default: jest.fn(),
}));

function setupMatchMedia(matches = false) {
Expand All @@ -28,12 +29,7 @@ describe('ThemeStorage', () => {
beforeEach(() => {
window.localStorage.clear();
setupMatchMedia(false);
(getConfigValue as jest.Mock).mockImplementation(key => {
if (key === 'themes.enabled') {
return true;
}
return undefined;
});
(getConfig as unknown as jest.Mock).mockReturnValue({ themes: { enabled: true } });
});

describe('readStoredTheme', () => {
Expand Down Expand Up @@ -137,7 +133,7 @@ describe('ThemeStorage', () => {

describe('getInitialTheme', () => {
it('returns default mode when themes are disabled', () => {
(getConfigValue as jest.Mock).mockReturnValue(false);
(getConfig as unknown as jest.Mock).mockReturnValue({ themes: { enabled: false } });
window.localStorage.setItem(THEME_STORAGE_KEY, 'dark');
expect(getInitialTheme()).toBe('light');
});
Expand Down
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/App/ThemeStorage.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2025 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

import { getConfigValue } from '../../utils/config/get-config';
import getConfig from '../../utils/config/get-config';

export type ThemeMode = 'light' | 'dark';

Expand Down Expand Up @@ -50,7 +50,7 @@ export function writeStoredTheme(mode: ThemeMode, targetWindow?: Window | null)
}

export function getInitialTheme(): ThemeMode {
if (!getConfigValue('themes.enabled')) {
if (!getConfig().themes?.enabled) {
return DEFAULT_MODE;
}
const stored = readStoredTheme();
Expand Down
20 changes: 6 additions & 14 deletions packages/jaeger-ui/src/components/App/TopNav.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,16 @@ jest.mock('../../utils/config/get-config', () => {
return {
__esModule: true,
default: jest.fn(() => ({
dependencies: { menuEnabled: true },
deepDependencies: { menuEnabled: true },
qualityMetrics: {
menuEnabled: true,
menuLabel: 'Quality',
apiEndpoint: '/quality-metrics',
},
storageCapabilities: { metricsStorage: true },
themes: { enabled: true },
})),
getConfigValue: jest.fn(key => {
switch (key) {
case 'dependencies.menuEnabled':
case 'deepDependencies.menuEnabled':
case 'qualityMetrics.menuEnabled':
case 'monitor.menuEnabled':
case 'themes.enabled':
return true;
case 'qualityMetrics.menuLabel':
return 'Quality';
default:
return false;
}
}),
};
});

Expand Down
17 changes: 7 additions & 10 deletions packages/jaeger-ui/src/components/App/TopNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import * as diffUrl from '../TraceDiff/url';
import * as monitorATMUrl from '../Monitor/url';
import { ReduxState } from '../../types';
import { ConfigMenuItem, ConfigMenuGroup } from '../../types/config';
import { getConfigValue } from '../../utils/config/get-config';
import getConfig from '../../utils/config/get-config';
import prefixUrl from '../../utils/prefix-url';

import './TopNav.css';
Expand All @@ -40,34 +40,31 @@ const NAV_LINKS = [
},
];

if (getConfigValue('dependencies.menuEnabled')) {
if (getConfig().dependencies?.menuEnabled) {
NAV_LINKS.push({
to: dependencyGraph.getUrl(),
matches: dependencyGraph.matches,
text: 'System Architecture',
});
}

if (getConfigValue('deepDependencies.menuEnabled')) {
if (getConfig().deepDependencies?.menuEnabled) {
NAV_LINKS.push({
to: deepDependencies.getUrl(),
matches: deepDependencies.matches,
text: 'Service Dependencies',
});
}

if (getConfigValue('qualityMetrics.menuEnabled')) {
if (getConfig().qualityMetrics?.menuEnabled) {
NAV_LINKS.push({
to: qualityMetrics.getUrl(),
matches: qualityMetrics.matches,
text: getConfigValue('qualityMetrics.menuLabel'),
text: getConfig().qualityMetrics?.menuLabel ?? '',
});
Comment on lines +59 to 64
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qualityMetrics.menuLabel is optional, but falling back to an empty string can produce a blank/invisible nav item when qualityMetrics.menuEnabled is true and the embedded config omits menuLabel (e.g., if it overwrites the whole qualityMetrics object). Prefer a meaningful fallback label (e.g., the default-config label) or skip adding the nav entry when the label is missing.

Copilot uses AI. Check for mistakes.
}

// Show the Monitor item in the top nav when enabled in configuration.
// The Monitor page itself inspects storage capabilities and, if metrics
// storage is not supported, shows a landing page with setup instructions.
if (getConfigValue('monitor.menuEnabled')) {
if (getConfig().storageCapabilities?.metricsStorage) {
NAV_LINKS.push({
Comment on lines +67 to 68
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical Logic Change: This line changes the condition from monitor.menuEnabled to storageCapabilities?.metricsStorage, which is not a type-safe refactor but an actual behavior change.

Old code:

if (getConfigValue('monitor.menuEnabled')) {

New code:

if (getConfig().storageCapabilities?.metricsStorage) {

Impact: The Monitor menu item will now appear based on storage capabilities instead of the explicit monitor.menuEnabled config flag. This breaks the documented behavior in the comment above (lines 67-69 in old code) which states the Monitor page itself should handle storage capability checks while the menu is controlled by monitor.menuEnabled.

Fix: Change to getConfig().monitor?.menuEnabled to preserve the original logic.

Suggested change
if (getConfig().storageCapabilities?.metricsStorage) {
NAV_LINKS.push({
if (getConfig().monitor?.menuEnabled) {
NAV_LINKS.push({

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

to: monitorATMUrl.getUrl(),
matches: monitorATMUrl.matches,
Expand Down Expand Up @@ -123,7 +120,7 @@ export function TopNavImpl(props: Props) {
}
return { label: <CustomNavDropdown key={m.label} {...m} />, key: m.label };
}),
...(getConfigValue('themes.enabled')
...(getConfig().themes?.enabled
? [
{
label: <ThemeToggleButton />,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ describe('<SidePanel>', () => {
let trackDecorationViewDetailsSpy;

beforeAll(() => {
getConfigValueSpy = jest.spyOn(getConfig, 'getConfigValue').mockReturnValue(mockConfig);
getConfigValueSpy = jest
.spyOn(getConfig, 'default')
.mockReturnValue({ pathAgnosticDecorations: mockConfig });
trackDecorationSelectedSpy = jest.spyOn(track, 'trackDecorationSelected');
trackDecorationViewDetailsSpy = jest.spyOn(track, 'trackDecorationViewDetails');
});
Expand Down Expand Up @@ -85,7 +87,7 @@ describe('<SidePanel>', () => {

describe('render', () => {
it('renders null if there are no decorations', () => {
getConfigValueSpy.mockReturnValueOnce(undefined);
getConfigValueSpy.mockReturnValueOnce({});
const { container } = render(<SidePanel />);
expect(container.firstChild).toBeNull();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { IoExitOutline, IoInformationCircleOutline } from 'react-icons/io5';

import { TDdgVertex } from '../../../model/ddg/types';
import { TPathAgnosticDecorationSchema } from '../../../model/path-agnostic-decorations/types';
import { getConfigValue } from '../../../utils/config/get-config';
import getConfig from '../../../utils/config/get-config';
import DetailsPanel from './DetailsPanel';
import * as track from './index.track';

Expand All @@ -23,10 +23,7 @@ type TProps = {
const SidePanel: React.FC<TProps> = props => {
const { clearSelected, selectDecoration, selectedDecoration, selectedVertex } = props;

const decorations: TPathAgnosticDecorationSchema[] | undefined = useMemo(
() => getConfigValue('pathAgnosticDecorations'),
[]
);
const decorations = useMemo(() => getConfig().pathAgnosticDecorations, []);

useEffect(() => {
track.trackDecorationSelected(selectedDecoration);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ describe('DeepDependencyGraphPage', () => {
let getSearchUrlSpy;

beforeAll(() => {
getConfigValueSpy = jest.spyOn(getConfig, 'getConfigValue');
getConfigValueSpy = jest.spyOn(getConfig, 'default');
getSearchUrlSpy = jest.spyOn(getSearchUrl, 'getUrl');
});

Expand Down Expand Up @@ -738,7 +738,7 @@ describe('DeepDependencyGraphPage', () => {
const expectedHeader = 'There are no dependencies';
const { operation, service } = props.urlState;
const lookback = 'test look back';
getConfigValueSpy.mockReturnValue(lookback);
getConfigValueSpy.mockReturnValue({ search: { maxLookback: { value: lookback } } });
const mockUrl = 'test search url';
getSearchUrlSpy.mockReturnValue(mockUrl);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
TDdgVertex,
} from '../../model/ddg/types';
import { encode, encodeDistance } from '../../model/ddg/visibility-codec';
import { getConfigValue } from '../../utils/config/get-config';
import getConfig from '../../utils/config/get-config';
import { ReduxState } from '../../types';
import { TDdgStateEntry } from '../../types/TDdgState';

Expand Down Expand Up @@ -316,7 +316,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent<TProps, TSt
</>
);
} else {
const lookback = getConfigValue('search.maxLookback.value');
const lookback = getConfig().search?.maxLookback?.value;
const checkLink = getSearchUrl({
lookback,
minDuration: '0ms',
Expand Down
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/DependencyGraph/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import ErrorMessage from '../common/ErrorMessage';
import LoadingIndicator from '../common/LoadingIndicator';
import * as jaegerApiActions from '../../actions/jaeger-api';
import { FALLBACK_DAG_MAX_NUM_SERVICES } from '../../constants';
import { getConfigValue } from '../../utils/config/get-config';
import getConfig from '../../utils/config/get-config';
import { extractUiFindFromState } from '../common/UiFindInput';

import './index.css';
Expand All @@ -29,7 +29,7 @@ export const GRAPH_TYPES = {
};
export const sampleDatasetTypes = ['Backend', 'Small Graph', 'Large Graph'];

const dagMaxNumServices = getConfigValue('dependencies.dagMaxNumServices') || FALLBACK_DAG_MAX_NUM_SERVICES;
const dagMaxNumServices = getConfig().dependencies?.dagMaxNumServices ?? FALLBACK_DAG_MAX_NUM_SERVICES;

type TServiceCall = {
parent: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,28 @@ import React from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import '@testing-library/jest-dom';
import MonitorATMEmptyState from '.';
import { getConfigValue } from '../../../utils/config/get-config';
import getConfig from '../../../utils/config/get-config';

jest.mock('../../../utils/config/get-config', () => ({
getConfigValue: jest.fn(),
__esModule: true,
default: jest.fn(),
}));

describe('<MonitorATMEmptyState>', () => {
const mockClickHandler = jest.fn();

beforeEach(() => {
getConfigValue.mockReturnValue({
mainTitle: 'Get started with Service Performance Monitoring',
subTitle: 'subtitle',
description: 'description',
button: {
text: 'Click Me',
onClick: mockClickHandler,
getConfig.mockReturnValue({
monitor: {
emptyState: {
mainTitle: 'Get started with Service Performance Monitoring',
subTitle: 'subtitle',
description: 'description',
button: {
text: 'Click Me',
onClick: mockClickHandler,
},
},
},
});

Expand Down
Loading