new:dev:Choose Dropwizard Metrics as Metrics Collection Framework (qu…

…bole#120) We have wanted to standardize on Dropwizard Metrics for monitoring and the basis of alerting. Rubix depends on Hadoop jars and these bring in their own ecosystem of dependencies and their versions. Therefore we couldn't add Dropwizard Metrics because it may conflict with another version in the classpath. We considered multiple options: * An interface (qubole#99) * Shade the jar * Reuse a version in the class path by using provided scope. This commit has chosen the last option and has been tested with Presto & Hive.
jianlirong · May 10, 2018 · 5bc1d88 · 5bc1d88
1 parent f478880
commit 5bc1d88
Show file tree

Hide file tree

Showing 11 changed files with 138 additions and 7 deletions.
diff --git a/pom.xml b/pom.xml
@@ -50,6 +50,8 @@
         <commons-httpclient.version>3.0.1</commons-httpclient.version>
         <dep.hadoop1.version>1.2.0</dep.hadoop1.version>
         <dep.hadoop2.version>2.6.0</dep.hadoop2.version>
+        <dep.metrics.version>3.1.0</dep.metrics.version>
+        <dep.testng.version>6.9.8</dep.testng.version>
     </properties>
 
     <modules>
@@ -137,6 +139,12 @@
                 <version>${dep.hadoop2.version}</version>
             </dependency>
 
+            <dependency>
+                <groupId>io.dropwizard.metrics</groupId>
+                <artifactId>metrics-core</artifactId>
+                <version>${dep.metrics.version}</version>
+                <scope>provided</scope>
+            </dependency>
 
             <dependency>
                 <groupId>org.weakref</groupId>

diff --git a/rubix-bookkeeper/pom.xml b/rubix-bookkeeper/pom.xml
@@ -63,6 +63,20 @@
             <groupId>com.qubole.rubix</groupId>
             <artifactId>rubix-presto</artifactId>
         </dependency>
+
+        <dependency>
+            <groupId>io.dropwizard.metrics</groupId>
+            <artifactId>metrics-core</artifactId>
+            <version>${dep.metrics.version}</version>
+            <scope>provided</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>org.testng</groupId>
+            <artifactId>testng</artifactId>
+            <version>${dep.testng.version}</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

diff --git a/rubix-bookkeeper/src/main/java/com/qubole/rubix/bookkeeper/BookKeeper.java b/rubix-bookkeeper/src/main/java/com/qubole/rubix/bookkeeper/BookKeeper.java
@@ -12,6 +12,8 @@
  */
 package com.qubole.rubix.bookkeeper;
 
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.MetricRegistry;
 import com.google.common.base.Charsets;
 import com.google.common.base.Throwables;
 import com.google.common.cache.Cache;
@@ -64,6 +66,8 @@
  */
 public class BookKeeper implements com.qubole.rubix.spi.BookKeeperService.Iface
 {
+  public static final String METRIC_BOOKKEEPER_LOCAL_CACHE_COUNT = "rubix.bookkeeper.local_cache.count";
+
   private static Cache<String, FileMetadata> fileMetadataCache;
   private static ClusterManager clusterManager;
   private static Log log = LogFactory.getLog(BookKeeper.class.getName());
@@ -80,14 +84,30 @@ public class BookKeeper implements com.qubole.rubix.spi.BookKeeperService.Iface
   static long splitSize;
   private RemoteFetchProcessor fetchProcessor;
 
-  public BookKeeper(Configuration conf) throws FileNotFoundException
+  // Registry for gathering & storing necessary metrics
+  private final MetricRegistry metrics;
+
+  // Metrics counter to keep track of the total number of blocks hit
+  private Counter localCacheCount;
+
+  public BookKeeper(Configuration conf, MetricRegistry metrics) throws FileNotFoundException
   {
     this.conf = conf;
+    this.metrics = metrics;
+    initializeMetrics();
     initializeCache(conf);
     fetchProcessor = new RemoteFetchProcessor(conf);
     fetchProcessor.startAsync();
   }
 
+  /**
+   * Initialize the instruments used for gathering desired metrics.
+   */
+  private void initializeMetrics()
+  {
+    localCacheCount = metrics.counter(METRIC_BOOKKEEPER_LOCAL_CACHE_COUNT);
+  }
+
   @Override
   public List<BlockLocation> getCacheStatus(String remotePath, long fileLength, long lastModified, long startBlock, long endBlock, int clusterType)
       throws TException
@@ -140,6 +160,8 @@ public List<BlockLocation> getCacheStatus(String remotePath, long fileLength, lo
     try {
       for (long blockNum = startBlock; blockNum < endBlock; blockNum++) {
         totalRequests++;
+        localCacheCount.inc();
+
         long split = (blockNum * blockSize) / splitSize;
         if (!blockSplits.get(split).equalsIgnoreCase(nodeName)) {
           blockLocations.add(new BlockLocation(Location.NON_LOCAL, blockSplits.get(split)));

diff --git a/rubix-bookkeeper/src/main/java/com/qubole/rubix/bookkeeper/BookKeeperServer.java b/rubix-bookkeeper/src/main/java/com/qubole/rubix/bookkeeper/BookKeeperServer.java
@@ -12,6 +12,7 @@
  */
 package com.qubole.rubix.bookkeeper;
 
+import com.codahale.metrics.MetricRegistry;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Throwables;
 import com.qubole.rubix.spi.BookKeeperService;
@@ -40,6 +41,9 @@ public class BookKeeperServer extends Configured implements Tool
   public static BookKeeper bookKeeper;
   public static BookKeeperService.Processor processor;
 
+  // Registry for gathering & storing necessary metrics
+  private static MetricRegistry metrics;
+
   public static Configuration conf;
 
   private static TServer server;
@@ -72,8 +76,9 @@ public void run()
 
   public static void startServer(Configuration conf)
   {
+    metrics = new MetricRegistry();
     try {
-      bookKeeper = new BookKeeper(conf);
+      bookKeeper = new BookKeeper(conf, metrics);
     }
     catch (FileNotFoundException e) {
       log.error("Cache directories could not be created", e);

diff --git a/rubix-bookkeeper/src/test/java/com/qubole/rubix/bookkeeper/TestBookKeeperMetrics.java b/rubix-bookkeeper/src/test/java/com/qubole/rubix/bookkeeper/TestBookKeeperMetrics.java
@@ -0,0 +1,74 @@
+/**
+ * Copyright (c) 2016. Qubole Inc
+ * 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. See accompanying LICENSE file.
+ */
+package com.qubole.rubix.bookkeeper;
+
+import com.codahale.metrics.MetricRegistry;
+import com.qubole.rubix.spi.CacheConfig;
+import com.qubole.rubix.spi.ClusterType;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.thrift.shaded.TException;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+import static org.testng.Assert.assertEquals;
+
+public class TestBookKeeperMetrics
+{
+  private static final int BLOCK_SIZE = 100;
+
+  private final MetricRegistry metrics = new MetricRegistry();
+  private final Configuration conf = new Configuration();
+
+  private BookKeeper bookKeeper;
+
+  @BeforeClass
+  public void setUp() throws IOException
+  {
+    // Set configuration values for testing
+    CacheConfig.setCacheDataDirPrefix(conf, "/tmp/media/ephemeral");
+    CacheConfig.setMaxDisks(conf, 5);
+    CacheConfig.setBlockSize(conf, BLOCK_SIZE);
+
+    // Create cache directories
+    Files.createDirectories(Paths.get(CacheConfig.getCacheDirPrefixList(conf)));
+    for (int i = 0; i < CacheConfig.getCacheMaxDisks(conf); i++) {
+      Files.createDirectories(Paths.get(CacheConfig.getCacheDirPrefixList(conf) + i));
+    }
+
+    bookKeeper = new BookKeeper(conf, metrics);
+  }
+
+  /**
+   * Verify that the metric representing total block hits is correctly registered & incremented.
+   *
+   * @throws TException when file metadata cannot be fetched or refreshed.
+   */
+  @Test
+  public void verifyBlockHitsMetricIsReported() throws TException
+  {
+    final String remotePath = "/tmp/testPath";
+    final long lastModified = 1514764800; // 2018-01-01T00:00:00
+    final long fileLength = 5000;
+    final long startBlock = 20;
+    final long endBlock = 23;
+    final long totalRequests = endBlock - startBlock;
+
+    assertEquals(metrics.getCounters().get(BookKeeper.METRIC_BOOKKEEPER_LOCAL_CACHE_COUNT).getCount(), 0);
+    bookKeeper.getCacheStatus(remotePath, fileLength, lastModified, startBlock, endBlock, ClusterType.TEST_CLUSTER_MANAGER.ordinal());
+    assertEquals(metrics.getCounters().get(BookKeeper.METRIC_BOOKKEEPER_LOCAL_CACHE_COUNT).getCount(), totalRequests);
+  }
+}
diff --git a/rubix-core/pom.xml b/rubix-core/pom.xml
@@ -40,7 +40,7 @@
         <dependency>
             <groupId>org.testng</groupId>
             <artifactId>testng</artifactId>
-            <version>6.9.8</version>
+            <version>${dep.testng.version}</version>
             <scope>test</scope>
         </dependency>
 

diff --git a/rubix-hadoop1/pom.xml b/rubix-hadoop1/pom.xml
diff --git a/rubix-hadoop2/pom.xml b/rubix-hadoop2/pom.xml
@@ -73,7 +73,7 @@
         <dependency>
             <groupId>org.testng</groupId>
             <artifactId>testng</artifactId>
-            <version>6.9.8</version>
+            <version>${dep.testng.version}</version>
             <scope>test</scope>
         </dependency>
     </dependencies>

diff --git a/rubix-presto/pom.xml b/rubix-presto/pom.xml
@@ -60,7 +60,7 @@
         <dependency>
             <groupId>org.testng</groupId>
             <artifactId>testng</artifactId>
-            <version>6.9.8</version>
+            <version>${dep.testng.version}</version>
             <scope>test</scope>
         </dependency>
         <dependency>

diff --git a/rubix-spi/pom.xml b/rubix-spi/pom.xml
@@ -35,7 +35,7 @@
         <dependency>
             <groupId>org.testng</groupId>
             <artifactId>testng</artifactId>
-            <version>6.9.8</version>
+            <version>${dep.testng.version}</version>
             <scope>test</scope>
         </dependency>
     </dependencies>

diff --git a/rubix-tests/pom.xml b/rubix-tests/pom.xml
@@ -35,10 +35,18 @@
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+            <groupId>io.dropwizard.metrics</groupId>
+            <artifactId>metrics-core</artifactId>
+            <version>${dep.metrics.version}</version>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>org.testng</groupId>
             <artifactId>testng</artifactId>
-            <version>6.9.8</version>
+            <version>${dep.testng.version}</version>
+            <scope>test</scope>
         </dependency>
 
         <dependency>