search

lightstep / lightstep-tracer-go

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

change default behavior to use gRPC #83

Closed djspoons closed 7 years ago

djspoons commented 7 years ago

Not a difficult change, so the main thing would be to document the new behavior.

tedsuo commented 7 years ago

So there's one wrinkle with this: Options.UseGRPC is a boolean, as opposed to a *boolean, and there are a variety of ways users may be generating the Options object. This means there's no way to differentiate between "I didn't set this field" and "I explicitly set it to false".

The safest thing to do would be to remove Options.UseGRPC and replace it with Options.UseThrift. This will of course break compilation everyone currently using gRPC, and silently switch everyone using thrift by default over to gRPC. Not ideal - i'd rather break compilation for everyone who is switching, and leave the gRPC users alone.

Since we can't have the compile error where I would like it, I propose we break no one and leave the UseGRPC flag present but mark it as deprecated.

djspoons commented 7 years ago

That reasoning (and the conclusion) make sense to me.

tedsuo commented 7 years ago

Cool, the PR is here btw, let me know when we want to pull it in: https://github.com/lightstep/lightstep-tracer-go/pull/84