lightstep / lightstep-tracer-go

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

Mitigates null pointer exception when using HTTP client #166

Closed bg451 closed 6 years ago

bg451 commented 6 years ago

There is currently an issue in the HTTP client where a null httpRequest causes a panic https://github.com/lightstep/lightstep-tracer-go/issues/161. The cause for the null request in the first place is unknown, so the goal for this PR is to mitigate the panic and correctly capture errors returned by (client).Translate.

bg451 commented 6 years ago

It's a little unfortunate that there's a little bit of code duplication from the test_utils_test.go, perhaps there's some work we can do to move a lot of the mocks into a subpackage.

bg451 commented 6 years ago

@joeblubaugh in the interest of getting a patch out to a customer, I'm going to go ahead and merge this. @JulianGriggs and I walked through this code and there's a lot of refactor work that needs to be done in order to get the Flush method in abetter state. I have time carved out at the EOD to deep dive into the code.

The issue comes down to postFlush being fairly complex. It'd be nice to the decouple the piece for going to a not flushing state, creating the status report, and determining whether to retry/clear the buffer.