search

lightstep / lightstep-tracer-go

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

use standard context package instead of x/net/context #174

Closed achille-roussel closed 6 years ago

achille-roussel commented 6 years ago

Following up on our conversation in https://github.com/lightstep/lightstep-tracer-go/issues/149

This PR modifies the source files to use the standard context package instead of x/net/context. This means that this repository will not be compatible with Go 1.6 and earlier anymore. The README does not mention the minimum Go version currently supported so I'm not sure if this is going to be a deal-breaker or not.

If we need support for Go 1.6 we can probably figure out a way to support either packages with a compile-time check, but it could introduce lots of complexity in the code.

I was concerned that x/net/http2 may have used x/net/context directly but it seems they have moved to the standard context package already, there's only one leftover in a test:

github.com/golang/net/http2 $ grep -r 'x/net/context' .
./transport_test.go:    "golang.org/x/net/context"

There are two places where x/net/context is still referenced after this change:

lightstep-tracer-go $ grep -r x/net/context .
./collectorpb/collectorpbfakes/fake_collector_service_client.go:    "golang.org/x/net/context"
./collectorpb/collector.pb.go:  context "golang.org/x/net/context"

I tried re-generating those files but they kept importing x/net/context, I'm not super familiar with the build process here so I could take a pointer or two on how to fix this.

iredelmeier commented 6 years ago

LGTM. We'll deal with the x/net/context in the fakes separately.