[server] Fix stats handling for SINGLE_GET requests (#1381)

In a previous commit, when removing some metrics, we ended up removing some key metrics about key and value size values for single_get requests as well. That change also led to NPEs as the caller expected the K/V stats to always be recorded for Single-get requests. This commit adds Avg and Max stats by default for key and value sizes in single get requests.

services/venice-server/src/main/java/com/linkedin/venice/stats/ServerHttpRequestStats.java

@@ -295,23 +295,33 @@ public ServerHttpRequestStats(
         () -> totalStats.earlyTerminatedEarlyRequestCountSensor,
         new OccurrenceRate());
 
-    if (isKeyValueProfilingEnabled) {
-      // size profiling is expensive
+    if (isKeyValueProfilingEnabled || requestType == RequestType.SINGLE_GET) {
+      final MeasurableStat[] valueSizeStats;
+      final MeasurableStat[] keySizeStats;
       String requestValueSizeSensorName = "request_value_size";
+      String requestKeySizeSensorName = "request_key_size";
+      if (isKeyValueProfilingEnabled) {
+        valueSizeStats = TehutiUtils
+            .getFineGrainedPercentileStatWithAvgAndMax(getName(), getFullMetricName(requestValueSizeSensorName));
+        keySizeStats = TehutiUtils
+            .getFineGrainedPercentileStatWithAvgAndMax(getName(), getFullMetricName(requestKeySizeSensorName));
+      } else {
+        valueSizeStats = new MeasurableStat[] { new Avg(), new Max() };
+        keySizeStats = new MeasurableStat[] { new Avg(), new Max() };
+      }
+
       requestValueSizeSensor = registerPerStoreAndTotal(
           requestValueSizeSensorName,
           totalStats,
           () -> totalStats.requestValueSizeSensor,
-          TehutiUtils
-              .getFineGrainedPercentileStatWithAvgAndMax(getName(), getFullMetricName(requestValueSizeSensorName)));
-      String requestKeySizeSensorName = "request_key_size";
+          valueSizeStats);
       requestKeySizeSensor = registerPerStoreAndTotal(
           requestKeySizeSensorName,
           totalStats,
           () -> totalStats.requestKeySizeSensor,
-          TehutiUtils
-              .getFineGrainedPercentileStatWithAvgAndMax(getName(), getFullMetricName(requestKeySizeSensorName)));
+          keySizeStats);
     }
+
     misroutedStoreVersionSensor = registerPerStoreAndTotal(
         "misrouted_store_version_request_count",
         totalStats,

services/venice-server/src/test/java/com/linkedin/venice/stats/AggServerHttpRequestStatsTest.java

@@ -13,10 +13,13 @@
 
 
 public class AggServerHttpRequestStatsTest {
-  protected MetricsRepository metricsRepository;
-  protected MockTehutiReporter reporter;
-  protected AggServerHttpRequestStats singleGetStats;
-  protected AggServerHttpRequestStats batchGetStats;
+  private MetricsRepository metricsRepository;
+  private MockTehutiReporter reporter;
+  private MetricsRepository metricsRepositoryForKVProfiling;
+  private MockTehutiReporter reporterForKVProfiling;
+  private AggServerHttpRequestStats singleGetStats;
+  private AggServerHttpRequestStats singleGetStatsWithKVProfiling;
+  private AggServerHttpRequestStats batchGetStats;
 
   private static final String STORE_FOO = "store_foo";
   private static final String STORE_BAR = "store_bar";
@@ -26,14 +29,21 @@ public class AggServerHttpRequestStatsTest {
   @BeforeTest
   public void setUp() {
     this.metricsRepository = new MetricsRepository();
-    Assert.assertEquals(metricsRepository.metrics().size(), 0);
     this.reporter = new MockTehutiReporter();
     this.metricsRepository.addReporter(reporter);
+
+    this.metricsRepositoryForKVProfiling = new MetricsRepository();
+    this.reporterForKVProfiling = new MockTehutiReporter();
+    this.metricsRepositoryForKVProfiling.addReporter(reporterForKVProfiling);
+
+    Assert.assertEquals(metricsRepository.metrics().size(), 0);
+    Assert.assertEquals(metricsRepositoryForKVProfiling.metrics().size(), 0);
+
     this.singleGetStats = new AggServerHttpRequestStats(
         "test_cluster",
         metricsRepository,
         RequestType.SINGLE_GET,
-        true,
+        false,
         Mockito.mock(ReadOnlyStoreRepository.class),
         true,
         false);
@@ -45,18 +55,31 @@ public void setUp() {
         Mockito.mock(ReadOnlyStoreRepository.class),
         true,
         false);
+
+    this.singleGetStatsWithKVProfiling = new AggServerHttpRequestStats(
+        "test_cluster",
+        metricsRepositoryForKVProfiling,
+        RequestType.SINGLE_GET,
+        true,
+        Mockito.mock(ReadOnlyStoreRepository.class),
+        true,
+        false);
   }
 
   @AfterTest
   public void cleanUp() {
     metricsRepository.close();
+    metricsRepositoryForKVProfiling.close();
     reporter.close();
+    reporterForKVProfiling.close();
   }
 
   @Test
   public void testMetrics() {
     ServerHttpRequestStats singleGetServerStatsFoo = singleGetStats.getStoreStats(STORE_FOO);
     ServerHttpRequestStats singleGetServerStatsBar = singleGetStats.getStoreStats(STORE_BAR);
+    ServerHttpRequestStats singleGetServerStatsWithKvProfilingFoo =
+        singleGetStatsWithKVProfiling.getStoreStats(STORE_FOO);
 
     singleGetServerStatsFoo.recordSuccessRequest();
     singleGetServerStatsFoo.recordSuccessRequest();
@@ -66,6 +89,9 @@ public void testMetrics() {
     singleGetServerStatsFoo.recordKeySizeInByte(100);
     singleGetServerStatsFoo.recordValueSizeInByte(1000);
 
+    singleGetServerStatsWithKvProfilingFoo.recordKeySizeInByte(100);
+    singleGetServerStatsWithKvProfilingFoo.recordValueSizeInByte(1000);
+
     Assert.assertTrue(
         reporter.query("." + STORE_FOO + "--success_request.OccurrenceRate").value() > 0,
         "success_request rate should be positive");
@@ -75,15 +101,34 @@ public void testMetrics() {
     Assert.assertTrue(
         reporter.query(".total--success_request_ratio.RatioStat").value() > 0,
         "success_request_ratio should be positive");
+    Assert.assertTrue(
+        reporter.query(".store_foo--request_key_size.Avg").value() > 0,
+        "Avg value for request key size should always be recorded");
+    Assert.assertTrue(
+        reporter.query(".store_foo--request_key_size.Max").value() > 0,
+        "Max value for request key size should always be recorded");
 
-    String[] percentileStrings = new String[] { "0_01", "0_01", "0_1", "1", "2", "3", "4", "5", "10", "20", "30", "40",
-        "50", "60", "70", "80", "90", "95", "99", "99_9" };
+    Assert.assertTrue(
+        reporterForKVProfiling.query(".store_foo--request_key_size.Avg").value() > 0,
+        "Avg value for request key size should always be recorded");
+    Assert.assertTrue(
+        reporterForKVProfiling.query(".store_foo--request_key_size.Max").value() > 0,
+        "Max value for request key size should always be recorded");
+
+    String[] fineGrainedPercentiles = new String[] { "0_01", "0_01", "0_1", "1", "2", "3", "4", "5", "10", "20", "30",
+        "40", "50", "60", "70", "80", "90", "95", "99", "99_9" };
+    for (String fineGrainedPercentile: fineGrainedPercentiles) {
+      Assert.assertNull(
+          metricsRepository.getMetric(".store_foo--request_key_size." + fineGrainedPercentile + "thPercentile"));
+      Assert.assertNull(
+          metricsRepository.getMetric(".store_foo--request_value_size." + fineGrainedPercentile + "thPercentile"));
 
-    for (int i = 0; i < percentileStrings.length; i++) {
       Assert.assertTrue(
-          reporter.query(".store_foo--request_key_size." + percentileStrings[i] + "thPercentile").value() > 0);
+          reporterForKVProfiling.query(".store_foo--request_key_size." + fineGrainedPercentile + "thPercentile")
+              .value() > 0);
       Assert.assertTrue(
-          reporter.query(".store_foo--request_value_size." + percentileStrings[i] + "thPercentile").value() > 0);
+          reporterForKVProfiling.query(".store_foo--request_value_size." + fineGrainedPercentile + "thPercentile")
+              .value() > 0);
     }
 
     singleGetStats.handleStoreDeleted(STORE_FOO);