Skip to content

Commit 0616dbe

Browse files
daniel-rusnakdaniel-rusnak
committed
#621 - add the logging of requestId (port to 2.35.*)
Added 'RequestIdInterceptor' on the apache http client to generate 'requestId' for the client requests. It wasn't added to Spring 'RestTemplate' because the Sardine client doesn't use it. Added 'ResponseMissingRequestIdInterceptor' on the apache http client to add missing 'X-GDC-REQUEST' header in responses coming from WebDav server. Added the 'requestId' information to exceptions to easier tracking the problem.
1 parent f8f61e1 commit 0616dbe

8 files changed

Lines changed: 229 additions & 8 deletions

File tree

‎src/main/java/com/gooddata/GoodData.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,8 @@ private HttpClientBuilder createHttpClientBuilder(final GoodDataSettings setting
288288

289289
return HttpClientBuilder.create()
290290
.setUserAgent(StringUtils.isNotBlank(settings.getUserAgent()) ? String.format("%s %s", settings.getUserAgent(), getUserAgent()) : getUserAgent())
291+
.addInterceptorFirst(new RequestIdInterceptor())
292+
.addInterceptorFirst(new ResponseMissingRequestIdInterceptor())
291293
.setConnectionManager(connectionManager)
292294
.setDefaultRequestConfig(requestConfig.build());
293295
}

‎src/main/java/com/gooddata/RequestIdInterceptor.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright (C) 2004-2021, GoodData(R) Corporation. All rights reserved.
3+
* This source code is licensed under the BSD-style license found in the
4+
* LICENSE.txt file in the root directory of this source tree.
5+
*/
6+
package com.gooddata;
7+
8+
import org.apache.commons.lang3.RandomStringUtils;
9+
import org.apache.http.Header;
10+
import org.apache.http.HttpException;
11+
import org.apache.http.HttpRequest;
12+
import org.apache.http.HttpRequestInterceptor;
13+
import org.apache.http.annotation.Contract;
14+
import org.apache.http.annotation.ThreadingBehavior;
15+
import org.apache.http.protocol.HttpContext;
16+
17+
import java.io.IOException;
18+
19+
import static com.gooddata.gdc.Header.GDC_REQUEST_ID;
20+
21+
/**
22+
* Intercepts the client-side requests on low-level in order to be able to catch requests also from the Sardine,
23+
* that is working independently from Spring {@link org.springframework.web.client.RestTemplate} to set
24+
* the X-GDC-REQUEST header to them.
25+
*/
26+
@Contract(threading = ThreadingBehavior.IMMUTABLE)
27+
public class RequestIdInterceptor implements HttpRequestInterceptor {
28+
29+
@Override
30+
public void process(final HttpRequest request, final HttpContext context) throws HttpException, IOException {
31+
final StringBuilder requestIdBuilder = new StringBuilder();
32+
final Header requestIdHeader = request.getFirstHeader(GDC_REQUEST_ID);
33+
if (requestIdHeader != null) {
34+
requestIdBuilder.append(requestIdHeader.getValue()).append(":");
35+
}
36+
String requestId = requestIdBuilder.append(RandomStringUtils.randomAlphanumeric(16)).toString();
37+
request.setHeader(GDC_REQUEST_ID, requestId);
38+
}
39+
}

