Update some mailgun deps to remove dependency on mgo

[freeformz]

https://github.com/mailgun/timetools has a dependency on mgo's BSON package.
mgo hasn't been updated in ~4+ years and is unsupported.

Additionally, https://github.com/mailgun/holster has replaced:

• https://github.com/mailgun/timetools
• https://github.com/mailgun/ttlmap

While here, bump the minimally supported version of Go to go1.17.

Closes #210

[ldez]

#196 (comment)

[freeformz]

@ldez does that continue to be a problem with go1.17+ graph pruning? https://go.dev/doc/go1.17#graph-pruning

[ldez]

Yes, the problems are still here. The Holster has too many unrelated dependencies.

[thrawn01]

Holster was put together with the hope that one day graph pruning would be implemented in go. Since it now supports this, I don't see an issue with including it in the code base. The dependencies introduced with this PR only includes what is needed by the code.

require (
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/gravitational/trace v0.0.0-20190726142706-a535a178675f // indirect
	github.com/jonboulle/clockwork v0.1.0 // indirect
	github.com/pkg/errors v0.9.1 // indirect
	github.com/pmezard/go-difflib v1.0.0 // indirect
	golang.org/x/crypto v0.0.0-20220427172511-eb4f295cb31f // indirect
	golang.org/x/sys v0.0.0-20220307203707-22a9840ba4d7 // indirect
	golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
)

[freeformz]

@ldez Please compare the go.mod file in this pr to the go.mod file of holster. AFAICT go 1.17+ graph pruning solves the problem as long as the packages you are using from that module don't also pull in those deps, which afaict the ones I've started using do not.

[freeformz]

PS: Until moving to go 1.18 you'll have to run go mod tidy -compat=1.17 to get the full pruning.

[ldez]

Maybe I was not clear: the Holster has too many unrelated dependencies, it's not a graph problem but a transitivity problem.

For example: google.golang.org/grpc, go.opentelemetry.io/otel, ...

[freeformz]

@ldez We are talking about the same thing here. I too am talking about transitive dependencies. Where are they in the transitive package graph of this pr? For example:

$ go mod why go.opentelemetry.io/otel/... 
go: downloading go.opentelemetry.io/otel/exporters/jaeger v1.4.1
go: downloading github.com/go-logr/logr v1.2.2
go: downloading github.com/go-logr/stdr v1.2.2
go: downloading github.com/stretchr/objx v0.1.0
# go.opentelemetry.io/otel
(main module does not need package go.opentelemetry.io/otel)

# go.opentelemetry.io/otel/attribute
(main module does not need package go.opentelemetry.io/otel/attribute)

# go.opentelemetry.io/otel/baggage
(main module does not need package go.opentelemetry.io/otel/baggage)

# go.opentelemetry.io/otel/codes
(main module does not need package go.opentelemetry.io/otel/codes)

# go.opentelemetry.io/otel/internal
(main module does not need package go.opentelemetry.io/otel/internal)

# go.opentelemetry.io/otel/internal/baggage
(main module does not need package go.opentelemetry.io/otel/internal/baggage)

# go.opentelemetry.io/otel/internal/global
(main module does not need package go.opentelemetry.io/otel/internal/global)

# go.opentelemetry.io/otel/internal/internaltest
(main module does not need package go.opentelemetry.io/otel/internal/internaltest)

# go.opentelemetry.io/otel/internal/matchers
(main module does not need package go.opentelemetry.io/otel/internal/matchers)

# go.opentelemetry.io/otel/internal/trace/noop
(main module does not need package go.opentelemetry.io/otel/internal/trace/noop)

# go.opentelemetry.io/otel/propagation
(main module does not need package go.opentelemetry.io/otel/propagation)

# go.opentelemetry.io/otel/semconv/v1.4.0
(main module does not need package go.opentelemetry.io/otel/semconv/v1.4.0)

# go.opentelemetry.io/otel/semconv/v1.5.0
(main module does not need package go.opentelemetry.io/otel/semconv/v1.5.0)

# go.opentelemetry.io/otel/semconv/v1.6.1
(main module does not need package go.opentelemetry.io/otel/semconv/v1.6.1)

# go.opentelemetry.io/otel/semconv/v1.7.0
(main module does not need package go.opentelemetry.io/otel/semconv/v1.7.0)

# go.opentelemetry.io/otel/exporters/jaeger
(main module does not need package go.opentelemetry.io/otel/exporters/jaeger)

# go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/agent
(main module does not need package go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/agent)

# go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/agent/agent-remote
(main module does not need package go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/agent/agent-remote)

# go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/jaeger
(main module does not need package go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/jaeger)

# go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/jaeger/collector-remote
(main module does not need package go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/jaeger/collector-remote)

# go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/zipkincore
(main module does not need package go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/zipkincore)

# go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/zipkincore/zipkin_collector-remote
(main module does not need package go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/zipkincore/zipkin_collector-remote)

# go.opentelemetry.io/otel/exporters/jaeger/internal/third_party/thrift/lib/go/thrift
(main module does not need package go.opentelemetry.io/otel/exporters/jaeger/internal/third_party/thrift/lib/go/thrift)

# go.opentelemetry.io/otel/sdk/instrumentation
(main module does not need package go.opentelemetry.io/otel/sdk/instrumentation)

# go.opentelemetry.io/otel/sdk/internal
(main module does not need package go.opentelemetry.io/otel/sdk/internal)

# go.opentelemetry.io/otel/sdk/internal/env
(main module does not need package go.opentelemetry.io/otel/sdk/internal/env)

# go.opentelemetry.io/otel/sdk/resource
(main module does not need package go.opentelemetry.io/otel/sdk/resource)

