You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by re...@apache.org on 2020/11/20 14:20:27 UTC
[cxf] 01/02: CXF-8367: Micrometer always reports outcome=UNKNOWN on
success cases (#726)
This is an automated email from the ASF dual-hosted git repository.
reta pushed a commit to branch 3.4.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 2c8f01789005fea35730a92f547ee323bbcbecba
Author: Andriy Redko <dr...@gmail.com>
AuthorDate: Fri Nov 20 08:55:44 2020 -0500
CXF-8367: Micrometer always reports outcome=UNKNOWN on success cases (#726)
(cherry picked from commit c4e1d0ef71d4a86e5dc641c4a793cf294ac59509)
---
.../java/org/apache/cxf/message/MessageUtils.java | 51 ++++++++++++++++++++++
.../metrics/micrometer/provider/StandardTags.java | 14 +++---
.../provider/jaxws/JaxwsFaultCodeProvider.java | 9 ++--
.../micrometer/provider/StandardTagsTest.java | 25 +++--------
.../provider/jaxws/JaxwsFaultCodeProviderTest.java | 34 ++++++++++++++-
.../transport/http/AbstractHTTPDestination.java | 40 ++---------------
6 files changed, 101 insertions(+), 72 deletions(-)
diff --git a/core/src/main/java/org/apache/cxf/message/MessageUtils.java b/core/src/main/java/org/apache/cxf/message/MessageUtils.java
index 34937f9..043c6af 100644
--- a/core/src/main/java/org/apache/cxf/message/MessageUtils.java
+++ b/core/src/main/java/org/apache/cxf/message/MessageUtils.java
@@ -20,6 +20,7 @@
package org.apache.cxf.message;
import java.lang.reflect.Method;
+import java.net.HttpURLConnection;
import java.util.Optional;
import java.util.logging.Logger;
@@ -205,4 +206,54 @@ public final class MessageUtils {
}
return Optional.ofNullable(method);
}
+
+ /**
+ * Gets the response code from the message and tries to deduct one if it
+ * is not set yet.
+ * @param message message to get response code from
+ * @return response code (or deducted value assuming success)
+ */
+ public static int getReponseCodeFromMessage(Message message) {
+ Integer i = (Integer)message.get(Message.RESPONSE_CODE);
+ if (i != null) {
+ return i.intValue();
+ }
+ int code = hasNoResponseContent(message) ? HttpURLConnection.HTTP_ACCEPTED : HttpURLConnection.HTTP_OK;
+ // put the code in the message so that others can get it
+ message.put(Message.RESPONSE_CODE, code);
+ return code;
+ }
+
+ /**
+ * Determines if the current message has no response content.
+ * The message has no response content if either:
+ * - the request is oneway and the current message is no partial
+ * response or an empty partial response.
+ * - the request is not oneway but the current message is an empty partial
+ * response.
+ * @param message
+ * @return
+ */
+ public static boolean hasNoResponseContent(Message message) {
+ final boolean ow = isOneWay(message);
+ final boolean pr = MessageUtils.isPartialResponse(message);
+ final boolean epr = MessageUtils.isEmptyPartialResponse(message);
+
+ //REVISIT may need to provide an option to choose other behavior?
+ // old behavior not suppressing any responses => ow && !pr
+ // suppress empty responses for oneway calls => ow && (!pr || epr)
+ // suppress additionally empty responses for decoupled twoway calls =>
+ return (ow && !pr) || epr;
+ }
+
+ /**
+ * Checks if the message is oneway or not
+ * @param message the message under consideration
+ * @return true if the message has been marked as oneway
+ */
+ public static boolean isOneWay(Message message) {
+ final Exchange ex = message.getExchange();
+ return ex != null && ex.isOneWay();
+ }
+
}
diff --git a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/StandardTags.java b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/StandardTags.java
index 07247a5..f62d0ce 100644
--- a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/StandardTags.java
+++ b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/StandardTags.java
@@ -24,6 +24,7 @@ import java.util.Optional;
import org.apache.cxf.common.util.StringUtils;
import org.apache.cxf.message.Message;
+import org.apache.cxf.message.MessageUtils;
import io.micrometer.core.instrument.Tag;
@@ -65,10 +66,8 @@ public class StandardTags {
public Tag status(Message response) {
return ofNullable(response)
- .map(e -> e.get(Message.RESPONSE_CODE))
- .filter(e -> e instanceof Integer)
- .map(e -> (Integer) e)
- .map(String::valueOf)
+ .map(e -> MessageUtils.getReponseCodeFromMessage(response))
+ .map(code -> Integer.toString(code))
.map(status -> Tag.of("status", status))
.orElse(STATUS_UNKNOWN);
}
@@ -90,11 +89,8 @@ public class StandardTags {
}
public Tag outcome(Message response) {
- Optional<Integer> statusCode =
- ofNullable(response)
- .map(e -> e.get(Message.RESPONSE_CODE))
- .filter(e -> e instanceof Integer)
- .map(e -> (Integer) e);
+ Optional<Integer> statusCode = ofNullable(response)
+ .map(e -> MessageUtils.getReponseCodeFromMessage(response));
if (!statusCode.isPresent()) {
return OUTCOME_UNKNOWN;
}
diff --git a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProvider.java b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProvider.java
index 7b9f7f0..9e11c50 100644
--- a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProvider.java
+++ b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProvider.java
@@ -26,20 +26,17 @@ public class JaxwsFaultCodeProvider {
public String getFaultCode(Exchange ex, boolean client) {
FaultMode fm = ex.get(FaultMode.class);
+ // We check OutFaultMessage/InFaultMessage only because some features propagate the
+ // fault mode using InMessage/OutMessage (which may not end-up with a fault), for
+ // example check MAPAggregatorImpl.
if (client) {
if (fm == null && ex.getInFaultMessage() != null) {
fm = ex.getInFaultMessage().get(FaultMode.class);
}
- if (fm == null && ex.getOutMessage() != null) {
- fm = ex.getOutMessage().get(FaultMode.class);
- }
} else {
if (fm == null && ex.getOutFaultMessage() != null) {
fm = ex.getOutFaultMessage().get(FaultMode.class);
}
- if (fm == null && ex.getInMessage() != null) {
- fm = ex.getInMessage().get(FaultMode.class);
- }
}
if (fm == null) {
diff --git a/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/StandardTagsTest.java b/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/StandardTagsTest.java
index 92a13b3..c5e25ba 100644
--- a/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/StandardTagsTest.java
+++ b/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/StandardTagsTest.java
@@ -192,26 +192,27 @@ public class StandardTagsTest {
}
@Test
- public void testStatusReturnWithUnknownWhenStatusIsNull() {
+ public void testStatusReturnWith200WhenResponseCodeIsNotSet() {
// given
// when
Tag actual = underTest.status(response);
// then
- assertThat(actual, is(Tag.of(STATUS_METRIC_NAME, "UNKNOWN")));
+ assertThat(actual, is(Tag.of(STATUS_METRIC_NAME, "200")));
}
@Test
- public void testStatusReturnWithUnknownWhenResponseCodeIsNotInteger() {
+ public void testStatusReturnWith202WhenResponseCodeIsNullAndResponseIsPartial() {
// given
- doReturn("").when(request).get(Message.RESPONSE_CODE);
+ doReturn(null).when(request).get(Message.RESPONSE_CODE);
+ doReturn(true).when(request).get(Message.EMPTY_PARTIAL_RESPONSE_MESSAGE);
// when
Tag actual = underTest.status(request);
// then
- assertThat(actual, is(Tag.of(STATUS_METRIC_NAME, "UNKNOWN")));
+ assertThat(actual, is(Tag.of(STATUS_METRIC_NAME, "202")));
}
@Test
@@ -327,18 +328,6 @@ public class StandardTagsTest {
Tag actual = underTest.outcome(response);
// then
- assertThat(actual, is(Tag.of(OUTCOME_METRIC_NAME, "UNKNOWN")));
- }
-
- @Test
- public void testOutcomeReturnWithUnknownWhenMethodIsNotInteger() {
- // given
- doReturn(new Object()).when(response).get(Message.BASE_PATH);
-
- // when
- Tag actual = underTest.outcome(response);
-
- // then
- assertThat(actual, is(Tag.of(OUTCOME_METRIC_NAME, "UNKNOWN")));
+ assertThat(actual, is(Tag.of(OUTCOME_METRIC_NAME, "SUCCESS")));
}
}
diff --git a/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProviderTest.java b/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProviderTest.java
index 0dae669..326ab34 100644
--- a/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProviderTest.java
+++ b/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProviderTest.java
@@ -77,6 +77,21 @@ public class JaxwsFaultCodeProviderTest {
// then
assertThat(actual, equalTo(RUNTIME_FAULT_STRING));
}
+
+ @Test
+ public void testFaultModeIsNotPresentButInFaultModeIsPresentThenShouldReturnThat() {
+ // given
+ doReturn(null).when(ex).get(FaultMode.class);
+ doReturn(message).when(ex).getInFaultMessage();
+ doReturn(RUNTIME_FAULT).when(message).get(FaultMode.class);
+
+ // when
+ String actual = underTest.getFaultCode(ex, true);
+
+ // then
+ assertThat(actual, equalTo(RUNTIME_FAULT_STRING));
+ }
+
@Test
public void testFaultModeIsNotPresentButOutFaultModeIsMissingThenShouldReturnNull() {
@@ -92,7 +107,7 @@ public class JaxwsFaultCodeProviderTest {
}
@Test
- public void testNeitherFaultModeNorOutFaultModePresentsThenShouldReturnInMessagesFaultMode() {
+ public void testNeitherFaultModeNorOutFaultModePresentsThenShouldNotReturnInMessageFaultMode() {
// given
doReturn(null).when(ex).get(FaultMode.class);
doReturn(null).when(ex).getOutFaultMessage();
@@ -103,7 +118,22 @@ public class JaxwsFaultCodeProviderTest {
String actual = underTest.getFaultCode(ex, false);
// then
- assertThat(actual, equalTo(RUNTIME_FAULT_STRING));
+ assertThat(actual, is(nullValue()));
+ }
+
+ @Test
+ public void testNeitherFaultModeNorOutFaultModePresentsThenShouldNotReturnOutMessageFaultMode() {
+ // given
+ doReturn(null).when(ex).get(FaultMode.class);
+ doReturn(null).when(ex).getInFaultMessage();
+ doReturn(message).when(ex).getOutMessage();
+ doReturn(RUNTIME_FAULT).when(message).get(FaultMode.class);
+
+ // when
+ String actual = underTest.getFaultCode(ex, true);
+
+ // then
+ assertThat(actual, is(nullValue()));
}
@Test
diff --git a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/AbstractHTTPDestination.java b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/AbstractHTTPDestination.java
index 2beaf93..2be88b3 100644
--- a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/AbstractHTTPDestination.java
+++ b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/AbstractHTTPDestination.java
@@ -22,7 +22,6 @@ package org.apache.cxf.transport.http;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
-import java.net.HttpURLConnection;
import java.net.ServerSocket;
import java.net.URL;
import java.nio.charset.StandardCharsets;
@@ -235,8 +234,7 @@ public abstract class AbstractHTTPDestination
* @return true iff the message has been marked as oneway
*/
protected final boolean isOneWay(Message message) {
- Exchange ex = message.getExchange();
- return ex != null && ex.isOneWay();
+ return MessageUtils.isOneWay(message);
}
public void invoke(final ServletConfig config,
@@ -632,7 +630,7 @@ public abstract class AbstractHTTPDestination
HttpServletResponse response = getHttpResponseFromMessage(outMessage);
- int responseCode = getReponseCodeFromMessage(outMessage);
+ int responseCode = MessageUtils.getReponseCodeFromMessage(outMessage);
if (responseCode >= 300) {
String ec = (String)outMessage.get(Message.ERROR_MESSAGE);
if (!StringUtils.isEmpty(ec)) {
@@ -645,7 +643,7 @@ public abstract class AbstractHTTPDestination
outMessage.put(RESPONSE_HEADERS_COPIED, "true");
- if (hasNoResponseContent(outMessage)) {
+ if (MessageUtils.hasNoResponseContent(outMessage)) {
response.setContentLength(0);
response.flushBuffer();
closeResponseOutputStream(response);
@@ -669,38 +667,6 @@ public abstract class AbstractHTTPDestination
}
}
- private int getReponseCodeFromMessage(Message message) {
- Integer i = (Integer)message.get(Message.RESPONSE_CODE);
- if (i != null) {
- return i.intValue();
- }
- int code = hasNoResponseContent(message) ? HttpURLConnection.HTTP_ACCEPTED : HttpURLConnection.HTTP_OK;
- // put the code in the message so that others can get it
- message.put(Message.RESPONSE_CODE, code);
- return code;
- }
-
- /**
- * Determines if the current message has no response content.
- * The message has no response content if either:
- * - the request is oneway and the current message is no partial
- * response or an empty partial response.
- * - the request is not oneway but the current message is an empty partial
- * response.
- * @param message
- * @return
- */
- private boolean hasNoResponseContent(Message message) {
- final boolean ow = isOneWay(message);
- final boolean pr = MessageUtils.isPartialResponse(message);
- final boolean epr = MessageUtils.isEmptyPartialResponse(message);
-
- //REVISIT may need to provide an option to choose other behavior?
- // old behavior not suppressing any responses => ow && !pr
- // suppress empty responses for oneway calls => ow && (!pr || epr)
- // suppress additionally empty responses for decoupled twoway calls =>
- return (ow && !pr) || epr;
- }
private HttpServletResponse getHttpResponseFromMessage(Message message) throws IOException {
Object responseObj = message.get(HTTP_RESPONSE);