Skip to content

Commit 0f03dfe

Browse files
committed
Fix requestEndTime is set after send response when empty content request
1 parent d897515 commit 0f03dfe

4 files changed

Lines changed: 73 additions & 1 deletion

File tree

core/src/main/java/com/linecorp/armeria/common/logging/DefaultRequestLog.java

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,25 @@ public int currentAttempt() {
11181118
return currentAttempt;
11191119
}
11201120

1121+
@Override
1122+
public void requestEnd() {
1123+
requestEnd0(System.nanoTime());
1124+
}
1125+
1126+
@Override
1127+
public void requestEnd(long requestEndTimeNanos) {
1128+
requestEnd0(requestEndTimeNanos);
1129+
}
1130+
1131+
private void requestEnd0(long requestEndTimeNanos) {
1132+
if (isAvailable(REQUEST_END_TIME)) {
1133+
return;
1134+
}
1135+
1136+
this.requestEndTimeNanos = requestEndTimeNanos;
1137+
updateFlags(REQUEST_END_TIME);
1138+
}
1139+
11211140
@Override
11221141
public void endRequest() {
11231142
endRequest0(null);
@@ -1183,7 +1202,10 @@ private void endRequest0(@Nullable Throwable requestCause, long requestEndTimeNa
11831202
setNamesIfAbsent();
11841203
}
11851204

1186-
this.requestEndTimeNanos = requestEndTimeNanos;
1205+
// Set requestEndTimeNanos if it is not already set
1206+
if (!isAvailable(REQUEST_END_TIME)) {
1207+
this.requestEndTimeNanos = requestEndTimeNanos;
1208+
}
11871209

11881210
setRequestCause(requestCause);
11891211
updateFlags(flags);

core/src/main/java/com/linecorp/armeria/common/logging/RequestLogBuilder.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,16 @@ void session(@Nullable Channel channel, SessionProtocol sessionProtocol, @Nullab
189189
@UnstableApi
190190
void requestCause(Throwable cause);
191191

192+
/**
193+
* Sets the {@link RequestLog#requestEndTimeNanos()}.
194+
*/
195+
void requestEnd();
196+
197+
/**
198+
* Sets the {@link RequestLog#requestEndTimeNanos()} with the specified timestamp.
199+
*/
200+
void requestEnd(long requestEndTimeNanos);
201+
192202
/**
193203
* Finishes the collection of the {@link Request} information. This method sets the following properties:
194204
* <ul>

core/src/main/java/com/linecorp/armeria/server/EmptyContentDecodedHttpRequest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ final class EmptyContentDecodedHttpRequest implements DecodedHttpRequest {
8383
@Override
8484
public void init(ServiceRequestContext ctx) {
8585
this.ctx = ctx;
86+
87+
// EmptyContentDecodedHttpRequest does not have any additional data to read.
88+
ctx.logBuilder().requestEnd();
8689
}
8790

8891
@Nullable

core/src/test/java/com/linecorp/armeria/server/EmptyContentDecodedHttpRequestTest.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,17 @@
2222
import org.junit.jupiter.api.Test;
2323
import org.junit.jupiter.api.extension.RegisterExtension;
2424

25+
import com.linecorp.armeria.client.WebClient;
26+
import com.linecorp.armeria.common.AggregatedHttpResponse;
2527
import com.linecorp.armeria.common.ExchangeType;
2628
import com.linecorp.armeria.common.HttpMethod;
29+
import com.linecorp.armeria.common.HttpResponse;
30+
import com.linecorp.armeria.common.HttpStatus;
2731
import com.linecorp.armeria.common.RequestHeaders;
32+
import com.linecorp.armeria.common.logging.RequestLog;
33+
import com.linecorp.armeria.server.annotation.Get;
2834
import com.linecorp.armeria.testing.junit5.common.EventLoopExtension;
35+
import com.linecorp.armeria.testing.junit5.server.ServerExtension;
2936

3037
import reactor.test.StepVerifier;
3138

@@ -34,6 +41,21 @@ class EmptyContentDecodedHttpRequestTest {
3441
@RegisterExtension
3542
static EventLoopExtension eventLoop = new EventLoopExtension();
3643

44+
@RegisterExtension
45+
static ServerExtension server = new ServerExtension() {
46+
@Override
47+
protected void configure(ServerBuilder sb) {
48+
sb.annotatedService(new AnnotatedService());
49+
}
50+
};
51+
52+
static class AnnotatedService {
53+
@Get("/get")
54+
public HttpResponse get() {
55+
return HttpResponse.of(HttpStatus.OK);
56+
}
57+
}
58+
3759
@Test
3860
void emptyContent() {
3961
final RoutingContext routingContext = mock(RoutingContext.class);
@@ -47,4 +69,19 @@ void emptyContent() {
4769
.verify();
4870
assertThat(req.headers()).isEqualTo(headers);
4971
}
72+
73+
@Test
74+
void requestEndTimeNanosIsFasterThanResponseStartTimeNanos()
75+
throws InterruptedException {
76+
final WebClient webClient = server.webClient();
77+
78+
final AggregatedHttpResponse response = webClient.get("/get")
79+
.aggregate()
80+
.join();
81+
assertThat(response.status()).isEqualTo(HttpStatus.OK);
82+
83+
final ServiceRequestContext sctx = server.requestContextCaptor().take();
84+
final RequestLog log = sctx.log().whenComplete().join();
85+
assertThat(log.requestEndTimeNanos()).isLessThan(log.responseStartTimeNanos());
86+
}
5087
}

0 commit comments

Comments
 (0)