{"action":"created","issue":{"url":"https://api.github.com/repos/uber/tchannel-go/issues/261","repository_url":"https://api.github.com/repos/uber/tchannel-go","labels_url":"https://api.github.com/repos/uber/tchannel-go/issues/261/labels{/name}","comments_url":"https://api.github.com/repos/uber/tchannel-go/issues/261/comments","events_url":"https://api.github.com/repos/uber/tchannel-go/issues/261/events","html_url":"https://github.com/uber/tchannel-go/issues/261","id":141781448,"number":261,"title":"Mechanism to hook in to generated server code with objects generated at runtime for Request Logging","user":{"login":"IvanMalison","id":1246619,"avatar_url":"https://avatars.githubusercontent.com/u/1246619?v=3","gravatar_id":"","url":"https://api.github.com/users/IvanMalison","html_url":"https://github.com/IvanMalison","followers_url":"https://api.github.com/users/IvanMalison/followers","following_url":"https://api.github.com/users/IvanMalison/following{/other_user}","gists_url":"https://api.github.com/users/IvanMalison/gists{/gist_id}","starred_url":"https://api.github.com/users/IvanMalison/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/IvanMalison/subscriptions","organizations_url":"https://api.github.com/users/IvanMalison/orgs","repos_url":"https://api.github.com/users/IvanMalison/repos","events_url":"https://api.github.com/users/IvanMalison/events{/privacy}","received_events_url":"https://api.github.com/users/IvanMalison/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2016-03-18T05:42:01Z","updated_at":"2016-04-26T21:00:51Z","closed_at":null,"body":"> Hi @IvanMalison, can you give examples of how you expect this to be used?\r\n\r\nSee https://code.uberinternal.com/diffusion/EVGOL/browse/master/requestlogger/request_logger.go;44392961db1b3250fb8d2fdb7caceba9add04db4$85\r\n\r\nBasically, it should be possible to intercept the arguments that are received by an arbitrary call into the Server object in a uniform way. The existing `Handle` interface does not suffice for this because the only interface it provides is a `thrift.TProtocol` object which can only be read once.\r\n\r\n> I'd like to brainstorm to come up with solutions that:\r\n\r\n> don't involve use of empty interfaces, at least not in the generated code\r\n\r\nI'm pretty skeptical of any dogma against empty interfaces. We are already using `athrift.TStruct` which might as well be empty interfaces since they just have serialization methods. \r\n\r\n> similar to above, not have to do type cases.\r\n\r\nI don't think this approach used any type cases. Are you referring to the type assertions that are used once the types are passed back to handleArgs so they can be passed to their respective handlers?\r\n\r\n> no perf impact\r\n\r\n> Ideally none, but minimal changes to the generated code\r\n\r\nThis is a strange constraint.\r\n\r\n> If there needs more changes to the generated code, then thrift-gen supports templates, so extra code can be generated if the user wants, and the normal generated code should have just the minimal hook.\r\n\r\nI'm not sure that templates will suffice in this case, since the generated code needs to have access to objects that are constructed at runtime, elsewhere in the application.\r\n \r\n\r\n--------------------------------------------------\r\nI'm definitely willing to compromise here, but I'm also very skeptical of aggressive performance optimization without benchmarking to back up the idea that performance is a real issue. Do we really think that a signficant portion of a services cpu cycles will be consumed by marshalling between interfaces{} and concrete struct types -- I'd be very surprised to find that the changes that I've made here have even a 10% overall performance impact on even the leanest of services. Premature optimization is the root of all evil and all that stuff. I realize that tchannel-go needs to be quite lean because it is something that will be consumed by absolutely everyone, but for the same reason, it is critically important that the interfaces it provides are convenient and clean. \r\n\r\nAn alternative approach that could work to solve the problem of request logging is to add a Method\r\n\r\n```AddRequestHook(hook func (ctx Context, args interface{}, resp interface{}, unhandled error))```\r\n\r\nto the TChanServer Interface that would populate an instance variable on the server that the generated code would check for and call where appropriate.\r\n\r\n\r\nThe biggest problem with this approach from my perspective is that it makes it much more difficult (possibly impossible) to do something like uncaught exception handling with recover."},"comment":{"url":"https://api.github.com/repos/uber/tchannel-go/issues/comments/214884766","html_url":"https://github.com/uber/tchannel-go/issues/261#issuecomment-214884766","issue_url":"https://api.github.com/repos/uber/tchannel-go/issues/261","id":214884766,"user":{"login":"prashantv","id":140159,"avatar_url":"https://avatars.githubusercontent.com/u/140159?v=3","gravatar_id":"","url":"https://api.github.com/users/prashantv","html_url":"https://github.com/prashantv","followers_url":"https://api.github.com/users/prashantv/followers","following_url":"https://api.github.com/users/prashantv/following{/other_user}","gists_url":"https://api.github.com/users/prashantv/gists{/gist_id}","starred_url":"https://api.github.com/users/prashantv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/prashantv/subscriptions","organizations_url":"https://api.github.com/users/prashantv/orgs","repos_url":"https://api.github.com/users/prashantv/repos","events_url":"https://api.github.com/users/prashantv/events{/privacy}","received_events_url":"https://api.github.com/users/prashantv/received_events","type":"User","site_admin":false},"created_at":"2016-04-26T21:00:50Z","updated_at":"2016-04-26T21:00:50Z","body":"@IvanMalison yeah manually changing it probably the easiest option. The `meta` code is not the same as generated code for a couple of reasons:\r\n - we have to avoid circular dependencies, thrift can't rely on meta, if meta relies on tchannel/thrift\r\n - we have to put the meta TChannel code inside of tchannel/thrift to avoid above issue, but we don't want to export the meta server, so we manually unexport the identifiers.\r\n"}}