Response/Request Size GRPC Metrics Instrumentation by asweet-confluent · Pull Request #14342 · open-telemetry/opentelemetry-java-instrumentation

asweet-confluent · 2025-07-28T05:46:03Z

Adds rpc.request.body.size and rpc.response.body.size attributes. This is just an updated version of #11833.

asweet-confluent · 2025-07-28T20:20:27Z

+                                    equalTo(RPC_RESPONSE_BODY_SIZE, requestSerializedSize),
+                                    equalTo(RPC_REQUEST_BODY_SIZE, requestSerializedSize),


Does anyone know why the server spans are using request size for rpc.response.body.size and response size for rpc.request.body.size? This is how the original PR was written.

CC @crossoverJie

Sorry, I'm not really sure about the specific reasons, you can check the commit history of this repo.

However, the previous PR was blocked by this PR, maybe it can be re-pushed.

out of curiosity, are the two of you working together on this? I just want to understand why we have two PRs for the same thing and make sure we're all on the same page. I think the reason the other PR stalled is because there were some open questions about semantic conventions, and as @crossoverJie said we might need to revisit and push on that before we can move forward with this.

are the two of you working together on this?

No, the other PR is just very out of date I and I wanted to get this done so I rebased it myself. I assumed it was abandoned :)

I think the reason the other PR stalled is because there were some open questions about semantic conventions, and as @crossoverJie said we might need to revisit and push on that before we can move forward with this.

AFAIK these are already in the semantic conventions:

server

client

AFAIK these are already in the semantic conventions

What you have in the semantic conventions is rpc.server.request.size, rpc.server.response.size, rpc.client.request.size and rpc.client.response.size metrics. Semantic conventions don't seem to define the span attributes rpc.request.body.size and rpc.response.body.size. Either these attributes would need to be added to semantic conventions or the api that is used to produce the metrics would need to be changed so that it could include values that are not span attributes. Our convention is not to emit telemetry that is not in the spec in default configuration.
To me

equalTo(RPC_RESPONSE_BODY_SIZE, requestSerializedSize), equalTo(RPC_REQUEST_BODY_SIZE, requestSerializedSize)

looks like a bug since both the request and response size are the same.

Got it - how about I rip out the code for trace attributes and just leave the metrics?

asweet-confluent · 2025-07-29T14:42:33Z

@jaydeluca looks like the failing tests are dead links unrelated to these changes. I'm still unsure of this though.

otelbot-java-instrumentation · 2025-08-22T02:23:54Z

🔧 The result from spotlessApply was committed to the PR branch.

otelbot-java-instrumentation · 2025-08-22T06:40:22Z

🔧 The result from spotlessApply was committed to the PR branch.

asweet-confluent · 2025-08-22T16:46:34Z

@laurit @jaydeluca I removed the payload size attributes from the traces so we're just left with the metrics. The original implementation worked by adding the attributes to the traces, then using those attributes to record the metrics. It looks like there's no straightforward way for a OperationMetrics to access the requests directly, so I ended up using a context customizer to store the payload size and access it in the Rpc*Metrics classes. Let me know if there's a better approach to this; I had a hard time understanding all the different abstractions at play.

jaydeluca · 2025-08-28T17:32:39Z

