From c0c8ed896e701013b27565d651f2bee4ec55fd22 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Fri, 8 Apr 2022 09:24:42 -0700 Subject: [PATCH] Bug Fix, return default ID when log4j ThreadContext is empty (#538) (#539) (#543) Signed-off-by: penghuo (cherry picked from commit 54b7257415648f7ca1330099525c5682e0f744ad) --- .../org/opensearch/sql/legacy/JdbcTestIT.java | 21 +++++++++++++++++++ .../opensearch/sql/legacy/utils/LogUtils.java | 11 ++++------ .../legacy/unittest/utils/LogUtilsTest.java | 6 +++--- 3 files changed, 28 insertions(+), 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 index 24e75f58ed..3076d1d50e 100644 --- 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 index a8a040262b..4830dd4413 100644 --- 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. *

@@ -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 index d7545ddc90..564ce8c9ea 100644 --- 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