[router]Integrate router request_size, response_size and key_size metrics to otel #1939

namithanivead · 2025-07-17T05:07:37Z

Problem Statement

Add router request_size, response_size and key_size metrics in OTel

Solution

This change add

CALL_SIZE metrics in Otel with dimensions request and response sizes. Along with this tehuti metrics request_size, response_size will also be updated
KEY_SIZE to measure the keysize in bytes when KeyValueProfilingEnabled

Code changes

Added new code behind a config. If so list the config names and their default values in the PR description.
Introduced new log lines.
- Confirmed if logs need to be rate limited to avoid excessive logging.

Concurrency-Specific Checks

Both reviewer and PR author to verify

Code has no race conditions or thread safety issues.
Proper synchronization mechanisms (e.g., synchronized, RWLock) are used where needed.
No blocking calls inside critical sections that could lead to deadlocks or performance degradation.
Verified thread-safe collections are used (e.g., ConcurrentHashMap, CopyOnWriteArrayList).
Validated proper exception handling in multi-threaded code to avoid silent thread termination.

How was this PR tested?

[ X] New unit tests added.
New integration tests added.
Modified or extended existing tests.
Verified backward compatibility (if applicable).

Does this PR introduce any user-facing or breaking changes?

[ X] No. You can skip the rest of this section.
Yes. Clearly explain the behavior change and its impact.

m-nagarajan · 2025-07-17T18:57:03Z

...ces/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterHttpRequestStats.java

