Skip to content

Commit

Permalink
Fix the HttpChannel trace termination in case of exceptions (opensear…
Browse files Browse the repository at this point in the history
…ch-project#10325)

Signed-off-by: Andriy Redko <[email protected]>
  • Loading branch information
reta authored Oct 3, 2023
1 parent 5aa1863 commit bddf0d3
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@
*/
@InternalApi
class DefaultTracer implements Tracer {
static final String THREAD_NAME = "th_name";
/**
* Current thread name.
*/
static final String THREAD_NAME = "thread.name";

private final TracingTelemetry tracingTelemetry;
private final TracerContextStorage<String, Span> tracerContextStorage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ static int resolvePublishPort(Settings settings, List<TransportAddress> boundAdd
}

public void onException(HttpChannel channel, Exception e) {
channel.handleException(e);
if (lifecycle.started() == false) {
// just close and ignore - we are already stopped and just need to make sure we release all resources
CloseableChannel.closeChannel(channel);
Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/org/opensearch/http/HttpChannel.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
* @opensearch.internal
*/
public interface HttpChannel extends CloseableChannel {
/**
* Notify HTTP channel that exception happens and the response may not be sent (for example, timeout)
* @param ex the exception being raised
*/
default void handleException(Exception ex) {}

/**
* Sends an http response to the channel. The listener will be executed once the send process has been
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ public static HttpChannel create(HttpChannel delegate, Span span, Tracer tracer)
}
}

@Override
public void handleException(Exception ex) {
span.addEvent("The HttpChannel was closed without sending the response");
span.setError(ex);
span.endSpan();
}

@Override
public void close() {
delegate.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ public void handleRejection(Exception exp) {
try (SpanScope scope = tracer.withSpanInScope(span)) {
delegate.handleRejection(exp);
} finally {
span.setError(exp);
span.endSpan();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public interface TransportResponseHandler<T extends TransportResponse> extends W
* It should be used to clear up the resources held by the {@link TransportResponseHandler}.
* @param exp exception
*/
default void handleRejection(Exception exp) {};
default void handleRejection(Exception exp) {}

default <Q extends TransportResponse> TransportResponseHandler<Q> wrap(Function<Q, T> converter, Writeable.Reader<Q> reader) {
final TransportResponseHandler<T> self = this;
Expand Down

0 comments on commit bddf0d3

Please sign in to comment.