Merge pull request #56 from confluentinc/pankaj/RemoveExtraChanges

docs/operations/metrics.md

@@ -86,6 +86,7 @@ Most metric values reset each emission period, as specified in `druid.monitoring
 |`subquery/fallback/unknownReason/count`|Number of subqueries which cannot be materialized as frames due other reasons.|This metric is only available if the `SubqueryCountStatsMonitor` module is included.| |
 |`query/rowLimit/exceeded/count`|Number of queries whose inlined subquery results exceeded the given row limit|This metric is only available if the `SubqueryCountStatsMonitor` module is included.| |
 |`query/byteLimit/exceeded/count`|Number of queries whose inlined subquery results exceeded the given byte limit|This metric is only available if the `SubqueryCountStatsMonitor` module is included.| |
+|`httpClient/channelAcquire/time`|Time in nannoseconds spent by the httpclient to acquire the channel.| |
 
 ### Historical
 
@@ -133,7 +134,6 @@ Most metric values reset each emission period, as specified in `druid.monitoring
 |`jetty/threadPool/min`|Number of minimum threads allocatable.|`druid.server.http.numThreads` plus a small fixed number of threads allocated for Jetty acceptors and selectors.|
 |`jetty/threadPool/max`|Number of maximum threads allocatable.|`druid.server.http.numThreads` plus a small fixed number of threads allocated for Jetty acceptors and selectors.|
 |`jetty/threadPool/queueSize`|Size of the worker queue.|Not much higher than `druid.server.http.queueSize`.|
-|`jetty/httpClient/threadpool/queueSize`|Size of the worker queue at jetty http client.|Less than or equal to 1024(default value).|
 
 ### Cache
 

processing/src/main/java/org/apache/druid/java/util/http/client/pool/MetricsEmittingResourcePool.java

@@ -38,7 +38,7 @@ public class MetricsEmittingResourcePool<K, V> extends ResourcePool<K, V>
     long startTime = System.nanoTime();
     ResourceContainer<V> retVal = super.take(key);
     long totalduration = System.nanoTime() - startTime;
-    emitter.emit(ServiceMetricEvent.builder().setDimension("destination", key.toString()).setMetric("httpClient/channelAcquire/time", totalduration));
+    emitter.emit(ServiceMetricEvent.builder().setDimension("server", key.toString()).setMetric("httpClient/channelAcquire/time", totalduration));
     return retVal;
   }
 

server/src/main/java/org/apache/druid/guice/http/JettyHttpClientModule.java

@@ -23,27 +23,16 @@ import com.google.common.base.Preconditions;
 import com.google.inject.Binder;
 import com.google.inject.Binding;
 import com.google.inject.Module;
-import com.google.inject.Provides;
-import com.google.inject.Singleton;
 import org.apache.druid.guice.JsonConfigProvider;
 import org.apache.druid.guice.LazySingleton;
 import org.apache.druid.guice.annotations.Global;
 import org.apache.druid.java.util.common.lifecycle.Lifecycle;
-import org.apache.druid.java.util.emitter.service.ServiceEmitter;
-import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
-import org.apache.druid.java.util.metrics.AbstractMonitor;
-import org.apache.druid.java.util.metrics.MonitorUtils;
-import org.apache.druid.server.metrics.DataSourceTaskIdHolder;
-import org.apache.druid.server.metrics.MetricsModule;
-import org.apache.druid.server.metrics.MonitorsConfig;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
 
-import javax.inject.Provider;
 import javax.net.ssl.SSLContext;
 import java.lang.annotation.Annotation;
