search

reddit / baseplate.go

Reddit's Service Framework in Go
https://pkg.go.dev/github.com/reddit/baseplate.go
BSD 3-Clause "New" or "Revised" License
92 stars 77 forks source link

thriftbp: Report request/response payload sizes from client side #650

Closed fishy closed 5 months ago

fishy commented 6 months ago

We currently already have a middleware to report that from the server side, but that's simulated by reconstruct THeaderProtocols and read/write to them, which will not include the header size and will not reflect compressions (zlib, etc.).

This client side reporting is done directly at the TTransport (TSocket) level, so it will reflect the exact number actually read/written at that level. If zlib is enabled in THeaderProtocol the size will reflect compressed payload. The only thing it will not reflect is potential SSL layer (e.g. when using TSSLSocket over TSocket, which we don't support yet).

fishy commented 6 months ago

do we want to make it "more accurate" than the server? That might be confusing...

why not? the server ones always come with the big caveat that they're estimations (see the doccomment here), and also the client metrics will have different labels compared to the server ones.

we can potentially do the same thing on the server side in the future (make the server one more accurate), but that requires open up new apis from upstream thrift (in order to do that we need both of these things at the same time: 1) access to the underlying TTransport; and 2) a clear boundary when a request is finished. we currently have apis for them separately but not an api with access to both).