[Fix] OpenSearch and OpenSearch Dashboards Service Monitor Bug #581
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: VILJkid <[email protected]>
VILJkid
requested review from
TheAlgo,
DandyDeveloper,
peterzhuamazon,
prudhvigodithi and
gaiksaya
as code owners
VILJkid
changed the title
port Value Bug| @@ -92,9 +92,9 @@ helm uninstall my-release | |||
| | `opensearchDashboardsYml.defaultMode` | Allow you to set the defaultMode for the opensearch_dashboards.yml mounted as configMap | | | |||
| | `dashboardAnnotations` | Allows you to configure custom annotation in the deployement of the OpenSearchDashboards container | {} | | |||
| | `serviceMonitor.enabled` | Enables the creation of a [ServiceMonitor] resource for Prometheus monitoring. Requires the Prometheus Operator to be installed in your Kubernetes cluster. | `false` | | |||
| | `serviceMonitor.portName` | Name of the port in the OpenSearch Dashboards service that exposes metrics. This should match the port name defined in your OpenSearch service configuration. Applicable only if `serviceMonitor.enabled` is set to `true`. | `metrics` | | |||
There was a problem hiding this comment.
Does this mean the name of serviceMonitor.portName should be same as OpenSearch service created by the chart ? If so the value passed is portName: metrics, so is metrics is the OpenSearch service name?
There was a problem hiding this comment.
Yes. Correct. metrics is the name of the OpenSearch service which exposes the metrics.
Reference from the existing Service template:
There was a problem hiding this comment.
Got it thanks @VILJkid then should we use the same syntax ?
{{ .Values.service.metricsPortName | default "metrics" }}
@peterzhuamazon @TheAlgo @getsaurabh02
There was a problem hiding this comment.
Sounds good! Then creating portName won't be necessary. We can directly use the syntax to fetch the service name.
Thanks @prudhvigodithi! I'll make the necessary changes.
There was a problem hiding this comment.
Thanks @prudhvigodithi,
The changes are implemented in the latest commits. Kindly review and let me know if it needs any further changes.
eyenx
approved these changes
Aug 26, 2024
3 tasks
…arch servicemonitor Signed-off-by: VILJkid <[email protected]>
…ch dashboards servicemonitor Signed-off-by: VILJkid <[email protected]>
Signed-off-by: VILJkid <[email protected]>
VILJkid
referenced
this pull request
Aug 27, 2024
* feat: Add serviceMonitor resource Signed-off-by: Shubham Gupta <[email protected]> Signed-off-by: VILJkid <[email protected]> * fix: metrics path Signed-off-by: Shubham Gupta <[email protected]> Signed-off-by: VILJkid <[email protected]> * (lint) : trailing newline Signed-off-by: VILJkid <[email protected]> * (update) : readme and changelog with comments Signed-off-by: VILJkid <[email protected]> * (add) : opensearch-dashboard for serviceMonitor Signed-off-by: VILJkid <[email protected]> * (chore) : bump changelog version Signed-off-by: VILJkid <[email protected]> * (chore) : bump version Signed-off-by: VILJkid <[email protected]> * (lint) : add newline Signed-off-by: VILJkid <[email protected]> * (fix) : changelog compare versions Signed-off-by: VILJkid <[email protected]> * (fix) : bump minor versions instead of patch Signed-off-by: VILJkid <[email protected]> --------- Signed-off-by: Shubham Gupta <[email protected]> Signed-off-by: VILJkid <[email protected]> Co-authored-by: Shubham Gupta <[email protected]> Co-authored-by: VILJkid <[email protected]>
schroedermatthias
approved these changes
Aug 28, 2024
prudhvigodithi
approved these changes
Aug 28, 2024
prudhvigodithi
added
the
backport 1.x
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR addresses a bug in the
ServiceMonitortemplate forOpenSearchandOpenSearch Dashboards. Previously, theportconfiguration incorrectly used a port number, butServiceMonitorrequires a port name as a string. The fix updates the configuration to use the correct port name, ensuring that Prometheus can properly scrape metrics from theOpenSearchservice.Issues Resolved
#579
Check List
For any changes to files within Helm chart directories:
CHANGELOG.mdupdated to reflect changePR sponsored by Obmondo