fishy commented on pull request #1992:
URL: https://github.com/apache/thrift/pull/1992#issuecomment-705725300
@voodoo-dn So this is not as low level as I expected (e.g. it still cannot
get the size on the wire for the requests and responses). The only thing this
can do and middlewares can't are getting the requests and responses, but that
comes with a price.
In your interceptor implementations both requests and responses are
`interface{}`, e.g. typeless. Why can they not be `thrift.TStruct` instead?
What are the cases they are not actually `thrift.TStruct`? `interface{}` is
something I want to avoid as much as possible. If you look at the current
thrift library code, we only use it in `TSimpleJSONProtocol`, `sync.Pool`, and
test codes:
```
$ git grep "interface{}"
debug_protocol.go:func (tdp *TDebugProtocol) logf(format string, v
...interface{}) {
deserializer.go: New: func() interface{} {
deserializer.go: New: func() interface{} {
http_transport.go: New: func() interface{} {
json_protocol_test.go: str1 := new([]interface{})
json_protocol_test.go: str1 := new([]interface{})
serializer.go: New: func() interface{} {
serializer.go: New: func() interface{} {
simple_json_protocol.go:func (p *TSimpleJSONProtocol) readSingleValue()
(interface{}, TType, error) {
simple_json_protocol.go: return make([]interface{},
0), LIST, NewTProtocolException(e)
simple_json_protocol.go: return
make(map[string]interface{}), STRUCT, NewTProtocolException(e)
simple_json_protocol_test.go: str1 := new([]interface{})
simple_json_protocol_test.go: str1 := new([]interface{})
```
And in thrift compiled go code we only use it for the `Scan` functions
generated for the thrift defined types.
Middlewares can do everything in the list you mentioned, but without the
requests and responses. And looking at your example code, they are only used in
logging. So it seems to me that they are not something required to function,
but more like nice to have things.
So to summarize my impression is that this is a relatively high price to pay
(with compiler changes and weaker type system) for a relatively low gain (as I
mentioned in [the discussion in the
ticket](https://issues.apache.org/jira/browse/THRIFT-5164?focusedCommentId=17134411&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17134411),
it's doable in middleware as well, just more complex). I'm also doubtful of
the usefulness of blindly logging all requests/responses for every request in
every endpoint.
Also I'm curious: how do you implement your `maskArg` function so it works
for all your different request types? (Serhii mentioned that they have 100
endpoints, and I assume they are from the same company as you?)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
