Fixed cache stats behavior for overall /_nodes/stats call

Signed-off-by: Peter Alfonsi <[email protected]>
opensearch-project · May 14, 2024 · c365f30 · c365f30
1 parent cd145bc
commit c365f30
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 1 deletion.
diff --git a/...c/internalClusterTest/java/org/opensearch/indices/CacheStatsAPIIndicesRequestCacheIT.java b/...c/internalClusterTest/java/org/opensearch/indices/CacheStatsAPIIndicesRequestCacheIT.java
@@ -211,6 +211,37 @@ public void testCacheClear() throws Exception {
         checkCacheStatsAPIResponse(statsHolder, List.of(), expectedTotal, true, true);
     }
 
+    public void testNodesStatsResponse() throws Exception {
+        // Test the cache stats responses are in the expected place in XContent when we call the overall API GET /_nodes/stats.
+        String index = "index";
+        Client client = client();
+
+        startIndex(client, index);
+
+        NodesStatsResponse nodeStatsResponse = client.admin()
+            .cluster()
+            .prepareNodesStats("data:true")
+            .all() // This mimics /_nodes/stats
+            .get();
+        XContentBuilder builder = XContentFactory.jsonBuilder();
+        Map<String, String> paramMap = new HashMap<>();
+        ToXContent.Params params = new ToXContent.MapParams(paramMap);
+
+        builder.startObject();
+        nodeStatsResponse.toXContent(builder, params);
+        builder.endObject();
+        Map<String, Object> xContentMap = XContentHelper.convertToMap(MediaTypeRegistry.JSON.xContent(), builder.toString(), true);
+        // Values should be at nodes.[node_id].caches.request_cache
+        // Get the node id
+        Map<String, Object> nodesResponse = (Map<String, Object>) xContentMap.get("nodes");
+        assertEquals(1, nodesResponse.size());
+        String nodeId = nodesResponse.keySet().toArray(String[]::new)[0];
+        Map<String, Object> cachesResponse = (Map<String, Object>) ((Map<String, Object>) nodesResponse.get(nodeId)).get("caches");
+        assertNotNull(cachesResponse);
+        // Request cache should be present in the response
+        assertTrue(cachesResponse.containsKey("request_cache"));
+    }
+
     private void startIndex(Client client, String indexName) throws InterruptedException {
         assertAcked(
             client.admin()

diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStatsFlags.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStatsFlags.java
@@ -138,7 +138,7 @@ public CommonStatsFlags all() {
         includeUnloadedSegments = false;
         includeAllShardIndexingPressureTrackers = false;
         includeOnlyTopIndexingPressureMetrics = false;
-        includeCaches = EnumSet.noneOf(CacheType.class);
+        includeCaches = EnumSet.allOf(CacheType.class);
         levels = new String[0];
         return this;
     }

diff --git a/server/src/main/java/org/opensearch/common/cache/service/NodeCacheStats.java b/server/src/main/java/org/opensearch/common/cache/service/NodeCacheStats.java
@@ -8,6 +8,7 @@
 
 package org.opensearch.common.cache.service;
 
+import org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest;
 import org.opensearch.action.admin.indices.stats.CommonStatsFlags;
 import org.opensearch.common.annotation.ExperimentalApi;
 import org.opensearch.common.cache.CacheType;
@@ -51,13 +52,15 @@ public void writeTo(StreamOutput out) throws IOException {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject(NodesStatsRequest.Metric.CACHE_STATS.metricName());
         for (CacheType type : statsByCache.keySet()) {
             if (flags.getIncludeCaches().contains(type)) {
                 builder.startObject(type.getValue());
                 statsByCache.get(type).toXContent(builder, params);
                 builder.endObject();
             }
         }
+        builder.endObject();
         return builder;
     }