Skip to content

Conversation

@scbizu
Copy link
Contributor

@scbizu scbizu commented Sep 21, 2024

@scbizu scbizu added the dependencies Pull requests that update a dependency file label Sep 21, 2024
@scbizu scbizu self-assigned this Sep 21, 2024
@scbizu scbizu marked this pull request as draft September 21, 2024 11:11
@scbizu scbizu force-pushed the fix/high-high-cardinality branch 2 times, most recently from ce3a8f2 to f81ffa4 Compare September 21, 2024 11:16
Signed-off-by: scbizu <scbizu@gmail.com>
@scbizu scbizu force-pushed the fix/high-high-cardinality branch from f81ffa4 to 9e3de5b Compare September 21, 2024 11:22
Signed-off-by: scbizu <scbizu@gmail.com>
Signed-off-by: scbizu <scbizu@gmail.com>
Copy link
Contributor

@cbuto cbuto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@AnotherKamila
Copy link

Sorry to add noise, but any reason this hasn't been merged? The metrics cardinality issue is affecting my production, so I'd be happy to help get this merged.

@scbizu
Copy link
Contributor Author

scbizu commented Mar 4, 2025

@AnotherKamila Sorry , I've almost forgot this PR , really busy these days 😣 . I will back to this again .

* deprecate the external dependency of go-gin-prometheus
* use zap logger(cm) replace of logrus logger(go-gin-prometheus) for better investigation

Signed-off-by: scbizu <scbizu@gmail.com>
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Mar 4, 2025
@scbizu scbizu requested a review from cbuto March 4, 2025 13:31
@scbizu
Copy link
Contributor Author

scbizu commented Mar 4, 2025

Hi @cbuto , as we talked before (if I remember it's right) , I've moved the prometheus middleware inside CM , and remove the outside dependency as it is not maintained recently. Please take a look again ?

(And I add the toolchain in go.mod 😆)

@scbizu scbizu marked this pull request as ready for review March 4, 2025 13:35
@scbizu scbizu merged commit 5e32c8d into main Mar 5, 2025
2 checks passed
@scbizu scbizu deleted the fix/high-high-cardinality branch March 5, 2025 02:22
@scbizu
Copy link
Contributor Author

scbizu commented Mar 5, 2025

@AnotherKamila Try to bump to the canary image to get the bug fixed 👋

dmytrop-salt pushed a commit to Secful/chartmuseum that referenced this pull request Oct 13, 2025
* router: fix prometheus metrics high cardinality issue

Closes helm#448
Refers to [CVE(internal)](https://github.com/helm/chartmuseum/security/advisories/GHSA-67g5-8r7h-h9qv)

Signed-off-by: scbizu <scbizu@gmail.com>

* mod: bump

Signed-off-by: scbizu <scbizu@gmail.com>

* ci: build PR workflow bumps to Go 1.23

Signed-off-by: scbizu <scbizu@gmail.com>

* server: fixup tests

Signed-off-by: scbizu <scbizu@gmail.com>

* router: move prometheus middleware inside CM

* deprecate the external dependency of go-gin-prometheus
* use zap logger(cm) replace of logrus logger(go-gin-prometheus) for better investigation

Signed-off-by: scbizu <scbizu@gmail.com>

---------

Signed-off-by: scbizu <scbizu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Possibly high cardinality on chartmuseum_requests_total labels

4 participants