MetricsCollectionProfiles: Reword and update KEP
#1791
Conversation
rexagod
force-pushed
the
metrics-collection-profiles
branch
from
559e546 to
34d451e
Compare
|
/cc @JoaoBraveCoding Requesting a review here. If things look good to you, I'll request the API folks to take a look. 🙂 |
ServiceMonitor examples Signed-off-by: JoaoBraveCoding <jmarcal@redhat.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Signed-off-by: JoaoBraveCoding <jmarcal@redhat.com>
Co-authored-by: Pranshu Srivastava <prasriva@redhat.com>
Co-authored-by: Pranshu Srivastava <prasriva@redhat.com>
added documentation for the CLI utility based on the recent discussion. Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
Co-authored-by: Joao Marcal <joao.marcal12@gmail.com>
Co-authored-by: Joao Marcal <joao.marcal12@gmail.com>
Co-authored-by: Joao Marcal <joao.marcal12@gmail.com>
Co-authored-by: Junqi Zhao <juzhao@redhat.com>
rexagod
force-pushed
the
metrics-collection-profiles
branch
2 times, most recently
from
44fc0aa to
4670400
Compare
Re-opening the KEP PR to backfill on the required proposal context. Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
rexagod
force-pushed
the
metrics-collection-profiles
branch
from
4670400 to
577a948
Compare
|
@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-sigs/prow repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JoaoBraveCoding The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
| data: | ||
| config.yaml: | | ||
| prometheusK8s: | ||
| collectionProfile: full |
There was a problem hiding this comment.
Enum values should be PascalCase
| // metrics that are exposed by the platform components. In the `minimal` | ||
| // profile, Prometheus only collects metrics necessary for the default | ||
| // platform alerts, recording rules, telemetry and console dashboards. | ||
| CollectionProfile CollectionProfile `json:"collectionProfile,omitempty"` |
There was a problem hiding this comment.
Is this field required or optional?
What happens when it is not set?
How does the upgrade work for existing clusters, is there any action needed?
| reviewers: | ||
| - openshift/openshift-team-monitoring | ||
| approvers: | ||
| - "@openshift/openshift-team-monitoring" |
There was a problem hiding this comment.
Should be a single person
| approvers: | ||
| - "@openshift/openshift-team-monitoring" | ||
| api-approvers: | ||
| - "@dgrisonnet" |
There was a problem hiding this comment.
Damien is not an API approver, this should be me, or David Eads
| Given this, the following steps would be necessary to implement this | ||
| enhancement: | ||
|
|
||
| - expose a configuration option in the cluster-monitoring-operator ConfigMap |
There was a problem hiding this comment.
Given we are moving away from the configmap to something more structured, what impact does adding new configmap items have on that effort? It can only hinder the effort no?
| - monitors with the | ||
| `monitoring.openshift.io/collection-profile: <selected profile>` label. | ||
| - monitors without the `monitoring.openshift.io/collection-profile` profile | ||
| label present, to retain the default behaviour (for components that didn't | ||
| opt-in for metrics collection profile). |
There was a problem hiding this comment.
Does this mechanism allow a component to disable scraping altogether? Or just to enable filtering the metrics it exposes? Or both?
What implementation is needed from the operator dev, is it just the yaml, or do they need operator/operand changes too?
|
|
||
| OpenShift teams can decide if they want to adopt this feature. Without any | ||
| change to a monitor, if a user picks a profile in the CMO config, things | ||
| will work as they did before. When an OpenShift team wants to implement |
There was a problem hiding this comment.
If you decide later to add an additional profile, how would that impact existing teams? What would you have to do before you could introduce the new profile?
| metricRelabelings: | ||
| - sourceLabels: [__name__] | ||
| action: keep | ||
| regex: "federate_samples|federate_filtered_samples" |
There was a problem hiding this comment.
If I end up with a very large regex, can I split this into multiple entries within the metricRelabelings list?
There was a problem hiding this comment.
not directly but you can do tricks to split a long expression into several steps
metricRelabelings:
- sourceLabels: [__name__]
action: replace
replacement: "keep"
regex: "federate_samples|federate_filtered_samples"
targetLabel: __tmp_keep_or_drop
- sourceLabels: [__name__]
action: replace
regex: "foo|bar"
replacement: "keep"
targetLabel: __tmp_keep_or_drop
- action: replace
regex: "keep"
replacement: ""
Re-opening the KEP PR to backfill on the required proposal context.
Signed-off-by: Pranshu Srivastava rexagod@gmail.com
Continues: #1298