‎src/main/java/com/gooddata/ResponseMissingRequestIdInterceptor.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright (C) 2004-2021, GoodData(R) Corporation. All rights reserved.
3+
* This source code is licensed under the BSD-style license found in the
4+
* LICENSE.txt file in the root directory of this source tree.
5+
*/
6+
package com.gooddata;
7+
8+
import org.apache.http.Header;
9+
import org.apache.http.HttpException;
10+
import org.apache.http.HttpResponse;
11+
import org.apache.http.HttpResponseInterceptor;
12+
import org.apache.http.annotation.Contract;
13+
import org.apache.http.annotation.ThreadingBehavior;
14+
import org.apache.http.protocol.HttpContext;
15+
import org.apache.http.protocol.HttpCoreContext;
16+
17+
import java.io.IOException;
18+
19+
import static com.gooddata.gdc.Header.GDC_REQUEST_ID;
20+
21+
22+
/**
23+
* Intercepts responses to check if they have set the X-GDC-REQUEST header for easier debugging.
24+
* If not, it takes this header from the request sent.
25+
*/
26+
@Contract(threading = ThreadingBehavior.IMMUTABLE)
27+
public class ResponseMissingRequestIdInterceptor implements HttpResponseInterceptor {
28+
29+
@Override
30+
public void process(final HttpResponse response, final HttpContext context) throws HttpException, IOException {
31+
32+
if (response.getFirstHeader(GDC_REQUEST_ID) == null) {
33+
HttpCoreContext coreContext = HttpCoreContext.adapt(context);
34+
Header requestIdHeader = coreContext.getRequest().getFirstHeader(GDC_REQUEST_ID);
35+
response.setHeader(GDC_REQUEST_ID, requestIdHeader.getValue());
36+
}
37+
}
38+
}

‎src/main/java/com/gooddata/gdc/DataStoreService.java‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
import com.github.sardine.Sardine;
99
import com.github.sardine.impl.SardineException;
10+
import com.github.sardine.impl.SardineImpl;
11+
import com.gooddata.GoodDataRestException;
1012
import com.gooddata.UriPrefixer;
1113
import org.apache.http.Header;
1214
import org.apache.http.HeaderIterator;
@@ -24,19 +26,23 @@
2426
import org.apache.http.client.methods.CloseableHttpResponse;
2527
import org.apache.http.client.methods.HttpUriRequest;
2628
import org.apache.http.conn.ClientConnectionManager;
29+
import org.apache.http.entity.InputStreamEntity;
2730
import org.apache.http.impl.client.CloseableHttpClient;
2831
import org.apache.http.impl.client.HttpClientBuilder;
32+
import org.apache.http.message.BasicHeader;
2933
import org.apache.http.params.HttpParams;
34+
import org.apache.http.protocol.HTTP;
3035
import org.apache.http.protocol.HttpContext;
3136
import org.springframework.http.HttpMethod;
3237
import org.springframework.http.HttpStatus;
3338
import org.springframework.http.ResponseEntity;
34-
import org.springframework.web.client.RestClientException;
3539
import org.springframework.web.client.RestTemplate;
3640

3741
import java.io.IOException;
3842
import java.io.InputStream;
3943
import java.net.URI;
44+
import java.util.Collections;
45+
import java.util.List;
4046
import java.util.Locale;
4147

