You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2023/11/01 09:13:07 UTC
(servicecomb-java-chassis) branch 2.8.x updated: [#3727]Fix metrics not correct when timeout/connection timeout (#4006)
This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch 2.8.x
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/2.8.x by this push:
new a2a1c2d85 [#3727]Fix metrics not correct when timeout/connection timeout (#4006)
a2a1c2d85 is described below
commit a2a1c2d859315b1fbec6a87a4c22c7a8b0843d3f
Author: liubao68 <bi...@qq.com>
AuthorDate: Wed Nov 1 17:13:02 2023 +0800
[#3727]Fix metrics not correct when timeout/connection timeout (#4006)
---
.../rest/client/RestClientCodecFilter.java | 28 ++++++++++++++++++++--
.../transport/rest/client/RestClientSender.java | 25 +++++++++++--------
.../rest/client/http/RestClientInvocation.java | 8 +------
.../rest/client/http/TestRestClientInvocation.java | 23 +++++++++---------
4 files changed, 54 insertions(+), 30 deletions(-)
diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientCodecFilter.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientCodecFilter.java
index 6a530f256..2cf13cbad 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientCodecFilter.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientCodecFilter.java
@@ -16,10 +16,13 @@
*/
package org.apache.servicecomb.transport.rest.client;
+import java.util.Map;
import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionStage;
import javax.annotation.Nonnull;
+import org.apache.servicecomb.common.rest.RestConst;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.core.filter.ConsumerFilter;
import org.apache.servicecomb.core.filter.FilterNode;
@@ -67,9 +70,13 @@ public class RestClientCodecFilter implements ConsumerFilter {
public CompletableFuture<Response> onFilter(Invocation invocation, FilterNode nextNode) {
invocation.getInvocationStageTrace().startGetConnection();
startClientFiltersRequest(invocation);
+ CompletionStage<HttpClientRequest> createRequest =
+ transportContextFactory.createHttpClientRequest(invocation).toCompletionStage()
+ .whenComplete((c, e) -> invocation.getInvocationStageTrace().finishGetConnection());
return CompletableFuture.completedFuture(null)
- .thenCompose(v -> transportContextFactory.createHttpClientRequest(invocation).toCompletionStage())
+ .thenCompose(v -> createRequest)
.thenAccept(httpClientRequest -> prepareTransportContext(invocation, httpClientRequest))
+ .thenAccept(v -> invocation.onStartSendRequest())
.thenAccept(v -> encoder.encode(invocation))
.thenCompose(v -> nextNode.onFilter(invocation))
.thenApply(response -> decoder.decode(invocation, response))
@@ -81,12 +88,29 @@ public class RestClientCodecFilter implements ConsumerFilter {
}
protected void prepareTransportContext(Invocation invocation, HttpClientRequest httpClientRequest) {
- invocation.getInvocationStageTrace().finishGetConnection();
+ copyExtraHttpHeaders(invocation, httpClientRequest);
RestClientTransportContext transportContext = transportContextFactory.create(invocation, httpClientRequest);
invocation.setTransportContext(transportContext);
}
+ @SuppressWarnings("unchecked")
+ protected void copyExtraHttpHeaders(Invocation invocation, HttpClientRequest httpClientRequest) {
+ Map<String, String> httpHeaders = (Map<String, String>) invocation.getHandlerContext()
+ .get(RestConst.CONSUMER_HEADER);
+ if (httpHeaders == null) {
+ return;
+ }
+ httpHeaders.forEach((key, value) -> {
+ if ("Content-Length".equalsIgnoreCase(key)) {
+ return;
+ }
+ if (null != value) {
+ httpClientRequest.putHeader(key, value);
+ }
+ });
+ }
+
protected void finishClientFiltersResponse(Invocation invocation) {
invocation.getInvocationStageTrace().finishClientFiltersResponse();
}
diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSender.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSender.java
index 3bd08dbc1..6816d24ec 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSender.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestClientSender.java
@@ -75,9 +75,11 @@ public class RestClientSender {
protected void runInVertxContext() {
sendInVertxContext()
- .exceptionally(throwable -> {
- future.completeExceptionally(throwable);
- return null;
+ .whenComplete((v, e) -> {
+ if (e != null) {
+ future.completeExceptionally(e);
+ }
+ writeFinished();
});
}
@@ -87,11 +89,12 @@ public class RestClientSender {
Multimap<String, Part> uploads = requestParameters.getUploads();
if (uploads == null) {
if (requestParameters.getBodyBuffer() != null) {
- httpClientRequest.end(requestParameters.getBodyBuffer());
+ return CompletableFuture.completedFuture(null).thenCompose(
+ v -> httpClientRequest.end(requestParameters.getBodyBuffer()).toCompletionStage());
} else {
- httpClientRequest.end();
+ return CompletableFuture.completedFuture(null).thenCompose(
+ v -> httpClientRequest.end().toCompletionStage());
}
- return CompletableFuture.completedFuture(null);
}
if (requestParameters.getBodyBuffer() != null) {
@@ -100,6 +103,10 @@ public class RestClientSender {
return sendFiles();
}
+ private void writeFinished() {
+ invocation.getInvocationStageTrace().finishWriteToBuffer(System.nanoTime());
+ }
+
protected CompletableFuture<Void> sendFiles() {
CompletableFuture<Void> sendFileFuture = CompletableFuture.completedFuture(null);
@@ -111,8 +118,7 @@ public class RestClientSender {
sendFileFuture = sendFileFuture.thenCompose(v -> sendFile(entry.getValue(), name, boundary));
}
- return sendFileFuture
- .thenAccept(v -> httpClientRequest.end(genBoundaryEndBuffer(boundary)));
+ return sendFileFuture.thenCompose(v -> httpClientRequest.end(genBoundaryEndBuffer(boundary)).toCompletionStage());
}
private CompletableFuture<Void> sendFile(Part part, String name, String boundary) {
@@ -167,8 +173,7 @@ public class RestClientSender {
protected void processMetrics() {
InvocationStageTrace stageTrace = invocation.getInvocationStageTrace();
- stageTrace.finishWriteToBuffer(System.nanoTime());
- // even failed and did not received response, still set time for it
+ // even failed and did not receive response, still set time for it
// that will help to know the real timeout time
stageTrace.finishReceiveResponse();
stageTrace.startClientFiltersResponse();
diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
index 6ddf1e23b..ded7b6e2c 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
@@ -103,11 +103,9 @@ public class RestClientInvocation {
IpPort ipPort = (IpPort) invocation.getEndpoint().getAddress();
Future<HttpClientRequest> requestFuture = createRequest(ipPort, path);
-
invocation.getInvocationStageTrace().startGetConnection();
+ requestFuture.onComplete(r -> invocation.getInvocationStageTrace().finishGetConnection());
requestFuture.compose(clientRequest -> {
- invocation.getInvocationStageTrace().finishGetConnection();
-
this.clientRequest = clientRequest;
clientRequest.putHeader(org.apache.servicecomb.core.Const.TARGET_MICROSERVICE, invocation.getMicroserviceName());
@@ -270,10 +268,6 @@ public class RestClientInvocation {
InvocationStageTrace stageTrace = invocation.getInvocationStageTrace();
- if (stageTrace.getFinishWriteToBuffer() == 0) {
- stageTrace.finishWriteToBuffer(System.nanoTime());
- }
-
// even failed and did not received response, still set time for it
// that will help to know the real timeout time
if (stageTrace.getFinishReceiveResponse() == 0) {
diff --git a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestRestClientInvocation.java b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestRestClientInvocation.java
index ea5dd4800..9e806176f 100644
--- a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestRestClientInvocation.java
+++ b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestRestClientInvocation.java
@@ -179,7 +179,8 @@ public class TestRestClientInvocation {
MatcherAssert.assertThat(headers.names(),
Matchers.containsInAnyOrder(org.apache.servicecomb.core.Const.TARGET_MICROSERVICE,
org.apache.servicecomb.core.Const.CSE_CONTEXT));
- Assertions.assertEquals(TARGET_MICROSERVICE_NAME, headers.get(org.apache.servicecomb.core.Const.TARGET_MICROSERVICE));
+ Assertions.assertEquals(TARGET_MICROSERVICE_NAME,
+ headers.get(org.apache.servicecomb.core.Const.TARGET_MICROSERVICE));
Assertions.assertEquals("{}", headers.get(org.apache.servicecomb.core.Const.CSE_CONTEXT));
Assertions.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartClientFiltersRequest());
}
@@ -216,7 +217,8 @@ public class TestRestClientInvocation {
MatcherAssert.assertThat(headers.names(),
Matchers.containsInAnyOrder(org.apache.servicecomb.core.Const.TARGET_MICROSERVICE,
org.apache.servicecomb.core.Const.CSE_CONTEXT));
- Assertions.assertEquals(TARGET_MICROSERVICE_NAME, headers.get(org.apache.servicecomb.core.Const.TARGET_MICROSERVICE));
+ Assertions.assertEquals(TARGET_MICROSERVICE_NAME,
+ headers.get(org.apache.servicecomb.core.Const.TARGET_MICROSERVICE));
Assertions.assertEquals("{}", headers.get(org.apache.servicecomb.core.Const.CSE_CONTEXT));
Assertions.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartClientFiltersRequest());
operationConfig.setClientRequestHeaderFilterEnabled(true);
@@ -286,11 +288,11 @@ public class TestRestClientInvocation {
when(invocation.getContext()).thenReturn(contextMap);
restClientInvocation.setCseContext();
- String context = headers.get(org.apache.servicecomb.core.Const.CSE_CONTEXT);
- HttpServletRequestEx requestEx = new MockUp<HttpServletRequestEx>(){
+ String context = headers.get(org.apache.servicecomb.core.Const.CSE_CONTEXT);
+ HttpServletRequestEx requestEx = new MockUp<HttpServletRequestEx>() {
@Mock
- public String getHeader(String name){
- if (StringUtils.equals(name, org.apache.servicecomb.core.Const.CSE_CONTEXT)){
+ public String getHeader(String name) {
+ if (StringUtils.equals(name, org.apache.servicecomb.core.Const.CSE_CONTEXT)) {
return context;
} else {
return null;
@@ -324,11 +326,11 @@ public class TestRestClientInvocation {
};
restClientInvocation.setCseContext();
- String context = headers.get(org.apache.servicecomb.core.Const.CSE_CONTEXT);
- HttpServletRequestEx requestEx = new MockUp<HttpServletRequestEx>(){
+ String context = headers.get(org.apache.servicecomb.core.Const.CSE_CONTEXT);
+ HttpServletRequestEx requestEx = new MockUp<HttpServletRequestEx>() {
@Mock
- public String getHeader(String name){
- if (StringUtils.equals(name, org.apache.servicecomb.core.Const.CSE_CONTEXT)){
+ public String getHeader(String name) {
+ if (StringUtils.equals(name, org.apache.servicecomb.core.Const.CSE_CONTEXT)) {
return context;
} else {
return null;
@@ -419,7 +421,6 @@ public class TestRestClientInvocation {
Assertions.assertEquals(nanoTime, invocation.getInvocationStageTrace().getStartClientFiltersResponse());
Assertions.assertEquals(nanoTime, invocation.getInvocationStageTrace().getFinishClientFiltersResponse());
Assertions.assertEquals(nanoTime, invocation.getInvocationStageTrace().getFinishReceiveResponse());
- Assertions.assertEquals(nanoTime, invocation.getInvocationStageTrace().getFinishWriteToBuffer());
}
@Test