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

fix policyfilter metrics #2282

Merged
merged 3 commits into from
Apr 5, 2024
Merged

fix policyfilter metrics #2282

merged 3 commits into from
Apr 5, 2024

Conversation

kkourt
Copy link
Contributor

@kkourt kkourt commented Apr 2, 2024

323ec1e removed the error label from metrics. This PR reverts this patch and fixes the issue of potentially high cardinality in metrics.

@kkourt kkourt added the release-note/misc This PR makes changes that have no direct user impact. label Apr 2, 2024
kkourt added 2 commits April 2, 2024 14:32
This reverts commit 425f91e.

Signed-off-by: Kornilios Kourtis <[email protected]>
Commit 425f91e ("policyfiltermetrics:
Remove error label") was reverted in the previous patch included some
misc fixes for log messages. Add them here.

Signed-off-by: Kornilios Kourtis <[email protected]>
@kkourt kkourt force-pushed the pr/kkourt/policyfilter-errors branch 2 times, most recently from f896496 to 4366929 Compare April 3, 2024 06:12
@kkourt kkourt changed the title fix policyfilter errors. fix policyfilter metrics Apr 3, 2024
@kkourt kkourt force-pushed the pr/kkourt/policyfilter-errors branch 2 times, most recently from 3f78912 to 05bec85 Compare April 3, 2024 06:26
Copy link

netlify bot commented Apr 3, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit c0fe8ab
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/660fabcdadd58f0008f88512
😎 Deploy Preview https://deploy-preview-2282--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kkourt kkourt marked this pull request as ready for review April 3, 2024 06:29
@kkourt kkourt requested review from mtardy and a team as code owners April 3, 2024 06:29
Copy link
Contributor

@lambdanis lambdanis left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this! I left two comments, ideally we should init the metric with all possible label combinations.

pkg/metrics/policyfiltermetrics/policyfiltermetrics.go Outdated Show resolved Hide resolved
pkg/metrics/policyfiltermetrics/policyfiltermetrics.go Outdated Show resolved Hide resolved
@lambdanis lambdanis added the area/metrics Related to prometheus metrics label Apr 3, 2024
This patch implements ErrorLabel() for generating error labels for
metrics. This fixes an issue with the previous implementation where the
cardinality of the error_type label was potentially unbounded.

Signed-off-by: Kornilios Kourtis <[email protected]>
@kkourt kkourt force-pushed the pr/kkourt/policyfilter-errors branch from 05bec85 to c0fe8ab Compare April 5, 2024 07:44
@kkourt kkourt requested a review from lambdanis April 5, 2024 07:52
Copy link
Contributor

@lambdanis lambdanis left a comment

Choose a reason for hiding this comment

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

looks good!

@kkourt kkourt merged commit bd63a46 into main Apr 5, 2024
41 checks passed
@kkourt kkourt deleted the pr/kkourt/policyfilter-errors branch April 5, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Related to prometheus metrics release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants