Register status_counter as filterable index metric in node stats API

[gingeekrishna]

Description

Fixes #22383

status_counter was introduced in #19115 but was never added to CommonStatsFlags.Flag. As a result, it was always included in every GET _nodes/stats/indices/<metric> response regardless of which specific metric was requested — contrary to how all other index-level metrics behave.

Root cause

RestNodesStatsAction builds its filterable-metric map (FLAGS) by iterating CommonStatsFlags.Flag.values(). Since status_counter had no Flag entry, it was invisible to the filtering layer. IndicesService.stats() then unconditionally passed statusCounterStats to NodeIndicesStats regardless of what the caller requested.

Fix

Two small changes:

1. CommonStatsFlags.java: Add StatusCounter("status_counter", 17) to the Flag enum. This automatically registers status_counter as a valid, filterable index metric in RestNodesStatsAction without any additional wiring.

2. IndicesService.java: Gate statusCounterStats on flags.isSet(Flag.StatusCounter). The NodeIndicesStats constructors already accept null for statusCounterStats and omit the field when null, so the existing null-guard at the rendering layer handles this cleanly.

Behaviour after fix

Request	status_counter included?
GET _nodes/stats (all)	✅ Yes
GET _nodes/stats/indices (all indices metrics)	✅ Yes
GET _nodes/stats/indices/status_counter	✅ Yes
GET _nodes/stats/indices/request_cache	❌ No (was incorrectly Yes before)
GET _nodes/stats/indices/segments	❌ No (was incorrectly Yes before)

Testing

