search

lightstep / lightstep-tracer-go

The Lightstep distributed tracing library for Go
https://lightstep.com
MIT License
98 stars 54 forks source link

Fix/span drops message larger than max #277

Closed szuecs closed 3 years ago

szuecs commented 3 years ago

Fix #275 and likely also #273.

I tested with big span sizes, that had to be split into 2-5 chunks and 1k rps constant traffic a local modified build of skipper, that used the proposed modification. The algorithm to compute the number of parts is: pick 2 items at random and use the bigger one to compute the heuristic of average span size, that will be multiplied by number of spans. This algorithm shows good results (at 12:30 the patched skipper with the 2-at-random was deployed, before I tried 1-at-random):

image

szuecs commented 3 years ago

It would be great if you can guide me fixing the tests, because half of them are broken and I think there are assumptions in tests that hold not true anymore. Basically right now Flush() returns fast in case there are no rawSpans and it seems 50% of the tests rely that Flush() does something without having rawSpans.

codeboten commented 3 years ago

hey @szuecs, I created a PR to your fork https://github.com/szuecs/lightstep-tracer-go/pull/1. The change fixes one issue which was breaking some of the tests where parts was never set if heuristicByteSize was not greater than tracer.opts.GRPCMaxCallSendMsgSizeBytes

I've also restored the behaviour to go through Flush for now even when there are no rawSpans. Will need to do a bit more digging on our end to see if removing this impacts anything else.

szuecs commented 3 years ago

Thanks @codeboten !