refactor: Control Monitor tab visibility via storage capabilities of the backend

[hharshhsaini]

Which problem is this PR solving?

Part of #3540 - Control Monitor tab visibility via storage capabilities

Problem

Three overlapping mechanisms controlled Monitor/SPM tab visibility:

• monitor.menuEnabled — static local config flag (default true, always on)
• isATMActivated — fragile runtime detection via HTTP 501 probing
• StorageCapabilities — correct server-reported mechanism, but only had archiveStorage

Changes

Replace isATMActivated and monitor.menuEnabled with a single source of truth: storageCapabilities.metricsStorage, reported by the query service at startup.

• Types: added metricsStorage?: boolean to StorageCapabilities; removed menuEnabled/emptyState from MonitorConfig; deleted MonitorEmptyStateConfig; removed isATMActivated from MetricsReduxState
• Defaults: added metricsStorage: false to default storageCapabilities and index.html; removed monitor.menuEnabled/emptyState from default config
• Reducer: removed isATMActivated state and HTTP 501 detection; 501 errors now stored in serviceError like any other API error
• Components: TopNav reads storageCapabilities.metricsStorage instead of monitor.menuEnabled; ServicesView drops isATMActivated fetch guard and EmptyState render
• Deleted: Monitor/EmptyState/ directory (4 files)
• Tests: removed all isATMActivated references; updated config mocks; deleted obsolete EmptyState tests

[Copilot]

Pull request overview

This PR replaces three overlapping ATM/Monitor tab visibility mechanisms (monitor.menuEnabled, isATMActivated, and partial use of StorageCapabilities) with a single source of truth: storageCapabilities.metricsStorage, reported by the query service at startup.

Changes:

• Added metricsStorage?: boolean to StorageCapabilities and removed MonitorEmptyStateConfig, menuEnabled, and emptyState from MonitorConfig and related types/defaults.
• Removed the isATMActivated runtime 501-detection mechanism from the metrics reducer and all related component/test references.
• Deleted the Monitor/EmptyState/ component directory (4 files) and updated TopNav to gate the Monitor tab on storageCapabilities.metricsStorage.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/types/config.ts	Adds metricsStorage to StorageCapabilities; removes MonitorEmptyStateConfig and cleans up MonitorConfig
src/types/metrics.ts	Removes isATMActivated field from MetricsReduxState
src/reducers/metrics.ts	Removes isATMActivated from initial state and 501-detection logic
src/reducers/metrics.mock.ts	Removes isATMActivated from mock initial state
src/reducers/metrics.test.js	Removes isATMActivated from all expected state objects
src/constants/default-config.ts	Adds metricsStorage: false default; removes monitor.emptyState and menuEnabled
index.html	Adds metricsStorage: false to DEFAULT_STORAGE_CAPABILITIES
src/components/App/TopNav.tsx	Switches Monitor tab guard to storageCapabilities.metricsStorage
src/components/App/TopNav.test.js	Updates mock config key to match new guard
src/components/Monitor/ServicesView/index.tsx	Removes isATMActivated fetch guard and EmptyState render block
src/components/Monitor/ServicesView/index.test.js	Removes isATMActivated from all test props/state
src/components/Monitor/index.test.js	Removes isATMActivated from initial state and deletes EmptyState test
src/components/Monitor/EmptyState/index.tsx	Deleted entirely
src/components/Monitor/EmptyState/index.test.js	Deleted entirely
src/components/Monitor/EmptyState/index.css	Deleted entirely

---

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

[github-actions]

Hi @hharshhsaini, thanks for your contribution! To ensure quality reviews, we limit how many concurrent PRs new contributors can open:

• Open: 7
• Limit: 2

This PR is currently on hold. We will automatically move this into the review queue once your existing PRs are merged or closed.

Please see our Contributing Guidelines for details on our tiered quota policy.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.15%. Comparing base (e7d1e62) to head (88c5ed8).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3554      +/-   ##
==========================================
+ Coverage   88.60%   89.15%   +0.55%     
==========================================
  Files         299      304       +5     
  Lines        9484     9719     +235     
  Branches     2420     2562     +142     
