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

v2 go module semantic versioning #2826

Closed
optiman opened this issue Oct 27, 2020 · 15 comments
Closed

v2 go module semantic versioning #2826

optiman opened this issue Oct 27, 2020 · 15 comments
Assignees
Labels
keepalive An issue or PR that will be kept alive and never marked as stale.

Comments

@optiman
Copy link

optiman commented Oct 27, 2020

Describe the bug
I'm using go client library (/pkg/promtail/client) instead of promtail to push log entries to Loki.
After updating Loki to v2.0.0 I cannot use go client library in projects with go.mod because Loki's module path does not contain /v2 suffix as required by the go command.

To preserve import compatibility, the go command requires that modules with major version v2 or later use a module path with that major version as the final element. For example, version v2.0.0 of example.com/m must instead use module path example.com/m/v2, and packages in that module would use that path as their import path prefix, as in example.com/m/v2/sub/pkg.

To Reproduce

  • mkdir myproject && cd myproject
  • go mod init myproject
  • go get github.com/grafana/[email protected]

Expected behavior
Be able to use Loki v2 go client library in projects with go.mod file (module-aware mode).

Environment:
local, linux

Screenshots, Promtail config, or terminal output
go get github.com/grafana/[email protected]: github.com/grafana/[email protected]: invalid version: module contains a go.mod file, so major version must be compatible: should be v0 or v1, not v2

@slim-bean
Copy link
Collaborator

Yes this was expected and unfortunately is (IMO) a limitation and annoyance with Go mod. I don't think it's appropriate that Go mod scans repos and uses tags to assign versions, this really limits what we can do for versioning a project which was never built with the intention of being a library.

We made the decision to use the 2.0.0 tag knowing it would break go mod.

Sadly, your use case is completely acceptable and we want to encourage people to use things like the promtail client package in their code.

However, we want the ability to control the Loki version number as it feels appropriate to us and the feature set in the project, not as it pertains to it's use as a go library.

This creates a dilemma for us, and we chose to go the path of tying the version number to the project state and features, not to the use as a library.

We appreciate this issue, apologies for my long response but you were lucky to be the first to find this 😃 and I want to have something to reference when others find this problem too!

The workaround is to install Loki based on commit hashes now and not version tags.

The commit hash for the 2.0.0 tag is 6978ee5d73878d7406bda68296a3dd8ecda4d538

❯ go get -u github.com/grafana/loki@6978ee5d73878d7406bda68296a3dd8ecda4d538                                                                                
go: github.com/grafana/loki 6978ee5d73878d7406bda68296a3dd8ecda4d538 => v1.6.2-0.20201026154740-6978ee5d7387
go: downloading github.com/grafana/loki v1.6.2-0.20201026154740-6978ee5d7387

I know this is not ideal, and we are considering alternatives for something like the client package you are using maybe it makes sense to put this in a separate repo, we are still undecided here.

Thank you for reporting this, sorry if my answer is not what you were hoping for!

@optiman
Copy link
Author

optiman commented Oct 28, 2020

I completely agree that this is a limitation and major annoyance while working with go modules, but it seems that they have become de-facto standard nowadays.
So we should come up with a way to use (preferebly without workarounds) the go client without incompatible changes to the Loki project itself.
I think, moving the client to a separate repo with module-compatible versioning is a sensible compromise.

And thank you for the workaround, this should work for the time being.

@jeschkies
Copy link
Contributor

I think, moving the client to a separate repo with module-compatible versioning is a sensible compromise.

This probably makes sense. If the overhead of managing multiple repositories is too big one could import the promtail repo as a submodule or subrepo.

@stale
Copy link

stale bot commented Nov 27, 2020

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Nov 27, 2020
@optiman
Copy link
Author

optiman commented Nov 29, 2020

Still relevant.

@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Nov 29, 2020
@stale
Copy link

stale bot commented Dec 29, 2020

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Dec 29, 2020
@realvasko
Copy link

If using the commit hash is the official workaround for now, is it possible to include it somewhere in the notes? I tried pulling v2.1.0 with 1b79df3754f60426459ea51b84fb8a387c4b9ff5, but go mod tells me it's getting v1.6.2-0.20201223215703-1b79df3754f6, which is odd.

@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Jan 6, 2021
@stale
Copy link

stale bot commented Feb 7, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Feb 7, 2021
@kavirajk
Copy link
Contributor

kavirajk commented Mar 4, 2021

Actually we don't have to create subdirectories or anything to make it module compatible.

The idea of subdirectory comes from the official blog (which is unfortunate). because during that time, creating subdirectory was the only way for old go tools chain to understand the go modules semantics.

