export storageconsumer data as metrics and alerts #2227

leelavg · 2023-10-23T08:56:01Z

encode the versions from storageconsumer CR status to comparable numbers
export these numbers are metrics
create alerts for version incompatbility and client checkin via heartbeats

openshift-ci · 2023-10-23T08:56:06Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

leelavg · 2023-10-23T08:56:16Z

only last commit is reviewable for now.

leelavg · 2023-10-23T13:28:17Z

@aruniiird, @jmolmo could you pls review last commit in this PR?

jmolmo · 2023-10-24T08:43:32Z

metrics/internal/collectors/storageconsumer.go

 	for _, storageConsumer := range storageConsumers {
 		ch <- prometheus.MustNewConstMetric(c.StorageConsumerMetadata,
 			prometheus.GaugeValue, 1,
 			storageConsumer.Name,
 			string(storageConsumer.Status.State))
+
+		ch <- prometheus.MustNewConstMetric(c.LastHeartbeat,
+			prometheus.GaugeValue, float64(storageConsumer.Status.LastHeartbeat.Time.Unix()),


Probably a heartbeat based on a timestamp would be of type "counter" instead "gauge". Do you expect to have decrements on this metric?

There'll be no decrements, however _count and _sum derived metrics from a counter type doesn't mean anything in this context and so I used Gauge to represent the instant value with no correlation to previous values.

metrics/mixin/alerts/storage-client.libsonnet

jmolmo

Probably you need to replace the var in the "critical" alert

Please add documentation about the new alerts in the runbooks repo.
Here you have an example for CephClusterCriticallyFull alert

Besides that, Alerts in OpenShift will ship a link to the corresponding runbook in this repository, to make fixing problems even easier. Link should be passed as runbook_url field in alert annotations
See the example

metrics/mixin/alerts/storage-client.libsonnet

leelavg · 2023-10-25T04:48:02Z

Please add documentation about the new alerts in the runbooks repo.

@jmolmo, can this be deferred to a later point, ie, after 4.14 and before 4.14.z as we require the PR to be feature complate
And also we aren't merging to main as this will be a custom build until release and I'll definitely raise PRs for the runbooks after this PR gets merged as I don't want to update at two places in parallel if we require changes to the alerts wrt pr comments

umangachapagain

Lots of unrelated changes in the alert yaml files, please remove it from commit.
Looks good otherwise.

leelavg · 2023-10-30T10:33:56Z

Lots of unrelated changes in the alert yaml files, please remove it from commit. Looks good otherwise.

I just followed README and these are generated when I ran corresponding make targets
Ran pending make command and fix obc alerts issue #2225 should probably fix it, anyways, if these are generated during build I can remove from this PR, can I?

jmolmo · 2023-10-30T10:50:54Z

Lots of unrelated changes in the alert yaml files, please remove it from commit. Looks good otherwise.

I just followed README and these are generated when I ran corresponding make targets

Ran 'make gen-latest-prometheus-rules-yamls' command #2225 should probably fix it, anyways, if these are generated during build I can remove from this PR, can I?

I think, everything will be more clear if we merge first:
#2225

@umangachapagain can you review it and approve if it is ok?

leelavg · 2023-10-30T11:28:57Z

@jmolmo both the branches differ and for now if they are generated during build time then this PR isn't dependent on any other.

jmolmo · 2023-10-30T11:52:50Z

@jmolmo both the branches differ and for now if they are generated during build time then this PR isn't dependent on any other.

It seems that the modifications in the rules file are not generated during the build, you need to execute explicitly the command "make gen-latest-prometheus-rules-yamls":
#2225 is set because it was forgotten to execute it.
If you want to add your rules, you will need also to execute the make command an you will generate your modifications and also the modifications in #2225.
This is why I say that in order to make thing easier for ourselves in the future, probably #2225 should be merged first.

leelavg · 2023-10-31T11:44:42Z

It seems that the modifications in the rules file are not generated during the build

ack, got it. Here we are in a different situation, Ran pending make command and fix obc alerts issue #2225 was raised for a different branch, for the HCI effort pls consider the current base branch (fusion-hci-4.14) is entirely different
so I added the changes generated after following README to this PR itself
when we take in changes from main, these changes shouldn't conflict (mostly) as they are generated by same tooling

I've rebased on the base branch and this PR can receive your votes for the merge if you are fine with it @jmolmo @umangachapagain

thanks.

metrics/deploy/prometheus-ocs-rules.yaml

metrics/deploy/rbac.yaml

- encode the versions from storageconsumer CR status to comparable numbers - export these numbers are metrics - create alerts for version incompatbility and client checkin via heartbeats Signed-off-by: Leela Venkaiah G <[email protected]>

umangachapagain · 2023-10-31T12:43:27Z

metrics/deploy/rbac.yaml

+  resources:
+  - clusterversions
+  verbs:
+    - get
+    - list
+    - watch


These are still not added to the other file.

I don't think so, already added to exporter-role.yaml

openshift-ci · 2023-10-31T12:58:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leelavg, umangachapagain

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [umangachapagain]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 23, 2023

jmolmo reviewed Oct 24, 2023

View reviewed changes

metrics/mixin/alerts/storage-client.libsonnet Outdated Show resolved Hide resolved

jmolmo suggested changes Oct 24, 2023

View reviewed changes

metrics/mixin/alerts/storage-client.libsonnet Outdated Show resolved Hide resolved

openshift-ci bot assigned jmolmo Oct 24, 2023

leelavg force-pushed the csi-metrics-1 branch 7 times, most recently from ade1461 to 02822e3 Compare October 30, 2023 09:47

umangachapagain requested changes Oct 30, 2023

View reviewed changes

openshift-ci bot assigned umangachapagain Oct 30, 2023

leelavg force-pushed the csi-metrics-1 branch from 02822e3 to 84c4ee3 Compare October 30, 2023 10:40

leelavg changed the title ~~export storageconsumer data as metrics~~ export storageconsumer data as metrics and alerts Oct 31, 2023

leelavg force-pushed the csi-metrics-1 branch from 84c4ee3 to 4d7c089 Compare October 31, 2023 06:48

nb-ohad force-pushed the fusion-hci-4.14 branch from 7d4fda1 to 9fbb0c7 Compare October 31, 2023 08:09

leelavg force-pushed the csi-metrics-1 branch from 4d7c089 to b25dbbf Compare October 31, 2023 11:38

leelavg marked this pull request as ready for review October 31, 2023 11:40

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2023

leelavg force-pushed the csi-metrics-1 branch from b25dbbf to 06a407f Compare October 31, 2023 11:50

umangachapagain requested changes Oct 31, 2023

View reviewed changes

leelavg force-pushed the csi-metrics-1 branch from 06a407f to 86e8069 Compare October 31, 2023 12:33

umangachapagain reviewed Oct 31, 2023

View reviewed changes

umangachapagain approved these changes Oct 31, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 31, 2023

openshift-ci bot merged commit c4698e0 into red-hat-storage:fusion-hci-4.14 Oct 31, 2023
6 checks passed

leelavg deleted the csi-metrics-1 branch December 11, 2024 11:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

export storageconsumer data as metrics and alerts #2227

export storageconsumer data as metrics and alerts #2227

leelavg commented Oct 23, 2023 •

edited

Loading

openshift-ci bot commented Oct 23, 2023

leelavg commented Oct 23, 2023

leelavg commented Oct 23, 2023

jmolmo Oct 24, 2023

leelavg Oct 24, 2023

jmolmo left a comment

leelavg commented Oct 25, 2023

umangachapagain left a comment

leelavg commented Oct 30, 2023

jmolmo commented Oct 30, 2023

leelavg commented Oct 30, 2023

jmolmo commented Oct 30, 2023

leelavg commented Oct 31, 2023

umangachapagain Oct 31, 2023

leelavg Oct 31, 2023

openshift-ci bot commented Oct 31, 2023

export storageconsumer data as metrics and alerts #2227

export storageconsumer data as metrics and alerts #2227

Conversation

leelavg commented Oct 23, 2023 • edited Loading

openshift-ci bot commented Oct 23, 2023

leelavg commented Oct 23, 2023

leelavg commented Oct 23, 2023

jmolmo Oct 24, 2023

Choose a reason for hiding this comment

leelavg Oct 24, 2023

Choose a reason for hiding this comment

jmolmo left a comment

Choose a reason for hiding this comment

leelavg commented Oct 25, 2023

umangachapagain left a comment

Choose a reason for hiding this comment

leelavg commented Oct 30, 2023

jmolmo commented Oct 30, 2023

leelavg commented Oct 30, 2023

jmolmo commented Oct 30, 2023

leelavg commented Oct 31, 2023

umangachapagain Oct 31, 2023

Choose a reason for hiding this comment

leelavg Oct 31, 2023

Choose a reason for hiding this comment

openshift-ci bot commented Oct 31, 2023

leelavg commented Oct 23, 2023 •

edited

Loading