==========================================
+ Hits         8403     8665     +262     
+ Misses       1078     1050      -28     
- Partials        3        4       +1     

☔ 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.

[hharshhsaini]

Codecov Report

✅ All modified and coverable lines are covered by tests. ✅ Project coverage is 88.59%. Comparing base (e7d1e62) to head (6eeca16).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3554      +/-   ##
==========================================
- Coverage   88.60%   88.59%   -0.01%     
==========================================
  Files         299      298       -1     
  Lines        9484     9468      -16     
  Branches     2420     2492      +72     
==========================================
- Hits         8403     8388      -15     
+ Misses       1078     1077       -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:

Codecov reports -0.01% drop due to deletion of Monitor/EmptyState/ (dead code). All modified and coverable lines remain covered per Codecov's own report. The

ServicesView/index.test.js
suite cannot be counted because of a pre-existing redux-first-history module resolution failure on main (unrelated to this PR). Net coverage delta from actual code changes is neutral/positive.

[hharshhsaini]

@yurishkuro please add the appropriate changelog:refactoring label. This is a pure refactoring PR with no user-visible behavior changes.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.

---

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

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

packages/jaeger-ui/src/components/Monitor/ServicesView/index.test.js:291

• Two isATMActivated references remain in index.test.js that were not cleaned up in this refactor:

1. Line 239: isATMActivated: true is still present in the propsNoServices metrics object inside the 'fetches metrics only when services are available' test. This is now a stale property that no longer exists on MetricsReduxState.

2. Lines 270–291: The test 'fetches metrics when isATMActivated is null (initial state)' and its associated isATMActivated: null at line 282 still exist. The PR description explicitly states this test should have been deleted ("Delete the 'fetches metrics when isATMActivated is null' test"), and the comment on line 282 ("Initial state before any metrics fetch") still references the removed concept. These leftover references should be removed.

        isATMActivated: true,
      },
      fetchAllServiceMetrics: mockFetchAllServiceMetrics,
      fetchAggregatedServiceMetrics: mockFetchAggregatedServiceMetrics,
    };
    useServices.mockReturnValue({ data: [], isLoading: false });
    cleanup();
    renderWithRouter(<MonitorATMServicesView {...propsNoServices} />);

    // Should not fetch when no services
    expect(mockFetchAllServiceMetrics).not.toHaveBeenCalled();
    expect(mockFetchAggregatedServiceMetrics).not.toHaveBeenCalled();

    cleanup();
    const propsWithServices = {
      ...props,
      metrics: {
        ...originInitialState,
        serviceMetrics,
        serviceOpsMetrics,
        loading: false,
      },
      fetchAllServiceMetrics: mockFetchAllServiceMetrics,
      fetchAggregatedServiceMetrics: mockFetchAggregatedServiceMetrics,
    };
    useServices.mockReturnValue({ data: ['apple'], isLoading: false });
    renderWithRouter(<MonitorATMServicesView {...propsWithServices} />);
    expect(mockFetchAllServiceMetrics).toHaveBeenCalled();
    expect(mockFetchAggregatedServiceMetrics).toHaveBeenCalled();
  });

  it('fetches metrics when isATMActivated is null (initial state)', () => {
    cleanup();
    mockFetchAllServiceMetrics.mockClear();
    mockFetchAggregatedServiceMetrics.mockClear();

    const propsWithNullATM = {
      ...props,
      metrics: {
        ...originInitialState,
        serviceMetrics: null,
        serviceOpsMetrics: undefined,
        loading: false,
        isATMActivated: null, // Initial state before any metrics fetch
      },
      fetchAllServiceMetrics: mockFetchAllServiceMetrics,
      fetchAggregatedServiceMetrics: mockFetchAggregatedServiceMetrics,
    };
    useServices.mockReturnValue({ data: ['apple'], isLoading: false });
    renderWithRouter(<MonitorATMServicesView {...propsWithNullATM} />);
    expect(mockFetchAllServiceMetrics).toHaveBeenCalled();
    expect(mockFetchAggregatedServiceMetrics).toHaveBeenCalled();
  });