If we don't care about that (no one shouldn't anymore, particularly Loki shouldn't), we can just change our module path in go.mod every time we release major version.

e.g: we just have to change our go.mod in v2 branch and add "/v2" in our module path(it becomes module github.com/grafana/loki/v2). Literally that's the only change needed.

This practice is covered in Go Wiki. https://github.com/golang/go/wiki/Modules#releasing-modules-v2-or-higher

Also as a proof of concept I created this repository with multiple major versions (with separate branch) and involving only change in module path (no subdirectories or anything)

https://github.com/kavirajk/v

And usage here https://play.golang.org/p/ZVumxvdSw1L

I can create a PR to see if anything else breaks!

// @grafana/loki-team @slim-bean

@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Mar 4, 2021
@optiman
Copy link
Author

optiman commented Mar 5, 2021

Was separate client idea abandoned? I see some work being done here, but not so much recently.
https://github.com/grafana/loki-client-go

Semantic versioning isn't the only problem when using go client.
There are a lot of replace directives in go.mod of this repository, not every one of them necessary for the client, only for Loki itself.
So when you use the same dependencies in your project, and then add github.com/grafana/loki to your go.mod just to use the client, you are now restricted by those versions, that the whole Loki project uses, otherwise compilation fails.
At least that was my experience.

@stale
Copy link

stale bot commented Jun 3, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Jun 3, 2021
@stale stale bot closed this as completed Jun 11, 2021
cyriltovena pushed a commit to cyriltovena/loki that referenced this issue Jun 11, 2021
* Add -store.max-query-length support to blocks storage

Signed-off-by: Marco Pracucci <[email protected]>

* Added PR number to CHANGELOG

Signed-off-by: Marco Pracucci <[email protected]>

* Update pkg/util/validation/limits.go

Signed-off-by: Marco Pracucci <[email protected]>

Co-authored-by: Peter Štibraný <[email protected]>

* Updated doc

Signed-off-by: Marco Pracucci <[email protected]>

Co-authored-by: Peter Štibraný <[email protected]>
@kavirajk kavirajk reopened this Oct 7, 2021
@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Oct 7, 2021
@kavirajk kavirajk added the keepalive An issue or PR that will be kept alive and never marked as stale. label Oct 7, 2021
@kavirajk
Copy link
Contributor

[cross posting here for more context from #4352]

Even if we decide to go with supporting our module path with major version, I think this can happen only on next major release (say Loki v3). Purely because of technical reasons mentioned on the Go wiki

For example, if you are the author of foo, and the latest tag for the foo repository is v2.2.2, 
and foo has not yet adopted modules, then the best practice would be to use 
v3.0.0 for the first release of foo to adopt modules (and hence the first release of foo to contain a go.mod file).

bahlo added a commit to axiomhq/axiom-loki-multiplexer that referenced this issue Jan 4, 2022
lukasmalkmus pushed a commit to axiomhq/axiom-loki-multiplexer that referenced this issue Jan 4, 2022
kevincantu added a commit to aporeto-inc/tracer that referenced this issue May 27, 2023
Note: their v2 numbering is flawed: grafana/loki#2826, so to update specify main:
```
go get -u github.com/grafana/loki@main
```

To sanity-check, on pcn-staging:
```
make build
 /Users/kcantu/panw/tracer/tracer --monitoring-url="https://monitoring.staging.network.prismacloud.io" --monitoring-cert="$HOME/.tracer/staging-cert.pem" --monitoring-cert-key="$HOME/.tracer/staging-key.pem" --since 1h
 /Users/kcantu/panw/tracer/tracer --monitoring-url="https://monitoring.staging.network.prismacloud.io" --monitoring-cert="$HOME/.tracer/staging-cert.pem" --monitoring-cert-key="$HOME/.tracer/staging-key.pem" --open 0dc1574097540bd5
```
dgoodwin added a commit to dgoodwin/sippy that referenced this issue Jun 16, 2023
This appears to be required, see grafana/loki#2826
@prochac
Copy link

prochac commented Oct 20, 2023

Workaround:

go.mod

require github.com/grafana/loki v0.0.0-20231016134057-a17308db6f24 // v2.9.2

main.go

import _ "github.com/grafana/loki/pkg/loki" // github.com/grafana/loki is not a Go package

@drornir-velocity
Copy link

Releasing v3 would be amazing, ty

@chaudum
Copy link
Contributor

chaudum commented Oct 29, 2024

Has been addressed with Loki 3.0 release

@chaudum chaudum closed this as completed Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keepalive An issue or PR that will be kept alive and never marked as stale.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants