[client][server] Emit system store metric only for server (linkedin#764)

[server][client] Emit system store metric only for server System store metrics are useful only in server side. This PR prevents emission of meta system store metrics in DaVinci/thin clients. Co-authored-by: Sourav Maji <[email protected]>
m-nagarajan · Nov 28, 2023 · c139a39 · c139a39
1 parent d8b6660
commit c139a39
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 16 deletions.
diff --git a/...s/venice-thin-client/src/main/java/com/linkedin/venice/client/stats/BasicClientStats.java b/...s/venice-thin-client/src/main/java/com/linkedin/venice/client/stats/BasicClientStats.java
@@ -16,6 +16,10 @@
  * This class offers very basic metrics for client, and right now, it is directly used by DaVinci.
  */
 public class BasicClientStats extends AbstractVeniceHttpStats {
+  private static final String SYSTEM_STORE_NAME_PREFIX = "venice_system_store_";
+
+  private static final MetricsRepository dummySystemStoreMetricRepo = new MetricsRepository();
+
   private final Sensor requestSensor;
   private final Sensor healthySensor;
   private final Sensor unhealthySensor;
@@ -38,7 +42,10 @@ public static BasicClientStats getClientStats(
   }
 
   protected BasicClientStats(MetricsRepository metricsRepository, String storeName, RequestType requestType) {
-    super(metricsRepository, storeName, requestType);
+    super(
+        storeName.startsWith(SYSTEM_STORE_NAME_PREFIX) ? dummySystemStoreMetricRepo : metricsRepository,
+        storeName,
+        requestType);
     requestSensor = registerSensor("request", requestRate);
     Rate healthyRequestRate = new OccurrenceRate();
     healthySensor = registerSensor("healthy_request", healthyRequestRate);

diff --git a/...venice-client-common/src/main/java/com/linkedin/venice/stats/AbstractVeniceHttpStats.java b/...venice-client-common/src/main/java/com/linkedin/venice/stats/AbstractVeniceHttpStats.java
@@ -7,13 +7,10 @@
 
 
 public abstract class AbstractVeniceHttpStats extends AbstractVeniceStats {
-  private static final String SYSTEM_STORE_NAME_PREFIX = "venice_system_store_";
-  private static final MetricsRepository dummySystemStoreMetricRepo = new MetricsRepository();
-
   private final RequestType requestType;
 
   public AbstractVeniceHttpStats(MetricsRepository metricsRepository, String storeName, RequestType requestType) {
-    super(storeName.startsWith(SYSTEM_STORE_NAME_PREFIX) ? dummySystemStoreMetricRepo : metricsRepository, storeName);
+    super(metricsRepository, storeName);
     this.requestType = requestType;
   }
 

diff --git a/...ices/venice-server/src/main/java/com/linkedin/venice/listener/HttpChannelInitializer.java b/...ices/venice-server/src/main/java/com/linkedin/venice/listener/HttpChannelInitializer.java
@@ -63,6 +63,8 @@ public class HttpChannelInitializer extends ChannelInitializer<SocketChannel> {
   AggServerQuotaTokenBucketStats quotaTokenBucketStats;
   List<ServerInterceptor> aclInterceptors;
 
+  private boolean isDaVinciClient;
+
   public HttpChannelInitializer(
       ReadOnlyStoreRepository storeMetadataRepository,
       CompletableFuture<HelixCustomizedViewOfflinePushRepository> customizedViewRepository,
@@ -75,6 +77,7 @@ public HttpChannelInitializer(
       StorageReadRequestHandler requestHandler) {
     this.serverConfig = serverConfig;
     this.requestHandler = requestHandler;
+    this.isDaVinciClient = serverConfig.isDaVinciClient();
 
     boolean isKeyValueProfilingEnabled = serverConfig.isKeyValueProfilingEnabled();
     boolean isUnregisterMetricForDeletedStoreEnabled = serverConfig.isUnregisterMetricForDeletedStoreEnabled();
@@ -84,19 +87,22 @@ public HttpChannelInitializer(
         RequestType.SINGLE_GET,
         isKeyValueProfilingEnabled,
         storeMetadataRepository,
-        isUnregisterMetricForDeletedStoreEnabled);
+        isUnregisterMetricForDeletedStoreEnabled,
+        isDaVinciClient);
     this.multiGetStats = new AggServerHttpRequestStats(
         metricsRepository,
         RequestType.MULTI_GET,
         isKeyValueProfilingEnabled,
         storeMetadataRepository,
-        isUnregisterMetricForDeletedStoreEnabled);
+        isUnregisterMetricForDeletedStoreEnabled,
+        isDaVinciClient);
     this.computeStats = new AggServerHttpRequestStats(
         metricsRepository,
         RequestType.COMPUTE,
         isKeyValueProfilingEnabled,
         storeMetadataRepository,
-        isUnregisterMetricForDeletedStoreEnabled);
+        isUnregisterMetricForDeletedStoreEnabled,
+        isDaVinciClient);
 
     if (serverConfig.isComputeFastAvroEnabled()) {
       LOGGER.info("Fast avro for compute is enabled");

diff --git a/services/venice-server/src/main/java/com/linkedin/venice/listener/ListenerService.java b/services/venice-server/src/main/java/com/linkedin/venice/listener/ListenerService.java
@@ -64,6 +64,8 @@ public class ListenerService extends AbstractVeniceService {
 
   private StorageReadRequestHandler storageReadRequestHandler;
 
+  private boolean isDaVinciClient;
+
   public ListenerService(
       StorageEngineRepository storageEngineRepository,
       ReadOnlyStoreRepository storeMetadataRepository,

diff --git a/...ices/venice-server/src/main/java/com/linkedin/venice/stats/AggServerHttpRequestStats.java b/...ices/venice-server/src/main/java/com/linkedin/venice/stats/AggServerHttpRequestStats.java
@@ -16,10 +16,11 @@ public AggServerHttpRequestStats(
       RequestType requestType,
       boolean isKeyValueProfilingEnabled,
       ReadOnlyStoreRepository metadataRepository,
-      boolean unregisterMetricForDeletedStoreEnabled) {
+      boolean unregisterMetricForDeletedStoreEnabled,
+      boolean isDaVinciClient) {
     super(
         metricsRepository,
-        new ServerHttpRequestStatsSupplier(requestType, isKeyValueProfilingEnabled),
+        new ServerHttpRequestStatsSupplier(requestType, isKeyValueProfilingEnabled, isDaVinciClient),
         metadataRepository,
         unregisterMetricForDeletedStoreEnabled);
   }
@@ -28,9 +29,15 @@ static class ServerHttpRequestStatsSupplier implements StatsSupplier<ServerHttpR
     private final RequestType requestType;
     private final boolean isKeyValueProfilingEnabled;
 
-    ServerHttpRequestStatsSupplier(RequestType requestType, boolean isKeyValueProfilingEnabled) {
+    private boolean isDaVinciClient;
+
+    ServerHttpRequestStatsSupplier(
+        RequestType requestType,
+        boolean isKeyValueProfilingEnabled,
+        boolean isDaVinciClient) {
       this.requestType = requestType;
       this.isKeyValueProfilingEnabled = isKeyValueProfilingEnabled;
+      this.isDaVinciClient = isDaVinciClient;
     }
 
     @Override
@@ -48,7 +55,8 @@ public ServerHttpRequestStats get(
           storeName,
           requestType,
           isKeyValueProfilingEnabled,
-          totalStats);
+          totalStats,
+          isDaVinciClient);
     }
   }
 

diff --git a/services/venice-server/src/main/java/com/linkedin/venice/stats/ServerHttpRequestStats.java b/services/venice-server/src/main/java/com/linkedin/venice/stats/ServerHttpRequestStats.java
@@ -59,13 +59,16 @@ public class ServerHttpRequestStats extends AbstractVeniceHttpStats {
   private final Sensor successRequestKeyRatioSensor, successRequestRatioSensor;
   private final Sensor misroutedStoreVersionSensor;
 
+  private static final MetricsRepository dummySystemStoreMetricRepo = new MetricsRepository();
+
   public ServerHttpRequestStats(
       MetricsRepository metricsRepository,
       String storeName,
       RequestType requestType,
       boolean isKeyValueProfilingEnabled,
-      ServerHttpRequestStats totalStats) {
-    super(metricsRepository, storeName, requestType);
+      ServerHttpRequestStats totalStats,
+      boolean isDaVinciClient) {
+    super(isDaVinciClient ? dummySystemStoreMetricRepo : metricsRepository, storeName, requestType);
 
     /**
      * Check java doc of function: {@link TehutiUtils.RatioStat} to understand why choosing {@link Rate} instead of

diff --git a/.../venice-server/src/test/java/com/linkedin/venice/stats/AggServerHttpRequestStatsTest.java b/.../venice-server/src/test/java/com/linkedin/venice/stats/AggServerHttpRequestStatsTest.java
@@ -34,13 +34,15 @@ public void setUp() {
         RequestType.SINGLE_GET,
         false,
         Mockito.mock(ReadOnlyStoreRepository.class),
-        true);
+        true,
+        false);
     this.batchGetStats = new AggServerHttpRequestStats(
         metricsRepository,
         RequestType.MULTI_GET,
         false,
         Mockito.mock(ReadOnlyStoreRepository.class),
-        true);
+        true,
+        false);
   }
 
   @AfterTest