---

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

[hharshhsaini]

@yurishkuro @Parship12 Could you please take a look.

This implements everything from issue #3540. two checks still need a maintainer:

1. 'check-label' needs a 'changelog:refactoring' label
2. 'codecov/project' shows -0.01% but 'codecov/patch' is 100% so all changed lines are covered, the drop is just from deleting the 'EmptyState' dead code

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

---

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

[jkowall]

Thanks for the contribution

[yurishkuro]

PR removes existing configuration options

[hharshhsaini]

PR removes existing configuration options

@yurishkuro the PR follows the implementation steps in this issue exactly -> Step 2 asks to remove menuEnabled from MonitorConfig and Step 3 asks to remove emptyState from defaults.

Could you clarify what you'd like? Options:

1. Keep monitor.menuEnabled as a deprecated alias that maps to storageCapabilities.metricsStorage
2. Remove as specified in the issue (current approach)

Happy to go with whichever direction you prefer.

[github-actions]

PR quota unlocked!

@hharshhsaini, this PR has been moved out of the waiting room and into the active review queue:

• Open: 6
• Limit: 10

Thank you for your patience.

[yurishkuro]

Resolves #3540

how does this resolve the issue if step 1 (server side) is not implemented?

[hharshhsaini]

Resolves #3540

how does this resolve the issue if step 1 (server side) is not implemented?

@yurishkuro , This PR strictly covers the frontend React implementation (Steps 2-7 of the issue design) to remove the fragile isATMActivated ping checks and cleanly wire up storageCapabilities.metricsStorage across the Redux/TypeScript layers.

Because JAEGER_STORAGE_CAPABILITIES is injected at serve-time by the query-service backend, the Step 1 requirement actually requires a companion PR in the main jaegertracing/jaeger Go repository to build and inject the metricsStorage boolean. My intention was to land the UI readiness first, and then follow up with the Go backend PR so the entire pipeline connects. Does this staged approach work for you, or would you prefer I link a companion backend PR before we merge this UI state?"

[yurishkuro]

no it doesn't make sense. If we land this PR the SPM functionality will become permanently disabled since the backend is not going to send the right capabilities.

[yurishkuro]

Please do not follow the roadmap from the issue verbatim. Aim for smaller changes. I think we can first switch the logic to use capability settings instead of config flag, and then clean up unused config flags later.

[hharshhsaini]

@yurishkuro, doing it all at once was far too heavy. I have completely reverted the MonitorEmptyState component deletions and the 501 isATMActivated Redux cleanup logic. I heavily stripped down this PR so it's strictly laser-focused on just swapping out the TopNav configuration read from monitor.menuEnabled over to storageCapabilities.metricsStorage as you requested! We can do the flag cleanups in a later PR...

[hharshhsaini]

@yurishkuro @jkowall The code changes are ready and passing perfectly locally, but the Unit Tests GitHub Action workflow has randomly deadlocked on the Run npm ci caching step for the last few hours.

Since I don't have the permissions to cancel/restart the worker on this repository from my fork, could you please either smash the "Re-run" button on the stuck job, or just squash and merge it manually since the code is approved?

[hharshhsaini]

@yurishkuro @jkowall Thank you so much for the review and merge. As discussed, the capability injection is now live.

For the follow-up cleanup PR, I plan to:

• Strip the isATMActivated Redux state logic and its associated HTTP 501 ping checks from reducers/metrics.ts.
• Delete the Monitor/EmptyState folder/component entirely since the UI no longer needs to query the backend during runtime to test SPM availability.
• Clean up the unused monitor.menuEnabled configurations from the JS types.

Would you like me to tackle all three of these dead-code cleanups in a single PR, or should I break them down further? Let me know