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

[receiver/k8sclusterreceiver] add HierarchicalResourceQuota #29673

Closed

Conversation

hiwayama
Copy link
Contributor

@hiwayama hiwayama commented Dec 6, 2023

Description:

Add hnc.x-k8s.io/v1alpha2 HierarchicalResourceQuota as a target for metrics collection (Optional)
https://github.com/kubernetes-sigs/hierarchical-namespaces

A custom controller of HierarchicalResourceQuota has no imformer.
So, I use dynamic informers to collect these metrics.

Link to tracking Issue:
#29555

Testing:
I tested to collect hrq metrics with k8sclusterreceiver and debugexporter in a k8s cluster which is installed HierarchicalResourceQuota
ut: Add test case to collect metrics from hrq in receiver/k8sclusterreceiver/receiver_test.go#TestReceiver

Documentation:
I added some options for hrq.
The default value of hrq sets false.

@hiwayama hiwayama force-pushed the feature/k8sclusterreceiver/hrq branch from 263c955 to 29fad05 Compare December 6, 2023 13:45
@hiwayama hiwayama force-pushed the feature/k8sclusterreceiver/hrq branch 5 times, most recently from 83b6ce9 to 478b4c5 Compare December 6, 2023 16:44
@hiwayama hiwayama force-pushed the feature/k8sclusterreceiver/hrq branch 11 times, most recently from ae3ec9c to 2e8ce0f Compare December 7, 2023 16:15
@hiwayama hiwayama changed the title [wip][k8sclusterreceiver] add hrq [wip][receiver/k8sclusterreceiver] add HierarchicalResourceQuota Dec 7, 2023
@hiwayama hiwayama force-pushed the feature/k8sclusterreceiver/hrq branch from 2e8ce0f to c7ee961 Compare December 7, 2023 16:31
@hiwayama
Copy link
Contributor Author

hiwayama commented Dec 7, 2023

Please code review.

I'm sorry. The label pkg/stanza was given in error. Please remove that label.

@hiwayama hiwayama marked this pull request as ready for review December 7, 2023 17:34
@hiwayama hiwayama requested a review from a team December 7, 2023 17:34
@hiwayama hiwayama changed the title [wip][receiver/k8sclusterreceiver] add HierarchicalResourceQuota [receiver/k8sclusterreceiver] add HierarchicalResourceQuota Dec 7, 2023
@hiwayama hiwayama force-pushed the feature/k8sclusterreceiver/hrq branch from d1ce42e to 4d2b444 Compare December 19, 2023 07:30
@hiwayama hiwayama requested a review from povilasv December 19, 2023 08:34
Copy link
Contributor

github-actions bot commented Jan 3, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jan 3, 2024
@povilasv
Copy link
Contributor

povilasv commented Jan 5, 2024

I think we are also missing some docs around how to use this / RBAC? :)

Sorry for delayed reviews. I will try to also build this and test it early next week.

@hiwayama
Copy link
Contributor Author

hiwayama commented Jan 5, 2024

Thank you for your reponse.

I think we are also missing some docs around how to use this / RBAC? :)

I missed writing docs about how to use and RBAC.
I'll write them.

@hiwayama
Copy link
Contributor Author

hiwayama commented Jan 5, 2024

@povilasv

I think we are also missing some docs around how to use this / RBAC? :)

I added some descriptions about RBAC bfb5b3b

If you would like another method please let me know.
For example, should it be a separate item like descrptions about OpenShift?

@github-actions github-actions bot removed the Stale label Jan 6, 2024
Copy link
Contributor

@povilasv povilasv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me. @TylerHelmuth / @dmitryax would be great if you could take a look

Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @dmitryax @jinja2 would appreciate your review

@@ -497,3 +507,19 @@ metrics:

# k8s.node.allocatable_* metrics (k8s.node.allocatable_cpu, k8s.node.allocatable_memory, etc) are controlled
# by allocatable_types_to_report config option. By default, none of them are reported.
k8s.hierarchical_resource_quota.hard_limit:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
k8s.hierarchical_resource_quota.hard_limit:
k8s.hierarchicalresourcequota.limit:

This will keep the metric names in-line with the attr names which do not have underscore. Are we adding hard_limit explicitly cause there is another type of limit with these (e.g. soft_limit)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refer to the following lines.
Should I change that setting to 'limit'?

k8s.resource_quota.hard_limit:
enabled: true
description: The upper limit for a particular resource in a specific namespace. Will only be sent if a quota is specified. CPU requests/limits will be sent as millicores
unit: "{resource}"
gauge:
value_type: int
attributes:
- resource
k8s.resource_quota.used:
enabled: true
description: The usage for a particular resource in a specific namespace. Will only be sent if a quota is specified. CPU requests/limits will be sent as millicores
unit: "{resource}"
gauge:
value_type: int
attributes:
- resource

@povilasv
Copy link
Contributor

There is an interesting issue about building a way for users to monitor custom CRDS without us adding new dependencies in k8sclusterreceiver - #30655

Maybe we should pursue this instead? 🤔

@hiwayama
Copy link
Contributor Author

Thank you for sharing the issue.
It is better to implement to monitor custom CRD like KSM.

It's OK to close this PR.

Copy link
Contributor

github-actions bot commented Feb 6, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 6, 2024
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants