diff --git a/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/MetricEntityState.java b/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/MetricEntityState.java index 2f9a823122..69aabb428e 100644 --- a/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/MetricEntityState.java +++ b/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/MetricEntityState.java @@ -6,9 +6,12 @@ import io.opentelemetry.api.metrics.LongCounter; import io.tehuti.metrics.MeasurableStat; import io.tehuti.metrics.Sensor; +import io.tehuti.utils.RedundantLogFilter; import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; /** @@ -18,7 +21,9 @@ * 3. multiple tehuti Sensors for this Otel Metric */ public class MetricEntityState { - private MetricEntity metricEntity; + private static final Logger LOGGER = LogManager.getLogger(MetricEntityState.class); + private static final RedundantLogFilter REDUNDANT_LOG_FILTER = RedundantLogFilter.getRedundantLogFilter(); + private final MetricEntity metricEntity; /** Otel metric */ private Object otelMetric = null; /** Map of tehuti names and sensors: 1 Otel metric can cover multiple Tehuti sensors */ @@ -105,6 +110,12 @@ void recordTehutiMetric(TehutiMetricNameEnum tehutiMetricNameEnum, double value) Sensor sensor = tehutiSensors.get(tehutiMetricNameEnum); if (sensor != null) { sensor.record(value); + } else { + // Log using Redundant log filters to catch any bad tehutiMetricNameEnum is passed in + String errorLog = "Tehuti Sensor with name '" + tehutiMetricNameEnum + "' not found."; + if (!REDUNDANT_LOG_FILTER.isRedundantLog(errorLog)) { + LOGGER.error(errorLog); + } } } } @@ -119,7 +130,13 @@ public void record(TehutiMetricNameEnum tehutiMetricNameEnum, double value, Attr recordTehutiMetric(tehutiMetricNameEnum, value); } + /** used only for testing */ Map getTehutiSensors() { return tehutiSensors; } + + /** used only for testing */ + static RedundantLogFilter getRedundantLogFilter() { + return REDUNDANT_LOG_FILTER; + } } diff --git a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/MetricEntityStateTest.java b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/MetricEntityStateTest.java index 3e2248df96..93aeea3749 100644 --- a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/MetricEntityStateTest.java +++ b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/MetricEntityStateTest.java @@ -4,6 +4,8 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; import com.linkedin.venice.stats.VeniceOpenTelemetryMetricsRepository; import io.opentelemetry.api.common.Attributes; @@ -11,6 +13,7 @@ import io.opentelemetry.api.metrics.LongCounter; import io.tehuti.metrics.MeasurableStat; import io.tehuti.metrics.Sensor; +import io.tehuti.utils.RedundantLogFilter; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -26,7 +29,7 @@ public class MetricEntityStateTest { private Sensor mockSensor; private enum TestTehutiMetricNameEnum implements TehutiMetricNameEnum { - TEST_METRIC; + TEST_METRIC_1, TEST_METRIC_2; private final String metricName; @@ -65,19 +68,19 @@ public void testCreateMetricWithOtelEnabled() { @Test public void testAddTehutiSensorsSuccessfully() { MetricEntityState metricEntityState = new MetricEntityState(mockMetricEntity, mockOtelRepository); - metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC, mockSensor); + metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC_1, mockSensor); Assert.assertNotNull(metricEntityState.getTehutiSensors()); - Assert.assertTrue(metricEntityState.getTehutiSensors().containsKey(TestTehutiMetricNameEnum.TEST_METRIC)); + assertTrue(metricEntityState.getTehutiSensors().containsKey(TestTehutiMetricNameEnum.TEST_METRIC_1)); } - @Test(expectedExceptions = IllegalArgumentException.class, expectedExceptionsMessageRegExp = ".*Sensor with name 'TEST_METRIC' already exists.*") + @Test(expectedExceptions = IllegalArgumentException.class, expectedExceptionsMessageRegExp = ".*Sensor with name 'TEST_METRIC_1' already exists.*") public void testAddTehutiSensorThrowsExceptionOnDuplicate() { MetricEntityState metricEntityState = new MetricEntityState(mockMetricEntity, mockOtelRepository); - metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC, mockSensor); + metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC_1, mockSensor); // Adding the same sensor name again should throw an exception - metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC, mockSensor); + metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC_1, mockSensor); } @Test @@ -111,9 +114,9 @@ public void testRecordOtelMetricCounter() { @Test public void testRecordTehutiMetric() { MetricEntityState metricEntityState = new MetricEntityState(mockMetricEntity, mockOtelRepository); - metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC, mockSensor); + metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC_1, mockSensor); - metricEntityState.recordTehutiMetric(TestTehutiMetricNameEnum.TEST_METRIC, 15.0); + metricEntityState.recordTehutiMetric(TestTehutiMetricNameEnum.TEST_METRIC_1, 15.0); verify(mockSensor, times(1)).record(15.0); } @@ -124,13 +127,27 @@ public void testRecordMetricsWithBothOtelAndTehuti() { when(mockMetricEntity.getMetricType()).thenReturn(MetricType.HISTOGRAM); MetricEntityState metricEntityState = new MetricEntityState(mockMetricEntity, mockOtelRepository); + RedundantLogFilter logFilter = metricEntityState.getRedundantLogFilter(); metricEntityState.setOtelMetric(doubleHistogram); - metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC, mockSensor); + metricEntityState.addTehutiSensors(TestTehutiMetricNameEnum.TEST_METRIC_1, mockSensor); Attributes attributes = Attributes.builder().put("key", "value").build(); - metricEntityState.record(TestTehutiMetricNameEnum.TEST_METRIC, 20.0, attributes); + // case 1: check using valid Tehuti metric that was added to metricEntityState + metricEntityState.record(TestTehutiMetricNameEnum.TEST_METRIC_1, 20.0, attributes); verify(doubleHistogram, times(1)).record(20.0, attributes); verify(mockSensor, times(1)).record(20.0); + assertFalse(logFilter.isRedundantLog("Tehuti Sensor with name 'TEST_METRIC_1' not found.", false)); + + // case 2: check using a Tehuti metric that was not added to metricEntityState and verify it called + // REDUNDANT_LOG_FILTER + metricEntityState.record(TestTehutiMetricNameEnum.TEST_METRIC_2, 20.0, attributes); + // otel metric should be called for the second time + verify(doubleHistogram, times(2)).record(20.0, attributes); + // Tehuti metric should be not called for the second time as we passed in an invalid metric name + verify(mockSensor, times(1)).record(20.0); + // This should have invoked the log filter for TEST_METRIC_2 + assertFalse(logFilter.isRedundantLog("Tehuti Sensor with name 'TEST_METRIC_1' not found.", false)); + assertTrue(logFilter.isRedundantLog("Tehuti Sensor with name 'TEST_METRIC_2' not found.", false)); } } diff --git a/services/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterHttpRequestStats.java b/services/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterHttpRequestStats.java index 93fef88a5b..9c7c95baa3 100644 --- a/services/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterHttpRequestStats.java +++ b/services/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterHttpRequestStats.java @@ -363,11 +363,14 @@ public void recordIncomingRequest() { } public void recordHealthyRequest(Double latency, HttpResponseStatus responseStatus) { - TehutiMetricNameEnum tehutiMetricNameEnum = RouterTehutiMetricNameEnum.HEALTHY_REQUEST; VeniceResponseStatusCategory veniceResponseStatusCategory = VeniceResponseStatusCategory.HEALTHY; - recordRequestMetric(tehutiMetricNameEnum, responseStatus, veniceResponseStatusCategory); + recordRequestMetric(RouterTehutiMetricNameEnum.HEALTHY_REQUEST, responseStatus, veniceResponseStatusCategory); if (latency != null) { - recordLatencyMetric(tehutiMetricNameEnum, latency, responseStatus, veniceResponseStatusCategory); + recordLatencyMetric( + RouterTehutiMetricNameEnum.HEALTHY_REQUEST_LATENCY, + latency, + responseStatus, + veniceResponseStatusCategory); } } @@ -400,10 +403,13 @@ public void recordReadQuotaUsage(int quotaUsage) { } public void recordTardyRequest(double latency, HttpResponseStatus responseStatus) { - TehutiMetricNameEnum tehutiMetricNameEnum = RouterTehutiMetricNameEnum.TARDY_REQUEST; VeniceResponseStatusCategory veniceResponseStatusCategory = VeniceResponseStatusCategory.TARDY; - recordRequestMetric(tehutiMetricNameEnum, responseStatus, veniceResponseStatusCategory); - recordLatencyMetric(tehutiMetricNameEnum, latency, responseStatus, veniceResponseStatusCategory); + recordRequestMetric(RouterTehutiMetricNameEnum.TARDY_REQUEST, responseStatus, veniceResponseStatusCategory); + recordLatencyMetric( + RouterTehutiMetricNameEnum.TARDY_REQUEST_LATENCY, + latency, + responseStatus, + veniceResponseStatusCategory); } public void recordThrottledRequest(double latency, HttpResponseStatus responseStatus) { @@ -465,7 +471,10 @@ public void recordBadRequest(HttpResponseStatus responseStatus) { } public void recordBadRequestKeyCount(int keyCount) { - recordKeyCountMetric(keyCount, RequestValidationOutcome.INVALID_KEY_COUNT_LIMIT_EXCEEDED); + recordKeyCountMetric( + RouterTehutiMetricNameEnum.BAD_REQUEST_KEY_COUNT, + keyCount, + RequestValidationOutcome.INVALID_KEY_COUNT_LIMIT_EXCEEDED); } public void recordRequestThrottledByRouterCapacity() { @@ -548,10 +557,13 @@ public void recordFindUnhealthyHostRequest() { } public void recordKeyNum(int keyNum) { - recordKeyCountMetric(keyNum, RequestValidationOutcome.VALID); + recordKeyCountMetric(RouterTehutiMetricNameEnum.KEY_NUM, keyNum, RequestValidationOutcome.VALID); } - public void recordKeyCountMetric(int keyNum, RequestValidationOutcome outcome) { + public void recordKeyCountMetric( + TehutiMetricNameEnum tehutiMetricNameEnum, + int keyNum, + RequestValidationOutcome outcome) { Attributes dimensions = null; if (emitOpenTelemetryMetrics) { dimensions = Attributes.builder() @@ -559,7 +571,7 @@ public void recordKeyCountMetric(int keyNum, RequestValidationOutcome outcome) { .put(getDimensionName(VENICE_REQUEST_VALIDATION_OUTCOME), outcome.getOutcome()) .build(); } - keyCountMetric.record(RouterTehutiMetricNameEnum.KEY_NUM, keyNum, dimensions); + keyCountMetric.record(tehutiMetricNameEnum, keyNum, dimensions); } public void recordRequestUsage(int usage) {