Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
d0b39ce
Add the Response Handing Params container and implement error_trace h…
fdesu Oct 14, 2025
6c1c71d
Issue-19639 Add separate grpc.detailed_errors.enabled error handling …
fdesu Oct 15, 2025
702f7d9
Add the CHANGELOG.md entry
fdesu Oct 15, 2025
6392849
Polish formatting
fdesu Oct 15, 2025
9b42790
Merge branch 'main' into issue/19639-grpc-error-handling-setting
fdesu Oct 20, 2025
d54aa0d
Polish docs for the setting
fdesu Oct 25, 2025
8ebf89e
Merge remote-tracking branch 'upstream/main' into issue/19639-grpc-er…
fdesu Oct 25, 2025
bf4d92f
Apply formatting
fdesu Oct 25, 2025
9a73221
Merge remote-tracking branch 'upstream/main' into issue/19639-grpc-er…
fdesu Nov 4, 2025
b330328
Apply formatting
fdesu Nov 4, 2025
483e803
Add basic error stacktrace full vs summary
fdesu Nov 4, 2025
f6d8f9a
Polish tests and add more test cases to verify error summary trimming
fdesu Nov 4, 2025
6f06a3e
Encapsulate handling of global request parameters in an initializable…
fdesu Nov 5, 2025
c4f605b
Polishing
fdesu Nov 5, 2025
80b2bd2
Polishing
fdesu Nov 5, 2025
b4f51b0
Polishing
fdesu Nov 5, 2025
0c3696f
Polish javadocs
fdesu Nov 6, 2025
a63b935
Merge remote-tracking branch 'upstream/main' into issue/19639-grpc-er…
fdesu Nov 6, 2025
725d80b
Merge latest CHANGELOG
fdesu Nov 6, 2025
3f52281
trigger CI
fdesu Nov 6, 2025
e2b0a5f
Make sure error details are included in regular errors
fdesu Nov 7, 2025
402308f
Further polishing and a bit more testing
fdesu Nov 10, 2025
9cd0575
Merge remote-tracking branch 'upstream/main' into issue/19639-grpc-er…
fdesu Nov 10, 2025
990958a
Merge remote-tracking branch 'upstream/main' into issue/19639-grpc-er…
fdesu Nov 17, 2025
72138c1
Polishing
fdesu Nov 17, 2025
1c05dc2
Trigger CI because of flaky ChildQuerySearchIT
fdesu Nov 14, 2025
fef43c4
Trigger CI again after flaky fail
fdesu Nov 14, 2025
2f08602
Rerun CI because of other flaky test
fdesu Nov 14, 2025
a5a64e3
Trigger CI
fdesu Nov 17, 2025
2907384
Merge remote-tracking branch 'upstream/main' into issue/19639-grpc-er…
fdesu Dec 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add pluggable gRPC interceptors with explicit ordering([#19005](https://github.com/opensearch-project/OpenSearch/pull/19005))
- Add BindableServices extension point to transport-grpc-spi ([#19304](https://github.com/opensearch-project/OpenSearch/pull/19304))
- Add metrics for the merged segment warmer feature ([#18929](https://github.com/opensearch-project/OpenSearch/pull/18929))
- Add handling for the global gRPC error_trace parameter ([#19644](https://github.com/opensearch-project/OpenSearch/pull/19644))
- Handle deleted documents for filter rewrite subaggregation optimization ([#19643](https://github.com/opensearch-project/OpenSearch/pull/19643))
- Handle deleted documents for filter rewrite sub-aggregation optimization ([#19643](https://github.com/opensearch-project/OpenSearch/pull/19643))
Comment on lines +18 to 20
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Incomplete changelog entry and duplicate entry detected.

Line 18 describes the PR but only mentions the error_trace parameter, omitting the grpc.detailed_errors.enabled setting that was introduced according to the PR objectives. Additionally, lines 19 and 20 are duplicate entries—both reference PR #19643 with nearly identical descriptions but inconsistent terminology ("subaggregation" vs. "sub-aggregation").

Apply this diff to expand the gRPC entry and remove the duplicate:

- Add handling for the global gRPC error_trace parameter ([#19644](https://github.com/opensearch-project/OpenSearch/pull/19644))
- Handle deleted documents for filter rewrite subaggregation optimization ([#19643](https://github.com/opensearch-project/OpenSearch/pull/19643))
- Handle deleted documents for filter rewrite sub-aggregation optimization ([#19643](https://github.com/opensearch-project/OpenSearch/pull/19643))
+ Add handling for the global gRPC error_trace parameter and grpc.detailed_errors.enabled setting ([#19644](https://github.com/opensearch-project/OpenSearch/pull/19644))
+ Handle deleted documents for filter rewrite sub-aggregation optimization ([#19643](https://github.com/opensearch-project/OpenSearch/pull/19643))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Add handling for the global gRPC error_trace parameter ([#19644](https://github.com/opensearch-project/OpenSearch/pull/19644))
- Handle deleted documents for filter rewrite subaggregation optimization ([#19643](https://github.com/opensearch-project/OpenSearch/pull/19643))
- Handle deleted documents for filter rewrite sub-aggregation optimization ([#19643](https://github.com/opensearch-project/OpenSearch/pull/19643))
- Add handling for the global gRPC error_trace parameter and grpc.detailed_errors.enabled setting ([#19644](https://github.com/opensearch-project/OpenSearch/pull/19644))
- Handle deleted documents for filter rewrite sub-aggregation optimization ([#19643](https://github.com/opensearch-project/OpenSearch/pull/19643))
🤖 Prompt for AI Agents
In CHANGELOG.md around lines 18 to 20, the gRPC changelog entry is incomplete
and there is a duplicate entry for PR #19643; update the gRPC line to mention
both the global gRPC error_trace parameter and the new
grpc.detailed_errors.enabled setting (briefly describe their effect), remove one
of the duplicate lines for PR #19643, and normalize the remaining PR #19643
entry to use consistent wording (e.g., "sub-aggregation") so the three bullets
read: the expanded gRPC item and a single, consistently-worded filter rewrite
sub-aggregation optimization line.

- Add bulk collect API for filter rewrite sub-aggregation optimization ([#19933](https://github.com/opensearch-project/OpenSearch/pull/19933))
- Allow collectors take advantage of preaggregated data using collectRange API ([#20009](https://github.com/opensearch-project/OpenSearch/pull/20009))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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<QueryBuilderProtoConverter> queryConverters = new ArrayList<>();
Expand Down Expand Up @@ -182,6 +186,7 @@ public Map<String, Supplier<AuxTransport>> getAuxTransports(
}

return Collections.singletonMap(GRPC_TRANSPORT_SETTING_KEY, () -> {
GrpcParamsHandler.initialize(settings);
List<BindableService> grpcServices = new ArrayList<>(
List.of(new DocumentServiceImpl(client), new SearchServiceImpl(client, queryUtils))
);
Expand Down Expand Up @@ -233,6 +238,7 @@ public Map<String, Supplier<AuxTransport>> 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<BindableService> grpcServices = new ArrayList<>(
List.of(new DocumentServiceImpl(client), new SearchServiceImpl(client, queryUtils))
);
Expand Down Expand Up @@ -282,7 +288,8 @@ public List<Setting<?>> 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
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Boolean> SETTING_GRPC_DETAILED_ERRORS_ENABLED = Setting.boolSetting(
"grpc.detailed_errors.enabled",
true,
Setting.Property.NodeScope
);

/**
* Port range on which servers bind.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -26,15 +27,18 @@
public class BulkRequestActionListener implements ActionListener<BulkResponse> {
private static final Logger logger = LogManager.getLogger(BulkRequestActionListener.class);
private final StreamObserver<org.opensearch.protobufs.BulkResponse> 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<org.opensearch.protobufs.BulkResponse> responseObserver) {
public BulkRequestActionListener(StreamObserver<org.opensearch.protobufs.BulkResponse> responseObserver, GlobalParams params) {
super();
this.responseObserver = responseObserver;
this.params = params;
}

/**
Expand All @@ -47,12 +51,12 @@ public BulkRequestActionListener(StreamObserver<org.opensearch.protobufs.BulkRes
public void onResponse(org.opensearch.action.bulk.BulkResponse response) {
// Bulk execution succeeded. Convert the opensearch internal response to protobuf
try {
org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(response);
org.opensearch.protobufs.BulkResponse protoResponse = BulkResponseProtoUtils.toProto(response, params);
responseObserver.onNext(protoResponse);
responseObserver.onCompleted();
} catch (RuntimeException | IOException e) {
logger.error("Failed to convert bulk response to protobuf: " + e.getMessage());
StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e);
StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e, params);
responseObserver.onError(grpcError);
}
}
Expand All @@ -66,7 +70,7 @@ public void onResponse(org.opensearch.action.bulk.BulkResponse response) {
@Override
public void onFailure(Exception e) {
logger.error("BulkRequestActionListener failed to process bulk request: " + e.getMessage());
StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e);
StatusRuntimeException grpcError = GrpcErrorHandler.convertToGrpcError(e, params);
responseObserver.onError(grpcError);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.apache.logging.log4j.Logger;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.core.action.ActionListener;
import org.opensearch.protobufs.GlobalParams;
import org.opensearch.transport.grpc.proto.response.search.SearchResponseProtoUtils;
import org.opensearch.transport.grpc.util.GrpcErrorHandler;

Expand All @@ -27,35 +28,38 @@ public class SearchRequestActionListener implements ActionListener<SearchRespons
private static final Logger logger = LogManager.getLogger(SearchRequestActionListener.class);

private final StreamObserver<org.opensearch.protobufs.SearchResponse> 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<org.opensearch.protobufs.SearchResponse> responseObserver) {
public SearchRequestActionListener(StreamObserver<org.opensearch.protobufs.SearchResponse> 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);
}
}

@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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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();
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -30,18 +31,19 @@ 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());
shardInfoBuilder.setFailed(shardInfo.getFailed());

// Add any shard failures
for (ReplicationResponse.ShardInfo.Failure failure : shardInfo.getFailures()) {
shardInfoBuilder.addFailures(toProto(failure));
shardInfoBuilder.addFailures(toProto(failure, params));
}

return shardInfoBuilder.build();
Expand All @@ -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();
Expand Down
Loading
Loading