Bug Fix, return default ID when log4j ThreadContext is empty (#538)

Signed-off-by: penghuo <penghuo@gmail.com>
opensearch-project · Apr 5, 2022 · 54b7257 · 54b7257
1 parent 5170a6e
commit 54b7257
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 10 deletions.
diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java
@@ -9,6 +9,7 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 
+import java.io.IOException;
 import org.json.JSONArray;
 import org.json.JSONObject;
 import org.junit.Assert;
@@ -40,6 +41,26 @@ public void testPercentilesQuery() {
     assertThat(percentileRow.getDouble("99.9"), equalTo(39.0));
   }
 
+  // https://github.com/opensearch-project/sql/issues/537
+  @Test
+  public void testSlowQuery() throws IOException {
+    // set slow log threshold = 0s
+    updateClusterSettings(new ClusterSetting(PERSISTENT, "plugins.sql.slowlog", "0"));
+
+    JSONObject response = executeJdbcRequest(
+        "SELECT percentiles(age, 25.0, 50.0, 75.0, 99.9) age_percentiles " +
+            "FROM opensearch-sql_test_index_people");
+
+    assertThat(response.getJSONArray("datarows").length(), equalTo(1));
+    JSONObject percentileRow = (JSONObject) response.query("/datarows/0/0");
+    assertThat(percentileRow.getDouble("25.0"), equalTo(31.5));
+    assertThat(percentileRow.getDouble("50.0"), equalTo(33.5));
+    assertThat(percentileRow.getDouble("75.0"), equalTo(36.5));
+    assertThat(percentileRow.getDouble("99.9"), equalTo(39.0));
+
+    wipeAllClusterSettings();
+  }
+
   @Ignore("flaky test, trigger resource not enough exception. "
       + "ORDER BY date_format(insert_time, 'dd-MM-YYYY') can't be pushed down ")
   public void testDateTimeInQuery() {

diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/utils/LogUtils.java b/legacy/src/main/java/org/opensearch/sql/legacy/utils/LogUtils.java
@@ -7,6 +7,7 @@
 package org.opensearch.sql.legacy.utils;
 
 import java.util.Map;
+import java.util.Optional;
 import java.util.UUID;
 import org.apache.logging.log4j.ThreadContext;
 
@@ -20,6 +21,8 @@ public class LogUtils {
      */
     private static final String REQUEST_ID_KEY = "request_id";
 
+    private static final String EMPTY_ID = "ID";
+
     /**
      * Generates a random UUID and adds to the {@link ThreadContext} as the request id.
      * <p>
@@ -38,13 +41,7 @@ public static void addRequestId() {
      * @return the current request id from {@link ThreadContext}
      */
     public static String getRequestId() {
-
-        final String requestId = ThreadContext.get(REQUEST_ID_KEY);
-        if (requestId == null) {
-            throw new IllegalStateException("Request id not present in current context");
-        }
-
-        return requestId;
+        return Optional.ofNullable(ThreadContext.get(REQUEST_ID_KEY)).orElseGet(() -> EMPTY_ID);
     }
 
     /**

diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/LogUtilsTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/LogUtilsTest.java
@@ -8,6 +8,7 @@
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertEquals;
 
 import org.apache.logging.log4j.ThreadContext;
 import org.junit.After;
@@ -44,10 +45,9 @@ public void addRequestId_alreadyExists() {
         Assert.assertThat(requestId2, not(equalTo(requestId)));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test
     public void getRequestId_doesNotExist() {
-
-        LogUtils.getRequestId();
+        assertEquals("ID", LogUtils.getRequestId());
     }
 
     @Test