MON-3231: add metric for the configured profile #2030

rexagod · 2023-07-02T16:22:07Z

- adds a metric with the label value set to the current collection profile.

I added CHANGELOG entry for this change.
No user facing changes, so no entry in CHANGELOG was needed.

openshift-ci-robot · 2023-07-02T16:22:09Z

@rexagod: No Jira issue with key MON-2301 exists in the tracker at https://issues.redhat.com/.
Once a valid jira issue is referenced in the title of this pull request, request a refresh with /jira refresh.

In response to this:

adds a metric with the label value set to the current collection profile.

I added CHANGELOG entry for this change.

No user facing changes, so no entry in CHANGELOG was needed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-07-02T16:23:22Z

@rexagod: This pull request references MON-3231 which is a valid jira issue.

In response to this:

adds a metric with the label value set to the current collection profile.

I added CHANGELOG entry for this change.

No user facing changes, so no entry in CHANGELOG was needed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rexagod · 2023-07-03T07:22:27Z

/test e2e-aws-ovn-single-node

simonpasquier

Since collection profiles are gated via TechPreview, I would only expose the metric if tech preview is explicitly enabled.

I'd also recommend exposing all collection profile values, e.g.

cluster_monitoring_operator_collection_profile{..., collection_profile="full"} 1
cluster_monitoring_operator_collection_profile{..., collection_profile="minimal"} 0

Having 0 for all series would mean that the collection profile is misconfigured.

pkg/metrics/metrics.go

jan--f

Generally this looks good to me. Can you please add separate commits for the lint fix, the metrics refactor and for adding the new metric? This will make future change archeology much easier.
Also this does not add the telemetry config yet, we could add the CMO side for this here too?

One thing I'd like to raise for discussion: Should we maybe add a cluster_monitoring_operator_configuration metric instead of single metrics for config options we're interested in? We're mostly interested in the label we set anyway and I think such a metadata metric can make it easier to add additional signals and send them to telemetry.

pkg/manifests/config.go

jan--f · 2023-07-03T09:02:44Z

One thing I'd like to raise for discussion: Should we maybe add a cluster_monitoring_operator_configuration metric instead of single metrics for config options we're interested in? We're mostly interested in the label we set anyway and I think such a metadata metric can make it easier to add additional signals and send them to telemetry.

Briefly discussed in slack, individual metrics have clear benefits.

rexagod · 2023-07-03T22:27:25Z

Can you please add separate commits for the lint fix, the metrics refactor and for adding the new metric? This will make future change archeology much easier.

@jan--f Does it sound good if do this once the PR is LGTM'd and good to merge? That way I can keep adding any lint fixes or refactors during further reviews and split them all in the end.

Documentation/data-collection.md

pkg/manifests/config.go

pkg/metrics/metrics.go

pkg/manifests/config.go

pkg/metrics/metrics.go

jan--f · 2023-07-05T09:26:24Z

/lgtm
/hold
for better commits :)

simonpasquier

one last nit :)

manifests/0000_50_cluster-monitoring-operator_04-config.yaml

jsonnet/rules.libsonnet

* Allow govet on `Main()`. * Remove trailing whitespaces for telemetry metrics. * Reword description for `manifests.NewConfigFromString`.

pkg/metrics/metrics.go

move meta metrics under the `metrics` package.

add meta metric and telemeter signal that depicts the current state of the various collection profiles and the one that is active, respectively.

openshift-ci · 2023-07-05T22:05:25Z

@rexagod: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

jan--f · 2023-07-06T12:25:50Z

/lgtm

openshift-ci · 2023-07-06T12:26:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jan--f, rexagod

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~~ [jan--f]

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

jan--f · 2023-07-06T12:47:18Z

/unhold

openshift-ci bot requested review from jan--f and raptorsun July 2, 2023 16:23

rexagod force-pushed the 2301 branch from f0956b1 to e3eb059 Compare July 2, 2023 16:23

rexagod changed the title ~~MON-2301: add metric for the configured profile~~ MON-3231: add metric for the configured profile Jul 2, 2023

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 2, 2023

rexagod force-pushed the 2301 branch 5 times, most recently from f254d61 to 700ac82 Compare July 2, 2023 21:33

simonpasquier reviewed Jul 3, 2023

View reviewed changes

pkg/metrics/metrics.go Outdated Show resolved Hide resolved

jan--f reviewed Jul 3, 2023

View reviewed changes

pkg/manifests/config.go Outdated Show resolved Hide resolved

rexagod force-pushed the 2301 branch from 700ac82 to 0184d63 Compare July 3, 2023 22:23

rexagod force-pushed the 2301 branch 2 times, most recently from ae68b0e to 3538917 Compare July 4, 2023 02:04

simonpasquier reviewed Jul 4, 2023

View reviewed changes

rexagod force-pushed the 2301 branch from e43314a to c352488 Compare July 4, 2023 21:18

jan--f suggested changes Jul 5, 2023

View reviewed changes

pkg/metrics/metrics.go Outdated Show resolved Hide resolved

pkg/metrics/metrics.go Outdated Show resolved Hide resolved

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 5, 2023

openshift-ci bot assigned jan--f Jul 5, 2023

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 5, 2023

simonpasquier reviewed Jul 5, 2023

View reviewed changes

manifests/0000_50_cluster-monitoring-operator_04-config.yaml Outdated Show resolved Hide resolved

jsonnet/rules.libsonnet Outdated Show resolved Hide resolved

chore: lint nits

ef04e2f

* Allow govet on `Main()`. * Remove trailing whitespaces for telemetry metrics. * Reword description for `manifests.NewConfigFromString`.

rexagod force-pushed the 2301 branch from ed96306 to 6d4f8f2 Compare July 5, 2023 18:39

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 5, 2023

rexagod commented Jul 5, 2023

View reviewed changes

pkg/metrics/metrics.go Show resolved Hide resolved

rexagod added 2 commits July 6, 2023 00:57

*: refactor meta metrics

0dfc332

move meta metrics under the `metrics` package.

feat: add meta metric and telemeter signal for collection profiles

9001b91

add meta metric and telemeter signal that depicts the current state of the various collection profiles and the one that is active, respectively.

rexagod force-pushed the 2301 branch from 6d4f8f2 to 9001b91 Compare July 5, 2023 19:35

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

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 6, 2023

openshift-merge-robot merged commit 60f4899 into openshift:master Jul 6, 2023
16 checks passed

jan--f mentioned this pull request Jul 17, 2023

MON-3274: sync telemeter config with apiserver_list_watch_request_success_total:rate:sum rhobs/configuration#551

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MON-3231: add metric for the configured profile #2030

MON-3231: add metric for the configured profile #2030

rexagod commented Jul 2, 2023

openshift-ci-robot commented Jul 2, 2023

openshift-ci-robot commented Jul 2, 2023 •

edited by openshift-ci bot

rexagod commented Jul 3, 2023

simonpasquier left a comment

jan--f left a comment

jan--f commented Jul 3, 2023

rexagod commented Jul 3, 2023

jan--f commented Jul 5, 2023

simonpasquier left a comment

openshift-ci bot commented Jul 5, 2023

jan--f commented Jul 6, 2023

openshift-ci bot commented Jul 6, 2023

jan--f commented Jul 6, 2023

MON-3231: add metric for the configured profile #2030

MON-3231: add metric for the configured profile #2030

Conversation

rexagod commented Jul 2, 2023

openshift-ci-robot commented Jul 2, 2023

openshift-ci-robot commented Jul 2, 2023 • edited by openshift-ci bot

rexagod commented Jul 3, 2023

simonpasquier left a comment

Choose a reason for hiding this comment

jan--f left a comment

Choose a reason for hiding this comment

jan--f commented Jul 3, 2023

rexagod commented Jul 3, 2023

jan--f commented Jul 5, 2023

simonpasquier left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jul 5, 2023

jan--f commented Jul 6, 2023

openshift-ci bot commented Jul 6, 2023

jan--f commented Jul 6, 2023

openshift-ci-robot commented Jul 2, 2023 •

edited by openshift-ci bot