You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2018/06/30 03:29:11 UTC
[GitHub] liubao68 closed pull request #777: [SCB-696] access log print %v
correctly in short-lived connections
liubao68 closed pull request #777: [SCB-696] access log print %v correctly in short-lived connections
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/777
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogParam.java b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogParam.java
index fcc2d4d7d..809030f2f 100644
--- a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogParam.java
+++ b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/AccessLogParam.java
@@ -31,6 +31,14 @@
private long endMillisecond;
+ /**
+ * If client send request via a short-lived connection, the connection may be closed before the corresponding
+ * access log is generated, and then the
+ * {@linkplain org.apache.servicecomb.transport.rest.vertx.accesslog.element.impl.LocalHostItem LocalHostItem}
+ * may get "0.0.0.0" as result. So we need to get local address before the connection is closed.
+ */
+ private String localAddress;
+
public T getContextData() {
return contextData;
}
@@ -58,12 +66,22 @@ public long getEndMillisecond() {
return this;
}
+ public String getLocalAddress() {
+ return localAddress;
+ }
+
+ public AccessLogParam<T> setLocalAddress(String localAddress) {
+ this.localAddress = localAddress;
+ return this;
+ }
+
@Override
public String toString() {
final StringBuilder sb = new StringBuilder("AccessLogParam{");
sb.append("contextData=").append(contextData);
sb.append(", startMillisecond=").append(startMillisecond);
sb.append(", endMillisecond=").append(endMillisecond);
+ sb.append(", localAddress='").append(localAddress).append('\'');
sb.append('}');
return sb.toString();
}
diff --git a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/element/impl/LocalHostItem.java b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/element/impl/LocalHostItem.java
index 220632869..cbb1e87b3 100644
--- a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/element/impl/LocalHostItem.java
+++ b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/element/impl/LocalHostItem.java
@@ -31,6 +31,10 @@
@Override
public String getFormattedItem(AccessLogParam<RoutingContext> accessLogParam) {
+ return accessLogParam.getLocalAddress();
+ }
+
+ public static String getLocalAddress(AccessLogParam<RoutingContext> accessLogParam) {
HttpServerRequest request = accessLogParam.getContextData().request();
if (null == request) {
return EMPTY_RESULT;
diff --git a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/impl/AccessLogHandler.java b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/impl/AccessLogHandler.java
index 01b5676c5..0edc7c05b 100644
--- a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/impl/AccessLogHandler.java
+++ b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/accesslog/impl/AccessLogHandler.java
@@ -19,6 +19,7 @@
import org.apache.servicecomb.transport.rest.vertx.accesslog.AccessLogGenerator;
import org.apache.servicecomb.transport.rest.vertx.accesslog.AccessLogParam;
+import org.apache.servicecomb.transport.rest.vertx.accesslog.element.impl.LocalHostItem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -36,11 +37,17 @@ public AccessLogHandler(String rawPattern) {
@Override
public void handle(RoutingContext context) {
- AccessLogParam<RoutingContext> accessLogParam = new AccessLogParam<>();
- accessLogParam.setStartMillisecond(System.currentTimeMillis()).setContextData(context);
+ AccessLogParam<RoutingContext> accessLogParam = getRoutingContextAccessLogParam(context);
context.response().endHandler(event -> LOGGER.info(accessLogGenerator.generateLog(accessLogParam)));
context.next();
}
+
+ private AccessLogParam<RoutingContext> getRoutingContextAccessLogParam(RoutingContext context) {
+ AccessLogParam<RoutingContext> accessLogParam = new AccessLogParam<>();
+ accessLogParam.setStartMillisecond(System.currentTimeMillis()).setContextData(context);
+ accessLogParam.setLocalAddress(LocalHostItem.getLocalAddress(accessLogParam));
+ return accessLogParam;
+ }
}
diff --git a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/element/impl/LocalHostItemTest.java b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/element/impl/LocalHostItemTest.java
index f70c88e34..285529d72 100644
--- a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/element/impl/LocalHostItemTest.java
+++ b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/element/impl/LocalHostItemTest.java
@@ -20,6 +20,7 @@
import static org.junit.Assert.assertEquals;
import org.apache.servicecomb.transport.rest.vertx.accesslog.AccessLogParam;
+import org.junit.Assert;
import org.junit.Test;
import org.mockito.Mockito;
@@ -32,7 +33,16 @@
public static final LocalHostItem ELEMENT = new LocalHostItem();
@Test
- public void getFormattedElement() {
+ public void getFormattedItem() {
+ String localAddress = "192.168.0.1";
+ AccessLogParam<RoutingContext> accessLogParam = new AccessLogParam<>();
+ accessLogParam.setLocalAddress(localAddress);
+
+ Assert.assertEquals(localAddress, ELEMENT.getFormattedItem(accessLogParam));
+ }
+
+ @Test
+ public void getLocalAddress() {
AccessLogParam<RoutingContext> param = new AccessLogParam<>();
RoutingContext context = Mockito.mock(RoutingContext.class);
HttpServerRequest request = Mockito.mock(HttpServerRequest.class);
@@ -44,26 +54,26 @@ public void getFormattedElement() {
Mockito.when(request.localAddress()).thenReturn(localAddress);
Mockito.when(localAddress.host()).thenReturn(localHost);
- String result = ELEMENT.getFormattedItem(param);
+ String result = LocalHostItem.getLocalAddress(param);
assertEquals(localHost, result);
}
@Test
- public void getFormattedElementOnRequestIsNull() {
+ public void getLocalAddressOnRequestIsNull() {
AccessLogParam<RoutingContext> param = new AccessLogParam<>();
RoutingContext context = Mockito.mock(RoutingContext.class);
param.setContextData(context);
Mockito.when(context.request()).thenReturn(null);
- String result = ELEMENT.getFormattedItem(param);
+ String result = LocalHostItem.getLocalAddress(param);
assertEquals("-", result);
}
@Test
- public void getFormattedElementOnLocalAddressIsNull() {
+ public void getLocalAddressOnLocalAddressIsNull() {
AccessLogParam<RoutingContext> param = new AccessLogParam<>();
RoutingContext context = Mockito.mock(RoutingContext.class);
HttpServerRequest request = Mockito.mock(HttpServerRequest.class);
@@ -72,13 +82,13 @@ public void getFormattedElementOnLocalAddressIsNull() {
Mockito.when(context.request()).thenReturn(request);
Mockito.when(request.localAddress()).thenReturn(null);
- String result = ELEMENT.getFormattedItem(param);
+ String result = LocalHostItem.getLocalAddress(param);
assertEquals("-", result);
}
@Test
- public void getFormattedElementOnHostIsNull() {
+ public void getLocalAddressOnHostIsNull() {
AccessLogParam<RoutingContext> param = new AccessLogParam<>();
RoutingContext context = Mockito.mock(RoutingContext.class);
HttpServerRequest request = Mockito.mock(HttpServerRequest.class);
@@ -89,13 +99,13 @@ public void getFormattedElementOnHostIsNull() {
Mockito.when(request.localAddress()).thenReturn(localAddress);
Mockito.when(localAddress.host()).thenReturn(null);
- String result = ELEMENT.getFormattedItem(param);
+ String result = LocalHostItem.getLocalAddress(param);
assertEquals("-", result);
}
@Test
- public void getFormattedElementIsEmpty() {
+ public void getLocalAddressIsEmpty() {
AccessLogParam<RoutingContext> param = new AccessLogParam<>();
RoutingContext context = Mockito.mock(RoutingContext.class);
HttpServerRequest request = Mockito.mock(HttpServerRequest.class);
@@ -107,7 +117,7 @@ public void getFormattedElementIsEmpty() {
Mockito.when(request.localAddress()).thenReturn(localAddress);
Mockito.when(localAddress.host()).thenReturn(localHost);
- String result = ELEMENT.getFormattedItem(param);
+ String result = LocalHostItem.getLocalAddress(param);
assertEquals("-", result);
}
diff --git a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/impl/AccessLogHandlerTest.java b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/impl/AccessLogHandlerTest.java
index ce96d4956..efd4ee2b7 100644
--- a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/impl/AccessLogHandlerTest.java
+++ b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/accesslog/impl/AccessLogHandlerTest.java
@@ -40,7 +40,7 @@
public class AccessLogHandlerTest {
private static final AccessLogHandler ACCESS_LOG_HANDLER = new AccessLogHandler(
- "%h - - %s durationMillisecond=[%D] %{test-config}user-defined %{cookie-name}C");
+ "%h - - %s durationMillisecond=[%D] %{test-config}user-defined %{cookie-name}C %v");
private LogCollector logCollector;
@@ -73,7 +73,8 @@ public int getStatusCode() {
}
}.getMockInstance();
HttpServerRequest httpServerRequest = Mockito.mock(HttpServerRequest.class);
- SocketAddress socketAddress = Mockito.mock(SocketAddress.class);
+ SocketAddress remoteSocketAddress = Mockito.mock(SocketAddress.class);
+ SocketAddress localSocketAddress = Mockito.mock(SocketAddress.class);
Holder<Integer> counter = new Holder<>();
counter.value = 0;
@@ -93,12 +94,14 @@ long currentTimeMillis() {
Mockito.when(routingContext.cookies()).thenReturn(cookies);
Mockito.when(routingContext.response()).thenReturn(httpServerResponse);
Mockito.when(routingContext.request()).thenReturn(httpServerRequest);
- Mockito.when(httpServerRequest.remoteAddress()).thenReturn(socketAddress);
- Mockito.when(socketAddress.host()).thenReturn("localhost");
-
+ Mockito.when(httpServerRequest.remoteAddress()).thenReturn(remoteSocketAddress);
+ Mockito.when(remoteSocketAddress.host()).thenReturn("192.168.0.22");
+ Mockito.when(httpServerRequest.localAddress()).thenReturn(localSocketAddress);
+ Mockito.when(localSocketAddress.host()).thenReturn("192.168.0.33");
ACCESS_LOG_HANDLER.handle(routingContext);
- Assert.assertEquals("localhost - - 200 durationMillisecond=[122] user-defined-test-config cookie-value",
+ Assert.assertEquals(
+ "192.168.0.22 - - 200 durationMillisecond=[122] user-defined-test-config cookie-value 192.168.0.33",
logCollector.getEvents().get(0).getMessage());
}
}
\ No newline at end of file
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services