-
Notifications
You must be signed in to change notification settings - Fork 370
OCPBUGS-12903: Add new web console usage metrics #1910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCPBUGS-12903: Add new web console usage metrics #1910
Conversation
@jerolimov: This pull request references ODC-7258 which is a valid jira issue. In response to this:
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. |
f473c15
to
4fce377
Compare
@jerolimov: This pull request references ODC-7258 which is a valid jira issue. In response to this:
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. |
3df5569
to
436884f
Compare
@jerolimov: This pull request references ODC-7258 which is a valid jira issue. In response to this:
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. |
436884f
to
64b5fc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @jerolimov thanks for the comprehensive context and the drive-by typo fixes 😄
Can you please add some more information on possible label value sets? We are usually trying to only ingest bounded value sets.
For example perspective
: I suppose this will be either admin
or dev
? state
will likely one of enabled
, disabled
and maybe failed
or so?
role
, reason
, event
, name
(plugin name) seem like they have the potential to have unbounded value sets. Is that the case? If so we need a strong argument, why that is needed or we work on the recording rules to make those value sets finite.
Hey @jan--f, thanks for the review. 😄
Sure, FYI, I tried to explain it in this google docs as well.
For console_customization_perspectives_info
Whenever this escalates we are fine to reduce this to In the
The reason for failures is currently always Internally we have some more errors which we might want to use. But we (I) decided to just use If needed we could add reasons like login
There aren't other events planned at the moment.
This is maybe more unbound than perspectives, as the console supports "dynamic plugins" and other teams are invited to extend the console. Currently, I would expect values like If this escalates I would expect we need to filter them or track just "redhat" vs "marketplace" vs "customer" plugins. But I don't think we have that distinction yet. (cc @spadgett) |
cc @spadgett |
/cc @simonpasquier |
Ok so the anticipated label value sets have me a bit worried or rather the open-endedness. We don't currently check for cardinality increases in telemeter. Even if we did, at that stage it would be too late :) So for this PR, lets change the recording rule in a way so that we really only collect the label values that we need to have in telemetry.
This allows us to protect against future label value proliferation, to reason about the number of timeseries this PR adds and it forces you to actually reason about what data you need. Also fair warning: This looks like it'll add quite a few timeseries, so we'll like have to get @eparis's thumbs up. No big deal, just another admin step. 😀 |
64b5fc2
to
d564ba3
Compare
Oh and in the PR description you have a small error:
That second label is probably called |
@jerolimov: This pull request references ODC-7258 which is a valid jira issue. In response to this:
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. |
Oh yeah, copy and paste error. Fixed that. 😏 |
Thanks for that recommendation @jan--f. 👍 Just FYI: I checked our anonymized analytics where we have an event when a user switches perspective. We have currently only users switching between "admin" and "dev". So this shows that other perspectives aren't really used today (on the clusters we monitoring with frontend analytics). So I'm not worried about too many time series, but I understand that you are. 😄 Is there maybe to group all other perspectives' as "other" or something instead of just dropping them? Does this make sense to you? When there is no easy way I can apply the filter to
Sure, that's fine. |
/retest |
/skip |
/retest |
/retest-required |
/retest |
1 similar comment
/retest |
/test e2e-agnostic-operator |
5 similar comments
/test e2e-agnostic-operator |
/test e2e-agnostic-operator |
/test e2e-agnostic-operator |
/test e2e-agnostic-operator |
/test e2e-agnostic-operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jerolimov, simonpasquier 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:
Approvers can indicate their approval by writing |
/retest-required |
@jerolimov: The following tests failed, say
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. |
@jerolimov: Jira Issue OCPBUGS-12903: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-12903 has been moved to the MODIFIED state. In response to this:
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. |
@jerolimov: Jira Issue OCPBUGS-12903 is in an unrecognized state (ON_QA) and will not be moved to the MODIFIED state. In response to this:
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. |
Fixes: OCPBUGS-12903
This PR adds new console metrics that are collected in ODC-7232 and should make them later available in Superset DataHat or Tableau.
See also PR openshift/console#12527 for detailed information about the collected metrics.
Update: PR openshift/console#12684 reduced the metrics cardinality based on the feedback in this PR here.
A quick overview:
cluster_version_capability
with labelname
is already part of telemetry, and could be used in internal Superset or Tableau, or ❓console_auth_login_requests_total
console_auth_login_successes_total
by labelrole
console_auth_login_failures_total
by labelreason
console_auth_logout_requests_total
by labelreason
Sum CounterVecconsole_usage_total
by labelsevent
andperspective
console_usage_users
by labelrole
Update: with PR OCPBUGS-10956: Reduce metrics cardinality by grouping well-known and other perspectives and plugins console#12684 only used roles was reported.
console_plugins_info
by labelsname
andstate
Name could be just
"redhat"
,"demo"
and"other"
at the momentconsole_customization_perspectives_info
by labelsname
andstate
Name could be just
"admin"
,"dev"
,"acm"
and"other"
An example of what the console
/metrics
endpoint provides: