gRPC support request/response size #11833
Conversation
# Conflicts: # docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt
...c/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcMetricsAdvice.java
Outdated
Show resolved
Hide resolved
...c/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcMetricsAdvice.java
Outdated
Show resolved
Hide resolved
| default int getClientRequestSize(REQUEST request) { | ||
| return 0; | ||
| } | ||
|
|
||
| default int getClientResponseSize(REQUEST request) { | ||
| return 0; | ||
| } | ||
|
|
||
| default int getServerRequestSize(REQUEST request) { | ||
| return 0; | ||
| } | ||
|
|
||
| default int getServerResponseSize(REQUEST request) { | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
the type for these should be Long, and these should return null by default. We need to consider whether this should be split into separate client/server getters similarly to what we do for http client/server. Would it make sense to introduce a response parameter too? Or perhaps just have getRequestSize and getResponseSize in the getter, we already have separate extractors for client/server these could be used to set the correct attribute.
There was a problem hiding this comment.
Thank for your suggestion, Done.
|
|
||
| Long rpcServerRequestBodySize = | ||
| RpcMessageBodySizeUtil.getRpcServerRequestBodySize(endAttributes, state.startAttributes()); | ||
| if (rpcServerRequestBodySize != null && rpcServerRequestBodySize > 0) { |
There was a problem hiding this comment.
> 0 condition is weird. I think you can get rid of it by changing the type in getter from int -> Long so you can distinguish when getter does not implement this.
| ServerAttributes.SERVER_PORT)); | ||
| } | ||
|
|
||
| static void applyClientRequestSizeAdvice(LongHistogramBuilder builder) { |
There was a problem hiding this comment.
as you are always going to use the same set of attributes you could make all these delegate to a common method or extract the attributes list to a variable
...st/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java
Outdated
Show resolved
Hide resolved
| private static final AttributeKey<Long> RPC_CLIENT_REQUEST_BODY_SIZE = | ||
| AttributeKey.longKey("rpc.client.request.body.size"); | ||
| private static final AttributeKey<Long> RPC_CLIENT_RESPONSE_BODY_SIZE = | ||
| AttributeKey.longKey("rpc.client.response.body.size"); | ||
| private static final AttributeKey<Long> RPC_SERVER_REQUEST_BODY_SIZE = | ||
| AttributeKey.longKey("rpc.server.request.body.size"); | ||
| private static final AttributeKey<Long> RPC_SERVER_RESPONSE_BODY_SIZE = | ||
| AttributeKey.longKey("rpc.server.response.body.size"); |
There was a problem hiding this comment.
in tests you should use RpcIncubatingAttributes
There was a problem hiding this comment.
These attributes do not currently exist in RpcIncubatingAttributes, Maybe we should define them in semantic-conventions
There was a problem hiding this comment.
yes, you should try getting these included in the semantic attributes
Resolves #9736.
reference https://opentelemetry.io/docs/specs/semconv/rpc/rpc-metrics/#metric-rpcserverrequestsize