# go.opentelemetry.io/otel/sdk/trace
(main module does not need package go.opentelemetry.io/otel/sdk/trace)

# go.opentelemetry.io/otel/sdk/trace/tracetest
(main module does not need package go.opentelemetry.io/otel/sdk/trace/tracetest)

# go.opentelemetry.io/otel/trace
(main module does not need package go.opentelemetry.io/otel/trace)

[freeformz]

^ (a pruned transitive dependency)

compared to...

$ go mod why -m github.com/pkg/errors     
# github.com/pkg/errors
github.com/vulcand/oxy/cbreaker
github.com/mailgun/holster/v4/clock
github.com/pkg/errors

(an unpruned transitive dependency)

[ldez]

I don't think we are talking about the same thing: you are talking about oxy-holster relations, I'm talking about project-oxy-holster relations.

Offtopic: In all cases, I'm not able to merge a PR on oxy because the CI is broken, and nobody answer to my requests, take a look here #208 (comment)

[freeformz]

How does what I said not apply to the project-oxy-holster relations?

[thrawn01]

@ldez Does Traefik have some reason not to upgrade to 1.7 or 1.8 to get the graph pruning benefit? This would only effect projects that include oxy yet are NOT using 1.7 or 1.8.

[ldez]

Does Traefik have some reason not to upgrade to 1.7 or 1.8 to get the graph pruning benefit?

Yes, mainly because some tools like golangci-lint (because a problem with SSA, the Go team is working on)
golangci/golangci-lint#2649

[freeformz]

That is 1.18 specific. My bump was to a min version of 1.17, specifically to avoid stuff like that and to gain pruning. Given the state of things like you point out (PS: Thanks for your work on golangci-lint!).

Holster is also go1.17 atm and I would expect a bump to go1.18 now there is also out of the question for the same reasons.

See also: https://github.com/freeformz/oxy-deps-test/blob/main/go.mod for the transitive project dependencies.

Also golangci-lint-1.45.2 has no problem running on my oxy branch with go1.17.

[freeformz]

FWIW: With go1.16 holster won't even vendor because that has been upgraded to 1.17 as the minimally supported version.

$ go mod vendor
internal error: failed to find embedded files of github.com/mailgun/holster/v4/clock: //go:build comment without // +build comment

[ldez]

@freeformz I will spend some time on your PR tomorrow (it's 11pm for me)

FYI: I will fix the CI in a few minutes

[freeformz]

@ldez Thanks! (Get some sleep). I am happy to make modifications, let me know.

[ldez]

Can you rebase your PR?
(It took me longer than expected, now I'm going to sleep 😄)

[freeformz]

@ldez done. Now I'm going back to bed myself.

[ldez]

Note: the changes of time.Duration to clock.Duration are breaking the API contract.

[freeformz]

@ldez I can revert those. They are type aliases, so it's really the same type under the covers. I was getting kind of overly "complete" in doing that change.

[ldez]

I don't why but I was thinking that clock.Duration was an interface.

I just mixed clock.Clock and clock.Duration.

[ldez]

Could you don't squash commits?
When I will merge I will squash the commits of the PR (GitHub has a button for that)

[freeformz]

Okay. I won't anymore.

[ldez]

Just a tip: instead of git push --force use git push --force-with-lease

https://git-scm.com/docs/git-push#Documentation/git-push.txt---force-with-leaseltrefnamegt

You dropped my commit.

[ldez]

The problem I expected occurs: when I update the dependency in Traefik, the module google.golang.org/grpc is updated (from v1.38.0 to v1.40.0)

The module google.golang.org/grpc contains breaking changes inside minor versions.

The holster uses this module in the package etcdutil.
https://github.com/mailgun/holster/blob/b3c1e976421efc59d96ee63d2ecbe0c541283128/etcdutil/config.go#L14

This is why I said that the Holster has too many unrelated dependencies and the problem is project-oxy-holster.

[ldez]

Which dependencies of the Holster (as a library) are problems from my point of view:

• github.com/Shopify/toxiproxy -> github.com/prometheus/client_golang
• github.com/ahmetb/go-linq
• github.com/golang/protobuf
• github.com/hashicorp/consul
• go.etcd.io/etcd
• google.golang.org/grpc
• go.opentelemetry.io/otel

Why they are problems: because oxy just needs a fake clock implementation.

The influence on Prometheus, protobuf, consul, etcd, grpc, and opentelemetry dependencies is a problem because those dependencies are not related to oxy features.
And those dependencies can have a bad impact on clients of oxy.

I'm not a huge fan of utils/helper libraries or packages, but for me, a utils/helper library must be lean and it must be focused on one topic to avoid unnecessary dependencies.

I really want to drop deprecated dependencies, but I don't want to introduce side effects.

Maybe the Holster must be a multi-module package or maybe we have to carry the code, for now, I don't know what is the best solution.

[freeformz]

I was surprised by this, but then realized that Traefik has a direct dependency on google.golang.org/grpc and so does holster, so that dependency won't be filtered out by graph pruning, even if the packages imported from holster don't use the grpc package. Maybe there is some room for improvement in the graph pruning implementation. Anyway.

FWIW: If those dependencies are really that sensitive to Traefik then they should/could be replaced in Traefik's go.mod file to ensure they are locked to a specific version, no matter what dependencies say.

Another workaround would be to create an internal vendor a copy of holster's clock package (or the older timetools stuff; personally I like clock better - but this isn't my repo) until the overarching problem is addressed by go and/or holster.

Which course would you like me to pursue ?

[ldez]

Let's go for an internal package with a copy of holster clock.
Don't forget the license 😉

[freeformz]

@ldez done, let me know if anything else needs to be changed.

[ldez]

Thanks 👍 👍 👍