Implement otel retry metrics #12064

AgraVator · 2025-05-14T15:45:50Z

implements A96

opentelemetry/src/main/java/io/grpc/opentelemetry/internal/OpenTelemetryConstants.java

opentelemetry/src/main/java/io/grpc/opentelemetry/GrpcOpenTelemetry.java

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java

kannanjgithub · 2025-06-25T13:17:17Z

opentelemetry/src/test/java/io/grpc/opentelemetry/OpenTelemetryMetricsModuleTest.java

+      Optional<MetricData> metric = openTelemetryTesting.getMetrics().stream()
+          .filter(m -> m.getName().equals(metricName))
+          .findFirst();
+      if (metric.isPresent()) {


When will this metric not be present for the operations done by the tested on the tracer?

Whenever they are not enabled or have default values

As this is test code, it is risky to have the assertion inside an if statement. If the absence of the metric is not expected, then it should not cause the test to pass.

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java

AgraVator · 2025-06-30T03:59:16Z

opentelemetry/src/test/java/io/grpc/opentelemetry/OpenTelemetryMetricsModuleTest.java

+      Optional<MetricData> metric = openTelemetryTesting.getMetrics().stream()
+          .filter(m -> m.getName().equals(metricName))
+          .findFirst();
+      if (metric.isPresent()) {


Whenever they are not enabled or have default values

AgraVator · 2025-07-14T06:31:58Z

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java

+      if (info.isTransparentRetry()) {
+        transparentRetriesPerCall.incrementAndGet();
+      } else if (info.isHedging()) {
+        hedgedAttemptsPerCall.incrementAndGet();
+      } else {
        attemptsPerCall.incrementAndGet();


Should this not assume them to be mutually exclusive ?

core/src/main/java/io/grpc/internal/RetriableStream.java

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java

ejona86 · 2025-07-28T17:29:19Z

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java

          return new ClientStreamTracer() {};
        }
        if (++activeStreams == 1 && attemptStopwatch.isRunning()) {
          attemptStopwatch.stop();


This attemptStopwatch looks to be for grpc.client.attempt.duration? This looks busted (pre-existing). It looks like attemptStopwatch is tracking the retry_delay right now, as it only runs when there are no streams. Each stream needs its own attempt stopwatch, right? But right now there is only one per call.

CallAttemptsTracerFactory.attemptEnded is for recording a stream ended (StatsTraceContext.streamClosed calls it), so for the first attempt and each retry attempt end the attemptStopWatch is started, so it does measure the time between stream attempts, not per call.

But there is a problem. RetriableStream creates its own anonymous tracerFactory and not take the one put by MetricsClientInterceptor in the callOptions, so this needs to change.

I have a question. A call can only have 1 stream, and the only way the activeStreams can be > 1 is if the tracer factory is shared between calls (and that would mess up the calculation for time between stream attempts anyway). The way the OpenTelemetryMetricsModule.MetricsClientInterceptor creates the factory though is per call, not shared between calls. So we we can never have > 1 streams and there should be no need for synchronization either.

What I said about RetriableStream's anonymous tracerFactory is not a problem, StatsTraceContext.streamClosed iterates over all tracers and calls stream closed on each of them.

ejona86 · 2025-07-28T17:31:29Z

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java

        }
        if (++activeStreams == 1 && attemptStopwatch.isRunning()) {
          attemptStopwatch.stop();
+          retryDelayNanos = attemptStopwatch.elapsed(TimeUnit.NANOSECONDS);


I can't tell how retryDelayNanos is synchronized.

Its the same as with OpenCensus

implements [A96](https://github.com/grpc/proposal/pull/488/files)

AgraVator requested review from ejona86 and kannanjgithub May 14, 2025 15:45

kannanjgithub reviewed May 19, 2025

View reviewed changes

opentelemetry/src/main/java/io/grpc/opentelemetry/internal/OpenTelemetryConstants.java Outdated Show resolved Hide resolved

kannanjgithub reviewed May 19, 2025

View reviewed changes

opentelemetry/src/main/java/io/grpc/opentelemetry/GrpcOpenTelemetry.java Show resolved Hide resolved

kannanjgithub reviewed May 19, 2025

View reviewed changes

opentelemetry/src/main/java/io/grpc/opentelemetry/GrpcOpenTelemetry.java Show resolved Hide resolved

AgraVator force-pushed the implement-otel-retry-metric branch from a7a16ce to 3529769 Compare May 27, 2025 07:02

AgraVator added 4 commits June 24, 2025 13:37

otel: implement retry metrics

c3b473a

otel: add null checks for retry instrumentation

6c6a0f5

otel: update the retry stats as per gRFC A96

f9c5a68

otel: adds test cases for retry metrics

bd69ed5

AgraVator force-pushed the implement-otel-retry-metric branch from 3529769 to bd69ed5 Compare June 24, 2025 11:38

AgraVator commented Jun 24, 2025

View reviewed changes

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java Outdated Show resolved Hide resolved

fix: formatting fixes

fab9a26

kannanjgithub reviewed Jun 25, 2025

View reviewed changes

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java Show resolved Hide resolved

kannanjgithub reviewed Jun 25, 2025

View reviewed changes

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java Outdated Show resolved Hide resolved

kannanjgithub reviewed Jun 25, 2025

View reviewed changes

opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricsModule.java Show resolved Hide resolved

kannanjgithub reviewed Jun 25, 2025

View reviewed changes

suggested changes

43a746b

AgraVator requested a review from kannanjgithub July 14, 2025 06:58

AgraVator commented Jul 14, 2025

View reviewed changes

ejona86 reviewed Jul 28, 2025

View reviewed changes

Suggested changes

ceb928a

AgraVator requested a review from ejona86 August 13, 2025 11:27

rename isHedging arg

4b9ff17

ejona86 approved these changes Sep 23, 2025

View reviewed changes

AgraVator merged commit d380191 into grpc:master Sep 24, 2025
16 checks passed

AgraVator added a commit to AgraVator/grpc-java that referenced this pull request Sep 26, 2025

Implement otel retry metrics (grpc#12064)

97f3c8a

implements [A96](https://github.com/grpc/proposal/pull/488/files)

AgraVator added a commit to AgraVator/grpc-java that referenced this pull request Sep 26, 2025

Implement otel retry metrics (grpc#12064)

911b117

implements [A96](https://github.com/grpc/proposal/pull/488/files)

AgraVator added a commit to AgraVator/grpc-java that referenced this pull request Oct 1, 2025

Implement otel retry metrics (grpc#12064)

aa95bff

implements [A96](https://github.com/grpc/proposal/pull/488/files)

AgraVator mentioned this pull request Oct 1, 2025

Implement otel retry metrics (v1.76.x backport) #12394

Closed

AgraVator added a commit to AgraVator/grpc-java that referenced this pull request Oct 6, 2025

Implement otel retry metrics (grpc#12064)

50fe415

implements [A96](https://github.com/grpc/proposal/pull/488/files)

Implement otel retry metrics #12064

Implement otel retry metrics #12064

Uh oh!

Conversation

AgraVator commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AgraVator commented May 14, 2025 •

edited

Loading