4248
import static com.gooddata.util.Validate.notEmpty;
@@ -102,7 +108,13 @@ public void upload(String path, InputStream stream) {
102108

103109
private void upload(URI url, InputStream stream) {
104110
try {
105-
sardine.put(url.toString(), stream);
111+
// The variable name means that data size in bytes to set to Content-Length header.
112+
// Taken from {@link Sardine#put(String, InputStream, String, boolean, long)}
113+
final int contentLength = -1;
114+
// We need to use it this way, if we want to track request_id in the stacktrace.
115+
InputStreamEntity entity = new InputStreamEntity(stream, contentLength);
116+
List<Header> headers = Collections.singletonList(new BasicHeader(HTTP.EXPECT_DIRECTIVE, HTTP.EXPECT_CONTINUE));
117+
((SardineImpl) sardine).put(url.toString(), entity, headers, new GdcSardineResponseHandler());
106118
} catch (SardineException e) {
107119
if (HttpStatus.INTERNAL_SERVER_ERROR.value() == e.getStatusCode()) {
108120
// this error may occur when user issues request to WebDAV before SST and TT were obtained
@@ -144,7 +156,7 @@ public InputStream download(String path) {
144156
notEmpty(path, "path");
145157
final URI uri = getUri(path);
146158
try {
147-
return sardine.get(uri.toString());
159+
return ((GdcSardine)sardine).get(uri.toString(), Collections.emptyList(), new GdcSardineResponseHandler());
148160
} catch (IOException e) {
149161
throw new DataStoreException("Unable to download from " + uri, e);
150162
}
@@ -165,7 +177,7 @@ public void delete(String path) {
165177
if (HttpStatus.MOVED_PERMANENTLY.equals(result.getStatusCode())) {
166178
restTemplate.exchange(result.getHeaders().getLocation(), HttpMethod.DELETE, org.springframework.http.HttpEntity.EMPTY, Void.class);
167179
}
168-
} catch (RestClientException e) {
180+
} catch (GoodDataRestException e) {
169181
throw new DataStoreException("Unable to delete " + uri, e);
170182
}
171183
}

‎src/main/java/com/gooddata/gdc/GdcSardine.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,18 @@
88
import static com.gooddata.util.Validate.notNull;
99

1010
import com.github.sardine.impl.SardineImpl;
11+
import com.github.sardine.impl.io.ContentLengthInputStream;
12+
import com.github.sardine.impl.io.HttpMethodReleaseInputStream;
13+
import org.apache.http.Header;
14+
import org.apache.http.HttpResponse;
1115
import org.apache.http.client.ResponseHandler;
16+
import org.apache.http.client.methods.HttpGet;
1217
import org.apache.http.client.methods.HttpRequestBase;
1318
import org.apache.http.impl.client.HttpClientBuilder;
1419

1520
import java.io.IOException;
21+
import java.util.List;
22+
import java.util.Map;
1623

1724
/**
1825
* This class extends SardineImpl, connections were not correctly closed by parent
@@ -35,4 +42,36 @@ protected <T> T execute(HttpRequestBase request, ResponseHandler<T> responseHand
3542
request.releaseConnection();
3643
}
3744
}
45+
46+
/**
47+
* The method body is retrieved from {@link SardineImpl#get(String, Map)} and extended about the response handler
48+
* to be able to handle responses arbitrarily.
49+
* @param url Path to the resource including protocol and hostname
50+
* @param headers Additional HTTP headers to add to the request
51+
* @param responseHandler Arbitrary response handler to manipulate with responses
52+
* @return Data stream to read from
53+
* @throws IOException I/O error or HTTP response validation failure
54+
*/
55+
public <T> ContentLengthInputStream get(String url, List<Header> headers, ResponseHandler<T> responseHandler) throws IOException {
56+
57+
HttpGet get = new HttpGet(url);
58+
for (Header header : headers)
59+
{
60+
get.addHeader(header);
61+
}
62+
// Must use #execute without handler, otherwise the entity is consumed
63+
// already after the handler exits.
64+
HttpResponse response = this.execute(get);
65+
try
66+
{
67+
responseHandler.handleResponse(response);
68+
// Will abort the read when closed before EOF.
69+
return new ContentLengthInputStream(new HttpMethodReleaseInputStream(response), response.getEntity().getContentLength());
70+
}
71+
catch (IOException ex)
72+
{
73+
get.abort();
74+
throw ex;
75+
}
76+
}
3877
}

‎src/main/java/com/gooddata/gdc/GdcSardineException.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright (C) 2007-2021, GoodData(R) Corporation. All rights reserved.
3+
*/
4+
package com.gooddata.gdc;
5+
6+
import com.github.sardine.impl.SardineException;
7+
8+
/**
9+
* Extended Sardine exception about X-GDC-REQUEST header value.
10+
*/
11+
public class GdcSardineException extends SardineException {
12+
13+
private final String requestId;
14+
15+
/**
16+
* @param msg Custom description of failure
17+
* @param statusCode Error code returned by server
18+
* @param responsePhrase Response phrase following the error code
19+
* @param requestId The X-GDC-REQUEST identifier.
20+
*/
21+
public GdcSardineException(String requestId, String msg, int statusCode, String responsePhrase) {
22+
super(msg, statusCode, responsePhrase);
23+
this.requestId = requestId;
24+
}
25+
26+
@Override
27+
public String getMessage() {
28+
return String.format("[request_id=%s]: %s (%d %s)", this.requestId, super.getMessage(), this.getStatusCode(),
29+
this.getResponsePhrase()
30+
);
31+
}
32+
33+
}

‎src/main/java/com/gooddata/gdc/GdcSardineResponseHandler.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright (C) 2004-2021, GoodData(R) Corporation. All rights reserved.
3+
* This source code is licensed under the BSD-style license found in the
4+
* LICENSE.txt file in the root directory of this source tree.
5+
*/
6+
package com.gooddata.gdc;
7+
8+
import com.github.sardine.impl.SardineException;
9+
import com.gooddata.gdc.GdcSardineException;
10+
import org.apache.http.Header;
11+
import org.apache.http.HttpResponse;
12+
import org.apache.http.HttpStatus;
13+
import org.apache.http.StatusLine;
14+
import org.apache.http.client.ResponseHandler;
15+
16+
import java.io.IOException;
17+
18+
import static com.gooddata.gdc.Header.GDC_REQUEST_ID;
19+
20+
21+
/**
22+
* A basic validation response handler that extends the functionality of {@link com.github.sardine.impl.handler.ValidatingResponseHandler}
23+
* about the addition of X-GDC-REQUEST header to exception.
24+
*/
25+
class GdcSardineResponseHandler implements ResponseHandler<Void> {
26+
27+
@Override
28+
public Void handleResponse(HttpResponse response) throws IOException {
29+
StatusLine statusLine = response.getStatusLine();
30+
int statusCode = statusLine.getStatusCode();
31+
if (statusCode >= HttpStatus.SC_OK && statusCode < HttpStatus.SC_MULTIPLE_CHOICES)
32+
{
33+
return null;
34+
}
35+
Header requestIdHeader = response.getFirstHeader(GDC_REQUEST_ID);
36+
if (requestIdHeader != null) {
37+
throw new GdcSardineException(requestIdHeader.getValue(), "Unexpected response", statusLine.getStatusCode(),
38+
statusLine.getReasonPhrase()
39+
);
40+
} else {
41+
throw new SardineException("Unexpected response", statusLine.getStatusCode(), statusLine.getReasonPhrase());
42+
}
43+
}
44+
}

‎src/test/java/com/gooddata/gdc/DatastoreServiceAT.java‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@
1818
import java.util.UUID;
1919

2020
import static com.gooddata.util.ResourceUtils.readFromResource;
21-
import static org.hamcrest.CoreMatchers.equalTo;
21+
import static org.hamcrest.CoreMatchers.containsString;
22+
import static org.hamcrest.CoreMatchers.instanceOf;
2223
import static org.hamcrest.CoreMatchers.is;
24+
import static org.hamcrest.CoreMatchers.equalTo;
2325
import static org.hamcrest.MatcherAssert.assertThat;
24-
import static org.testng.Assert.assertEquals;
2526
import static org.testng.Assert.fail;
2627

2728
/**
@@ -60,6 +61,17 @@ public void datastoreDownload() throws Exception {
6061
}
6162

6263
@Test(groups = "datastore", dependsOnMethods = "datastoreDownload")
64+
public void verifyRequestIdInException() throws Exception {
65+
DataStoreService dataStoreService = AbstractGoodDataAT.gd.getDataStoreService();
66+
67+
try (InputStream ignored = dataStoreService.download(this.directory + "/none.txt")) {
68+
fail("The exception should contain the request_id in its stacktrace.");
69+
} catch (Exception e){
70+
assertThat(e.getCause().toString(), containsString("request_id"));
71+
}
72+
}
73+
74+
@Test(groups = "datastore", dependsOnMethods = "verifyRequestIdInException")
6375
public void datastoreDelete() throws Exception {
6476
DataStoreService dataStoreService = gd.getDataStoreService();
6577
dataStoreService.delete(this.file);
@@ -68,8 +80,10 @@ public void datastoreDelete() throws Exception {
6880
try {
6981
dataStoreService.delete(this.directory);
7082
fail("Exception was expected, as there is nothing to delete");
71-
} catch (GoodDataRestException e) {
72-
assertEquals(404, e.getStatusCode());
83+
} catch (DataStoreException e) {
84+
assertThat(e.getCause().toString(), containsString("request_id"));
85+
assertThat(e.getCause().toString(), containsString("404"));
86+
assertThat(e.getCause(), instanceOf(GoodDataRestException.class));
7387
}
7488
}
7589

0 commit comments

Comments
 (0)