Compilation Error from Jaeger exporter when updating go packages dependencies

[davidgwcurve]

Hey,

When doing a go get -u ./... in our go service, we get an error from the go.opentelemetry.io/otel/exporters/trace/jaeger exporter with the below text.

The root cause of this is that go adds github.com/apache/thrift v0.14.0 // indirect to the go.mod. It looks like v0.17.0 of the Jaeger exporter is using v0.13.0 of Thrift and there are some breaking changes go is trying to bring in when updating.

# go.opentelemetry.io/otel/exporters/trace/jaeger/internal/gen-go/jaeger
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:67:34: not enough arguments in call to oprot.WriteMessageBegin
        have (string, thrift.TMessageType, int32)
        want (context.Context, string, thrift.TMessageType, int32)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:76:32: not enough arguments in call to oprot.WriteMessageEnd
        have ()
        want (context.Context)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:109:22: cannot assign error to err (type thrift.TException) in multiple assignment:
        error does not implement thrift.TException (missing TExceptionType method)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:109:47: not enough arguments in call to iprot.ReadMessageBegin
        have ()
        want (context.Context)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:116:12: not enough arguments in call to iprot.Skip
        have (number)
        want (context.Context, thrift.TType)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:117:22: not enough arguments in call to iprot.ReadMessageEnd
        have ()
        want (context.Context)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:119:25: not enough arguments in call to oprot.WriteMessageBegin
        have (string, thrift.TMessageType, int32)
        want (context.Context, string, thrift.TMessageType, int32)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:120:10: not enough arguments in call to x1.Write
        have (thrift.TProtocol)
        want (context.Context, thrift.TProtocol)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:121:23: not enough arguments in call to oprot.WriteMessageEnd
        have ()
        want (context.Context)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:132:9: cannot use args.Read(iprot) (type error) as type thrift.TException in assignment:
        error does not implement thrift.TException (missing TExceptionType method)
../../pkg/mod/go.opentelemetry.io/otel/exporters/trace/[email protected]/internal/gen-go/jaeger/agent.go:132:9: too many errors

The otel packages we are using are

	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.17.0
	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.17.0
	go.opentelemetry.io/contrib/propagators v0.17.0
	go.opentelemetry.io/otel v0.17.0
	go.opentelemetry.io/otel/exporters/trace/jaeger v0.17.0
	go.opentelemetry.io/otel/sdk v0.17.0
	go.opentelemetry.io/otel/trace v0.17.0

[Aneurysm9]

This seems to be an artifact of Go treating modules at v0.x as being stable across minor version increments and "helpfully" incrementing the version of the Thrift dependency for you. While we may update that dependency at a later time, the v0.17.0 version of the Jaeger exporter will never be compatible with v0.14.0 of the Thrift package as it is immutable. I believe the only option is to manually remove the indirect dependency that go get automatically added.

[davidgwcurve]

Fair enough @Aneurysm9 , I got it to build again by removing the indirect reference. I thought I would raise this in case you needed to bump the version used by the Jaeger exporter :)

[davidgwcurve]

I'm assuming it will take going to GA and v1 for this to be resolved? I'm a little worried about having to make sure the team knows to remove the imported indirect if they do a go get -u and ensuring our automated dependency updates don't break from this.

[MrAlias]

The suggestion here is we should look into vendoring the thrift library.

[Aneurysm9]

I'm assuming it will take going to GA and v1 for this to be resolved? I'm a little worried about having to make sure the team knows to remove the imported indirect if they do a go get -u and ensuring our automated dependency updates don't break from this.

Yup, we're not keen on the situation either. We discussed it at the SIG meeting today and, as @MrAlias mentioned, we decided that the best approach to take for now was to vendor the dependency inside the generated Thrift package. This is done in #1551 with import path rewriting, so there shouldn't be an issue if this exporter is used in a system that otherwise uses a different version of Thrift. Should Thrift ever hit a stable 1.0 release then we can re-evaluate using the module again.

Thanks for bringing this issue to our attention, it's good that we caught it now while we can more easily address it.

[davidgwcurve]

Thanks a lot for the quick response and action to this!