Skip to content

Commit

Permalink
addressed comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Sourav Maji committed Feb 26, 2025
1 parent 97fc991 commit 7392175
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
import com.linkedin.venice.router.stats.HealthCheckStats;
import com.linkedin.venice.router.stats.LongTailRetryStatsProvider;
import com.linkedin.venice.router.stats.RouteHttpRequestStats;
import com.linkedin.venice.router.stats.RouterHttpRequestStats;
import com.linkedin.venice.router.stats.RouterMetricEntity;
import com.linkedin.venice.router.stats.RouterStats;
import com.linkedin.venice.router.stats.RouterThrottleStats;
Expand Down Expand Up @@ -111,6 +110,7 @@
import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.util.concurrent.DefaultThreadFactory;
import io.tehuti.Metric;
import io.tehuti.metrics.MetricConfig;
import io.tehuti.metrics.MetricsRepository;
import io.tehuti.metrics.Sensor;
Expand Down Expand Up @@ -952,7 +952,9 @@ public ReadOnlySchemaRepository getSchemaRepository() {
}

public double getInFlightRequestRate() {
return RouterHttpRequestStats.getInFlightRequestRate();
Metric metric = localMetricRepo.getMetric(TOTAL_INFLIGHT_REQUEST_COUNT);
// max return -infinity when there are no samples. validate only against finite value
return Double.isFinite(metric.value()) ? metric.value() : 0.0;
}

private void handleExceptionInStartServices(VeniceException e, boolean async) throws VeniceException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ public AggRouterHttpRequestStats(
requestType,
stats,
isKeyValueProfilingEnabled,
totalInFlightRequestSensor,
inflightMetricRepo);
totalInFlightRequestSensor);
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.linkedin.venice.router.stats;

import static com.linkedin.venice.router.RouterServer.TOTAL_INFLIGHT_REQUEST_COUNT;
import static com.linkedin.venice.router.stats.RouterMetricEntity.ABORTED_RETRY_COUNT;
import static com.linkedin.venice.router.stats.RouterMetricEntity.ALLOWED_RETRY_COUNT;
import static com.linkedin.venice.router.stats.RouterMetricEntity.CALL_COUNT;
Expand Down Expand Up @@ -46,7 +45,6 @@
import io.netty.handler.codec.http.HttpResponseStatus;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import io.tehuti.Metric;
import io.tehuti.metrics.MeasurableStat;
import io.tehuti.metrics.MetricsRepository;
import io.tehuti.metrics.Sensor;
Expand Down Expand Up @@ -142,8 +140,7 @@ public RouterHttpRequestStats(
RequestType requestType,
ScatterGatherStats scatterGatherStats,
boolean isKeyValueProfilingEnabled,
Sensor totalInFlightRequestSensor,
VeniceMetricsRepository inflightMetricRepo) {
Sensor totalInFlightRequestSensor) {
super(metricsRepository, storeName, requestType);
VeniceOpenTelemetryMetricsRepository otelRepository = null;
if (metricsRepository instanceof VeniceMetricsRepository) {
Expand Down Expand Up @@ -391,7 +388,6 @@ public RouterHttpRequestStats(

metaStoreShadowReadSensor = registerSensor("meta_store_shadow_read", new OccurrenceRate());
this.totalInFlightRequestSensor = totalInFlightRequestSensor;
this.inflightMetricRepo = inflightMetricRepo;
}

private String getDimensionName(VeniceMetricsDimensions dimension) {
Expand Down Expand Up @@ -649,12 +645,6 @@ private Sensor registerSensorFinal(String sensorName, MeasurableStat... stats) {
return this.registerSensor(sensorName, stats);
}

static public double getInFlightRequestRate() {
Metric metric = inflightMetricRepo.getMetric(TOTAL_INFLIGHT_REQUEST_COUNT);
// max return -infinity when there are no samples. validate only against finite value
return Double.isFinite(metric.value()) ? metric.value() : 0.0;
}

/** used only for testing */
boolean emitOpenTelemetryMetrics() {
return this.emitOpenTelemetryMetrics;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.linkedin.venice.stats.VeniceMetricsRepository;
import com.linkedin.venice.tehuti.MockTehutiReporter;
import com.linkedin.venice.utils.metrics.MetricsRepositoryUtils;
import io.tehuti.Metric;
import io.tehuti.metrics.MetricConfig;
import io.tehuti.metrics.MetricsRepository;
import io.tehuti.metrics.Sensor;
Expand All @@ -30,13 +31,15 @@ public class RouteHttpRequestStatsTest {
private RouteHttpRequestStats stats;
private RouterHttpRequestStats routerHttpRequestStats;

private VeniceMetricsRepository localMetricRepo;

@BeforeSuite
public void setUp() {
MetricsRepository metrics = MetricsRepositoryUtils.createSingleThreadedVeniceMetricsRepository();
reporter = new MockTehutiReporter();
metrics.addReporter(reporter);
MetricConfig metricConfig = new MetricConfig().timeWindow(1, TimeUnit.SECONDS);
VeniceMetricsRepository localMetricRepo = new VeniceMetricsRepository(
localMetricRepo = new VeniceMetricsRepository(
new VeniceMetricsConfig.Builder().setServiceName(ROUTER_SERVICE_NAME)
.setMetricPrefix(ROUTER_SERVICE_METRIC_PREFIX)
.setTehutiMetricConfig(metricConfig)
Expand All @@ -51,8 +54,7 @@ public void setUp() {
RequestType.SINGLE_GET,
mock(ScatterGatherStats.class),
false,
totalInflightRequestSensor,
localMetricRepo);
totalInflightRequestSensor);
}

@Test
Expand All @@ -75,10 +77,16 @@ public void routerMetricsTest() {
@Test
public void routerInFlightMetricTest() {
routerHttpRequestStats.recordIncomingRequest();
Assert.assertTrue(RouterHttpRequestStats.getInFlightRequestRate() > 0.0);
Assert.assertTrue(getInFlightRequestRate() > 0.0);
// After waiting for metric time window, it wil be reset back to 0
waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, true, () -> {
Assert.assertEquals(RouterHttpRequestStats.getInFlightRequestRate(), 0.0);
Assert.assertEquals(getInFlightRequestRate(), 0.0);
});
}

public double getInFlightRequestRate() {
Metric metric = localMetricRepo.getMetric(TOTAL_INFLIGHT_REQUEST_COUNT);
// max return -infinity when there are no samples. validate only against finite value
return Double.isFinite(metric.value()) ? metric.value() : 0.0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public void routerMetricsTest(boolean useVeniceMetricRepository, boolean isOtelE
RequestType.SINGLE_GET,
mock(ScatterGatherStats.class),
false,
null,
null);

if (useVeniceMetricRepository && isOtelEnabled) {
Expand Down

0 comments on commit 7392175

Please sign in to comment.