From a68e709b59b3a6b66dafd36a8c83993242458d0e Mon Sep 17 00:00:00 2001 From: Ben Chaplin Date: Fri, 24 Oct 2025 10:50:55 -0400 Subject: [PATCH 1/3] Fix error trace tests --- muted-tests.yml | 3 --- .../http/SearchErrorTraceIT.java | 12 ++++----- .../search/ErrorTraceHelper.java | 18 +++++++++---- .../xpack/search/AsyncSearchErrorTraceIT.java | 27 ++++++++----------- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index 550be4d453dcc..9c2a1a039286e 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -360,9 +360,6 @@ tests: - class: org.elasticsearch.search.CCSDuelIT method: testTermsAggsWithProfile issue: https://github.com/elastic/elasticsearch/issues/132880 -- class: org.elasticsearch.xpack.search.AsyncSearchErrorTraceIT - method: testAsyncSearchFailingQueryErrorTraceDefault - issue: https://github.com/elastic/elasticsearch/issues/133010 - class: org.elasticsearch.xpack.security.authc.AuthenticationServiceTests method: testInvalidToken issue: https://github.com/elastic/elasticsearch/issues/133328 diff --git a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java index f407592f63132..f1bd549e19796 100644 --- a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java +++ b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java @@ -68,8 +68,8 @@ public void testSearchFailingQueryErrorTraceDefault() throws IOException { } } """); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testSearchFailingQueryErrorTraceTrue() throws IOException { @@ -87,8 +87,8 @@ public void testSearchFailingQueryErrorTraceTrue() throws IOException { } """); searchRequest.addParameter("error_trace", "true"); + ErrorTraceHelper.expectStackTraceObserved(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceObserved(internalCluster()); } public void testSearchFailingQueryErrorTraceFalse() throws IOException { @@ -106,8 +106,8 @@ public void testSearchFailingQueryErrorTraceFalse() throws IOException { } """); searchRequest.addParameter("error_trace", "false"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testDataNodeLogsStackTrace() throws IOException { @@ -155,8 +155,8 @@ public void testMultiSearchFailingQueryErrorTraceDefault() throws IOException { searchRequest.setEntity( new NByteArrayEntity(requestBody, ContentType.create(contentType.mediaTypeWithoutParameters(), (Charset) null)) ); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testMultiSearchFailingQueryErrorTraceTrue() throws IOException { @@ -172,8 +172,8 @@ public void testMultiSearchFailingQueryErrorTraceTrue() throws IOException { new NByteArrayEntity(requestBody, ContentType.create(contentType.mediaTypeWithoutParameters(), (Charset) null)) ); searchRequest.addParameter("error_trace", "true"); + ErrorTraceHelper.expectStackTraceObserved(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceObserved(internalCluster()); } public void testMultiSearchFailingQueryErrorTraceFalse() throws IOException { @@ -189,8 +189,8 @@ public void testMultiSearchFailingQueryErrorTraceFalse() throws IOException { new NByteArrayEntity(requestBody, ContentType.create(contentType.mediaTypeWithoutParameters(), (Charset) null)) ); searchRequest.addParameter("error_trace", "false"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); getRestClient().performRequest(searchRequest); - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testDataNodeLogsStackTraceMultiSearch() throws IOException { diff --git a/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java b/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java index ab8308391b202..0be2c53e6de90 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java +++ b/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java @@ -42,15 +42,23 @@ public enum ErrorTraceHelper { ; - public static void assertStackTraceObserved(InternalTestCluster internalTestCluster) { - assertStackTraceObserved(internalTestCluster, true); + /** + * Sets up transport interception to assert that stack traces are present in error responses for batched query requests. + * Must be called before executing requests that are expected to generate errors. + */ + public static void expectStackTraceObserved(InternalTestCluster internalTestCluster) { + expectStackTraceObserved(internalTestCluster, true); } - public static void assertStackTraceCleared(InternalTestCluster internalTestCluster) { - assertStackTraceObserved(internalTestCluster, false); + /** + * Sets up transport interception to assert that stack traces are NOT present in error responses for batched query requests. + * Must be called before executing requests that are expected to generate errors. + */ + public static void expectStackTraceCleared(InternalTestCluster internalTestCluster) { + expectStackTraceObserved(internalTestCluster, false); } - private static void assertStackTraceObserved(InternalTestCluster internalCluster, boolean shouldObserveStackTrace) { + private static void expectStackTraceObserved(InternalTestCluster internalCluster, boolean shouldObserveStackTrace) { internalCluster.getDataNodeInstances(TransportService.class) .forEach( ts -> asInstanceOf(MockTransportService.class, ts).addRequestHandlingBehavior( diff --git a/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java b/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java index c132d38dd2792..2996289d87012 100644 --- a/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java +++ b/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java @@ -75,14 +75,13 @@ public void testAsyncSearchFailingQueryErrorTraceDefault() throws Exception { """); createAsyncRequest.addParameter("keep_on_completion", "true"); createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest); - if (createAsyncResponseEntity.get("is_running").equals("true")) { + if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was not sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception { @@ -102,15 +101,14 @@ public void testAsyncSearchFailingQueryErrorTraceTrue() throws Exception { createAsyncRequest.addParameter("error_trace", "true"); createAsyncRequest.addParameter("keep_on_completion", "true"); createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceObserved(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest); - if (createAsyncResponseEntity.get("is_running").equals("true")) { + if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId); getAsyncRequest.addParameter("error_trace", "true"); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceObserved(internalCluster()); } public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception { @@ -130,15 +128,14 @@ public void testAsyncSearchFailingQueryErrorTraceFalse() throws Exception { createAsyncRequest.addParameter("error_trace", "false"); createAsyncRequest.addParameter("keep_on_completion", "true"); createAsyncRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest); - if (createAsyncResponseEntity.get("is_running").equals("true")) { + if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId); getAsyncRequest.addParameter("error_trace", "false"); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was not sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testDataNodeLogsStackTrace() throws Exception { @@ -172,7 +169,7 @@ public void testDataNodeLogsStackTrace() throws Exception { try (var mockLog = MockLog.capture(SearchService.class)) { ErrorTraceHelper.addSeenLoggingExpectations(numShards, mockLog, errorTriggeringIndex); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncRequest); - if (createAsyncResponseEntity.get("is_running").equals("true")) { + if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId); // Use the same value of error_trace as the search request @@ -205,15 +202,14 @@ public void testAsyncSearchFailingQueryErrorTraceFalseOnSubmitAndTrueOnGet() thr createAsyncSearchRequest.addParameter("error_trace", "false"); createAsyncSearchRequest.addParameter("keep_on_completion", "true"); createAsyncSearchRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceCleared(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest); - if (createAsyncResponseEntity.get("is_running").equals("true")) { + if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId); getAsyncRequest.addParameter("error_trace", "true"); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was not sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceCleared(internalCluster()); } public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() throws Exception { @@ -233,15 +229,14 @@ public void testAsyncSearchFailingQueryErrorTraceTrueOnSubmitAndFalseOnGet() thr createAsyncSearchRequest.addParameter("error_trace", "true"); createAsyncSearchRequest.addParameter("keep_on_completion", "true"); createAsyncSearchRequest.addParameter("wait_for_completion_timeout", "0ms"); + ErrorTraceHelper.expectStackTraceObserved(internalCluster()); Map createAsyncResponseEntity = performRequestAndGetResponseEntity(createAsyncSearchRequest); - if (createAsyncResponseEntity.get("is_running").equals("true")) { + if (Boolean.TRUE.equals(createAsyncResponseEntity.get("is_running"))) { String asyncExecutionId = (String) createAsyncResponseEntity.get("id"); Request getAsyncRequest = new Request("GET", "/_async_search/" + asyncExecutionId); getAsyncRequest.addParameter("error_trace", "false"); awaitAsyncRequestDoneRunning(getAsyncRequest); } - // check that the stack trace was sent from the data node to the coordinating node - ErrorTraceHelper.assertStackTraceObserved(internalCluster()); } private Map performRequestAndGetResponseEntity(Request r) throws IOException { From f7f14233d791e9dc9ce004d73eaa929e2059ab80 Mon Sep 17 00:00:00 2001 From: Ben Chaplin Date: Fri, 24 Oct 2025 13:19:50 -0400 Subject: [PATCH 2/3] Clear added handling after each test --- .../http/SearchErrorTraceIT.java | 6 ++++ .../search/ErrorTraceHelper.java | 28 +++++++++++-------- .../xpack/search/AsyncSearchErrorTraceIT.java | 6 ++++ 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java index f1bd549e19796..45a2d8aefcc58 100644 --- a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java +++ b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java @@ -24,6 +24,7 @@ import org.elasticsearch.test.MockLog; import org.elasticsearch.test.transport.MockTransportService; import org.elasticsearch.xcontent.XContentType; +import org.junit.After; import org.junit.BeforeClass; import java.io.IOException; @@ -44,6 +45,11 @@ public static void setDebugLogLevel() { Configurator.setLevel(SearchService.class, Level.DEBUG); } + @After + public void clearExpectations() { + ErrorTraceHelper.clearExpectations(internalCluster()); + } + private void setupIndexWithDocs() { createIndex("test1", "test2"); indexRandom( diff --git a/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java b/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java index 0be2c53e6de90..fcda14a69702c 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java +++ b/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java @@ -46,16 +46,16 @@ public enum ErrorTraceHelper { * Sets up transport interception to assert that stack traces are present in error responses for batched query requests. * Must be called before executing requests that are expected to generate errors. */ - public static void expectStackTraceObserved(InternalTestCluster internalTestCluster) { - expectStackTraceObserved(internalTestCluster, true); + public static void expectStackTraceObserved(InternalTestCluster internalCluster) { + expectStackTraceObserved(internalCluster, true); } /** * Sets up transport interception to assert that stack traces are NOT present in error responses for batched query requests. * Must be called before executing requests that are expected to generate errors. */ - public static void expectStackTraceCleared(InternalTestCluster internalTestCluster) { - expectStackTraceObserved(internalTestCluster, false); + public static void expectStackTraceCleared(InternalTestCluster internalCluster) { + expectStackTraceObserved(internalCluster, false); } private static void expectStackTraceObserved(InternalTestCluster internalCluster, boolean shouldObserveStackTrace) { @@ -88,21 +88,22 @@ public void sendResponse(TransportResponse response) { } catch (IOException e) { throw new UncheckedIOException(e); } finally { + // Always forward to the original channel + channel.sendResponse(response); if (nodeQueryResponse != null) { nodeQueryResponse.decRef(); } } - - // Forward to the original channel - channel.sendResponse(response); } @Override public void sendResponse(Exception error) { - inspectStackTraceAndAssert(error); - - // Forward to the original channel - channel.sendResponse(error); + try { + inspectStackTraceAndAssert(error); + } finally { + // Always forward to the original channel + channel.sendResponse(error); + } } private void inspectStackTraceAndAssert(Exception error) { @@ -123,6 +124,11 @@ private void inspectStackTraceAndAssert(Exception error) { ); } + public static void clearExpectations(InternalTestCluster internalCluster) { + internalCluster.getDataNodeInstances(TransportService.class) + .forEach(ts -> asInstanceOf(MockTransportService.class, ts).clearAllRules()); + } + /** * Adds expectations for debug logging of a message and exception on each shard of the given index. * diff --git a/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java b/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java index 2996289d87012..11800c52a2843 100644 --- a/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java +++ b/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java @@ -21,6 +21,7 @@ import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.test.transport.MockTransportService; import org.elasticsearch.xcontent.XContentType; +import org.junit.After; import org.junit.BeforeClass; import java.io.IOException; @@ -49,6 +50,11 @@ public static void setDebugLogLevel() { Configurator.setLevel(SearchService.class, Level.DEBUG); } + @After + public void clearExpectations() { + ErrorTraceHelper.clearExpectations(internalCluster()); + } + private void setupIndexWithDocs() { createIndex("test1", "test2"); indexRandom( From 2626c52750238661b13f5d2558446830df6a28aa Mon Sep 17 00:00:00 2001 From: Ben Chaplin Date: Fri, 24 Oct 2025 13:53:52 -0400 Subject: [PATCH 3/3] Remove clean up (it's already done in ESIntegTestCase) --- .../java/org/elasticsearch/http/SearchErrorTraceIT.java | 6 ------ .../java/org/elasticsearch/search/ErrorTraceHelper.java | 5 ----- .../elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java | 6 ------ 3 files changed, 17 deletions(-) diff --git a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java index 45a2d8aefcc58..f1bd549e19796 100644 --- a/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java +++ b/qa/smoke-test-http/src/internalClusterTest/java/org/elasticsearch/http/SearchErrorTraceIT.java @@ -24,7 +24,6 @@ import org.elasticsearch.test.MockLog; import org.elasticsearch.test.transport.MockTransportService; import org.elasticsearch.xcontent.XContentType; -import org.junit.After; import org.junit.BeforeClass; import java.io.IOException; @@ -45,11 +44,6 @@ public static void setDebugLogLevel() { Configurator.setLevel(SearchService.class, Level.DEBUG); } - @After - public void clearExpectations() { - ErrorTraceHelper.clearExpectations(internalCluster()); - } - private void setupIndexWithDocs() { createIndex("test1", "test2"); indexRandom( diff --git a/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java b/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java index fcda14a69702c..1c7c8a5960313 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java +++ b/test/framework/src/main/java/org/elasticsearch/search/ErrorTraceHelper.java @@ -124,11 +124,6 @@ private void inspectStackTraceAndAssert(Exception error) { ); } - public static void clearExpectations(InternalTestCluster internalCluster) { - internalCluster.getDataNodeInstances(TransportService.class) - .forEach(ts -> asInstanceOf(MockTransportService.class, ts).clearAllRules()); - } - /** * Adds expectations for debug logging of a message and exception on each shard of the given index. * diff --git a/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java b/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java index 11800c52a2843..2996289d87012 100644 --- a/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java +++ b/x-pack/plugin/async-search/src/internalClusterTest/java/org/elasticsearch/xpack/search/AsyncSearchErrorTraceIT.java @@ -21,7 +21,6 @@ import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.test.transport.MockTransportService; import org.elasticsearch.xcontent.XContentType; -import org.junit.After; import org.junit.BeforeClass; import java.io.IOException; @@ -50,11 +49,6 @@ public static void setDebugLogLevel() { Configurator.setLevel(SearchService.class, Level.DEBUG); } - @After - public void clearExpectations() { - ErrorTraceHelper.clearExpectations(internalCluster()); - } - private void setupIndexWithDocs() { createIndex("test1", "test2"); indexRandom(