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

#4253 - Upgrade Redis - metrics #4322

Merged
merged 3 commits into from
Feb 6, 2025
Merged

Conversation

guru-aot
Copy link
Collaborator

@guru-aot guru-aot commented Feb 6, 2025

Enable Metrics to check alerts in sysdig.
Currently only dev namespaces enabled, to test the metrics.

image

Ignore the below files, as they are copied from bitnami to enable metrics
image

@guru-aot guru-aot self-assigned this Feb 6, 2025
@guru-aot guru-aot added Devops Devops DB DB migration involved Technical Debt labels Feb 6, 2025
Copy link
Collaborator

@bidyashish bidyashish left a comment

Choose a reason for hiding this comment

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

lgtm

enabled: true
## Enable this if you're using https://github.com/coreos/prometheus-operator
##
serviceMonitor:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is serviceMonitor required to have metrics enabled?

Copy link
Collaborator Author

@guru-aot guru-aot Feb 6, 2025

Choose a reason for hiding this comment

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

yes to expose the metrics to prometheus operator, as we are not using it, disabling it

Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot Feb 6, 2025

Choose a reason for hiding this comment

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

So, the answer to the question "Is serviceMonitor required to have metrics enabled" is NO?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes the answer is NO :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest to have it disabled here also.

Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot 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 looking into it. Just minor questions, please take a look.

Copy link

sonarqubecloud bot commented Feb 6, 2025

Copy link

github-actions bot commented Feb 6, 2025

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 22.23% ( 3902 / 17551 )
Methods: 10.14% ( 226 / 2229 )
Lines: 25.57% ( 3368 / 13173 )
Branches: 14.33% ( 308 / 2149 )

Copy link

github-actions bot commented Feb 6, 2025

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 65.59% ( 589 / 898 )
Methods: 59.63% ( 65 / 109 )
Lines: 68.72% ( 468 / 681 )
Branches: 51.85% ( 56 / 108 )

Copy link

github-actions bot commented Feb 6, 2025

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 87.15% ( 1404 / 1611 )
Methods: 84.66% ( 160 / 189 )
Lines: 89.38% ( 1161 / 1299 )
Branches: 67.48% ( 83 / 123 )

Copy link

github-actions bot commented Feb 6, 2025

E2E SIMS API Coverage Report

Totals Coverage
Statements: 67.8% ( 6063 / 8943 )
Methods: 65.24% ( 745 / 1142 )
Lines: 71.64% ( 4751 / 6632 )
Branches: 48.5% ( 567 / 1169 )

Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot 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 making the changes, look good 👍

@guru-aot guru-aot added this pull request to the merge queue Feb 6, 2025
Merged via the queue into main with commit 6a8cf23 Feb 6, 2025
21 checks passed
@guru-aot guru-aot deleted the feature/#4253-Upgrade_Redis-Metrics branch February 6, 2025 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DB DB migration involved Devops Devops Technical Debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants