Run command mod tiny

[bogdandrutu]

No description provided.

[songy23]

LGTM, though it looks like the modified go.sum doesn't work on Windows.

[jeanbza]

go: verifying git.apache.org/[email protected]: checksum mismatch
	downloaded: h1:692K1/SsOcQvkvMRMdt60FCq2AvKpuQNM6sIeH3mN4s=
	go.sum:     h1:CMxsZlAmxKs+VAZMlDDL0wXciMblJcutQbEe3A9CYUM=

Maybe try rm go.sum && go mod tidy? Alternatively, try re-running the windows build. There's nothing windows specific about that failure - might just be a hiccup.

[dmitshur]

It might've been a network error during download. It would be interesting to see what happens when you restart it.

[bogdandrutu]

@dmitshur my current best guess it is because we vendor thrift and we imported it via the vendor path, changed everything to use github.com/apache/thrift

[dmitshur]

It seems that the checksum mismatch keeps happening on Windows, it's not a network error. I see you're currently using Go 1.11 in appveyor.yml#L9. I wonder if anything would change if you were to update it to Go 1.12. It might be a bug in 1.11, so the question is whether it's been resolved in 1.12.

[bogdandrutu]

@dmitshur I made progress by forcing to upgrade go but found different issue on 1.12. I will force 1.11.5.

[bogdandrutu]

I think I won this battle :)

[bogdandrutu]

@jadekler I think it was because something changed in 1.11.2 when they fix a bug in go.sum, I remember seeing this bug at one point.

[dmitshur]

@bogdandrutu Thank you for working on this!

Are you planning on tagging a new release version? That'll enable some other projects that depend on [email protected] to update to a newer version.

[bogdandrutu]

we were planning to do in the next few days, if you cannot wait let me know. @rghetia what do you think?

[dmitshur]

See #1064 (comment). I think it might make sense to try to resolve that issue before making a new tag.

[rghetia]

we were planning to do in the next few days, if you cannot wait let me know. @rghetia what do you think?

I plan to release early next week with metric support.

[jeanbza]

Friendly ping - is there still a plan to release this week?

[songy23]

This change has caused trouble for projects that have transitive dependency on libraries that don't have module enabled and have a different version of Thrift.

For example, consider projects who 1. depend on both OpenCensus and Jaeger/tchannel-go and 2. use go module instead of vendor. If we try to use OC-Go v0.20.0 (which contains this commit), those projects won't build. See census-instrumentation/opencensus-service#511 (comment).

A straightforward fix is to just revert this commit and do a new release, though I suspect that will cause other problems. @rghetia @bogdandrutu any ideas here?

[dmitshur]

@songy23 Can you share some reproducible steps that demonstrate the failure to build?

Reverting this commit, as you said, has its own problems as well. It might be better for Jaeger and tchannel-go to update to be compatible with the current version of Thrift. But investigating this is easier if there are reproducible steps demonstrating a failure. Posting that in a new issue will be better. Thanks.

[songy23]

@dmitshur An example is https://travis-ci.org/census-instrumentation/opencensus-service/builds/515317658#L1009. This can be reproduced if you have both OC-Go 0.20.0 and Jaeger/TChannel in the same go.mod file:

require (
  github.com/apache/thrift v0.12.0 // required by oc-go 0.20.0
  ...
  github.com/uber/tchannel-go v1.10.0
  go.opencensus.io v0.20.0
)

As you can see in https://github.com/uber/tchannel-go they don't use go modules and they vendored a Thrift directory, which is not compatible with Thrift 0.12.0.

I've filed a tracking issue #1092 and we can continue discussions there.