Skip to content
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

feat: add _id label to all hypershift operator metrics #2991

Merged

Conversation

typeid
Copy link
Member

@typeid typeid commented Sep 7, 2023

What this PR does / why we need it:

This PR adds the _id label to all the hypershift operator metrics that didn't have it yet.
For ROSA, this significantly facilitates promql queries on metrics coming from the hypershift operator and hosted clusters, as the hosted cluster metrics also contain the same _id label.
Currently, to combine metrics from HC and the hypershift operator in a query, we need to aggregate another metric containing both _id and exported_namespace. With this change, we can directly aggregate through _id.

Which issue(s) this PR fixes
Fixes #HOSTEDCP-1183 (note the ticket isn't groomed yet)

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

@openshift-ci openshift-ci bot added the area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release label Sep 7, 2023
@netlify
Copy link

netlify bot commented Sep 7, 2023

Deploy Preview for hypershift-docs ready!

Name Link
🔨 Latest commit 390d8f4
🔍 Latest deploy log https://app.netlify.com/sites/hypershift-docs/deploys/64f96f2642dccd00077f6f61
😎 Deploy Preview https://deploy-preview-2991--hypershift-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@enxebre
Copy link
Member

enxebre commented Sep 7, 2023

/approve
ptal cc @csrwng

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 7, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre, typeid

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 7, 2023
@typeid
Copy link
Member Author

typeid commented Sep 7, 2023

/retest

@csrwng
Copy link
Contributor

csrwng commented Sep 12, 2023

/lgtm

@csrwng csrwng added the lgtm Indicates that a PR is ready to be merged. label Sep 12, 2023
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 5e2cb1d and 2 for PR HEAD 390d8f4 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD b81f7a3 and 1 for PR HEAD 390d8f4 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 13, 2023

@typeid: 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.

@openshift-merge-robot openshift-merge-robot merged commit 046001c into openshift:main Sep 13, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants