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

Sig4v go.mod has not been tagged or released since inception #709

Closed
tsipo opened this issue Oct 24, 2024 · 7 comments · Fixed by #715
Closed

Sig4v go.mod has not been tagged or released since inception #709

tsipo opened this issue Oct 24, 2024 · 7 comments · Fixed by #715

Comments

@tsipo
Copy link

tsipo commented Oct 24, 2024

The sigv4 README states the following:

This is a separate module from github.com/prometheus/common to prevent it from having and propagating a dependency on the AWS SDK.

This module is considered internal to Prometheus, without any stability guarantees for external usage.

The problem is that any attempt to import the github.com/prometheus/common/sigv4 package and running go get -u ./... brings over the dependency github.com/prometheus/common/sigv4 v0.1.0 which is the go.mod file of this module since its inception - 2021-06-23.

This module has not been tagged ever since.

As a result, even Prometheus itself is not using any of the changes to this module since the original tagging!
prometheus/prometheus go.mod
has the same github.com/prometheus/common/sigv4 v0.1.0 dependency.

Please tag and release this module on a regular basis. Thank you!
(Alternatively you can split this module out to a separate repository if this does not create circular dependency issues).

@tsipo
Copy link
Author

tsipo commented Oct 24, 2024

BTW I discovered this with the following use-case which I don't think should be considered "external": to use the sigv4 implementation in this module in order to create a Prometheus client. This is namely the only reasonable way to connect to AMP (AWS Managed Prometheus) Prometheus API without implementing it all by hand.

Such implementation looks roughly like this:

  1. Call NewRoundTripperFromConfigWithContext to create a RoundTripper from configuration (prometheus/common/config package)
  2. Call NewSigV4RoundTripper to get a SigV4 RoundTripper from the RoundTripper we have from 1 and SigV4Config we read (prometheus/common/sigv4 package)
  3. Feed this RoundTripper to an api.Config and call NewClient with it (prometheus/client_golang/api package)

I can get around it by running go get github.com/prometheus/common/sigv4@653e0fa (which brings over the latest commit as of now) but this is very very hacky.

@tsipo tsipo changed the title Sig4v go.mod has not been tagged since inception Sig4v go.mod has not been tagged or released since inception Oct 25, 2024
@tsipo
Copy link
Author

tsipo commented Oct 25, 2024

Looking at this module, I would recommend the following. I tag here @SuperQ as he's the last one to change the subpackage go.mod - apologies if that's the wrong person to drag into this discussion.

I have done some more digging and found out the following:

  1. github.com/prometheus/common/sigv4 has a dependency on github.com/prometheus/common - only for the usage of config.Secret; this is fine.
  2. From all of the Prometheus repos, only github.com/prometheus/prometheus depends on github.com/prometheus/common/sigv4; github.com/prometheus/common, github.com/prometheus/client_golang, and github.com/prometheus/client_model do not. This is good news - no circular dependencies.
  3. github.com/prometheus/common/sigv4 has a dependency on github.com/aws/aws-sdk-go which is already in maintenance mode - and only until 2025-07-31. AWS recommends to move to github.com/aws/aws-sdk-go-v2.

In view of all of these, I'd recommend the following:

  1. Move sigv4 out of the github.com/prometheus/common repository to its own small (github.com/prometheus/sigv4) repository. The new repository will have a dependency on github.com/prometheus/common which is perfectly fine.
  2. Tag and release the new sigv4 repository normally, like any other repository.
  3. Change the dependencies in github.com/prometheus/prometheus repository to the new sigv4 repository - not a huge effort.
  4. Change the implementation in the new repository to use AWS SDK v2 i.s.o. AWS SDK v1.

Please let me know what you think. I may be able to participate in this effort.
I do not know who maintains the sigv4 part - @SuperQ if that's not an issue for you, please tag in comments others. Thank you!

@SuperQ
Copy link
Member

SuperQ commented Oct 25, 2024

I agree, we have quite a number of issues with the current setup. I would like to split the repo.

@tsipo
Copy link
Author

tsipo commented Oct 25, 2024

Thank you @SuperQ . I am not a member of the org. How can we get it going?

@roidelapluie
Copy link
Member

I agree, we have quite a number of issues with the current setup. I would like to split the repo.

fine to me

@SuperQ
Copy link
Member

SuperQ commented Oct 29, 2024

Initial code copy: prometheus/sigv4#1

@SuperQ
Copy link
Member

SuperQ commented Nov 6, 2024

SuperQ added a commit to prometheus/prometheus that referenced this issue Nov 6, 2024
Migrate use of prometheus/common/sigv4 to prometheus/sigv4.

Related to: prometheus/common#709

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Nov 6, 2024
Mark the sigv4 module deprecated in favor of the new repo location.
* https://github.com/prometheus/sigv4

Closes: #709

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit to prometheus/prometheus that referenced this issue Nov 6, 2024
Migrate use of prometheus/common/sigv4 to prometheus/sigv4.

Related to: prometheus/common#709

Signed-off-by: SuperQ <[email protected]>
@SuperQ SuperQ closed this as completed in 6ad2990 Nov 6, 2024
SuperQ added a commit to prometheus/prometheus that referenced this issue Nov 8, 2024
Migrate use of prometheus/common/sigv4 to prometheus/sigv4.

Related to: prometheus/common#709

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit to prometheus/prometheus that referenced this issue Nov 8, 2024
Migrate use of prometheus/common/sigv4 to prometheus/sigv4.

Related to: prometheus/common#709

Signed-off-by: SuperQ <[email protected]>
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

Successfully merging a pull request may close this issue.

3 participants