gRPC support request/response size#11833
gRPC support request/response size#11833crossoverJie wants to merge 28 commits intoopen-telemetry:mainfrom
Conversation
# Conflicts: # docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt
| 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
| 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
|
This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 14 days. |
|
Superseded by #14342 |
3 participants
Resolves #9736.
reference https://opentelemetry.io/docs/specs/semconv/rpc/rpc-metrics/#metric-rpcserverrequestsize