Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent incompatibility issues between common and dependent repositories #603

Open
ArthurSens opened this issue Mar 11, 2024 · 5 comments
Open

Comments

@ArthurSens
Copy link
Member

ArthurSens commented Mar 11, 2024

Common is not a stable repository, but it is being used by others that are considered stable, like client_golang and Prometheus itself.

Last week we had an incompatibility problem between common and client_golang which forced us to rush and cut a new release. We don't want to tell prometheus/common "no more breaking changes!!!!!", but it would be nice if these changes could be communicated better when they happen.

One suggestion is to add a CI workflow that tries to build the latest stable release of client_golang and/or Prometheus on pull requests. The workflow doesn't need to be "Required to pass", but if merged with failures let's communicate with dependent projects within the Prometheus organization

@beorn7
Copy link
Member

beorn7 commented Mar 12, 2024

I'm not sure if there is anything to gain here.

The issue you are talking about was deliberately breaking, so a CI workflow would only have told us what we already knew. (And yes, there can be too much of CI testing. prometheus/prometheus is suffering from too long test durations, and the probability of hitting a flaky test is also increasing more and more.)

The release clearly called out the breaking change:

image

Would it have helped if we had given an additional heads-up to prometheus/client_golang and prometheus/prometheus maintainers? The tests in all depending repos will automatically flag if bumping the version of the dependency will break things. Would knowing about the breaking change even before that have changed any action anyone did? I guess we could have done a coordinated release of a bunch of Prometheus projects. But breaking changes in prometheus/common will happen again and again. We cannot sync every release with releases of a bunch of other Prometheus projects.

The Go module contract is that pre-v1 versions might have breaking changes anytime. Each version of prometheus/client_golang was referring to a version of prometheus/common that worked with it. IMHO the problem only happens if people blindly update their indirect dependencies to something newer than what the go.mod file of their direct dependencies need. My impression is that noisy so-called “security scanners” educate people to blindly update their dependencies all the time (which is already risky with v1+, but pretty dangerous with v0.*. There might even be behavioral changes that the compiler doesn't catch and where you have to rely on really good test coverage.)

The only real problem I see is that you might have two different dependencies that both depend on the same indirect dependency, but in different, non-compatible versions. That's exactly the diamond-shaped dependency problem that Go modules intends to solve with the v2/v3/… prefixes. People are still not really happy, but that's a different problem.

Even with the best communication, we cannot solve the diamond-shaped dependency problem.

My bottom line here is: As long as we deliberately treat prometheus/common as non-stable, there is zero gain in treating breaking changes specially beyond telling users in the release note about it. If we want special treatment of breaking changes, we should do what Go modules want us to do: Bump to v1 and bump the major version for every breaking change. (Which is not meant sarcastically. We could and should do that if wo desire those guarantees.)

@ArthurSens
Copy link
Member Author

We're having a few extra problems when using common in client_golang, now related to supported go versions.

Since common 0.49.0, the minimal supported go version is 1.21, but client_golang promises support for the 3 latest go versions. This means we're stuck with common 0.48.0 until mid-August, when go 1.23 is released.

This is preventing us from adopting new features like prometheus/client_golang#1392, prometheus/client_golang#1408 and even discouraging contributions like prometheus/client_golang#1479. I'm not sure if bumping common to v1.0.0 and promising some guarantees is the way to go, but I think we could improve the relationship between client_golang and common.

Does it sound reasonable to follow client_golang's approach and require tests passing in 3 latest go versions? cc @bwplotka @kakkoyun and @SuperQ just so you stay in the loop

@SuperQ
Copy link
Member

SuperQ commented Mar 28, 2024

I've never understood why we support versions of Go that Go itself doesn't support.

@beorn7
Copy link
Member

beorn7 commented Mar 28, 2024

You can always use build tags to gate features that only work with more recent Go versions.

Supporting the last 3 Go versions used to be very little overhead. There are definitely users out there that are stuck with older Go compilers for one reason or another. (I'm not making any statement here if that's a good practice or not. It was just very easy to satisfy those users.) If supporting older Go versions is any kind of significant hassle, I don't see a problem not supporting them. Users can always just use older versions of our libraries. (And maybe the go directive in go.mod works sufficiently well enough that the right version is picked automatically?)

@SuperQ
Copy link
Member

SuperQ commented Mar 28, 2024

Yes, it's gotten a bit harder since people are wanting to use a number of the latest features in Go. Like the new slices package.

The good news is, the go directive is making it easier to be more clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants