Implement error_trace parameter for HTTP Bulk requests

CHANGELOG.md

@@ -28,6 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add async periodic flush task support for pull-based ingestion ([#19878](https://github.com/opensearch-project/OpenSearch/pull/19878))
 - Add support for context aware segments ([#19098](https://github.com/opensearch-project/OpenSearch/pull/19098))
 - Implement GRPC FunctionScoreQuery ([#19888](https://github.com/opensearch-project/OpenSearch/pull/19888))
+- Implement error_trace parameter for bulk requests ([#19985](https://github.com/opensearch-project/OpenSearch/pull/19985))
 - Allow the truncate filter in normalizers ([#19778](https://github.com/opensearch-project/OpenSearch/issues/19778))
 - Support pull-based ingestion message mappers and raw payload support ([#19765](https://github.com/opensearch-project/OpenSearch/pull/19765)]
 

qa/smoke-test-http/src/test/java/org/opensearch/http/DetailedErrorsDisabledIT.java

@@ -34,17 +34,26 @@
 
 import java.io.IOException;
 
+import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.ParseException;
+import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
 import org.apache.hc.core5.http.io.entity.EntityUtils;
+import org.opensearch.action.search.MultiSearchRequest;
+import org.opensearch.action.search.SearchRequest;
 import org.opensearch.client.Request;
 import org.opensearch.client.Response;
 import org.opensearch.client.ResponseException;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.core.xcontent.MediaType;
+import org.opensearch.core.xcontent.MediaTypeRegistry;
+import org.opensearch.index.query.QueryStringQueryBuilder;
+import org.opensearch.search.builder.SearchSourceBuilder;
 import org.opensearch.test.OpenSearchIntegTestCase.ClusterScope;
 import org.opensearch.test.OpenSearchIntegTestCase.Scope;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 
 /**
  * Tests that when disabling detailed errors, a request with the error_trace parameter returns an HTTP 400 response.
@@ -73,4 +82,19 @@ public void testThatErrorTraceParamReturns400() throws IOException, ParseExcepti
                    containsString("\"error\":\"error traces in responses are disabled.\""));
         assertThat(response.getStatusLine().getStatusCode(), is(400));
     }
+
+    public void testDetailedStackTracesAreNotIncludedWhenErrorTraceIsDisabledForBulkApis() throws IOException, ParseException {
+        MediaType contentType = MediaTypeRegistry.JSON;
+        MultiSearchRequest multiSearchRequest = new MultiSearchRequest().add(
+            new SearchRequest("missing_index")
+                .source(new SearchSourceBuilder().query(new QueryStringQueryBuilder("foo").field("*"))));
+        Request request = new Request("POST", "/_msearch");
+        byte[] requestBody = MultiSearchRequest.writeMultiLineFormat(multiSearchRequest, contentType.xContent());
+        request.setEntity(new ByteArrayEntity(requestBody, ContentType.APPLICATION_JSON));
+
+        Response response = getRestClient().performRequest(request);
+
+        assertThat(EntityUtils.toString(response.getEntity()), not(containsString("stack_trace")));
+    }
+
 }

qa/smoke-test-http/src/test/java/org/opensearch/http/DetailedErrorsEnabledIT.java

@@ -32,16 +32,24 @@
 
 package org.opensearch.http;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+
+import java.io.IOException;
+
+import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.ParseException;
+import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
 import org.apache.hc.core5.http.io.entity.EntityUtils;
+import org.opensearch.action.search.MultiSearchRequest;
+import org.opensearch.action.search.SearchRequest;
 import org.opensearch.client.Request;
 import org.opensearch.client.Response;
 import org.opensearch.client.ResponseException;
-
-import java.io.IOException;
-
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.not;
+import org.opensearch.core.xcontent.MediaType;
+import org.opensearch.core.xcontent.MediaTypeRegistry;
+import org.opensearch.index.query.QueryStringQueryBuilder;
+import org.opensearch.search.builder.SearchSourceBuilder;
 
 /**
  * Tests that by default the error_trace parameter can be used to show stacktraces
@@ -54,23 +62,40 @@ public void testThatErrorTraceWorksByDefault() throws IOException, ParseExceptio
             request.addParameter("error_trace", "true");
             getRestClient().performRequest(request);
             fail("request should have failed");
-        } catch(ResponseException e) {
+        } catch (ResponseException e) {
             Response response = e.getResponse();
             assertThat(response.getHeader("Content-Type"), containsString("application/json"));
             assertThat(EntityUtils.toString(response.getEntity()),
-                    containsString("\"stack_trace\":\"OpenSearchException[Validation Failed: 1: index / indices is missing;]; " +
+                containsString("\"stack_trace\":\"OpenSearchException[Validation Failed: 1: index / indices is missing;]; " +
                     "nested: ActionRequestValidationException[Validation Failed: 1:"));
         }
 
         try {
             getRestClient().performRequest(new Request("DELETE", "/"));
             fail("request should have failed");
-        } catch(ResponseException e) {
+        } catch (ResponseException e) {
             Response response = e.getResponse();
             assertThat(response.getHeader("Content-Type"), containsString("application/json; charset=UTF-8"));
             assertThat(EntityUtils.toString(response.getEntity()),
-                    not(containsString("\"stack_trace\":\"[Validation Failed: 1: index / indices is missing;]; "
+                not(containsString("\"stack_trace\":\"[Validation Failed: 1: index / indices is missing;]; "
                     + "nested: ActionRequestValidationException[Validation Failed: 1:")));
         }
     }
+
+    public void testDetailedStackTracesAreIncludedWhenErrorTraceIsExplicitlyEnabledForBulkApis() throws IOException, ParseException {
+        MediaType contentType = MediaTypeRegistry.JSON;
+        MultiSearchRequest multiSearchRequest = new MultiSearchRequest().add(
+            new SearchRequest("missing_index")
+                .source(new SearchSourceBuilder().query(new QueryStringQueryBuilder("foo").field("*"))));
+        Request request = new Request("POST", "/_msearch");
+        request.addParameter("error_trace", "true");
+        byte[] requestBody = MultiSearchRequest.writeMultiLineFormat(multiSearchRequest, contentType.xContent());
+        request.setEntity(new ByteArrayEntity(requestBody, ContentType.APPLICATION_JSON));
+
+        Response response = getRestClient().performRequest(request);
+
+        assertThat(EntityUtils.toString(response.getEntity()),
+            containsString("\"stack_trace\":\"[missing_index] IndexNotFoundException[no such index [missing_index]]"));
+    }
+
 }

rest-api-spec/src/main/resources/rest-api-spec/test/bulk/100_error_traces.yml

@@ -0,0 +1,34 @@
+setup:
+  - skip:
+      version: " - 3.3.99"
+      reason: "Error trace support was added in 3.4"
+---
+"Returns a detailed error stack trace when requested":
+
+  - do:
+      bulk:
+        error_trace: true
+        body:
+          - '{"update": {"_index": "index_does_not_exist_321", "_id": "123"}}'
+          - '{"doc": {"field": "value"}}'
+
+  - match: { errors: true }
+  - match: { items.0.update.status: 404 }
+  - match: { items.0.update.error.type: document_missing_exception }
+  - match: { items.0.update.error.reason: "[123]: document missing" }
+  - match: { items.0.update.error.stack_trace: "/.*\\[\\[index_does_not_exist_321\\]\\[0\\].*DocumentMissingException\\[\\[123\\]:.*document.*missing\\].*/" }
+---
+"Skips a detailed error stack trace when requested":
+
+  - do:
+      bulk:
+        error_trace: false
+        body:
+          - '{"update": {"_index": "index_does_not_exist_321", "_id": "123"}}'
+          - '{"doc": {"field": "value"}}'
+
+  - match: { errors: true }
+  - match: { items.0.update.status: 404 }
+  - match: { items.0.update.error.type: document_missing_exception }
+  - match: { items.0.update.error.reason: "[123]: document missing" }
+  - match: { items.0.update.error.stack_trace: null }

rest-api-spec/src/main/resources/rest-api-spec/test/mget/90_error_traces.yml

@@ -0,0 +1,36 @@
+setup:
+  - skip:
+      version: " - 3.3.99"
+      reason: "Error trace support was added in 3.4"
+---
+"Returns a detailed error stack trace when requested":
+
+  - do:
+      mget:
+        error_trace: true
+        index: index_does_not_exist_321
+        body:
+          ids: [ 1 ]
+
+  - match: { docs.0.error.type: index_not_found_exception }
+  - match: { docs.0.error.reason: "no such index [index_does_not_exist_321]" }
+  - match: { docs.0.error.stack_trace: "/.*\\[index_does_not_exist_321\\].*IndexNotFoundException.*/" }
+  - match: { docs.0.error.root_cause.0.type: index_not_found_exception }
+  - match: { docs.0.error.root_cause.0.reason: "no such index [index_does_not_exist_321]" }
+  - match: { docs.0.error.root_cause.0.stack_trace: "/.*\\[index_does_not_exist_321\\].*IndexNotFoundException.*/" }
+---
+"Skips a detailed error stack trace when requested":
+
+  - do:
+      mget:
+        error_trace: false
+        index: index_does_not_exist_321
+        body:
+          ids: [ 1 ]
+
+  - match: { docs.0.error.type: index_not_found_exception }
+  - match: { docs.0.error.reason: "no such index [index_does_not_exist_321]" }
+  - match: { docs.0.error.stack_trace: null }
+  - match: { docs.0.error.root_cause.0.type: index_not_found_exception }
+  - match: { docs.0.error.root_cause.0.reason: "no such index [index_does_not_exist_321]" }
+  - match: { docs.0.error.root_cause.0.stack_trace: null }

rest-api-spec/src/main/resources/rest-api-spec/test/msearch/30_error_traces.yml

@@ -0,0 +1,40 @@
+setup:
+  - skip:
+      version: " - 3.3.99"
+      reason: "Error trace support was added in 3.4"
+---
+"Returns a detailed error stack trace when requested":
+
+  - do:
+      msearch:
+        error_trace: true
+        body:
+          - index: index_does_not_exist_321
+          - query:
+              match: { foo: foo }
+
+  - match: { responses.0.status: 404 }
+  - match: { responses.0.error.type: index_not_found_exception }
+  - match: { responses.0.error.reason: "no such index [index_does_not_exist_321]" }
+  - match: { responses.0.error.stack_trace: "/.*\\[index_does_not_exist_321\\].*IndexNotFoundException.*/" }
+  - match: { responses.0.error.root_cause.0.type: index_not_found_exception }
+  - match: { responses.0.error.root_cause.0.reason: "no such index [index_does_not_exist_321]" }
+  - match: { responses.0.error.root_cause.0.stack_trace: "/.*\\[index_does_not_exist_321\\].*IndexNotFoundException.*/" }
+---
+"Skips a detailed error stack trace when requested":
+
+  - do:
+      msearch:
+        error_trace: false
+        body:
+          - index: index_does_not_exist_321
+          - query:
+              match: { foo: foo }
+
+  - match: { responses.0.status: 404 }
+  - match: { responses.0.error.type: index_not_found_exception }
+  - match: { responses.0.error.reason: "no such index [index_does_not_exist_321]" }
+  - match: { responses.0.error.stack_trace: null }
+  - match: { responses.0.error.root_cause.0.type: index_not_found_exception }
+  - match: { responses.0.error.root_cause.0.reason: "no such index [index_does_not_exist_321]" }
+  - match: { responses.0.error.root_cause.0.stack_trace: null }

rest-api-spec/src/main/resources/rest-api-spec/test/mtermvectors/30_error_traces.yml

@@ -0,0 +1,34 @@
+setup:
+  - skip:
+      version: " - 3.3.99"
+      reason: "Error trace support was added in 3.4"
+---
+"Returns a detailed error stack trace when requested":
+
+  - do:
+      mtermvectors:
+        "error_trace": true
+        "index": "index_does_not_exist_321"
+        "ids": [ "testing_document" ]
+
+  - match: { docs.0.error.type: index_not_found_exception }
+  - match: { docs.0.error.reason: "no such index [index_does_not_exist_321]" }
+  - match: { docs.0.error.stack_trace: "/.*\\[index_does_not_exist_321\\].*IndexNotFoundException.*/" }
+  - match: { docs.0.error.root_cause.0.type: index_not_found_exception }
+  - match: { docs.0.error.root_cause.0.reason: "no such index [index_does_not_exist_321]" }
+  - match: { docs.0.error.root_cause.0.stack_trace: "/.*\\[index_does_not_exist_321\\].*IndexNotFoundException.*/" }
+---
+"Skips a detailed error stack trace when requested":
+
+  - do:
+      mtermvectors:
+        "error_trace": false
+        "index": "index_does_not_exist_321"
+        "ids": [ "testing_document" ]
+
+  - match: { docs.0.error.type: index_not_found_exception }
+  - match: { docs.0.error.reason: "no such index [index_does_not_exist_321]" }
+  - match: { docs.0.error.stack_trace: null }
+  - match: { docs.0.error.root_cause.0.type: index_not_found_exception }
+  - match: { docs.0.error.root_cause.0.reason: "no such index [index_does_not_exist_321]" }
+  - match: { docs.0.error.root_cause.0.stack_trace: null }

server/src/main/java/org/opensearch/rest/AbstractRestChannel.java

@@ -64,6 +64,7 @@ public abstract class AbstractRestChannel implements RestChannel {
     private final boolean pretty;
     private final boolean human;
     private final String acceptHeader;
+    private final boolean detailedErrorStackTraceRequested;
 
     private BytesStreamOutput bytesOut;
 
@@ -82,6 +83,7 @@ protected AbstractRestChannel(RestRequest request, boolean detailedErrorsEnabled
         this.filterPath = request.param("filter_path", null);
         this.pretty = request.paramAsBoolean("pretty", false);
         this.human = request.paramAsBoolean("human", false);
+        this.detailedErrorStackTraceRequested = request.paramAsBoolean("error_trace", false);
     }
 
     @Override
@@ -189,4 +191,8 @@ public RestRequest request() {
     public boolean detailedErrorsEnabled() {
         return detailedErrorsEnabled;
     }
+
+    public boolean detailedErrorStackTraceEnabled() {
+        return detailedErrorStackTraceRequested;
+    }
 }

server/src/main/java/org/opensearch/rest/RestChannel.java

@@ -66,5 +66,10 @@ XContentBuilder newBuilder(@Nullable MediaType mediaType, @Nullable MediaType re
      */
     boolean detailedErrorsEnabled();
 
+    /**
+     * @return true if detailed stack traces should be included in the response.
+     */
+    boolean detailedErrorStackTraceEnabled();
+
     void sendResponse(RestResponse response);
 }

server/src/main/java/org/opensearch/rest/RestController.java

@@ -436,7 +436,7 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel
         }
         // error_trace cannot be used when we disable detailed errors
         // we consume the error_trace parameter first to ensure that it is always consumed
-        if (request.paramAsBoolean("error_trace", false) && channel.detailedErrorsEnabled() == false) {
+        if (channel.detailedErrorStackTraceEnabled() && channel.detailedErrorsEnabled() == false) {
             channel.sendResponse(
                 BytesRestResponse.createSimpleErrorResponse(channel, BAD_REQUEST, "error traces in responses are disabled.")
             );
@@ -636,6 +636,11 @@ public boolean detailedErrorsEnabled() {
             return delegate.detailedErrorsEnabled();
         }
 
+        @Override
+        public boolean detailedErrorStackTraceEnabled() {
+            return delegate.detailedErrorStackTraceEnabled();
+        }
+
         @Override
         public void sendResponse(RestResponse response) {
             close();
@@ -699,6 +704,11 @@ public boolean detailedErrorsEnabled() {
             return delegate.detailedErrorsEnabled();
         }
 
+        @Override
+        public boolean detailedErrorStackTraceEnabled() {
+            return delegate.detailedErrorStackTraceEnabled();
+        }
+
         @Override
         public void sendResponse(RestResponse response) {
             close();

server/src/main/java/org/opensearch/rest/action/RestStatusToXContentListener.java

@@ -69,7 +69,7 @@ public RestStatusToXContentListener(RestChannel channel, Function<Response, Stri
     @Override
     public RestResponse buildResponse(Response response, XContentBuilder builder) throws Exception {
         assert response.isFragment() == false; // would be nice if we could make default methods final
-        response.toXContent(builder, channel.request());
+        response.toXContent(builder, getRequestParams());
         RestResponse restResponse = new BytesRestResponse(response.status(), builder);
         if (RestStatus.CREATED == restResponse.status()) {
             final String location = extractLocation.apply(response);

server/src/main/java/org/opensearch/rest/action/RestToXContentListener.java

@@ -32,6 +32,7 @@
 
 package org.opensearch.rest.action;
 
+import org.opensearch.OpenSearchException;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.core.xcontent.ToXContent;
 import org.opensearch.core.xcontent.ToXContentObject;
@@ -40,6 +41,8 @@
 import org.opensearch.rest.RestChannel;
 import org.opensearch.rest.RestResponse;
 
+import java.util.Map;
+
 /**
  * A REST based action listener that assumes the response is of type {@link ToXContent} and automatically
  * builds an XContent based response (wrapping the toXContent in startObject/endObject).
@@ -59,11 +62,20 @@ public final RestResponse buildResponse(Response response) throws Exception {
 
     public RestResponse buildResponse(Response response, XContentBuilder builder) throws Exception {
         assert response.isFragment() == false; // would be nice if we could make default methods final
-        response.toXContent(builder, channel.request());
+        response.toXContent(builder, getRequestParams());
         return new BytesRestResponse(getStatus(response), builder);
     }
 
     protected RestStatus getStatus(Response response) {
         return RestStatus.OK;
     }
+
+    protected ToXContent.Params getRequestParams() {
+        ToXContent.Params params = channel.request();
+        // We add detailed stack traces if the request indicates so
+        if (channel.detailedErrorStackTraceEnabled()) {
+            return new ToXContent.DelegatingMapParams(Map.of(OpenSearchException.REST_EXCEPTION_SKIP_STACK_TRACE, "false"), params);
+        }
+        return params;
+    }
 }