[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 <sidvjmostwanted@gmail.com>
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 <sidvjmostwanted@gmail.com>
…ch dashboards servicemonitor Signed-off-by: VILJkid <sidvjmostwanted@gmail.com>
Signed-off-by: VILJkid <sidvjmostwanted@gmail.com>
VILJkid
referenced
this pull request
Aug 27, 2024
* feat: Add serviceMonitor resource Signed-off-by: Shubham Gupta <sgupta@obmondo.com> Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> * fix: metrics path Signed-off-by: Shubham Gupta <sgupta@obmondo.com> Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> * (lint) : trailing newline Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> * (update) : readme and changelog with comments Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> * (add) : opensearch-dashboard for serviceMonitor Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> * (chore) : bump changelog version Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> * (chore) : bump version Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> * (lint) : add newline Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> * (fix) : changelog compare versions Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> * (fix) : bump minor versions instead of patch Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> --------- Signed-off-by: Shubham Gupta <sgupta@obmondo.com> Signed-off-by: VILJkid <sidvjmostwanted@gmail.com> Co-authored-by: Shubham Gupta <sgupta@obmondo.com> Co-authored-by: VILJkid <sidvjmostwanted@gmail.com>
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