@@ -101,7 +103,8 @@ public class RouterHttpRequestStats extends AbstractVeniceHttpStats {
  private final Sensor badRequestKeyCountSensor;

  /** OTel metrics yet to be added */


Nit: Move this comment to be below the requestSizeMetric metric.

m-nagarajan · 2025-07-17T18:57:48Z

...ces/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterHttpRequestStats.java

+        CALL_SIZE.getMetricEntity(),
+        otelRepository,
+        this::registerSensorFinal,
+        RouterTehutiMetricNameEnum.REQUEST_SIZE,


We will add RESPONSE_SIZE to this PR as well right?

m-nagarajan · 2025-07-17T18:59:13Z

...ces/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterHttpRequestStats.java

+        otelRepository,
+        this::registerSensorFinal,
+        RouterTehutiMetricNameEnum.REQUEST_SIZE,
+        singletonList(new Avg()),


Original requestSizeSensor was not just Avg()

it had TehutiUtils.getPercentileStat(getName(), getFullMetricName("request_size")), new Avg()

…tion

lluwm · 2025-07-18T22:54:05Z

...ces/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterHttpRequestStats.java

@@ -179,6 +184,8 @@ public RouterHttpRequestStats(
    Rate requestRate = new OccurrenceRate();
    Rate healthyRequestRate = new OccurrenceRate();
    Rate tardyRequestRate = new OccurrenceRate();
+    SampledStat requestSize = new Avg();


Is requestSize used somewhere else? If not, I think we can remove the local variable.

lluwm · 2025-07-18T22:59:04Z

services/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterMetricEntity.java

+   * Size of request and response in bytes
+   */
+  CALL_SIZE(
+      MetricType.HISTOGRAM, MetricUnit.NUMBER, "Size of request and response in bytes",


@m-nagarajan , probably a question for you, do we have to define a new unit for bytes here?

added new unit for BYTES

lluwm · 2025-07-18T23:01:14Z

...venice-router/src/test/java/com/linkedin/venice/router/stats/RouterHttpRequestStatsTest.java

+    assertEquals(
+        metricsRepository.getMetric("." + storeName + "--" + REQUEST_SIZE.getMetricName() + ".Avg").value(),
+        512.0);


Looks like this only tests the tehuti metrics part, can we also add a test to verify the correctness of the OTEL metrics?

…tion

…ing is enabled

…sts are passing now

lluwm · 2025-08-01T21:22:16Z

...ces/venice-router/src/main/java/com/linkedin/venice/router/stats/RouterHttpRequestStats.java

+    if (isKeyValueProfilingEnabled) {
+      if (storeName.equals(STORE_NAME_FOR_TOTAL_STAT)) {


I don't quite follow why we changed the condition here?

The condition was updated to handle metric creation correctly based on the type of profiling enabled (isKeyValueProfilingEnabled) and the specific store being monitored (storeName).
When isKeyValueProfilingEnabled is true:

If storeName.equals(STORE_NAME_FOR_TOTAL_STAT), we only want to emit Tehuti metrics (not OTEL). So, we create the metrics the same way, but pass null for OTEL to ensure only Tehuti sensors are registered.

For other stores (i.e., storeName != STORE_NAME_FOR_TOTAL_STAT), we only want to emit OTEL metrics (not Tehuti), so we skip the Tehuti sensor setup.

When isKeyValueProfilingEnabled is false:

We fallback to the default behavior: emit OTEL metrics for response size only (no key size sensors).

nvijayak added 5 commits July 16, 2025 09:35

integrate request_size metrics to otel

2cbef02

remove wildcard

4f475a1

remove requestSizeSensor

c558c68

Add TehutiMetricNameEnum

57c6abd

fixing the tests

212da67

namithanivead marked this pull request as draft July 17, 2025 05:07

m-nagarajan reviewed Jul 17, 2025

View reviewed changes

nvijayak and others added 2 commits July 18, 2025 10:28

address the PR comments

3709d52

Merge branch 'linkedin:main' into nvijayak/router_OtelMetrics_integra…

0959185

…tion

lluwm reviewed Jul 18, 2025

View reviewed changes

namithanivead and others added 3 commits July 18, 2025 20:47

Merge branch 'linkedin:main' into nvijayak/router_OtelMetrics_integra…

c3f92c8

…tion

Merge branch 'linkedin:main' into nvijayak/router_OtelMetrics_integra…

38593dc

…tion

add response size and key size metrics to Otel

c7c195f

namithanivead changed the title ~~[router]Integrate router request_size metrics to otel~~ [router]Integrate router request_size, response_size and key_size metrics to otel Jul 21, 2025

namithanivead and others added 12 commits July 23, 2025 10:39

Merge branch 'linkedin:main' into nvijayak/router_OtelMetrics_integra…

8b5909b

…tion

record responseSizeMetrics when keyvalueprofiling is enabled

ed42e99

Merge branch 'linkedin:main' into nvijayak/router_OtelMetrics_integra…

4faee98

…tion

Merge branch 'linkedin:main' into nvijayak/router_OtelMetrics_integra…

8ff9464

…tion

modify the test case to verify key size is recorded if keyValueProfil…

30d2fa8

…ing is enabled

remove unnecessary semicolon

1805015

fix the failing tests

a7402ce

fix the failing tests

04f96da

fix the failing tests

0c939ed

Corrected the tehuti metrics name for Keysize to key_size_in_byte. te…

e0bd038

…sts are passing now

addressing the PR comment

15c75b5

add unit test cases

68ee863

namithanivead marked this pull request as ready for review August 1, 2025 18:24

lluwm reviewed Aug 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[router]Integrate router request_size, response_size and key_size metrics to otel #1939

[router]Integrate router request_size, response_size and key_size metrics to otel #1939

Uh oh!

namithanivead commented Jul 17, 2025 •

edited

Loading

Uh oh!

m-nagarajan Jul 17, 2025

Uh oh!

m-nagarajan Jul 17, 2025

Uh oh!

m-nagarajan Jul 17, 2025

Uh oh!

lluwm Jul 18, 2025

Uh oh!

lluwm Jul 18, 2025

Uh oh!

namithanivead Aug 1, 2025

Uh oh!

lluwm Jul 18, 2025

Uh oh!

lluwm Aug 1, 2025

Uh oh!

namithanivead Aug 2, 2025

Uh oh!

Uh oh!

		@@ -101,7 +103,8 @@ public class RouterHttpRequestStats extends AbstractVeniceHttpStats {
		private final Sensor badRequestKeyCountSensor;

		/** OTel metrics yet to be added */

		if (isKeyValueProfilingEnabled) {
		if (storeName.equals(STORE_NAME_FOR_TOTAL_STAT)) {

[router]Integrate router request_size, response_size and key_size metrics to otel #1939

Are you sure you want to change the base?

[router]Integrate router request_size, response_size and key_size metrics to otel #1939

Uh oh!

Conversation

namithanivead commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem Statement

Solution

Code changes

Concurrency-Specific Checks

How was this PR tested?

Does this PR introduce any user-facing or breaking changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

namithanivead commented Jul 17, 2025 •

edited

Loading