[Tracing Redesign][2/N] Implement tracing unary Interceptors #2304
Conversation
|
|
| defer span.Finish() | ||
|
|
||
| err := h.Handle(ctx, req, resw) | ||
| if appErrSetter, ok := resw.(interface{ IsApplicationError() bool }); ok { |
There was a problem hiding this comment.
The writer wrapper is probably still the right way to go with, the reasons are:
- We also want to get the
applicationErrorMeta - For the case where yarpc dispatcher is not used (hence obs middleware is not used), the original writer doesn't have
IsApplicationError()method at all.
There was a problem hiding this comment.
Moreover, IsApplicationError() is implemented only for tChannel it seems. So tag value won't be set for other transports.
| @@ -22,10 +22,15 @@ package tracinginterceptor | |||
|
|
|||
| import ( | |||
| "context" | |||
| "go.uber.org/yarpc/transport/tchannel/tracing" | |||
There was a problem hiding this comment.
Please rebase to the fresh dev branch, I added linter for imports.
There was a problem hiding this comment.
This hasn't been addressed in this commit, but I will address it eventually.
| defer span.Finish() | ||
|
|
||
| err := h.Handle(ctx, req, resw) | ||
| if appErrSetter, ok := resw.(interface{ IsApplicationError() bool }); ok { |
There was a problem hiding this comment.
Moreover, IsApplicationError() is implemented only for tChannel it seems. So tag value won't be set for other transports.
| } | ||
| return err | ||
| } | ||
| span.SetTag(errorCodeTag, unknownInternalYarpcError) |
There was a problem hiding this comment.
We have two sort of error code span tags here: 1) rpc.yarpc.status_code, 2) error.code.
And we have 3 conditions here: 1) L198 - yarpc code derivable from err, 2) L204 - application err, 3) L217 - none of above
But they are not consistent:
- condition 1. sets tag 1) and tag 2)
- condition 2. sets tag 1)
- condition 3. sets tag 2)
Let's just just use tag 1) - rpc.yarpc.status_code for all of them. So for condition 3, can we use
span.SetTag(rpcStatusCodeTag, int(yarpcerrors.CodeUnknown))
And for condition 1, let's remove error.code tag.
For this general error.code span tag, let's add it during our ingestion pipeline, and use similar format like muttley error code: https://engwiki.uberinternal.com/display/TE0OBSERVAPPS/RPC+Error+Codes
Added ExtendedResponseWriter, implemented tracing for unary inbounds/outbounds.
This PR introduces the unary interceptors in YARPC Go to improve the structure and functionality of tracing and middleware handling. The primary focus is on separating the unary interceptors to allow for specialized optimizations and cleaner handling of tracing operations.
Key Changes:
Introduced a dedicated UnaryInbound and UnaryOutbound middleware architecture, simplifying the tracing logic for unary requests.
Introduced inbound.go and outbound.go files to reflect the specialized unary middleware design, improving maintainability and reducing complexity.
Enhanced test coverage for the unary path, ensuring that all middleware logic is properly unit tested with specific focus on the flow of unary RPCs.
Updated helper methods for extracting and applying tracing tags to unary requests, ensuring clearer separation of concerns.