consider making `request` field in json2.CodecRequest public
mediocregopher opened this issue · 6 comments
I'm using the github.com/gorilla/rpc/v2/json2
package for implementing a json rpc 2 server. I'd like to write a middleware-like wrapper around it to do some logging automatically, since we want that functionality for all of our projects that use an rpc server. There's two ways I could go about doing this, that I can see:
-
Write a proper
http.Handler
wrapper. The problem with this is that I'd have to decode the json at this point, and then within the rpc handler it would be decoding it again, duplicating work. I'd also need a struct to decode into,serverRequest
is private so I'd have to copy-paste that out so my wrapper could see, something I don't want to do. Additionally I'd like to be able to log any rpc errors the server is returning to the client. anhttp.Handler
wrapper would have a lot of difficulty doing this, since it can't get the response's output afaik, andserverResponse
is private besides. -
Write a wrapper around
json2.Codec
andjson2.CodecRequest
which passes everything back to those, but only after logging about it. The problem with this approach is thatjson2.CodecRequest
doesn't expose any information about the request, except for the method name via theMethod()
method.
My solution would be to make the serverRequest
type public, make its instance in CodecRequest
(called request
) public, and add a field onto CodecRequest
which can house the original *http.Request
and is public as well. Doing these things would make writing wrappers around the interface much more approachable.
I'm happy to do the work for this and submit a PR, if the maintainers of this project would be open to the idea. Thanks!
actually, on further review, I don't think it's necessary to add *http.Request
to CodecRequest
, my wrapper around Codec
has access to that so I can pass it along from that point.
It sounds like a reasonable addition. Would be good to see an example of it working before deciding whether it's a good way to do it or not. The wrapper idea sounds promising though.
Sweet! I made a PR with the changes I was envisioning, let me know what you think :)
Thanks. Do you have an example of it being used somewhere? Just in a gist or something would be fine.
Good thing you had me do that, in doing it I realized that it's not actually necessary to change anything on gorilla's side, because the reply
argument to ReadRequest
has the json unmarshalled into it. So All the necessary information is available anyway. I'll go ahead and close this and my PR.
For the future generations who are curious how a wrapper might work, here's the wrapper code, and some example code which uses it: https://gist.github.com/mediocregopher/f35d61ffdec752a90ef4
Cool, thanks for documenting it. Might be good to have in the package docs if I get around to it...