diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e1d096868dea..64cc98b852c83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added - Add support for forward translog reading ([#20163](https://github.com/opensearch-project/OpenSearch/pull/20163)) - Added public getter method in `SourceFieldMapper` to return excluded field ([#20205](https://github.com/opensearch-project/OpenSearch/pull/20205)) +- Add handling for the grpc.detailed_errors.enabled setting and global gRPC error_trace parameter ([#19644](https://github.com/opensearch-project/OpenSearch/pull/19644)) - Add integ test for simulating node join left event when data node cluster state publication lag because the cluster applier thread being busy ([#19907](https://github.com/opensearch-project/OpenSearch/pull/19907)). - Relax jar hell check when extended plugins share transitive dependencies ([#20103](https://github.com/opensearch-project/OpenSearch/pull/20103)) - Added public getter method in `SourceFieldMapper` to return included field ([#20290](https://github.com/opensearch-project/OpenSearch/pull/20290)) diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/GrpcPlugin.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/GrpcPlugin.java index c2aa802762a54..200ae101c5453 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/GrpcPlugin.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/GrpcPlugin.java @@ -42,6 +42,7 @@ import org.opensearch.transport.grpc.spi.GrpcServiceFactory; import org.opensearch.transport.grpc.spi.QueryBuilderProtoConverter; import org.opensearch.transport.grpc.ssl.SecureNetty4GrpcServerTransport; +import org.opensearch.transport.grpc.util.GrpcParamsHandler; import org.opensearch.watcher.ResourceWatcherService; import java.util.ArrayList; @@ -58,6 +59,7 @@ import static org.opensearch.transport.grpc.Netty4GrpcServerTransport.GRPC_TRANSPORT_SETTING_KEY; import static org.opensearch.transport.grpc.Netty4GrpcServerTransport.SETTING_GRPC_BIND_HOST; +import static org.opensearch.transport.grpc.Netty4GrpcServerTransport.SETTING_GRPC_DETAILED_ERRORS_ENABLED; import static org.opensearch.transport.grpc.Netty4GrpcServerTransport.SETTING_GRPC_EXECUTOR_COUNT; import static org.opensearch.transport.grpc.Netty4GrpcServerTransport.SETTING_GRPC_HOST; import static org.opensearch.transport.grpc.Netty4GrpcServerTransport.SETTING_GRPC_KEEPALIVE_TIMEOUT; @@ -78,7 +80,9 @@ public final class GrpcPlugin extends Plugin implements NetworkPlugin, ExtensiblePlugin { private static final Logger logger = LogManager.getLogger(GrpcPlugin.class); - /** The name of the gRPC thread pool */ + /** + * The name of the gRPC thread pool + */ public static final String GRPC_THREAD_POOL_NAME = "grpc"; private final List queryConverters = new ArrayList<>(); @@ -182,6 +186,7 @@ public Map> getAuxTransports( } return Collections.singletonMap(GRPC_TRANSPORT_SETTING_KEY, () -> { + GrpcParamsHandler.initialize(settings); List grpcServices = new ArrayList<>( List.of(new DocumentServiceImpl(client), new SearchServiceImpl(client, queryUtils)) ); @@ -233,6 +238,7 @@ public Map> getSecureAuxTransports( throw new RuntimeException("createComponents must be called first to initialize server provided resources."); } return Collections.singletonMap(GRPC_SECURE_TRANSPORT_SETTING_KEY, () -> { + GrpcParamsHandler.initialize(settings); List grpcServices = new ArrayList<>( List.of(new DocumentServiceImpl(client), new SearchServiceImpl(client, queryUtils)) ); @@ -282,7 +288,8 @@ public List> getSettings() { SETTING_GRPC_MAX_MSG_SIZE, SETTING_GRPC_MAX_CONNECTION_AGE, SETTING_GRPC_MAX_CONNECTION_IDLE, - SETTING_GRPC_KEEPALIVE_TIMEOUT + SETTING_GRPC_KEEPALIVE_TIMEOUT, + SETTING_GRPC_DETAILED_ERRORS_ENABLED ); } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/Netty4GrpcServerTransport.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/Netty4GrpcServerTransport.java index 8ea67aa9d7191..c66469f3652a6 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/Netty4GrpcServerTransport.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/Netty4GrpcServerTransport.java @@ -187,6 +187,18 @@ public class Netty4GrpcServerTransport extends AuxTransport { Setting.Property.NodeScope ); + /** + * Enables detailed error message for gRPC requests. If enabled, returns detailed + * error information when requested via {@code error_trace} parameter in the request scope. + * Otherwise, only a summary is generated. The case when the detailed error trace is requested + * but server explicitly turned it off, the error response is generated. + */ + public static final Setting SETTING_GRPC_DETAILED_ERRORS_ENABLED = Setting.boolSetting( + "grpc.detailed_errors.enabled", + true, + Setting.Property.NodeScope + ); + /** * Port range on which servers bind. */ diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListener.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListener.java index 6e1f6306c2aae..020d24b9cd65e 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListener.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListener.java @@ -12,6 +12,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.action.bulk.BulkResponse; import org.opensearch.core.action.ActionListener; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.transport.grpc.proto.response.document.bulk.BulkResponseProtoUtils; import org.opensearch.transport.grpc.util.GrpcErrorHandler; @@ -26,15 +27,18 @@ public class BulkRequestActionListener implements ActionListener { private static final Logger logger = LogManager.getLogger(BulkRequestActionListener.class); private final StreamObserver responseObserver; + private final GlobalParams params; /** * Creates a new BulkRequestActionListener. * * @param responseObserver The gRPC stream observer to send the response back to the client + * @param params parameters that are going to change how responses and errors are handled */ - public BulkRequestActionListener(StreamObserver responseObserver) { + public BulkRequestActionListener(StreamObserver responseObserver, GlobalParams params) { super(); this.responseObserver = responseObserver; + this.params = params; } /** @@ -47,12 +51,12 @@ public BulkRequestActionListener(StreamObserver responseObserver; + private final GlobalParams params; /** * Constructs a new SearchRequestActionListener. * * @param responseObserver the gRPC stream observer to send the search response to + * @param params parameters that are going to change how responses and errors are handled */ - public SearchRequestActionListener(StreamObserver responseObserver) { + public SearchRequestActionListener(StreamObserver responseObserver, GlobalParams params) { super(); this.responseObserver = responseObserver; + this.params = params; } @Override public void onResponse(SearchResponse response) { // Search execution succeeded. Convert the opensearch internal response to protobuf try { - org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto(response); + org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto(response, params); responseObserver.onNext(protoResponse); responseObserver.onCompleted(); } catch (RuntimeException | IOException e) { logger.error("Failed to convert search response to protobuf: " + e.getMessage()); - StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e); + StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e, params); responseObserver.onError(grpcError); } } @@ -55,7 +59,7 @@ public void onResponse(SearchResponse response) { @Override public void onFailure(Exception e) { logger.debug("SearchRequestActionListener failed to process search request: " + e.getMessage()); - StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e); + StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e, params); responseObserver.onError(grpcError); } } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtils.java index 85e9ab47d873c..248663289feb8 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtils.java @@ -10,6 +10,7 @@ import org.opensearch.action.bulk.BulkShardRequest; import org.opensearch.protobufs.BulkRequest; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.document.RestBulkAction; import org.opensearch.search.fetch.subphase.FetchSourceContext; @@ -90,9 +91,10 @@ public static org.opensearch.action.bulk.BulkRequest prepareRequest(BulkRequest throw new UnsupportedOperationException("type param is not supported"); } - // TODO support global_params - if (request.hasGlobalParams()) { - throw new UnsupportedOperationException("global_params param is not supported yet"); + // TODO support global_params parameters + GlobalParams params = request.getGlobalParams(); + if (params.hasHuman() || params.getFilterPathCount() > 0) { + throw new UnsupportedOperationException("global_params.human or filter_path params are not supported yet"); } return bulkRequest; diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtils.java index 0473c46c3156e..3ca0612b98016 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtils.java @@ -15,6 +15,7 @@ import org.opensearch.core.common.io.stream.NamedWriteableRegistry; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.query.QueryBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.SearchRequest; import org.opensearch.protobufs.SearchRequestBody; import org.opensearch.rest.RestRequest; @@ -189,9 +190,10 @@ protected static void parseSearchRequest( throw new UnsupportedOperationException("typed_keys param is not supported yet"); } - // TODO support global_params - if (request.hasGlobalParams()) { - throw new UnsupportedOperationException("global_params param is not supported yet"); + // TODO support global_params parameters + GlobalParams params = request.getGlobalParams(); + if (params.hasHuman() || params.getFilterPathCount() > 0) { + throw new UnsupportedOperationException("global_params.human or filter_path params are not supported yet"); } } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkItemResponseProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkItemResponseProtoUtils.java index 5b876653a102a..b3be006809c3e 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkItemResponseProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkItemResponseProtoUtils.java @@ -14,6 +14,7 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.index.get.GetResult; import org.opensearch.protobufs.ErrorCause; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ResponseItem; import org.opensearch.transport.grpc.proto.response.document.common.DocWriteResponseProtoUtils; import org.opensearch.transport.grpc.proto.response.document.get.GetResultProtoUtils; @@ -39,16 +40,17 @@ private BulkItemResponseProtoUtils() { * * * @param response The BulkItemResponse to convert + * @param params The global gRPC request parameters * @return A Protocol Buffer ResponseItem representation * @throws IOException if there's an error during conversion * */ - public static ResponseItem toProto(BulkItemResponse response) throws IOException { + public static ResponseItem toProto(BulkItemResponse response, GlobalParams params) throws IOException { ResponseItem.Builder responseItemBuilder; if (response.isFailed() == false) { DocWriteResponse docResponse = response.getResponse(); - responseItemBuilder = DocWriteResponseProtoUtils.toProto(docResponse); + responseItemBuilder = DocWriteResponseProtoUtils.toProto(docResponse, params); int grpcStatusCode = RestToGrpcStatusConverter.getGrpcStatusCode(docResponse.status()); responseItemBuilder.setStatus(grpcStatusCode); @@ -63,7 +65,7 @@ public static ResponseItem toProto(BulkItemResponse response) throws IOException int grpcStatusCode = RestToGrpcStatusConverter.getGrpcStatusCode(failure.getStatus()); responseItemBuilder.setStatus(grpcStatusCode); - ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(failure.getCause()); + ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(failure.getCause(), params); responseItemBuilder.setError(errorCause); } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkResponseProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkResponseProtoUtils.java index 0522ad9bb5740..8cc788901ca99 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkResponseProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkResponseProtoUtils.java @@ -11,6 +11,7 @@ import org.opensearch.action.bulk.BulkResponse; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import java.io.IOException; @@ -30,10 +31,11 @@ private BulkResponseProtoUtils() { * This method is equivalent to {@link BulkResponse#toXContent(XContentBuilder, ToXContent.Params)} * * @param response The BulkResponse to convert + * @param params The global gRPC request parameters * @return A Protocol Buffer BulkResponse representation * @throws IOException if there's an error during conversion */ - public static org.opensearch.protobufs.BulkResponse toProto(BulkResponse response) throws IOException { + public static org.opensearch.protobufs.BulkResponse toProto(BulkResponse response, GlobalParams params) throws IOException { // System.out.println("=== grpc bulk response=" + response.toString()); org.opensearch.protobufs.BulkResponse.Builder bulkResponse = org.opensearch.protobufs.BulkResponse.newBuilder(); @@ -51,7 +53,7 @@ public static org.opensearch.protobufs.BulkResponse toProto(BulkResponse respons // Add individual item responses for each operation in the bulk request for (BulkItemResponse bulkItemResponse : response.getItems()) { - org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse); + org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse, params); org.opensearch.protobufs.Item.Builder itemBuilder = org.opensearch.protobufs.Item.newBuilder(); // Wrap ResponseItem in Item based on operation type diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/common/DocWriteResponseProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/common/DocWriteResponseProtoUtils.java index 3dc80830b9518..e50fd0681e85a 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/common/DocWriteResponseProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/common/DocWriteResponseProtoUtils.java @@ -10,6 +10,7 @@ import org.opensearch.action.DocWriteResponse; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ResponseItem; import org.opensearch.protobufs.ShardInfo; @@ -31,10 +32,11 @@ private DocWriteResponseProtoUtils() { * This method is equivalent to the {@link DocWriteResponse#innerToXContent(XContentBuilder, ToXContent.Params)} * * @param response The DocWriteResponse to convert + * @param params The global gRPC request parameters * @return A ResponseItem.Builder with the DocWriteResponse data * */ - public static ResponseItem.Builder toProto(DocWriteResponse response) throws IOException { + public static ResponseItem.Builder toProto(DocWriteResponse response, GlobalParams params) throws IOException { ResponseItem.Builder responseItem = ResponseItem.newBuilder(); // Set the index name @@ -56,7 +58,7 @@ public static ResponseItem.Builder toProto(DocWriteResponse response) throws IOE responseItem.setForcedRefresh(true); } // Handle shard information - ShardInfo shardInfo = ShardInfoProtoUtils.toProto(response.getShardInfo()); + ShardInfo shardInfo = ShardInfoProtoUtils.toProto(response.getShardInfo(), params); responseItem.setXShards(shardInfo); // Set sequence number and primary term if available diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/common/ShardInfoProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/common/ShardInfoProtoUtils.java index 7b519ad6153cd..57be4f19b9fc9 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/common/ShardInfoProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/document/common/ShardInfoProtoUtils.java @@ -10,6 +10,7 @@ import org.opensearch.action.support.replication.ReplicationResponse; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ShardFailure; import org.opensearch.protobufs.ShardInfo; import org.opensearch.transport.grpc.proto.response.exceptions.opensearchexception.OpenSearchExceptionProtoUtils; @@ -30,10 +31,11 @@ private ShardInfoProtoUtils() { * Similar to {@link ReplicationResponse.ShardInfo#toXContent(XContentBuilder, ToXContent.Params)} * * @param shardInfo The shard information to convert to protobuf format + * @param params The global gRPC request parameters * @return The protobuf representation of the shard information * @throws IOException If there's an error during conversion */ - public static ShardInfo toProto(ReplicationResponse.ShardInfo shardInfo) throws IOException { + public static ShardInfo toProto(ReplicationResponse.ShardInfo shardInfo, GlobalParams params) throws IOException { ShardInfo.Builder shardInfoBuilder = ShardInfo.newBuilder(); shardInfoBuilder.setTotal(shardInfo.getTotal()); shardInfoBuilder.setSuccessful(shardInfo.getSuccessful()); @@ -41,7 +43,7 @@ public static ShardInfo toProto(ReplicationResponse.ShardInfo shardInfo) throws // Add any shard failures for (ReplicationResponse.ShardInfo.Failure failure : shardInfo.getFailures()) { - shardInfoBuilder.addFailures(toProto(failure)); + shardInfoBuilder.addFailures(toProto(failure, params)); } return shardInfoBuilder.build(); @@ -55,12 +57,12 @@ public static ShardInfo toProto(ReplicationResponse.ShardInfo shardInfo) throws * @return The protobuf representation of the shard failure * @throws IOException If there's an error during conversion */ - private static ShardFailure toProto(ReplicationResponse.ShardInfo.Failure failure) throws IOException { + private static ShardFailure toProto(ReplicationResponse.ShardInfo.Failure failure, GlobalParams params) throws IOException { ShardFailure.Builder shardFailure = ShardFailure.newBuilder(); shardFailure.setIndex(failure.index()); shardFailure.setShard(failure.shardId()); shardFailure.setNode(failure.nodeId()); - shardFailure.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(failure.getCause())); + shardFailure.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(failure.getCause(), params)); shardFailure.setStatus(failure.status().name()); shardFailure.setPrimary(failure.primary()); return shardFailure.build(); diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/opensearchexception/OpenSearchExceptionProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/opensearchexception/OpenSearchExceptionProtoUtils.java index 757de75993004..1d71d17135178 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/opensearchexception/OpenSearchExceptionProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/opensearchexception/OpenSearchExceptionProtoUtils.java @@ -17,6 +17,7 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.protobufs.ErrorCause; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ObjectMap; import org.opensearch.protobufs.StringArray; import org.opensearch.protobufs.StringOrStringArray; @@ -31,6 +32,7 @@ import org.opensearch.transport.grpc.proto.response.exceptions.SearchParseExceptionProtoUtils; import org.opensearch.transport.grpc.proto.response.exceptions.SearchPhaseExecutionExceptionProtoUtils; import org.opensearch.transport.grpc.proto.response.exceptions.TooManyBucketsExceptionProtoUtils; +import org.opensearch.transport.grpc.util.GrpcParamsHandler; import java.io.IOException; import java.util.AbstractMap; @@ -53,16 +55,18 @@ private OpenSearchExceptionProtoUtils() { /** * Converts an OpenSearchException to its Protocol Buffer representation. - * This method is equivalent to the {@link OpenSearchException#toXContent(XContentBuilder, ToXContent.Params)} + * The corresponding global gRPC request parameters are used to control whether the full stacktrace is included or only a summary. + * This method is equivalent to the {@link OpenSearchException#toXContent(XContentBuilder, ToXContent.Params)}. * * @param exception The OpenSearchException to convert + * @param params Global gRPC params to control how much details of an error should be included * @return A Protocol Buffer ErrorCause representation * @throws IOException if there's an error during conversion */ - public static ErrorCause toProto(OpenSearchException exception) throws IOException { + public static ErrorCause toProto(OpenSearchException exception, GlobalParams params) throws IOException { Throwable ex = ExceptionsHelper.unwrapCause(exception); if (ex != exception) { - return generateThrowableProto(ex); + return generateThrowableProto(ex, params); } else { return innerToProto( exception, @@ -70,7 +74,8 @@ public static ErrorCause toProto(OpenSearchException exception) throws IOExcepti exception.getMessage(), exception.getHeaders(), exception.getMetadata(), - exception.getCause() + exception.getCause(), + params ); } } @@ -80,24 +85,27 @@ public static ErrorCause toProto(OpenSearchException exception) throws IOExcepti * as Protocol Buffers. *

* This method is usually used when the {@link Throwable} is rendered as a part of another Protocol Buffer object. + * The corresponding global gRPC request parameters are used to control whether the full stacktrace is included or only a summary. * It is equivalent to the {@link OpenSearchException#generateThrowableXContent(XContentBuilder, ToXContent.Params, Throwable)} * * @param t The throwable to convert + * @param params Global gRPC params to control how much details of an error should be included * @return A Protocol Buffer ErrorCause representation * @throws IOException if there's an error during conversion */ - public static ErrorCause generateThrowableProto(Throwable t) throws IOException { + public static ErrorCause generateThrowableProto(Throwable t, GlobalParams params) throws IOException { t = ExceptionsHelper.unwrapCause(t); if (t instanceof OpenSearchException ose) { - return toProto(ose); + return toProto(ose, params); } else { - return innerToProto(t, getExceptionName(t), t.getMessage(), emptyMap(), emptyMap(), t.getCause()); + return innerToProto(t, getExceptionName(t), t.getMessage(), emptyMap(), emptyMap(), t.getCause(), params); } } /** * Inner helper method for converting a Throwable to its Protocol Buffer representation. + * The corresponding global gRPC request parameters are used to control whether the full stacktrace is included or only a summary. * This method is equivalent to the {@link OpenSearchException#innerToXContent(XContentBuilder, ToXContent.Params, Throwable, String, String, Map, Map, Throwable)}. * * @param throwable The throwable to convert @@ -106,6 +114,7 @@ public static ErrorCause generateThrowableProto(Throwable t) throws IOException * @param headers The exception headers * @param metadata The exception metadata * @param cause The exception cause + * @param params Global gRPC params to control how much details of an error should be included * @return A Protocol Buffer ErrorCause representation * @throws IOException if there's an error during conversion */ @@ -115,7 +124,8 @@ public static ErrorCause innerToProto( String message, Map> headers, Map> metadata, - Throwable cause + Throwable cause, + GlobalParams params ) throws IOException { ErrorCause.Builder errorCauseBuilder = ErrorCause.newBuilder(); @@ -153,7 +163,7 @@ public static ErrorCause innerToProto( } if (cause != null) { - errorCauseBuilder.setCausedBy(generateThrowableProto(cause)); + errorCauseBuilder.setCausedBy(generateThrowableProto(cause, params)); } if (headers.isEmpty() == false) { @@ -164,13 +174,15 @@ public static ErrorCause innerToProto( } // Add stack trace - errorCauseBuilder.setStackTrace(ExceptionsHelper.stackTrace(throwable)); + if (GrpcParamsHandler.isDetailedStackTraceRequested(params)) { + errorCauseBuilder.setStackTrace(ExceptionsHelper.stackTrace(throwable)); + } // Add suppressed exceptions Throwable[] allSuppressed = throwable.getSuppressed(); if (allSuppressed.length > 0) { for (Throwable suppressed : allSuppressed) { - errorCauseBuilder.addSuppressed(generateThrowableProto(suppressed)); + errorCauseBuilder.addSuppressed(generateThrowableProto(suppressed, params)); } } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/DefaultShardOperationFailedExceptionProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/DefaultShardOperationFailedExceptionProtoUtils.java index a62c357d88f2f..2a1b6b088cad1 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/DefaultShardOperationFailedExceptionProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/DefaultShardOperationFailedExceptionProtoUtils.java @@ -13,6 +13,7 @@ import org.opensearch.core.action.support.DefaultShardOperationFailedException; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ShardFailure; import org.opensearch.transport.grpc.proto.response.exceptions.opensearchexception.OpenSearchExceptionProtoUtils; @@ -33,21 +34,27 @@ private DefaultShardOperationFailedExceptionProtoUtils() { * This method is overridden by various exception classes, which are hardcoded here. * * @param exception The DefaultShardOperationFailedException to convert + * @param params The global gRPC request parameters * @return A Protocol Buffer Struct containing the exception metadata */ - public static ShardFailure toProto(DefaultShardOperationFailedException exception) throws IOException { + public static ShardFailure toProto(DefaultShardOperationFailedException exception, GlobalParams params) throws IOException { ShardFailure.Builder shardFailureBuilder = ShardFailure.newBuilder(); switch (exception) { - case AddIndexBlockResponse.AddBlockShardResult.Failure addBlockFailure -> innerToProto(shardFailureBuilder, addBlockFailure); + case AddIndexBlockResponse.AddBlockShardResult.Failure addBlockFailure -> innerToProto( + shardFailureBuilder, + addBlockFailure, + params + ); case IndicesShardStoresResponse.Failure indicesShardStoresFailure -> innerToProto( shardFailureBuilder, - indicesShardStoresFailure + indicesShardStoresFailure, + params ); - case CloseIndexResponse.ShardResult.Failure closeIndexFailure -> innerToProto(shardFailureBuilder, closeIndexFailure); + case CloseIndexResponse.ShardResult.Failure closeIndexFailure -> innerToProto(shardFailureBuilder, closeIndexFailure, params); case null -> { } - default -> parentInnerToProto(shardFailureBuilder, exception); + default -> parentInnerToProto(shardFailureBuilder, exception, params); } return shardFailureBuilder.build(); } @@ -58,14 +65,18 @@ public static ShardFailure toProto(DefaultShardOperationFailedException exceptio * * @param shardFailureBuilder the builder to populate with failure information * @param exception The AddIndexBlockResponse.AddBlockShardResult.Failure to convert + * @param params The global gRPC request parameters * @throws IOException if there's an error during conversion */ - public static void innerToProto(ShardFailure.Builder shardFailureBuilder, AddIndexBlockResponse.AddBlockShardResult.Failure exception) - throws IOException { + public static void innerToProto( + ShardFailure.Builder shardFailureBuilder, + AddIndexBlockResponse.AddBlockShardResult.Failure exception, + GlobalParams params + ) throws IOException { if (exception.getNodeId() != null) { shardFailureBuilder.setNode(exception.getNodeId()); } - parentInnerToProto(shardFailureBuilder, exception); + parentInnerToProto(shardFailureBuilder, exception, params); } /** @@ -74,12 +85,16 @@ public static void innerToProto(ShardFailure.Builder shardFailureBuilder, AddInd * * @param shardFailureBuilder the builder to populate with failure information * @param exception The IndicesShardStoresResponse.Failure to convert + * @param params The global gRPC request parameters * @throws IOException if there's an error during conversion */ - public static void innerToProto(ShardFailure.Builder shardFailureBuilder, IndicesShardStoresResponse.Failure exception) - throws IOException { + public static void innerToProto( + ShardFailure.Builder shardFailureBuilder, + IndicesShardStoresResponse.Failure exception, + GlobalParams params + ) throws IOException { shardFailureBuilder.setNode(exception.nodeId()); - parentInnerToProto(shardFailureBuilder, exception); + parentInnerToProto(shardFailureBuilder, exception, params); } /** @@ -88,14 +103,18 @@ public static void innerToProto(ShardFailure.Builder shardFailureBuilder, Indice * * @param shardFailureBuilder the builder to populate with failure information * @param exception The CloseIndexResponse.ShardResult.Failure to convert + * @param params The global gRPC request parameters * @throws IOException if there's an error during conversion */ - public static void innerToProto(ShardFailure.Builder shardFailureBuilder, CloseIndexResponse.ShardResult.Failure exception) - throws IOException { + public static void innerToProto( + ShardFailure.Builder shardFailureBuilder, + CloseIndexResponse.ShardResult.Failure exception, + GlobalParams params + ) throws IOException { if (exception.getNodeId() != null) { shardFailureBuilder.setNode(exception.getNodeId()); } - parentInnerToProto(shardFailureBuilder, exception); + parentInnerToProto(shardFailureBuilder, exception, params); } /** @@ -104,17 +123,21 @@ public static void innerToProto(ShardFailure.Builder shardFailureBuilder, CloseI * * @param shardFailureBuilder the builder to populate with failure information * @param exception The DefaultShardOperationFailedException to convert + * @param params The global gRPC request parameters * @throws IOException if there's an error during conversion */ - public static void parentInnerToProto(ShardFailure.Builder shardFailureBuilder, DefaultShardOperationFailedException exception) - throws IOException { + public static void parentInnerToProto( + ShardFailure.Builder shardFailureBuilder, + DefaultShardOperationFailedException exception, + GlobalParams params + ) throws IOException { shardFailureBuilder.setShard(exception.shardId()); if (exception.index() != null) { shardFailureBuilder.setIndex(exception.index()); } shardFailureBuilder.setStatus(exception.status().name()); if (exception.reason() != null) { - shardFailureBuilder.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(exception.getCause())); + shardFailureBuilder.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(exception.getCause(), params)); } } } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ReplicationResponseShardInfoFailureProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ReplicationResponseShardInfoFailureProtoUtils.java index deb1e2fcf4cd1..e650d31ea2b66 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ReplicationResponseShardInfoFailureProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ReplicationResponseShardInfoFailureProtoUtils.java @@ -10,6 +10,7 @@ import org.opensearch.action.support.replication.ReplicationResponse; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ShardFailure; import org.opensearch.transport.grpc.proto.response.exceptions.opensearchexception.OpenSearchExceptionProtoUtils; @@ -29,9 +30,10 @@ private ReplicationResponseShardInfoFailureProtoUtils() { * This method is overridden by various exception classes, which are hardcoded here. * * @param exception The ReplicationResponse.ShardInfo.Failure to convert metadata from + * @param params The global gRPC request parameters * @return A map containing the exception's metadata as ObjectMap.Value objects */ - public static ShardFailure toProto(ReplicationResponse.ShardInfo.Failure exception) throws IOException { + public static ShardFailure toProto(ReplicationResponse.ShardInfo.Failure exception, GlobalParams params) throws IOException { ShardFailure.Builder shardFailure = ShardFailure.newBuilder(); if (exception.index() != null) { shardFailure.setIndex(exception.index()); @@ -40,7 +42,7 @@ public static ShardFailure toProto(ReplicationResponse.ShardInfo.Failure excepti if (exception.nodeId() != null) { shardFailure.setNode(exception.nodeId()); } - shardFailure.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(exception.getCause())); + shardFailure.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(exception.getCause(), params)); shardFailure.setStatus(exception.status().name()); shardFailure.setPrimary(exception.primary()); return shardFailure.build(); diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtils.java index 37c035ee3af39..b5ddae0195424 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtils.java @@ -13,6 +13,7 @@ import org.opensearch.core.action.support.DefaultShardOperationFailedException; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ShardFailure; import org.opensearch.snapshots.SnapshotShardFailure; @@ -32,14 +33,15 @@ private ShardOperationFailedExceptionProtoUtils() { * This method is overridden by various exception classes, which are hardcoded here. * * @param exception The ShardOperationFailedException to convert metadata from + * @param params The global gRPC request parameters * @return ShardFailure */ - public static ShardFailure toProto(ShardOperationFailedException exception) throws IOException { + public static ShardFailure toProto(ShardOperationFailedException exception, GlobalParams params) throws IOException { return switch (exception) { - case ShardSearchFailure ssf -> ShardSearchFailureProtoUtils.toProto(ssf); - case SnapshotShardFailure ssf -> SnapshotShardFailureProtoUtils.toProto(ssf); - case DefaultShardOperationFailedException dsofe -> DefaultShardOperationFailedExceptionProtoUtils.toProto(dsofe); - case ReplicationResponse.ShardInfo.Failure sf -> ReplicationResponseShardInfoFailureProtoUtils.toProto(sf); + case ShardSearchFailure ssf -> ShardSearchFailureProtoUtils.toProto(ssf, params); + case SnapshotShardFailure ssf -> SnapshotShardFailureProtoUtils.toProto(ssf, params); + case DefaultShardOperationFailedException dsofe -> DefaultShardOperationFailedExceptionProtoUtils.toProto(dsofe, params); + case ReplicationResponse.ShardInfo.Failure sf -> ReplicationResponseShardInfoFailureProtoUtils.toProto(sf, params); case null -> throw new UnsupportedOperationException( "Unsupported ShardOperationFailedException [null] cannot be converted to proto." ); diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardSearchFailureProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardSearchFailureProtoUtils.java index 4d8c4555f7389..f0f15ba0704a2 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardSearchFailureProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardSearchFailureProtoUtils.java @@ -10,6 +10,7 @@ import org.opensearch.action.search.ShardSearchFailure; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ShardFailure; import org.opensearch.transport.grpc.proto.response.exceptions.opensearchexception.OpenSearchExceptionProtoUtils; @@ -29,16 +30,17 @@ private ShardSearchFailureProtoUtils() { * Similar to {@link ShardSearchFailure#toXContent(XContentBuilder, ToXContent.Params)} * * * @param exception The ShardSearchFailure to convert + * @param params The global gRPC request parameters * @return A Protocol Buffer Struct containing the exception metadata */ - public static ShardFailure toProto(ShardSearchFailure exception) throws IOException { + public static ShardFailure toProto(ShardSearchFailure exception, GlobalParams params) throws IOException { ShardFailure.Builder shardFailure = ShardFailure.newBuilder(); shardFailure.setShard(exception.shardId()); shardFailure.setIndex(exception.index()); if (exception.shard() != null && exception.shard().getNodeId() != null) { shardFailure.setNode(exception.shard().getNodeId()); } - shardFailure.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(exception.getCause())); + shardFailure.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(exception.getCause(), params)); return shardFailure.build(); } } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/SnapshotShardFailureProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/SnapshotShardFailureProtoUtils.java index a5cf33fad567a..f366f3b48a860 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/SnapshotShardFailureProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/SnapshotShardFailureProtoUtils.java @@ -9,8 +9,12 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ShardFailure; import org.opensearch.snapshots.SnapshotShardFailure; +import org.opensearch.transport.grpc.proto.response.exceptions.opensearchexception.OpenSearchExceptionProtoUtils; + +import java.io.IOException; /** * Utility class for converting SnapshotShardFailure objects to Protocol Buffers. @@ -26,17 +30,19 @@ private SnapshotShardFailureProtoUtils() { * Similar to {@link SnapshotShardFailure#toXContent(XContentBuilder, ToXContent.Params)} * * * @param exception The SnapshotShardFailure to convert + * @param params The global gRPC request parameters * @return A Protocol Buffer Struct containing the exception metadata */ - public static ShardFailure toProto(SnapshotShardFailure exception) { + public static ShardFailure toProto(SnapshotShardFailure exception, GlobalParams params) throws IOException { ShardFailure.Builder shardFailure = ShardFailure.newBuilder(); shardFailure.setIndex(exception.index()); // shardFailure.setIndexUuid(exception.index()); // TODO no field called index_uuid in ShardFailure protos shardFailure.setShard(exception.shardId()); - // shardFailure.setReason(exception.reason()); // TODO ErrorCause type in ShardFailure, not string - shardFailure.setIndex(exception.index()); - shardFailure.setNode(exception.nodeId()); + if (exception.nodeId() != null) { + shardFailure.setNode(exception.nodeId()); + } shardFailure.setStatus(exception.status().name()); + shardFailure.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(exception.getCause(), params)); return shardFailure.build(); } } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/ProtoActionsProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/ProtoActionsProtoUtils.java index 5913c858d7d6d..fe54266c85b0c 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/ProtoActionsProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/ProtoActionsProtoUtils.java @@ -11,6 +11,7 @@ import org.opensearch.core.action.ShardOperationFailedException; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.SearchResponse; import org.opensearch.rest.action.RestActions; @@ -36,6 +37,7 @@ private ProtoActionsProtoUtils() { * @param skipped the number of skipped shards * @param failed the number of failed shards * @param shardFailures the array of shard operation failures + * @param params The global gRPC request parameters * @throws IOException if there's an error during conversion */ protected static void buildBroadcastShardsHeader( @@ -44,8 +46,11 @@ protected static void buildBroadcastShardsHeader( int successful, int skipped, int failed, - ShardOperationFailedException[] shardFailures + ShardOperationFailedException[] shardFailures, + GlobalParams params ) throws IOException { - searchResponseProtoBuilder.setXShards(ShardStatisticsProtoUtils.getShardStats(total, successful, skipped, failed, shardFailures)); + searchResponseProtoBuilder.setXShards( + ShardStatisticsProtoUtils.getShardStats(total, successful, skipped, failed, shardFailures, params) + ); } } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchResponseProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchResponseProtoUtils.java index 287d1f47b9126..63af4dafda628 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchResponseProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/SearchResponseProtoUtils.java @@ -12,6 +12,7 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.protobufs.ClusterStatistics; +import org.opensearch.protobufs.GlobalParams; import java.io.IOException; import java.util.Map; @@ -32,12 +33,13 @@ private SearchResponseProtoUtils() { * This method is equivalent to {@link SearchResponse#toXContent(XContentBuilder, ToXContent.Params)} * * @param response The SearchResponse to convert + * @param params The global gRPC request parameters * @return A Protocol Buffer SearchResponse representation * @throws IOException if there's an error during conversion */ - public static org.opensearch.protobufs.SearchResponse toProto(SearchResponse response) throws IOException { + public static org.opensearch.protobufs.SearchResponse toProto(SearchResponse response, GlobalParams params) throws IOException { org.opensearch.protobufs.SearchResponse.Builder searchResponseProtoBuilder = org.opensearch.protobufs.SearchResponse.newBuilder(); - toProto(response, searchResponseProtoBuilder); + toProto(response, searchResponseProtoBuilder, params); return searchResponseProtoBuilder.build(); } @@ -47,10 +49,14 @@ public static org.opensearch.protobufs.SearchResponse toProto(SearchResponse res * * @param response The SearchResponse to convert * @param searchResponseProtoBuilder The builder to populate with the SearchResponse data + * @param params The global gRPC request parameters * @throws IOException if there's an error during conversion */ - public static void toProto(SearchResponse response, org.opensearch.protobufs.SearchResponse.Builder searchResponseProtoBuilder) - throws IOException { + public static void toProto( + SearchResponse response, + org.opensearch.protobufs.SearchResponse.Builder searchResponseProtoBuilder, + GlobalParams params + ) throws IOException { // Set optional fields only if they exist if (response.getScrollId() != null) { @@ -86,7 +92,8 @@ public static void toProto(SearchResponse response, org.opensearch.protobufs.Sea response.getSuccessfulShards(), response.getSkippedShards(), response.getFailedShards(), - response.getShardFailures() + response.getShardFailures(), + params ); // Add clusters information diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/ShardStatisticsProtoUtils.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/ShardStatisticsProtoUtils.java index 05aad90870ad4..a6667b84024c5 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/ShardStatisticsProtoUtils.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/ShardStatisticsProtoUtils.java @@ -13,6 +13,7 @@ import org.opensearch.core.common.util.CollectionUtils; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.protobufs.ShardStatistics; import org.opensearch.transport.grpc.proto.response.exceptions.shardoperationfailedexception.ShardOperationFailedExceptionProtoUtils; @@ -38,6 +39,7 @@ private ShardStatisticsProtoUtils() { * @param skipped the number of skipped shards * @param failed the number of failed shards * @param shardFailures the array of shard operation failures + * @param params The global gRPC request parameters * @return A Protocol Buffer ShardStatistics representation * @throws IOException if there's an error during conversion */ @@ -46,7 +48,8 @@ protected static ShardStatistics getShardStats( int successful, int skipped, int failed, - ShardOperationFailedException[] shardFailures + ShardOperationFailedException[] shardFailures, + GlobalParams params ) throws IOException { ShardStatistics.Builder shardStats = ShardStatistics.newBuilder(); shardStats.setTotal(total); @@ -57,7 +60,7 @@ protected static ShardStatistics getShardStats( shardStats.setFailed(failed); if (CollectionUtils.isEmpty(shardFailures) == false) { for (ShardOperationFailedException shardFailure : ExceptionsHelper.groupBy(shardFailures)) { - shardStats.addFailures(ShardOperationFailedExceptionProtoUtils.toProto(shardFailure)); + shardStats.addFailures(ShardOperationFailedExceptionProtoUtils.toProto(shardFailure, params)); } } return shardStats.build(); diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/services/DocumentServiceImpl.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/services/DocumentServiceImpl.java index e1348bc78961f..952797febfba1 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/services/DocumentServiceImpl.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/services/DocumentServiceImpl.java @@ -15,6 +15,7 @@ import org.opensearch.transport.grpc.listeners.BulkRequestActionListener; import org.opensearch.transport.grpc.proto.request.document.bulk.BulkRequestProtoUtils; import org.opensearch.transport.grpc.util.GrpcErrorHandler; +import org.opensearch.transport.grpc.util.GrpcParamsHandler; import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; @@ -44,12 +45,13 @@ public DocumentServiceImpl(Client client) { @Override public void bulk(org.opensearch.protobufs.BulkRequest request, StreamObserver responseObserver) { try { + GrpcParamsHandler.validateStackTraceDetailsConfiguration(request.getGlobalParams()); org.opensearch.action.bulk.BulkRequest bulkRequest = BulkRequestProtoUtils.prepareRequest(request); - BulkRequestActionListener listener = new BulkRequestActionListener(responseObserver); + BulkRequestActionListener listener = new BulkRequestActionListener(responseObserver, request.getGlobalParams()); client.bulk(bulkRequest, listener); } catch (RuntimeException e) { logger.debug("DocumentServiceImpl failed: {} - {}", e.getClass().getSimpleName(), e.getMessage()); - StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e); + StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e, request.getGlobalParams()); responseObserver.onError(grpcError); } } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/services/SearchServiceImpl.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/services/SearchServiceImpl.java index f5bca635c19ab..b0ab074efbe1a 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/services/SearchServiceImpl.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/services/SearchServiceImpl.java @@ -16,6 +16,7 @@ import org.opensearch.transport.grpc.proto.request.search.SearchRequestProtoUtils; import org.opensearch.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils; import org.opensearch.transport.grpc.util.GrpcErrorHandler; +import org.opensearch.transport.grpc.util.GrpcParamsHandler; import java.io.IOException; @@ -61,14 +62,14 @@ public void search( org.opensearch.protobufs.SearchRequest request, StreamObserver responseObserver ) { - try { + GrpcParamsHandler.validateStackTraceDetailsConfiguration(request.getGlobalParams()); org.opensearch.action.search.SearchRequest searchRequest = SearchRequestProtoUtils.prepareRequest(request, client, queryUtils); - SearchRequestActionListener listener = new SearchRequestActionListener(responseObserver); + SearchRequestActionListener listener = new SearchRequestActionListener(responseObserver, request.getGlobalParams()); client.search(searchRequest, listener); } catch (RuntimeException | IOException e) { logger.debug("SearchServiceImpl failed to process search request, request=" + request + ", error=" + e.getMessage()); - StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e); + StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e, request.getGlobalParams()); responseObserver.onError(grpcError); } } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/util/GrpcErrorHandler.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/util/GrpcErrorHandler.java index 5313ccd1f1af1..b77871e46cfdd 100644 --- a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/util/GrpcErrorHandler.java +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/util/GrpcErrorHandler.java @@ -21,8 +21,10 @@ import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.protobufs.GlobalParams; import java.io.IOException; +import java.util.Map; import java.util.concurrent.TimeoutException; import io.grpc.Status; @@ -34,6 +36,10 @@ public class GrpcErrorHandler { private static final Logger logger = LogManager.getLogger(GrpcErrorHandler.class); + private static final ToXContent.MapParams INCLUDE_STACK_TRACES = new ToXContent.MapParams( + Map.of(OpenSearchException.REST_EXCEPTION_SKIP_STACK_TRACE, "false") + ); + private GrpcErrorHandler() { // Utility class, no instances } @@ -44,58 +50,78 @@ private GrpcErrorHandler() { * with enhanced XContent details for OpenSearchExceptions and full stack traces for debugging. * * @param e The exception to convert + * @param params * @return StatusRuntimeException with appropriate gRPC status and enhanced error details */ - public static StatusRuntimeException convertToGrpcError(Exception e) { + public static StatusRuntimeException convertToGrpcError(Exception e, GlobalParams params) { + boolean shouldIncludeDetailedStackTrace = GrpcParamsHandler.isDetailedStackTraceRequested(params); // ========== OpenSearch Business Logic Exceptions ========== // Custom OpenSearch exceptions which extend {@link OpenSearchException}. // Uses {@link RestToGrpcStatusConverter} for REST -> gRPC status mapping and // follows {@link OpenSearchException#generateFailureXContent} unwrapping logic switch (e) { case OpenSearchException ose -> { - return handleOpenSearchException(ose); + return handleOpenSearchException(ose, shouldIncludeDetailedStackTrace); } // ========== OpenSearch Core System Exceptions ========== // Low-level OpenSearch exceptions that don't extend OpenSearchException - include full details case OpenSearchRejectedExecutionException osree -> { - return Status.RESOURCE_EXHAUSTED.withDescription(ExceptionsHelper.stackTrace(osree)).asRuntimeException(); + return Status.RESOURCE_EXHAUSTED.withDescription(getErrorDetailsForConfig(osree, shouldIncludeDetailedStackTrace)) + .asRuntimeException(); } case NotXContentException notXContentException -> { - return Status.INVALID_ARGUMENT.withDescription(ExceptionsHelper.stackTrace(notXContentException)).asRuntimeException(); + return Status.INVALID_ARGUMENT.withDescription( + getErrorDetailsForConfig(notXContentException, shouldIncludeDetailedStackTrace) + ).asRuntimeException(); } case NotCompressedException notCompressedException -> { - return Status.INVALID_ARGUMENT.withDescription(ExceptionsHelper.stackTrace(notCompressedException)).asRuntimeException(); + return Status.INVALID_ARGUMENT.withDescription( + getErrorDetailsForConfig(notCompressedException, shouldIncludeDetailedStackTrace) + ).asRuntimeException(); } // ========== 3. Third-party Library Exceptions ========== // External library exceptions (Jackson JSON parsing) - include full details case InputCoercionException inputCoercionException -> { - return Status.INVALID_ARGUMENT.withDescription(ExceptionsHelper.stackTrace(inputCoercionException)).asRuntimeException(); + return Status.INVALID_ARGUMENT.withDescription( + getErrorDetailsForConfig(inputCoercionException, shouldIncludeDetailedStackTrace) + ).asRuntimeException(); } case JsonParseException jsonParseException -> { - return Status.INVALID_ARGUMENT.withDescription(ExceptionsHelper.stackTrace(jsonParseException)).asRuntimeException(); + return Status.INVALID_ARGUMENT.withDescription( + getErrorDetailsForConfig(jsonParseException, shouldIncludeDetailedStackTrace) + ).asRuntimeException(); } // ========== 4. Standard Java Exceptions ========== // Generic Java runtime exceptions - include full exception details for debugging case IllegalArgumentException illegalArgumentException -> { - return Status.INVALID_ARGUMENT.withDescription(ExceptionsHelper.stackTrace(illegalArgumentException)).asRuntimeException(); + return Status.INVALID_ARGUMENT.withDescription( + getErrorDetailsForConfig(illegalArgumentException, shouldIncludeDetailedStackTrace) + ).asRuntimeException(); } case IllegalStateException illegalStateException -> { - return Status.FAILED_PRECONDITION.withDescription(ExceptionsHelper.stackTrace(illegalStateException)).asRuntimeException(); + return Status.FAILED_PRECONDITION.withDescription( + getErrorDetailsForConfig(illegalStateException, shouldIncludeDetailedStackTrace) + ).asRuntimeException(); } case SecurityException securityException -> { - return Status.PERMISSION_DENIED.withDescription(ExceptionsHelper.stackTrace(securityException)).asRuntimeException(); + return Status.PERMISSION_DENIED.withDescription( + getErrorDetailsForConfig(securityException, shouldIncludeDetailedStackTrace) + ).asRuntimeException(); } case TimeoutException timeoutException -> { - return Status.DEADLINE_EXCEEDED.withDescription(ExceptionsHelper.stackTrace(timeoutException)).asRuntimeException(); + return Status.DEADLINE_EXCEEDED.withDescription(getErrorDetailsForConfig(timeoutException, shouldIncludeDetailedStackTrace)) + .asRuntimeException(); } case InterruptedException interruptedException -> { - return Status.CANCELLED.withDescription(ExceptionsHelper.stackTrace(interruptedException)).asRuntimeException(); + return Status.CANCELLED.withDescription(getErrorDetailsForConfig(interruptedException, shouldIncludeDetailedStackTrace)) + .asRuntimeException(); } case IOException ioException -> { - return Status.INTERNAL.withDescription(ExceptionsHelper.stackTrace(ioException)).asRuntimeException(); + return Status.INTERNAL.withDescription(getErrorDetailsForConfig(ioException, shouldIncludeDetailedStackTrace)) + .asRuntimeException(); } case null -> { logger.warn("Unexpected null exception type, treating as INTERNAL error"); @@ -105,7 +131,7 @@ public static StatusRuntimeException convertToGrpcError(Exception e) { // Safety fallback for any unexpected exception to {@code Status.INTERNAL} with full debugging info default -> { logger.warn("Unmapped exception type: {}, treating as INTERNAL error", e.getClass().getSimpleName()); - return Status.INTERNAL.withDescription(ExceptionsHelper.stackTrace(e)).asRuntimeException(); + return Status.INTERNAL.withDescription(getErrorDetailsForConfig(e, shouldIncludeDetailedStackTrace)).asRuntimeException(); } } } @@ -119,7 +145,7 @@ public static StatusRuntimeException convertToGrpcError(Exception e) { * @param ose The OpenSearchException to convert * @return StatusRuntimeException with mapped gRPC status and HTTP-identical error message */ - private static StatusRuntimeException handleOpenSearchException(OpenSearchException ose) { + private static StatusRuntimeException handleOpenSearchException(OpenSearchException ose, boolean shouldIncludeDetailedStackTrace) { Status grpcStatus = RestToGrpcStatusConverter.convertRestToGrpcStatus(ose.status()); // Use existing HTTP logic but enhance description with metadata from XContent @@ -127,7 +153,7 @@ private static StatusRuntimeException handleOpenSearchException(OpenSearchExcept String baseDescription = ExceptionsHelper.summaryMessage(unwrapped); // Extract metadata using the same XContent infrastructure as HTTP - String enhancedDescription = enhanceDescriptionWithXContentMetadata(unwrapped, baseDescription); + String enhancedDescription = enhanceDescriptionWithXContentMetadata(unwrapped, baseDescription, shouldIncludeDetailedStackTrace); return grpcStatus.withDescription(enhancedDescription).asRuntimeException(); } @@ -142,7 +168,11 @@ private static StatusRuntimeException handleOpenSearchException(OpenSearchExcept * @param baseDescription The base description from ExceptionsHelper.summaryMessage() * @return Enhanced description with full JSON metadata from XContent */ - private static String enhanceDescriptionWithXContentMetadata(Throwable exception, String baseDescription) { + private static String enhanceDescriptionWithXContentMetadata( + Throwable exception, + String baseDescription, + boolean shouldIncludeDetailedStackTrace + ) { try { // Use the exact same method as HTTP error responses try (XContentBuilder builder = XContentFactory.jsonBuilder()) { @@ -151,7 +181,13 @@ private static String enhanceDescriptionWithXContentMetadata(Throwable exception // Use the same method as HTTP REST responses (BytesRestResponse.build) // This includes root_cause analysis, just like HTTP - OpenSearchException.generateFailureXContent(builder, ToXContent.EMPTY_PARAMS, (Exception) exception, true); + ToXContent.Params params = shouldIncludeDetailedStackTrace ? INCLUDE_STACK_TRACES : ToXContent.EMPTY_PARAMS; + OpenSearchException.generateFailureXContent( + builder, + params, + (Exception) exception, + GrpcParamsHandler.isDetailedErrorsEnabled() + ); // Add status field like HTTP does org.opensearch.core.rest.RestStatus restStatus = ExceptionsHelper.status((Exception) exception); @@ -175,4 +211,15 @@ private static String enhanceDescriptionWithXContentMetadata(Throwable exception return baseDescription; } } + + /** + * Gets either a full stack trace of an error or just a message based on the flag that comes from the gRPC request Global Params. + * @param exception The exception to get details from + * @param shouldIncludeDetailedStackTrace Flag indicating whether to include full stack trace + * @return String with either full stack trace or just the error message + */ + private static String getErrorDetailsForConfig(Throwable exception, boolean shouldIncludeDetailedStackTrace) { + return shouldIncludeDetailedStackTrace ? ExceptionsHelper.stackTrace(exception) : exception.getMessage(); + } + } diff --git a/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/util/GrpcParamsHandler.java b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/util/GrpcParamsHandler.java new file mode 100644 index 0000000000000..77f16f5961889 --- /dev/null +++ b/modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/util/GrpcParamsHandler.java @@ -0,0 +1,82 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.transport.grpc.util; + +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.protobufs.GlobalParams; +import org.opensearch.rest.AbstractRestChannel; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestController; +import org.opensearch.rest.RestRequest; + +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.opensearch.transport.grpc.Netty4GrpcServerTransport.SETTING_GRPC_DETAILED_ERRORS_ENABLED; + +/** + * Central utility class to handle how global gRPC request parameters are handled. + * Handling of the server side {@code detailedErrorsEnabled} and the fail-fast behaviour + * in case of faulty configuration is aligned with the + * {@link RestController#tryAllHandlers(RestRequest, RestChannel, ThreadContext)} and the + * {@link AbstractRestChannel}. + */ +public class GrpcParamsHandler { + + /** + * Indicates whether detailed error traces are enabled on the gRPC server. + */ + private static final AtomicBoolean detailedErrorsEnabled = new AtomicBoolean(true); + + private GrpcParamsHandler() {} + + /** + * Initializes the handler with the given settings. + * + * @param settings the node settings + */ + public static void initialize(Settings settings) { + detailedErrorsEnabled.set(SETTING_GRPC_DETAILED_ERRORS_ENABLED.get(settings)); + } + + /** + * Checks whether detailed stack trace was requested in the gRPC request parameters. + * This method can be further extended to encapsulate more complex behaviour like + * skipping stack traces as per server configuration, etc. + * + * @param globalParams the global gRPC request parameters + * @return if, in case of an exception, a detailed stack trace should be included in the response + */ + public static boolean isDetailedStackTraceRequested(GlobalParams globalParams) { + return globalParams.getErrorTrace(); + } + + /** + * Checks if detailed errors are enabled on the gRPC server. + * + * @return true if detailed errors are enabled, false otherwise + */ + public static boolean isDetailedErrorsEnabled() { + return detailedErrorsEnabled.get(); + } + + /** + * Validates if error details are allowed to be shared in the response + * based on the grpc server configuration and request parameters. + * + * @param globalRequestParams The global parameters from the gRPC request + * @throws IllegalArgumentException if error tracing is requested but disabled by the server side + */ + public static void validateStackTraceDetailsConfiguration(GlobalParams globalRequestParams) { + if (detailedErrorsEnabled.get() == false && globalRequestParams.getErrorTrace()) { + throw new IllegalArgumentException("error traces in responses are disabled."); + } + } + +} diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/GrpcPluginTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/GrpcPluginTests.java index 544b2d91d8755..ca7d7f0607747 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/GrpcPluginTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/GrpcPluginTests.java @@ -144,7 +144,7 @@ public void testGetSettings() { assertTrue("SETTING_GRPC_KEEPALIVE_TIMEOUT should be included", settings.contains(SETTING_GRPC_KEEPALIVE_TIMEOUT)); // Verify the number of settings - assertEquals("Should return 13 settings", 13, settings.size()); + assertEquals("Should return 14 settings", 14, settings.size()); } private static class LoadableMockServiceFactory implements GrpcServiceFactory { diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/TestFixtures.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/TestFixtures.java new file mode 100644 index 0000000000000..d1e02789a4de3 --- /dev/null +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/TestFixtures.java @@ -0,0 +1,24 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.transport.grpc; + +import org.opensearch.common.settings.Settings; +import org.opensearch.protobufs.GlobalParams; + +public final class TestFixtures { + + public static final GlobalParams GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE = GlobalParams.newBuilder().setErrorTrace(true).build(); + + public static final GlobalParams GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE = GlobalParams.newBuilder().setErrorTrace(false).build(); + + public static Settings settingsWithGivenStackTraceConfig(boolean stackTracesEnabled) { + return Settings.builder().put(Netty4GrpcServerTransport.SETTING_GRPC_DETAILED_ERRORS_ENABLED.getKey(), stackTracesEnabled).build(); + } + +} diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListenerTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListenerTests.java index 5f1b6e1839a04..a4a57c9f07743 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListenerTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/BulkRequestActionListenerTests.java @@ -23,6 +23,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.verify; @@ -37,7 +38,7 @@ public class BulkRequestActionListenerTests extends OpenSearchTestCase { public void setUp() throws Exception { super.setUp(); MockitoAnnotations.openMocks(this); - listener = new BulkRequestActionListener(responseObserver); + listener = new BulkRequestActionListener(responseObserver, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); } public void testOnResponseWithSuccessfulResponse() { diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/SearchRequestActionListenerTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/SearchRequestActionListenerTests.java index 2b3986586a85d..fcad8461bd654 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/SearchRequestActionListenerTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/listeners/SearchRequestActionListenerTests.java @@ -10,6 +10,7 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.action.search.SearchResponseSections; import org.opensearch.action.search.ShardSearchFailure; +import org.opensearch.protobufs.GlobalParams; import org.opensearch.search.SearchHits; import org.opensearch.test.OpenSearchTestCase; @@ -19,7 +20,6 @@ import org.mockito.MockitoAnnotations; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -34,7 +34,7 @@ public class SearchRequestActionListenerTests extends OpenSearchTestCase { public void setUp() throws Exception { super.setUp(); MockitoAnnotations.openMocks(this); - listener = new SearchRequestActionListener(responseObserver); + listener = new SearchRequestActionListener(responseObserver, GlobalParams.newBuilder().build()); } public void testOnResponse() { @@ -60,13 +60,6 @@ public void testOnResponse() { } public void testOnFailure() { - // Create a mock StreamObserver - @SuppressWarnings("unchecked") - StreamObserver mockResponseObserver = mock(StreamObserver.class); - - // Create a SearchRequestActionListener - SearchRequestActionListener listener = new SearchRequestActionListener(mockResponseObserver); - // Create an exception Exception exception = new Exception("Test exception"); @@ -74,6 +67,6 @@ public void testOnFailure() { listener.onFailure(exception); // Verify that onError was called with a StatusRuntimeException - verify(mockResponseObserver, times(1)).onError(any(StatusRuntimeException.class)); + verify(responseObserver, times(1)).onError(any(StatusRuntimeException.class)); } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtilsTests.java index 699ab999a40c9..a5b8128bc255b 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/document/bulk/BulkRequestProtoUtilsTests.java @@ -142,7 +142,9 @@ public void testPrepareRequestWithTypeThrowsUnsupportedOperationException() { public void testPrepareRequestWithGlobalParamsThrowsUnsupportedOperationException() { // Create a protobuf BulkRequest with global_params - BulkRequest request = BulkRequest.newBuilder().setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().build()).build(); + BulkRequest request = BulkRequest.newBuilder() + .setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().setHuman(true).build()) + .build(); // Call prepareRequest, should throw UnsupportedOperationException UnsupportedOperationException exception = expectThrows( @@ -150,6 +152,6 @@ public void testPrepareRequestWithGlobalParamsThrowsUnsupportedOperationExceptio () -> BulkRequestProtoUtils.prepareRequest(request) ); - assertEquals("global_params param is not supported yet", exception.getMessage()); + assertEquals("global_params.human or filter_path params are not supported yet", exception.getMessage()); } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java index 2faddbdbcc4ea..17ae549d9e00e 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/SearchRequestProtoUtilsTests.java @@ -409,7 +409,7 @@ public void testParseSearchRequestWithTypedKeysThrowsUnsupportedOperationExcepti public void testParseSearchRequestWithGlobalParamsThrowsUnsupportedOperationException() throws IOException { // Create a protobuf SearchRequest with global_params org.opensearch.protobufs.SearchRequest protoRequest = org.opensearch.protobufs.SearchRequest.newBuilder() - .setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().build()) + .setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().setHuman(true).build()) .build(); // Create a SearchRequest to populate @@ -421,7 +421,7 @@ public void testParseSearchRequestWithGlobalParamsThrowsUnsupportedOperationExce () -> SearchRequestProtoUtils.parseSearchRequest(searchRequest, protoRequest, namedWriteableRegistry, size -> {}, queryUtils) ); - assertEquals("global_params param is not supported yet", exception.getMessage()); + assertEquals("global_params.human or filter_path params are not supported yet", exception.getMessage()); } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/BulkResponseProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/BulkResponseProtoUtilsTests.java index 5f80ee55c593e..b0ebba0da7eeb 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/BulkResponseProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/BulkResponseProtoUtilsTests.java @@ -22,6 +22,8 @@ import io.grpc.Status; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; + public class BulkResponseProtoUtilsTests extends OpenSearchTestCase { public void testToProtoWithSuccessfulResponse() throws IOException { @@ -37,7 +39,10 @@ public void testToProtoWithSuccessfulResponse() throws IOException { BulkResponse bulkResponse = new BulkResponse(responses, 100); // Convert to Protocol Buffer - org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(bulkResponse); + org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto( + bulkResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the conversion assertEquals("Should have the correct took time", 100, protoResponse.getTook()); @@ -65,7 +70,10 @@ public void testToProtoWithFailedResponse() throws IOException { BulkResponse bulkResponse = new BulkResponse(responses, 100); // Convert to Protocol Buffer - org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(bulkResponse); + org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto( + bulkResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the conversion assertEquals("Should have the correct took time", 100, protoResponse.getTook()); @@ -94,7 +102,10 @@ public void testToProtoWithIngestTook() throws IOException { BulkResponse bulkResponse = new BulkResponse(responses, 100, 50); // Convert to Protocol Buffer - org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(bulkResponse); + org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto( + bulkResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the conversion assertEquals("Should have the correct took time", 100, protoResponse.getTook()); diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkItemResponseProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkItemResponseProtoUtilsTests.java index 0badb26e0cf65..b4546edc50031 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkItemResponseProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/bulk/BulkItemResponseProtoUtilsTests.java @@ -30,6 +30,8 @@ import io.grpc.Status; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; + public class BulkItemResponseProtoUtilsTests extends OpenSearchTestCase { public void testToProtoWithIndexResponse() throws IOException { @@ -47,7 +49,10 @@ public void testToProtoWithIndexResponse() throws IOException { BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.INDEX, indexResponse); // Convert to protobuf ResponseItem - org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse); + org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto( + bulkItemResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("ResponseItem should not be null", responseItem); @@ -72,7 +77,10 @@ public void testToProtoWithCreateResponse() throws IOException { BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.CREATE, indexResponse); // Convert to protobuf ResponseItem - org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse); + org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto( + bulkItemResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("ResponseItem should not be null", responseItem); @@ -97,7 +105,10 @@ public void testToProtoWithDeleteResponse() throws IOException { BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.DELETE, deleteResponse); // Convert to protobuf ResponseItem - org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse); + org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto( + bulkItemResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("ResponseItem should not be null", responseItem); @@ -122,7 +133,10 @@ public void testToProtoWithUpdateResponse() throws IOException { BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.UPDATE, updateResponse); // Convert to protobuf Item - org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse); + org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto( + bulkItemResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("ResponseItem should not be null", responseItem); @@ -165,7 +179,10 @@ public void testToProtoWithUpdateResponseAndGetResult() throws IOException { BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.UPDATE, updateResponse); // Convert to protobuf Item - org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse); + org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto( + bulkItemResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("ResponseItem should not be null", responseItem); @@ -195,7 +212,10 @@ public void testToProtoWithFailure() throws IOException { BulkItemResponse bulkItemResponse = new BulkItemResponse(0, DocWriteRequest.OpType.INDEX, failure); // Convert to protobuf Item - org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto(bulkItemResponse); + org.opensearch.protobufs.ResponseItem responseItem = BulkItemResponseProtoUtils.toProto( + bulkItemResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("ResponseItem should not be null", responseItem); @@ -210,6 +230,6 @@ public void testToProtoWithFailure() throws IOException { public void testToProtoWithNullResponse() throws IOException { // Call toProto with null, should throw NullPointerException - expectThrows(NullPointerException.class, () -> BulkItemResponseProtoUtils.toProto(null)); + expectThrows(NullPointerException.class, () -> BulkItemResponseProtoUtils.toProto(null, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE)); } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/DocWriteResponseProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/DocWriteResponseProtoUtilsTests.java index 1019f4bce9743..708500a84c6bd 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/DocWriteResponseProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/DocWriteResponseProtoUtilsTests.java @@ -17,6 +17,8 @@ import java.io.IOException; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; + public class DocWriteResponseProtoUtilsTests extends OpenSearchTestCase { public void testToProtoWithIndexResponse() throws IOException { @@ -32,7 +34,7 @@ public void testToProtoWithIndexResponse() throws IOException { indexResponse.setForcedRefresh(true); // Convert to protobuf ResponseItem.Builder - ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse); + ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ResponseItem.Builder should not be null", responseItemBuilder); @@ -70,7 +72,7 @@ public void testToProtoWithEmptyId() throws IOException { indexResponse.setShardInfo(shardInfo); // Convert to protobuf ResponseItem.Builder - ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse); + ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ResponseItem.Builder should not be null", responseItemBuilder); @@ -94,7 +96,7 @@ public void testToProtoWithNoSeqNo() throws IOException { indexResponse.setShardInfo(shardInfo); // Convert to protobuf ResponseItem.Builder - ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse); + ResponseItem.Builder responseItemBuilder = DocWriteResponseProtoUtils.toProto(indexResponse, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ResponseItem.Builder should not be null", responseItemBuilder); @@ -109,6 +111,6 @@ public void testToProtoWithNoSeqNo() throws IOException { public void testToProtoWithNullResponse() throws IOException { // Call toProto with null, should throw NullPointerException - expectThrows(NullPointerException.class, () -> DocWriteResponseProtoUtils.toProto(null)); + expectThrows(NullPointerException.class, () -> DocWriteResponseProtoUtils.toProto(null, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE)); } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/ShardInfoProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/ShardInfoProtoUtilsTests.java index d4c873e39ff53..a4f4a6867d472 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/ShardInfoProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/document/common/ShardInfoProtoUtilsTests.java @@ -19,6 +19,8 @@ import java.util.ArrayList; import java.util.List; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; + public class ShardInfoProtoUtilsTests extends OpenSearchTestCase { public void testToProtoWithNoFailures() throws IOException { @@ -26,7 +28,7 @@ public void testToProtoWithNoFailures() throws IOException { ReplicationResponse.ShardInfo shardInfo = new ReplicationResponse.ShardInfo(5, 3, new ReplicationResponse.ShardInfo.Failure[0]); // Convert to protobuf ShardInfo - ShardInfo protoShardInfo = ShardInfoProtoUtils.toProto(shardInfo); + ShardInfo protoShardInfo = ShardInfoProtoUtils.toProto(shardInfo, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ShardInfo should not be null", protoShardInfo); @@ -55,7 +57,7 @@ public void testToProtoWithFailures() throws IOException { ReplicationResponse.ShardInfo shardInfo = new ReplicationResponse.ShardInfo(5, 3, failures); // Convert to protobuf ShardInfo - ShardInfo protoShardInfo = ShardInfoProtoUtils.toProto(shardInfo); + ShardInfo protoShardInfo = ShardInfoProtoUtils.toProto(shardInfo, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ShardInfo should not be null", protoShardInfo); @@ -87,6 +89,6 @@ public void testToProtoWithFailures() throws IOException { public void testToProtoWithNullShardInfo() throws IOException { // Call toProto with null, should throw NullPointerException - expectThrows(NullPointerException.class, () -> ShardInfoProtoUtils.toProto(null)); + expectThrows(NullPointerException.class, () -> ShardInfoProtoUtils.toProto(null, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE)); } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/OpenSearchExceptionProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/OpenSearchExceptionProtoUtilsTests.java index 421c0677d1bfc..fabb29e0fa62c 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/OpenSearchExceptionProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/OpenSearchExceptionProtoUtilsTests.java @@ -31,6 +31,8 @@ import java.util.List; import java.util.Map; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -42,7 +44,7 @@ public void testToProtoWithOpenSearchException() throws IOException { OpenSearchException exception = new OpenSearchException("Test exception"); // Convert to Protocol Buffer - ErrorCause errorCause = OpenSearchExceptionProtoUtils.toProto(exception); + ErrorCause errorCause = OpenSearchExceptionProtoUtils.toProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // Verify the conversion // The actual type format uses underscores instead of dots @@ -53,13 +55,27 @@ public void testToProtoWithOpenSearchException() throws IOException { assertFalse("Should not have a cause", errorCause.hasCausedBy()); } + public void testToProtoWithOpenSearchExceptionSummary() throws IOException { + // Create an OpenSearchException + OpenSearchException exception = new OpenSearchException("Test exception"); + + // Convert to Protocol Buffer + ErrorCause errorCause = OpenSearchExceptionProtoUtils.toProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // Verify the conversion + // The actual type format uses underscores instead of dots + assertFalse("Should not have a stack trace", errorCause.hasStackTrace()); + assertFalse("Should not have suppressed exceptions", errorCause.getSuppressedList().iterator().hasNext()); + assertFalse("Should not have a cause", errorCause.hasCausedBy()); + } + public void testToProtoWithNestedOpenSearchException() throws IOException { // Create a nested OpenSearchException IOException cause = new IOException("Cause exception"); OpenSearchException exception = new OpenSearchException("Test exception", cause); // Convert to Protocol Buffer - ErrorCause errorCause = OpenSearchExceptionProtoUtils.toProto(exception); + ErrorCause errorCause = OpenSearchExceptionProtoUtils.toProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // Verify the conversion // The actual type format uses underscores instead of dots @@ -71,6 +87,30 @@ public void testToProtoWithNestedOpenSearchException() throws IOException { assertTrue("Should have a cause", errorCause.hasCausedBy()); ErrorCause causedBy = errorCause.getCausedBy(); // The actual type format uses underscores instead of dots + assertTrue("Should have a stack trace for cause", causedBy.getStackTrace().length() > 0); + assertEquals("Cause should have the correct type", "i_o_exception", causedBy.getType()); + assertEquals("Cause should have the correct reason", "Cause exception", causedBy.getReason()); + } + + public void testToProtoWithNestedOpenSearchExceptionSummary() throws IOException { + // Create a nested OpenSearchException + IOException cause = new IOException("Cause exception"); + OpenSearchException exception = new OpenSearchException("Test exception", cause); + + // Convert to Protocol Buffer + ErrorCause errorCause = OpenSearchExceptionProtoUtils.toProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // Verify the conversion + // The actual type format uses underscores instead of dots + assertEquals("Should have the correct type", "exception", errorCause.getType()); + assertEquals("Should have the correct reason", "Test exception", errorCause.getReason()); + assertFalse("Should not have a stack trace", errorCause.hasStackTrace()); + + // Verify the cause + assertTrue("Should have a cause", errorCause.hasCausedBy()); + ErrorCause causedBy = errorCause.getCausedBy(); + // The actual type format uses underscores instead of dots + assertFalse("Should not have a stack trace on the cause", causedBy.hasStackTrace()); assertEquals("Cause should have the correct type", "i_o_exception", causedBy.getType()); assertEquals("Cause should have the correct reason", "Cause exception", causedBy.getReason()); } @@ -80,7 +120,7 @@ public void testGenerateThrowableProtoWithOpenSearchException() throws IOExcepti OpenSearchException exception = new OpenSearchException("Test exception"); // Convert to Protocol Buffer - ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception); + ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the conversion // The actual type format uses underscores instead of dots @@ -93,7 +133,7 @@ public void testGenerateThrowableProtoWithIOException() throws IOException { IOException exception = new IOException("Test IO exception"); // Convert to Protocol Buffer - ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception); + ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the conversion // The actual type format uses underscores instead of dots @@ -106,7 +146,7 @@ public void testGenerateThrowableProtoWithRuntimeException() throws IOException RuntimeException exception = new RuntimeException("Test runtime exception"); // Convert to Protocol Buffer - ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception); + ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the conversion // The actual type format uses underscores instead of dots @@ -119,7 +159,7 @@ public void testGenerateThrowableProtoWithNullMessage() throws IOException { RuntimeException exception = new RuntimeException((String) null); // Convert to Protocol Buffer - ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception); + ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the conversion // The actual type format uses underscores instead of dots @@ -133,7 +173,7 @@ public void testGenerateThrowableProtoWithSuppressedExceptions() throws IOExcept exception.addSuppressed(new IllegalArgumentException("Suppressed exception")); // Convert to Protocol Buffer - ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception); + ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the conversion // The actual type format uses underscores instead of dots @@ -153,7 +193,7 @@ public void testInnerToProtoWithBasicException() throws IOException { RuntimeException exception = new RuntimeException("Test exception"); // Convert to Protocol Buffer - ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception); + ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // Verify the conversion // The actual type format uses underscores instead of dots @@ -162,6 +202,20 @@ public void testInnerToProtoWithBasicException() throws IOException { assertTrue("Should have a stack trace", errorCause.getStackTrace().length() > 0); } + public void testInnerToProtoReturnsOnlyBasicExceptionSummary() throws IOException { + // Create a basic exception + RuntimeException exception = new RuntimeException("Test exception"); + + // Convert to Protocol Buffer + ErrorCause errorCause = OpenSearchExceptionProtoUtils.generateThrowableProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // Verify the conversion + // The actual type format uses underscores instead of dots + assertEquals("Should have the correct type", "runtime_exception", errorCause.getType()); + assertEquals("Should have the correct reason", "Test exception", errorCause.getReason()); + assertFalse("Should not have a stack trace", errorCause.hasStackTrace()); + } + public void testHeaderToProtoWithSingleValue() throws IOException { // Create a header with a single value String key = "test-header"; diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/DefaultShardOperationFailedExceptionProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/DefaultShardOperationFailedExceptionProtoUtilsTests.java index 83e09ec0af966..567ffcbb39dd5 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/DefaultShardOperationFailedExceptionProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/DefaultShardOperationFailedExceptionProtoUtilsTests.java @@ -18,6 +18,8 @@ import java.io.IOException; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; + public class DefaultShardOperationFailedExceptionProtoUtilsTests extends OpenSearchTestCase { public void testToProtoWithDefaultShardOperationFailedException() throws IOException { @@ -29,7 +31,7 @@ public void testToProtoWithDefaultShardOperationFailedException() throws IOExcep ); // Call the method under test - ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception); + ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ShardFailure should not be null", shardFailure); @@ -49,7 +51,7 @@ public void testToProtoWithAddIndexBlockResponseFailure() throws IOException { ); // Call the method under test - ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception); + ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ShardFailure should not be null", shardFailure); @@ -70,7 +72,7 @@ public void testToProtoWithIndicesShardStoresResponseFailure() throws IOExceptio ); // Call the method under test - ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception); + ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ShardFailure should not be null", shardFailure); @@ -91,7 +93,7 @@ public void testToProtoWithCloseIndexResponseFailure() throws IOException { ); // Call the method under test - ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception); + ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ShardFailure should not be null", shardFailure); @@ -112,7 +114,7 @@ public void testToProtoWithNullNodeId() throws IOException { ); // Call the method under test - ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception); + ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Verify the result assertNotNull("ShardFailure should not be null", shardFailure); @@ -124,7 +126,7 @@ public void testToProtoWithNullNodeId() throws IOException { } public void testToProtoWithNull() throws IOException { - ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(null); + ShardFailure shardFailure = DefaultShardOperationFailedExceptionProtoUtils.toProto(null, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); assertNotNull("ShardFailure should not be null", shardFailure); assertFalse("Index should not be set", shardFailure.hasIndex()); assertFalse("Status should not be set", shardFailure.hasStatus()); diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtilsTests.java index 42c51d1916b75..299313231fd7b 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtilsTests.java @@ -21,6 +21,7 @@ import java.io.IOException; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; import static org.mockito.Mockito.mock; public class ShardOperationFailedExceptionProtoUtilsTests extends OpenSearchTestCase { @@ -35,7 +36,10 @@ public void testToProtoWithShardSearchFailure() throws IOException { ShardSearchFailure shardSearchFailure = new ShardSearchFailure(new Exception("fake exception"), searchShardTarget); // Call the method under test - ShardFailure protoFailure = ShardOperationFailedExceptionProtoUtils.toProto(shardSearchFailure); + ShardFailure protoFailure = ShardOperationFailedExceptionProtoUtils.toProto( + shardSearchFailure, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto failure should not be null", protoFailure); @@ -53,7 +57,10 @@ public void testToProtoWithSnapshotShardFailure() throws IOException { SnapshotShardFailure shardSearchFailure = new SnapshotShardFailure("test_node", shardId, "Snapshot failed"); // Call the method under test - ShardFailure protoFailure = ShardOperationFailedExceptionProtoUtils.toProto(shardSearchFailure); + ShardFailure protoFailure = ShardOperationFailedExceptionProtoUtils.toProto( + shardSearchFailure, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto failure should not be null", protoFailure); @@ -72,7 +79,10 @@ public void testToProtoWithDefaultShardOperationFailedException() throws IOExcep ); // Call the method under test - ShardFailure protoFailure = ShardOperationFailedExceptionProtoUtils.toProto(defaultShardOperationFailedException); + ShardFailure protoFailure = ShardOperationFailedExceptionProtoUtils.toProto( + defaultShardOperationFailedException, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto failure should not be null", protoFailure); @@ -93,7 +103,10 @@ public void testToProtoWithReplicationResponseShardInfoFailure() throws IOExcept ); // Call the method under test - ShardFailure protoFailure = ShardOperationFailedExceptionProtoUtils.toProto(replicationResponseFailure); + ShardFailure protoFailure = ShardOperationFailedExceptionProtoUtils.toProto( + replicationResponseFailure, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto failure should not be null", protoFailure); @@ -111,7 +124,7 @@ public void testToProtoWithUnsupportedShardOperationFailedException() { // Call the method under test, should throw UnsupportedOperationException UnsupportedOperationException exception = expectThrows( UnsupportedOperationException.class, - () -> ShardOperationFailedExceptionProtoUtils.toProto(mockFailure) + () -> ShardOperationFailedExceptionProtoUtils.toProto(mockFailure, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE) ); assertTrue( @@ -124,7 +137,7 @@ public void testToProtoWithNullShardOperationFailedException() { // Call the method under test with null, should throw UnsupportedOperationException UnsupportedOperationException exception = expectThrows( UnsupportedOperationException.class, - () -> ShardOperationFailedExceptionProtoUtils.toProto(null) + () -> ShardOperationFailedExceptionProtoUtils.toProto(null, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE) ); assertEquals("Unsupported ShardOperationFailedException [null] cannot be converted to proto.", exception.getMessage()); diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchResponseProtoUtilsTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchResponseProtoUtilsTests.java index 2408a6d8dc6ae..9aa23c0f25749 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchResponseProtoUtilsTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/search/SearchResponseProtoUtilsTests.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.Map; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -41,7 +42,10 @@ public void testToProtoWithBasicResponse() throws IOException { when(mockResponse.getInternalResponse()).thenReturn(mock(SearchResponseSections.class)); // Call the method under test - org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto(mockResponse); + org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto( + mockResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto response should not be null", protoResponse); @@ -69,7 +73,10 @@ public void testToProtoWithScrollId() throws IOException { when(mockResponse.getInternalResponse()).thenReturn(mock(SearchResponseSections.class)); // Call the method under test - org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto(mockResponse); + org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto( + mockResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto response should not be null", protoResponse); @@ -92,7 +99,10 @@ public void testToProtoWithPointInTimeId() throws IOException { when(mockResponse.getInternalResponse()).thenReturn(mock(SearchResponseSections.class)); // Call the method under test - org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto(mockResponse); + org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto( + mockResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto response should not be null", protoResponse); @@ -126,7 +136,10 @@ public void testToProtoWithPhaseTook() throws IOException { when(mockResponse.getInternalResponse()).thenReturn(mock(SearchResponseSections.class)); // Call the method under test - org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto(mockResponse); + org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto( + mockResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto response should not be null", protoResponse); @@ -155,7 +168,10 @@ public void testToProtoWithTerminatedEarly() throws IOException { when(mockResponse.getInternalResponse()).thenReturn(mock(SearchResponseSections.class)); // Call the method under test - org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto(mockResponse); + org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto( + mockResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto response should not be null", protoResponse); @@ -178,7 +194,10 @@ public void testToProtoWithNumReducePhases() throws IOException { when(mockResponse.getInternalResponse()).thenReturn(mock(SearchResponseSections.class)); // Call the method under test - org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto(mockResponse); + org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto( + mockResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto response should not be null", protoResponse); @@ -200,7 +219,10 @@ public void testToProtoWithClusters() throws IOException { when(mockResponse.getInternalResponse()).thenReturn(mock(SearchResponseSections.class)); // Call the method under test - org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto(mockResponse); + org.opensearch.protobufs.SearchResponse protoResponse = SearchResponseProtoUtils.toProto( + mockResponse, + GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE + ); // Verify the result assertNotNull("Proto response should not be null", protoResponse); diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/services/SearchServiceImplTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/services/SearchServiceImplTests.java index 379452e11770b..9b717aeba2206 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/services/SearchServiceImplTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/services/SearchServiceImplTests.java @@ -14,14 +14,18 @@ import org.opensearch.transport.client.node.NodeClient; import org.opensearch.transport.grpc.proto.request.search.query.AbstractQueryBuilderProtoUtils; import org.opensearch.transport.grpc.proto.request.search.query.QueryBuilderProtoTestUtils; +import org.opensearch.transport.grpc.util.GrpcParamsHandler; +import org.junit.After; import org.junit.Before; import java.io.IOException; +import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import static org.opensearch.transport.grpc.TestFixtures.settingsWithGivenStackTraceConfig; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; @@ -42,6 +46,12 @@ public void setup() throws IOException { MockitoAnnotations.openMocks(this); queryUtils = QueryBuilderProtoTestUtils.createQueryUtils(); service = new SearchServiceImpl(client, queryUtils); + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(true)); + } + + @After + public void resetStackTraceSettings() { + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(true)); } public void testConstructorWithNullClient() { @@ -66,7 +76,7 @@ public void testConstructorWithBothNull() { assertEquals("Client cannot be null", exception.getMessage()); } - public void testSearchSuccess() throws IOException { + public void testSearchSuccess() { // Create a test request SearchRequest request = createTestSearchRequest(); @@ -77,7 +87,7 @@ public void testSearchSuccess() throws IOException { verify(client).search(any(org.opensearch.action.search.SearchRequest.class), any()); } - public void testSearchWithException() throws IOException { + public void testSearchWithException() { // Create a test request SearchRequest request = createTestSearchRequest(); @@ -91,10 +101,39 @@ public void testSearchWithException() throws IOException { verify(responseObserver).onError(any()); } + public void testErrorTraceConfigValidationFailsWhenServerSettingIsDisabledAndRequestRequiresStackTrace() { + // Setup request and the service, server setting is off and request requires a stack trace + SearchRequest request = createTestSearchRequest(); + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(false)); + SearchServiceImpl serviceThatFailsToProvideErrorInfo = new SearchServiceImpl(client, queryUtils); + + // Call search method + serviceThatFailsToProvideErrorInfo.search(request, responseObserver); + + // Verify that responseObserver.onError reports request parameter must be disabled + verify(responseObserver).onError(any(StatusRuntimeException.class)); + } + + public void testErrorTraceConfigValidationPassesWhenServerSettingIsDisabledAndRequestSkipsStackTrace() { + // Setup request and the service, server setting is off and request skips a stack trace + SearchRequest request = createTestSearchRequest().toBuilder() + .setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().setErrorTrace(false)) + .build(); + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(false)); + SearchServiceImpl serviceThatFailsToProvideErrorInfo = new SearchServiceImpl(client, queryUtils); + + // Call search method + serviceThatFailsToProvideErrorInfo.search(request, responseObserver); + + // Verify that client.search was called + verify(client).search(any(org.opensearch.action.search.SearchRequest.class), any()); + } + private SearchRequest createTestSearchRequest() { return SearchRequest.newBuilder() .addIndex("test-index") .setSearchRequestBody(SearchRequestBody.newBuilder().setSize(10).build()) + .setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().setErrorTrace(true).build()) .build(); } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/services/document/DocumentServiceImplTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/services/document/DocumentServiceImplTests.java index 3215238247bbd..82169eb8d4d71 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/services/document/DocumentServiceImplTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/services/document/DocumentServiceImplTests.java @@ -15,14 +15,18 @@ import org.opensearch.test.OpenSearchTestCase; import org.opensearch.transport.client.node.NodeClient; import org.opensearch.transport.grpc.services.DocumentServiceImpl; +import org.opensearch.transport.grpc.util.GrpcParamsHandler; +import org.junit.After; import org.junit.Before; import java.io.IOException; +import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import static org.opensearch.transport.grpc.TestFixtures.settingsWithGivenStackTraceConfig; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; @@ -41,9 +45,15 @@ public class DocumentServiceImplTests extends OpenSearchTestCase { public void setup() throws IOException { MockitoAnnotations.openMocks(this); service = new DocumentServiceImpl(client); + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(true)); } - public void testBulkSuccess() throws IOException { + @After + public void resetStackTraceSettings() { + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(true)); + } + + public void testBulkSuccess() { // Create a test request BulkRequest request = createTestBulkRequest(); @@ -54,7 +64,7 @@ public void testBulkSuccess() throws IOException { verify(client).bulk(any(org.opensearch.action.bulk.BulkRequest.class), any()); } - public void testBulkError() throws IOException { + public void testBulkError() { // Create a test request BulkRequest request = createTestBulkRequest(); @@ -68,6 +78,34 @@ public void testBulkError() throws IOException { verify(responseObserver).onError(any(RuntimeException.class)); } + public void testErrorTraceConfigValidationFailsWhenServerSettingIsDisabledAndRequestRequiresStackTrace() { + // Setup request and the service, server setting is off and request requires a stack trace + BulkRequest request = createTestBulkRequest(); + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(false)); + DocumentServiceImpl serviceThatFailsToProvideErrorInfo = new DocumentServiceImpl(client); + + // Call bulk method + serviceThatFailsToProvideErrorInfo.bulk(request, responseObserver); + + // Verify that an error was sent + verify(responseObserver).onError(any(StatusRuntimeException.class)); + } + + public void testErrorTraceConfigValidationPassesWhenServerSettingIsDisabledAndRequestSkipsStackTrace() { + // Setup request and the service, server setting is off and request does not require a stack trace + BulkRequest request = createTestBulkRequest().toBuilder() + .setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().setErrorTrace(false)) + .build(); + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(false)); + DocumentServiceImpl serviceThatFailsToProvideErrorInfo = new DocumentServiceImpl(client); + + // Call bulk method + serviceThatFailsToProvideErrorInfo.bulk(request, responseObserver); + + // Verify that client.bulk was called + verify(client).bulk(any(org.opensearch.action.bulk.BulkRequest.class), any()); + } + private BulkRequest createTestBulkRequest() { IndexOperation indexOp = IndexOperation.newBuilder().setXIndex("test-index").setXId("test-id").build(); @@ -76,6 +114,9 @@ private BulkRequest createTestBulkRequest() { .setObject(ByteString.copyFromUtf8("{\"field\":\"value\"}")) .build(); - return BulkRequest.newBuilder().addBulkRequestBody(requestBody).build(); + return BulkRequest.newBuilder() + .addBulkRequestBody(requestBody) + .setGlobalParams(org.opensearch.protobufs.GlobalParams.newBuilder().setErrorTrace(true)) + .build(); } } diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/util/GrpcErrorHandlerTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/util/GrpcErrorHandlerTests.java index c399e14a5fa6f..56c02b495eb32 100644 --- a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/util/GrpcErrorHandlerTests.java +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/util/GrpcErrorHandlerTests.java @@ -17,6 +17,8 @@ import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException; import org.opensearch.core.rest.RestStatus; import org.opensearch.test.OpenSearchTestCase; +import org.junit.After; +import org.junit.Before; import java.io.IOException; import java.util.concurrent.TimeoutException; @@ -24,6 +26,10 @@ import io.grpc.Status; import io.grpc.StatusRuntimeException; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE; +import static org.opensearch.transport.grpc.TestFixtures.settingsWithGivenStackTraceConfig; + /** * Tests for GrpcErrorHandler utility. * Validates that exceptions are properly converted to appropriate gRPC StatusRuntimeException @@ -31,7 +37,17 @@ */ public class GrpcErrorHandlerTests extends OpenSearchTestCase { - public void testOpenSearchExceptionConversion() { + @Before + public void setUpGrpcParamsHandler() { + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(true)); + } + + @After + public void resetGrpcParamsHandlerStackTraces() { + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(true)); + } + + public void testOpenSearchExceptionConversionWithTheStackTrace() { OpenSearchException exception = new OpenSearchException("Test exception") { @Override public RestStatus status() { @@ -39,22 +55,82 @@ public RestStatus status() { } }; - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // BAD_REQUEST -> INVALID_ARGUMENT via RestToGrpcStatusConverter assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); - // Uses ExceptionsHelper.summaryMessage() format + XContent details - assertTrue(result.getMessage().contains("OpenSearchException[Test exception]")); - assertTrue(result.getMessage().contains("details=")); - assertTrue(result.getMessage().contains("\"type\":\"exception\"")); - assertTrue(result.getMessage().contains("\"reason\":\"Test exception\"")); - assertTrue(result.getMessage().contains("\"status\":400")); + assertTrue( + "Error type with the cause must be present", + result.getMessage().contains("INVALID_ARGUMENT: OpenSearchException[Test exception];") + ); + assertTrue("Details must be present", result.getMessage().contains("details=")); + assertTrue("Exception type must be populated", result.getMessage().contains("\"type\":\"exception\"")); + assertTrue("Reason must be given", result.getMessage().contains("\"reason\":\"Test exception\"")); + assertTrue("Status must be populated", result.getMessage().contains("\"status\":400")); + assertTrue("Stack trace must be populated", result.getMessage().contains("\"stack_trace\":\"OpenSearchException[Test exception]")); + } + + public void testOpenSearchExceptionConversionWithoutTheStackTrace() { + OpenSearchException exception = new OpenSearchException("Test exception") { + @Override + public RestStatus status() { + return RestStatus.BAD_REQUEST; + } + }; + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // BAD_REQUEST -> INVALID_ARGUMENT via RestToGrpcStatusConverter + assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); + assertTrue( + "Error type with the cause must be present", + result.getMessage().contains("INVALID_ARGUMENT: OpenSearchException[Test exception];") + ); + assertTrue("Details must be present", result.getMessage().contains("details=")); + assertTrue("Exception type must be present", result.getMessage().contains("\"type\":\"exception\"")); + assertTrue("Reason must be present", result.getMessage().contains("\"reason\":\"Test exception\"")); + assertTrue("Status must be populated", result.getMessage().contains("\"status\":400")); + assertFalse("Stack trace must be omitted", result.getMessage().contains("\"stack_trace\"")); + } + + public void testOpenSearchExceptionConversionWhenDetailedErrorsAreDisabledOnTheServerSide() { + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(false)); + OpenSearchException exception = new OpenSearchException("Test exception") { + @Override + public RestStatus status() { + return RestStatus.BAD_REQUEST; + } + }; + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // BAD_REQUEST -> INVALID_ARGUMENT via RestToGrpcStatusConverter + assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); + assertTrue( + "Error type with the cause must be present", + result.getMessage().contains("INVALID_ARGUMENT: OpenSearchException[Test exception];") + ); + assertTrue("Details must be present", result.getMessage().contains("details=")); + assertFalse("Exception type must be omitted", result.getMessage().contains("\"type\"")); + assertFalse("Reason must be omitted", result.getMessage().contains("\"reason\"")); + assertTrue("Status must be populated", result.getMessage().contains("\"status\":400")); + assertFalse("Stack trace must be omitted", result.getMessage().contains("\"stack_trace\"")); } public void testIllegalArgumentExceptionConversion() { IllegalArgumentException exception = new IllegalArgumentException("Invalid parameter"); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // IllegalArgumentException -> INVALID_ARGUMENT via direct gRPC mapping + assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("INVALID_ARGUMENT: Invalid parameter")); + } + + public void testIllegalArgumentExceptionConversionWithExceptionDetails() { + IllegalArgumentException exception = new IllegalArgumentException("Invalid parameter"); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // IllegalArgumentException -> INVALID_ARGUMENT via direct gRPC mapping assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); @@ -65,7 +141,17 @@ public void testIllegalArgumentExceptionConversion() { public void testInputCoercionExceptionConversion() { InputCoercionException exception = new InputCoercionException(null, "Cannot coerce string to number", null, String.class); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // InputCoercionException -> INVALID_ARGUMENT via direct gRPC mapping + assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("INVALID_ARGUMENT: Cannot coerce string to number")); + } + + public void testInputCoercionExceptionConversionWithExceptionDetails() { + InputCoercionException exception = new InputCoercionException(null, "Cannot coerce string to number", null, String.class); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // InputCoercionException -> INVALID_ARGUMENT via direct gRPC mapping assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); @@ -77,7 +163,17 @@ public void testInputCoercionExceptionConversion() { public void testJsonParseExceptionConversion() { JsonParseException exception = new JsonParseException(null, "Unexpected character"); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // JsonParseException -> INVALID_ARGUMENT via direct gRPC mapping + assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("INVALID_ARGUMENT: Unexpected character")); + } + + public void testJsonParseExceptionConversionWithExceptionDetails() { + JsonParseException exception = new JsonParseException(null, "Unexpected character"); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // JsonParseException -> INVALID_ARGUMENT via direct gRPC mapping assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); @@ -89,7 +185,17 @@ public void testJsonParseExceptionConversion() { public void testOpenSearchRejectedExecutionExceptionConversion() { OpenSearchRejectedExecutionException exception = new OpenSearchRejectedExecutionException("Thread pool full"); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // OpenSearchRejectedExecutionException -> RESOURCE_EXHAUSTED via direct gRPC mapping + assertEquals(Status.RESOURCE_EXHAUSTED.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("RESOURCE_EXHAUSTED: Thread pool full")); + } + + public void testOpenSearchRejectedExecutionExceptionConversionWithExceptionDetails() { + OpenSearchRejectedExecutionException exception = new OpenSearchRejectedExecutionException("Thread pool full"); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // OpenSearchRejectedExecutionException -> RESOURCE_EXHAUSTED via direct gRPC mapping assertEquals(Status.RESOURCE_EXHAUSTED.getCode(), result.getStatus().getCode()); @@ -101,7 +207,17 @@ public void testOpenSearchRejectedExecutionExceptionConversion() { public void testIllegalStateExceptionConversion() { IllegalStateException exception = new IllegalStateException("Invalid state"); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // IllegalStateException -> FAILED_PRECONDITION via direct gRPC mapping + assertEquals(Status.FAILED_PRECONDITION.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("FAILED_PRECONDITION: Invalid state")); + } + + public void testIllegalStateExceptionConversionWithExceptionDetails() { + IllegalStateException exception = new IllegalStateException("Invalid state"); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // IllegalStateException -> FAILED_PRECONDITION via direct gRPC mapping assertEquals(Status.FAILED_PRECONDITION.getCode(), result.getStatus().getCode()); @@ -112,7 +228,17 @@ public void testIllegalStateExceptionConversion() { public void testSecurityExceptionConversion() { SecurityException exception = new SecurityException("Access denied"); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // SecurityException -> PERMISSION_DENIED via direct gRPC mapping + assertEquals(Status.PERMISSION_DENIED.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("PERMISSION_DENIED: Access denied")); + } + + public void testSecurityExceptionConversionWithExceptionDetails() { + SecurityException exception = new SecurityException("Access denied"); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // SecurityException -> PERMISSION_DENIED via direct gRPC mapping assertEquals(Status.PERMISSION_DENIED.getCode(), result.getStatus().getCode()); @@ -123,7 +249,17 @@ public void testSecurityExceptionConversion() { public void testTimeoutExceptionConversion() { TimeoutException exception = new TimeoutException("Operation timed out"); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // TimeoutException -> DEADLINE_EXCEEDED via direct gRPC mapping + assertEquals(Status.DEADLINE_EXCEEDED.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("DEADLINE_EXCEEDED: Operation timed out")); + } + + public void testTimeoutExceptionConversionWithExceptionDetails() { + TimeoutException exception = new TimeoutException("Operation timed out"); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // TimeoutException -> DEADLINE_EXCEEDED via direct gRPC mapping assertEquals(Status.DEADLINE_EXCEEDED.getCode(), result.getStatus().getCode()); @@ -134,7 +270,17 @@ public void testTimeoutExceptionConversion() { public void testInterruptedExceptionConversion() { InterruptedException exception = new InterruptedException(); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // InterruptedException -> CANCELLED via direct gRPC mapping + assertEquals(Status.CANCELLED.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("CANCELLED")); + } + + public void testInterruptedExceptionConversionWithExceptionDetails() { + InterruptedException exception = new InterruptedException(); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // InterruptedException -> CANCELLED via direct gRPC mapping assertEquals(Status.CANCELLED.getCode(), result.getStatus().getCode()); @@ -145,7 +291,17 @@ public void testInterruptedExceptionConversion() { public void testIOExceptionConversion() { IOException exception = new IOException("I/O error"); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // IOException -> INTERNAL via direct gRPC mapping + assertEquals(Status.INTERNAL.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("INTERNAL: I/O error")); + } + + public void testIOExceptionConversionWithExceptionDetails() { + IOException exception = new IOException("I/O error"); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // IOException -> INTERNAL via direct gRPC mapping assertEquals(Status.INTERNAL.getCode(), result.getStatus().getCode()); @@ -156,7 +312,17 @@ public void testIOExceptionConversion() { public void testUnknownExceptionConversion() { RuntimeException exception = new RuntimeException("Unknown error"); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + + // RuntimeException -> INTERNAL via fallback (unknown exception type) + assertEquals(Status.INTERNAL.getCode(), result.getStatus().getCode()); + assertTrue(result.getMessage().contains("INTERNAL: Unknown error")); + } + + public void testUnknownExceptionConversionWithExceptionDetails() { + RuntimeException exception = new RuntimeException("Unknown error"); + + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE); // RuntimeException -> INTERNAL via fallback (unknown exception type) assertEquals(Status.INTERNAL.getCode(), result.getStatus().getCode()); @@ -172,7 +338,7 @@ public RestStatus status() { } }; - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // NOT_FOUND -> NOT_FOUND via RestToGrpcStatusConverter assertEquals(Status.NOT_FOUND.getCode(), result.getStatus().getCode()); @@ -184,7 +350,7 @@ public RestStatus status() { public void testCircuitBreakingExceptionInCleanMessage() { CircuitBreakingException exception = new CircuitBreakingException("Memory circuit breaker", 100, 90, null); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // CircuitBreakingException extends OpenSearchException with TOO_MANY_REQUESTS -> RESOURCE_EXHAUSTED assertEquals(Status.RESOURCE_EXHAUSTED.getCode(), result.getStatus().getCode()); @@ -200,7 +366,7 @@ public void testSearchPhaseExecutionExceptionInCleanMessage() { new org.opensearch.action.search.ShardSearchFailure[0] ); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // SearchPhaseExecutionException extends OpenSearchException with SERVICE_UNAVAILABLE -> UNAVAILABLE assertEquals(Status.UNAVAILABLE.getCode(), result.getStatus().getCode()); @@ -221,7 +387,7 @@ public RestStatus status() { }; exception.addMetadata("opensearch.test_key", "test_value"); - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Should include metadata in JSON details assertTrue(result.getMessage().contains("details=")); @@ -247,7 +413,7 @@ public void metadataToXContent( } }; - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(exception, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Should fall back to base description when XContent extraction fails assertEquals(Status.INVALID_ARGUMENT.getCode(), result.getStatus().getCode()); @@ -272,7 +438,7 @@ public RestStatus status() { } }; - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(wrappedException); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(wrappedException, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // Should include root_cause array like HTTP responses assertTrue(result.getMessage().contains("root_cause")); @@ -281,7 +447,7 @@ public RestStatus status() { } public void testNullExceptionConversion() { - StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(null); + StatusRuntimeException result = GrpcErrorHandler.convertToGrpcError(null, GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); // null -> INTERNAL via case null handling assertEquals(Status.INTERNAL.getCode(), result.getStatus().getCode()); diff --git a/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/util/GrpcParamsHandlerTests.java b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/util/GrpcParamsHandlerTests.java new file mode 100644 index 0000000000000..c44d165183369 --- /dev/null +++ b/modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/util/GrpcParamsHandlerTests.java @@ -0,0 +1,56 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.transport.grpc.util; + +import org.opensearch.test.OpenSearchTestCase; +import org.junit.After; + +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE; +import static org.opensearch.transport.grpc.TestFixtures.GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE; +import static org.opensearch.transport.grpc.TestFixtures.settingsWithGivenStackTraceConfig; + +public class GrpcParamsHandlerTests extends OpenSearchTestCase { + + @After + public void resetStackTraceSettings() { + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(true)); + } + + public void testValidationFailsWhenDetailedErrorsDisabledAndClientRequestedStackTrace() { + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(false)); + + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> GrpcParamsHandler.validateStackTraceDetailsConfiguration(GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE) + ); + assertEquals("error traces in responses are disabled.", exception.getMessage()); + } + + public void testValidationPassesWhenDetailedErrorsDisabledAndClientDoesNotRequestStackTrace() { + GrpcParamsHandler.initialize(settingsWithGivenStackTraceConfig(false)); + + try { + GrpcParamsHandler.validateStackTraceDetailsConfiguration(GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE); + } catch (Exception e) { + fail("Validation should pass without exceptions when stack traces are not requested."); + } + } + + public void testGrpcParamsHandlerPicksErrorTraceRequestParameter() { + assertTrue( + "Params handler must directly pick error_trace=true", + GrpcParamsHandler.isDetailedStackTraceRequested(GLOBAL_PARAMS_WITH_ERROR_TRACE_TRUE) + ); + assertFalse( + "Params handler must directly pick error_trace=false", + GrpcParamsHandler.isDetailedStackTraceRequested(GLOBAL_PARAMS_WITH_ERROR_TRACE_FALSE) + ); + } + +}