-import java.util.Map;
 
 /**
  *
@@ -73,12 +62,10 @@ public class JettyHttpClientModule implements Module
           .toProvider(new HttpClientProvider(annotationClazz))
           .in(LazySingleton.class);
 
-    MetricsModule.register(binder, JettyHttpClientModule.JettyMonitor.class);
   }
 
   public static class HttpClientProvider extends AbstractHttpClientProvider<HttpClient>
   {
-    private QueuedThreadPool threadPool;
 
     public HttpClientProvider(Class<? extends Annotation> annotation)
     {
@@ -105,9 +92,9 @@ public class JettyHttpClientModule implements Module
       httpClient.setMaxRequestsQueuedPerDestination(config.getNumRequestsQueued());
       httpClient.setConnectTimeout(config.getClientConnectTimeout());
       httpClient.setRequestBufferSize(config.getRequestBuffersize());
-      threadPool = new QueuedThreadPool(config.getNumMaxThreads());
+      final QueuedThreadPool pool = new QueuedThreadPool(config.getNumMaxThreads());
-      threadPool.setName(JettyHttpClientModule.class.getSimpleName() + "-threadPool-" + threadPool.hashCode());
+      pool.setName(JettyHttpClientModule.class.getSimpleName() + "-threadPool-" + pool.hashCode());
-      httpClient.setExecutor(threadPool);
+      httpClient.setExecutor(pool);
 
       final Lifecycle lifecycle = getLifecycleProvider().get();
 
@@ -140,34 +127,4 @@ public class JettyHttpClientModule implements Module
       return httpClient;
     }
   }
-
-  @Provides
-  @Singleton
-  public JettyHttpClientModule.JettyMonitor getJettyMonitor(DataSourceTaskIdHolder dataSourceTaskIdHolder, Provider<HttpClientProvider> httpClientProvider)
-  {
-    return new JettyHttpClientModule.JettyMonitor(dataSourceTaskIdHolder.getDataSource(), dataSourceTaskIdHolder.getTaskId(), httpClientProvider.get().threadPool);
-  }
-
-  public static class JettyMonitor extends AbstractMonitor
-  {
-    private final Map<String, String[]> dimensions;
-    private final QueuedThreadPool threadPool;
-
-    public JettyMonitor(String dataSource, String taskId, QueuedThreadPool threadPool)
-    {
-      this.dimensions = MonitorsConfig.mapOfDatasourceAndTaskID(dataSource, taskId);
-      this.threadPool = threadPool;
-    }
-
-    @Override
-    public boolean doMonitor(ServiceEmitter emitter)
-    {
-      final ServiceMetricEvent.Builder builder = new ServiceMetricEvent.Builder();
-      MonitorUtils.addDimensionsToBuilder(builder, dimensions);
-      if (threadPool != null) {
-        emitter.emit(builder.setMetric("jetty/httpClient/threadpool/queueSize", threadPool.getQueueSize()));
-      }
-      return true;
-    }
-  }
 }

server/src/test/java/org/apache/druid/guice/JettyHttpClientModuleTest.java

@@ -1,67 +0,0 @@
-/*
- * 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.druid.guice;
-
-import org.apache.druid.guice.http.JettyHttpClientModule;
-import org.apache.druid.java.util.common.Pair;
-import org.apache.druid.java.util.emitter.core.Emitter;
-import org.apache.druid.java.util.emitter.core.Event;
-import org.apache.druid.java.util.emitter.service.ServiceEmitter;
-import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
-import org.apache.druid.server.initialization.jetty.JettyServerModule;
-import org.eclipse.jetty.util.thread.QueuedThreadPool;
-import org.junit.Assert;
-import org.junit.Test;
-import org.mockito.Mockito;
-
-import java.util.ArrayList;
-import java.util.List;
-
-public class JettyHttpClientModuleTest
-{
-  @Test
-  public void testJettyHttpClientModule()
-  {
-    List<Event> events = new ArrayList<>();
-    ServiceEmitter serviceEmitter = new ServiceEmitter("service", "host", Mockito.mock(Emitter.class))
-    {
-      @Override
-      public void emit(Event event)
-      {
-        events.add(event);
-      }
-    };
-    QueuedThreadPool threadPool = Mockito.mock(QueuedThreadPool.class);
-    JettyServerModule.setJettyServerThreadPool(threadPool);
-    Mockito.when(threadPool.getQueueSize()).thenReturn(50);
-
-    JettyHttpClientModule.JettyMonitor jettyMonitor = new JettyHttpClientModule.JettyMonitor("ds", "t0", threadPool);
-    jettyMonitor.doMonitor(serviceEmitter);
-
-    Assert.assertEquals(1, events.size());
-    Pair<String, Number> expectedEvent = new Pair<>("jetty/httpClient/threadpool/queueSize", 50);
-
-
-    ServiceMetricEvent actual = (ServiceMetricEvent) (events.get(0));
-    Assert.assertEquals(expectedEvent.lhs, actual.getMetric());
-    Assert.assertEquals(expectedEvent.rhs, actual.getValue());
-
-  }
-}