You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2020/04/02 04:32:22 UTC
[james-project] 15/15: JAMES-3078 MDC hierarchical MDC context for
reactor
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit f08f82f9c3b7f30c4df824ba3b7be46fbeb9c5ae
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu Mar 26 12:25:29 2020 +0700
JAMES-3078 MDC hierarchical MDC context for reactor
---
.../james/imap/processor/UnselectProcessor.java | 3 +-
.../java/org/apache/james/util/MDCBuilder.java | 5 ++
.../java/org/apache/james/util/ReactorUtils.java | 50 +++++++++++++++++++
.../org/apache/james/util/ReactorUtilsTest.java | 50 +++++++++++++++++++
.../draft/methods/GetVacationResponseMethod.java | 5 +-
.../draft/methods/SetVacationResponseMethod.java | 8 ++-
.../james/jmap/http/AuthenticationRoutes.java | 57 ++++++++++++++--------
.../org/apache/james/jmap/http/DownloadRoutes.java | 22 +++++++--
.../org/apache/james/jmap/http/JMAPApiRoutes.java | 16 +++---
.../org/apache/james/jmap/http/LoggingHelper.java | 46 +++++++++++++++++
.../org/apache/james/jmap/http/UploadRoutes.java | 12 ++++-
.../apache/james/jmap/http/JMAPApiRoutesTest.java | 9 ++--
.../java/org/apache/james/jmap/JMAPRoutes.java | 1 -
13 files changed, 243 insertions(+), 41 deletions(-)
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/UnselectProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/UnselectProcessor.java
index 64176c1..f6fa95a 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/UnselectProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/UnselectProcessor.java
@@ -66,8 +66,7 @@ public class UnselectProcessor extends AbstractMailboxProcessor<UnselectRequest>
@Override
protected Closeable addContextToMDC(UnselectRequest request) {
- return MDCBuilder.create()
- .addContext(MDCBuilder.ACTION, "UNSELECT")
+ return MDCBuilder.of(MDCBuilder.ACTION, "UNSELECT")
.build();
}
}
diff --git a/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java b/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java
index bac3b4b..9bf6c5b 100644
--- a/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java
+++ b/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java
@@ -98,6 +98,11 @@ public class MDCBuilder {
return new MDCBuilder();
}
+ public static MDCBuilder of(String key, Object value) {
+ return create()
+ .addContext(key, value);
+ }
+
private final ImmutableMap.Builder<String, String> contextMap = ImmutableMap.builder();
private final ImmutableList.Builder<MDCBuilder> nestedBuilder = ImmutableList.builder();
diff --git a/server/container/util/src/main/java/org/apache/james/util/ReactorUtils.java b/server/container/util/src/main/java/org/apache/james/util/ReactorUtils.java
index a87ab17..df9c937 100644
--- a/server/container/util/src/main/java/org/apache/james/util/ReactorUtils.java
+++ b/server/container/util/src/main/java/org/apache/james/util/ReactorUtils.java
@@ -18,16 +18,23 @@
****************************************************************/
package org.apache.james.util;
+import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.util.Iterator;
import java.util.Optional;
+import java.util.function.Consumer;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
+import reactor.core.publisher.Signal;
+import reactor.util.context.Context;
public class ReactorUtils {
+
+ public static final String MDC_KEY_PREFIX = "MDC-";
+
public static <T> Mono<T> executeAndEmpty(Runnable runnable) {
return Mono.fromRunnable(runnable).then(Mono.empty());
}
@@ -100,4 +107,47 @@ public class ReactorUtils {
private static int byteToInt(ByteBuffer buffer) {
return buffer.get() & 0xff;
}
+
+ public static Consumer<Signal<?>> logOnError(Consumer<Throwable> errorLogStatement) {
+ return signal -> {
+ if (!signal.isOnError()) {
+ return;
+ }
+ try {
+ try (Closeable mdc = retrieveMDCBuilder(signal).build()) {
+ errorLogStatement.accept(signal.getThrowable());
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ };
+ }
+
+ public static Consumer<Signal<?>> log(Runnable logStatement) {
+ return signal -> {
+ try (Closeable mdc = retrieveMDCBuilder(signal).build()) {
+ logStatement.run();
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ };
+ }
+
+ public static Context context(String keySuffix, MDCBuilder mdcBuilder) {
+ return Context.of(mdcKey(keySuffix), mdcBuilder);
+ }
+
+ private static String mdcKey(String value) {
+ return MDC_KEY_PREFIX + value;
+ }
+
+ private static MDCBuilder retrieveMDCBuilder(Signal<?> signal) {
+ return signal.getContext().stream()
+ .filter(entry -> entry.getKey() instanceof String)
+ .filter(entry -> entry.getValue() instanceof MDCBuilder)
+ .filter(entry -> ((String) entry.getKey()).startsWith(MDC_KEY_PREFIX))
+ .map(entry -> (MDCBuilder) entry.getValue())
+ .reduce(MDCBuilder.create(), MDCBuilder::addContext);
+ }
+
}
diff --git a/server/container/util/src/test/java/org/apache/james/util/ReactorUtilsTest.java b/server/container/util/src/test/java/org/apache/james/util/ReactorUtilsTest.java
index 06edbc5..1ef5184 100644
--- a/server/container/util/src/test/java/org/apache/james/util/ReactorUtilsTest.java
+++ b/server/container/util/src/test/java/org/apache/james/util/ReactorUtilsTest.java
@@ -27,11 +27,13 @@ import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.IntStream;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.RandomStringUtils;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
+import org.slf4j.MDC;
import com.google.common.collect.ImmutableList;
import com.google.common.primitives.Bytes;
@@ -280,4 +282,52 @@ class ReactorUtilsTest {
assertThat(chunks).isEqualTo(expected);
}
}
+
+ @Nested
+ class MDCTest {
+ @Test
+ void contextShouldEnhanceMDC() {
+ String value = "value";
+ String key = "key";
+
+ Flux.just(1)
+ .doOnEach(ReactorUtils.log(() -> {
+ assertThat(MDC.get(key)).isEqualTo(value);
+ }))
+ .subscriberContext(ReactorUtils.context("test", MDCBuilder.of(key, value)))
+ .blockLast();
+ }
+
+ @Test
+ void contextShouldNotOverwritePreviousKeys() {
+ String value1 = "value1";
+ String value2 = "value2";
+ String key = "key";
+
+ Flux.just(1)
+ .doOnEach(ReactorUtils.log(() -> {
+ assertThat(MDC.get(key)).isEqualTo(value1);
+ }))
+ .subscriberContext(ReactorUtils.context("test", MDCBuilder.of(key, value1)))
+ .subscriberContext(ReactorUtils.context("test", MDCBuilder.of(key, value2)))
+ .blockLast();
+ }
+
+ @Test
+ void contextShouldCombineMDCs() {
+ String value1 = "value1";
+ String value2 = "value2";
+ String key1 = "key1";
+ String key2 = "key2";
+
+ Flux.just(1)
+ .doOnEach(ReactorUtils.log(() -> {
+ assertThat(MDC.get(key1)).isEqualTo(value1);
+ assertThat(MDC.get(key2)).isEqualTo(value2);
+ }))
+ .subscriberContext(ReactorUtils.context("test1", MDCBuilder.of(key1, value1)))
+ .subscriberContext(ReactorUtils.context("test2", MDCBuilder.of(key2, value2)))
+ .blockLast();
+ }
+ }
}
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetVacationResponseMethod.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetVacationResponseMethod.java
index 1b11bd3..da38ebd 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetVacationResponseMethod.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetVacationResponseMethod.java
@@ -19,6 +19,8 @@
package org.apache.james.jmap.draft.methods;
+import static org.apache.james.jmap.http.LoggingHelper.jmapAction;
+
import javax.inject.Inject;
import org.apache.james.jmap.api.vacation.AccountId;
@@ -76,7 +78,8 @@ public class GetVacationResponseMethod implements Method {
.methodCallId(methodCallId)
.responseName(RESPONSE_NAME)
.response(response)
- .build()))));
+ .build()))))
+ .subscriberContext(jmapAction("VACATION"));
}
private Mono<GetVacationResponse> process(MailboxSession mailboxSession) {
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetVacationResponseMethod.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetVacationResponseMethod.java
index d8952b8..4da3a22 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetVacationResponseMethod.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetVacationResponseMethod.java
@@ -19,6 +19,9 @@
package org.apache.james.jmap.draft.methods;
+import static org.apache.james.jmap.http.LoggingHelper.jmapAction;
+import static org.apache.james.util.ReactorUtils.context;
+
import javax.inject.Inject;
import org.apache.james.jmap.api.vacation.AccountId;
@@ -32,6 +35,7 @@ import org.apache.james.jmap.draft.model.SetVacationResponse;
import org.apache.james.jmap.draft.model.VacationResponse;
import org.apache.james.mailbox.MailboxSession;
import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.util.MDCBuilder;
import com.google.common.base.Preconditions;
@@ -77,7 +81,9 @@ public class SetVacationResponseMethod implements Method {
SetVacationRequest setVacationRequest = (SetVacationRequest) request;
return metricFactory.runPublishingTimerMetricLogP99(JMAP_PREFIX + METHOD_NAME.getName(),
- () -> process(methodCallId, mailboxSession, setVacationRequest));
+ () -> process(methodCallId, mailboxSession, setVacationRequest)
+ .subscriberContext(jmapAction("SET_VACATION"))
+ .subscriberContext(context("set-vacation", MDCBuilder.of("update", setVacationRequest.getUpdate()))));
}
private Flux<JmapResponse> process(MethodCallId methodCallId, MailboxSession mailboxSession, SetVacationRequest setVacationRequest) {
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java
index 5470969..db645dd 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java
@@ -29,6 +29,11 @@ import static io.netty.handler.codec.http.HttpResponseStatus.UNAUTHORIZED;
import static org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE;
import static org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE_UTF8;
import static org.apache.james.jmap.http.JMAPUrls.AUTHENTICATION;
+import static org.apache.james.jmap.http.LoggingHelper.jmapAction;
+import static org.apache.james.jmap.http.LoggingHelper.jmapAuthContext;
+import static org.apache.james.jmap.http.LoggingHelper.jmapContext;
+import static org.apache.james.util.ReactorUtils.log;
+import static org.apache.james.util.ReactorUtils.logOnError;
import java.io.IOException;
import java.util.Objects;
@@ -118,29 +123,40 @@ public class AuthenticationRoutes implements JMAPRoutes {
} else {
throw new RuntimeException(objectRequest.getClass() + " " + objectRequest);
}
- })
- .onErrorResume(BadRequestException.class, e -> handleBadRequest(response, e))
- .onErrorResume(e -> handleInternalError(response, e))))
+ })))
+ .onErrorResume(BadRequestException.class, e -> handleBadRequest(response, e))
+ .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e)))
+ .onErrorResume(e -> handleInternalError(response, e))
+ .subscriberContext(jmapContext(request))
+ .subscriberContext(jmapAction("auth-post"))
.subscribeOn(Schedulers.elastic());
}
private Mono<Void> returnEndPointsResponse(HttpServerRequest req, HttpServerResponse resp) {
- try {
return authenticator.authenticate(req)
- .then(resp.status(OK)
- .header(CONTENT_TYPE, JSON_CONTENT_TYPE_UTF8)
- .sendString(Mono.just(mapper.writeValueAsString(EndPointsResponse
- .builder()
- .api(JMAPUrls.JMAP)
- .eventSource(JMAPUrls.NOT_IMPLEMENTED)
- .upload(JMAPUrls.UPLOAD)
- .download(JMAPUrls.DOWNLOAD)
- .build())))
- .then())
+ .flatMap(session -> returnEndPointsResponse(resp)
+ .subscriberContext(jmapAuthContext(session)))
.onErrorResume(BadRequestException.class, e -> handleBadRequest(resp, e))
+ .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e)))
.onErrorResume(InternalErrorException.class, e -> handleInternalError(resp, e))
.onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(resp, e))
+ .subscriberContext(jmapContext(req))
+ .subscriberContext(jmapAction("returnEndPoints"))
.subscribeOn(Schedulers.elastic());
+ }
+
+ private Mono<Void> returnEndPointsResponse(HttpServerResponse resp) {
+ try {
+ return resp.status(OK)
+ .header(CONTENT_TYPE, JSON_CONTENT_TYPE_UTF8)
+ .sendString(Mono.just(mapper.writeValueAsString(EndPointsResponse
+ .builder()
+ .api(JMAPUrls.JMAP)
+ .eventSource(JMAPUrls.NOT_IMPLEMENTED)
+ .upload(JMAPUrls.UPLOAD)
+ .download(JMAPUrls.DOWNLOAD)
+ .build())))
+ .then();
} catch (JsonProcessingException e) {
throw new InternalErrorException("Error serializing endpoint response", e);
}
@@ -151,8 +167,11 @@ public class AuthenticationRoutes implements JMAPRoutes {
return authenticator.authenticate(req)
.flatMap(session -> Mono.from(accessTokenManager.revoke(AccessToken.fromString(authorizationHeader)))
- .then(resp.status(NO_CONTENT).send().then()))
+ .then(resp.status(NO_CONTENT).send().then())
+ .subscriberContext(jmapAuthContext(session)))
.onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(resp, e))
+ .subscriberContext(jmapContext(req))
+ .subscriberContext(jmapAction("auth-delete"))
.subscribeOn(Schedulers.elastic());
}
@@ -204,8 +223,8 @@ public class AuthenticationRoutes implements JMAPRoutes {
case EXPIRED:
return returnForbiddenAuthentication(resp);
case INVALID:
- LOGGER.warn("Use of an invalid ContinuationToken : {}", request.getToken().serialize());
- return returnUnauthorizedResponse(resp);
+ return returnUnauthorizedResponse(resp)
+ .doOnEach(log(() -> LOGGER.warn("Use of an invalid ContinuationToken : {}", request.getToken().serialize())));
case OK:
return manageAuthenticationResponse(request, resp);
default:
@@ -221,8 +240,8 @@ public class AuthenticationRoutes implements JMAPRoutes {
if (success) {
return returnAccessTokenResponse(resp, username);
} else {
- LOGGER.info("Authentication failure for {}", username);
- return returnUnauthorizedResponse(resp);
+ return returnUnauthorizedResponse(resp)
+ .doOnEach(log(() -> LOGGER.info("Authentication failure for {}", username)));
}
});
}
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java
index d639d16..57b99bd 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java
@@ -23,6 +23,10 @@ import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND;
import static io.netty.handler.codec.http.HttpResponseStatus.OK;
import static org.apache.james.jmap.HttpConstants.TEXT_PLAIN_CONTENT_TYPE;
import static org.apache.james.jmap.http.JMAPUrls.DOWNLOAD;
+import static org.apache.james.jmap.http.LoggingHelper.jmapAction;
+import static org.apache.james.jmap.http.LoggingHelper.jmapAuthContext;
+import static org.apache.james.jmap.http.LoggingHelper.jmapContext;
+import static org.apache.james.util.ReactorUtils.logOnError;
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
@@ -116,9 +120,13 @@ public class DownloadRoutes implements JMAPRoutes {
private Mono<Void> post(HttpServerRequest request, HttpServerResponse response, DownloadPath downloadPath) {
return authenticator.authenticate(request)
.flatMap(session -> Mono.from(metricFactory.runPublishingTimerMetric("JMAP-download-post",
- respondAttachmentAccessToken(session, downloadPath, response)))
- .onErrorResume(InternalErrorException.class, e -> handleInternalError(response, e))
- .onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e)))
+ respondAttachmentAccessToken(session, downloadPath, response)))
+ .subscriberContext(jmapAuthContext(session)))
+ .onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e))
+ .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e)))
+ .onErrorResume(e -> handleInternalError(response, e))
+ .subscriberContext(jmapContext(request))
+ .subscriberContext(jmapAction("download-post"))
.subscribeOn(Schedulers.elastic());
}
@@ -142,9 +150,13 @@ public class DownloadRoutes implements JMAPRoutes {
private Mono<Void> get(HttpServerRequest request, HttpServerResponse response, DownloadPath downloadPath) {
return authenticator.authenticate(request)
.flatMap(session -> Mono.from(metricFactory.runPublishingTimerMetric("JMAP-download-get",
- download(session, downloadPath, response)))
- .onErrorResume(InternalErrorException.class, e -> handleInternalError(response, e)))
+ download(session, downloadPath, response)))
+ .subscriberContext(jmapAuthContext(session)))
.onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e))
+ .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e)))
+ .onErrorResume(e -> handleInternalError(response, e))
+ .subscriberContext(jmapContext(request))
+ .subscriberContext(jmapAction("download-get"))
.subscribeOn(Schedulers.elastic());
}
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java
index 9973616..9a483e2 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java
@@ -22,6 +22,9 @@ import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE;
import static io.netty.handler.codec.http.HttpResponseStatus.OK;
import static org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE;
import static org.apache.james.jmap.http.JMAPUrls.JMAP;
+import static org.apache.james.jmap.http.LoggingHelper.jmapAuthContext;
+import static org.apache.james.jmap.http.LoggingHelper.jmapContext;
+import static org.apache.james.util.ReactorUtils.logOnError;
import java.io.IOException;
@@ -87,15 +90,16 @@ public class JMAPApiRoutes implements JMAPRoutes {
private Mono<Void> post(HttpServerRequest request, HttpServerResponse response) {
return authenticator.authenticate(request)
.flatMap(session -> Flux.merge(
- userProvisioner.provisionUser(session),
- defaultMailboxesProvisioner.createMailboxesIfNeeded(session))
- .then()
- .thenReturn(session))
- .flatMap(session -> Mono.from(metricFactory.runPublishingTimerMetric("JMAP-request",
- post(request, response, session))))
+ userProvisioner.provisionUser(session),
+ defaultMailboxesProvisioner.createMailboxesIfNeeded(session))
+ .then(Mono.from(metricFactory.runPublishingTimerMetric("JMAP-request",
+ post(request, response, session))))
+ .subscriberContext(jmapAuthContext(session)))
.onErrorResume(BadRequestException.class, e -> handleBadRequest(response, e))
.onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e))
+ .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e)))
.onErrorResume(e -> handleInternalError(response, e))
+ .subscriberContext(jmapContext(request))
.subscribeOn(Schedulers.elastic());
}
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/LoggingHelper.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/LoggingHelper.java
new file mode 100644
index 0000000..af504f63
--- /dev/null
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/LoggingHelper.java
@@ -0,0 +1,46 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.jmap.http;
+
+import static org.apache.james.util.ReactorUtils.context;
+
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.util.MDCBuilder;
+
+import reactor.netty.http.server.HttpServerRequest;
+import reactor.util.context.Context;
+
+public interface LoggingHelper {
+ static Context jmapAuthContext(MailboxSession session) {
+ return context("JMAP_AUTH",
+ MDCBuilder.of(MDCBuilder.USER, session.getUser().asString()));
+ }
+
+ static Context jmapContext(HttpServerRequest req) {
+ return context("JMAP", MDCBuilder.create()
+ .addContext(MDCBuilder.PROTOCOL, "JMAP")
+ .addContext(MDCBuilder.IP, req.hostAddress().getHostString()));
+ }
+
+ static Context jmapAction(String action) {
+ return context("JMAP_ACTION",
+ MDCBuilder.of(MDCBuilder.ACTION, action));
+ }
+}
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java
index f29ae56..ff0f306 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java
@@ -23,6 +23,10 @@ import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST;
import static io.netty.handler.codec.http.HttpResponseStatus.CREATED;
import static org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE_UTF8;
import static org.apache.james.jmap.http.JMAPUrls.UPLOAD;
+import static org.apache.james.jmap.http.LoggingHelper.jmapAction;
+import static org.apache.james.jmap.http.LoggingHelper.jmapAuthContext;
+import static org.apache.james.jmap.http.LoggingHelper.jmapContext;
+import static org.apache.james.util.ReactorUtils.logOnError;
import java.io.EOFException;
import java.io.IOException;
@@ -91,11 +95,15 @@ public class UploadRoutes implements JMAPRoutes {
return response.status(BAD_REQUEST).send();
} else {
return authenticator.authenticate(request)
- .flatMap(session -> post(request, response, contentType, session))
+ .flatMap(session -> post(request, response, contentType, session)
+ .subscriberContext(jmapAuthContext(session)))
.onErrorResume(CancelledUploadException.class, e -> handleCanceledUpload(response, e))
.onErrorResume(BadRequestException.class, e -> handleBadRequest(response, e))
.onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e))
- .onErrorResume(InternalErrorException.class, e -> handleInternalError(response, e))
+ .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e)))
+ .onErrorResume(e -> handleInternalError(response, e))
+ .subscriberContext(jmapContext(request))
+ .subscriberContext(jmapAction("upload-get"))
.subscribeOn(Schedulers.elastic());
}
}
diff --git a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JMAPApiRoutesTest.java b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JMAPApiRoutesTest.java
index e8cafca..5cb214f 100644
--- a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JMAPApiRoutesTest.java
+++ b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JMAPApiRoutesTest.java
@@ -24,18 +24,19 @@ import static io.restassured.config.RestAssuredConfig.newConfig;
import static org.apache.james.jmap.http.JMAPUrls.JMAP;
import static org.hamcrest.Matchers.equalTo;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.nio.charset.StandardCharsets;
-import java.util.stream.Stream;
+import org.apache.james.core.Username;
import org.apache.james.jmap.draft.methods.ErrorResponse;
import org.apache.james.jmap.draft.methods.Method;
import org.apache.james.jmap.draft.methods.RequestHandler;
import org.apache.james.jmap.draft.model.InvocationResponse;
import org.apache.james.jmap.draft.model.MethodCallId;
-import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MailboxSessionUtil;
import org.apache.james.metrics.tests.RecordingMetricFactory;
import org.junit.After;
import org.junit.Before;
@@ -85,8 +86,8 @@ public class JMAPApiRoutesTest {
.setBasePath(JMAP)
.build();
- when(mockedAuthFilter.authenticate(any()))
- .thenReturn(Mono.just(mock(MailboxSession.class)));
+ doReturn(Mono.just(MailboxSessionUtil.create(Username.of("bob"))))
+ .when(mockedAuthFilter).authenticate(any());
when(mockedUserProvisionner.provisionUser(any()))
.thenReturn(Mono.empty());
when(mockedMailboxesProvisionner.createMailboxesIfNeeded(any()))
diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutes.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutes.java
index 565c9c1..c42085d 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutes.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutes.java
@@ -48,7 +48,6 @@ public interface JMAPRoutes {
Logger logger();
default Mono<Void> handleInternalError(HttpServerResponse response, Throwable e) {
- logger().error("Internal error", e);
return response.status(INTERNAL_SERVER_ERROR).send();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org