feat(plugins): additional helm controller metrics #770
+215
−1
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
gciezkowski-acc
changed the title
gciezkowski-acc
force-pushed
the
feat/439_additional_helm_controller_metrics
branch
from
49dcb61
to
7d4c08f
Compare
gciezkowski-acc
force-pushed
the
feat/439_additional_helm_controller_metrics
branch
from
7e1bf73
to
a6c4de4
Compare
IvoGoman
requested changes
Nov 27, 2024
Comment on lines
77
to
81
| - alert: GreenhousePluginHasErrors | ||
| annotations: | ||
| summary: "Plugin has errors" | ||
| description: "Plugin {{ $labels.plugin }} in organization {{ $labels.organization }} has more then 10 errors" | ||
| expr: increase(greenhouse_plugin_reconcile_total{result="error"}[15m] > 0) by (plugin, organization) |
There was a problem hiding this comment.
Suggested change
| - alert: GreenhousePluginHasErrors | |
| annotations: | |
| summary: "Plugin has errors" | |
| description: "Plugin {{ $labels.plugin }} in organization {{ $labels.organization }} has more then 10 errors" | |
| expr: increase(greenhouse_plugin_reconcile_total{result="error"}[15m] > 0) by (plugin, organization) | |
| - alert: GreenhousePluginConstantlyFailing | |
| annotations: | |
| summary: "Plugin reconciliation is constantly failing" | |
| description: "Plugin {{ $labels.plugin }} in organization {{ $labels.organization }} keeps failing with {{ $labels.reason}}" | |
| expr: sum by (organization, plugin) (rate(greenhouse_plugin_reconcile_total{result="error"}[5m])) > 0 | |
| for: 15m |
pkg/controllers/plugin/metrics.go
Outdated
Comment on lines
49
to
55
| case greenhousev1alpha1.HelmDriftDetectedCondition: | ||
| if condition.IsTrue() { | ||
| result = metricResultError | ||
| reason = metricReasonDiffFailed | ||
| } | ||
| case greenhousev1alpha1.StatusUpToDateCondition: | ||
| if condition.IsFalse() { | ||
| result = metricResultError | ||
| reason = metricReasonUpgradeFailed | ||
| } |
There was a problem hiding this comment.
These are not reflecting errors in reconciling the helm release.
pkg/controllers/plugin/metrics.go
Outdated
Comment on lines
61
to
68
|
|
||
| pluginReconcileTotalLabels := prometheus.Labels{ | ||
| "pluginDefinition": plugin.Spec.PluginDefinition, | ||
| "clusterName": plugin.Spec.ClusterName, | ||
| "plugin": plugin.Name, | ||
| "organization": plugin.Namespace, | ||
| "result": result, | ||
| "reason": reason, | ||
| } | ||
| pluginReconcileTotal.With(pluginReconcileTotalLabels).Inc() | ||
| } |
There was a problem hiding this comment.
Currently, it is not possible to know why the HelmReconcileFailedCondition is set to true.
Could you instead increase the metrics whenever the condition is set to True, False in the code.
Suggested change
| pluginReconcileTotalLabels := prometheus.Labels{ | |
| "pluginDefinition": plugin.Spec.PluginDefinition, | |
| "clusterName": plugin.Spec.ClusterName, | |
| "plugin": plugin.Name, | |
| "organization": plugin.Namespace, | |
| "result": result, | |
| "reason": reason, | |
| } | |
| pluginReconcileTotal.With(pluginReconcileTotalLabels).Inc() | |
| } | |
| type( | |
| metricResult string | |
| metricReason string | |
| ) | |
| const ( | |
| success metricResult = "success" | |
| error metricResult = "error" | |
| templateFailed metricReason = "template_failed" | |
| diffFailed metricReason = "diff_failed" | |
| upgradeFailed metricReason = "upgrade_failed" | |
| uninstallFailed metricReason = "uninstall_failed" | |
| ) | |
| func updateMetrics(plugin *greenhousev1alpha1.Plugin, result metricResult, reason metricReason) { | |
| pluginReconcileTotalLabels := prometheus.Labels{ | |
| "pluginDefinition": plugin.Spec.PluginDefinition, | |
| "clusterName": plugin.Spec.ClusterName, | |
| "plugin": plugin.Name, | |
| "organization": plugin.Namespace, | |
| "result": result, | |
| "reason": reason, | |
| } | |
| pluginReconcileTotal.With(pluginReconcileTotalLabels).Inc() | |
| } |
gciezkowski-acc
force-pushed
the
feat/439_additional_helm_controller_metrics
branch
from
88770d2
to
d91bd71
Compare
gciezkowski-acc
force-pushed
the
feat/439_additional_helm_controller_metrics
branch
from
989be64
to
c9e05e2
Compare
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
I added additional metric for Helm Controller to detect success or error for helm reconciliation.
What type of PR is this? (check all applicable)
Related Tickets & Documents
Added tests?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Added to documentation?
Checklist