Fixes #266 - MDC logging of the load balancer address now logs the proper address

CHANGELOG.md

@@ -19,6 +19,7 @@ This project aims to adhere to [Semantic Versioning](http://semver.org/).
    leave the default Apache HttpClient [retry behavior](https://hc.apache.org/httpcomponents-client-4.5.x/tutorial/html/fundamentals.html#d5e316).
  - [Content type is set for file object in directory listing when it isn't available](https://github.com/joyent/java-manta/issues/341)
  - [Fixes validation guard clauses that are not validating anything](https://github.com/joyent/java-manta/issues/346)
+ - [MDC logging of the load balancer address now logs the proper address](https://github.com/joyent/java-manta/issues/266)
 ### Changed
  - Validation of paths passed to `MantaClient` is now more consistently strict.
    More useful errors should be thrown sooner for invalid paths, without any

java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaClient.java

@@ -22,6 +22,7 @@
 import com.joyent.manta.exception.MantaException;
 import com.joyent.manta.exception.MantaIOException;
 import com.joyent.manta.exception.MantaJobException;
+import com.joyent.manta.exception.MantaNoHttpResponseException;
 import com.joyent.manta.exception.OnCloseAggregateException;
 import com.joyent.manta.http.ContentTypeLookup;
 import com.joyent.manta.http.EncryptionHttpHelper;
@@ -1756,7 +1757,7 @@ public UUID createJob(final MantaJob job) throws IOException {
             try {
                 return httpHelper.executeAndCloseRequest(post,
                         jobIdFunction, "POST   {} response [{}] {} ", path);
-            } catch (NoHttpResponseException e) {
+            } catch (NoHttpResponseException | MantaNoHttpResponseException e) {
                 lastException = e;
                 LOG.warn("Error posting createJob. Retrying.", e);
             }

java-manta-client-unshaded/src/main/java/com/joyent/manta/client/MantaObjectOutputStream.java

@@ -310,7 +310,10 @@ public synchronized void close() throws IOException {
         } catch (InterruptedException e) {
             // continue execution if interrupted
         } catch (ExecutionException e) {
-            MantaIOException mioe = new MantaIOException(e);
+            /* We wrap the cause because the stack trace for the
+             * ExecutionException offers nothing useful and is just a wrapper
+             * for exceptions that are thrown within a Future. */
+            MantaIOException mioe = new MantaIOException(e.getCause());
 
             if (this.objectResponse != null) {
                 final String requestId = this.objectResponse.getHeaderAsString(

java-manta-client-unshaded/src/main/java/com/joyent/manta/exception/MantaNoHttpResponseException.java

@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2017, Joyent, Inc. All rights reserved.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+package com.joyent.manta.exception;
+
+/**
+ * Signals that the target server failed to respond with a valid HTTP response.
+ *
+ * This exception typically wraps {@link org.apache.http.NoHttpResponseException}.
+ *
+ * @author <a href="https://github.com/dekobon">Elijah Zupancic</a>
+ * @since 3.1.7
+ */
+public class MantaNoHttpResponseException extends MantaIOException {
+    /**
+     * Constructs an instance with {@code null}
+     * as its error detail message.
+     */
+    public MantaNoHttpResponseException() {
+    }
+
+    /**
+     * Constructs an instance with the specified detail message.
+     *
+     * @param message The detail message (which is saved for later retrieval
+     *                by the {@link #getMessage()} method)
+     */
+    public MantaNoHttpResponseException(final String message) {
+        super(message);
+    }
+
+    /**
+     * Constructs an instance with the specified detail message
+     * and cause.
+     *
+     * <p>Note that the detail message associated with {@code cause} is
+     * <i>not</i> automatically incorporated into this exception's detail
+     * message.</p>
+     *
+     * @param message The detail message (which is saved for later retrieval
+     *                by the {@link #getMessage()} method)
+     * @param cause The cause (which is saved for later retrieval by the
+     *              {@link #getCause()} method).  (A null value is permitted,
+     *              and indicates that the cause is nonexistent or unknown.)
+     */
+    public MantaNoHttpResponseException(final String message, final Throwable cause) {
+        super(message, cause);
+    }
+
+    /**
+     * Constructs an instance with the specified cause and a
+     * detail message of {@code (cause==null ? null : cause.toString())}
+     * (which typically contains the class and detail message of {@code cause}).
+     * This constructor is useful for IO exceptions that are little more
+     * than wrappers for other throwables.
+     *
+     * @param cause The cause (which is saved for later retrieval by the
+     *              {@link #getCause()} method).  (A null value is permitted,
+     *              and indicates that the cause is nonexistent or unknown.)
+     */
+    public MantaNoHttpResponseException(final Throwable cause) {
+        super(cause);
+    }
+}

java-manta-client-unshaded/src/main/java/com/joyent/manta/http/MantaConnectionFactory.java

@@ -48,6 +48,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.management.DynamicMBean;
 import java.io.Closeable;
 import java.io.IOException;
 import java.net.InetSocketAddress;
@@ -58,7 +59,6 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
-import javax.management.DynamicMBean;
 
 /**
  * Factory class that creates instances of
@@ -271,6 +271,7 @@ protected HttpClientBuilder createBuilder() {
                 .setKeepAliveStrategy(new DefaultConnectionKeepAliveStrategy())
                 .setDefaultRequestConfig(requestConfig)
                 .setConnectionManagerShared(false)
+                .setRequestExecutor(new MantaHttpRequestExecutor())
                 .setConnectionBackoffStrategy(new DefaultBackoffStrategy());
 
         if (config.getRetries() > 0) {

java-manta-client-unshaded/src/main/java/com/joyent/manta/http/MantaHttpRequestExecutor.java

@@ -0,0 +1,128 @@
+/*
+ * Copyright (c) 2017, Joyent, Inc. All rights reserved.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+package com.joyent.manta.http;
+
+import com.joyent.manta.exception.MantaIOException;
+import com.joyent.manta.exception.MantaNoHttpResponseException;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.http.HttpClientConnection;
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.NoHttpResponseException;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestExecutor;
+import org.slf4j.MDC;
+
+import java.io.IOException;
+
+/**
+ * Extended implementation of {@link HttpRequestExecutor} with Manta specific
+ * extensions for logging and exception handling.
+ *
+ * @author <a href="https://github.com/dekobon">Elijah Zupancic</a>
+ * @since 3.1.7
+ */
+public class MantaHttpRequestExecutor extends HttpRequestExecutor {
+    /**
+     * Creates new instance of HttpRequestExecutor.
+     *
+     * @param waitForContinue Maximum time in milliseconds to wait for a 100-continue response
+     */
+    public MantaHttpRequestExecutor(final int waitForContinue) {
+        super(waitForContinue);
+    }
+
+    /**
+     * Creates new instance of HttpRequestExecutor.
+     */
+    public MantaHttpRequestExecutor() {
+    }
+
+    /**
+     * Adds a context value for the Manta load balancer associated with the
+     * request to the MDC object with the key <code>mantaLoadBalancerAddress</code>
+     * and proxies the parent class
+     * {@link HttpRequestExecutor#doSendRequest(HttpRequest, HttpClientConnection, HttpContext)}
+     * method.
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    protected HttpResponse doSendRequest(final HttpRequest request,
+                                         final HttpClientConnection conn,
+                                         final HttpContext context) throws IOException, HttpException {
+        MDC.put("mantaLoadBalancerAddress", extractLoadBalancerAddress(conn));
+        return super.doSendRequest(request, conn, context);
+    }
+
+    /**
+     * Proxies the parent class
+     * {@link HttpRequestExecutor#doReceiveResponse(HttpRequest, HttpClientConnection, HttpContext)}
+     * method and catches {@link IOException} instances thrown. Those exceptions
+     * are then wrapped in a {@link MantaIOException} or
+     * {@link MantaNoHttpResponseException} instance in order to provide
+     * detailed information for debugging.
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    protected HttpResponse doReceiveResponse(
+            final HttpRequest request,
+            final HttpClientConnection conn,
+            final HttpContext context) throws HttpException, IOException {
+        HttpResponse response = null;
+
+        try {
+            response = super.doReceiveResponse(request, conn, context);
+
+        /* We catch all IOExceptions and wrap then in a MantaIOException because
+         * this allows us to capture key information like the request id and
+         * load balancer address directly in the exception message. */
+        } catch (IOException e) {
+            final MantaIOException mioe;
+
+            /* If the source exception is NoHttpResponseException we create
+             * a MantaNoHttpResponseException, so that we can act upon that
+             * exception type directly within Manta. */
+            if (e instanceof NoHttpResponseException) {
+                mioe = new MantaNoHttpResponseException(e);
+            } else {
+                mioe = new MantaIOException(e);
+            }
+
+            HttpHelper.annotateContextedException(mioe, request, response);
+
+            if (request.getFirstHeader(MantaHttpHeaders.REQUEST_ID) != null) {
+                mioe.setContextValue("requestId",
+                        request.getFirstHeader(MantaHttpHeaders.REQUEST_ID).getValue());
+            }
+
+            mioe.setContextValue("loadBalancerAddress", extractLoadBalancerAddress(conn));
+
+            throw mioe;
+        }
+
+        return response;
+    }
+
+    /**
+     * Extracts the remote load balancer IP address from the toString() method
+     * of a {@link HttpClientConnection}.
+     *
+     * @param conn connection to extract IP information from
+     * @return IP address string or null if connection is null
+     */
+    private static String extractLoadBalancerAddress(final HttpClientConnection conn) {
+        if (conn == null) {
+            return null;
+        }
+
+        return StringUtils.substringBetween(conn.toString(), "<->", ":");
+    }
+}

java-manta-client-unshaded/src/main/java/com/joyent/manta/http/ShufflingDnsResolver.java

@@ -8,7 +8,6 @@
 package com.joyent.manta.http;
 
 import org.apache.http.conn.DnsResolver;
-import org.slf4j.MDC;
 
 import java.net.InetAddress;
 import java.net.UnknownHostException;
@@ -28,8 +27,6 @@ public InetAddress[] resolve(final String host) throws UnknownHostException {
         final InetAddress[] addresses = InetAddress.getAllByName(host);
         shuffle(addresses);
 
-        MDC.put("mantaLoadBalancerAddress", addresses[0].getHostAddress());
-
         return addresses;
     }
 

java-manta-client-unshaded/src/test/java/com/joyent/manta/client/MantaClientConnectionFailuresIT.java

@@ -12,6 +12,7 @@
 import com.joyent.manta.config.ConfigContext;
 import com.joyent.manta.config.KeyPairFactory;
 import com.joyent.manta.config.TestConfigContext;
+import com.joyent.manta.exception.MantaNoHttpResponseException;
 import com.joyent.manta.http.MantaConnectionFactory;
 import org.apache.http.HttpException;
 import org.apache.http.HttpResponse;
@@ -125,7 +126,7 @@ public void canRetryOnNoHttpResponseException() throws IOException {
         boolean thrown = false;
         try {
             mantaClient.head(testPathPrefix);
-        } catch (NoHttpResponseException e) {
+        } catch (NoHttpResponseException | MantaNoHttpResponseException e) {
             thrown = true;
         } finally {
             mantaClient.closeWithWarning();

java-manta-it/src/test/resources/logback-test.xml

@@ -3,7 +3,7 @@
         <!-- encoders are assigned the type
              ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
         <encoder>
-            <pattern>[%thread] %-5level %logger [%X{mantaRequestId}] - %msg%n</pattern>
+            <pattern>[%thread] %-5level %logger [%X{mantaLoadBalancerAddress} %X{mantaRequestId}] - %msg%n</pattern>
         </encoder>
     </appender>