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

feat: add serviceMonitor in the helm chart #485

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

SamYSF
Copy link
Contributor

@SamYSF SamYSF commented Jun 13, 2024

Fixes #358

Create a serviceMonitor in the helm chart to expose the Promethus metrics.
image

@LinuxSuRen LinuxSuRen added enhancement New feature or request ospp 开源之夏 https://summer-ospp.ac.cn/ labels Jun 13, 2024
Copy link
Owner

Choose a reason for hiding this comment

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

This CRD is not in the core of Kubernetes. It would be better if you could add a conditional statement. Have it only when enable this feature. Actually, you already have that value but you didn't use it here. Or just forget?

@@ -109,3 +109,15 @@ affinity: {}

mongodb:
enabled: false

serviceMonitor:
enabled: true
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer to change this default value to false because This CRD is not in the core of Kubernetes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to false and used.

Comment on lines 115 to 116
interval: 15s
path: /metrics
Copy link
Owner

Choose a reason for hiding this comment

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

Forget to use these values?

Comment on lines 120 to 123
prometheus:
prometheusSpec:
podMonitorSelectorNilUsesHelmValues: false
serviceMonitorSelectorNilUsesHelmValues: false
Copy link
Owner

Choose a reason for hiding this comment

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

Add here.

@LinuxSuRen
Copy link
Owner

By the way, the e2e testing has failed.

Copy link

sonarcloud bot commented Jun 13, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@LinuxSuRen LinuxSuRen enabled auto-merge (squash) June 13, 2024 09:53
@LinuxSuRen LinuxSuRen disabled auto-merge June 13, 2024 09:55
@LinuxSuRen LinuxSuRen merged commit 2ba9c11 into LinuxSuRen:master Jun 13, 2024
14 checks passed
@SamYSF SamYSF deleted the add-service-monitor branch June 14, 2024 10:41
LinuxSuRen pushed a commit that referenced this pull request Jun 17, 2024
* chore(deps): update metabase/metabase docker tag to v0.47.4

* Update app version [skip ci]

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: github-action update-app-version <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ospp 开源之夏 https://summer-ospp.ac.cn/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a serviceMonitor in the helm chart
2 participants