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

router: fix prometheus metrics high cardinality issue #789

Merged
merged 5 commits into from
Mar 5, 2025

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 <[email protected]>
@scbizu scbizu force-pushed the fix/high-high-cardinality branch from f81ffa4 to 9e3de5b Compare September 21, 2024 11:22
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 <[email protected]>
@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 👋

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
3 participants