Support disabling client QoS (#366)

Support disabling client QoS
palantir · Feb 18, 2020 · 9028b4b · 9028b4b
1 parent 0593d6b
commit 9028b4b
Show file tree

Hide file tree

Showing 12 changed files with 104 additions and 61 deletions.
diff --git a/changelog/@unreleased/pr-366.v2.yml b/changelog/@unreleased/pr-366.v2.yml
@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Support disabling client QoS
+  links:
+  - https://github.com/palantir/dialogue/pull/366
diff --git a/...e-apache-hc4-client/src/main/java/com/palantir/dialogue/hc4/ApacheHttpClientChannels.java b/...e-apache-hc4-client/src/main/java/com/palantir/dialogue/hc4/ApacheHttpClientChannels.java
@@ -20,14 +20,11 @@
 import com.palantir.conjure.java.api.config.service.UserAgent;
 import com.palantir.conjure.java.client.config.CipherSuites;
 import com.palantir.conjure.java.client.config.ClientConfiguration;
-import com.palantir.conjure.java.client.config.NodeSelectionStrategy;
 import com.palantir.dialogue.Channel;
 import com.palantir.dialogue.blocking.BlockingChannelAdapter;
 import com.palantir.dialogue.core.Channels;
 import com.palantir.logsafe.Preconditions;
-import com.palantir.logsafe.SafeArg;
 import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
-import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.concurrent.TimeUnit;
@@ -39,29 +36,18 @@
 import org.apache.http.impl.client.HttpClients;
 import org.apache.http.impl.client.ProxyAuthenticationStrategy;
 import org.apache.http.impl.conn.SystemDefaultRoutePlanner;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 public final class ApacheHttpClientChannels {
-    private static final Logger log = LoggerFactory.getLogger(ApacheHttpClientChannels.class);
-
     private ApacheHttpClientChannels() {}
 
-    public static Channel create(ClientConfiguration conf, UserAgent baseAgent, TaggedMetricRegistry metrics) {
+    public static Channel create(ClientConfiguration conf, UserAgent baseAgent) {
         Preconditions.checkArgument(
                 !conf.fallbackToCommonNameVerification(), "fallback-to-common-name-verification is not supported");
         Preconditions.checkArgument(!conf.meshProxy().isPresent(), "Mesh proxy is not supported");
-        Preconditions.checkArgument(
-                conf.clientQoS() == ClientConfiguration.ClientQoS.ENABLED, "Disabling client QOS is not supported");
         Preconditions.checkArgument(
                 conf.serverQoS() == ClientConfiguration.ServerQoS.AUTOMATIC_RETRY,
                 "Propagating QoS exceptions is not supported");
         Preconditions.checkArgument(!conf.proxyCredentials().isPresent(), "Proxy credentials are not supported");
-        if (conf.nodeSelectionStrategy() != NodeSelectionStrategy.ROUND_ROBIN) {
-            log.warn(
-                    "Dialogue currently only supports ROUND_ROBIN node selection strategy. {} will be ignored",
-                    SafeArg.of("requestedStrategy", conf.nodeSelectionStrategy()));
-        }
         long socketTimeoutMillis =
                 Math.max(conf.readTimeout().toMillis(), conf.writeTimeout().toMillis());
         int connectTimeout = Ints.checkedCast(conf.connectTimeout().toMillis());
@@ -104,7 +90,7 @@ public static Channel create(ClientConfiguration conf, UserAgent baseAgent, Tagg
                 .map(uri -> BlockingChannelAdapter.of(new ApacheHttpClientBlockingChannel(client, url(uri))))
                 .collect(ImmutableList.toImmutableList());
 
-        return Channels.create(channels, baseAgent, metrics);
+        return Channels.create(channels, baseAgent, conf);
     }
 
     private static URL url(String uri) {

diff --git a/...c4-client/src/test/java/com/palantir/dialogue/hc4/ApacheApacheHttpClientChannelsTest.java b/...c4-client/src/test/java/com/palantir/dialogue/hc4/ApacheApacheHttpClientChannelsTest.java
@@ -21,7 +21,6 @@
 import com.palantir.conjure.java.client.config.ClientConfigurations;
 import com.palantir.dialogue.AbstractChannelTest;
 import com.palantir.dialogue.Channel;
-import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry;
 import java.net.URL;
 import java.nio.file.Paths;
 
@@ -37,8 +36,6 @@ protected Channel createChannel(URL baseUrl) {
                 .security(SSL_CONFIG)
                 .build();
         return ApacheHttpClientChannels.create(
-                ClientConfigurations.of(serviceConf),
-                UserAgent.of(UserAgent.Agent.of("test-service", "1.0.0")),
-                new DefaultTaggedMetricRegistry());
+                ClientConfigurations.of(serviceConf), UserAgent.of(UserAgent.Agent.of("test-service", "1.0.0")));
     }
 }
diff --git a/dialogue-core/src/main/java/com/palantir/dialogue/core/Channels.java b/dialogue-core/src/main/java/com/palantir/dialogue/core/Channels.java
@@ -18,18 +18,30 @@
 
 import com.google.common.collect.ImmutableList;
 import com.palantir.conjure.java.api.config.service.UserAgent;
+import com.palantir.conjure.java.client.config.ClientConfiguration;
+import com.palantir.conjure.java.client.config.NodeSelectionStrategy;
 import com.palantir.dialogue.Channel;
-import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
+import com.palantir.logsafe.SafeArg;
+import com.palantir.logsafe.exceptions.SafeIllegalStateException;
 import java.util.Collection;
 import java.util.List;
+import java.util.function.Function;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public final class Channels {
+    private static final Logger log = LoggerFactory.getLogger(Channels.class);
 
     private Channels() {}
 
     public static Channel create(
-            Collection<? extends Channel> channels, UserAgent userAgent, TaggedMetricRegistry metrics) {
-        DialogueClientMetrics clientMetrics = DialogueClientMetrics.of(metrics);
+            Collection<? extends Channel> channels, UserAgent userAgent, ClientConfiguration config) {
+        if (config.nodeSelectionStrategy() != NodeSelectionStrategy.ROUND_ROBIN) {
+            log.warn(
+                    "Dialogue currently only supports ROUND_ROBIN node selection strategy. {} will be ignored",
+                    SafeArg.of("requestedStrategy", config.nodeSelectionStrategy()));
+        }
+        DialogueClientMetrics clientMetrics = DialogueClientMetrics.of(config.taggedMetricRegistry());
         List<LimitedChannel> limitedChannels = channels.stream()
                 // Instrument inner-most channel with metrics so that we measure only the over-the-wire-time
                 .map(channel -> new InstrumentedChannel(channel, clientMetrics))
@@ -38,15 +50,27 @@ public static Channel create(
                 .map(TracedRequestChannel::new)
                 .map(channel -> new TracedChannel(channel, "Concurrency-Limited Dialogue Request"))
                 .map(ContentDecodingChannel::new)
-                .map(ConcurrencyLimitedChannel::create)
+                .map(concurrencyLimiter(config))
                 .collect(ImmutableList.toImmutableList());
 
         LimitedChannel limited = new RoundRobinChannel(limitedChannels);
-        Channel channel = new QueuedChannel(limited, DispatcherMetrics.of(metrics));
+        Channel channel = new QueuedChannel(limited, DispatcherMetrics.of(config.taggedMetricRegistry()));
         channel = new RetryingChannel(channel);
         channel = new UserAgentChannel(channel, userAgent);
         channel = new NeverThrowChannel(channel);
 
         return channel;
     }
+
+    private static Function<Channel, LimitedChannel> concurrencyLimiter(ClientConfiguration config) {
+        ClientConfiguration.ClientQoS clientQoS = config.clientQoS();
+        switch (clientQoS) {
+            case ENABLED:
+                return ConcurrencyLimitedChannel::create;
+            case DANGEROUS_DISABLE_SYMPATHETIC_CLIENT_QOS:
+                return UnlimitedChannel::new;
+        }
+        throw new SafeIllegalStateException(
+                "Encountered unknown client QoS configuration", SafeArg.of("ClientQoS", clientQoS));
+    }
 }
diff --git a/dialogue-core/src/main/java/com/palantir/dialogue/core/UnlimitedChannel.java b/dialogue-core/src/main/java/com/palantir/dialogue/core/UnlimitedChannel.java
@@ -0,0 +1,44 @@
+/*
+ * (c) Copyright 2020 Palantir Technologies Inc. All rights reserved.
+ *
+ * Licensed 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 com.palantir.dialogue.core;
+
+import com.google.common.util.concurrent.ListenableFuture;
+import com.palantir.dialogue.Channel;
+import com.palantir.dialogue.Endpoint;
+import com.palantir.dialogue.Request;
+import com.palantir.dialogue.Response;
+import com.palantir.logsafe.Preconditions;
+import java.util.Optional;
+
+/** Adapter from {@link Channel} to {@link LimitedChannel} which always returns a {@link Optional#isPresent() value}. */
+final class UnlimitedChannel implements LimitedChannel {
+
+    private final Channel delegate;
+
+    UnlimitedChannel(Channel delegate) {
+        this.delegate = Preconditions.checkNotNull(delegate, "Channel");
+    }
+
+    @Override
+    public Optional<ListenableFuture<Response>> maybeExecute(Endpoint endpoint, Request request) {
+        return Optional.of(delegate.execute(endpoint, request));
+    }
+
+    @Override
+    public String toString() {
+        return "UnlimitedChannel{delegate=" + delegate + '}';
+    }
+}
diff --git a/dialogue-core/src/test/java/com/palantir/dialogue/core/ChannelsTest.java b/dialogue-core/src/test/java/com/palantir/dialogue/core/ChannelsTest.java
@@ -25,14 +25,18 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
+import com.palantir.conjure.java.api.config.service.ServiceConfiguration;
 import com.palantir.conjure.java.api.config.service.UserAgent;
+import com.palantir.conjure.java.api.config.ssl.SslConfiguration;
+import com.palantir.conjure.java.client.config.ClientConfiguration;
+import com.palantir.conjure.java.client.config.ClientConfigurations;
 import com.palantir.dialogue.Channel;
 import com.palantir.dialogue.Endpoint;
 import com.palantir.dialogue.HttpMethod;
 import com.palantir.dialogue.Request;
 import com.palantir.dialogue.Response;
 import com.palantir.dialogue.UrlBuilder;
-import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry;
+import java.nio.file.Paths;
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import org.junit.Before;
@@ -45,6 +49,12 @@
 public final class ChannelsTest {
 
     public static final UserAgent USER_AGENT = UserAgent.of(UserAgent.Agent.of("foo", "1.0.0"));
+    private static final SslConfiguration SSL_CONFIG = SslConfiguration.of(
+            Paths.get("src/test/resources/trustStore.jks"), Paths.get("src/test/resources/keyStore.jks"), "keystore");
+    private static final ClientConfiguration stubConfig = ClientConfigurations.of(ServiceConfiguration.builder()
+            .addUris("http://localhost")
+            .security(SSL_CONFIG)
+            .build());
 
     @Mock
     private Channel delegate;
@@ -82,7 +92,7 @@ public String version() {
 
     @Before
     public void before() {
-        channel = Channels.create(ImmutableList.of(delegate), USER_AGENT, new DefaultTaggedMetricRegistry());
+        channel = Channels.create(ImmutableList.of(delegate), USER_AGENT, stubConfig);
 
         ListenableFuture<Response> expectedResponse = Futures.immediateFuture(response);
         when(delegate.execute(eq(endpoint), any())).thenReturn(expectedResponse);
@@ -102,8 +112,7 @@ public ListenableFuture<Response> execute(Endpoint _endpoint, Request _request)
             }
         };
 
-        channel =
-                Channels.create(ImmutableList.of(badUserImplementation), USER_AGENT, new DefaultTaggedMetricRegistry());
+        channel = Channels.create(ImmutableList.of(badUserImplementation), USER_AGENT, stubConfig);
 
         // this should never throw
         ListenableFuture<Response> future = channel.execute(endpoint, request);
@@ -121,8 +130,7 @@ public ListenableFuture<Response> execute(Endpoint _endpoint, Request _request)
             }
         };
 
-        channel =
-                Channels.create(ImmutableList.of(badUserImplementation), USER_AGENT, new DefaultTaggedMetricRegistry());
+        channel = Channels.create(ImmutableList.of(badUserImplementation), USER_AGENT, stubConfig);
 
         // this should never throw
         ListenableFuture<Response> future = channel.execute(endpoint, request);

diff --git a/...ient/src/main/java/com/palantir/dialogue/httpurlconnection/HttpUrlConnectionChannels.java b/...ient/src/main/java/com/palantir/dialogue/httpurlconnection/HttpUrlConnectionChannels.java
@@ -22,7 +22,6 @@
 import com.palantir.dialogue.blocking.BlockingChannelAdapter;
 import com.palantir.dialogue.core.Channels;
 import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
-import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
 import java.net.MalformedURLException;
 import java.net.URL;
 
@@ -31,12 +30,12 @@ public final class HttpUrlConnectionChannels {
 
     private HttpUrlConnectionChannels() {}
 
-    public static Channel create(ClientConfiguration conf, UserAgent baseAgent, TaggedMetricRegistry metrics) {
+    public static Channel create(ClientConfiguration conf, UserAgent baseAgent) {
         ImmutableList<Channel> channels = conf.uris().stream()
                 .map(uri -> BlockingChannelAdapter.of(new HttpUrlConnectionBlockingChannel(conf, url(uri))))
                 .collect(ImmutableList.toImmutableList());
 
-        return Channels.create(channels, baseAgent, metrics);
+        return Channels.create(channels, baseAgent, conf);
     }
 
     private static URL url(String uri) {

diff --git a/.../src/test/java/com/palantir/dialogue/httpurlconnection/HttpUrlConnectionChannelsTest.java b/.../src/test/java/com/palantir/dialogue/httpurlconnection/HttpUrlConnectionChannelsTest.java
@@ -21,7 +21,6 @@
 import com.palantir.conjure.java.client.config.ClientConfigurations;
 import com.palantir.dialogue.AbstractChannelTest;
 import com.palantir.dialogue.Channel;
-import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry;
 import java.net.URL;
 import java.nio.file.Paths;
 
@@ -37,8 +36,6 @@ protected Channel createChannel(URL baseUrl) {
                 .security(SSL_CONFIG)
                 .build();
         return HttpUrlConnectionChannels.create(
-                ClientConfigurations.of(serviceConf),
-                UserAgent.of(UserAgent.Agent.of("test-service", "1.0.0")),
-                new DefaultTaggedMetricRegistry());
+                ClientConfigurations.of(serviceConf), UserAgent.of(UserAgent.Agent.of("test-service", "1.0.0")));
     }
 }
diff --git a/dialogue-java-client/src/main/java/com/palantir/dialogue/JavaChannels.java b/dialogue-java-client/src/main/java/com/palantir/dialogue/JavaChannels.java
@@ -21,7 +21,6 @@
 import com.palantir.conjure.java.api.config.service.UserAgent;
 import com.palantir.conjure.java.client.config.ClientConfiguration;
 import com.palantir.dialogue.core.Channels;
-import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.http.HttpClient;
@@ -37,7 +36,7 @@ public final class JavaChannels {
 
     private JavaChannels() {}
 
-    public static Channel create(ClientConfiguration conf, UserAgent baseAgent, TaggedMetricRegistry metrics) {
+    public static Channel create(ClientConfiguration conf, UserAgent baseAgent) {
         // TODO(jellis): read/write timeouts
         // TODO(jellis): gcm cipher toggle
         // TODO(jellis): proxy creds + mesh proxy
@@ -61,7 +60,7 @@ public static Channel create(ClientConfiguration conf, UserAgent baseAgent, Tagg
                 .map(uri -> HttpChannel.of(client, url(uri)))
                 .collect(toList());
 
-        return Channels.create(channels, baseAgent, metrics);
+        return Channels.create(channels, baseAgent, conf);
     }
 
     private static URL url(String uri) {

diff --git a/dialogue-java-client/src/test/java/com/palantir/dialogue/JavaChannelsTest.java b/dialogue-java-client/src/test/java/com/palantir/dialogue/JavaChannelsTest.java
@@ -20,7 +20,6 @@
 import com.palantir.conjure.java.api.config.service.UserAgent;
 import com.palantir.conjure.java.api.config.ssl.SslConfiguration;
 import com.palantir.conjure.java.client.config.ClientConfigurations;
-import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry;
 import java.net.URL;
 import java.nio.file.Paths;
 
@@ -35,8 +34,6 @@ protected Channel createChannel(URL baseUrl) {
                 .security(SSL_CONFIG)
                 .build();
         return JavaChannels.create(
-                ClientConfigurations.of(serviceConf),
-                UserAgent.of(UserAgent.Agent.of("test-service", "1.0.0")),
-                new DefaultTaggedMetricRegistry());
+                ClientConfigurations.of(serviceConf), UserAgent.of(UserAgent.Agent.of("test-service", "1.0.0")));
     }
 }
diff --git a/dialogue-okhttp-client/src/main/java/com/palantir/dialogue/OkHttpChannels.java b/dialogue-okhttp-client/src/main/java/com/palantir/dialogue/OkHttpChannels.java
@@ -23,12 +23,9 @@
 import com.palantir.conjure.java.api.config.service.UserAgent;
 import com.palantir.conjure.java.client.config.CipherSuites;
 import com.palantir.conjure.java.client.config.ClientConfiguration;
-import com.palantir.conjure.java.client.config.NodeSelectionStrategy;
 import com.palantir.dialogue.core.Channels;
 import com.palantir.logsafe.Preconditions;
-import com.palantir.logsafe.SafeArg;
 import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
-import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.concurrent.ExecutorService;
@@ -97,20 +94,13 @@ public final class OkHttpChannels {
 
     private OkHttpChannels() {}
 
-    public static Channel create(ClientConfiguration config, UserAgent baseAgent, TaggedMetricRegistry metrics) {
+    public static Channel create(ClientConfiguration config, UserAgent baseAgent) {
         Preconditions.checkArgument(
                 !config.fallbackToCommonNameVerification(), "fallback-to-common-name-verification is not supported");
         Preconditions.checkArgument(!config.meshProxy().isPresent(), "Mesh proxy is not supported");
-        Preconditions.checkArgument(
-                config.clientQoS() == ClientConfiguration.ClientQoS.ENABLED, "Disabling client QOS is not supported");
         Preconditions.checkArgument(
                 config.serverQoS() == ClientConfiguration.ServerQoS.AUTOMATIC_RETRY,
                 "Propagating QoS exceptions is not supported");
-        if (config.nodeSelectionStrategy() != NodeSelectionStrategy.ROUND_ROBIN) {
-            log.warn(
-                    "Dialogue currently only supports ROUND_ROBIN node selection strategy. {} will be ignored",
-                    SafeArg.of("requestedStrategy", config.nodeSelectionStrategy()));
-        }
         OkHttpClient.Builder builder = new OkHttpClient()
                 .newBuilder()
                 .dispatcher(dispatcher)
@@ -149,7 +139,7 @@ public static Channel create(ClientConfiguration config, UserAgent baseAgent, Ta
                 .map(uri -> OkHttpChannel.of(client, url(uri)))
                 .collect(ImmutableList.toImmutableList());
 
-        return Channels.create(channels, baseAgent, metrics);
+        return Channels.create(channels, baseAgent, config);
     }
 
     private static URL url(String uri) {

diff --git a/dialogue-okhttp-client/src/test/java/com/palantir/dialogue/OkHttpChannelTest.java b/dialogue-okhttp-client/src/test/java/com/palantir/dialogue/OkHttpChannelTest.java
@@ -20,7 +20,6 @@
 import com.palantir.conjure.java.api.config.service.UserAgent;
 import com.palantir.conjure.java.api.config.ssl.SslConfiguration;
 import com.palantir.conjure.java.client.config.ClientConfigurations;
-import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry;
 import java.net.URL;
 import java.nio.file.Paths;
 
@@ -35,8 +34,6 @@ protected Channel createChannel(URL baseUrl) {
                 .security(SSL_CONFIG)
                 .build();
         return OkHttpChannels.create(
-                ClientConfigurations.of(serviceConf),
-                UserAgent.of(UserAgent.Agent.of("test-service", "1.0.0")),
-                new DefaultTaggedMetricRegistry());
+                ClientConfigurations.of(serviceConf), UserAgent.of(UserAgent.Agent.of("test-service", "1.0.0")));
     }
 }