search

lightstep / lightstep-tracer-go

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

vendor gRPC #55

Closed djspoons closed 8 years ago

djspoons commented 8 years ago

I'm not entirely sure we'd want to do this, but this would fix the client at a particular version of gRPC and prevent that code from leaking into the application.

djspoons commented 8 years ago

I believe the failed build is because we've configured CircleCI to use Ubuntu 12 (and therefore an older version of go) instead of Ubuntu 14.

djspoons commented 8 years ago

Fixed! It turns out that if you change the OS on CircleCI (e.g. from Precise to Trusty), you need to commit something for it to pick up the change. (And you'd learn that if you read all of the text on the settings page :/ )

djspoons commented 8 years ago

Since there was just a breaking change to gRPC, this seems like a good time to merge this PR. ;)

peterbourgon commented 8 years ago

With a few edge-case exceptions, libraries must not vendor their deps, as there is no effective mechanism for importers to respect vendored code in the standard toolchain.

tamird commented 8 years ago

This is bad news bears for downstream consumers. grpc-go depends on "golang.org/x/net/trace", which includes this gem: https://github.com/golang/net/blob/f4b625e/trace/trace.go#L114

Now because this is vendored, it results in the following init-time crash in any application that imports "golang.org/x/net/trace":

panic: http: multiple registrations for /debug/requests

goroutine 1 [running]:
panic(0x5189940, 0xc42015f3a0)
    /Users/tamird/src/go1.7/src/runtime/panic.go:500 +0x1a1
net/http.(*ServeMux).Handle(0x5f58180, 0x5374b7b, 0xf, 0x5bd7d80, 0x5522720)
    /Users/tamird/src/go1.7/src/net/http/server.go:2038 +0x5d5
net/http.(*ServeMux).HandleFunc(0x5f58180, 0x5374b7b, 0xf, 0x5522720)
    /Users/tamird/src/go1.7/src/net/http/server.go:2070 +0x55
net/http.HandleFunc(0x5374b7b, 0xf, 0x5522720)
    /Users/tamird/src/go1.7/src/net/http/server.go:2082 +0x4b
github.com/lightstep/lightstep-tracer-go/vendor/golang.org/x/net/trace.init.1()
    /Users/tamird/src/go/src/github.com/lightstep/lightstep-tracer-go/vendor/golang.org/x/net/trace/trace.go:122 +0x42
github.com/lightstep/lightstep-tracer-go/vendor/golang.org/x/net/trace.init()
    /Users/tamird/src/go/src/github.com/lightstep/lightstep-tracer-go/vendor/golang.org/x/net/trace/trace.go:1072 +0x556
github.com/lightstep/lightstep-tracer-go/vendor/google.golang.org/grpc.init()
    /Users/tamird/src/go/src/github.com/lightstep/lightstep-tracer-go/vendor/google.golang.org/grpc/trace.go:120 +0x80
github.com/lightstep/lightstep-tracer-go/collectorpb.init()
    /Users/tamird/src/go/src/github.com/lightstep/lightstep-tracer-go/collectorpb/collector.pb.go:762 +0x62
github.com/lightstep/lightstep-tracer-go.init()
    /Users/tamird/src/go/src/github.com/lightstep/lightstep-tracer-go/util.go:51 +0x71
github.com/cockroachdb/cockroach/pkg/util/tracing.init()
    github.com/cockroachdb/cockroach/pkg/util/tracing/_obj/_cgo_import.go:4 +0x94
github.com/cockroachdb/cockroach/pkg/roachpb.init()
    /Users/tamird/src/go/src/github.com/cockroachdb/cockroach/pkg/roachpb/method_string.go:17 +0x67
github.com/cockroachdb/cockroach/pkg/util/stop.init()
    /Users/tamird/src/go/src/github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:484 +0x62
github.com/cockroachdb/cockroach/pkg/cli.init()
    github.com/cockroachdb/cockroach/pkg/cli/_obj/_cgo_import.go:7 +0x60
main.init()
    /Users/tamird/src/go/src/github.com/cockroachdb/cockroach/main.go:42 +0x3d

There seems to be nothing sane to do but to revert this.

peterbourgon commented 8 years ago

@tamird It looks like Cockroach does not have its own vendor dir. How do you manage your dependencies?

tamird commented 8 years ago

We use https://github.com/robfig/glock. That's not relevant to this discussion, so let's not pollute this thread. Please feel free to join us on gitter: https://gitter.im/cockroachdb/cockroach.