From 65884a9168561c7c6c3d01937178a0c54a2dda40 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 17 Nov 2020 12:51:11 +0100 Subject: [PATCH 1/3] Enhance trace api to set codes which are accepted as successful. This way requests can be traced as successful even if the response code is 4xx or 5xx. --- .../sonia/scm/net/ahc/BaseHttpRequest.java | 31 +++++++++++++++++++ .../net/ahc/DefaultAdvancedHttpClient.java | 10 +++++- .../ahc/DefaultAdvancedHttpClientTest.java | 13 +++++++- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/net/ahc/BaseHttpRequest.java b/scm-core/src/main/java/sonia/scm/net/ahc/BaseHttpRequest.java index 302f2ac6fe..5b0ebc453c 100644 --- a/scm-core/src/main/java/sonia/scm/net/ahc/BaseHttpRequest.java +++ b/scm-core/src/main/java/sonia/scm/net/ahc/BaseHttpRequest.java @@ -257,6 +257,22 @@ public abstract class BaseHttpRequest return self(); } + /** + * Sets the response codes which should be traced as successful. + * + * Example: If 400 is set as {@link #acceptedStatusCodes} then all requests + * which get a response with status code 400 will be traced as successful (not failed) request + * + * @param codes status codes which should be traced as successful + * @return request instance + * + * @since 2.10.0 + */ + public T acceptStatusCodes(int... codes) { + this.acceptedStatusCodes = codes; + return self(); + } + /** * Disables tracing for the request. * This should only be done for internal requests. @@ -314,6 +330,18 @@ public abstract class BaseHttpRequest return spanKind; } + + /** + * Returns the response codes which are accepted as successful by tracer. + * + * @return codes + * + * @since 2.10.0 + */ + public int[] getAcceptedStatus() { + return acceptedStatusCodes; + } + /** * Returns true if the request decodes gzip compression. * @@ -434,4 +462,7 @@ public abstract class BaseHttpRequest /** kind of span for trace api */ private String spanKind = "HTTP Request"; + + /** codes which will be marked as successful by tracer */ + private int[] acceptedStatusCodes = new int[]{}; } diff --git a/scm-webapp/src/main/java/sonia/scm/net/ahc/DefaultAdvancedHttpClient.java b/scm-webapp/src/main/java/sonia/scm/net/ahc/DefaultAdvancedHttpClient.java index ee3bdbb1e5..75acfc97d2 100644 --- a/scm-webapp/src/main/java/sonia/scm/net/ahc/DefaultAdvancedHttpClient.java +++ b/scm-webapp/src/main/java/sonia/scm/net/ahc/DefaultAdvancedHttpClient.java @@ -52,6 +52,7 @@ import java.io.OutputStream; import java.net.*; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; +import java.util.Arrays; import java.util.Set; //~--- JDK imports ------------------------------------------------------------ @@ -206,7 +207,7 @@ public class DefaultAdvancedHttpClient extends AdvancedHttpClient try { DefaultAdvancedHttpResponse response = doRequest(request); span.label("status", response.getStatus()); - if (!response.isSuccessful()) { + if (isFailedRequest(request, response)) { span.failed(); } return response; @@ -219,6 +220,13 @@ public class DefaultAdvancedHttpClient extends AdvancedHttpClient } } + private boolean isFailedRequest(BaseHttpRequest request, AdvancedHttpResponse responseStatus) { + if (Arrays.stream(request.getAcceptedStatus()).anyMatch(code -> code == responseStatus.getStatus())) { + return false; + } + return !responseStatus.isSuccessful(); + } + @Nonnull private DefaultAdvancedHttpResponse doRequest(BaseHttpRequest request) throws IOException { HttpURLConnection connection = openConnection(request, new URL(request.getUrl())); diff --git a/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java b/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java index 89004ece27..f1f09521b7 100644 --- a/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java +++ b/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java @@ -317,6 +317,17 @@ public class DefaultAdvancedHttpClientTest verify(tracer, never()).span(anyString()); } + @Test + public void shouldNotTraceRequestIfUntracedResponseCode() throws IOException { + when(connection.getResponseCode()).thenReturn(400); + + new AdvancedHttpRequest(client, HttpMethod.GET, "https://www.scm-manager.org").acceptStatusCodes(400).request(); + verify(tracer).span("HTTP Request"); + verify(span).label("status", 400); + verify(span, never()).failed(); + verify(span).close(); + } + //~--- set methods ---------------------------------------------------------- @@ -328,7 +339,7 @@ public class DefaultAdvancedHttpClientTest public void setUp() { configuration = new ScmConfiguration(); - transformers = new HashSet(); + transformers = new HashSet<>(); client = new TestingAdvacedHttpClient(configuration, transformers); when(tracer.span(anyString())).thenReturn(span); } From 0ccb502a632a43dac5ec4e0c4c1e60c3f4e21fda Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 17 Nov 2020 12:53:36 +0100 Subject: [PATCH 2/3] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dabacc9d8c..67671bac16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Delete branches directly in the UI ([#1422](https://github.com/scm-manager/scm-manager/pull/1422)) - Lookup command which provides further repository information ([#1415](https://github.com/scm-manager/scm-manager/pull/1415)) - Include messages from scm protocol in modification or merge errors ([#1420](https://github.com/scm-manager/scm-manager/pull/1420)) +- Enhance trace api to accepted status codes ([#1430](https://github.com/scm-manager/scm-manager/pull/1430)) ### Fixed - Missing close of hg diff command ([#1417](https://github.com/scm-manager/scm-manager/pull/1417)) From 4411e24ea73aac23060a15d8c415795a238c311d Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 17 Nov 2020 12:57:49 +0100 Subject: [PATCH 3/3] add test --- .../scm/net/ahc/DefaultAdvancedHttpClientTest.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java b/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java index f1f09521b7..d81c806471 100644 --- a/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java +++ b/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java @@ -318,7 +318,7 @@ public class DefaultAdvancedHttpClientTest } @Test - public void shouldNotTraceRequestIfUntracedResponseCode() throws IOException { + public void shouldNotTraceRequestIfAcceptedResponseCode() throws IOException { when(connection.getResponseCode()).thenReturn(400); new AdvancedHttpRequest(client, HttpMethod.GET, "https://www.scm-manager.org").acceptStatusCodes(400).request(); @@ -328,6 +328,17 @@ public class DefaultAdvancedHttpClientTest verify(span).close(); } + @Test + public void shouldTraceRequestAsFailedIfAcceptedResponseCodeDoesntMatch() throws IOException { + when(connection.getResponseCode()).thenReturn(401); + + new AdvancedHttpRequest(client, HttpMethod.GET, "https://www.scm-manager.org").acceptStatusCodes(400).request(); + verify(tracer).span("HTTP Request"); + verify(span).label("status", 401); + verify(span).failed(); + verify(span).close(); + } + //~--- set methods ----------------------------------------------------------