Fix stats in slow logs to be a escaped JSON (elastic#44642)

Fields in JSON logs should be an escaped JSON fields. It is a broken json value at the moment
"stats": "["group1", "group2"]", -> "stats": "[\"group1\", \"group2\"]",
This should later be refactored into a JSON array of strings (the same as types in 7.x)

Author: pgomulka

server/src/main/java/org/elasticsearch/common/logging/ESLogMessage.java

@@ -19,9 +19,11 @@
 
 package org.elasticsearch.common.logging;
 
+import com.fasterxml.jackson.core.io.JsonStringEncoder;
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.common.SuppressLoggerChecks;
 
+import java.nio.charset.Charset;
 import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -30,6 +32,7 @@
  * A base class for custom log4j logger messages. Carries additional fields which will populate JSON fields in logs.
  */
 public abstract class ESLogMessage extends ParameterizedMessage {
+    private static final JsonStringEncoder JSON_STRING_ENCODER = JsonStringEncoder.getInstance();
     private final Map<String, Object> fields;
 
     /**
@@ -42,6 +45,11 @@ public ESLogMessage(Map<String, Object> fields, String messagePattern, Object...
         this.fields = fields;
     }
 
+    public static String escapeJson(String text) {
+        byte[] sourceEscaped = JSON_STRING_ENCODER.quoteAsUTF8(text);
+        return new String(sourceEscaped, Charset.defaultCharset());
+    }
+
     public String getValueFor(String key) {
         Object value = fields.get(key);
         return value!=null ? value.toString() : null;

server/src/main/java/org/elasticsearch/index/SearchSlowLog.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.index;
 
-import com.fasterxml.jackson.core.io.JsonStringEncoder;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.common.Strings;
@@ -33,7 +32,6 @@
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.tasks.Task;
 
-import java.nio.charset.Charset;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -173,14 +171,13 @@ private static Map<String, Object> prepareMap(SearchContext context, long tookIn
             }
             String[] types = context.getQueryShardContext().getTypes();
             messageFields.put("types", asJsonArray(types != null ? Arrays.stream(types) : Stream.empty()));
-            messageFields.put("stats", asJsonArray(context.groupStats() != null ? context.groupStats().stream() : Stream.empty()));
+            messageFields.put("stats", escapeJson(asJsonArray(
+                context.groupStats() != null ? context.groupStats().stream() : Stream.empty())));
             messageFields.put("search_type", context.searchType());
             messageFields.put("total_shards", context.numberOfShards());
 
             if (context.request().source() != null) {
-                byte[] sourceEscaped = JsonStringEncoder.getInstance()
-                                                        .quoteAsUTF8(context.request().source().toString(FORMAT_PARAMS));
-                String source = new String(sourceEscaped, Charset.defaultCharset());
+                String source = escapeJson(context.request().source().toString(FORMAT_PARAMS));
 
                 messageFields.put("source", source);
             } else {

server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java

@@ -42,7 +42,9 @@
 import org.hamcrest.Matchers;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.endsWith;
@@ -55,6 +57,9 @@
 public class SearchSlowLogTests extends ESSingleNodeTestCase {
     @Override
     protected SearchContext createSearchContext(IndexService indexService) {
+       return createSearchContext(indexService, new String[]{});
+    }
+    protected SearchContext createSearchContext(IndexService indexService, String ... groupStats) {
         BigArrays bigArrays = indexService.getBigArrays();
         ThreadPool threadPool = indexService.getThreadPool();
         return new TestSearchContext(bigArrays, indexService) {
@@ -150,6 +155,12 @@ public String getClusterAlias() {
                     return null;
                 }
             };
+
+            @Override
+            public List<String> groupStats() {
+                return Arrays.asList(groupStats);
+            }
+
             @Override
             public ShardSearchRequest request() {
                 return request;
@@ -177,6 +188,26 @@ public void testSlowLogHasJsonFields() throws IOException {
         assertThat(p.getValueFor("source"), equalTo("{\\\"query\\\":{\\\"match_all\\\":{\\\"boost\\\":1.0}}}"));
     }
 
+    public void testSlowLogsWithStats() throws IOException {
+        IndexService index = createIndex("foo");
+        SearchContext searchContext = createSearchContext(index,"group1");
+        SearchSourceBuilder source = SearchSourceBuilder.searchSource().query(QueryBuilders.matchAllQuery());
+        searchContext.request().source(source);
+        searchContext.setTask(new SearchTask(0, "n/a", "n/a", "test", null,
+            Collections.singletonMap(Task.X_OPAQUE_ID, "my_id")));
+
+        SearchSlowLog.SearchSlowLogMessage p = new SearchSlowLog.SearchSlowLogMessage(searchContext, 10);
+        assertThat(p.getValueFor("stats"), equalTo("[\\\"group1\\\"]"));
+
+        searchContext = createSearchContext(index, "group1", "group2");
+        source = SearchSourceBuilder.searchSource().query(QueryBuilders.matchAllQuery());
+        searchContext.request().source(source);
+        searchContext.setTask(new SearchTask(0, "n/a", "n/a", "test", null,
+            Collections.singletonMap(Task.X_OPAQUE_ID, "my_id")));
+        p = new SearchSlowLog.SearchSlowLogMessage(searchContext, 10);
+        assertThat(p.getValueFor("stats"), equalTo("[\\\"group1\\\", \\\"group2\\\"]"));
+    }
+
     public void testSlowLogSearchContextPrinterToLog() throws IOException {
         IndexService index = createIndex("foo");
         SearchContext searchContext = createSearchContext(index);