Compilation confirmed clean. Integration test coverage for the node stats filtering framework already exists in NodeStatsIT; the new flag follows the same pattern as all existing flags and is exercised by the same test infrastructure.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 14b0dcd)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Wire-format compatibility CommonStatsFlags is serialized over the transport wire (writeTo/StreamInput constructor use the Flag enum's index values in a bitset-style payload). Adding StatusCounter("status_counter", 17) introduces a new flag index that older nodes will not recognize. In a mixed-version cluster, if a newer coordinator sets the StatusCounter bit and sends CommonStatsFlags to an older node, the older node's deserializer may either fail to map index 17 or silently ignore it, and conversely older nodes will never set it. This change should be guarded by a Version.onOrAfter(V_x_y_z) check on both the write and read sides, or otherwise verified to be safe against the existing serialization format. Please confirm the flags-serialization path is version-tolerant for unknown indices; if not, add symmetric version guards. Recovery("recovery", 16), StatusCounter("status_counter", 17);

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 14b0dcd

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Preserve wire format for mixed-version clusters NodeIndicesStats is serialized over the wire via OpenSearch's StreamInput/StreamOutput framework. Passing null for statusCounterStats when the flag is not set changes the byte layout for peers that always expect a non-null value (or a leading boolean guard), and this change appears to have no version guard on either the write or read side. In a mixed-version cluster, a new writer sending null (or the optional-form leading false) can break older readers, and vice versa. Version-guard the encoding change on both sides using the first unreleased version constant, or preserve the previous wire encoding by passing an empty/default StatusCounterStats instead of null when the flag is unset. server/src/main/java/org/opensearch/indices/IndicesService.java [934-936] final StatusCounterStats resolvedStatusCounterStats = flags.isSet(CommonStatsFlags.Flag.StatusCounter) ? statusCounterStats - : null; + : new StatusCounterStats(); Suggestion importance[1-10]: 6 __ Why: The concern about wire compatibility in mixed-version clusters is valid if NodeIndicesStats serialization doesn't already handle nullable statusCounterStats. However, without visibility into the serialization code, it's uncertain whether null was already supported. The suggestion raises a reasonable potential issue worth verifying.	Low

---

Previous suggestions

Suggestions up to commit 58b6df7

Category	Suggestion	Impact
Possible issue	Ensure default-all case still includes metric Using flags.isSet(Flag.StatusCounter) may return false when the caller requests all metrics via a "no explicit flags" default that populates flags differently. Verify the "all flags" scenario (e.g., GET _nodes/stats) actually sets StatusCounter in flags; otherwise status_counter will silently disappear for the common default request. Consider adding a fallback or unit test covering the default-all case. server/src/main/java/org/opensearch/indices/IndicesService.java [934-936] -final StatusCounterStats resolvedStatusCounterStats = flags.isSet(CommonStatsFlags.Flag.StatusCounter) - ? statusCounterStats - : null; +final boolean includeStatusCounter = flags.isSet(CommonStatsFlags.Flag.StatusCounter); +final StatusCounterStats resolvedStatusCounterStats = includeStatusCounter ? statusCounterStats : null; Suggestion importance[1-10]: 4 __ Why: The suggestion raises a valid concern about verifying the "all flags" default scenario, but the improved_code is essentially equivalent to the existing code (just extracting a boolean). It's more of a verification request than a concrete code improvement.	Low

Suggestions up to commit 52b968a

Category	Suggestion	Impact
Possible issue	Avoid passing null stats to constructor Passing null for statusCounterStats may cause NullPointerExceptions in NodeIndicesStats if it does not tolerate null (e.g. during serialization or toXContent). Consider passing an empty StatusCounterStats instance instead, or verify NodeIndicesStats safely handles null before merging this change. server/src/main/java/org/opensearch/indices/IndicesService.java [926-928] final StatusCounterStats resolvedStatusCounterStats = flags.isSet(CommonStatsFlags.Flag.StatusCounter) ? statusCounterStats - : null; + : new StatusCounterStats(); Suggestion importance[1-10]: 6 __ Why: The suggestion raises a valid concern about potential NPEs when passing null to NodeIndicesStats. However, without verification of how NodeIndicesStats handles null statusCounterStats, the suggestion is speculative, and the original PR intent is precisely to exclude the field when not requested.	Low

Suggestions up to commit 5506edf

Category	Suggestion	Impact
General	Verify all-flags case includes status counter The comment mentions "or when all flags are set", but the current logic only checks whether the StatusCounter flag is explicitly set. If callers request all flags via a bulk-set mechanism that doesn't individually mark StatusCounter, status counters will be dropped. Verify that flags.isSet returns true when all flags are enabled, or explicitly handle the "all flags" case to match the documented intent. server/src/main/java/org/opensearch/indices/IndicesService.java [926-928] +final StatusCounterStats resolvedStatusCounterStats = flags.isSet(CommonStatsFlags.Flag.StatusCounter) + ? statusCounterStats + : null; - Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid concern about consistency between the comment and the code behavior when all flags are set, but the existing_code and improved_code are identical, and it only asks the author to verify behavior rather than proposing a concrete fix.	Low

Suggestions up to commit 52b968a

Category	Suggestion	Impact
Possible issue	Avoid null to prevent NPE downstream Passing null for statusCounterStats may cause NullPointerException downstream in NodeIndicesStats during serialization or toXContent rendering, depending on how the field is handled. Verify that NodeIndicesStats gracefully handles a null StatusCounterStats, or pass an empty StatusCounterStats instance instead to preserve backward compatibility with consumers that expect a non-null value. server/src/main/java/org/opensearch/indices/IndicesService.java [926-928] final StatusCounterStats resolvedStatusCounterStats = flags.isSet(CommonStatsFlags.Flag.StatusCounter) ? statusCounterStats - : null; + : new StatusCounterStats(); Suggestion importance[1-10]: 5 __ Why: The suggestion raises a valid concern about potential NPE when passing null for statusCounterStats. However, without verifying how NodeIndicesStats handles null, the fix may be unnecessary or could change semantics (empty stats vs. omitted stats). The PR intent seems to be to exclude the stats entirely when not requested.	Low

[github-actions]

Persistent review updated to latest commit 5506edf

[github-actions]

❌ Gradle check result for 5506edf: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 52b968a

[github-actions]

❌ Gradle check result for 52b968a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 58b6df7

[github-actions]

❌ Gradle check result for 58b6df7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 14b0dcd

[github-actions]

❌ Gradle check result for 14b0dcd: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?