Open achille-roussel opened 6 years ago
Hi @achille-roussel, thanks for doing the timing measurements while reporting this issue. We do still need to support all three transports, but maybe we can move configuration of the transport from run-time to compile time to avoid the large build size for users who don't need it.
We've been batting around one approach here and I'd be interested to hear your opinion on it.
We'd create a Transport
interface or similar for the Tracer, and move the code that implements this transport interface into independent packages in this library. Initializing the tracer would require a type that implements the Transport interface, which could be imported from one of these packages. Perhaps we'd depend on the HTTP client by default because it has the smallest footprint.
You're right that there are probably other cleanups that we could make that would impact overall LoC size - replacing all instances of x/net/context
now that the standard library includes it. I think we would be very open to getting specific issues on the tracer for those individual changes. I think the scope is a little large to discuss them all on this one issue.
Sorry I missed that you had responded here.
Decoupling the tracer and transport logic with some kind of Transport
interface (similar to what the net/http
package does with Client
and Transport
) seems like a really good move. It means programs could even provide their own, or wrap the transport to do metric collection or logging at this level, which can be extremely useful for debugging purposes.
I've been collecting more performance profiles on the lightstep tracer recently, I'll open issues/PRs for individual changes.
There's still room for improvement but it's making progress with the two PR you merged.
Before:
-rwxr-xr-x 1 achille achille 21285265 Oct 23 22:16 netc
After
-rwxr-xr-x 1 achille achille 21007845 Oct 23 22:17 netc
Without the lightstep client:
-rwxr-xr-x 1 achille achille 17628361 Oct 23 22:20 netc
Building with Go 1.11 has helped as well, when I initially opened this issue the binary size was 24MB (built with Go 1.10), it went down to 21MB after upgrading the compiler.
Hello, I've recently integrated a program with lightstep using the lightstep-tracer-go package and the size of the compiled binary went from 17MB to 24MB and build time increased by over 2x. It seems like the package has lots of dependencies, to give an idea of the situation:
Some of those dependencies are sub-packages of grpc and others but still... it's quite a lot.
What's most problematic about this is a lot of this code is unused in the resulting application (because the program will make use of either grpc, thrift, or http to send spans to the collectors), but the compiler cannot eliminate this code since it's not dead code, because the selection of the protocol happens at runtime through the options passed to the tracer when it's created.
Here's an example showing the cost of integrating lightstep-tracer-go in one of or production services (measurements done on OSX 17.5.0 with Go 1.10).
The main problem with slowing down build time is it gets in the way of developer productivity, more than doubling is a significant cost that we pay when attempting to iterate quickly on a project. The example I'm giving you is from a project that has already quite a bit of code, to put things in perspective here is the number of lines there were before adding lightstep-tracer-go (but including all other dependencies):
Then here is the number of lines of code in lightstep-tracer-go (including this package's dependencies):
So adding tracing with lightstep here is doubling the total line count of the program, which aligns with the 2x increase in build time in this case.
I totally get that adding new features requires pulling in a bunch of code, but looking at your implementation it seems like lots could be done to make things better. For example, you're using importing
golang.org/x/net/http2
to use HTTP2 over TLS, but my understanding is this is supported by the standard library already and could avoid taking a dependency (I'm totally missing internal context tho so feel free to just let me know what were the design decisions behind that choice). Overall it would be ideal if only the features needed by a lightstep client were to be included in the final compiled program instead of bringing in tons of unnecessary code.I'd love to get your take on the issue, and I'm available if you need help exploring how to improve the situation.