+                                point ->
+                                    point
+                                        .hasSum(20 /* bytes */)
+                                        .hasAttributesSatisfying(


Unless there is a reason not to (for example, many unrelated attributes we don't necessarily care about), we typically use the more stricter assertion

Suggested change

.hasAttributesSatisfying(

.hasAttributesSatisfyingExactly(

laurit · 2025-09-04T13:48:40Z

+import io.opentelemetry.context.ImplicitContextKeyed;
+import javax.annotation.Nullable;
+
+public class RpcMetricsHolder implements ImplicitContextKeyed {


@trask an alternative would be to introduce an OperationAttributeExtractor we could call into it in

opentelemetry-java-instrumentation/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java

Lines 250 to 256 in 14a5360

UnsafeAttributes attributes = new UnsafeAttributes();

for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor : attributesExtractors) {

extractor.onEnd(attributes, context, request, response, error);

}

span.setAllAttributes(attributes);

OperationListener[] operationListeners = context.get(START_OPERATION_LISTENERS);

after the span attributes. We could merge the attributes from onEnd with the ones from OperationAttributeExtractor and pass them to the OperationListener. WDYT?

trask · 2025-09-05T04:08:40Z

@laurit @jaydeluca I removed the payload size attributes from the traces so we're just left with the metrics. The original implementation worked by adding the attributes to the traces, then using those attributes to record the metrics. It looks like there's no straightforward way for a OperationMetrics to access the requests directly, so I ended up using a context customizer to store the payload size and access it in the Rpc*Metrics classes. Let me know if there's a better approach to this; I had a hard time understanding all the different abstractions at play.

given the complexity this introduces, what about making both the body size span attributes and body size metrics opt-in?

the RPC semconv stability SIG just kicked off yesterday. I suspect we'll get the body size span attribute into RPC semconv, but I also suspect that both it and the body size metrics will remain experimental when RPC semconv initially goes stable (similar to the state of body size span attributes and metrics in HTTP). So both of them will probably need to be opt-in anyways at some point in the future when this repo adopts the first stable RPC semconv release.

asweet-confluent · 2025-09-11T22:27:00Z

given the complexity this introduces, what about making both the body size span attributes and body size metrics opt-in?

If you're suggesting going back to the original implementation:

adding the attributes to the traces, then using those attributes to record the metrics

Doesn't that mean that enabling the metrics will require enabling the span attributes? Or we'd have to add the span attributes, conditionally generate the metrics from the span attributes, then conditionally remove the span attributes after.

laurit · 2025-09-18T11:38:56Z

@asweet-confluent I update this PR to use an experimental API that allows adding attributes that are used just for operation listeners.

asweet-confluent · 2025-11-06T15:27:18Z

@laurit Thank you so much for pushing this one over the finish line!

crossoverJie and others added 22 commits July 25, 2025 01:32

grpc support request/response size

4516222

fix ci

e8f6b0e

fix ci

01a1bb0

fix ci

6121b30

fix test

9680219

fix ci

cce97f1

fix muzzle

22a791f

fix muzzle

c5c46dd

fix muzzle

d17a36c

fix ci

9f6055d

fix ci

b9f6cee

fix ci

fcc4468

revert ci

6dc7346

revert ci

9b350b4

fix muzzle

69ee2bd

fix muzzle

a10ffca

fix muzzle

e45c225

fix muzzle

bfe9087

use static import asList

43054ad

fix with cr

1438a0a

remove client&server prefix.

063160f

fix tests

d16cccf

asweet-confluent requested a review from a team as a code owner July 28, 2025 05:46

jaydeluca reviewed Jul 28, 2025

View reviewed changes

Comment thread ...test/java/io/opentelemetry/javaagent/instrumentation/armeria/grpc/v1_14/ArmeriaGrpcTest.java Outdated

Comment thread ...test/java/io/opentelemetry/javaagent/instrumentation/armeria/grpc/v1_14/ArmeriaGrpcTest.java Outdated

Fix tests

4387287

asweet-confluent force-pushed the grpc-req-res-size branch from 1256e3d to 4387287 Compare July 28, 2025 20:17

asweet-confluent commented Jul 28, 2025

View reviewed changes

asweet-confluent and others added 2 commits July 28, 2025 13:21

Merge branch 'main' into grpc-req-res-size

3cbfa5a

Remove newlines

484f0e8

./gradlew spotlessApply

bc2abb0

asweet-confluent and others added 2 commits August 21, 2025 23:36

Fix metrics tests

337ea9d

./gradlew spotlessApply

b37fd07

asweet-confluent requested a review from laurit August 22, 2025 16:46

jaydeluca reviewed Aug 28, 2025

View reviewed changes

asweet-confluent added 2 commits August 29, 2025 16:58

PR feedback

cac3159

PR feedback

f168f5f

laurit reviewed Sep 4, 2025

View reviewed changes

fix failing test

eb617be

laurit mentioned this pull request Sep 5, 2025

Add experimental api for adding operation attributes #14590

Merged

laurit added 4 commits September 17, 2025 15:15

Merge branch 'main' into grpc-req-res-size

e1ec0f3

Merge branch 'main' into grpc-req-res-size

4d63ab1

Merge branch 'main' into grpc-req-res-size

2b966ec

Rpc size metrics

fef2b4f

laurit added 4 commits September 18, 2025 15:57

fix latest dep test

0e0e2e1

use reflection to read the message size

db98e02

Merge branch 'main' into grpc-req-res-size

8a7ef80

revert change

5db036f

laurit approved these changes Sep 26, 2025

View reviewed changes

laurit added this to the v2.21.0 milestone Sep 26, 2025

laurit mentioned this pull request Sep 30, 2025

gRPC support request/response size #11833

Closed

jaydeluca approved these changes Oct 16, 2025

View reviewed changes

trask merged commit 5d27f02 into open-telemetry:main Oct 16, 2025
89 checks passed

		equalTo(RPC_RESPONSE_BODY_SIZE, requestSerializedSize),
		equalTo(RPC_REQUEST_BODY_SIZE, requestSerializedSize),

	UnsafeAttributes attributes = new UnsafeAttributes();
	for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor : attributesExtractors) {
	extractor.onEnd(attributes, context, request, response, error);
	}
	span.setAllAttributes(attributes);

	OperationListener[] operationListeners = context.get(START_OPERATION_LISTENERS);

Conversation

asweet-confluent commented Jul 28, 2025

Uh oh!

Uh oh!

Uh oh!

asweet-confluent Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

crossoverJie Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

jaydeluca Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

asweet-confluent Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

laurit Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

asweet-confluent Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

asweet-confluent commented Jul 29, 2025

Uh oh!

otelbot-java-instrumentation Bot commented Aug 22, 2025

Uh oh!

otelbot-java-instrumentation Bot commented Aug 22, 2025

Uh oh!

asweet-confluent commented Aug 22, 2025

Uh oh!

Uh oh!

jaydeluca Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

laurit Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trask commented Sep 5, 2025

Uh oh!

asweet-confluent commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laurit commented Sep 18, 2025

Uh oh!

Uh oh!

asweet-confluent commented Nov 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

laurit Sep 4, 2025 •

edited

Loading

asweet-confluent commented Sep 11, 2025 •

edited

Loading