chore: replace getConfigValue with direct getConfig() property access

[Copilot]

getConfigValue(path: string) accepted arbitrary string paths and returned any, making typos silently return undefined at runtime and forcing callers to add unverified type annotations or casts.

Changes

• Deleted getConfigValue from get-config.ts along with the now-unused lodash/get import
• Migrated all ~20 call sites to direct getConfig() property access, removing manual type annotations and casts that only existed to compensate for the any return type:

// before
const docsLink = getConfigValue('monitor.docsLink');  // any; typo = silent undefined
const schemas = getConfigValue('pathAgnosticDecorations') as TPathAgnosticDecorationSchema[] | undefined;

// after
const docsLink = getConfig().monitor?.docsLink;        // string | undefined, compiler-checked
const schemas = getConfig().pathAgnosticDecorations;   // readonly TPathAgnosticDecorationSchema[] | undefined

• Type fixes surfaced by the migration:

  • link-patterns.ts: replaced .filter(Boolean) with a type predicate to properly narrow ProcessedLinkPattern[]
  • transform-trace-data.ts: cast readonly string[] → string[] for orderTags parameter
  • TopNav.tsx: added ?? '' fallback for qualityMetrics.menuLabel (string | undefined)
  • DependencyGraph/index.tsx: replaced || with ?? for dagMaxNumServices fallback

• Updated all test files that mocked getConfigValue to mock the getConfig default export instead, passing structured config objects rather than scalar values

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[chore]: getConfigValue provides no type safety for config access</issue_title>
<issue_description>## Problem

getConfigValue accepts an arbitrary string path and returns any:

// get-config.ts
export function getConfigValue(path: string) {
  return _get(getConfig(), path);
}

This means two classes of error go undetected at compile time:

1. Mistyped or renamed paths are silently undefined at runtime

getConfigValue('monitor.menuEnable')      // typo — returns undefined, no error
getConfigValue('storagecapabilities.metricsStorage') // wrong case — returns undefined

A recent bug in this codebase was caused by exactly this: storageCapabilities.metricsStorage
was accessed through getConfigValue and the result was used as a boolean gate, but the path
was wrong in an earlier version of the code and silently returned undefined (falsy), causing
the Monitor page to always show the empty state.

2. Return types are any, so callers must annotate manually or cast

// Caller must add an explicit annotation just to get type safety:
const config: MonitorEmptyStateConfig = getConfigValue('monitor.emptyState');

// Or cast, which defeats the purpose entirely:
const schemas = getConfigValue('pathAgnosticDecorations') as TPathAgnosticDecorationSchema[] | undefined;

These manual annotations are not checked against the actual type at that path in Config —
they are just asserted. A refactor that changes the Config type will not flag these call sites.

Scope: getConfigValue is called in ~20 places across the codebase. getConfig() is used
directly in a handful of places and is already safe by construction (TypeScript checks property
access against the Config type).

Proposal — Replace getConfigValue calls with direct getConfig() access

For all call sites where the path is a fixed, known string, replace:

// before
const docsLink = getConfigValue('monitor.docsLink');
const enabled: boolean = getConfigValue('storageCapabilities.metricsStorage');

with direct property access:

// after
const docsLink = getConfig().monitor.docsLink;           // type: string | undefined
const enabled = getConfig().storageCapabilities.metricsStorage; // type: boolean

This requires no new infrastructure and is already how tracking/index.ts and
TraceTimelineViewer/duck.ts access config. The memoization in getConfig() means there is
no performance difference.

Suggested migration order

1. Switch all getConfigValue call sites to direct getConfig() property access, removing
   any manual type annotations or casts that were only needed to compensate for the any
   return type. Affected files: TopNav.tsx, Monitor/ServicesView/index.tsx,
   Monitor/EmptyState/index.tsx, ThemeStorage.ts, DependencyGraph/index.tsx,
   link-patterns.ts, path-agnostic-decorations.ts, DeepDependencies/index.tsx,
   DeepDependencies/SidePanel/index.tsx, SearchForm.tsx, SearchResults/AltViewOptions.tsx,
   TraceId.tsx, TraceDiffGraph.tsx, transform-trace-data.ts, get-target.ts,
   process-scripts.ts.
2. Delete getConfigValue from get-config.ts and remove its export from any index files.
3. Delete the getConfigValue tests, or convert them to test getConfig() directly.
   </issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [chore]: getConfigValue provides no type safety for config access #3588

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[graphite-app]

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



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

[Copilot]

Pull request overview

This PR removes the unsafe getConfigValue(path: string): any helper and migrates callers to direct, type-checked getConfig() property access across jaeger-ui, updating tests accordingly.

Changes:

• Deleted getConfigValue from get-config.ts (and removed the lodash/get dependency).
• Migrated config access at call sites to getConfig().… with optional chaining / nullish coalescing where needed.
• Updated Jest tests to mock / spy on the getConfig default export instead of getConfigValue.

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/jaeger-ui/src/utils/config/process-scripts.ts	Uses getConfig().scripts instead of string-path lookup.
packages/jaeger-ui/src/utils/config/process-scripts.test.js	Mocks getConfig default export to return structured config.
packages/jaeger-ui/src/utils/config/get-target.ts	Reads forbidNewPage via getConfig() property access.
packages/jaeger-ui/src/utils/config/get-target.test.js	Updates mocks to getConfig default export.
packages/jaeger-ui/src/utils/config/get-config.ts	Removes getConfigValue and lodash/get import.
packages/jaeger-ui/src/utils/config/get-config.test.js	Deletes getConfigValue tests and import.
packages/jaeger-ui/src/model/transform-trace-data.ts	Replaces getConfigValue('topTagPrefixes') with getConfig().topTagPrefixes and adjusts typing.
packages/jaeger-ui/src/model/link-patterns.ts	Switches to getConfig().linkPatterns and adds a type-predicate filter.
packages/jaeger-ui/src/components/common/TraceId.tsx	Reads traceIdDisplayLength via getConfig().
packages/jaeger-ui/src/components/common/TraceId.test.js	Mocks getConfig default export.
packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.tsx	Reads traceDiff.helpLink via getConfig().traceDiff?.helpLink.
packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.test.js	Spies on getConfig default export and returns structured config.
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx	Uses getConfig().deepDependencies?.menuEnabled.
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js	Updates config mocking to getConfig default export.
packages/jaeger-ui/src/components/SearchTracePage/SearchForm.tsx	Uses getConfig().search?.maxLimit for max constraint.
packages/jaeger-ui/src/components/Monitor/index.tsx	Gates services vs empty state on getConfig().storageCapabilities?.metricsStorage.
packages/jaeger-ui/src/components/Monitor/index.test.js	Updates mock config object to include storageCapabilities.metricsStorage.
packages/jaeger-ui/src/components/Monitor/ServicesView/index.tsx	Reads monitor.docsLink via getConfig().monitor?.docsLink.
packages/jaeger-ui/src/components/Monitor/ServicesView/index.test.js	Updates mock config shape for monitor docs link.
packages/jaeger-ui/src/components/Monitor/EmptyState/index.tsx	Reads monitor.emptyState via getConfig().monitor?.emptyState ?? {}.
packages/jaeger-ui/src/components/Monitor/EmptyState/index.test.js	Updates mocks to return nested monitor.emptyState.
packages/jaeger-ui/src/components/DependencyGraph/index.tsx	Uses ?? fallback with getConfig().dependencies?.dagMaxNumServices.
packages/jaeger-ui/src/components/DeepDependencies/index.tsx	Reads search.maxLookback.value via getConfig().search?.maxLookback?.value.
packages/jaeger-ui/src/components/DeepDependencies/index.test.js	Updates config spy to return nested search.maxLookback.value.
packages/jaeger-ui/src/components/DeepDependencies/SidePanel/index.tsx	Uses getConfig().pathAgnosticDecorations for decorations.
packages/jaeger-ui/src/components/DeepDependencies/SidePanel/index.test.js	Updates config spy to return pathAgnosticDecorations.
packages/jaeger-ui/src/components/App/TopNav.tsx	Switches nav feature gates to getConfig() property reads (incl. monitor / quality metrics).
packages/jaeger-ui/src/components/App/TopNav.test.js	Updates mock config structure for nav gating and labels.
packages/jaeger-ui/src/components/App/ThemeStorage.ts	Uses getConfig().themes?.enabled instead of getConfigValue.
packages/jaeger-ui/src/components/App/ThemeStorage.test.ts	Mocks getConfig default export to enable/disable themes.
packages/jaeger-ui/src/components/App/ThemeProvider.test.tsx	Mocks getConfig default export to enable/disable themes.
packages/jaeger-ui/src/actions/path-agnostic-decorations.ts	Reads pathAgnosticDecorations via getConfig() directly.
packages/jaeger-ui/src/actions/path-agnostic-decorations.test.js	Updates spies/mocks to return structured getConfig() results.

Comments suppressed due to low confidence (1)

packages/jaeger-ui/src/components/App/TopNav.tsx:73

• TopNav now gates the "Monitor" nav link solely on storageCapabilities.metricsStorage. However, Config still defines monitor.menuEnabled and the default config sets it to true, so configs that explicitly disable the monitor menu will no longer be honored, and the Monitor empty-state/setup page also becomes undiscoverable when metrics storage is disabled. Either (a) include getConfig().monitor?.menuEnabled in this condition (optionally in addition to metricsStorage, depending on intended UX), or (b) remove/deprecate monitor.menuEnabled from the config type + default config if it is no longer supported to avoid misleading configuration.

if (getConfig().storageCapabilities?.metricsStorage) {
  NAV_LINKS.push({
    to: monitorATMUrl.getUrl(),
    matches: monitorATMUrl.matches,
    text: 'Monitor',
  });
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

This test description says "forbidNewPage is true" but the mocked config sets forbidNewPage: false, so the wording is misleading when reading failures. Update the it(...) text to reflect the false case.

[Copilot]

This test description says "forbidNewPage is true" but the mocked config sets forbidNewPage: false, so the wording is misleading when reading failures. Update the it(...) text to reflect the false case.

[Copilot]

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.

[codecov]

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.12%. Comparing base (372ba8e) to head (75ccfdb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/jaeger-ui/src/components/App/TopNav.tsx	83.33%	0 Missing and 1 partial ⚠️
packages/jaeger-ui/src/model/link-patterns.ts	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3589      +/-   ##
==========================================
- Coverage   89.14%   89.12%   -0.02%     
==========================================
  Files         304      304              
  Lines        9716     9713       -3     
  Branches     2592     2587       -5     
==========================================
- Hits         8661     8657       -4     
- Misses       1052     1053       +1     
  Partials        3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.