Skip to content

Commit 2fff3f3

Browse files
committed
common: increase robustness of access log file
Motivaition: The information about HTTP-based requests (WebDAV and REST) are only logged in the access log file when the request has been processed. Currently, this logging only happens if the request is handled normally; i.e., no exception was thrown and (for async requests) the processing didn't take too long. This means that a request that triggers certain failure modes would not have an entry in the access log file. Modification: Update existing `requestCompleted` method to accept a `NetLoggerBuilder` consumer. This consumer should add any mode-specific additional logging information. Create methods that handle the three modes in which the processing of a request might finish: `requestCompletedNormally`, `requestCompletedExceptionally`, `requestTimedOut`. All three modes add mode-specific information to the access log entry. The access log entry key-value pairs derived from the response are moved to the `requestCompletedNormally` method. The other modes will use default responses, rendering this information unreliable in those cases. The async listener `LogOnComplete` is updated so that `onTimeout` and `onError` event notification trigger `requestTimedOut` and `requestCompletedExceptionally` logging, respectively. The `NetLoggerBuilder` class is updated to accept key-value pairs where the value has type Duration. Result: Fix bug where webdav and frontend door access log files would fail to log requests that trigger certain failures in dCache. Target: master Requires-notes: yes Requires-book: no Request: 11.1 Request: 11.0 Request: 10.2 Request: 10.1 Request: 10.0 Request: 9.2 Closes: #7939 Patch: https://rb.dcache.org/r/14581/ Acked-by: Tigran Mkrtchyan
1 parent 361f98b commit 2fff3f3

File tree

2 files changed

+58
-13
lines changed

2 files changed

+58
-13
lines changed

modules/cells/src/main/java/org/dcache/util/NetLoggerBuilder.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.google.common.net.InetAddresses;
99
import java.net.InetSocketAddress;
1010
import java.security.Principal;
11+
import java.time.Duration;
1112
import java.time.ZonedDateTime;
1213
import java.time.format.DateTimeFormatter;
1314
import java.util.function.Function;
@@ -242,6 +243,10 @@ public NetLoggerBuilder add(String name, long value) {
242243
return add(name, String.valueOf(value));
243244
}
244245

246+
public NetLoggerBuilder add(String name, Duration d) {
247+
return add(name, TimeUtils.describe(d).orElse("0"));
248+
}
249+
245250
public NetLoggerBuilder add(String name, Exception e) {
246251
return add(name + ".class", e.getClass().getSimpleName())
247252
.add(name + ".message", e.getMessage());

modules/dcache/src/main/java/org/dcache/http/AbstractLoggingHandler.java

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
import java.net.InetAddress;
2727
import java.net.InetSocketAddress;
2828
import java.security.cert.X509Certificate;
29+
import java.time.Duration;
2930
import java.util.Optional;
31+
import java.util.function.Consumer;
3032
import javax.security.auth.Subject;
3133
import javax.servlet.AsyncEvent;
3234
import javax.servlet.AsyncListener;
@@ -43,6 +45,8 @@
4345
import org.eclipse.jetty.server.handler.HandlerWrapper;
4446
import org.slf4j.Logger;
4547
import org.slf4j.LoggerFactory;
48+
import static com.google.common.base.Throwables.throwIfInstanceOf;
49+
import static com.google.common.base.Throwables.throwIfUnchecked;
4650

4751
/**
4852
* This class act as a base logging class for logging servlet-based activity. It is expected that
@@ -61,25 +65,34 @@ private class LogOnComplete implements AsyncListener {
6165

6266
@Override
6367
public void onComplete(AsyncEvent event) {
64-
requestCompleted((HttpServletRequest) event.getSuppliedRequest(),
65-
(HttpServletResponse) event.getSuppliedResponse());
68+
requestCompletedNormally(
69+
(HttpServletRequest) event.getSuppliedRequest(),
70+
(HttpServletResponse) event.getSuppliedResponse());
6671
}
6772

6873
@Override
6974
public void onTimeout(AsyncEvent event) {
70-
LOGGER.warn("Unexpected timeout on async processing of {}",
71-
event.getAsyncContext().getRequest());
75+
var timeout = Duration.ofMillis(event.getAsyncContext().getTimeout());
76+
requestTimedOut(
77+
(HttpServletRequest) event.getSuppliedRequest(),
78+
(HttpServletResponse) event.getSuppliedResponse(),
79+
timeout);
7280
}
7381

7482
@Override
7583
public void onError(AsyncEvent event) {
76-
LOGGER.warn("Unexpected error on async processing of {}",
77-
event.getAsyncContext().getRequest());
84+
Throwable cause = event.getThrowable();
85+
if (cause instanceof Exception) {
86+
requestCompletedExceptionally(
87+
(HttpServletRequest) event.getSuppliedRequest(),
88+
(HttpServletResponse) event.getSuppliedResponse(),
89+
(Exception) cause);
90+
}
7891
}
7992

8093
@Override
8194
public void onStartAsync(AsyncEvent event) {
82-
LOGGER.warn("Unexpected error on async processing of {}",
95+
LOGGER.warn("Unexpected reuse of async processing context for {}",
8396
event.getAsyncContext().getRequest());
8497
}
8598
}
@@ -107,17 +120,46 @@ public void handle(String target, Request baseRequest,
107120
request.setAttribute(REMOTE_ADDRESS, remoteAddress(request).orElse(null));
108121
request.setAttribute(PROCESSING_TIME, processingTime);
109122

110-
super.handle(target, baseRequest, request, response);
123+
try {
124+
super.handle(target, baseRequest, request, response);
125+
} catch (IOException|RuntimeException|ServletException e) {
126+
requestCompletedExceptionally(request, response, e);
127+
128+
throwIfInstanceOf(e, IOException.class);
129+
throwIfInstanceOf(e, ServletException.class);
130+
throwIfUnchecked(e);
131+
throw new AssertionError(e);
132+
}
111133

112134
if (request.isAsyncStarted()) {
113135
request.getAsyncContext().addListener(new LogOnComplete());
114136
} else {
115-
requestCompleted(request, response);
137+
requestCompletedNormally(request, response);
116138
}
117139
}
118140
}
119141

120-
private void requestCompleted(HttpServletRequest request, HttpServletResponse response) {
142+
private void requestCompletedExceptionally(HttpServletRequest request, HttpServletResponse response, Exception e) {
143+
requestCompleted(request, response, l -> {
144+
l.add("failure", e);
145+
});
146+
}
147+
148+
private void requestTimedOut(HttpServletRequest request, HttpServletResponse response, Duration timeout) {
149+
requestCompleted(request, response, l -> {
150+
l.add("timeout", timeout);
151+
});
152+
}
153+
154+
private void requestCompletedNormally(HttpServletRequest request, HttpServletResponse response) {
155+
requestCompleted(request, response, l -> {
156+
l.add("response.reason", getReason(response));
157+
l.add("response.code", response.getStatus());
158+
l.add("location", response.getHeader("Location"));
159+
});
160+
}
161+
162+
private void requestCompleted(HttpServletRequest request, HttpServletResponse response, Consumer<NetLoggerBuilder> logEnricher) {
121163
var processingTime = (Stopwatch) request.getAttribute(PROCESSING_TIME);
122164

123165
processingTime.stop();
@@ -127,6 +169,7 @@ private void requestCompleted(HttpServletRequest request, HttpServletResponse re
127169
.omitNullValues();
128170
describeOperation(log, request, response);
129171
log.add("duration", processingTime.elapsed().toMillis());
172+
logEnricher.accept(log);
130173
log.toLogger(accessLogger());
131174
}
132175

@@ -135,9 +178,6 @@ protected void describeOperation(NetLoggerBuilder log,
135178
log.add("session", CDC.getSession());
136179
log.add("request.method", request.getMethod());
137180
log.add("request.url", request.getRequestURL());
138-
log.add("response.code", response.getStatus());
139-
log.add("response.reason", getReason(response));
140-
log.add("location", response.getHeader("Location"));
141181
log.add("socket.remote", (InetSocketAddress) request.getAttribute(REMOTE_ADDRESS));
142182
log.add("user-agent", request.getHeader("User-Agent"));
143183

0 commit comments

Comments
 (0)