proposal: add support for go modules

[mbyio]

Sorry if this is a duplicate, I looked but didn't see an existing issue.

Do you mind adding go modules support for this library? That way, people using go modules can make sure they have consistent indirect dependency versions. Currently, people using Go modules end up with a bunch of "indirect" versions in their go.mod file because of this library (for the libraries that this library imports). Since Go isn't sure which versions to use, it just picks the latest of each. But, if you add go modules support, you can indicate compatibility yourself.

I don't think this will break compatibility with dep or with the use of gopkg.in, it will simply enhance the experience for go modules users and improve safety and maintainability.

[gbbr]

Hi there! 👋We don't want to enforce any versions to be used with our integrations. Users should be able to use any version they wish, as long as it is within the compatible ones. Is there anything that you are trying to achieve and the current setup is stopping you? If so, please provide a concrete example. It will help in finding a solution.

[mbyio]

Sure, so like all dependency specification things, this is just a safety and convenience thing, so users don't have to manually research what versions are compatible with this library. You could just specify that there are no restrictions if you want. Go modules is just a way of encoding that information for tools and users.

One note - in the core code, under ddtrace, you do have a dependency on github.com/tinylib/msgp, which is picked up by go modules. Since go modules can't read your Gopkg.toml, it just picks the most recent version at the time it noticed the dependency, and it adds that version to go.mod with a comment // indirect. So in a project that is only directly using this library, the go.mod file might look like this:

module github.com/mbyio/my-service

go 1.12

require (
	github.com/opentracing/opentracing-go v1.1.0 // indirect
	github.com/philhofer/fwd v1.0.0 // indirect
	github.com/tinylib/msgp v1.1.0 // indirect
	gopkg.in/DataDog/dd-trace-go.v1 v1.15.0
)

Later, when developing, someone might want to use a different version of one of your indirect dependencies (maybe they heard about a security vulnerability, etc.), and they might be confused as to where that version comes from. They'll have to research to find that these dependencies are pulled in by gopkg.in/DataDog/dd-trace-go.v1 and to figure out that, actually, you generally intend to provide compatibility with all indirect dependency versions.

So to summarize, people can still use your library as-is, but this is a safety and convenience thing which can make it easier to setup. It will also make it safer for you to add dependencies/constraints in the future, if that ever becomes necessary.

[gbbr]

Thanks for the detailed explanation. It helps. You bring up good points. I think this is definitely something we need to look into. Please allow us some time and we'll respond with a good solution.

[gbbr]

I am adding the v2 label to this. We should consider using modules for the next major version. It could be nice to consider having multiple modules within the same repo: one for the tracer, and one for each integration.

[gbbr]

It seems that this has become easier now based on https://github.com/golang/go/wiki/Modules#faqs--multi-module-repositories. We could tag each contrib using its own version.

[knusbaum]

Since more and more issues are popping up around module support, and with go 1.14, "module support in the go command is now ready for production use." we may want to talk about exactly how and when we want to do this.

We don't yet have a timeline for v2. Do we need to look at some sort of modules solution for the v1 tracer?

[gbbr]

Since more and more issues are popping up around module support, and with go 1.14, "module support in the go command is now ready for production use." we may want to talk about exactly how and when we want to do this.

Correct, let's do this. The idea was that the entire repository would turn into a single module. We can potentially do this today, but read on...

We don't yet have a timeline for v2. Do we need to look at some sort of modules solution for the v1 tracer?

We could, but it's not going to help with #606 and #548 because even though those will start working, they will only work for users who have opted into modules. This means that modules will become a requirement to use dd-trace-go, breaking current users which didn't opt in. This would be quite unfortunate because we would be introducing a requirement which only applies to two integrations that most people might not be using.

So I think the only remaining option is to leave this for v2. It might be worthwhile exploring to see if we can obtain another organisation to keep our integrations separate from the core library (in the style of https://github.com/opentracing-contrib)

[knusbaum]

The idea was that the entire repository would turn into a single module.

I think this is the best way. I'm not sure exactly how we will handle integrations with multiple versions like grpc. Maybe vendoring will continue to be the best solution, as ugly as it is.

We could, but it's not going to help with #606 and #548 because even though those will start working, they will only work for users who have opted into modules. This means that modules will become a requirement to use dd-trace-go, breaking current users which didn't opt in. This would be quite unfortunate because we would be introducing a requirement which only applies to two integrations that most people might not be using.

Breaking existing users would make this a non-starter for v1. However, I am pretty sure that dd-trace-go should continue to work with GOPATH even if we add module support. As far as I can tell, it should just enable the use of modules for those users who want it. go-redis/redis v7 and labstack/echo v4 will only work for those users that opt into modules, but the rest should continue working as they do today. The only backwards-compatibility issues I've seen with modules is the versioned package imports, which we won't run into with dd-trace-go until v2 anyway.

It's totally possible I'm missing something, though. Are there other specific issues you forsee?

[gbbr]

I was mistakenly thinking that the go get command will not be able to fetch those special import paths which are module specific in an environment that has not opted into modules. It seems this was false and can be read in the wiki section "Releasing Modules" point "1. Major branch":

Go versions 1.9.7+, 1.10.3+, and 1.11 are able to properly consume and build a v2+ module created using this approach without requiring updates to consumer code that has not yet opted in to modules (as described in the "Semantic Import Versioning" section above).

That's great news and simplifies things a lot.

The next thing that is slightly unclear to me: what should be listed as requirements in the go.mod file? I think what makes sense is each minimum supported major version for each integration.

[knusbaum]

The next thing that is slightly unclear to me: what should be listed as requirements in the go.mod file? I think what makes sense is each minimum supported major version for each integration.

This was unclear to me as well, and I was thinking the same thing: minimum supported major version for each integration.

[roopakv]

has there been any progress on go module support? if we could break down the tasks would be easy to get started am happy to help out

[gbbr]

We have this in our backlog but didn't get around to it. There's a lot of edge cases to consider and scenarios that we need to test, so we would prefer to do this work internally, but thank you for offering 🙏

[knusbaum]

To summarize what needs to be done:

• Add go.mod file - This should not contain any contrib dependencies. Those should be resolved by the importing project. Compatibility can be controlled by versioned import paths.
• Remove a bunch of stuff from the CI pipeline - We no longer need to vendor particular versions for some things
• Use versioned import paths for some contrib packages

Let me know if I have this right, or if I'm missing something.

[roopakv]

@knusbaum your contrib packages are wrappers so they can continue to be here (I think). You just need to make sure that they rely on go mod to fetch other packages (go mod tidy will help create the current dependencies you use)

However the second point you made is the main one. I would add in addition that you don't clone the repo within GOPATH. that would be a great test.

[gbbr]

Got an initial PR working in #672. It also contains a CI refactor. @roopakv @mbyio comments welcome.

[gbbr]

PR is merged. Help with testing welcome.

[gbbr]

There's a v1.25.0-rc2 out for testing. It contains a few other changes too. Feel free to try it out and give feedback. This does not mean that we freeze master and have started to prepare for releasing 1.25. It is just an intermediate testing step which requires tagging. Development and merging PRs should continue as usual.

[priyanshi-gupta]

Released in 1.25.0